From b83b73841df2693746a884a5eb9bfb3781c5dc1b Mon Sep 17 00:00:00 2001 From: Carlos Bentzen Date: Thu, 9 Jan 2025 00:42:48 +0100 Subject: [PATCH] webrtc: fix duplicate payload types with RTX and multiple video codecs Before this patch, there could be duplicate payload types in offers that have, within a media section, multiple codecs and RTX enabled: ``` m=video 9 UDP/TLS/RTP/SAVPF 96 97 97 <-- HAS DUPLICATES a=sendrecv a=rtpmap:96 VP8/90000 a=rtcp-fb:96 nack a=rtcp-fb:96 nack pli a=rtcp-fb:96 ccm fir a=rtcp-fb:96 transport-cc a=rtpmap:97 H264/90000 a=rtcp-fb:97 nack a=rtcp-fb:97 nack pli a=rtcp-fb:97 ccm fir a=rtcp-fb:97 transport-cc a=rtpmap:97 rtx/90000 <--------- PT IS DUPLICATE a=fmtp:97 apt=96 ``` Fix this by populating the media_mapping array with all media formats rather than only the first one. The added test case reproduces the issue, which fails without this patch. Part-of: --- .../gst-plugins-bad/ext/webrtc/gstwebrtcbin.c | 9 ++-- .../tests/check/elements/webrtcbin.c | 44 +++++++++++++++++++ 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c b/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c index 96977cec22..a4e8a2d97c 100644 --- a/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c +++ b/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c @@ -3608,6 +3608,7 @@ sdp_media_from_transceiver (GstWebRTCBin * webrtc, GstSDPMedia * media, for (i = 0; i < gst_caps_get_size (caps); i++) { GstCaps *format = gst_caps_new_empty (); GstStructure *s = gst_structure_copy (gst_caps_get_structure (caps, i)); + gint media_pt; if (i == 0) { gst_structure_foreach_id_str (extmap, @@ -3631,6 +3632,9 @@ sdp_media_from_transceiver (GstWebRTCBin * webrtc, GstSDPMedia * media, return FALSE; } + if (gst_structure_get_int (s, "payload", &media_pt)) + find_or_create_payload_map_for_media_pt (media_mapping, media_pt); + gst_caps_unref (format); } @@ -3643,10 +3647,7 @@ sdp_media_from_transceiver (GstWebRTCBin * webrtc, GstSDPMedia * media, guint rtx_target_ssrc = -1; gint media_pt; - if (gst_structure_get_int (s, "payload", &media_pt) && - webrtc->bundle_policy == GST_WEBRTC_BUNDLE_POLICY_NONE) - find_or_create_payload_map_for_media_pt (media_mapping, media_pt); - + gst_structure_get_int (s, "payload", &media_pt); rtx_target_pt = media_pt; if (!gst_structure_get_int (s, "clock-rate", &clockrate)) diff --git a/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c b/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c index 9541b97c10..965b696443 100644 --- a/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c +++ b/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c @@ -6501,6 +6501,49 @@ GST_START_TEST (test_offer_rollback) GST_END_TEST; +GST_START_TEST (test_video_rtx_no_duplicate_payloads) +{ + struct test_webrtc *t = test_webrtc_new (); + GstWebRTCRTPTransceiverDirection direction; + GstWebRTCRTPTransceiver *trans; + GstCaps *caps; + + VAL_SDP_INIT (no_duplicate_payloads, on_sdp_media_no_duplicate_payloads, + NULL, NULL); + guint media_format_count[] = { 3 }; + VAL_SDP_INIT (media_formats, on_sdp_media_count_formats, + media_format_count, &no_duplicate_payloads); + VAL_SDP_INIT (count, _count_num_sdp_media, GUINT_TO_POINTER (1), + &media_formats); + const gchar *expected_offer_setup[] = { "actpass", }; + VAL_SDP_INIT (offer_setup, on_sdp_media_setup, expected_offer_setup, &count); + const gchar *expected_offer_direction[] = { "sendrecv", }; + VAL_SDP_INIT (offer, on_sdp_media_direction, expected_offer_direction, + &offer_setup); + + t->on_negotiation_needed = NULL; + t->on_ice_candidate = NULL; + t->on_pad_added = _pad_added_fakesink; + + /* Setup sendrecv transceiver with VP8 and H264. + * The RTX's payload type shouldn't be 96 or 97 since those are already taken. */ + caps = gst_caps_from_string (VP8_RTP_CAPS (96) ";" H264_RTP_CAPS (97)); + direction = GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_SENDRECV; + g_signal_emit_by_name (t->webrtc1, "add-transceiver", direction, caps, + &trans); + gst_caps_unref (caps); + fail_unless (trans != NULL); + g_object_set (trans, "do-nack", TRUE, NULL); + gst_object_unref (trans); + + /* We don't really care about the answer in this test */ + test_validate_sdp (t, &offer, NULL); + + test_webrtc_free (t); +} + +GST_END_TEST; + static Suite * webrtcbin_suite (void) { @@ -6602,6 +6645,7 @@ webrtcbin_suite (void) sctpdec); } tcase_add_test (tc, test_offer_rollback); + tcase_add_test (tc, test_video_rtx_no_duplicate_payloads); } else { GST_WARNING ("Some required elements were not found. " "All media tests are disabled. nicesrc %p, nicesink %p, "