From 3296c678b3a639b4757b16b4e7ee1654754dceaf Mon Sep 17 00:00:00 2001
From: Thibault Saunier <tsaunier@igalia.com>
Date: Sun, 15 May 2022 16:53:12 +0000
Subject: [PATCH] rtcpbuffer: Allow padding on first reduced size packets

It is valid to have the padding set to 1 on the first packet and it
happens very often from TWCC packets coming from libwebrtc. This means
that we were totally ignoring many TWCC packets.

Fix test that checked that a first packet with padding was not valid and
instead test a single twcc packet with padding to check precisely what
this patch was about.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/2422>
---
 .../gst-libs/gst/rtp/gstrtcpbuffer.c          |  5 ++--
 .../gst-libs/gst/rtp/gstrtcpbuffer.h          |  4 +--
 .../gst-plugins-base/tests/check/libs/rtp.c   | 27 ++++++++++---------
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/subprojects/gst-plugins-base/gst-libs/gst/rtp/gstrtcpbuffer.c b/subprojects/gst-plugins-base/gst-libs/gst/rtp/gstrtcpbuffer.c
index 5b5820779f..884440f113 100644
--- a/subprojects/gst-plugins-base/gst-libs/gst/rtp/gstrtcpbuffer.c
+++ b/subprojects/gst-plugins-base/gst-libs/gst/rtp/gstrtcpbuffer.c
@@ -109,8 +109,7 @@ gst_rtcp_buffer_validate_data_internal (guint8 * data, guint len,
   if (G_UNLIKELY (header_mask != GST_RTCP_VALID_VALUE))
     goto wrong_mask;
 
-  /* no padding when mask succeeds */
-  padding = FALSE;
+  padding = data[0] & 0x20;
 
   /* store len */
   data_len = len;
@@ -129,7 +128,7 @@ gst_rtcp_buffer_validate_data_internal (guint8 * data, guint len,
     if (data_len < 4)
       break;
 
-    /* padding only allowed on last packet */
+    /* Version already checked for first packet through mask */
     if (padding)
       break;
 
diff --git a/subprojects/gst-plugins-base/gst-libs/gst/rtp/gstrtcpbuffer.h b/subprojects/gst-plugins-base/gst-libs/gst/rtp/gstrtcpbuffer.h
index 3681c6d959..b6410a5a1f 100644
--- a/subprojects/gst-plugins-base/gst-libs/gst/rtp/gstrtcpbuffer.h
+++ b/subprojects/gst-plugins-base/gst-libs/gst/rtp/gstrtcpbuffer.h
@@ -262,10 +262,10 @@ typedef enum
 /**
  * GST_RTCP_REDUCED_SIZE_VALID_MASK:
  *
- * Mask for version, padding bit and packet type pair allowing reduced size
+ * Mask for version and packet type pair allowing reduced size
  * packets, basically it accepts other types than RR and SR
  */
-#define GST_RTCP_REDUCED_SIZE_VALID_MASK (0xc000 | 0x2000 | 0xf8)
+#define GST_RTCP_REDUCED_SIZE_VALID_MASK (0xc000 | 0xf8)
 
 /**
  * GST_RTCP_VALID_VALUE:
diff --git a/subprojects/gst-plugins-base/tests/check/libs/rtp.c b/subprojects/gst-plugins-base/tests/check/libs/rtp.c
index 11eec6d0c4..ca56a722d8 100644
--- a/subprojects/gst-plugins-base/tests/check/libs/rtp.c
+++ b/subprojects/gst-plugins-base/tests/check/libs/rtp.c
@@ -31,6 +31,8 @@
 
 #define RTP_HEADER_LEN 12
 
+static GstBuffer *create_feedback_buffer (gboolean with_padding);
+
 GST_START_TEST (test_rtp_buffer)
 {
   GstBuffer *buf;
@@ -1073,20 +1075,19 @@ GST_END_TEST;
 
 GST_START_TEST (test_rtcp_validate_reduced_with_padding)
 {
-  /* Reduced size packet with padding. */
-  guint8 rtcp_pkt[] = {
-    0xA0, 0xcd, 0x00, 0x08,     /* P=1, Type FB, length = 8 */
-    0x97, 0x6d, 0x21, 0x6a,
-    0x4d, 0x16, 0xaf, 0x14,
-    0x10, 0x1f, 0xd9, 0x91,
-    0x0f, 0xb7, 0x50, 0x88,
-    0x3b, 0x79, 0x31, 0x50,
-    0xbe, 0x19, 0x12, 0xa8,
-    0xbb, 0xce, 0x9e, 0x3e,
-    0x00, 0x00, 0x00, 0x04      /* RTCP padding */
-  };
+  GstRTCPPacket packet;
+  GstRTCPBuffer rtcp = GST_RTCP_BUFFER_INIT;
+  GstBuffer *buffer = create_feedback_buffer (TRUE);
 
-  fail_if (gst_rtcp_buffer_validate_data_reduced (rtcp_pkt, sizeof (rtcp_pkt)));
+  gst_rtcp_buffer_map (buffer, GST_MAP_READ, &rtcp);
+  fail_unless (gst_rtcp_buffer_get_first_packet (&rtcp, &packet));
+  fail_unless (gst_rtcp_packet_get_padding (&packet));
+  gst_rtcp_buffer_unmap (&rtcp);
+
+  fail_unless (gst_rtcp_buffer_validate_reduced (buffer));
+  fail_if (gst_rtcp_buffer_validate (buffer));
+
+  gst_buffer_unref (buffer);
 }
 
 GST_END_TEST;