From c830f87a32355c915433af2e8721ca35cc18ab82 Mon Sep 17 00:00:00 2001 From: Johan Sternerup Date: Thu, 8 Aug 2024 12:28:11 +0000 Subject: [PATCH] twcc: Handle wrapping of reference time Previously the wrapping of the 24-bit reference time was not handled correctly when transforming it into GstClockTime. Given the unit of 64ms the span that could be represented by 24 bits is 12 days and depending on the start value we could get a wrapping problem anytime within this time frame. This turned out to be particularly problematic for the GCC algorithm in gst-plugins-rs which tried to evict old packages based on the "oldest" timestamp, which due to wrapping problems could be in the future. Thus, the container managing the packets could grow without limits for a long time thereby creating both CPU and memory problems. Part-of: --- .../gst-plugins-good/gst/rtpmanager/rtptwcc.c | 31 +++++++++++++++++-- .../tests/check/elements/rtpsession.c | 17 +++++----- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/subprojects/gst-plugins-good/gst/rtpmanager/rtptwcc.c b/subprojects/gst-plugins-good/gst/rtpmanager/rtptwcc.c index 2b642bff6b..bd42987961 100644 --- a/subprojects/gst-plugins-good/gst/rtpmanager/rtptwcc.c +++ b/subprojects/gst-plugins-good/gst/rtpmanager/rtptwcc.c @@ -103,6 +103,9 @@ struct _RTPTWCCManager GstClockTime next_feedback_send_time; GstClockTime feedback_interval; + + guint64 remote_ts_base; + gint64 base_time_prev; }; G_DEFINE_TYPE (RTPTWCCManager, rtp_twcc_manager, G_TYPE_OBJECT); @@ -124,6 +127,8 @@ rtp_twcc_manager_init (RTPTWCCManager * twcc) twcc->feedback_interval = GST_CLOCK_TIME_NONE; twcc->next_feedback_send_time = GST_CLOCK_TIME_NONE; + + twcc->remote_ts_base = -1; } static void @@ -1015,6 +1020,7 @@ rtp_twcc_manager_parse_fci (RTPTWCCManager * twcc, guint16 base_seqnum; guint16 packet_count; GstClockTime base_time; + gint64 base_time_ext; GstClockTime ts_rounded; guint8 fb_pkt_count; guint packets_parsed = 0; @@ -1029,12 +1035,17 @@ rtp_twcc_manager_parse_fci (RTPTWCCManager * twcc, base_seqnum = GST_READ_UINT16_BE (&fci_data[0]); packet_count = GST_READ_UINT16_BE (&fci_data[2]); - base_time = GST_READ_UINT24_BE (&fci_data[4]) * REF_TIME_UNIT; + base_time = GST_READ_UINT24_BE (&fci_data[4]); + /* Sign-extend the base_time from a 24-bit integer into a 64-bit signed integer + * so that we can calculate diffs with regular 64-bit operations. */ + base_time_ext = + (base_time & 0x800000) ? base_time | 0xFFFFFFFFFF800000 : base_time; fb_pkt_count = fci_data[7]; GST_DEBUG ("Parsed TWCC feedback: base_seqnum: #%u, packet_count: %u, " "base_time %" GST_TIME_FORMAT " fb_pkt_count: %u", - base_seqnum, packet_count, GST_TIME_ARGS (base_time), fb_pkt_count); + base_seqnum, packet_count, GST_TIME_ARGS (base_time * REF_TIME_UNIT), + fb_pkt_count); twcc_packets = g_array_sized_new (FALSE, FALSE, sizeof (RTPTWCCPacket), packet_count); @@ -1064,7 +1075,21 @@ rtp_twcc_manager_parse_fci (RTPTWCCManager * twcc, if (twcc->sent_packets->len > 0) first_sent_pkt = &g_array_index (twcc->sent_packets, SentPacket, 0); - ts_rounded = base_time; + if (twcc->remote_ts_base == -1) { + /* Add an initial offset of 1 << 24 so that we don't risk going below 0 if + * a future extended timestamp is earlier than the first. */ + twcc->remote_ts_base = (G_GINT64_CONSTANT (1) << 24) + base_time_ext; + } else { + /* Calculate our internal accumulated reference timestamp by continously + * adding the diff between the current and the previous sign-extended + * reference time. */ + twcc->remote_ts_base += base_time_ext - twcc->base_time_prev; + } + twcc->base_time_prev = base_time_ext; + /* Our internal accumulated reference time is in units of 64ms, propagate as + * GstClockTime in ns. */ + ts_rounded = twcc->remote_ts_base * REF_TIME_UNIT; + for (i = 0; i < twcc_packets->len; i++) { RTPTWCCPacket *pkt = &g_array_index (twcc_packets, RTPTWCCPacket, i); gint16 delta = 0; diff --git a/subprojects/gst-plugins-good/tests/check/elements/rtpsession.c b/subprojects/gst-plugins-good/tests/check/elements/rtpsession.c index 83bb8a9fcd..bfb4909d51 100644 --- a/subprojects/gst-plugins-good/tests/check/elements/rtpsession.c +++ b/subprojects/gst-plugins-good/tests/check/elements/rtpsession.c @@ -2882,13 +2882,16 @@ typedef struct } TWCCPacket; #define TWCC_DELTA_UNIT (250 * GST_USECOND) +#define TWCC_REF_TIME_UNIT (64 * GST_MSECOND) +#define TWCC_REF_TIME_INITIAL_OFFSET ((1 << 24) * TWCC_REF_TIME_UNIT) static void fail_unless_equals_twcc_clocktime (GstClockTime twcc_packet_ts, GstClockTime pkt_ts) { fail_unless_equals_clocktime ( - (twcc_packet_ts / TWCC_DELTA_UNIT) * TWCC_DELTA_UNIT, pkt_ts); + (twcc_packet_ts / TWCC_DELTA_UNIT) * TWCC_DELTA_UNIT + + TWCC_REF_TIME_INITIAL_OFFSET, pkt_ts); } #define twcc_push_packets(h, packets) \ @@ -3739,17 +3742,17 @@ GST_START_TEST (test_twcc_delta_ts_rounding) }; TWCCPacket exp_packets[] = { - {2002, 9 * GST_SECOND + 366250000, FALSE} + {2002, TWCC_REF_TIME_INITIAL_OFFSET + 9 * GST_SECOND + 366250000, FALSE} , - {2003, 9 * GST_SECOND + 366250000, FALSE} + {2003, TWCC_REF_TIME_INITIAL_OFFSET + 9 * GST_SECOND + 366250000, FALSE} , - {2017, 9 * GST_SECOND + 366750000, FALSE} + {2017, TWCC_REF_TIME_INITIAL_OFFSET + 9 * GST_SECOND + 366750000, FALSE} , - {2019, 9 * GST_SECOND + 391500000, FALSE} + {2019, TWCC_REF_TIME_INITIAL_OFFSET + 9 * GST_SECOND + 391500000, FALSE} , - {2020, 9 * GST_SECOND + 426750000, FALSE} + {2020, TWCC_REF_TIME_INITIAL_OFFSET + 9 * GST_SECOND + 426750000, FALSE} , - {2025, 9 * GST_SECOND + 427000000, TRUE} + {2025, TWCC_REF_TIME_INITIAL_OFFSET + 9 * GST_SECOND + 427000000, TRUE} , };