From 18f6fec49f0985d2e187cf3d84c2fd1c49dd3839 Mon Sep 17 00:00:00 2001 From: Philippe Normand Date: Fri, 28 Mar 2025 12:19:20 +0000 Subject: [PATCH] webrtcbin: Make mid optional in offers and answers The mid attribute is not strictly required. Two new tests cover this change, they remove the mid and group attributes from the SDP offers and answers. Part-of: --- .../gst-plugins-bad/ext/webrtc/gstwebrtcbin.c | 43 +++-- .../gst-plugins-bad/ext/webrtc/webrtcsdp.c | 22 +-- .../tests/check/elements/webrtcbin.c | 162 ++++++++++++++++++ 3 files changed, 197 insertions(+), 30 deletions(-) diff --git a/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c b/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c index b2d343f20e..5a073e8bba 100644 --- a/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c +++ b/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c @@ -4540,8 +4540,6 @@ _create_answer_task (GstWebRTCBin * webrtc, const GstStructure * options, } mid = gst_sdp_media_get_attribute_val (media, "mid"); - /* XXX: not strictly required but a lot of functionality requires a mid */ - g_assert (mid); /* set the a=setup: attribute */ offer_setup = _get_dtls_setup_from_media (offer_media); @@ -4603,12 +4601,22 @@ _create_answer_task (GstWebRTCBin * webrtc, const GstStructure * options, _remove_optional_offer_fields (offer_caps); - rtp_trans = _find_transceiver_for_mid (webrtc, mid); - if (!rtp_trans) { - g_set_error (error, GST_WEBRTC_ERROR, GST_WEBRTC_ERROR_INVALID_STATE, - "Transceiver for media with mid %s not found", mid); - gst_caps_unref (offer_caps); - goto rejected; + if (mid) { + rtp_trans = _find_transceiver_for_mid (webrtc, mid); + if (!rtp_trans) { + g_set_error (error, GST_WEBRTC_ERROR, GST_WEBRTC_ERROR_INVALID_STATE, + "Transceiver for media with mid %s not found", mid); + gst_caps_unref (offer_caps); + goto rejected; + } + } else { + rtp_trans = _find_transceiver_for_mline (webrtc, i); + if (!rtp_trans) { + g_set_error (error, GST_WEBRTC_ERROR, GST_WEBRTC_ERROR_INVALID_STATE, + "Transceiver for media with mline %u not found", i); + gst_caps_unref (offer_caps); + goto rejected; + } } GstCaps *current_caps = _find_codec_preferences (webrtc, rtp_trans, i, error); @@ -4623,7 +4631,8 @@ _create_answer_task (GstWebRTCBin * webrtc, const GstStructure * options, const gchar *last_mid = gst_sdp_media_get_attribute_val (last_media, "mid"); /* FIXME: assumes no shenanigans with recycling transceivers */ - g_assert (g_strcmp0 (mid, last_mid) == 0); + if (mid != last_mid) + g_assert (g_strcmp0 (mid, last_mid) == 0); if (!current_caps) current_caps = _rtp_caps_from_media (last_media); } @@ -6433,11 +6442,10 @@ _create_and_associate_transceivers_from_sdp (GstWebRTCBin * webrtc, mid = gst_sdp_media_get_attribute_val (media, "mid"); direction = _get_direction_from_media (media); - /* XXX: not strictly required but a lot of functionality requires a mid */ - if (!mid) { - g_set_error (error, GST_WEBRTC_ERROR, GST_WEBRTC_ERROR_SDP_SYNTAX_ERROR, - "Missing mid attribute in media"); - goto out; + if (mid) { + trans = _find_transceiver_for_mid (webrtc, mid); + } else { + trans = _find_transceiver_for_mline (webrtc, i); } if (bundled) @@ -6445,7 +6453,6 @@ _create_and_associate_transceivers_from_sdp (GstWebRTCBin * webrtc, else transport_idx = i; - trans = _find_transceiver_for_mid (webrtc, mid); if (sd->source == SDP_LOCAL) { /* If the media description was not yet associated with an RTCRtpTransceiver object then run the following steps: */ @@ -6469,8 +6476,10 @@ _create_and_associate_transceivers_from_sdp (GstWebRTCBin * webrtc, } trans->mline = i; /* Set transceiver.[[Mid]] to transceiver.[[JsepMid]] */ - g_free (trans->mid); - trans->mid = g_strdup (mid); + g_clear_pointer (&trans->mid, g_free); + if (mid) { + trans->mid = g_strdup (mid); + } g_object_notify (G_OBJECT (trans), "mid"); /* If transceiver.[[Stopped]] is true, abort these sub steps */ if (trans->stopped) diff --git a/subprojects/gst-plugins-bad/ext/webrtc/webrtcsdp.c b/subprojects/gst-plugins-bad/ext/webrtc/webrtcsdp.c index 0ece0c6250..4e2636be2c 100644 --- a/subprojects/gst-plugins-bad/ext/webrtc/webrtcsdp.c +++ b/subprojects/gst-plugins-bad/ext/webrtc/webrtcsdp.c @@ -193,16 +193,10 @@ _media_has_attribute_key (const GstSDPMedia * media, const gchar * key) } static gboolean -_media_has_mid (const GstSDPMedia * media, guint media_idx, GError ** error) +_media_has_mid (const GstSDPMedia * media, guint media_idx) { const gchar *mid = gst_sdp_media_get_attribute_val (media, "mid"); - if (IS_EMPTY_SDP_ATTRIBUTE (mid)) { - g_set_error (error, GST_WEBRTC_ERROR, GST_WEBRTC_ERROR_SDP_SYNTAX_ERROR, - "media %u is missing or contains an empty \'mid\' attribute", - media_idx); - return FALSE; - } - return TRUE; + return !IS_EMPTY_SDP_ATTRIBUTE (mid); } const gchar * @@ -308,11 +302,13 @@ validate_sdp (GstWebRTCSignalingState state, SDPSource source, const GstSDPMedia *media = gst_sdp_message_get_media (sdp->sdp, i); const gchar *mid; gboolean media_in_bundle = FALSE; - if (!_media_has_mid (media, i, error)) - goto fail; - mid = gst_sdp_media_get_attribute_val (media, "mid"); - media_in_bundle = is_bundle - && g_strv_contains ((const gchar **) group_members, mid); + + if (_media_has_mid (media, i)) { + mid = gst_sdp_media_get_attribute_val (media, "mid"); + media_in_bundle = + is_bundle && g_strv_contains ((const gchar **) group_members, mid); + } + if (!_media_get_ice_ufrag (sdp->sdp, i)) { g_set_error (error, GST_WEBRTC_ERROR, GST_WEBRTC_ERROR_SDP_SYNTAX_ERROR, "media %u is missing or contains an empty \'ice-ufrag\' attribute", diff --git a/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c b/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c index c2bb5edf11..bb06c3dc94 100644 --- a/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c +++ b/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c @@ -5044,6 +5044,166 @@ a=group:BUNDLE \r\n\ GST_END_TEST; +static GstWebRTCSessionDescription * +remove_sdp_attributes (const GstWebRTCSessionDescription * desc, guint n_attrs, + const char *const *attributes) +{ + GstSDPMessage *sdp; + guint total_medias = gst_sdp_message_medias_len (desc->sdp); + + gst_sdp_message_copy (desc->sdp, &sdp); + for (guint i = 0; i < total_medias; i++) { + gst_sdp_message_remove_media (sdp, i); + } + + for (guint i = 0; i < total_medias; i++) { + const GstSDPMedia *media = gst_sdp_message_get_media (desc->sdp, i); + guint total_attributes = gst_sdp_media_attributes_len (media); + GstSDPMedia *new_media; + + gst_sdp_media_copy (media, &new_media); + for (guint ii = 0; ii < total_attributes; ii++) { + const GstSDPAttribute *attribute = + gst_sdp_media_get_attribute (new_media, ii); + for (guint iii = 0; iii < n_attrs; iii++) { + if (!g_strcmp0 (attribute->key, attributes[iii])) { + gst_sdp_media_remove_attribute (new_media, ii); + ii--; + total_attributes--; + break; + } + } + } + gst_sdp_message_add_media (sdp, new_media); + gst_sdp_media_free (new_media); + } + + return gst_webrtc_session_description_new (desc->type, sdp); +} + +static void +do_missing_mid_test (gboolean is_offer) +{ + struct test_webrtc *t = test_webrtc_new (); + const gchar *attributes_to_remove[] = { "mid", "group" }; + GstWebRTCSessionDescription *modified_desc = NULL; + GstPromise *promise; + GstPromiseResult res; + const GstStructure *s; + GstWebRTCSessionDescription *desc; + GstHarness *h1; + + t->on_negotiation_needed = NULL; + t->on_ice_candidate = NULL; + t->on_pad_added = _pad_added_fakesink; + + h1 = gst_harness_new_with_element (t->webrtc1, "sink_0", NULL); + add_audio_test_src_harness (h1, 0xDEADBEEF); + t->harnesses = g_list_prepend (t->harnesses, h1); + + fail_if (gst_element_set_state (t->webrtc1, GST_STATE_READY) == + GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, GST_STATE_READY) == + GST_STATE_CHANGE_FAILURE); + + promise = gst_promise_new (); + g_signal_emit_by_name (t->webrtc1, "create-offer", NULL, promise); + res = gst_promise_wait (promise); + fail_unless_equals_int (res, GST_PROMISE_RESULT_REPLIED); + s = gst_promise_get_reply (promise); + fail_unless (s != NULL); + fail_if (gst_structure_has_field (s, "error")); + gst_structure_get (s, "offer", GST_TYPE_WEBRTC_SESSION_DESCRIPTION, &desc, + NULL); + fail_unless (desc != NULL); + gst_promise_unref (promise); + + promise = gst_promise_new (); + g_signal_emit_by_name (t->webrtc1, "set-local-description", desc, promise); + res = gst_promise_wait (promise); + fail_unless_equals_int (res, GST_PROMISE_RESULT_REPLIED); + s = gst_promise_get_reply (promise); + fail_if (s && gst_structure_has_field (s, "error")); + gst_promise_unref (promise); + + if (is_offer) { + modified_desc = + remove_sdp_attributes (desc, G_N_ELEMENTS (attributes_to_remove), + attributes_to_remove); + } + promise = gst_promise_new (); + g_signal_emit_by_name (t->webrtc2, "set-remote-description", + modified_desc ? modified_desc : desc, promise); + res = gst_promise_wait (promise); + fail_unless_equals_int (res, GST_PROMISE_RESULT_REPLIED); + s = gst_promise_get_reply (promise); + fail_if (s && gst_structure_has_field (s, "error")); + gst_promise_unref (promise); + + g_clear_pointer (&modified_desc, gst_webrtc_session_description_free); + gst_webrtc_session_description_free (desc); + + promise = gst_promise_new (); + g_signal_emit_by_name (t->webrtc2, "create-answer", NULL, promise); + res = gst_promise_wait (promise); + fail_unless_equals_int (res, GST_PROMISE_RESULT_REPLIED); + s = gst_promise_get_reply (promise); + fail_unless (s != NULL); + s = gst_promise_get_reply (promise); + fail_unless (s != NULL); + fail_if (gst_structure_has_field (s, "error")); + gst_structure_get (s, "answer", GST_TYPE_WEBRTC_SESSION_DESCRIPTION, &desc, + NULL); + fail_unless (desc != NULL); + gst_promise_unref (promise); + + promise = gst_promise_new (); + g_signal_emit_by_name (t->webrtc2, "set-local-description", desc, promise); + res = gst_promise_wait (promise); + fail_unless_equals_int (res, GST_PROMISE_RESULT_REPLIED); + s = gst_promise_get_reply (promise); + fail_if (s && gst_structure_has_field (s, "error")); + gst_promise_unref (promise); + + if (!is_offer) { + modified_desc = + remove_sdp_attributes (desc, G_N_ELEMENTS (attributes_to_remove), + attributes_to_remove); + } + promise = gst_promise_new (); + g_signal_emit_by_name (t->webrtc1, "set-remote-description", + modified_desc ? modified_desc : desc, promise); + res = gst_promise_wait (promise); + fail_unless_equals_int (res, GST_PROMISE_RESULT_REPLIED); + s = gst_promise_get_reply (promise); + fail_if (s && gst_structure_has_field (s, "error")); + gst_promise_unref (promise); + + g_clear_pointer (&modified_desc, gst_webrtc_session_description_free); + gst_webrtc_session_description_free (desc); + + fail_if (gst_element_set_state (t->webrtc1, GST_STATE_PLAYING) == + GST_STATE_CHANGE_FAILURE); + fail_if (gst_element_set_state (t->webrtc2, GST_STATE_PLAYING) == + GST_STATE_CHANGE_FAILURE); + + test_webrtc_free (t); +} + +GST_START_TEST (test_missing_mid_in_offer) +{ + do_missing_mid_test (TRUE); +} + +GST_END_TEST; + +GST_START_TEST (test_missing_mid_in_answer) +{ + do_missing_mid_test (FALSE); +} + +GST_END_TEST; + static void new_jitterbuffer_set_fast_start (GstElement * rtpbin, GstElement * rtpjitterbuffer, guint session_id, guint ssrc, @@ -6624,6 +6784,8 @@ webrtcbin_suite (void) tcase_add_test (tc, test_sdp_session_setup_attribute); tcase_add_test (tc, test_rtp_header_extension_sendonly_recvonly_pair); tcase_add_test (tc, test_invalid_bundle_in_pending_remote_description); + tcase_add_test (tc, test_missing_mid_in_offer); + tcase_add_test (tc, test_missing_mid_in_answer); if (sctpenc && sctpdec) { tcase_add_test (tc, test_data_channel_create); tcase_add_test (tc, test_data_channel_create_two_channels);