From 339f950e7936208d9b8ea0acb7bcd0b8011d9b05 Mon Sep 17 00:00:00 2001 From: Thibault Saunier Date: Thu, 30 Jun 2022 15:15:22 +0000 Subject: [PATCH] rtprtx: Fix copying extension headers There was a typo leading to reading memory from the buffer we were writing to. Part-of: --- .../gst/rtpmanager/gstrtprtxreceive.c | 2 +- .../gst/rtpmanager/gstrtprtxsend.c | 6 +- .../tests/check/elements/rtprtx.c | 108 ++++++++++++++++++ 3 files changed, 112 insertions(+), 4 deletions(-) diff --git a/subprojects/gst-plugins-good/gst/rtpmanager/gstrtprtxreceive.c b/subprojects/gst-plugins-good/gst/rtpmanager/gstrtprtxreceive.c index 2dcd2f80f6..274ceed226 100644 --- 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 5e92e25ee9..349bb169be 100644 --- 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 4eec054d3c..1580c8bf4a 100644 --- 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; }