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: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/8729>

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,6 +4601,7 @@ _create_answer_task (GstWebRTCBin * webrtc, const GstStructure * options,
 
       _remove_optional_offer_fields (offer_caps);
 
+      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,
@@ -4610,6 +4609,15 @@ _create_answer_task (GstWebRTCBin * webrtc, const GstStructure * options,
           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);
       if (*error) {
@@ -4623,6 +4631,7 @@ _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 */
+        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) {
-    if (!mid) {
+      trans = _find_transceiver_for_mid (webrtc, mid);
-      g_set_error (error, GST_WEBRTC_ERROR, GST_WEBRTC_ERROR_SDP_SYNTAX_ERROR,
+    } else {
-          "Missing mid attribute in media");
+      trans = _find_transceiver_for_mline (webrtc, i);
-      goto out;
     }
 
     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);
+        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)

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)) {
+  return !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;
 }
 
 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;
+    if (_media_has_mid (media, i)) {
       mid = gst_sdp_media_get_attribute_val (media, "mid");
-    media_in_bundle = is_bundle
+      media_in_bundle =
-        && g_strv_contains ((const gchar **) group_members, mid);
+          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",

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);