webrtc: fix hangup when duplicate sctp association IDs chosen

Fixes an issue where the webrtcbin would hangup when finalizing due
to the sctpenc hanging up when finalizing. This occurred when the
webrtcbin chose to use a sctp association ID already in use.

The sctpenc would fail to reach the paused state, but startup a task
anyways that was never stopped.

This commit modifies the behavior to not choose sctp association IDs
randomly, and instead only choose one that is free. It also prevents the
sctpenc from starting up that task if it fails to reach the paused state.

Fixes: #4188
Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/8607>

subprojects/gst-plugins-bad/docs/plugins/gst_plugins_cache.json

@@ -243704,6 +243704,18 @@
                     }
                 },
                 "properties": {
+                    "automatic-association-id": {
+                        "blurb": "Whether a SCTP Association ID should be automatically generated.",
+                        "conditionally-available": false,
+                        "construct": false,
+                        "construct-only": false,
+                        "controllable": false,
+                        "default": "false",
+                        "mutable": "null",
+                        "readable": true,
+                        "type": "gboolean",
+                        "writable": true
+                    },
                     "local-sctp-port": {
                         "blurb": "Local sctp port for the sctp association. The remote port is configured via the GstSctpEnc element.",
                         "conditionally-available": false,

subprojects/gst-plugins-bad/ext/sctp/gstsctpdec.c

@@ -64,6 +64,7 @@ enum
 
   PROP_GST_SCTP_ASSOCIATION_ID,
   PROP_LOCAL_SCTP_PORT,
+  PROP_AUTOMATIC_ASSOCIATION_ID,
 
   NUM_PROPERTIES
 };
@@ -72,6 +73,7 @@ static GParamSpec *properties[NUM_PROPERTIES];
 
 #define DEFAULT_GST_SCTP_ASSOCIATION_ID 1
 #define DEFAULT_LOCAL_SCTP_PORT 0
+#define DEFAULT_AUTOMATIC_ASSOCIATION_ID FALSE
 #define MAX_SCTP_PORT 65535
 #define MAX_GST_SCTP_ASSOCIATION_ID 65535
 
@@ -207,6 +209,20 @@ gst_sctp_dec_class_init (GstSctpDecClass * klass)
       0, MAX_SCTP_PORT, DEFAULT_LOCAL_SCTP_PORT,
       G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
 
+  /**
+   * GstSctpDec:automatic-association-id:
+   *
+   * Whether a SCTP Association ID should be automatically generated.
+   *
+   * Since: 1.28
+   */
+  properties[PROP_AUTOMATIC_ASSOCIATION_ID] =
+      g_param_spec_boolean ("automatic-association-id",
+      "Automatic SCTP Association ID",
+      "Whether a SCTP Association ID should be automatically generated.",
+      DEFAULT_AUTOMATIC_ASSOCIATION_ID,
+      G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
+
   g_object_class_install_properties (gobject_class, NUM_PROPERTIES, properties);
 
   signals[SIGNAL_RESET_STREAM] = g_signal_new ("reset-stream",
@@ -226,7 +242,9 @@ gst_sctp_dec_init (GstSctpDec * self)
 {
   self->sctp_association_id = DEFAULT_GST_SCTP_ASSOCIATION_ID;
   self->local_sctp_port = DEFAULT_LOCAL_SCTP_PORT;
+  self->automatic_association_id = DEFAULT_AUTOMATIC_ASSOCIATION_ID;
 
+  self->automatic_sctp_association = NULL;
   self->flow_combiner = gst_flow_combiner_new ();
 
   self->sink_pad = gst_pad_new_from_static_template (&sink_template, "sink");
@@ -246,11 +264,29 @@ gst_sctp_dec_set_property (GObject * object, guint prop_id,
 
   switch (prop_id) {
     case PROP_GST_SCTP_ASSOCIATION_ID:
-      self->sctp_association_id = g_value_get_uint (value);
+      if (self->automatic_association_id) {
+        GST_WARNING_OBJECT (object,
+            "Cannot modify association id if automatic association id enabled");
+      } else {
+        self->sctp_association_id = g_value_get_uint (value);
+      }
       break;
     case PROP_LOCAL_SCTP_PORT:
       self->local_sctp_port = g_value_get_uint (value);
       break;
+    case PROP_AUTOMATIC_ASSOCIATION_ID:
+      self->automatic_association_id = g_value_get_boolean (value);
+      if (self->automatic_association_id && !self->automatic_sctp_association) {
+        self->automatic_sctp_association = gst_sctp_association_create ();
+        if (self->automatic_sctp_association)
+          self->sctp_association_id =
+              self->automatic_sctp_association->association_id;
+      } else if (!self->automatic_association_id
+          && self->automatic_sctp_association) {
+        gst_clear_object (&self->automatic_sctp_association);
+        self->sctp_association_id = DEFAULT_GST_SCTP_ASSOCIATION_ID;
+      }
+      break;
     default:
       G_OBJECT_WARN_INVALID_PROPERTY_ID (self, prop_id, pspec);
       break;
@@ -270,6 +306,9 @@ gst_sctp_dec_get_property (GObject * object, guint prop_id, GValue * value,
     case PROP_LOCAL_SCTP_PORT:
       g_value_set_uint (value, self->local_sctp_port);
       break;
+    case PROP_AUTOMATIC_ASSOCIATION_ID:
+      g_value_set_boolean (value, self->automatic_association_id);
+      break;
     default:
       G_OBJECT_WARN_INVALID_PROPERTY_ID (self, prop_id, pspec);
       break;
@@ -284,6 +323,9 @@ gst_sctp_dec_finalize (GObject * object)
   gst_flow_combiner_free (self->flow_combiner);
   self->flow_combiner = NULL;
 
+  if (self->automatic_sctp_association)
+    gst_object_unref (self->automatic_sctp_association);
+
   G_OBJECT_CLASS (parent_class)->finalize (object);
 }
 
@@ -467,7 +509,13 @@ configure_association (GstSctpDec * self)
 {
   gint state;
 
-  self->sctp_association = gst_sctp_association_get (self->sctp_association_id);
+  if (self->automatic_sctp_association) {
+    self->sctp_association = self->automatic_sctp_association;
+    self->automatic_sctp_association = NULL;
+  } else {
+    self->sctp_association =
+        gst_sctp_association_get (self->sctp_association_id);
+  }
 
   g_object_get (self->sctp_association, "state", &state, NULL);
 

subprojects/gst-plugins-bad/ext/sctp/gstsctpdec.h

@@ -50,7 +50,9 @@ struct _GstSctpDec
   GstPad *sink_pad;
   guint sctp_association_id;
   guint local_sctp_port;
+  gboolean automatic_association_id;
 
+  GstSctpAssociation *automatic_sctp_association;
   GstSctpAssociation *sctp_association;
   gulong signal_handler_stream_reset;
 };

subprojects/gst-plugins-bad/ext/sctp/gstsctpenc.c

@@ -395,8 +395,9 @@ gst_sctp_enc_change_state (GstElement * element, GstStateChange transition)
     case GST_STATE_CHANGE_NULL_TO_READY:
       break;
     case GST_STATE_CHANGE_READY_TO_PAUSED:
-      gst_pad_start_task (self->src_pad,
-          (GstTaskFunction) gst_sctp_enc_srcpad_loop, self->src_pad, NULL);
+      if (ret != GST_STATE_CHANGE_FAILURE)
+        gst_pad_start_task (self->src_pad,
+            (GstTaskFunction) gst_sctp_enc_srcpad_loop, self->src_pad, NULL);
       break;
     case GST_STATE_CHANGE_PLAYING_TO_PAUSED:
       break;

subprojects/gst-plugins-bad/ext/sctp/sctpassociation.c

@@ -67,7 +67,22 @@ gst_sctp_association_state_get_type (void)
   return id;
 }
 
-G_DEFINE_TYPE (GstSctpAssociation, gst_sctp_association, G_TYPE_OBJECT);
+static void
+_init_debug (void)
+{
+  static gsize _init = 0;
+
+  if (g_once_init_enter (&_init)) {
+    GST_DEBUG_CATEGORY_INIT (gst_sctp_association_debug_category,
+        "sctpassociation", 0, "debug category for sctpassociation");
+    GST_DEBUG_CATEGORY_INIT (gst_sctp_debug_category, "sctplib", 0,
+        "debug category for messages from usrsctp");
+    g_once_init_leave (&_init, 1);
+  }
+}
+
+G_DEFINE_TYPE_WITH_CODE (GstSctpAssociation, gst_sctp_association,
+    G_TYPE_OBJECT, _init_debug ());
 
 enum
 {
@@ -361,11 +376,6 @@ gst_sctp_association_get (guint32 association_id)
   GstSctpAssociation *association;
 
   G_LOCK (associations_lock);
-  GST_DEBUG_CATEGORY_INIT (gst_sctp_association_debug_category,
-      "sctpassociation", 0, "debug category for sctpassociation");
-  GST_DEBUG_CATEGORY_INIT (gst_sctp_debug_category,
-      "sctplib", 0, "debug category for messages from usrsctp");
-
   if (!associations) {
     associations =
         g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, NULL);
@@ -386,6 +396,40 @@ gst_sctp_association_get (guint32 association_id)
   return association;
 }
 
+GstSctpAssociation *
+gst_sctp_association_create (void)
+{
+  GstSctpAssociation *association;
+  guint association_id = 0;
+
+  G_LOCK (associations_lock);
+  if (!associations) {
+    associations =
+        g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, NULL);
+  }
+
+  while (association_id < G_MAXUINT16) {
+    association =
+        g_hash_table_lookup (associations, GUINT_TO_POINTER (association_id));
+    if (!association)
+      break;
+    association_id++;
+  }
+
+  if (association) {
+    G_UNLOCK (associations_lock);
+    return NULL;
+  }
+
+  association =
+      g_object_new (GST_SCTP_TYPE_ASSOCIATION, "association-id", association_id,
+      NULL);
+  g_hash_table_insert (associations, GUINT_TO_POINTER (association_id),
+      association);
+  G_UNLOCK (associations_lock);
+  return association;
+}
+
 gboolean
 gst_sctp_association_start (GstSctpAssociation * self)
 {

subprojects/gst-plugins-bad/ext/sctp/sctpassociation.h

@@ -103,6 +103,7 @@ struct _GstSctpAssociationClass
 GType gst_sctp_association_get_type (void);
 
 GstSctpAssociation *gst_sctp_association_get (guint32 association_id);
+GstSctpAssociation *gst_sctp_association_create (void);
 
 gboolean gst_sctp_association_start (GstSctpAssociation * self);
 void gst_sctp_association_set_on_packet_out (GstSctpAssociation * self,

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

@@ -193,13 +193,14 @@ static void
 webrtc_sctp_transport_constructed (GObject * object)
 {
   WebRTCSCTPTransport *sctp = WEBRTC_SCTP_TRANSPORT (object);
-  guint association_id;
-
-  association_id = g_random_int_range (0, G_MAXUINT16);
 
   sctp->sctpdec =
       g_object_ref_sink (gst_element_factory_make ("sctpdec", NULL));
-  g_object_set (sctp->sctpdec, "sctp-association-id", association_id, NULL);
+  g_object_set (sctp->sctpdec, "automatic-association-id", TRUE, NULL);
+
+  guint association_id;
+  g_object_get (sctp->sctpdec, "sctp-association-id", &association_id, NULL);
+
   sctp->sctpenc =
       g_object_ref_sink (gst_element_factory_make ("sctpenc", NULL));
   g_object_set (sctp->sctpenc, "sctp-association-id", association_id, NULL);