Commit graph

60 commits

Author SHA1 Message Date
Tim-Philipp Müller
781b5ac781 tests: rtpjitterbuffer: fix compiler warning due to c99-ism
rtpjitterbuffer.c:592:3: error: ‘for’ loop initial declarations are only allowed in C99 mode
2017-01-09 19:04:04 +00:00
Havard Graff
0a81f71df5 tests/jitterbuffer: Major refactoring and cleanups
* Changed PCMU->TEST for common macros
* Changed verify-functions (lost & rtx) into macros.
* Remove option to add marker-bit for test-buffers (not used anywhere)
* Add new push_test_buffer function that makes sure there are correlation
  between dts and the time on the clock. (classic test-mistake)
* Established a generic starting-point for tests with the
  construct_deterministic_initial_state function and use it where
  applicable, which removes lots of "boilerplate" everywhere.
* Add basic lost-event test
* Remove as much "magic constants" as possible.
* Remove 3 tests that no longer are testing anything that others don't,
  and was completely unmaintainable.
* Remove unnecessary use of the testclock
* Verify each test is testing what it actually says it does (and modify
  where it doesn't)

In general, make the tests much smaller, better, more maintainable and
readable.

https://bugzilla.gnome.org/show_bug.cgi?id=774409
2016-12-14 15:00:37 +02:00
Sebastian Dröge
63938ef730 gst: Don't declare variables inside the for loop header
This is a C99 feature.
2016-12-13 22:32:46 +02:00
Havard Graff
1a4393fb4d rtpjitterbuffer: fix timer-reuse bug
When doing rtx, the jitterbuffer will always add an rtx-timer for the next
sequence number.

In the case of the packet corresponding to that sequence number arriving,
that same timer will be reused, and simply moved on to wait for the
following sequence number etc.

Once an rtx-timer expires (after all retries), it will be rescheduled as
a lost-timer instead for the same sequence number.

Now, if this particular sequence-number now arrives (after the timer has
become a lost-timer), the reuse mechanism *should* now set a new
rtx-timer for the next sequence number, but the bug is that it does
not change the timer-type, and hence schedules a lost-timer for that
following sequence number, with the result that you will have a very
early lost-event for a packet that might still arrive, and you will
never be able to send any rtx for this packet.

Found by Erlend Graff - erlend@pexip.com

https://bugzilla.gnome.org/show_bug.cgi?id=773891
2016-11-04 16:56:56 +02:00
Havard Graff
fb9c75db36 rtpjitterbuffer: fix lost-event using dts instead of pts
The lost-event was using a different time-domain (dts) than the outgoing
buffers (pts). Given certain network-conditions these two would become
sufficiently different and the lost-event contained timestamp/duration
that was really wrong. As an example GstAudioDecoder could produce
a stream that jumps back and forth in time after receiving a lost-event.

The previous behavior calculated the pts (based on the rtptime) inside the
rtp_jitter_buffer_insert function, but now this functionality has been
refactored into a new function rtp_jitter_buffer_calculate_pts that is
called much earlier in the _chain function to make pts available to
various calculations that wrongly used dts previously
(like the lost-event).

There are however two calculations where using dts is the right thing to
do: calculating the receive-jitter and the rtx-round-trip-time, where the
arrival time of the buffer from the network is the right metric
(and is what dts in fact is today).

The patch also adds two tests regarding B-frames or the
“rtptime-going-backwards”-scenario, as there were some concerns that this
patch might break this behavior (which the tests shows it does not).
2016-11-04 16:51:20 +02:00
Havard Graff
bea35f97c8 rtpjitterbuffer: fix bug in reschedule_timer
The new timeout is always going to be (timeout + delay), however, the
old behavior compared the current timeout to just (timeout), basically
being (delay) off.

This would happen if rtx-delay == rtx-retry-timeout, with the result that
a second rtx attempt for any buffers would be scheduled immediately instead
of after rtx-delay ms.

Simply calculate (new_timeout = timeout + delay) and then use that instead.

https://bugzilla.gnome.org/show_bug.cgi?id=773905
2016-11-04 16:40:14 +02:00
Tim-Philipp Müller
e6d188967a tests: fix indentation 2016-09-15 09:53:07 +01:00
Havard Graff
f440b074b1 rtpjitterbuffer: improved rtx-rtt averaging
The basic idea is this:
1. For *larger* rtx-rtt, weigh a new measurement as before
2. For *smaller* rtx-rtt, be a bit more conservative and weigh a bit less
3. For very large measurements, consider them "outliers"
   and count them a lot less

The idea being that reducing the rtx-rtt is much more harmful then
increasing it, since we don't want to be underestimating the rtt of the
network, and when using this number to estimate the latency you need for
you jitterbuffer, you would rather want it to be a bit larger then a bit
smaller, potentially losing rtx-packets. The "outlier-detector" is there
to prevent a single skewed measurement to affect the outcome too much.
On wireless networks, these are surprisingly common.

https://bugzilla.gnome.org/show_bug.cgi?id=769768
2016-09-14 19:37:50 -04:00
Stian Selnes
f8238f0a9f rtpjitterbuffer: Detect whether to assume equidistant spacing when loss
Assuming equidistant packet spacing when that's not true leads to more
loss than necessary in the case of reordering and jitter. Typically this
is true for video where one frame often consists of multiple packets
with the same rtp timestamp. In this case it's better to assume that the
missing packets have the same timestamp as the last received packet, so
that the scheduled lost timer does not time out too early causing the
packets to be considered lost even though they may arrive in time.

https://bugzilla.gnome.org/show_bug.cgi?id=769768
2016-09-14 19:37:50 -04:00
Stian Selnes
2eb7383816 rtpjitterbuffer: Don't request rtx if 'now' is past retry period
There is no need to schedule another EXPECTED timer if we're already
past the retry period. Under normal operation this won't happen, but if
there are more timers than the jitterbuffer is able to process in
real-time, scheduling more timers will just make the situation worse.
Instead, consider this packet as lost and move on. This scenario can
occur with high loss rate, low rtt and high configured latency.

https://bugzilla.gnome.org/show_bug.cgi?id=769768
2016-09-14 19:37:50 -04:00
Stian Selnes
ab49dfd0b2 rtpjitterbuffer: Fix lost duration when gap after lost timer
This patch fixes an issue with the estimated gap duration when there is
a gap immediately after a lost timer has been processed. Previously
there was a discrepancy beteen the gap in seqnum and gap in dts which
would cause wrong calculated duration. The issue would only be seen with
retranmission enabled since when it's disabled lost timers are only
created when a packet is received and the actual gap length and last dts
is known.

https://bugzilla.gnome.org/show_bug.cgi?id=769768
2016-09-14 19:37:50 -04:00
Havard Graff
8087a8a31c rtpjitterbuffer: Improved expected-timer handling when gap > 0
https://bugzilla.gnome.org/show_bug.cgi?id=769768
2016-09-14 19:37:50 -04:00
Stian Selnes
38a7545003 rtpjitterbuffer: Major improvements for RTX stats
Stats should also be collected for unsuccessful packets.

rtx-rtt is very important for determining the necessary configured
latency on the jitterbuffer. It's especially important to be able to
increase the latency when retransmitted packets arrive too late and are
considered lost. This patch includes these late packets in the
calculation of the various rtx stats, making them more correct and
useful.

Also in the case where the original packet arrives after a NACK is sent,
the received RTX packet should update the stats since it provides useful
information about RTT.

The RTT is only updated if and only if all requested retranmissions are
received. That way the RTT is guaranteed to make sense. If not we don't
know which request the packet is a response to and the RTT may be bogus.
A consequence of this patch is that RTT is not updated for a request
when one of the RTX packets for that seqnum is lost, but that since
measured RTT will be more accurate.

The implementation store the RTX information from the timed out timers
and use this when the retransmitted packet arrives. For performance
these timers are stored separately from the "normal" timers in order to
not impact performance (see attached performance test).

https://bugzilla.gnome.org/show_bug.cgi?id=769768
2016-09-14 19:37:50 -04:00
Havard Graff
1b868cc9b1 rtpjitterbuffer: Add and expose more stats and increase testing of it
Add num-pushed and num-lost.
Expose num-late, num-duplicates and avg-jitter.

https://bugzilla.gnome.org/show_bug.cgi?id=769768
2016-09-14 19:37:50 -04:00
Sebastian Dröge
a1eefe23de rtpjitterbuffer: Fix unit test by disabling adaptive misorder/dropout calculations
Need to set max-misorder-time and max-dropout-time to 0 so the
jitterbuffer does not base them on packet rate calculations.
If it does, out gap is big enough to be considered a new stream and
we wait for a few consecutive packets just to be sure

https://bugzilla.gnome.org/show_bug.cgi?id=751311
2016-08-18 09:58:58 +03:00
Havard Graff
8f7962e1c3 rtpjitterbuffer: Fix stall when receiving already lost packet
When a packet arrives that has already been considered lost as part of a
large gap the "lost timer" for this will be cancelled. If the remaining
packets of this large gap never arrives, there will be missing entries
in the queue and the loop function will keep waiting for these packets
to arrive and never push another packet, effectively stalling the
pipeline.

The proposed fix conciders parts of a large gap definitely lost (since
they are calculated from latency) and ignores the late arrivals.

In practice the issue is rare since large gaps are scheduled immediately,
and for the stall to happen the late arrival needs to be processed
before this times out.

https://bugzilla.gnome.org/show_bug.cgi?id=765933
2016-05-06 14:32:42 +03:00
Tim-Philipp Müller
7335d03070 tests: fix indentation 2016-02-19 14:44:11 +00:00
Havard Graff
69436d5a61 tests: rtpjitterbuffer: port testharness to GstHarness and cleanup/improve
Probably found a bug as well, in that there are some timestamps in
there that are looking very wrong. (marked with FIXME)

https://bugzilla.gnome.org/show_bug.cgi?id=762267
2016-02-19 14:44:02 +00:00
Havard Graff
d52765fabb tests: rtpjitterbuffer: test cleanups/improvements
Use fail_unless and friends instead of g_assert
Factor seq-num checking out to separate function
Check more return-values from push and crank and others

https://bugzilla.gnome.org/show_bug.cgi?id=762254
2016-02-19 11:26:45 +00:00
Stian Selnes
fb4c2909ca tests: rtpjitterbuffer: fix leaks in unit test
https://bugzilla.gnome.org/show_bug.cgi?id=762214
2016-02-19 11:07:52 +00:00
Stian Selnes
3eeca9c7d2 rtpjitterbuffer: Add test for big seqnum gap handling
Make sure that the packets queued when detecting a big gap are pushed
after reset (5 consective seqnums) and not dropped.

https://bugzilla.gnome.org/show_bug.cgi?id=762211
2016-02-18 09:39:01 +02:00
Thiago Santos
241e0c2722 rtpjitterbuffer: fix build error with gcc (Debian 4.9.2-21) 4.9.2
Replace static constants with macros to make gcc happy

  CC       elements/elements_rtpjitterbuffer-rtpjitterbuffer.o
elements/rtpjitterbuffer.c:387:1: error: initializer element is not constant
 static const GstClockTime PCMU_BUF_DURATION = PCMU_BUF_MS * GST_MSECOND;
 ^
elements/rtpjitterbuffer.c:388:1: error: initializer element is not constant
 static const guint PCMU_BUF_SIZE = 64000 * PCMU_BUF_MS / 1000;
 ^
elements/rtpjitterbuffer.c:390:5: error: initializer element is not constant
     PCMU_BUF_CLOCK_RATE * PCMU_BUF_MS / 1000;
2015-07-08 23:49:12 -03:00
Thiago Santos
3edf9e4f58 rtpjitterbuffer: run indent and fix some comments
Fix indent on this file and break some comment lines into two to make
it fit 80 chars per line
2015-07-08 23:49:09 -03:00
Havard Graff
ddd032f56b rtpjitterbuffer: fix gap-time calculation and remove "late"
The amount of time that is completely expired and not worth waiting for,
is the duration of the packets in the gap (gap * duration) - the
latency (size) of the jitterbuffer (priv->latency_ns). This is the duration
that we make a "multi-lost" packet for.

The "late" concept made some sense in 0.10 as it reflected that a buffer
coming in had not been waited for at all, but had a timestamp that was
outside the jitterbuffer to wait for. With the rewrite of the waiting
(timeout) mechanism in 1.0, this no longer makes any sense, and the
variable no longer reflects anything meaningful (num > 0 is useless,
the duration is what matters)

Fixed up the tests that had been slightly modified in 1.0 to allow faulty
behavior to sneak in, and port some of them to use GstHarness.

https://bugzilla.gnome.org/show_bug.cgi?id=738363
2015-07-08 23:18:48 +03:00
Sebastian Dröge
3df0cce65d rtpjitterbuffer: If possible, always update the current time before looping over all timers
If we have a clock, update "now" now with the very latest running time we have.
If timers are unscheduled below we otherwise wouldn't update now (it's only updated
when timers expire), and also for the very first loop iteration now would otherwise
always be 0.

Also the time is used for the timeout functions, e.g. to calculate any times
for the next timeouts and we would otherwise pass too old times there.

https://bugzilla.gnome.org/show_bug.cgi?id=751636
2015-07-02 16:45:59 +02:00
Sebastian Dröge
91c8688ed7 rtpjitterbuffer: Fix RTX unit test
The calculations were a bit off everywhere, even before the changes done
recently to the delay for RTX of expected future packets. It only worked by
accident, but now the calculations are all correct again. Hopefully.
2015-04-27 16:37:23 +02:00
Wim Taymans
13804eab7d check: add jitterbuffer unit test
See https://bugzilla.gnome.org/show_bug.cgi?id=745539
2015-03-06 11:40:53 +01:00
Wim Taymans
b2e1598e4a rtpjitterbuffer: increment accepted packets after loss
When we detect a lost packet, expect packets with higher
seqnum on the input.

Also update the unit test.

Fixes https://bugzilla.gnome.org/show_bug.cgi?id=729524
2014-05-09 18:10:32 +02:00
Jason Litzinger
9068e1bb8e Add new test case. 2014-05-09 18:10:32 +02:00
Sebastian Dröge
02e62c139d rtpjitterbuffer: Fix hundreds of memory leaks in the test 2014-04-17 17:26:36 +02:00
Wim Taymans
eee515cb2c rtpjitterbuffer: serialize events in the buffer
Serialize events into the jitterbuffer by inserting them with a -1
seqnum.
Update unit test to expect events from the streaming thread.

Fixes https://bugzilla.gnome.org/show_bug.cgi?id=652986
2013-12-10 11:57:37 +01:00
Wim Taymans
29d9b1e7de check: fix jitterbuffer check
Don't advance the clock to 240ms too early.

Fixes https://bugzilla.gnome.org/show_bug.cgi?id=710013
2013-11-25 17:39:52 +01:00
Wim Taymans
710d1f3a2a rtpjitterbuffer: improve clear-pt-map handling
Don't reset the expected output seqnum when clearing the pt map because this
could stall the jitterbuffer forever.
Add a unit test for this.

Fixes https://bugzilla.gnome.org/show_bug.cgi?id=709800
2013-11-25 15:52:22 +01:00
George Kiagiadakis
387e3b918a rtpjitterbuffer: Fix stats property field names and documentation 2013-11-15 16:23:34 +02:00
Torrie Fischer
22ceb80ba9 rtpjitterbuffer: implement rtx statistics 2013-11-14 09:24:26 +01:00
Wim Taymans
3c69d65b85 tests: add test for retransmission because of reordering 2013-09-23 14:45:27 +02:00
Wim Taymans
f40d6689f2 tests: remove timeouts from check
Timeouts make the test unreliable and are not needed.
2013-09-23 14:45:26 +02:00
Wim Taymans
a71014518c tests: add test for packet delay and retransmission 2013-09-23 14:45:24 +02:00
Wim Taymans
c959cdc8c1 tests: check both PTS and DTS 2013-09-23 14:45:24 +02:00
Wim Taymans
2e3346eff9 tests: add unit-test for multiple missing packets
Check if multiple missing packets generate retransmission events and that the
retranmission requests are canceled when the missing packet arrives.
2013-09-23 14:45:24 +02:00
Wim Taymans
3b9e2ee5a0 tests: improve debug 2013-09-23 14:45:23 +02:00
Wim Taymans
ee817363c7 tests: add retransmission jitterbuffer test
Store both DTS and PTS on buffers.
Make a queue for srcpad events.
Activate pads after linking so that we don't get RECONFIGURE events.
Add test for retransmission.
2013-09-19 14:32:48 +02:00
Wim Taymans
ee75d4b01e tests: fix comments 2013-09-18 15:08:45 +02:00
Wim Taymans
5c1f2f0045 rtpjitterbuffer: add another test
The test is modified slightly because the late lost packets are only
generated now when a large gap is received.
2013-09-18 14:23:55 +02:00
Wim Taymans
5d5fc03e04 rtpjitterbuffer: round gap duration to multiple of duration
Make sure the gap duration in the lost event is a multiple of the packet
duration.
Enable another test.
2013-09-18 14:12:47 +02:00
Wim Taymans
6e4a051d40 rtpjitterbuffer: keep track of duration
Keep track of the estimated duration of missing packets and use it in the lost
event.
Enable another unit test
2013-09-18 12:29:38 +02:00
Wim Taymans
ac3bb3acf6 rtpjitterbuffer: handle large gaps with one lost event
When we have a large number of missing packets, generate one lost event for all
the packets that have no chance of being pushed out in time.
Fix and activate unit test for large gaps.
2013-09-18 11:59:28 +02:00
Nicolas Dufresne
7719b19aec rtpjitterbuffer-test: Send stream-start before caps followed by segment 2013-05-13 15:36:49 +02:00
Wim Taymans
50391c7773 check: add (but disable) more rtp jitterbuffer tests
Tests need to be ported to 1.0 before they can be enabled but added here so they
don't get forgotten.

See https://bugzilla.gnome.org/show_bug.cgi?id=667838
2012-12-13 12:36:20 +01:00
Tim-Philipp Müller
230cf41cc9 Fix FSF address
https://bugzilla.gnome.org/show_bug.cgi?id=687520
2012-11-04 00:07:18 +00:00