rtpjitterbuffer: fix divide-by-zero

The estimated packet-duration can sometimes end up as zero, and dividing
by that is never a good idea...

The test reproduces the scenario, and the fix is easy.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/966>

diff --git a/gst/rtpmanager/gstrtpjitterbuffer.c b/gst/rtpmanager/gstrtpjitterbuffer.c
index eec1f9e..8f318da 100644 (file)
--- a/gst/rtpmanager/gstrtpjitterbuffer.c
+++ b/gst/rtpmanager/gstrtpjitterbuffer.c
@@ -2498,7 +2498,7 @@ gst_rtp_jitter_buffer_handle_missing_packets (GstRtpJitterBuffer * jitterbuffer,
       guint lost_packets;
       GstClockTime lost_duration;
       GstClockTimeDiff gap_time;
-      guint saveable_packets;
+      guint saveable_packets = 0;
       GstClockTime saveable_duration;
 
       /* gap time represents the total duration of all missing packets */
@@ -2506,7 +2506,9 @@ 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 */
-      saveable_packets = offset / est_pkt_duration;
+      if (est_pkt_duration)
+        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);

diff --git a/tests/check/elements/rtpjitterbuffer.c b/tests/check/elements/rtpjitterbuffer.c
index af9a03a..a918630 100644 (file)
--- a/tests/check/elements/rtpjitterbuffer.c
+++ b/tests/check/elements/rtpjitterbuffer.c
@@ -1355,6 +1355,50 @@ GST_START_TEST (test_no_fractional_lost_event_durations)
 
 GST_END_TEST;
 
+GST_START_TEST (test_late_lost_with_same_pts)
+{
+  GstHarness *h = gst_harness_new ("rtpjitterbuffer");
+  GstClockTime dts, now;
+  guint latency_ms = 40;
+  guint16 seqnum;
+  guint rtp_ts;
+
+  g_object_set (h->element, "do-lost", TRUE, NULL);
+  seqnum = construct_deterministic_initial_state (h, latency_ms);
+
+  dts = seqnum * TEST_BUF_DURATION;
+  rtp_ts = seqnum * TEST_RTP_TS_DURATION;
+
+  /* set the time on the clock one buffer-length after the
+     length of the jitterbuffer */
+  now = dts + latency_ms * GST_MSECOND + TEST_BUF_DURATION;
+  gst_test_clock_set_time (GST_TEST_CLOCK (GST_ELEMENT_CLOCK (h->element)),
+      now);
+
+  /* now two buffers arrive, same arrival time (in the past, must
+     have spent a lot of time from udpsrc to jitterbuffer!),
+     with the same rtptimestamp (typical of videobuffers),
+     with a gap in between them */
+  fail_unless_equals_int (GST_FLOW_OK, gst_harness_push (h,
+          generate_test_buffer_full (dts, seqnum, rtp_ts)));
+
+  fail_unless_equals_int (GST_FLOW_OK, gst_harness_push (h,
+          generate_test_buffer_full (dts, seqnum + 2, rtp_ts)));
+
+  /* the lost event is generated immediately since we are already
+     too late to wait for anything */
+  verify_lost_event (h, seqnum + 1, dts, 0);
+  gst_buffer_unref (gst_harness_pull (h));
+  gst_buffer_unref (gst_harness_pull (h));
+
+  /* verify that we have pulled out all waiting buffers and events */
+  fail_unless_equals_int (0, gst_harness_buffers_in_queue (h));
+  fail_unless_equals_int (0, gst_harness_events_in_queue (h));
+
+  gst_harness_teardown (h);
+}
+
+GST_END_TEST;
 
 static void
 gst_test_clock_set_time_and_process (GstTestClock * testclock,
@@ -3316,7 +3360,7 @@ rtpjitterbuffer_suite (void)
       test_loss_equidistant_spacing_with_parameter_packets);
   tcase_add_loop_test (tc_chain, test_no_fractional_lost_event_durations, 0,
       G_N_ELEMENTS (no_fractional_lost_event_durations_input));
-
+  tcase_add_test (tc_chain, test_late_lost_with_same_pts);
 
   tcase_add_test (tc_chain, test_rtx_expected_next);
   tcase_add_test (tc_chain, test_rtx_not_bursting_requests);