From d75c67847966ea25a0fb42194fb196a2a2a4e2b6 Mon Sep 17 00:00:00 2001 From: Havard Graff Date: Sun, 25 Apr 2021 02:16:45 +0200 Subject: [PATCH] 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: --- gst/rtpmanager/gstrtpjitterbuffer.c | 6 +++-- tests/check/elements/rtpjitterbuffer.c | 46 +++++++++++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/gst/rtpmanager/gstrtpjitterbuffer.c b/gst/rtpmanager/gstrtpjitterbuffer.c index eec1f9e..8f318da 100644 --- 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 --- 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); -- 2.7.4