From 4df3da3bab8b8f3aa03437f1aa345cba47a7d000 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Mon, 17 Oct 2022 18:38:43 +0300 Subject: [PATCH] rtpbuffer: Initialize extended timestamp to the first wraparound period This allows correct handling of wrapping around backwards during the first wraparound period and avoids the infamous "Cannot unwrap, any wrapping took place yet" error message. It allows makes sure that for actual timestamp jumps a valid value is returned instead of 0, which then allows the caller to handle it properly. Not having this can have the caller see the same timestamp (0) for a very long time, which for example can cause rtpjitterbuffer to output the same timestamp for a very long time. Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/1500 Part-of: --- .../gst-libs/gst/rtp/gstrtpbuffer.c | 2 +- .../gst-plugins-base/tests/check/libs/rtp.c | 64 ++++++++++--------- 2 files changed, 36 insertions(+), 30 deletions(-) diff --git a/subprojects/gst-plugins-base/gst-libs/gst/rtp/gstrtpbuffer.c b/subprojects/gst-plugins-base/gst-libs/gst/rtp/gstrtpbuffer.c index d83bbd2324..228abf4568 100644 --- a/subprojects/gst-plugins-base/gst-libs/gst/rtp/gstrtpbuffer.c +++ b/subprojects/gst-plugins-base/gst-libs/gst/rtp/gstrtpbuffer.c @@ -1326,7 +1326,7 @@ gst_rtp_buffer_ext_timestamp (guint64 * exttimestamp, guint32 timestamp) ext = *exttimestamp; if (ext == -1) { - result = timestamp; + result = (G_GUINT64_CONSTANT (1) << 32) + timestamp; } else { /* pick wraparound counter from previous timestamp and add to new timestamp */ result = timestamp + (ext & ~(G_GUINT64_CONSTANT (0xffffffff))); diff --git a/subprojects/gst-plugins-base/tests/check/libs/rtp.c b/subprojects/gst-plugins-base/tests/check/libs/rtp.c index ca56a722d8..7a5a7b646e 100644 --- a/subprojects/gst-plugins-base/tests/check/libs/rtp.c +++ b/subprojects/gst-plugins-base/tests/check/libs/rtp.c @@ -1995,21 +1995,22 @@ GST_START_TEST (test_ext_timestamp_basic) /* no wraparound when timestamps are increasing */ result = gst_rtp_buffer_ext_timestamp (&exttimestamp, 0); - fail_unless_equals_uint64 (result, 0); + fail_unless_equals_uint64 (result, (G_GUINT64_CONSTANT (1) << 32) + 0); result = gst_rtp_buffer_ext_timestamp (&exttimestamp, 10); - fail_unless_equals_uint64 (result, 10); + fail_unless_equals_uint64 (result, (G_GUINT64_CONSTANT (1) << 32) + 10); result = gst_rtp_buffer_ext_timestamp (&exttimestamp, 10); - fail_unless_equals_uint64 (result, 10); + fail_unless_equals_uint64 (result, (G_GUINT64_CONSTANT (1) << 32) + 10); result = gst_rtp_buffer_ext_timestamp (&exttimestamp, G_GUINT64_CONSTANT (1) + G_MAXINT32); - fail_unless_equals_uint64 (result, G_GUINT64_CONSTANT (1) + G_MAXINT32); + fail_unless_equals_uint64 (result, + (G_GUINT64_CONSTANT (1) << 32) + G_GUINT64_CONSTANT (1) + G_MAXINT32); /* Even big bumps under G_MAXINT32 don't result in wrap-around */ exttimestamp = -1; result = gst_rtp_buffer_ext_timestamp (&exttimestamp, 1087500); - fail_unless_equals_uint64 (result, 1087500); + fail_unless_equals_uint64 (result, (G_GUINT64_CONSTANT (1) << 32) + 1087500); result = gst_rtp_buffer_ext_timestamp (&exttimestamp, 24); - fail_unless_equals_uint64 (result, 24); + fail_unless_equals_uint64 (result, (G_GUINT64_CONSTANT (1) << 32) + 24); } GST_END_TEST; @@ -2020,13 +2021,15 @@ GST_START_TEST (test_ext_timestamp_wraparound) fail_unless_equals_uint64 (gst_rtp_buffer_ext_timestamp (&ext_ts, G_MAXUINT32 - 90000 + G_GUINT64_CONSTANT (1)), - (G_MAXUINT32 - 90000 + G_GUINT64_CONSTANT (1))); + ((G_GUINT64_CONSTANT (1) << 32) + G_MAXUINT32 - 90000 + + G_GUINT64_CONSTANT (1))); fail_unless_equals_uint64 (gst_rtp_buffer_ext_timestamp (&ext_ts, 0), - G_MAXUINT32 + G_GUINT64_CONSTANT (1)); + (G_GUINT64_CONSTANT (1) << 32) + G_MAXUINT32 + G_GUINT64_CONSTANT (1)); fail_unless_equals_uint64 (gst_rtp_buffer_ext_timestamp (&ext_ts, 90000), - (G_MAXUINT32 + G_GUINT64_CONSTANT (1) + 90000)); + ((G_GUINT64_CONSTANT (1) << 32) + G_MAXUINT32 + G_GUINT64_CONSTANT (1) + + 90000)); } GST_END_TEST; @@ -2036,37 +2039,41 @@ GST_START_TEST (test_ext_timestamp_wraparound_disordered) { guint64 ext_ts = -1; - fail_unless (gst_rtp_buffer_ext_timestamp (&ext_ts, - G_MAXUINT32 - 90000 + G_GUINT64_CONSTANT (1)) - == (G_MAXUINT32 - 90000 + G_GUINT64_CONSTANT (1))); + fail_unless_equals_uint64 (gst_rtp_buffer_ext_timestamp (&ext_ts, + G_MAXUINT32 - 90000 + G_GUINT64_CONSTANT (1)), + (G_GUINT64_CONSTANT (1) << 32) + G_MAXUINT32 - 90000 + + G_GUINT64_CONSTANT (1)); - fail_unless (gst_rtp_buffer_ext_timestamp (&ext_ts, 0) - == G_MAXUINT32 + G_GUINT64_CONSTANT (1)); + fail_unless_equals_uint64 (gst_rtp_buffer_ext_timestamp (&ext_ts, 0), + (G_GUINT64_CONSTANT (1) << 32) + G_MAXUINT32 + G_GUINT64_CONSTANT (1)); /* Unwrapping around */ - fail_unless (gst_rtp_buffer_ext_timestamp (&ext_ts, - G_MAXUINT32 - 90000 + G_GUINT64_CONSTANT (1)) - == (G_MAXUINT32 - 90000 + G_GUINT64_CONSTANT (1))); + fail_unless_equals_uint64 (gst_rtp_buffer_ext_timestamp (&ext_ts, + G_MAXUINT32 - 90000 + G_GUINT64_CONSTANT (1)), + (G_GUINT64_CONSTANT (1) << 32) + G_MAXUINT32 - 90000 + + G_GUINT64_CONSTANT (1)); - fail_unless (gst_rtp_buffer_ext_timestamp (&ext_ts, 90000) - == (G_MAXUINT32 + G_GUINT64_CONSTANT (1) + 90000)); + fail_unless_equals_uint64 (gst_rtp_buffer_ext_timestamp (&ext_ts, 90000), + (G_GUINT64_CONSTANT (1) << 32) + G_MAXUINT32 + G_GUINT64_CONSTANT (1) + + 90000); } GST_END_TEST; -GST_START_TEST (test_ext_timestamp_wraparound_disordered_cannot_unwrap) +GST_START_TEST (test_ext_timestamp_wraparound_disordered_backwards) { guint64 ext_ts = -1; - fail_unless (gst_rtp_buffer_ext_timestamp (&ext_ts, 90000) - == 90000); + fail_unless_equals_uint64 (gst_rtp_buffer_ext_timestamp (&ext_ts, 90000), + (G_GUINT64_CONSTANT (1) << 32) + 90000); - /* Cannot unwrapping around */ - fail_unless (gst_rtp_buffer_ext_timestamp (&ext_ts, - G_MAXUINT32 - 90000 + G_GUINT64_CONSTANT (1)) == 0); + /* Wraps backwards */ + fail_unless_equals_uint64 (gst_rtp_buffer_ext_timestamp (&ext_ts, + G_MAXUINT32 - 90000 + G_GUINT64_CONSTANT (1)), + G_MAXUINT32 - 90000 + G_GUINT64_CONSTANT (1)); - fail_unless (gst_rtp_buffer_ext_timestamp (&ext_ts, 90000) - == 90000); + fail_unless_equals_uint64 (gst_rtp_buffer_ext_timestamp (&ext_ts, 90000), + (G_GUINT64_CONSTANT (1) << 32) + 90000); } GST_END_TEST; @@ -2366,8 +2373,7 @@ rtp_suite (void) tcase_add_test (tc_chain, test_ext_timestamp_basic); tcase_add_test (tc_chain, test_ext_timestamp_wraparound); tcase_add_test (tc_chain, test_ext_timestamp_wraparound_disordered); - tcase_add_test (tc_chain, - test_ext_timestamp_wraparound_disordered_cannot_unwrap); + tcase_add_test (tc_chain, test_ext_timestamp_wraparound_disordered_backwards); tcase_add_test (tc_chain, test_rtcp_compound_padding); tcase_add_test (tc_chain, test_rtp_buffer_extlen_wraparound);