rtpjitterbuffer: fixed stall on gap when using rtx

Co-authored-by: Håvard Graff <havard@pexip.com>
Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/1055>

diff --git a/gst/rtpmanager/gstrtpjitterbuffer.c b/gst/rtpmanager/gstrtpjitterbuffer.c
index 8f318da..db43cc8 100644 (file)
--- a/gst/rtpmanager/gstrtpjitterbuffer.c
+++ b/gst/rtpmanager/gstrtpjitterbuffer.c
@@ -2498,7 +2498,8 @@ gst_rtp_jitter_buffer_handle_missing_packets (GstRtpJitterBuffer * jitterbuffer,
       guint lost_packets;
       GstClockTime lost_duration;
       GstClockTimeDiff gap_time;
-      guint saveable_packets = 0;
+      guint max_saveable_packets = 0;
+      GstClockTime max_saveable_duration;
       GstClockTime saveable_duration;
 
       /* gap time represents the total duration of all missing packets */
@@ -2507,17 +2508,20 @@ gst_rtp_jitter_buffer_handle_missing_packets (GstRtpJitterBuffer * jitterbuffer,
       /* based on the estimated packet duration, we
          can figure out how many packets we could possibly save */
       if (est_pkt_duration)
-        saveable_packets = offset / est_pkt_duration;
+        max_saveable_packets = offset / est_pkt_duration;
 
       /* and say that the amount of lost packet is the sequence-number
          gap minus these saveable packets, but at least 1 */
-      lost_packets = MAX (1, (gint) gap - (gint) saveable_packets);
+      lost_packets = MAX (1, (gint) gap - (gint) max_saveable_packets);
 
-      /* now we know how many packets we can actually save */
-      saveable_packets = gap - lost_packets;
+      /* now we know how many packets we can possibly save */
+      max_saveable_packets = gap - lost_packets;
 
       /* we convert that to time */
-      saveable_duration = saveable_packets * est_pkt_duration;
+      max_saveable_duration = max_saveable_packets * est_pkt_duration;
+
+      /* determine the actual amount of time we can save */
+      saveable_duration = MIN (max_saveable_duration, gap_time);
 
       /* and we now have the duration we need to fill */
       lost_duration = GST_CLOCK_DIFF (saveable_duration, gap_time);

diff --git a/tests/check/elements/rtpjitterbuffer.c b/tests/check/elements/rtpjitterbuffer.c
index a918630..88baccf 100644 (file)
--- a/tests/check/elements/rtpjitterbuffer.c
+++ b/tests/check/elements/rtpjitterbuffer.c
@@ -3257,6 +3257,21 @@ check_for_stall (GstHarness * h, BufferArrayCtx * bufs, guint num_bufs)
   buffer_array_push (h, array, base_seqnum, base_rtptime);
   g_array_unref (array);
 
+  {
+    gint64 start_time = g_get_monotonic_time ();
+    gint64 timeout_s = 30;
+    while (gst_harness_buffers_in_queue (h) <= in_queue) {
+
+      gint64 duration_s =
+          (g_get_monotonic_time () - start_time) / G_USEC_PER_SEC;
+      if (duration_s > timeout_s)
+        break;
+
+      g_usleep (G_USEC_PER_SEC / 100);
+    }
+  }
+
+
   /* we expect at least some of those buffers to come through */
   return gst_harness_buffers_in_queue (h) > in_queue;
 }
@@ -3330,6 +3345,27 @@ GST_START_TEST (test_reset_using_rtx_packets_does_not_stall)
 
 GST_END_TEST;
 
+GST_START_TEST (test_gap_using_rtx_does_not_stall)
+{
+  GstHarness *h = gst_harness_new ("rtpjitterbuffer");
+
+  BufferArrayCtx bufs[] = {
+    /* *INDENT-OFF* */
+    { 201, -1440, FALSE, 185591 },
+    { 265,     1, FALSE,      0 },
+    /* *INDENT-ON* */
+  };
+
+  g_object_set (h->element, "do-lost", TRUE,
+      "do-retransmission", TRUE,
+      "rtx-next-seqnum", FALSE, "rtx-delay-reorder", 0, NULL);
+
+  fail_unless (check_for_stall (h, bufs, G_N_ELEMENTS (bufs)));
+  gst_harness_teardown (h);
+}
+
+GST_END_TEST;
+
 static Suite *
 rtpjitterbuffer_suite (void)
 {
@@ -3405,6 +3441,7 @@ 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);
+  tcase_add_test (tc_chain, test_gap_using_rtx_does_not_stall);
 
 
   return s;