webrtcbin: Store pending mid to make create-offer idempotent

If the mid is not stored in the transceiver, but it is stored in
last_offer, then a further create-offer call will just ignore that
transceiver.

Also include unit test for ensure it doesn't regress.

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

diff --git a/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c b/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c
index 11dfedf..1528133 100644 (file)
--- a/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c
+++ b/subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c
@@ -3213,7 +3213,7 @@ sdp_media_from_transceiver (GstWebRTCBin * webrtc, GstSDPMedia * media,
    * multiple dtls fingerprints https://tools.ietf.org/html/draft-ietf-mmusic-4572-update-05
    */
   GstSDPMessage *last_offer = _get_latest_self_generated_sdp (webrtc);
-  gchar *direction, *ufrag, *pwd, *mid;
+  gchar *direction, *ufrag, *pwd, *mid = NULL;
   gboolean bundle_only;
   guint rtp_session_idx;
   GstCaps *caps;
@@ -3422,11 +3422,13 @@ sdp_media_from_transceiver (GstWebRTCBin * webrtc, GstSDPMedia * media,
       return FALSE;
     }
     mid = g_strdup (trans->mid);
-  } else {
+    g_hash_table_insert (all_mids, g_strdup (mid), NULL);
+  }
+
+  if (mid == NULL) {
     const GstStructure *s = gst_caps_get_structure (caps, 0);
 
     mid = g_strdup (gst_structure_get_string (s, "a-mid"));
-
     if (mid) {
       if (g_hash_table_contains (all_mids, (gpointer) mid)) {
         g_set_error (error, GST_WEBRTC_ERROR,
@@ -3436,19 +3438,39 @@ sdp_media_from_transceiver (GstWebRTCBin * webrtc, GstSDPMedia * media,
             media_idx);
         return FALSE;
       }
+      g_free (WEBRTC_TRANSCEIVER (trans)->pending_mid);
+      WEBRTC_TRANSCEIVER (trans)->pending_mid = g_strdup (mid);
       g_hash_table_insert (all_mids, g_strdup (mid), NULL);
-    } else {
-      /* Make sure to avoid mid collisions */
-      while (TRUE) {
-        mid = g_strdup_printf ("%s%u", gst_sdp_media_get_media (media),
-            webrtc->priv->media_counter++);
-        if (g_hash_table_contains (all_mids, (gpointer) mid)) {
-          g_free (mid);
-        } else {
-          gst_sdp_media_add_attribute (media, "mid", mid);
-          g_hash_table_insert (all_mids, g_strdup (mid), NULL);
-          break;
-        }
+    }
+  }
+
+  if (mid == NULL) {
+    mid = g_strdup (WEBRTC_TRANSCEIVER (trans)->pending_mid);
+    if (mid) {
+      /* If it's already used, just ignore the pending one and generate
+       * a new one */
+      if (g_hash_table_contains (all_mids, (gpointer) mid)) {
+        g_clear_pointer (&mid, free);
+        g_clear_pointer (&WEBRTC_TRANSCEIVER (trans)->pending_mid, free);
+      } else {
+        gst_sdp_media_add_attribute (media, "mid", mid);
+        g_hash_table_insert (all_mids, g_strdup (mid), NULL);
+      }
+    }
+  }
+
+  if (mid == NULL) {
+    /* Make sure to avoid mid collisions */
+    while (TRUE) {
+      mid = g_strdup_printf ("%s%u", gst_sdp_media_get_media (media),
+          webrtc->priv->media_counter++);
+      if (g_hash_table_contains (all_mids, (gpointer) mid)) {
+        g_free (mid);
+      } else {
+        gst_sdp_media_add_attribute (media, "mid", mid);
+        g_hash_table_insert (all_mids, g_strdup (mid), NULL);
+        WEBRTC_TRANSCEIVER (trans)->pending_mid = g_strdup (mid);
+        break;
       }
     }
   }
@@ -3706,14 +3728,22 @@ _create_offer_task (GstWebRTCBin * webrtc, const GstStructure * options,
           || g_strcmp0 (gst_sdp_media_get_media (last_media), "video") == 0) {
         const gchar *last_mid;
         int j;
+
         last_mid = gst_sdp_media_get_attribute_val (last_media, "mid");
 
         for (j = 0; j < webrtc->priv->transceivers->len; j++) {
+          WebRTCTransceiver *wtrans;
+          const gchar *mid;
+
           trans = g_ptr_array_index (webrtc->priv->transceivers, j);
+          wtrans = WEBRTC_TRANSCEIVER (trans);
+
+          if (trans->mid)
+            mid = trans->mid;
+          else
+            mid = wtrans->pending_mid;
 
-          if (trans->mid && g_strcmp0 (trans->mid, last_mid) == 0) {
-            WebRTCTransceiver *wtrans = WEBRTC_TRANSCEIVER (trans);
-            const char *mid;
+          if (mid && g_strcmp0 (mid, last_mid) == 0) {
             GstSDPMedia media;
 
             memset (&media, 0, sizeof (media));
@@ -3800,6 +3830,11 @@ _create_offer_task (GstWebRTCBin * webrtc, const GstStructure * options,
       }
 
       g_hash_table_insert (all_mids, g_strdup (trans->mid), NULL);
+    } else if (WEBRTC_TRANSCEIVER (trans)->pending_mid &&
+        !g_hash_table_contains (all_mids,
+            WEBRTC_TRANSCEIVER (trans)->pending_mid)) {
+      g_hash_table_insert (all_mids,
+          g_strdup (WEBRTC_TRANSCEIVER (trans)->pending_mid), NULL);
     }
   }
 

diff --git a/subprojects/gst-plugins-bad/ext/webrtc/webrtctransceiver.c b/subprojects/gst-plugins-bad/ext/webrtc/webrtctransceiver.c
index 279db59..ba9c944 100644 (file)
--- a/subprojects/gst-plugins-bad/ext/webrtc/webrtctransceiver.c
+++ b/subprojects/gst-plugins-bad/ext/webrtc/webrtctransceiver.c
@@ -160,6 +160,8 @@ webrtc_transceiver_finalize (GObject * object)
   gst_caps_replace (&trans->last_retrieved_caps, NULL);
   gst_caps_replace (&trans->last_send_configured_caps, NULL);
 
+  g_free (trans->pending_mid);
+
   gst_event_replace (&trans->tos_event, NULL);
 
   G_OBJECT_CLASS (parent_class)->finalize (object);

diff --git a/subprojects/gst-plugins-bad/ext/webrtc/webrtctransceiver.h b/subprojects/gst-plugins-bad/ext/webrtc/webrtctransceiver.h
index 989873c..9f0e93c 100644 (file)
--- a/subprojects/gst-plugins-bad/ext/webrtc/webrtctransceiver.h
+++ b/subprojects/gst-plugins-bad/ext/webrtc/webrtctransceiver.h
@@ -54,6 +54,8 @@ struct _WebRTCTransceiver
    */
   GstCaps                  *last_send_configured_caps;
 
+  gchar                    *pending_mid;
+
   gboolean                 mline_locked;
 
   GstElement               *ulpfecdec;

diff --git a/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c b/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c
index eaaccac..76767d7 100644 (file)
--- a/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c
+++ b/subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c
@@ -5426,6 +5426,48 @@ GST_START_TEST (test_invalid_add_media_in_answer)
 
 GST_END_TEST;
 
+GST_START_TEST (test_data_channel_recreate_offer)
+{
+  GstHarness *h;
+  GstWebRTCDataChannel *channel;
+  GstPromise *promise;
+  const GstStructure *s;
+  GstPromiseResult res;
+  GstPad *pad;
+
+  h = gst_harness_new_with_padnames ("webrtcbin", "sink_0", NULL);
+  add_audio_test_src_harness (h, 0xDEADBEEF);
+
+  g_signal_emit_by_name (h->element, "create-data-channel", "label", NULL,
+      &channel);
+  fail_unless (GST_IS_WEBRTC_DATA_CHANNEL (channel));
+
+  pad = gst_element_get_static_pad (h->element, "sink_0");
+  fail_unless (pad != NULL);
+
+  promise = gst_promise_new ();
+  g_signal_emit_by_name (h->element, "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);
+  gst_promise_unref (promise);
+
+  promise = gst_promise_new ();
+  g_signal_emit_by_name (h->element, "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);
+  gst_promise_unref (promise);
+
+  gst_object_unref (pad);
+  gst_object_unref (channel);
+  gst_harness_teardown (h);
+}
+
+GST_END_TEST;
+
 static Suite *
 webrtcbin_suite (void)
 {
@@ -5502,6 +5544,7 @@ webrtcbin_suite (void)
       tcase_add_test (tc, test_renego_stream_add_data_channel);
       tcase_add_test (tc, test_renego_data_channel_add_stream);
       tcase_add_test (tc, test_renego_stream_data_channel_add_stream);
+      tcase_add_test (tc, test_data_channel_recreate_offer);
     } else {
       GST_WARNING ("Some required elements were not found. "
           "All datachannel tests are disabled. sctpenc %p, sctpdec %p", sctpenc,