rtptwcc: fixed guint8 overflow of feedback packet count

Co-authored-by: Havard Graff <havard.graff@gmail.com>
Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/-/merge_requests/927>

diff --git a/gst/rtpmanager/rtptwcc.c b/gst/rtpmanager/rtptwcc.c
index d5039d4..eab88fc 100644 (file)
--- a/gst/rtpmanager/rtptwcc.c
+++ b/gst/rtpmanager/rtptwcc.c
@@ -779,6 +779,7 @@ _check_for_lost_packets (RTPTWCCManager * twcc, GArray * twcc_packets,
     guint16 base_seqnum, guint16 packet_count, guint8 fb_pkt_count)
 {
   guint packets_lost;
+  gint8 fb_pkt_count_diff;
   guint i;
 
   /* first packet */
@@ -787,9 +788,12 @@ _check_for_lost_packets (RTPTWCCManager * twcc, GArray * twcc_packets,
     goto done;
   }
 
+  fb_pkt_count_diff =
+      (gint8) (fb_pkt_count - twcc->expected_parsed_fb_pkt_count);
+
   /* we have gone backwards, don't reset the expectations,
      but process the packet nonetheless */
-  if (fb_pkt_count < twcc->expected_parsed_fb_pkt_count) {
+  if (fb_pkt_count_diff < 0) {
     GST_WARNING ("feedback packet count going backwards (%u < %u)",
         fb_pkt_count, twcc->expected_parsed_fb_pkt_count);
     return;
@@ -797,7 +801,7 @@ _check_for_lost_packets (RTPTWCCManager * twcc, GArray * twcc_packets,
 
   /* we have jumped forwards, reset expectations, but don't trigger
      lost packets in case the missing fb-packet(s) arrive later */
-  if (fb_pkt_count > twcc->expected_parsed_fb_pkt_count) {
+  if (fb_pkt_count_diff > 0) {
     GST_WARNING ("feedback packet count jumped ahead (%u > %u)",
         fb_pkt_count, twcc->expected_parsed_fb_pkt_count);
     goto done;

diff --git a/tests/check/elements/rtpsession.c b/tests/check/elements/rtpsession.c
index d87de24..a11ed27 100644 (file)
--- a/tests/check/elements/rtpsession.c
+++ b/tests/check/elements/rtpsession.c
@@ -3727,6 +3727,60 @@ GST_START_TEST (test_twcc_feedback_interval)
 
 GST_END_TEST;
 
+GST_START_TEST (test_twcc_feedback_count_wrap)
+{
+  SessionHarness *h = session_harness_new ();
+  guint i;
+  GstBuffer *buf;
+  GstEvent *event;
+  GValueArray *packets_array;
+
+  guint8 fci1[] = {
+    0x05, 0xfd,                 /* base sequence number: 1533 */
+    0x00, 0x00,                 /* packet status count: 0 */
+    0x00, 0x00, 0x00,           /* reference time: 0 */
+    0xff,                       /* feedback packet count: 255 */
+    0x00, 0x00,                 /* packet chunk: run-length, 0 */
+    0x00,                       /* 0 recv-delta */
+  };
+
+  guint8 fci2[] = {
+    0x05, 0xfe,                 /* base sequence number: 1534 */
+    0x00, 0x00,                 /* packet status count: 0 */
+    0x00, 0x00, 0x00,           /* reference time: 0 */
+    0x01,                       /* feedback packet count: 1 */
+    0x00, 0x00,                 /* packet chunk: run-length, 0 */
+    0x00,                       /* 0 recv-delta */
+  };
+
+  buf = generate_twcc_feedback_rtcp (fci1, sizeof (fci1));
+  session_harness_recv_rtcp (h, buf);
+
+  buf = generate_twcc_feedback_rtcp (fci2, sizeof (fci2));
+  session_harness_recv_rtcp (h, buf);
+
+  /* two reconfigure events */
+  for (i = 0; i < 2; i++)
+    gst_event_unref (gst_harness_pull_upstream_event (h->send_rtp_h));
+
+
+  for (i = 0; i < 2; i++) {
+    event = gst_harness_pull_upstream_event (h->send_rtp_h);
+    packets_array =
+        g_value_get_boxed (gst_structure_get_value (gst_event_get_structure
+            (event), "packets"));
+
+    /* we expect zero packets due to feedback packet count jump ahead */
+    fail_unless_equals_int (packets_array->n_values, 0);
+    gst_event_unref (event);
+  }
+
+  session_harness_free (h);
+}
+
+GST_END_TEST;
+
+
 static Suite *
 rtpsession_suite (void)
 {
@@ -3793,6 +3847,7 @@ rtpsession_suite (void)
   tcase_add_test (tc_chain, test_twcc_send_and_recv);
   tcase_add_loop_test (tc_chain, test_twcc_feedback_interval, 0,
       G_N_ELEMENTS (test_twcc_feedback_interval_ctx));
+  tcase_add_test (tc_chain, test_twcc_feedback_count_wrap);
 
 
   return s;