From 3b14a24630cc593e18338e2f09a5f0ba2a28816c Mon Sep 17 00:00:00 2001 From: Tulio Beloqui Date: Fri, 18 Dec 2020 13:06:35 +0100 Subject: [PATCH] rtptwcc: fixed feedback packet count overflow that allowed late packets to be processed Co-authored-by: Havard Graff Part-of: --- gst/rtpmanager/rtptwcc.c | 6 +-- tests/check/elements/rtpsession.c | 72 +++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 3 deletions(-) diff --git a/gst/rtpmanager/rtptwcc.c b/gst/rtpmanager/rtptwcc.c index 3bb054063c..515c69b234 100644 --- a/gst/rtpmanager/rtptwcc.c +++ b/gst/rtpmanager/rtptwcc.c @@ -72,7 +72,7 @@ struct _RTPTWCCManager guint max_packets_per_rtcp; GArray *recv_packets; - guint8 fb_pkt_count; + guint64 fb_pkt_count; gint32 last_seqnum; GArray *sent_packets; @@ -438,7 +438,7 @@ rtp_twcc_manager_add_fci (RTPTWCCManager * twcc, GstRTCPPacket * packet) GST_WRITE_UINT16_BE (header.base_seqnum, first->seqnum); GST_WRITE_UINT16_BE (header.packet_count, packet_count); GST_WRITE_UINT24_BE (header.base_time, base_time); - GST_WRITE_UINT8 (header.fb_pkt_count, twcc->fb_pkt_count); + GST_WRITE_UINT8 (header.fb_pkt_count, twcc->fb_pkt_count % G_MAXUINT8); base_time *= REF_TIME_UNIT; ts_rounded = base_time; @@ -446,7 +446,7 @@ rtp_twcc_manager_add_fci (RTPTWCCManager * twcc, GstRTCPPacket * packet) GST_DEBUG ("Created TWCC feedback: base_seqnum: #%u, packet_count: %u, " "base_time %" GST_TIME_FORMAT " fb_pkt_count: %u", first->seqnum, packet_count, GST_TIME_ARGS (base_time), - twcc->fb_pkt_count); + twcc->fb_pkt_count % G_MAXUINT8); twcc->fb_pkt_count++; twcc->expected_recv_seqnum = first->seqnum + packet_count; diff --git a/tests/check/elements/rtpsession.c b/tests/check/elements/rtpsession.c index 451ae2d295..c8eb5bdbcb 100644 --- a/tests/check/elements/rtpsession.c +++ b/tests/check/elements/rtpsession.c @@ -3532,6 +3532,77 @@ GST_START_TEST (test_twcc_recv_packets_reordered) GST_END_TEST; +GST_START_TEST (test_twcc_recv_late_packet_fb_pkt_count_wrap) +{ + SessionHarness *h = session_harness_new (); + GstBuffer *buf; + guint i; + + guint8 exp_fci0[] = { + 0x01, 0x00, /* base sequence number: 256 */ + 0x00, 0x01, /* packet status count: 1 */ + 0x00, 0x00, 0x01, /* reference time: 0 */ + 0x00, /* feedback packet count: 00 */ + /* packet chunks: */ + 0x20, 0x01, /* 0 0 1 0 0 0 0 0 | 0 0 0 0 0 0 0 1 */ + 0x00, /* 0 recv-delta */ + 0x00, /* padding */ + }; + + guint8 exp_fci1[] = { + 0x01, 0x01, /* base sequence number: 257 */ + 0x00, 0x01, /* packet status count: 1 */ + 0x00, 0x00, 0x01, /* reference time: 0 */ + 0x01, /* feedback packet count: 0 */ + /* packet chunks: */ + 0x20, 0x01, /* 0 0 1 0 0 0 0 0 | 0 0 0 0 0 0 0 1 */ + 0x01, /* 1 recv-delta */ + 0x00, /* padding */ + }; + + session_harness_set_twcc_recv_ext_id ((h), TEST_TWCC_EXT_ID); + + /* Push packets to get the feedback packet count wrap limit */ + for (i = 0; i < 255; i++) { + fail_unless_equals_int (GST_FLOW_OK, + session_harness_recv_rtp ((h), + generate_twcc_recv_buffer (i, i * 250 * GST_USECOND, TRUE))); + + /* ignore the twcc for these ones */ + gst_buffer_unref (session_harness_produce_twcc (h)); + } + + /* push pkt #256 to jump ahead and force the overflow */ + fail_unless_equals_int (GST_FLOW_OK, + session_harness_recv_rtp ((h), + generate_twcc_recv_buffer (256, 256 * 250 * GST_USECOND, TRUE))); + + /* pkt #255 is late and should be dropped */ + fail_unless_equals_int (GST_FLOW_OK, + session_harness_recv_rtp ((h), + generate_twcc_recv_buffer (255, 255 * 250 * GST_USECOND, TRUE))); + + /* push pkt #257 to verify fci is correct */ + fail_unless_equals_int (GST_FLOW_OK, + session_harness_recv_rtp ((h), + generate_twcc_recv_buffer (257, 257 * 250 * GST_USECOND, TRUE))); + + + /* we expect a fci for pkt #256 */ + buf = session_harness_produce_twcc (h); + twcc_verify_fci (buf, exp_fci0); + gst_buffer_unref (buf); + + /* and one fci for pkt #257 */ + buf = session_harness_produce_twcc (h); + twcc_verify_fci (buf, exp_fci1); + gst_buffer_unref (buf); + + session_harness_free (h); +} + +GST_END_TEST; + GST_START_TEST (test_twcc_recv_rtcp_reordered) { SessionHarness *send_h = session_harness_new (); @@ -3894,6 +3965,7 @@ rtpsession_suite (void) tcase_add_test (tc_chain, test_twcc_delta_ts_rounding); tcase_add_test (tc_chain, test_twcc_double_gap); tcase_add_test (tc_chain, test_twcc_recv_packets_reordered); + tcase_add_test (tc_chain, test_twcc_recv_late_packet_fb_pkt_count_wrap); tcase_add_test (tc_chain, test_twcc_recv_rtcp_reordered); tcase_add_test (tc_chain, test_twcc_no_exthdr_in_buffer); tcase_add_test (tc_chain, test_twcc_send_and_recv);