From 1caa041c91b16f3bb1290410c4c391c33c608993 Mon Sep 17 00:00:00 2001 From: Philippe Normand Date: Mon, 27 May 2024 12:28:44 +0100 Subject: [PATCH] webrtcbin: Allow session level setup attribute in SDP An SDP answer can declare its setup attribute at the session level or at the media level. Until this patch we were validating only the latter case and an assert was raised in the former case. Part-of: --- .../gst-plugins-bad/ext/webrtc/gstwebrtcbin.c | 8 +++ .../gst-plugins-bad/ext/webrtc/webrtcsdp.c | 54 +++++++++++--- .../gst-plugins-bad/ext/webrtc/webrtcsdp.h | 2 + .../tests/check/elements/webrtcbin.c | 71 +++++++++++++++++++ 4 files changed, 125 insertions(+), 10 deletions(-) diff --git a/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c b/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c index b40b8e26be..6bc381af32 100644 --- a/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c +++ b/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c @@ -5737,7 +5737,15 @@ _update_transceiver_from_sdp_media (GstWebRTCBin * webrtc, GstWebRTCDTLSSetup local_setup, remote_setup; local_setup = _get_dtls_setup_from_media (local_media); + if (local_setup == GST_WEBRTC_DTLS_SETUP_NONE) + local_setup = + _get_dtls_setup_from_session (webrtc->current_local_description->sdp); + remote_setup = _get_dtls_setup_from_media (remote_media); + if (remote_setup == GST_WEBRTC_DTLS_SETUP_NONE) + remote_setup = + _get_dtls_setup_from_session (webrtc-> + current_remote_description->sdp); new_setup = _get_final_setup (local_setup, remote_setup); if (new_setup == GST_WEBRTC_DTLS_SETUP_NONE) { g_set_error (error, GST_WEBRTC_ERROR, GST_WEBRTC_ERROR_SDP_SYNTAX_ERROR, diff --git a/subprojects/gst-plugins-bad/ext/webrtc/webrtcsdp.c b/subprojects/gst-plugins-bad/ext/webrtc/webrtcsdp.c index 221ac4eb9a..67c8143d99 100644 --- a/subprojects/gst-plugins-bad/ext/webrtc/webrtcsdp.c +++ b/subprojects/gst-plugins-bad/ext/webrtc/webrtcsdp.c @@ -236,9 +236,20 @@ _media_get_ice_pwd (const GstSDPMessage * msg, guint media_idx) } static gboolean -_media_has_setup (const GstSDPMedia * media, guint media_idx, GError ** error) +_validate_setup_attribute (const gchar * setup, GError ** error) { static const gchar *valid_setups[] = { "actpass", "active", "passive", NULL }; + if (!g_strv_contains (valid_setups, setup)) { + g_set_error (error, GST_WEBRTC_ERROR, GST_WEBRTC_ERROR_SDP_SYNTAX_ERROR, + "SDP contains unknown \'setup\' attribute, \'%s\'", setup); + return FALSE; + } + return TRUE; +} + +static gboolean +_media_has_setup (const GstSDPMedia * media, guint media_idx, GError ** error) +{ const gchar *setup = gst_sdp_media_get_attribute_val (media, "setup"); if (IS_EMPTY_SDP_ATTRIBUTE (setup)) { g_set_error (error, GST_WEBRTC_ERROR, GST_WEBRTC_ERROR_SDP_SYNTAX_ERROR, @@ -246,13 +257,7 @@ _media_has_setup (const GstSDPMedia * media, guint media_idx, GError ** error) media_idx); return FALSE; } - if (!g_strv_contains (valid_setups, setup)) { - g_set_error (error, GST_WEBRTC_ERROR, GST_WEBRTC_ERROR_SDP_SYNTAX_ERROR, - "media %u contains unknown \'setup\' attribute, \'%s\'", media_idx, - setup); - return FALSE; - } - return TRUE; + return _validate_setup_attribute (setup, error); } #if 0 @@ -273,9 +278,11 @@ gboolean validate_sdp (GstWebRTCSignalingState state, SDPSource source, GstWebRTCSessionDescription * sdp, GError ** error) { - const gchar *group, *bundle_ice_ufrag = NULL, *bundle_ice_pwd = NULL; + const gchar *group, *bundle_ice_ufrag = NULL, *bundle_ice_pwd = NULL, *setup = + NULL; gchar **group_members = NULL; gboolean is_bundle = FALSE; + gboolean has_session_setup = FALSE; int i; if (!_check_valid_state_for_sdp_change (state, source, sdp->type, error)) @@ -290,6 +297,13 @@ validate_sdp (GstWebRTCSignalingState state, SDPSource source, if (is_bundle) group_members = g_strsplit (&group[6], " ", -1); + setup = gst_sdp_message_get_attribute_val (sdp->sdp, "setup"); + if (setup) { + if (!_validate_setup_attribute (setup, error)) + return FALSE; + has_session_setup = TRUE; + } + for (i = 0; i < gst_sdp_message_medias_len (sdp->sdp); i++) { const GstSDPMedia *media = gst_sdp_message_get_media (sdp->sdp, i); const gchar *mid; @@ -310,7 +324,7 @@ validate_sdp (GstWebRTCSignalingState state, SDPSource source, "media %u is missing or contains an empty \'ice-pwd\' attribute", i); goto fail; } - if (!_media_has_setup (media, i, error)) + if (!has_session_setup && !_media_has_setup (media, i, error)) goto fail; /* check parameters in bundle are the same */ if (media_in_bundle) { @@ -527,6 +541,26 @@ _get_dtls_setup_from_media (const GstSDPMedia * media) return SETUP (NONE); } +GstWebRTCDTLSSetup +_get_dtls_setup_from_session (const GstSDPMessage * sdp) +{ + const gchar *attr = gst_sdp_message_get_attribute_val (sdp, "setup"); + if (!attr) { + GST_LOG ("no setup attribute in session"); + return SETUP (NONE); + } + if (g_strcmp0 (attr, "actpass") == 0) { + return SETUP (ACTPASS); + } else if (g_strcmp0 (attr, "active") == 0) { + return SETUP (ACTIVE); + } else if (g_strcmp0 (attr, "passive") == 0) { + return SETUP (PASSIVE); + } + + GST_ERROR ("unknown setup value %s", attr); + return SETUP (NONE); +} + GstWebRTCDTLSSetup _intersect_dtls_setup (GstWebRTCDTLSSetup offer) { diff --git a/subprojects/gst-plugins-bad/ext/webrtc/webrtcsdp.h b/subprojects/gst-plugins-bad/ext/webrtc/webrtcsdp.h index c55709b504..80d21203c2 100644 --- a/subprojects/gst-plugins-bad/ext/webrtc/webrtcsdp.h +++ b/subprojects/gst-plugins-bad/ext/webrtc/webrtcsdp.h @@ -58,6 +58,8 @@ GstWebRTCRTPTransceiverDirection _get_final_direction (Gs G_GNUC_INTERNAL GstWebRTCDTLSSetup _get_dtls_setup_from_media (const GstSDPMedia * media); G_GNUC_INTERNAL +GstWebRTCDTLSSetup _get_dtls_setup_from_session (const GstSDPMessage * sdp); +G_GNUC_INTERNAL GstWebRTCDTLSSetup _intersect_dtls_setup (GstWebRTCDTLSSetup offer); G_GNUC_INTERNAL void _media_replace_setup (GstSDPMedia * media, diff --git a/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c b/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c index bccb8cef57..58faff1c7e 100644 --- a/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c +++ b/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c @@ -5842,6 +5842,76 @@ GST_START_TEST (test_ice_end_of_candidates) GST_END_TEST; +static void +_set_setup_session_attr_on_answer (struct test_webrtc *t, GstElement * element, + GstPromise * promise, gpointer user_data) +{ + GstSDPMessage *sdp; + GstSDPMessage *modified_sdp = NULL; + const GstSDPMedia *media; + GstSDPMedia *modified_media; + const gchar *attr; + + if (TEST_IS_OFFER_ELEMENT (t, element)) + return; + + sdp = t->answer_desc->sdp; + media = gst_sdp_message_get_media (sdp, 0); + attr = gst_sdp_media_get_attribute_val (media, "setup"); + + /* Remove the setup attribute from first media */ + gst_sdp_media_copy (media, &modified_media); + for (unsigned index = 0; + index < gst_sdp_media_attributes_len (modified_media); index++) { + const GstSDPAttribute *current = + gst_sdp_media_get_attribute (modified_media, index); + if (!g_str_equal (current->key, "setup")) + continue; + gst_sdp_media_remove_attribute (modified_media, index); + break; + } + + gst_sdp_message_copy (sdp, &modified_sdp); + + /* Add session-level setup attribute to modified answer */ + gst_sdp_message_add_attribute (modified_sdp, "setup", attr); + + /* Replace first media of answer with a media without session attribute */ + gst_sdp_message_remove_media (modified_sdp, 0); + gst_sdp_message_add_media (modified_sdp, modified_media); + gst_sdp_media_free (modified_media); + + gst_sdp_message_free (sdp); + t->answer_desc->sdp = modified_sdp; +} + +static void +_offer_created_do_nothing (struct test_webrtc *t, GstElement * element, + GstPromise * promise, gpointer user_data) +{ +} + +GST_START_TEST (test_sdp_session_setup_attribute) +{ + struct test_webrtc *t = create_audio_test (); + + t->on_offer_created = _offer_created_do_nothing; + t->on_answer_created = _set_setup_session_attr_on_answer; + + 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); + test_webrtc_create_offer (t); + test_webrtc_wait_for_state_mask (t, 1 << STATE_ANSWER_SET); + + test_webrtc_wait_for_ice_gathering_complete (t); + + test_webrtc_free (t); +} + +GST_END_TEST; + static Suite * webrtcbin_suite (void) { @@ -5908,6 +5978,7 @@ webrtcbin_suite (void) tcase_add_test (tc, test_add_turn_server); tcase_add_test (tc, test_msid); tcase_add_test (tc, test_ice_end_of_candidates); + tcase_add_test (tc, test_sdp_session_setup_attribute); if (sctpenc && sctpdec) { tcase_add_test (tc, test_data_channel_create); tcase_add_test (tc, test_data_channel_create_two_channels);