There's no reason to include the padding length in the rtp_info
struct passed to the RTP tap.
The struct already contains the entire raw data, the length of that
raw data, whether the raw data was truncated, the offset where the
payload begins, and the payload length. That is sufficient to deduce
the length of any padding, whether the payload length provided is
the length with or without the padding. (We also include whether
the padding bit was set, which is currently unused; it possibly
could be used to distinguish between having no payload because
padding was indicated but had a bogus value and having no payload
for some other reason.)
Since in practice we care about the payload without the padding,
pass in the length without padding rather than checking and
subtracting the padding each time. This helps avoid accidentally
including padding when processing payload. If for some reason we
need the padding value, calculate it.
As an example of the pitfalls in including the padding in the
payload length, after commits 2b072b8e76
and 4e5f0456c6 moved saving the
raw RTP payload from the RTP Stream Analysis dialog to the
RTP Player, RTP padding started being included when saving the
raw payload, which is not desired. This change restores the
previous behavior of not including the RTP padding.
The RTP player has always sent RTP padding to the audio codecs,
even the GTK+ player when it existed. This fixes that as well.
After 15013ab136, the expected
time of arrival is compared to the previous packet, in an effort
to handle clock changes better. If we're doing so, then there's
some chance that the expected time arrival moves backwards. That's
not a problem for the calculation, so long as we cast to signed
integers at some point.
Previously, the nominal and arrival times are calculated based on first packet
in the RTP stream, but there is a corner case: if the stream codec changes in the
middle, e.g., from AMR-WB to AMR, the nominal time will be calculated using the
current codec frequency, and it is not correct and will affect diff and jitter.
This fix will calculate nominal and arrival times based on previous in-sequence
RTP packet.
Remove the editor modeline blocks from the source files in ui that use 4
space indentation by running
perl -i -p0e 's{ \n+ /[ *\n]+ editor \s+ modelines .* shiftwidth= .* \*/ \s+ } {\n}gsix' $( ag -l shiftwidth=4 $( ag -g '\.(c|cpp|h|m|mm)') )
This gives us one source of indentation truth for these files, and it
*shouldn't* affect anyone since
- These files match the default in our top-level .editorconfig.
- The one notable editor that's likely to be used on these files and
*doesn't* support EditorConfig (Qt Creator) defaults to 4 space
indentation.
The patch changes:
- Removed first_packet_mac_addr staff. It was commented out many years ago...
- Removed delta_timestamp item. Not used.
- Sequence verification takes into account timestamp therefore it is able to detect delayed packets more clearly. As consequence of it, #16330 is solved.
- If packet is delayed, it is not used in calculation of diff/jitter/skew. It just mess output. As consequence of it, #16330 is solved.
I checked output with many RTP streams and looks well. But I have no
sample with wrapped timestamp and I have just a few samples with
missing/reordered packets. Nevertheless all are calculated same way as
before and #16330 is solved too.
The drift values should use the relative time (arrivaltime) instead
of the absolute time (current_time) otherwise, the values are wrong.
Bug: 16343
Change-Id: Icdc65476ab68ce51088314b7c9de939c86472ae9
Reviewed-on: https://code.wireshark.org/review/35908
Reviewed-by: Aymeric Moizard <amoizard@gmail.com>
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Changes:
- rtpstream_packet renamed to rtpstream_packet_cb to follow *_cb pattern
- variables/types used in iax2_analysis_dialog were created as copy of *rtp* ones, but names were left as *rtp* -> *iax2*
- struct _rtp_stream_info replaced with rtp_stream_info_t
- there was tap-rtp-analysis.h, but no tap-rtp-analysis.c - related content was moved from tap-rtp-common.c
- *rtp_stream* functions renamed to *rtpstream*
- renamed rtp_stream_info_t to rtpstream_info_t to follow *rtpstream* pattern.
- renamed ui/rtp_stream.c rtpstream_draw -> rtpstream_draw_cb
Change-Id: Ib11ff5367cc464ea1b0c73432bc50b0eb9cd203e
Reviewed-on: https://code.wireshark.org/review/28299
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Moshe Kaplan