webrtcbin: fix msid line and allow customization

From https://datatracker.ietf.org/doc/html/draft-ietf-mmusic-msid-16:

> Multiple media descriptions with the same value for msid-id and
> msid-appdata are not permitted.

Our previous implementation of simply using the CNAME as the msid
identifier and the name of the transceiver as the msid appdata was
misguided and incorrect, and created issues when bundling multiple
video streams together: the ontrack event was emitted with the same
streams for the two bundled medias, at least in Firefox.

Instead, use the transceiver name as the identifier, and expose
a msid-appdata property on transceivers to allow for further
customization by the application. When the property is not set,
msid-appdata can be left empty as it is specified as optional.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/2003>

diff --git a/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c b/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c
index be0d77b..bcea449 100644 (file)
--- a/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c
+++ b/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c
@@ -2724,6 +2724,23 @@ typedef struct
   WebRTCTransceiver *trans;
 } RtxSsrcData;
 
+/* https://tools.ietf.org/html/draft-ietf-mmusic-msid-16 */
+static gchar *
+_construct_msid (WebRTCTransceiver * trans, guint ssrc)
+{
+  gchar *ret;
+
+  if (trans->msid_appdata) {
+    ret =
+        g_strdup_printf ("%u msid:%s %s", ssrc, GST_OBJECT_NAME (trans),
+        trans->msid_appdata);
+  } else {
+    ret = g_strdup_printf ("%u msid:%s", ssrc, GST_OBJECT_NAME (trans));
+  }
+
+  return ret;
+}
+
 static gboolean
 _media_add_rtx_ssrc (GQuark field_id, const GValue * value, RtxSsrcData * data)
 {
@@ -2735,10 +2752,8 @@ _media_add_rtx_ssrc (GQuark field_id, const GValue * value, RtxSsrcData * data)
   /* http://www.freesoft.org/CIE/RFC/1889/24.htm */
   cname = gst_structure_get_string (sdes, "cname");
 
-  /* https://tools.ietf.org/html/draft-ietf-mmusic-msid-16 */
-  str =
-      g_strdup_printf ("%u msid:%s %s", g_value_get_uint (value),
-      cname, GST_OBJECT_NAME (data->trans));
+  str = _construct_msid (data->trans, g_value_get_uint (value));
+
   gst_sdp_media_add_attribute (data->media, "ssrc", str);
   g_free (str);
 
@@ -2775,10 +2790,8 @@ _media_add_ssrcs (GstSDPMedia * media, GstCaps * caps, GstWebRTCBin * webrtc,
     if (gst_structure_get_uint (s, "ssrc", &ssrc)) {
       gchar *str;
 
-      /* https://tools.ietf.org/html/draft-ietf-mmusic-msid-16 */
-      str =
-          g_strdup_printf ("%u msid:%s %s", ssrc, cname,
-          GST_OBJECT_NAME (trans));
+      str = _construct_msid (trans, ssrc);
+
       gst_sdp_media_add_attribute (media, "ssrc", str);
       g_free (str);
 

diff --git a/subprojects/gst-plugins-bad/ext/webrtc/webrtctransceiver.c b/subprojects/gst-plugins-bad/ext/webrtc/webrtctransceiver.c
index ed4f906..b754e92 100644 (file)
--- a/subprojects/gst-plugins-bad/ext/webrtc/webrtctransceiver.c
+++ b/subprojects/gst-plugins-bad/ext/webrtc/webrtctransceiver.c
@@ -38,6 +38,7 @@ G_DEFINE_TYPE_WITH_CODE (WebRTCTransceiver, webrtc_transceiver,
 #define DEFAULT_FEC_TYPE GST_WEBRTC_FEC_TYPE_NONE
 #define DEFAULT_DO_NACK FALSE
 #define DEFAULT_FEC_PERCENTAGE 100
+#define DEFAULT_MSID_APPDATA NULL
 
 enum
 {
@@ -46,6 +47,7 @@ enum
   PROP_FEC_TYPE,
   PROP_FEC_PERCENTAGE,
   PROP_DO_NACK,
+  PROP_MSID_APPDATA,
 };
 
 void
@@ -112,6 +114,22 @@ webrtc_transceiver_set_property (GObject * object, guint prop_id,
     case PROP_FEC_PERCENTAGE:
       trans->fec_percentage = g_value_get_uint (value);
       break;
+    case PROP_MSID_APPDATA:
+    {
+      gchar *new_msid_appdata = g_value_dup_string (value);
+
+      if (new_msid_appdata && strlen (new_msid_appdata) > 64) {
+        g_warning ("Msid appdata exceeds 64 characters: %s", new_msid_appdata);
+        g_free (new_msid_appdata);
+      } else {
+        if (trans->msid_appdata)
+          g_free (trans->msid_appdata);
+
+        trans->msid_appdata = new_msid_appdata;
+      }
+
+      break;
+    }
     default:
       G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
       break;
@@ -136,6 +154,9 @@ webrtc_transceiver_get_property (GObject * object, guint prop_id,
     case PROP_FEC_PERCENTAGE:
       g_value_set_uint (value, trans->fec_percentage);
       break;
+    case PROP_MSID_APPDATA:
+      g_value_set_string (value, trans->msid_appdata);
+      break;
     default:
       G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
       break;
@@ -152,6 +173,7 @@ webrtc_transceiver_finalize (GObject * object)
   gst_clear_object (&trans->ulpfecdec);
   gst_clear_object (&trans->ulpfecenc);
   gst_clear_object (&trans->redenc);
+  g_clear_pointer (&trans->msid_appdata, g_free);
 
   if (trans->local_rtx_ssrc_map)
     gst_structure_free (trans->local_rtx_ssrc_map);
@@ -203,6 +225,23 @@ webrtc_transceiver_class_init (WebRTCTransceiverClass * klass)
           "The amount of Forward Error Correction to apply",
           0, 100, DEFAULT_FEC_PERCENTAGE,
           G_PARAM_CONSTRUCT | G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS));
+
+  /**
+   * WebRTCTransceiver:msid-appdata:
+   *
+   * The appdata part of the media stream id, must not exceed 64 characters.
+   *
+   * Consult https://datatracker.ietf.org/doc/html/draft-ietf-mmusic-msid-16#section-2
+   * for more details.
+   *
+   * Since: 1.22
+   */
+  g_object_class_install_property (gobject_class,
+      PROP_MSID_APPDATA,
+      g_param_spec_string ("msid-appdata", "Msid appdata",
+          "The appdata part of the media stream id, must not exceed 64 characters",
+          NULL,
+          G_PARAM_CONSTRUCT | G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS));
 }
 
 static void

diff --git a/subprojects/gst-plugins-bad/ext/webrtc/webrtctransceiver.h b/subprojects/gst-plugins-bad/ext/webrtc/webrtctransceiver.h
index a3c80c5..b0528ef 100644 (file)
--- a/subprojects/gst-plugins-bad/ext/webrtc/webrtctransceiver.h
+++ b/subprojects/gst-plugins-bad/ext/webrtc/webrtctransceiver.h
@@ -47,6 +47,7 @@ struct _WebRTCTransceiver
   GstWebRTCFECType         fec_type;
   guint                    fec_percentage;
   gboolean                 do_nack;
+  gchar                   *msid_appdata;
 
   GstCaps                  *last_configured_caps;
 

diff --git a/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c b/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c
index 8508bd1..f7b44c8 100644 (file)
--- a/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c
+++ b/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c
@@ -4425,6 +4425,66 @@ GST_START_TEST (test_renego_rtx)
 
 GST_END_TEST;
 
+static void
+_check_msid_appdata (struct test_webrtc *t, GstElement * element,
+    GstWebRTCSessionDescription * desc, gpointer expected)
+{
+  guint i = 0;
+
+  for (i = 0; i < gst_sdp_message_medias_len (desc->sdp); i++) {
+    const GstSDPMedia *media = gst_sdp_message_get_media (desc->sdp, i);
+
+    if (g_strcmp0 (gst_sdp_media_get_media (media), "audio") == 0
+        || g_strcmp0 (gst_sdp_media_get_media (media), "video") == 0) {
+      int j;
+
+      for (j = 0; j < gst_sdp_media_attributes_len (media); j++) {
+        const GstSDPAttribute *attr = gst_sdp_media_get_attribute (media, j);
+
+        if (!g_strcmp0 (attr->key, "ssrc")) {
+          gchar **split = g_strsplit (attr->value, " ", 3);
+
+          fail_unless (g_strv_length (split) >= 2);
+
+          if (g_str_has_prefix (split[1], "msid:")) {
+            if (expected) {
+              fail_unless_equals_int (g_strv_length (split), 3);
+              fail_unless_equals_string (split[2], expected);
+            } else {
+              fail_unless_equals_int (g_strv_length (split), 2);
+            }
+          }
+
+          g_strfreev (split);
+        }
+      }
+    }
+  }
+}
+
+GST_START_TEST (test_msid_appdata)
+{
+  struct test_webrtc *t = create_audio_test ();
+  gchar *expected = g_strdup ("foobar");
+  GstWebRTCRTPTransceiver *trans;
+
+  VAL_SDP_INIT (offer, _check_msid_appdata, expected, NULL);
+  VAL_SDP_INIT (answer, _check_msid_appdata, expected, NULL);
+
+  g_signal_emit_by_name (t->webrtc1, "get-transceiver", 0, &trans);
+  g_object_set (trans, "msid-appdata", expected, NULL);
+  g_clear_object (&trans);
+
+  test_validate_sdp (t, &offer, &answer);
+
+  g_free (expected);
+
+  test_webrtc_free (t);
+}
+
+GST_END_TEST;
+
+
 static Suite *
 webrtcbin_suite (void)
 {
@@ -4480,6 +4540,7 @@ webrtcbin_suite (void)
     tcase_add_test (tc, test_codec_preferences_incompatible_extmaps);
     tcase_add_test (tc, test_codec_preferences_invalid_extmap);
     tcase_add_test (tc, test_renego_rtx);
+    tcase_add_test (tc, test_msid_appdata);
     if (sctpenc && sctpdec) {
       tcase_add_test (tc, test_data_channel_create);
       tcase_add_test (tc, test_data_channel_remote_notify);