rtprtx: Fix copying extension headers

There was a typo leading to reading memory from the buffer we were
writing to.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/2696>

diff --git a/subprojects/gst-plugins-good/gst/rtpmanager/gstrtprtxreceive.c b/subprojects/gst-plugins-good/gst/rtpmanager/gstrtprtxreceive.c
index 2dcd2f8..274ceed 100644 (file)
--- a/subprojects/gst-plugins-good/gst/rtpmanager/gstrtprtxreceive.c
+++ b/subprojects/gst-plugins-good/gst/rtpmanager/gstrtprtxreceive.c
@@ -677,7 +677,7 @@ rewrite_header_extensions (GstRtpRtxReceive * rtx, GstRTPBuffer * rtp)
          * the header extension space that needs to be accounted for.
          */
         memcpy (&map.data[write_offset],
-            &map.data[read_offset - hdr_unit_bytes], read_len + hdr_unit_bytes);
+            &pdata[read_offset - hdr_unit_bytes], read_len + hdr_unit_bytes);
         write_offset += read_len + hdr_unit_bytes;
       }
 

diff --git a/subprojects/gst-plugins-good/gst/rtpmanager/gstrtprtxsend.c b/subprojects/gst-plugins-good/gst/rtpmanager/gstrtprtxsend.c
index 5e92e25..349bb16 100644 (file)
--- a/subprojects/gst-plugins-good/gst/rtpmanager/gstrtprtxsend.c
+++ b/subprojects/gst-plugins-good/gst/rtpmanager/gstrtprtxsend.c
@@ -630,11 +630,11 @@ rewrite_header_extensions (GstRtpRtxSend * rtx, GstRTPBuffer * rtp)
       } else {
         /* TODO: may need to write mid at different times to the original
          * buffer to account for the difference in timing of acknowledgement
-         * of the rtx ssrc from the original ssrc.  This may add extra data to
+         * of the rtx ssrc from the original ssrc. This may add extra data to
          * the header extension space that needs to be accounted for.
          */
-        memcpy (&map.data[write_offset],
-            &map.data[read_offset - hdr_unit_bytes], read_len + hdr_unit_bytes);
+        memcpy (&map.data[write_offset], &pdata[read_offset - hdr_unit_bytes],
+            read_len + hdr_unit_bytes);
         write_offset += read_len + hdr_unit_bytes;
       }
 

diff --git a/subprojects/gst-plugins-good/tests/check/elements/rtprtx.c b/subprojects/gst-plugins-good/tests/check/elements/rtprtx.c
index 4eec054..1580c8b 100644 (file)
--- a/subprojects/gst-plugins-good/tests/check/elements/rtprtx.c
+++ b/subprojects/gst-plugins-good/tests/check/elements/rtprtx.c
@@ -929,6 +929,113 @@ GST_START_TEST (test_rtxsender_clock_rate_map)
 
 GST_END_TEST;
 
+#define TWCC_EXTMAP_STR "http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-01"
+GST_START_TEST (test_rtxsend_header_extensions_copy)
+{
+  const guint packets_num = 5;
+  guint master_ssrc = 1234567;
+  guint master_pt = 96;
+  guint rtx_pt = 99;
+  GstStructure *pt_map;
+  GstBuffer *inbufs[5];
+  GstHarness *hrecv = gst_harness_new ("rtprtxreceive");
+  GstHarness *hsend = gst_harness_new ("rtprtxsend");
+  GstRTPHeaderExtension *twcc;
+  guint twcc_hdr_id = 7;
+  guint i;
+
+  pt_map = gst_structure_new ("application/x-rtp-pt-map",
+      "96", G_TYPE_UINT, rtx_pt, NULL);
+  g_object_set (hrecv->element, "payload-type-map", pt_map, NULL);
+  g_object_set (hsend->element, "payload-type-map", pt_map, NULL);
+
+  gst_harness_set_src_caps_str (hsend, "application/x-rtp, "
+      "media = (string)video, payload = (int)96, "
+      "ssrc = (uint)1234567, clock-rate = (int)90000, "
+      "encoding-name = (string)RAW");
+  gst_harness_set_src_caps_str (hrecv, "application/x-rtp, "
+      "media = (string)video, payload = (int)96, "
+      "ssrc = (uint)1234567, clock-rate = (int)90000, "
+      "encoding-name = (string)RAW");
+
+  twcc = gst_rtp_header_extension_create_from_uri (TWCC_EXTMAP_STR);
+  gst_rtp_header_extension_set_id (twcc, twcc_hdr_id);
+
+  /* Push 'packets_num' packets through rtxsend to rtxreceive */
+  guint8 twcc_seq[2] = { 0, };
+  for (i = 0; i < packets_num; ++i) {
+    GstRTPBuffer rtp = GST_RTP_BUFFER_INIT;
+
+    twcc_seq[0] = i;
+    inbufs[i] = create_rtp_buffer (master_ssrc, master_pt, 100 + i);
+
+    fail_unless (gst_rtp_buffer_map (inbufs[i], GST_MAP_READWRITE, &rtp));
+    fail_unless (gst_rtp_buffer_add_extension_onebyte_header (&rtp,
+            twcc_hdr_id, "100", 2));
+    gst_rtp_buffer_unmap (&rtp);
+
+    fail_unless (gst_rtp_header_extension_write (twcc, inbufs[i],
+            GST_RTP_HEADER_EXTENSION_ONE_BYTE, inbufs[i], twcc_seq,
+            sizeof (twcc_seq)) >= 0);
+
+    gst_harness_push (hsend, gst_buffer_ref (inbufs[i]));
+    gst_harness_push (hrecv, gst_harness_pull (hsend));
+    pull_and_verify (hrecv, FALSE, master_ssrc, master_pt, 100 + i);
+  }
+  gst_clear_object (&twcc);
+
+  /* Getting rid of reconfigure event. Preparation before the next step */
+  gst_event_unref (gst_harness_pull_upstream_event (hrecv));
+  fail_unless_equals_int (gst_harness_upstream_events_in_queue (hrecv), 0);
+
+  /* Push 'packets_num' RTX events through rtxreceive to rtxsend.
+     Push RTX packets from rtxsend to rtxreceive and
+     check that the packet produced out of RTX packet is the same
+     as an original packet */
+  for (i = 0; i < packets_num; ++i) {
+    GstBuffer *outbuf;
+
+    gst_harness_push_upstream_event (hrecv,
+        create_rtx_event (master_ssrc, master_pt, 100 + i));
+    gst_harness_push_upstream_event (hsend,
+        gst_harness_pull_upstream_event (hrecv));
+    gst_harness_push (hrecv, gst_harness_pull (hsend));
+
+    outbuf = gst_harness_pull (hrecv);
+
+    compare_rtp_packets (inbufs[i], outbuf);
+    gst_buffer_unref (inbufs[i]);
+    gst_buffer_unref (outbuf);
+  }
+
+  /* Check RTX stats */
+  {
+    guint rtx_requests;
+    guint rtx_packets;
+    guint rtx_assoc_packets;
+    g_object_get (G_OBJECT (hsend->element),
+        "num-rtx-requests", &rtx_requests,
+        "num-rtx-packets", &rtx_packets, NULL);
+    fail_unless_equals_int (rtx_packets, packets_num);
+    fail_unless_equals_int (rtx_requests, packets_num);
+
+    g_object_get (G_OBJECT (hrecv->element),
+        "num-rtx-requests", &rtx_requests,
+        "num-rtx-packets", &rtx_packets,
+        "num-rtx-assoc-packets", &rtx_assoc_packets, NULL);
+    fail_unless_equals_int (rtx_packets, packets_num);
+    fail_unless_equals_int (rtx_requests, packets_num);
+    fail_unless_equals_int (rtx_assoc_packets, packets_num);
+  }
+
+  gst_structure_free (pt_map);
+  gst_harness_teardown (hrecv);
+  gst_harness_teardown (hsend);
+}
+
+GST_END_TEST;
+
+
 #define RTPHDREXT_STREAM_ID GST_RTP_HDREXT_BASE "sdes:rtp-stream-id"
 #define RTPHDREXT_REPAIRED_STREAM_ID GST_RTP_HDREXT_BASE "sdes:repaired-rtp-stream-id"
 
@@ -1081,6 +1188,7 @@ rtprtx_suite (void)
   tcase_add_test (tc_chain, test_rtxqueue_max_size_time);
   tcase_add_test (tc_chain, test_rtxsender_clock_rate_map);
   tcase_add_test (tc_chain, test_rtxsend_header_extensions);
+  tcase_add_test (tc_chain, test_rtxsend_header_extensions_copy);
 
   return s;
 }