webrtcbin: Make basic rollbacks work

Fixes for basic rollback (from have-local-offer or have-remote-offer to
stable). Allow having no SDP attached to the webrtc session description
in that case, and avoid all the transceiver and ICE update logic
normally applied when entering the stable signalling state

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

subprojects/gst-plugins-bad/ext/webrtc/gstwebrtcbin.c

@@ -6323,7 +6323,9 @@ _create_and_associate_transceivers_from_sdp (GstWebRTCBin * webrtc,
      * Restore the value of connection's [[ sctpTransport]] internal slot
      * to its value at the last stable signaling state.
      */
-    return ret;
+    GST_FIXME_OBJECT (webrtc,
+        "Rolling back transceiver associations is not implemented");
+    return TRUE;
   }
 
   /* FIXME: With some peers, it's possible we could have
@@ -6534,52 +6536,58 @@ _set_description_task (GstWebRTCBin * webrtc, struct set_description *sd)
         webrtc->signaling_state);
     const gchar *type_str =
         _enum_value_to_string (GST_TYPE_WEBRTC_SDP_TYPE, sd->sdp->type);
-    gchar *sdp_text = gst_sdp_message_as_text (sd->sdp->sdp);
     GST_INFO_OBJECT (webrtc, "Attempting to set %s %s in the %s state",
         _sdp_source_to_string (sd->source), type_str, state);
-    GST_TRACE_OBJECT (webrtc, "SDP contents\n%s", sdp_text);
-    g_free (sdp_text);
   }
 
-  if (!validate_sdp (webrtc->signaling_state, sd->source, sd->sdp, &error))
+  if (sd->sdp->type != GST_WEBRTC_SDP_TYPE_ROLLBACK) {
-    goto out;
+    {
+      gchar *sdp_text = gst_sdp_message_as_text (sd->sdp->sdp);
+      GST_TRACE_OBJECT (webrtc, "SDP contents\n%s", sdp_text);
+      g_free (sdp_text);
+    }
 
-  if (webrtc->bundle_policy != GST_WEBRTC_BUNDLE_POLICY_NONE)
+    if (!validate_sdp (webrtc->signaling_state, sd->source, sd->sdp, &error))
-    if (!_parse_bundle (sd->sdp->sdp, &bundled, &error))
       goto out;
 
-  if (bundled) {
+    if (webrtc->bundle_policy != GST_WEBRTC_BUNDLE_POLICY_NONE)
-    if (!_get_bundle_index (sd->sdp->sdp, bundled, &bundle_idx)) {
+      if (!_parse_bundle (sd->sdp->sdp, &bundled, &error))
-      g_set_error (&error, GST_WEBRTC_ERROR, GST_WEBRTC_ERROR_SDP_SYNTAX_ERROR,
+        goto out;
-          "Bundle tag is %s but no matching media found", bundled[0]);
+
+    if (bundled) {
+      if (!_get_bundle_index (sd->sdp->sdp, bundled, &bundle_idx)) {
+        g_set_error (&error, GST_WEBRTC_ERROR,
+            GST_WEBRTC_ERROR_SDP_SYNTAX_ERROR,
+            "Bundle tag is %s but no matching media found", bundled[0]);
+        goto out;
+      }
+    }
+
+    if (transceivers_media_num_cmp (webrtc,
+            get_previous_description (webrtc, sd->source, sd->sdp->type),
+            sd->sdp) < 0) {
+      g_set_error_literal (&error, GST_WEBRTC_ERROR,
+          GST_WEBRTC_ERROR_SDP_SYNTAX_ERROR,
+          "m=lines removed from the SDP. Processing a completely new connection "
+          "is not currently supported.");
       goto out;
     }
-  }
 
-  if (transceivers_media_num_cmp (webrtc,
+    if ((sd->sdp->type == GST_WEBRTC_SDP_TYPE_PRANSWER ||
-          get_previous_description (webrtc, sd->source, sd->sdp->type),
+            sd->sdp->type == GST_WEBRTC_SDP_TYPE_ANSWER) &&
-          sd->sdp) < 0) {
+        transceivers_media_num_cmp (webrtc,
-    g_set_error_literal (&error, GST_WEBRTC_ERROR,
+            get_last_generated_description (webrtc, sd->source, sd->sdp->type),
-        GST_WEBRTC_ERROR_SDP_SYNTAX_ERROR,
+            sd->sdp) != 0) {
-        "m=lines removed from the SDP. Processing a completely new connection "
+      g_set_error_literal (&error, GST_WEBRTC_ERROR,
-        "is not currently supported.");
+          GST_WEBRTC_ERROR_SDP_SYNTAX_ERROR,
-    goto out;
+          "Answer doesn't have the same number of m-lines as the offer.");
-  }
+      goto out;
+    }
 
-  if ((sd->sdp->type == GST_WEBRTC_SDP_TYPE_PRANSWER ||
+    if (!check_locked_mlines (webrtc, sd->sdp, &error))
-          sd->sdp->type == GST_WEBRTC_SDP_TYPE_ANSWER) &&
+      goto out;
-      transceivers_media_num_cmp (webrtc,
-          get_last_generated_description (webrtc, sd->source, sd->sdp->type),
-          sd->sdp) != 0) {
-    g_set_error_literal (&error, GST_WEBRTC_ERROR,
-        GST_WEBRTC_ERROR_SDP_SYNTAX_ERROR,
-        "Answer doesn't have the same number of m-lines as the offer.");
-    goto out;
   }
 
-  if (!check_locked_mlines (webrtc, sd->sdp, &error))
-    goto out;
-
   switch (sd->sdp->type) {
     case GST_WEBRTC_SDP_TYPE_OFFER:{
       if (sd->source == SDP_LOCAL) {
@@ -6639,7 +6647,24 @@ _set_description_task (GstWebRTCBin * webrtc, struct set_description *sd)
       break;
     }
     case GST_WEBRTC_SDP_TYPE_ROLLBACK:{
-      GST_FIXME_OBJECT (webrtc, "rollbacks are completely untested");
+      if (webrtc->signaling_state == GST_WEBRTC_SIGNALING_STATE_STABLE ||
+          webrtc->signaling_state ==
+          GST_WEBRTC_SIGNALING_STATE_HAVE_LOCAL_PRANSWER
+          || webrtc->signaling_state ==
+          GST_WEBRTC_SIGNALING_STATE_HAVE_REMOTE_PRANSWER) {
+        /* If description.type is "rollback" and connection's signaling state is
+         * either "stable", "have-local-pranswer", or "have-remote-pranswer",
+         * then reject p with a newly created InvalidStateError and abort these
+         * steps. */
+        const gchar *state_name =
+            _enum_value_to_string (GST_TYPE_WEBRTC_SIGNALING_STATE,
+            webrtc->signaling_state);
+        g_set_error (&error, GST_WEBRTC_ERROR, GST_WEBRTC_ERROR_INVALID_STATE,
+            "Cannot roll back from current state %s", state_name);
+        goto out;
+      }
+
+      GST_FIXME_OBJECT (webrtc, "Rollbacks are only partially implemented");
       if (sd->source == SDP_LOCAL) {
         if (webrtc->pending_local_description)
           gst_webrtc_session_description_free
@@ -6686,6 +6711,11 @@ _set_description_task (GstWebRTCBin * webrtc, struct set_description *sd)
     signalling_state_changed = TRUE;
   }
 
+  if (sd->sdp->type == GST_WEBRTC_SDP_TYPE_ROLLBACK) {
+    /* If rolling back, leave all the transceivers and other setup alone. There's no SDP attached to use anyway */
+    goto update_signaling_state;
+  }
+
   {
     gboolean ice_controller = FALSE;
 
@@ -6863,6 +6893,7 @@ _set_description_task (GstWebRTCBin * webrtc, struct set_description *sd)
     ICE_UNLOCK (webrtc);
   }
 
+update_signaling_state:
   /*
    * If connection's signaling state changed above, fire an event named
    * signalingstatechange at connection.
@@ -6879,7 +6910,8 @@ _set_description_task (GstWebRTCBin * webrtc, struct set_description *sd)
     PC_LOCK (webrtc);
   }
 
-  if (webrtc->signaling_state == GST_WEBRTC_SIGNALING_STATE_STABLE) {
+  if (webrtc->signaling_state == GST_WEBRTC_SIGNALING_STATE_STABLE
+      && sd->sdp->type != GST_WEBRTC_SDP_TYPE_ROLLBACK) {
     gboolean prev_need_negotiation = webrtc->priv->need_negotiation;
 
     /* If connection's signaling state is now stable, update the
@@ -6923,7 +6955,8 @@ gst_webrtc_bin_set_remote_description (GstWebRTCBin * webrtc,
 
   if (remote_sdp == NULL)
     goto bad_input;
-  if (remote_sdp->sdp == NULL)
+  if (remote_sdp->sdp == NULL
+      && remote_sdp->type != GST_WEBRTC_SDP_TYPE_ROLLBACK)
     goto bad_input;
 
   sd = g_new0 (struct set_description, 1);
@@ -6961,7 +6994,7 @@ gst_webrtc_bin_set_local_description (GstWebRTCBin * webrtc,
 
   if (local_sdp == NULL)
     goto bad_input;
-  if (local_sdp->sdp == NULL)
+  if (local_sdp->sdp == NULL && local_sdp->type != GST_WEBRTC_SDP_TYPE_ROLLBACK)
     goto bad_input;
 
   sd = g_new0 (struct set_description, 1);

subprojects/gst-plugins-bad/gst-libs/gst/webrtc/rtcsessiondescription.c

@@ -75,7 +75,9 @@ gst_webrtc_session_description_copy (const GstWebRTCSessionDescription * src)
   ret = g_new0 (GstWebRTCSessionDescription, 1);
 
   ret->type = src->type;
-  gst_sdp_message_copy (src->sdp, &ret->sdp);
+  if (src->sdp != NULL) {
+    gst_sdp_message_copy (src->sdp, &ret->sdp);
+  }
 
   return ret;
 }
@@ -91,7 +93,9 @@ gst_webrtc_session_description_free (GstWebRTCSessionDescription * desc)
 {
   g_return_if_fail (desc != NULL);
 
-  gst_sdp_message_free (desc->sdp);
+  if (desc->sdp != NULL) {
+    gst_sdp_message_free (desc->sdp);
+  }
   g_free (desc);
 }
 

subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c

@@ -6040,6 +6040,63 @@ GST_START_TEST (test_sdp_session_setup_attribute)
 
 GST_END_TEST;
 
+static void
+_rollback_complete (GstPromise * promise, gpointer user_data)
+{
+  GstPromiseResult result = gst_promise_wait (promise);
+  fail_unless (result == GST_PROMISE_RESULT_REPLIED);
+
+  const GstStructure *reply = gst_promise_get_reply (promise);
+  GError *error = NULL;
+  if (reply != NULL
+      && gst_structure_get (reply, "error", G_TYPE_ERROR, &error, NULL)) {
+    /* Ignore invalid-state error, which just means WebRTCbin already processed the remote
+     * offer/answer and went back to stable state */
+    if (error->domain != GST_WEBRTC_ERROR
+        || error->code != GST_WEBRTC_ERROR_INVALID_STATE) {
+      fail ("rollback request resulted in error: %s", error->message);
+      g_clear_error (&error);
+      return;
+    }
+    g_clear_error (&error);
+  }
+}
+
+static void
+_rollback_offer (struct test_webrtc *t, GstElement * element,
+    GstPromise * promise, gpointer user_data)
+{
+  GstWebRTCSessionDescription *rollback =
+      gst_webrtc_session_description_new (GST_WEBRTC_SDP_TYPE_ROLLBACK, NULL);
+
+  promise = gst_promise_new_with_change_func (_rollback_complete, t, NULL);
+  if (element == t->webrtc1) {
+    g_signal_emit_by_name (t->webrtc1, "set-local-description", rollback,
+        promise);
+  } else {
+    g_signal_emit_by_name (t->webrtc2, "set-remote-description", rollback,
+        promise);
+  }
+  gst_promise_unref (promise);
+  gst_webrtc_session_description_free (rollback);
+}
+
+GST_START_TEST (test_offer_rollback)
+{
+  struct test_webrtc *t = create_audio_test ();
+
+  t->on_offer_set = _rollback_offer;
+  t->offer_set_data = NULL;
+
+  t->on_answer_set = NULL;
+  t->answer_set_data = NULL;
+
+  test_validate_sdp (t, NULL, NULL);
+  test_webrtc_free (t);
+}
+
+GST_END_TEST;
+
 static Suite *
 webrtcbin_suite (void)
 {
@@ -6130,6 +6187,7 @@ webrtcbin_suite (void)
           "All datachannel tests are disabled. sctpenc %p, sctpdec %p", sctpenc,
           sctpdec);
     }
+    tcase_add_test (tc, test_offer_rollback);
   } else {
     GST_WARNING ("Some required elements were not found. "
         "All media tests are disabled. nicesrc %p, nicesink %p, "