webrtc: return error when sending on non-open datachannel
authorJohan Sternerup <johan.sternerup@axis.com>
Mon, 21 Mar 2022 09:29:21 +0000 (10:29 +0100)
committerGStreamer Marge Bot <gitlab-merge-bot@gstreamer-foundation.org>
Wed, 5 Oct 2022 11:08:30 +0000 (11:08 +0000)
According to W3C
specification (https://w3c.github.io/webrtc-pc/#datachannel-send) we
should return InvalidStateError exception when trying to send when the
channel is not open. In the world of C/glib/gstreamer we don't have
exceptions but have to rely on gboolean/GError instead. Introducing
these calls for a change in function signature of the action signals
used to send data on the datachannel. Changing the signature of the
existing "send-string" and "send-data" signals would mean an immediate
breaking change so instead we deprecate them. Furthermore, there is no
way to express GError** as an argument to an action signal in a way
that fits language bindings (pointer-to-pointer simply does not work)
and we have to use regular functions instead.

Therefore we introduce gst_webrtc_data_channel_send_data_full() and
gst_webrtc_data_channel_send_string_full() while deprecating the old
functions and corresponding signals.

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

subprojects/gst-plugins-bad/ext/webrtc/webrtcdatachannel.c
subprojects/gst-plugins-bad/gst-libs/gst/webrtc/datachannel.c
subprojects/gst-plugins-bad/gst-libs/gst/webrtc/datachannel.h
subprojects/gst-plugins-bad/gst-libs/gst/webrtc/webrtc-priv.h
subprojects/gst-plugins-bad/gst-libs/gst/webrtc/webrtc_fwd.h
subprojects/gst-plugins-bad/tests/check/elements/webrtcbin.c

index 0ce3368..bb2b023 100644 (file)
@@ -745,35 +745,30 @@ _is_within_max_message_size (WebRTCDataChannel * channel, gsize size)
   return size <= channel->sctp_transport->max_message_size;
 }
 
-static void
+static gboolean
 webrtc_data_channel_send_data (GstWebRTCDataChannel * base_channel,
-    GBytes * bytes)
+    GBytes * bytes, GError ** error)
 {
   WebRTCDataChannel *channel = WEBRTC_DATA_CHANNEL (base_channel);
   GstSctpSendMetaPartiallyReliability reliability;
   guint rel_param;
   guint32 ppid;
   GstBuffer *buffer;
+  gsize size = 0;
   GstFlowReturn ret;
 
   if (!bytes) {
     buffer = gst_buffer_new ();
     ppid = DATA_CHANNEL_PPID_WEBRTC_BINARY_EMPTY;
   } else {
-    gsize size;
     guint8 *data;
 
     data = (guint8 *) g_bytes_get_data (bytes, &size);
-    g_return_if_fail (data != NULL);
+    g_return_val_if_fail (data != NULL, FALSE);
     if (!_is_within_max_message_size (channel, size)) {
-      GError *error = NULL;
-      g_set_error (&error, GST_WEBRTC_ERROR,
-          GST_WEBRTC_ERROR_DATA_CHANNEL_FAILURE,
+      g_set_error (error, GST_WEBRTC_ERROR, GST_WEBRTC_ERROR_TYPE_ERROR,
           "Requested to send data that is too large");
-      _channel_store_error (channel, error);
-      _channel_enqueue_task (channel, (ChannelTask) _close_procedure, NULL,
-          NULL);
-      return;
+      return FALSE;
     }
 
     buffer = gst_buffer_new_wrapped_full (GST_MEMORY_FLAG_READONLY, data, size,
@@ -789,52 +784,61 @@ webrtc_data_channel_send_data (GstWebRTCDataChannel * base_channel,
       buffer);
 
   GST_WEBRTC_DATA_CHANNEL_LOCK (channel);
-  channel->parent.buffered_amount += gst_buffer_get_size (buffer);
+  if (channel->parent.ready_state == GST_WEBRTC_DATA_CHANNEL_STATE_OPEN) {
+    channel->parent.buffered_amount += size;
+  } else {
+    GST_WEBRTC_DATA_CHANNEL_UNLOCK (channel);
+    g_set_error (error, GST_WEBRTC_ERROR,
+        GST_WEBRTC_ERROR_INVALID_STATE, "channel is not open");
+    return FALSE;
+  }
   GST_WEBRTC_DATA_CHANNEL_UNLOCK (channel);
-  g_object_notify (G_OBJECT (&channel->parent), "buffered-amount");
 
   ret = gst_app_src_push_buffer (GST_APP_SRC (channel->appsrc), buffer);
-
-  if (ret != GST_FLOW_OK) {
-    GError *error = NULL;
-    g_set_error (&error, GST_WEBRTC_ERROR,
+  if (ret == GST_FLOW_OK) {
+    g_object_notify (G_OBJECT (&channel->parent), "buffered-amount");
+  } else {
+    g_set_error (error, GST_WEBRTC_ERROR,
         GST_WEBRTC_ERROR_DATA_CHANNEL_FAILURE, "Failed to send data");
-    _channel_store_error (channel, error);
+
+    GST_WEBRTC_DATA_CHANNEL_LOCK (channel);
+    channel->parent.buffered_amount -= size;
+    GST_WEBRTC_DATA_CHANNEL_UNLOCK (channel);
+
     _channel_enqueue_task (channel, (ChannelTask) _close_procedure, NULL, NULL);
+    return FALSE;
   }
+
+  return TRUE;
 }
 
-static void
+static gboolean
 webrtc_data_channel_send_string (GstWebRTCDataChannel * base_channel,
-    const gchar * str)
+    const gchar * str, GError ** error)
 {
   WebRTCDataChannel *channel = WEBRTC_DATA_CHANNEL (base_channel);
   GstSctpSendMetaPartiallyReliability reliability;
   guint rel_param;
   guint32 ppid;
   GstBuffer *buffer;
+  gsize size = 0;
   GstFlowReturn ret;
 
   if (!channel->parent.negotiated)
-    g_return_if_fail (channel->opened);
-  g_return_if_fail (channel->sctp_transport != NULL);
+    g_return_val_if_fail (channel->opened, FALSE);
+  g_return_val_if_fail (channel->sctp_transport != NULL, FALSE);
 
   if (!str) {
     buffer = gst_buffer_new ();
     ppid = DATA_CHANNEL_PPID_WEBRTC_STRING_EMPTY;
   } else {
-    gsize size = strlen (str);
     gchar *str_copy;
+    size = strlen (str);
 
     if (!_is_within_max_message_size (channel, size)) {
-      GError *error = NULL;
-      g_set_error (&error, GST_WEBRTC_ERROR,
-          GST_WEBRTC_ERROR_DATA_CHANNEL_FAILURE,
+      g_set_error (error, GST_WEBRTC_ERROR, GST_WEBRTC_ERROR_TYPE_ERROR,
           "Requested to send a string that is too large");
-      _channel_store_error (channel, error);
-      _channel_enqueue_task (channel, (ChannelTask) _close_procedure, NULL,
-          NULL);
-      return;
+      return FALSE;
     }
 
     str_copy = g_strdup (str);
@@ -852,19 +856,32 @@ webrtc_data_channel_send_string (GstWebRTCDataChannel * base_channel,
       buffer);
 
   GST_WEBRTC_DATA_CHANNEL_LOCK (channel);
-  channel->parent.buffered_amount += gst_buffer_get_size (buffer);
+  if (channel->parent.ready_state == GST_WEBRTC_DATA_CHANNEL_STATE_OPEN) {
+    channel->parent.buffered_amount += size;
+  } else {
+    GST_WEBRTC_DATA_CHANNEL_UNLOCK (channel);
+    g_set_error (error, GST_WEBRTC_ERROR,
+        GST_WEBRTC_ERROR_INVALID_STATE, "channel is not open");
+    return FALSE;
+  }
   GST_WEBRTC_DATA_CHANNEL_UNLOCK (channel);
-  g_object_notify (G_OBJECT (&channel->parent), "buffered-amount");
 
   ret = gst_app_src_push_buffer (GST_APP_SRC (channel->appsrc), buffer);
-
-  if (ret != GST_FLOW_OK) {
-    GError *error = NULL;
-    g_set_error (&error, GST_WEBRTC_ERROR,
+  if (ret == GST_FLOW_OK) {
+    g_object_notify (G_OBJECT (&channel->parent), "buffered-amount");
+  } else {
+    g_set_error (error, GST_WEBRTC_ERROR,
         GST_WEBRTC_ERROR_DATA_CHANNEL_FAILURE, "Failed to send string");
-    _channel_store_error (channel, error);
+
+    GST_WEBRTC_DATA_CHANNEL_LOCK (channel);
+    channel->parent.buffered_amount -= size;
+    GST_WEBRTC_DATA_CHANNEL_UNLOCK (channel);
+
     _channel_enqueue_task (channel, (ChannelTask) _close_procedure, NULL, NULL);
+    return FALSE;
   }
+
+  return TRUE;
 }
 
 static void
index e8a141f..38cd3ce 100644 (file)
@@ -339,7 +339,7 @@ gst_webrtc_data_channel_class_init (GstWebRTCDataChannelClass * klass)
    */
   gst_webrtc_data_channel_signals[SIGNAL_SEND_DATA] =
       g_signal_new_class_handler ("send-data", G_TYPE_FROM_CLASS (klass),
-      G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION,
+      G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION | G_SIGNAL_DEPRECATED,
       G_CALLBACK (gst_webrtc_data_channel_send_data), NULL, NULL, NULL,
       G_TYPE_NONE, 1, G_TYPE_BYTES);
 
@@ -524,7 +524,31 @@ gst_webrtc_data_channel_send_data (GstWebRTCDataChannel * channel,
   g_return_if_fail (GST_IS_WEBRTC_DATA_CHANNEL (channel));
 
   klass = GST_WEBRTC_DATA_CHANNEL_GET_CLASS (channel);
-  klass->send_data (channel, data);
+  (void) klass->send_data (channel, data, NULL);
+}
+
+/**
+ * gst_webrtc_data_channel_send_data_full:
+ * @channel: a #GstWebRTCDataChannel
+ * @data: (nullable): a #GBytes or %NULL
+ * @error: (nullable): location to a #GError or %NULL
+ *
+ * Send @data as a data message over @channel.
+ *
+ * Returns: TRUE if @channel is open and data could be queued
+ *
+ * Since: 1.22
+ */
+gboolean
+gst_webrtc_data_channel_send_data_full (GstWebRTCDataChannel * channel,
+    GBytes * data, GError ** error)
+{
+  GstWebRTCDataChannelClass *klass;
+
+  g_return_val_if_fail (GST_IS_WEBRTC_DATA_CHANNEL (channel), FALSE);
+
+  klass = GST_WEBRTC_DATA_CHANNEL_GET_CLASS (channel);
+  return klass->send_data (channel, data, error);
 }
 
 /**
@@ -543,7 +567,30 @@ gst_webrtc_data_channel_send_string (GstWebRTCDataChannel * channel,
   g_return_if_fail (GST_IS_WEBRTC_DATA_CHANNEL (channel));
 
   klass = GST_WEBRTC_DATA_CHANNEL_GET_CLASS (channel);
-  klass->send_string (channel, str);
+  (void) klass->send_string (channel, str, NULL);
+}
+
+/**
+ * gst_webrtc_data_channel_send_string_full:
+ * @channel: a #GstWebRTCDataChannel
+ * @str: (nullable): a string or %NULL
+ *
+ * Send @str as a string message over @channel.
+ *
+ * Returns: TRUE if @channel is open and data could be queued
+ *
+ * Since: 1.22
+ */
+gboolean
+gst_webrtc_data_channel_send_string_full (GstWebRTCDataChannel * channel,
+    const gchar * str, GError ** error)
+{
+  GstWebRTCDataChannelClass *klass;
+
+  g_return_val_if_fail (GST_IS_WEBRTC_DATA_CHANNEL (channel), FALSE);
+
+  klass = GST_WEBRTC_DATA_CHANNEL_GET_CLASS (channel);
+  return klass->send_string (channel, str, error);
 }
 
 /**
index 8146c65..408872a 100644 (file)
@@ -37,14 +37,22 @@ GType gst_webrtc_data_channel_get_type(void);
 #define GST_WEBRTC_DATA_CHANNEL_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS((obj) ,GST_TYPE_WEBRTC_DATA_CHANNEL,GstWebRTCDataChannelClass))
 
 GST_WEBRTC_API
-void gst_webrtc_data_channel_send_data (GstWebRTCDataChannel * channel, GBytes * data);
+gboolean gst_webrtc_data_channel_send_data_full (GstWebRTCDataChannel * channel, GBytes * data, GError ** error);
 
 GST_WEBRTC_API
-void gst_webrtc_data_channel_send_string (GstWebRTCDataChannel * channel, const gchar * str);
+gboolean gst_webrtc_data_channel_send_string_full (GstWebRTCDataChannel * channel, const gchar * str, GError ** error);
 
 GST_WEBRTC_API
 void gst_webrtc_data_channel_close (GstWebRTCDataChannel * channel);
 
+#ifndef GST_REMOVE_DEPRECATED
+GST_WEBRTC_DEPRECATED_FOR(gst_webrtc_data_channel_send_data_full)
+void gst_webrtc_data_channel_send_data (GstWebRTCDataChannel * channel, GBytes * data);
+
+GST_WEBRTC_DEPRECATED_FOR(gst_webrtc_data_channel_send_string_full)
+void gst_webrtc_data_channel_send_string (GstWebRTCDataChannel * channel, const gchar * str);
+#endif
+
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(GstWebRTCDataChannel, g_object_unref)
 
 G_END_DECLS
index 3da7669..67676a3 100644 (file)
@@ -222,8 +222,8 @@ struct _GstWebRTCDataChannelClass
 {
   GObjectClass        parent_class;
 
-  void              (*send_data)   (GstWebRTCDataChannel * channel, GBytes *data);
-  void              (*send_string) (GstWebRTCDataChannel * channel, const gchar *str);
+  gboolean          (*send_data)   (GstWebRTCDataChannel * channel, GBytes *data, GError ** error);
+  gboolean          (*send_string) (GstWebRTCDataChannel * channel, const gchar *str, GError ** error);
   void              (*close)       (GstWebRTCDataChannel * channel);
 
   gpointer           _padding[GST_PADDING];
index b452423..fe20675 100644 (file)
 # endif
 #endif
 
+/**
+ * GST_WEBRTC_DEPRECATED: (attributes doc.skip=true)
+ */
+/**
+ * GST_WEBRTC_DEPRECATED_FOR: (attributes doc.skip=true)
+ */
+#ifndef GST_DISABLE_DEPRECATED
+#define GST_WEBRTC_DEPRECATED GST_WEBRTC_API
+#define GST_WEBRTC_DEPRECATED_FOR(f) GST_WEBRTC_API
+#else
+#define GST_WEBRTC_DEPRECATED G_DEPRECATED GST_WEBRTC_API
+#define GST_WEBRTC_DEPRECATED_FOR(f) G_DEPRECATED_FOR(f) GST_WEBRTC_API
+#endif
+
 #include <gst/webrtc/webrtc-enumtypes.h>
 
 /**
@@ -472,6 +486,13 @@ GQuark gst_webrtc_error_quark (void);
  * Since: 1.20
  */
 /**
+ * GST_WEBRTC_ERROR_TYPE_ERROR:
+ *
+ * type-error (maps to JavaScript TypeError)
+ *
+ * Since: 1.22
+ */
+/**
  * GST_WEBRTC_ERROR_INVALID_MODIFICATION:
  *
  * invalid-modification (part of WebIDL specification)
@@ -488,6 +509,7 @@ typedef enum /*<underscore_name=gst_webrtc_error>*/
   GST_WEBRTC_ERROR_HARDWARE_ENCODER_NOT_AVAILABLE,
   GST_WEBRTC_ERROR_ENCODER_ERROR,
   GST_WEBRTC_ERROR_INVALID_STATE,
+  GST_WEBRTC_ERROR_TYPE_ERROR,
   GST_WEBRTC_ERROR_INTERNAL_FAILURE,
   GST_WEBRTC_ERROR_INVALID_MODIFICATION,
 } GstWebRTCError;
index e92a659..144b38d 100644 (file)
@@ -2094,13 +2094,16 @@ have_data_channel_transfer_string (struct test_webrtc *t, GstElement * element,
 {
   GObject *other = user_data;
   GstWebRTCDataChannelState state;
+  GError *error = NULL;
 
   g_object_get (our, "ready-state", &state, NULL);
   fail_unless_equals_int (GST_WEBRTC_DATA_CHANNEL_STATE_OPEN, state);
 
   g_object_set_data_full (our, "expected", g_strdup (test_string), g_free);
 
-  g_signal_emit_by_name (other, "send-string", test_string);
+  fail_unless (gst_webrtc_data_channel_send_string_full (GST_WEBRTC_DATA_CHANNEL
+          (other), test_string, &error));
+  g_assert_null (error);
 }
 
 GST_START_TEST (test_data_channel_transfer_string)
@@ -2171,6 +2174,7 @@ have_data_channel_transfer_data (struct test_webrtc *t, GstElement * element,
   GObject *other = user_data;
   GBytes *data = g_bytes_new_static (test_string, strlen (test_string));
   GstWebRTCDataChannelState state;
+  GError *error = NULL;
 
   g_object_get (our, "ready-state", &state, NULL);
   fail_unless_equals_int (GST_WEBRTC_DATA_CHANNEL_STATE_OPEN, state);
@@ -2178,7 +2182,9 @@ have_data_channel_transfer_data (struct test_webrtc *t, GstElement * element,
   g_object_set_data_full (our, "expected", g_bytes_ref (data),
       (GDestroyNotify) g_bytes_unref);
 
-  g_signal_emit_by_name (other, "send-data", data);
+  fail_unless (gst_webrtc_data_channel_send_data_full (GST_WEBRTC_DATA_CHANNEL
+          (other), data, &error));
+  g_assert_null (error);
   g_bytes_unref (data);
 }
 
@@ -2445,7 +2451,8 @@ have_data_channel_check_low_threshold_emitted (struct test_webrtc *t,
 
   g_signal_connect (our, "on-error", G_CALLBACK (on_channel_error_not_reached),
       NULL);
-  g_signal_emit_by_name (our, "send-string", "A");
+  gst_webrtc_data_channel_send_string_full (GST_WEBRTC_DATA_CHANNEL (our), "A",
+      NULL);
 }
 
 GST_START_TEST (test_data_channel_low_threshold)
@@ -2484,14 +2491,6 @@ GST_START_TEST (test_data_channel_low_threshold)
 GST_END_TEST;
 
 static void
-on_channel_error (GObject * channel, GError * error, struct test_webrtc *t)
-{
-  g_assert_nonnull (error);
-
-  test_webrtc_signal_state (t, STATE_CUSTOM);
-}
-
-static void
 have_data_channel_transfer_large_data (struct test_webrtc *t,
     GstElement * element, GObject * our, gpointer user_data)
 {
@@ -2500,6 +2499,7 @@ have_data_channel_transfer_large_data (struct test_webrtc *t,
   guint8 *random_data = g_new (guint8, size);
   GBytes *data;
   gsize i;
+  GError *error = NULL;
 
   for (i = 0; i < size; i++)
     random_data[i] = (guint8) (i & 0xff);
@@ -2511,9 +2511,15 @@ have_data_channel_transfer_large_data (struct test_webrtc *t,
       (GDestroyNotify) g_bytes_unref);
   g_signal_connect (our, "on-message-data", G_CALLBACK (on_message_data), t);
 
-  g_signal_connect (other, "on-error", G_CALLBACK (on_channel_error), t);
-  g_signal_emit_by_name (other, "send-data", data);
+  g_signal_connect (other, "on-error",
+      G_CALLBACK (on_channel_error_not_reached), NULL);
+  fail_if (gst_webrtc_data_channel_send_data_full (GST_WEBRTC_DATA_CHANNEL
+          (other), data, &error));
+  g_assert_nonnull (error);
+  g_clear_error (&error);
   g_bytes_unref (data);
+
+  test_webrtc_signal_state_unlocked (t, STATE_CUSTOM);
 }
 
 GST_START_TEST (test_data_channel_max_message_size)