From a24596423a6201c91bface3b3ff2133587dd8bf9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Olivier=20Cr=C3=AAte?= <olivier.crete@collabora.com>
Date: Thu, 26 Sep 2019 18:39:48 -0400
Subject: [PATCH] rtpjitterbuffer: Cancel timers instead of just unlocking loop
 thread

When the queue is full (and adding more packets would risk a seqnum
roll-over), the best approach is to just start pushing out packets
from the other side.  Just pushing out the packets results in the
timers being left hanging with old seqnums, so it's safer to just
execute them immediately in this case. It does limit the timer space
to the time it takes to receiver about 32k packets, but without
extended sequence number, this is the best RTP can do.

This also results in the test no longer needed to have timeouts or
timers as pushing packets in drives everything.

Fixes #619
---
 gst/rtpmanager/gstrtpjitterbuffer.c    |  9 +++++-
 tests/check/elements/rtpjitterbuffer.c | 45 ++++----------------------
 2 files changed, 15 insertions(+), 39 deletions(-)

diff --git a/gst/rtpmanager/gstrtpjitterbuffer.c b/gst/rtpmanager/gstrtpjitterbuffer.c
index 85d7846364..22e6451d63 100644
--- a/gst/rtpmanager/gstrtpjitterbuffer.c
+++ b/gst/rtpmanager/gstrtpjitterbuffer.c
@@ -2939,8 +2939,15 @@ gst_rtp_jitter_buffer_chain (GstPad * pad, GstObject * parent,
    * sequence number, let's allow at least 10k packets in any case. */
   while (rtp_jitter_buffer_is_full (priv->jbuf) &&
       priv->srcresult == GST_FLOW_OK) {
+    RtpTimer *timer = rtp_timer_queue_peek_earliest (priv->timers);
+    while (timer) {
+      timer->timeout = -1;
+      if (timer->type == RTP_TIMER_DEADLINE)
+        break;
+      timer = rtp_timer_get_next (timer);
+    }
+
     update_current_timer (jitterbuffer);
-    JBUF_SIGNAL_EVENT (priv);
     JBUF_WAIT_QUEUE (priv);
   }
   if (priv->srcresult != GST_FLOW_OK)
diff --git a/tests/check/elements/rtpjitterbuffer.c b/tests/check/elements/rtpjitterbuffer.c
index 1ce01ce3dc..cd58c52812 100644
--- a/tests/check/elements/rtpjitterbuffer.c
+++ b/tests/check/elements/rtpjitterbuffer.c
@@ -2647,65 +2647,34 @@ GST_START_TEST (test_performance)
 
 GST_END_TEST;
 
-static gpointer
-generate_harness_buffer (gpointer data)
-{
-  GstHarness *h = data;
-  guint i;
-
-  for (i = 32766; i < 41000; i++)
-    gst_harness_push (h, generate_test_buffer (1000 + i));
-
-  return NULL;
-}
-
-
 GST_START_TEST (test_fill_queue)
 {
   GstHarness *h = gst_harness_new ("rtpjitterbuffer");
   const gint num_consecutive = 40000;
-  gint i;
-  GstSegment segment;
-  GThread *t;
   GstBuffer *buf;
+  gint i;
+
+  gst_harness_use_testclock (h);
 
-  gst_segment_init (&segment, GST_FORMAT_TIME);
   gst_harness_set_src_caps (h, generate_caps ());
 
   gst_harness_play (h);
 
   gst_harness_push (h, generate_test_buffer (1000));
-  gst_harness_push (h, generate_test_buffer (1002));
-
-  for (i = 3; i < 32766; i++)
+  /* Skip 1001 */
+  for (i = 2; i < num_consecutive; i++)
     gst_harness_push (h, generate_test_buffer (1000 + i));
 
-  t = g_thread_new ("fill-queue-test-push", generate_harness_buffer, h);
-
-  /* Just give a chance to the thread to start and to try to push one packet */
-  g_usleep (100 * 1000);
-
-  fail_unless (gst_harness_crank_single_clock_wait (h));
-
   buf = gst_harness_pull (h);
   fail_unless_equals_int (1000, get_rtp_seq_num (buf));
   gst_buffer_unref (buf);
-
-  /* Gap at 1001 here */
-  fail_unless (gst_harness_crank_single_clock_wait (h));
-
-  buf = gst_harness_pull (h);
-  fail_unless_equals_int (1002, get_rtp_seq_num (buf));
-  gst_buffer_unref (buf);
-
-  for (i = 3; i < num_consecutive; i++) {
+  /* 1001 is skipped */
+  for (i = 2; i < num_consecutive; i++) {
     GstBuffer *buf = gst_harness_pull (h);
     fail_unless_equals_int (1000 + i, get_rtp_seq_num (buf));
     gst_buffer_unref (buf);
   }
 
-  g_thread_join (t);
-
   gst_harness_teardown (h);
 }