From 3df0cce65dfbe2bf4bfb4548e2f638d0995bcb74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Mon, 29 Jun 2015 15:53:52 +0200 Subject: [PATCH] rtpjitterbuffer: If possible, always update the current time before looping over all timers If we have a clock, update "now" now with the very latest running time we have. If timers are unscheduled below we otherwise wouldn't update now (it's only updated when timers expire), and also for the very first loop iteration now would otherwise always be 0. Also the time is used for the timeout functions, e.g. to calculate any times for the next timeouts and we would otherwise pass too old times there. https://bugzilla.gnome.org/show_bug.cgi?id=751636 --- gst/rtpmanager/gstrtpjitterbuffer.c | 14 +++++++++++++- tests/check/elements/rtpjitterbuffer.c | 21 +++++++++------------ 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/gst/rtpmanager/gstrtpjitterbuffer.c b/gst/rtpmanager/gstrtpjitterbuffer.c index f450c586b2..c9b816629b 100644 --- a/gst/rtpmanager/gstrtpjitterbuffer.c +++ b/gst/rtpmanager/gstrtpjitterbuffer.c @@ -3276,6 +3276,19 @@ wait_next_timeout (GstRtpJitterBuffer * jitterbuffer) GstClockTime timer_timeout = -1; gint i, len; + /* If we have a clock, update "now" now with the very latest running time + * we have. It is used below when timeouts are triggered to calculate + * any next possible timeout. If we only update it after waiting for the + * clock, we would give a too old time to the timeout functions. + */ + GST_OBJECT_LOCK (jitterbuffer); + if (GST_ELEMENT_CLOCK (jitterbuffer)) { + now = + gst_clock_get_time (GST_ELEMENT_CLOCK (jitterbuffer)) - + GST_ELEMENT_CAST (jitterbuffer)->base_time; + } + GST_OBJECT_UNLOCK (jitterbuffer); + GST_DEBUG_OBJECT (jitterbuffer, "now %" GST_TIME_FORMAT, GST_TIME_ARGS (now)); @@ -3369,7 +3382,6 @@ wait_next_timeout (GstRtpJitterBuffer * jitterbuffer) } if (ret != GST_CLOCK_UNSCHEDULED) { - now = timer_timeout + MAX (clock_jitter, 0); GST_DEBUG_OBJECT (jitterbuffer, "sync done, %d, #%d, %" G_GINT64_FORMAT, ret, priv->timer_seqnum, clock_jitter); } else { diff --git a/tests/check/elements/rtpjitterbuffer.c b/tests/check/elements/rtpjitterbuffer.c index cc1cd020cb..009a3c06dd 100644 --- a/tests/check/elements/rtpjitterbuffer.c +++ b/tests/check/elements/rtpjitterbuffer.c @@ -677,9 +677,6 @@ GST_START_TEST (test_only_one_lost_event_on_large_gaps) g_assert_cmpint (GST_BUFFER_PTS (out_buf), ==, 0); gst_buffer_unref (out_buf); - /* move time ahead 10 seconds */ - gst_test_clock_set_time (GST_TEST_CLOCK (data.clock), 10 * GST_SECOND); - /* wait a bit */ g_usleep (G_USEC_PER_SEC / 10); @@ -690,13 +687,16 @@ GST_START_TEST (test_only_one_lost_event_on_large_gaps) /* a buffer now arrives perfectly on time */ in_buf = generate_test_buffer (10 * GST_SECOND, FALSE, 500, 500 * 160); - gst_test_clock_set_time (GST_TEST_CLOCK (data.clock), 10 * GST_SECOND); g_assert_cmpint (gst_pad_push (data.test_src_pad, in_buf), ==, GST_FLOW_OK); /* release the wait */ gst_test_clock_wait_for_next_pending_id (GST_TEST_CLOCK (data.clock), &id); gst_test_clock_advance_time (GST_TEST_CLOCK (data.clock), GST_MSECOND * 20); test_id = gst_test_clock_process_next_clock_id (GST_TEST_CLOCK (data.clock)); + + /* move time ahead 10 seconds */ + gst_test_clock_set_time (GST_TEST_CLOCK (data.clock), 10 * GST_SECOND); + g_assert (id == test_id); gst_clock_id_unref (test_id); gst_clock_id_unref (id); @@ -860,13 +860,12 @@ GST_START_TEST (test_late_packets_still_makes_lost_events) g_object_set (data.jitter_buffer, "latency", jb_latency_ms, NULL); - gst_test_clock_set_time (GST_TEST_CLOCK (data.clock), 10 * GST_SECOND); - /* push the first buffer in */ in_buf = generate_test_buffer (0 * GST_MSECOND, TRUE, 0, 0); g_assert_cmpint (gst_pad_push (data.test_src_pad, in_buf), ==, GST_FLOW_OK); gst_test_clock_wait_for_next_pending_id (GST_TEST_CLOCK (data.clock), &id); + gst_test_clock_set_time (GST_TEST_CLOCK (data.clock), 10 * GST_SECOND); test_id = gst_test_clock_process_next_clock_id (GST_TEST_CLOCK (data.clock)); g_assert (test_id == id); gst_clock_id_unref (id); @@ -933,13 +932,12 @@ GST_START_TEST (test_all_packets_are_timestamped_zero) g_object_set (data.jitter_buffer, "latency", jb_latency_ms, NULL); - gst_test_clock_set_time (GST_TEST_CLOCK (data.clock), 10 * GST_SECOND); - /* push the first buffer in */ in_buf = generate_test_buffer (0 * GST_MSECOND, TRUE, 0, 0); g_assert_cmpint (gst_pad_push (data.test_src_pad, in_buf), ==, GST_FLOW_OK); gst_test_clock_wait_for_next_pending_id (GST_TEST_CLOCK (data.clock), &id); + gst_test_clock_set_time (GST_TEST_CLOCK (data.clock), 10 * GST_SECOND); test_id = gst_test_clock_process_next_clock_id (GST_TEST_CLOCK (data.clock)); g_assert (test_id == id); gst_clock_id_unref (test_id); @@ -1125,8 +1123,8 @@ GST_START_TEST (test_rtx_two_missing) g_assert_cmpint (gst_pad_push (data.test_src_pad, in_buf), ==, GST_FLOW_OK); /* wait for first retransmission request */ - gst_test_clock_set_time (GST_TEST_CLOCK (data.clock), 50 * GST_MSECOND); gst_test_clock_wait_for_next_pending_id (GST_TEST_CLOCK (data.clock), &id); + gst_test_clock_set_time (GST_TEST_CLOCK (data.clock), 50 * GST_MSECOND); tid = gst_test_clock_process_next_clock_id (GST_TEST_CLOCK (data.clock)); g_assert (id == tid); gst_clock_id_unref (id); @@ -1138,8 +1136,8 @@ GST_START_TEST (test_rtx_two_missing) verify_rtx_event (out_event, 2, 40 * GST_MSECOND, 10, 20 * GST_MSECOND); /* wait for second retransmission request */ - gst_test_clock_set_time (GST_TEST_CLOCK (data.clock), 60 * GST_MSECOND); gst_test_clock_wait_for_next_pending_id (GST_TEST_CLOCK (data.clock), &id); + gst_test_clock_set_time (GST_TEST_CLOCK (data.clock), 60 * GST_MSECOND); tid = gst_test_clock_process_next_clock_id (GST_TEST_CLOCK (data.clock)); g_assert (id == tid); gst_clock_id_unref (id); @@ -1279,8 +1277,6 @@ GST_START_TEST (test_rtx_packet_delay) GST_BUFFER_FLAG_SET (in_buf, GST_BUFFER_FLAG_DISCONT); g_assert_cmpint (gst_pad_push (data.test_src_pad, in_buf), ==, GST_FLOW_OK); - gst_test_clock_set_time (GST_TEST_CLOCK (data.clock), 20 * GST_MSECOND); - /* put second buffer, the jitterbuffer should now know that the packet spacing * is 20ms and should ask for retransmission of seqnum 2 in 20ms+10ms because * 2*jitter==0 and 0.5*packet_spacing==10ms */ @@ -1319,6 +1315,7 @@ GST_START_TEST (test_rtx_packet_delay) /* wait for timeout for rtx 6 -> 7 */ gst_test_clock_wait_for_next_pending_id (GST_TEST_CLOCK (data.clock), &id); + gst_test_clock_advance_time (GST_TEST_CLOCK (data.clock), GST_MSECOND * 60); tid = gst_test_clock_process_next_clock_id (GST_TEST_CLOCK (data.clock)); g_assert (id == tid); gst_clock_id_unref (id);