From e90859f4d8a23167fee9ef18779d3797ff510b60 Mon Sep 17 00:00:00 2001 From: Mathieu Duponchelle Date: Tue, 23 Nov 2021 20:12:06 +0100 Subject: [PATCH] webrtcbin: deduplicate extmaps When an extmap is defined twice for the same ID, firefox complains and errors out (chrome is smart enough to accept strict duplicates). To work around this, we deduplicate extmap attributes, and also error out when a different extmap is defined for the same ID. Part-of: --- .../gst-plugins-bad/ext/webrtc/gstwebrtcbin.c | 171 +++++++++++++++++- .../tests/check/elements/webrtcbin.c | 140 ++++++++++++++ 2 files changed, 309 insertions(+), 2 deletions(-) diff --git a/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c b/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c index 02ebc91c30..fd92e8a7e0 100644 --- a/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c +++ b/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c @@ -2766,6 +2766,146 @@ _add_fingerprint_to_media (GstWebRTCDTLSTransport * transport, g_free (val); } +static gchar * +_parse_extmap (GQuark field_id, const GValue * value, GError ** error) +{ + gchar *ret = NULL; + + if (G_VALUE_HOLDS_STRING (value)) { + ret = g_value_dup_string (value); + } else if (G_VALUE_HOLDS (value, GST_TYPE_ARRAY) + && gst_value_array_get_size (value) == 3) { + const GValue *val; + const gchar *direction, *extensionname, *extensionattributes; + + val = gst_value_array_get_value (value, 0); + direction = g_value_get_string (val); + + val = gst_value_array_get_value (value, 1); + extensionname = g_value_get_string (val); + + val = gst_value_array_get_value (value, 2); + extensionattributes = g_value_get_string (val); + + if (!extensionname || *extensionname == '\0') + goto done; + + if (direction && *direction != '\0' && extensionattributes + && *extensionattributes != '\0') { + ret = + g_strdup_printf ("/%s %s %s", direction, extensionname, + extensionattributes); + } else if (direction && *direction != '\0') { + ret = g_strdup_printf ("/%s %s", direction, extensionname); + } else if (extensionattributes && *extensionattributes != '\0') { + ret = g_strdup_printf ("%s %s", extensionname, extensionattributes); + } else { + ret = g_strdup (extensionname); + } + } + + if (!ret && error) { + gchar *val_str = gst_value_serialize (value); + + g_set_error (error, GST_WEBRTC_BIN_ERROR, + GST_WEBRTC_BIN_ERROR_CAPS_NEGOTIATION_FAILED, + "Invalid value for %s: %s", g_quark_to_string (field_id), val_str); + g_free (val_str); + } + +done: + return ret; +} + +typedef struct +{ + gboolean ret; + GstStructure *extmap; + GError **error; +} ExtmapData; + +static gboolean +_dedup_extmap_field (GQuark field_id, const GValue * value, ExtmapData * data) +{ + gboolean is_extmap = + g_str_has_prefix (g_quark_to_string (field_id), "extmap-"); + + if (!data->ret) + goto done; + + if (is_extmap) { + gchar *new_value = _parse_extmap (field_id, value, data->error); + + if (!new_value) { + data->ret = FALSE; + goto done; + } + + if (gst_structure_id_has_field (data->extmap, field_id)) { + gchar *old_value = + _parse_extmap (field_id, gst_structure_id_get_value (data->extmap, + field_id), NULL); + + g_assert (old_value); + + if (g_strcmp0 (new_value, old_value)) { + GST_ERROR + ("extmap contains different values for id %s (%s != %s)", + g_quark_to_string (field_id), old_value, new_value); + g_set_error (data->error, GST_WEBRTC_BIN_ERROR, + GST_WEBRTC_BIN_ERROR_CAPS_NEGOTIATION_FAILED, + "extmap contains different values for id %s (%s != %s)", + g_quark_to_string (field_id), old_value, new_value); + data->ret = FALSE; + } + + g_free (old_value); + + } + + if (data->ret) { + gst_structure_id_set_value (data->extmap, field_id, value); + } + + g_free (new_value); + } + +done: + return !is_extmap; +} + +static GstStructure * +_gather_extmap (GstCaps * caps, GError ** error) +{ + ExtmapData edata = + { TRUE, gst_structure_new_empty ("application/x-extmap"), error }; + guint i, n; + + n = gst_caps_get_size (caps); + + for (i = 0; i < n; i++) { + GstStructure *s = gst_caps_get_structure (caps, i); + + gst_structure_filter_and_map_in_place (s, + (GstStructureFilterMapFunc) _dedup_extmap_field, &edata); + + if (!edata.ret) { + gst_clear_structure (&edata.extmap); + break; + } + } + + return edata.extmap; +} + +static gboolean +_copy_field (GQuark field_id, const GValue * value, GstStructure * s) +{ + gst_structure_id_set_value (s, field_id, value); + + return TRUE; +} + /* based off https://tools.ietf.org/html/draft-ietf-rtcweb-jsep-18#section-5.2.1 */ static gboolean sdp_media_from_transceiver (GstWebRTCBin * webrtc, GstSDPMedia * media, @@ -2788,6 +2928,7 @@ sdp_media_from_transceiver (GstWebRTCBin * webrtc, GstSDPMedia * media, gchar *direction, *sdp_mid, *ufrag, *pwd; gboolean bundle_only; GstCaps *caps; + GstStructure *extmap; int i; if (trans->direction == GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_NONE @@ -2856,14 +2997,38 @@ sdp_media_from_transceiver (GstWebRTCBin * webrtc, GstSDPMedia * media, return FALSE; } + caps = gst_caps_make_writable (caps); + + /* When an extmap is defined twice for the same ID, firefox complains and + * errors out (chrome is smart enough to accept strict duplicates). + * + * To work around this, we deduplicate extmap attributes, and also error + * out when a different extmap is defined for the same ID. + * + * _gather_extmap will strip out all extmap- fields, which will then be + * added upon adding the first format for the media. + */ + extmap = _gather_extmap (caps, error); + + if (!extmap) { + GST_ERROR_OBJECT (webrtc, + "Failed to build extmap for transceiver %" GST_PTR_FORMAT, trans); + gst_caps_unref (caps); + return FALSE; + } + caps = _add_supported_attributes_to_caps (webrtc, WEBRTC_TRANSCEIVER (trans), caps); for (i = 0; i < gst_caps_get_size (caps); i++) { GstCaps *format = gst_caps_new_empty (); - const GstStructure *s = gst_caps_get_structure (caps, i); + GstStructure *s = gst_structure_copy (gst_caps_get_structure (caps, i)); - gst_caps_append_structure (format, gst_structure_copy (s)); + if (i == 0) { + gst_structure_foreach (extmap, (GstStructureForeachFunc) _copy_field, s); + } + + gst_caps_append_structure (format, s); GST_DEBUG_OBJECT (webrtc, "Adding %u-th caps %" GST_PTR_FORMAT " to %u-th media", i, format, media_idx); @@ -2875,6 +3040,8 @@ sdp_media_from_transceiver (GstWebRTCBin * webrtc, GstSDPMedia * media, gst_caps_unref (format); } + gst_clear_structure (&extmap); + { const GstStructure *s = gst_caps_get_structure (caps, 0); gint clockrate = -1; diff --git a/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c b/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c index 157f185c43..c53b1583e5 100644 --- a/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c +++ b/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c @@ -3490,6 +3490,19 @@ offer_set_produced_error (struct test_webrtc *t, GstElement * element, test_webrtc_signal_state_unlocked (t, STATE_CUSTOM); } +static void +offer_created_produced_error (struct test_webrtc *t, GstElement * element, + GstPromise * promise, gpointer user_data) +{ + const GstStructure *reply; + GError *error = NULL; + + reply = gst_promise_get_reply (promise); + fail_unless (gst_structure_get (reply, "error", G_TYPE_ERROR, &error, NULL)); + GST_INFO ("error produced: %s", error->message); + g_clear_error (&error); +} + GST_START_TEST (test_renego_lose_media_fails) { struct test_webrtc *t = create_audio_video_test (); @@ -3573,6 +3586,130 @@ GST_START_TEST (test_bundle_codec_preferences_rtx_no_duplicate_payloads) GST_END_TEST; +static void +on_sdp_media_no_duplicate_extmaps (struct test_webrtc *t, GstElement * element, + GstWebRTCSessionDescription * desc, gpointer user_data) +{ + const GstSDPMedia *media = gst_sdp_message_get_media (desc->sdp, 0); + + fail_unless (media != NULL); + + fail_unless_equals_string (gst_sdp_media_get_attribute_val_n (media, "extmap", + 0), "1 foobar"); + + fail_unless (gst_sdp_media_get_attribute_val_n (media, "extmap", 1) == NULL); +} + +/* In this test, we validate that identical extmaps for multiple formats + * in the caps of a single transceiver are deduplicated. This is necessary + * because Firefox will complain about duplicate extmap ids and fail negotiation + * otherwise. */ +GST_START_TEST (test_codec_preferences_no_duplicate_extmaps) +{ + struct test_webrtc *t = test_webrtc_new (); + GstWebRTCRTPTransceiver *trans; + GstWebRTCRTPTransceiverDirection direction; + VAL_SDP_INIT (extmaps, on_sdp_media_no_duplicate_extmaps, NULL, NULL); + GstCaps *caps; + GstStructure *s; + + caps = gst_caps_new_empty (); + + s = gst_structure_from_string (VP8_RTP_CAPS (96), NULL); + gst_structure_set (s, "extmap-1", G_TYPE_STRING, "foobar", NULL); + gst_caps_append_structure (caps, s); + s = gst_structure_from_string (H264_RTP_CAPS (97), NULL); + gst_structure_set (s, "extmap-1", G_TYPE_STRING, "foobar", NULL); + gst_caps_append_structure (caps, s); + + direction = GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_SENDONLY; + g_signal_emit_by_name (t->webrtc1, "add-transceiver", direction, caps, + &trans); + gst_caps_unref (caps); + fail_unless (trans != NULL); + + t->on_negotiation_needed = NULL; + t->on_pad_added = NULL; + t->on_ice_candidate = NULL; + + test_validate_sdp (t, &extmaps, NULL); + + test_webrtc_free (t); +} + +GST_END_TEST; + +/* In this test, we validate that trying to use different values + * for the same extmap id in multiple formats in the caps of a + * single transceiver errors out when creating the offer. */ +GST_START_TEST (test_codec_preferences_incompatible_extmaps) +{ + struct test_webrtc *t = test_webrtc_new (); + GstWebRTCRTPTransceiver *trans; + GstWebRTCRTPTransceiverDirection direction; + GstCaps *caps; + GstStructure *s; + + caps = gst_caps_new_empty (); + + s = gst_structure_from_string (VP8_RTP_CAPS (96), NULL); + gst_structure_set (s, "extmap-1", G_TYPE_STRING, "foobar", NULL); + gst_caps_append_structure (caps, s); + s = gst_structure_from_string (H264_RTP_CAPS (97), NULL); + gst_structure_set (s, "extmap-1", G_TYPE_STRING, "foobaz", NULL); + gst_caps_append_structure (caps, s); + + direction = GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_SENDONLY; + g_signal_emit_by_name (t->webrtc1, "add-transceiver", direction, caps, + &trans); + gst_caps_unref (caps); + fail_unless (trans != NULL); + + t->on_negotiation_needed = NULL; + t->on_pad_added = NULL; + t->on_ice_candidate = NULL; + t->on_offer_created = offer_created_produced_error; + + test_validate_sdp_full (t, NULL, NULL, STATE_OFFER_CREATED, TRUE); + + test_webrtc_free (t); +} + +GST_END_TEST; + +/* In this test, we validate that extmap values must be of the correct type */ +GST_START_TEST (test_codec_preferences_invalid_extmap) +{ + struct test_webrtc *t = test_webrtc_new (); + GstWebRTCRTPTransceiver *trans; + GstWebRTCRTPTransceiverDirection direction; + GstCaps *caps; + GstStructure *s; + + caps = gst_caps_new_empty (); + + s = gst_structure_from_string (VP8_RTP_CAPS (96), NULL); + gst_structure_set (s, "extmap-1", G_TYPE_INT, 42, NULL); + gst_caps_append_structure (caps, s); + + direction = GST_WEBRTC_RTP_TRANSCEIVER_DIRECTION_SENDONLY; + g_signal_emit_by_name (t->webrtc1, "add-transceiver", direction, caps, + &trans); + gst_caps_unref (caps); + fail_unless (trans != NULL); + + t->on_negotiation_needed = NULL; + t->on_pad_added = NULL; + t->on_ice_candidate = NULL; + t->on_offer_created = offer_created_produced_error; + + test_validate_sdp_full (t, NULL, NULL, STATE_OFFER_CREATED, TRUE); + + test_webrtc_free (t); +} + +GST_END_TEST; + GST_START_TEST (test_reject_request_pad) { struct test_webrtc *t = test_webrtc_new (); @@ -4275,6 +4412,9 @@ webrtcbin_suite (void) tcase_add_test (tc, test_codec_preferences_negotiation_sinkpad); tcase_add_test (tc, test_codec_preferences_negotiation_srcpad); tcase_add_test (tc, test_codec_preferences_in_on_new_transceiver); + tcase_add_test (tc, test_codec_preferences_no_duplicate_extmaps); + tcase_add_test (tc, test_codec_preferences_incompatible_extmaps); + tcase_add_test (tc, test_codec_preferences_invalid_extmap); if (sctpenc && sctpdec) { tcase_add_test (tc, test_data_channel_create); tcase_add_test (tc, test_data_channel_remote_notify);