rtpjitterbuffer: don't use RTX packets in rate-calc and reset-logic

The problem was this:

Due to the highly irregular arrival of RTX-packet the max-misorder variable
could be pushed very low. (-10).

If you then at some point get a big in the sequence-numbers (62 in the
test) you end up sending RTX-requests for some of those packets, and then
if the sender answers those requests, you are going to get a bunch of
RTX-packets arriving. (-13 and then 5 more packets in the test)

Now, if max-misorder is pushed very low at this point, these RTX-packets
will trigger the handle_big_gap_buffer() logic, and because they arriving
so neatly in order, (as they would, since they have been requested like
that), the gst_rtp_jitter_buffer_reset() will be called, and two things
will happen:
1. priv->next_seqnum will be set to the first RTX packet
2. the 5 RTX-packet will be pushed into the chain() function

However, at this point, these RTX-packets are no longer valid, the
jitterbuffer has already pushed lost-events for these, so they will now
be dropped on the floor, and never make it to the waiting loop-function.

And, since we now have a priv->next_seqnum that will never arrive
in the loop-function, the jitterbuffer is now stalled forever, and will
not push out another buffer.

The proposed fixes:
1. Don't use RTX in calculation of the packet-rate.
2. Don't use RTX in large-gap logic, as they are likely to be dropped.

1 files changed, 25 insertions, 0 deletions

diff --git a/tests/check/elements/rtpjitterbuffer.c b/tests/check/elements/rtpjitterbuffer.c
index f055b2243..99cc57ca0 100644
--- a/tests/check/elements/rtpjitterbuffer.c
+++ b/tests/check/elements/rtpjitterbuffer.c
@@ -3126,6 +3126,29 @@ GST_START_TEST (test_multiple_lost_do_not_stall)
 
 GST_END_TEST;
 
+GST_START_TEST (test_reset_using_rtx_packets_does_not_stall)
+{
+  GstHarness *h = gst_harness_new ("rtpjitterbuffer");
+  BufferArrayCtx bufs[] = {
+    /* *INDENT-OFF* */
+    {  1,    1 * TEST_RTP_TS_DURATION, FALSE, 2000000},
+    {  62,  62 * TEST_RTP_TS_DURATION, FALSE, 0},
+    { -13, -13 * TEST_RTP_TS_DURATION, TRUE, 10000},
+    {   1,   1 * TEST_RTP_TS_DURATION, TRUE, 0},
+    {   1,   1 * TEST_RTP_TS_DURATION, TRUE, 0},
+    {   1,   1 * TEST_RTP_TS_DURATION, TRUE, 0},
+    {   1,   1 * TEST_RTP_TS_DURATION, TRUE, 0},
+    {   1,   1 * TEST_RTP_TS_DURATION, TRUE, 0},
+    /* *INDENT-ON* */
+  };
+
+  g_object_set (h->element, "latency", 400,
+      "do-retransmission", TRUE, "do-lost", TRUE, "max-misorder-time", 1, NULL);
+  fail_unless (check_for_stall (h, bufs, G_N_ELEMENTS (bufs)));
+  gst_harness_teardown (h);
+}
+
+GST_END_TEST;
 
 static Suite *
 rtpjitterbuffer_suite (void)
@@ -3196,6 +3219,8 @@ rtpjitterbuffer_suite (void)
 
   tcase_add_test (tc_chain, test_reset_timers_does_not_stall);
   tcase_add_test (tc_chain, test_multiple_lost_do_not_stall);
+  tcase_add_test (tc_chain, test_reset_using_rtx_packets_does_not_stall);
+
 
   return s;
 }