multiqueue: Make assignment of queue IDs and pad names threadsafe

Also add a test for naming pads by the caller and return NULL
when requesting an already existing pad.

diff --git a/plugins/elements/gstmultiqueue.c b/plugins/elements/gstmultiqueue.c
index e012326..f043daa 100644 (file)
--- a/plugins/elements/gstmultiqueue.c
+++ b/plugins/elements/gstmultiqueue.c
@@ -174,7 +174,7 @@ struct _GstMultiQueueItem
   guint32 posid;
 };
 
-static GstSingleQueue *gst_single_queue_new (GstMultiQueue * mqueue, guint id);
+static GstSingleQueue *gst_single_queue_new (GstMultiQueue * mqueue, gint id);
 static void gst_single_queue_free (GstSingleQueue * squeue);
 
 static void wake_up_next_non_linked (GstMultiQueue * mq);
@@ -592,41 +592,20 @@ gst_multi_queue_request_new_pad (GstElement * element, GstPadTemplate * temp,
 {
   GstMultiQueue *mqueue = GST_MULTI_QUEUE (element);
   GstSingleQueue *squeue;
-  GList *tmp = NULL;
-  gint temp_id = 0, id = -1;
+  gint temp_id = -1;
 
-  if (name)
+  if (name) {
     sscanf (name + 4, "%d", &temp_id);
-
-  GST_MULTI_QUEUE_MUTEX_LOCK (mqueue);
-  while (id == -1) {
-    for (tmp = mqueue->queues; tmp; tmp = g_list_next (tmp)) {
-      squeue = (GstSingleQueue *) tmp->data;
-      if (squeue->id == temp_id) {
-        temp_id++;
-        break;
-      }
-    }
-    if (tmp == NULL)
-      id = temp_id;
+    GST_LOG_OBJECT (element, "name : %s (id %d)", GST_STR_NULL (name), temp_id);
   }
-  mqueue->nbqueues++;
-  GST_MULTI_QUEUE_MUTEX_UNLOCK (mqueue);
-
-  GST_LOG_OBJECT (element, "name : %s (id %d)", GST_STR_NULL (name), id);
 
   /* Create a new single queue, add the sink and source pad and return the sink pad */
-  squeue = gst_single_queue_new (mqueue, id);
-
-  GST_MULTI_QUEUE_MUTEX_LOCK (mqueue);
-  mqueue->queues = g_list_append (mqueue->queues, squeue);
-  mqueue->queues_cookie++;
-  GST_MULTI_QUEUE_MUTEX_UNLOCK (mqueue);
+  squeue = gst_single_queue_new (mqueue, temp_id);
 
   GST_DEBUG_OBJECT (mqueue, "Returning pad %s:%s",
       GST_DEBUG_PAD_NAME (squeue->sinkpad));
 
-  return squeue->sinkpad;
+  return squeue ? squeue->sinkpad : NULL;
 }
 
 static void
@@ -1727,15 +1706,37 @@ gst_single_queue_free (GstSingleQueue * sq)
 }
 
 static GstSingleQueue *
-gst_single_queue_new (GstMultiQueue * mqueue, guint id)
+gst_single_queue_new (GstMultiQueue * mqueue, gint id)
 {
   GstSingleQueue *sq;
   gchar *name;
+  GList *tmp;
+  gint temp_id = (id == -1) ? 0 : id;
+
+  GST_MULTI_QUEUE_MUTEX_LOCK (mqueue);
+
+  /* Find an unused queue ID, if possible the passed one */
+  for (tmp = mqueue->queues; tmp; tmp = g_list_next (tmp)) {
+    GstSingleQueue *sq2 = (GstSingleQueue *) tmp->data;
+    /* This works because the IDs are sorted in ascending order */
+    if (sq2->id == temp_id) {
+      /* If this ID was requested by the caller return NULL,
+       * otherwise just get us the next one */
+      if (id == -1)
+        temp_id = sq2->id + 1;
+      else
+        return NULL;
+    } else if (sq2->id > temp_id) {
+      break;
+    }
+  }
 
   sq = g_new0 (GstSingleQueue, 1);
+  mqueue->nbqueues++;
+  sq->id = temp_id;
 
-  GST_MULTI_QUEUE_MUTEX_LOCK (mqueue);
-  sq->id = id;
+  mqueue->queues = g_list_insert_before (mqueue->queues, tmp, sq);
+  mqueue->queues_cookie++;
 
   /* copy over max_size and extra_size so we don't need to take the lock
    * any longer when checking if the queue is full. */

diff --git a/tests/check/elements/multiqueue.c b/tests/check/elements/multiqueue.c
index e885ff3..467df9c 100644 (file)
--- a/tests/check/elements/multiqueue.c
+++ b/tests/check/elements/multiqueue.c
@@ -243,6 +243,50 @@ mq_sinkpad_to_srcpad (GstElement * mq, GstPad * sink)
   return srcpad;
 }
 
+GST_START_TEST (test_request_pads_named)
+{
+  GstElement *mq;
+  GstPad *sink1, *sink2, *sink3, *sink4;
+
+  mq = gst_element_factory_make ("multiqueue", NULL);
+
+  sink1 = gst_element_get_request_pad (mq, "sink1");
+  fail_unless (sink1 != NULL);
+  fail_unless (GST_IS_PAD (sink1));
+  fail_unless (GST_PAD_IS_SINK (sink1));
+  fail_unless_equals_string (GST_PAD_NAME (sink1), "sink1");
+  GST_LOG ("Got pad %s:%s", GST_DEBUG_PAD_NAME (sink1));
+
+  sink3 = gst_element_get_request_pad (mq, "sink3");
+  fail_unless (sink3 != NULL);
+  fail_unless (GST_IS_PAD (sink3));
+  fail_unless (GST_PAD_IS_SINK (sink3));
+  fail_unless_equals_string (GST_PAD_NAME (sink3), "sink3");
+  GST_LOG ("Got pad %s:%s", GST_DEBUG_PAD_NAME (sink3));
+
+  sink2 = gst_element_get_request_pad (mq, "sink2");
+  fail_unless (sink2 != NULL);
+  fail_unless (GST_IS_PAD (sink2));
+  fail_unless (GST_PAD_IS_SINK (sink2));
+  fail_unless_equals_string (GST_PAD_NAME (sink2), "sink2");
+  GST_LOG ("Got pad %s:%s", GST_DEBUG_PAD_NAME (sink2));
+
+  /* This gets us the first unused id, sink0 */
+  sink4 = gst_element_get_request_pad (mq, "sink%d");
+  fail_unless (sink4 != NULL);
+  fail_unless (GST_IS_PAD (sink4));
+  fail_unless (GST_PAD_IS_SINK (sink4));
+  fail_unless_equals_string (GST_PAD_NAME (sink4), "sink0");
+  GST_LOG ("Got pad %s:%s", GST_DEBUG_PAD_NAME (sink4));
+
+  GST_LOG ("Cleaning up");
+  gst_object_unref (sink1);
+  gst_object_unref (sink2);
+  gst_object_unref (mq);
+}
+
+GST_END_TEST;
+
 static GstCaps *
 mq_dummypad_getcaps (GstPad * sinkpad)
 {
@@ -652,6 +696,7 @@ multiqueue_suite (void)
   tcase_add_test (tc_chain, test_simple_shutdown_while_running);
 
   tcase_add_test (tc_chain, test_request_pads);
+  tcase_add_test (tc_chain, test_request_pads_named);
 
   tcase_add_test (tc_chain, test_output_order);