From: Matthew Waters Date: Mon, 1 Mar 2021 09:53:53 +0000 (+1100) Subject: webrtc: don't generate duplicate rtx payloads when bundle-policy is set X-Git-Tag: submit/tizen/20210906.055733~1^2~1 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=8157d38361c152b5d91d4279ed6aa6f3f665ae60;p=platform%2Fupstream%2Fgst-plugins-bad.git webrtc: don't generate duplicate rtx payloads when bundle-policy is set It was possible to generate a SDP that had an RTX payload type that matched one of the media payload types when providing caps via codec_preferences without any sink pads. Fixes m=video 9 UDP/TLS/RTP/SAVPF 96 ... a=rtpmap:96 VP8/90000 a=rtcp-fb:96 nack pli a=fmtp:96 apt=96 Change-Id: I0b2d360cb40abcf5a61035673d43f15a40fc09fa Part-of: --- diff --git a/ext/webrtc/gstwebrtcbin.c b/ext/webrtc/gstwebrtcbin.c index f28565888..3b7a3da79 100644 --- a/ext/webrtc/gstwebrtcbin.c +++ b/ext/webrtc/gstwebrtcbin.c @@ -2650,6 +2650,7 @@ gather_pad_pt (GstWebRTCBinPad * pad, GArray * reserved_pts) gint pt; if (gst_structure_get_int (s, "payload", &pt)) { + GST_TRACE_OBJECT (pad, "have reserved pt %u from received caps", pt); g_array_append_val (reserved_pts, pt); } } @@ -2660,11 +2661,32 @@ gather_reserved_pts (GstWebRTCBin * webrtc) { GstElement *element = GST_ELEMENT (webrtc); GArray *reserved_pts = g_array_new (FALSE, FALSE, sizeof (guint)); + guint i; GST_OBJECT_LOCK (webrtc); g_list_foreach (element->sinkpads, (GFunc) gather_pad_pt, reserved_pts); g_list_foreach (webrtc->priv->pending_pads, (GFunc) gather_pad_pt, reserved_pts); + + for (i = 0; i < webrtc->priv->transceivers->len; i++) { + GstWebRTCRTPTransceiver *trans; + + trans = g_ptr_array_index (webrtc->priv->transceivers, i); + if (trans->codec_preferences) { + guint j, n; + gint pt; + + n = gst_caps_get_size (trans->codec_preferences); + for (j = 0; j < n; j++) { + GstStructure *s = gst_caps_get_structure (trans->codec_preferences, j); + if (gst_structure_get_int (s, "payload", &pt)) { + GST_TRACE_OBJECT (trans, "have reserved pt %u from codec preferences", + pt); + g_array_append_val (reserved_pts, pt); + } + } + } + } GST_OBJECT_UNLOCK (webrtc); return reserved_pts; diff --git a/tests/check/elements/webrtcbin.c b/tests/check/elements/webrtcbin.c index f6b46aa89..488ba6238 100644 --- a/tests/check/elements/webrtcbin.c +++ b/tests/check/elements/webrtcbin.c @@ -2945,6 +2945,98 @@ GST_START_TEST (test_renego_lose_media_fails) GST_END_TEST; +static void +on_sdp_media_no_duplicate_payloads (struct test_webrtc *t, GstElement * element, + GstWebRTCSessionDescription * desc, gpointer user_data) +{ + GArray *media_formats = g_array_new (FALSE, FALSE, sizeof (int)); + int i, j, k; + + for (i = 0; i < gst_sdp_message_medias_len (desc->sdp); i++) { + const GstSDPMedia *media = gst_sdp_message_get_media (desc->sdp, i); + + for (j = 0; j < gst_sdp_media_formats_len (media); j++) { + int pt = atoi (gst_sdp_media_get_format (media, j)); + for (k = 0; k < media_formats->len; k++) { + int val = g_array_index (media_formats, int, k); + if (pt == val) + fail ("found an unexpected duplicate payload type %u within media %u", + pt, i); + } + g_array_append_val (media_formats, pt); + } + } + + g_array_free (media_formats, TRUE); +} + +static void +on_sdp_media_count_formats (struct test_webrtc *t, GstElement * element, + GstWebRTCSessionDescription * desc, gpointer user_data) +{ + guint *expected_n_media_formats = user_data; + int i; + + for (i = 0; i < gst_sdp_message_medias_len (desc->sdp); i++) { + const GstSDPMedia *media = gst_sdp_message_get_media (desc->sdp, i); + fail_unless_equals_int (gst_sdp_media_formats_len (media), + expected_n_media_formats[i]); + } +} + +GST_START_TEST (test_bundle_codec_preferences_rtx_no_duplicate_payloads) +{ + struct test_webrtc *t = test_webrtc_new (); + GstWebRTCRTPTransceiverDirection direction; + GstWebRTCRTPTransceiver *trans; + const gchar *expected_offer[] = { "recvonly" }; + const gchar *expected_answer[] = { "sendonly" }; + guint offer_media_format_count[] = { 2, }; + guint answer_media_format_count[] = { 1, }; + VAL_SDP_INIT (payloads, on_sdp_media_no_duplicate_payloads, NULL, NULL); + VAL_SDP_INIT (offer_media_formats, on_sdp_media_count_formats, + offer_media_format_count, &payloads); + VAL_SDP_INIT (answer_media_formats, on_sdp_media_count_formats, + answer_media_format_count, &payloads); + VAL_SDP_INIT (offer, on_sdp_media_direction, expected_offer, + &offer_media_formats); + VAL_SDP_INIT (answer, on_sdp_media_direction, expected_answer, + &answer_media_formats); + GstCaps *caps; + GstHarness *h; + + /* add a transceiver that will only receive an opus stream and check that + * the created offer is marked as recvonly */ + t->on_negotiation_needed = NULL; + t->on_ice_candidate = NULL; + t->on_pad_added = _pad_added_fakesink; + + gst_util_set_object_arg (G_OBJECT (t->webrtc1), "bundle-policy", + "max-bundle"); + gst_util_set_object_arg (G_OBJECT (t->webrtc2), "bundle-policy", + "max-bundle"); + + /* setup recvonly transceiver */ + caps = gst_caps_from_string (VP8_RTP_CAPS (96)); + direction = GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_RECVONLY; + g_signal_emit_by_name (t->webrtc1, "add-transceiver", direction, caps, + &trans); + g_object_set (GST_OBJECT (trans), "do-nack", TRUE, NULL); + gst_caps_unref (caps); + fail_unless (trans != NULL); + gst_object_unref (trans); + + /* setup sendonly peer */ + h = gst_harness_new_with_element (t->webrtc2, "sink_0", NULL); + add_fake_video_src_harness (h, 96); + t->harnesses = g_list_prepend (t->harnesses, h); + test_validate_sdp (t, &offer, &answer); + + test_webrtc_free (t); +} + +GST_END_TEST; + static Suite * webrtcbin_suite (void) { @@ -2987,6 +3079,8 @@ webrtcbin_suite (void) tcase_add_test (tc, test_bundle_max_compat_max_bundle_renego_add_stream); tcase_add_test (tc, test_renego_transceiver_set_direction); tcase_add_test (tc, test_renego_lose_media_fails); + tcase_add_test (tc, + test_bundle_codec_preferences_rtx_no_duplicate_payloads); if (sctpenc && sctpdec) { tcase_add_test (tc, test_data_channel_create); tcase_add_test (tc, test_data_channel_remote_notify);