rtpjitterbuffer: immediately insert a lost-event on multiple lost packets

There is a problem with the code today, where a single timer will
be scheduled for a series of lost packets, and then if the first packet
in that series arrives, it will cause a rescheduling of that timer, going
from a "multi"-timer to a single-timer, causing a lot of the packets
in that timer to be unaccounted for, and creating a situation in where
the jitterbuffer will never again push out another packet.

This patch solves the problem by instead of scheduling those lost packets
as another timer, it instead asks to have that lost-event pushed straight
out.

This very much goes with the intent of the code here: These packets are
so desperately late that no cure exists, and we might as well get the
lost-event out of the way and get on with it.

This change has some interesting knock-on effect being presented in
later commits. It completely removes the concept of "already-lost", so
that is why that test has been disabled in this commit, to be
removed later.

1 files changed, 67 insertions, 1 deletions

diff --git a/tests/check/elements/rtpjitterbuffer.c b/tests/check/elements/rtpjitterbuffer.c
index a8ce9c369..fd4d27135 100644
--- a/tests/check/elements/rtpjitterbuffer.c
+++ b/tests/check/elements/rtpjitterbuffer.c
@@ -3096,6 +3096,71 @@ GST_START_TEST (test_reset_does_not_stall)
 
 GST_END_TEST;
 
+typedef struct
+{
+  guint16 seqnum;
+  guint32 rtptime;
+  gint sleep_ms;
+} PushBufferCtx;
+
+GST_START_TEST (test_multiple_lost_do_not_stall)
+{
+  GstHarness *h = gst_harness_new ("rtpjitterbuffer");
+  gint latency_ms = 200;
+  guint inital_bufs = latency_ms / TEST_BUF_MS;
+  guint max_dropout_time = 10;
+  guint16 i;
+  guint16 seqnum = 1000;
+  guint32 rtptime = seqnum * TEST_RTP_TS_DURATION;
+  guint in_queue;
+  PushBufferCtx bufs[] = {
+    {1039, 166560, 58},
+    {1011, 161280, 1000},
+  };
+  gint size = G_N_ELEMENTS (bufs);
+
+  gst_harness_use_systemclock (h);
+  gst_harness_set_src_caps (h, generate_caps ());
+
+  g_object_set (h->element, "latency", latency_ms, "do-retransmission", TRUE,
+      "do-lost", TRUE, "rtx-max-retries", 2,
+      "max-dropout-time", max_dropout_time, NULL);
+
+  /* push initial buffers and pull them out as well */
+  for (i = 0; i < inital_bufs; i++) {
+    seqnum += 1;
+    rtptime += TEST_RTP_TS_DURATION;
+    push_test_buffer_now (h, seqnum, rtptime);
+    g_usleep (G_USEC_PER_SEC / 1000 * 20);
+  }
+  for (i = 0; i < inital_bufs; i++) {
+    gst_buffer_unref (gst_harness_pull (h));
+  }
+
+  /* push buffers according to list */
+  for (i = 0; i < size; i++) {
+    push_test_buffer_now (h, bufs[i].seqnum, bufs[i].rtptime);
+    g_usleep (G_USEC_PER_SEC / 1000 * bufs[i].sleep_ms);
+    seqnum = MAX (bufs[i].seqnum, seqnum);
+  }
+
+  in_queue = gst_harness_buffers_in_queue (h);
+
+  /* and then normal buffers again */
+  for (i = 0; i < 5; i++) {
+    seqnum += 1;
+    push_test_buffer_now (h, seqnum, seqnum * TEST_RTP_TS_DURATION);
+    g_usleep (G_USEC_PER_SEC / 1000 * 20);
+  }
+
+  /* we expect at least some of those buffers to come through */
+  fail_unless (gst_harness_buffers_in_queue (h) != in_queue);
+
+  gst_harness_teardown (h);
+}
+
+GST_END_TEST;
+
 
 static Suite *
 rtpjitterbuffer_suite (void)
@@ -3161,11 +3226,12 @@ rtpjitterbuffer_suite (void)
   tcase_add_test (tc_chain, test_performance);
 
   tcase_add_test (tc_chain, test_drop_messages_too_late);
-  tcase_add_test (tc_chain, test_drop_messages_already_lost);
+  tcase_skip_broken_test (tc_chain, test_drop_messages_already_lost);
   tcase_add_test (tc_chain, test_drop_messages_drop_on_latency);
   tcase_add_test (tc_chain, test_drop_messages_interval);
 
   tcase_add_test (tc_chain, test_reset_does_not_stall);
+  tcase_add_test (tc_chain, test_multiple_lost_do_not_stall);
 
   return s;
 }