bufferpool: Add support for reconfiguring a pool
authorNicolas Dufresne <nicolas.dufresne@collabora.co.uk>
Tue, 6 May 2014 19:35:14 +0000 (15:35 -0400)
committerNicolas Dufresne <nicolas.dufresne@collabora.com>
Thu, 8 May 2014 17:08:21 +0000 (13:08 -0400)
If a pool config is being configured again, check if the configuration have changed.
If not, skip that step. Finally, if the pool is active, try deactivating it.

https://bugzilla.gnome.org/show_bug.cgi?id=728268

gst/gstbufferpool.c
tests/check/gst/gstbufferpool.c

index 999422e..192693f 100644 (file)
@@ -575,9 +575,11 @@ wrong_config:
  * @pool: a #GstBufferPool
  * @config: (transfer full): a #GstStructure
  *
- * Set the configuration of the pool. The pool must be inactive and all buffers
- * allocated form this pool must be returned or else this function will do
- * nothing and return FALSE.
+ * Set the configuration of the pool. If the pool is already configured, and
+ * the configuration haven't change, this function will return %TRUE. If the
+ * pool is active, this function will try deactivating it. Buffers allocated
+ * form this pool must be returned or else this function will do nothing and
+ * return FALSE.
  *
  * @config is a #GstStructure that contains the configuration parameters for
  * the pool. A default and mandatory set of parameters can be configured with
@@ -605,9 +607,24 @@ gst_buffer_pool_set_config (GstBufferPool * pool, GstStructure * config)
   priv = pool->priv;
 
   GST_BUFFER_POOL_LOCK (pool);
+
+  /* nothing to do if config is unchanged */
+  if (priv->configured && gst_structure_is_equal (config, priv->config))
+    goto config_unchanged;
+
   /* can't change the settings when active */
-  if (priv->active)
-    goto was_active;
+  if (priv->active) {
+    GST_BUFFER_POOL_UNLOCK (pool);
+    if (!gst_buffer_pool_set_active (pool, FALSE)) {
+      GST_BUFFER_POOL_LOCK (pool);
+      goto was_active;
+    }
+    GST_BUFFER_POOL_LOCK (pool);
+
+    /* not likely but as we released the lock */
+    if (priv->active)
+      goto was_active;
+  }
 
   /* we can't change when outstanding buffers */
   if (g_atomic_int_get (&priv->outstanding) != 0)
@@ -635,6 +652,12 @@ gst_buffer_pool_set_config (GstBufferPool * pool, GstStructure * config)
 
   return result;
 
+config_unchanged:
+  {
+    gst_structure_free (config);
+    GST_BUFFER_POOL_UNLOCK (pool);
+    return TRUE;
+  }
   /* ERRORS */
 was_active:
   {
index b60f9da..ba27a2a 100644 (file)
@@ -26,9 +26,11 @@ create_pool (guint size, guint min_buf, guint max_buf)
 {
   GstBufferPool *pool = gst_buffer_pool_new ();
   GstStructure *conf = gst_buffer_pool_get_config (pool);
+  GstCaps *caps = gst_caps_new_empty_simple ("test/data");
 
-  gst_buffer_pool_config_set_params (conf, NULL, size, min_buf, max_buf);
+  gst_buffer_pool_config_set_params (conf, caps, size, min_buf, max_buf);
   gst_buffer_pool_set_config (pool, conf);
+  gst_caps_unref (caps);
 
   return pool;
 }
@@ -172,6 +174,52 @@ GST_START_TEST (test_buffer_modify_discard)
 
 GST_END_TEST;
 
+GST_START_TEST (test_pool_activation_and_config)
+{
+  GstBufferPool *pool = gst_buffer_pool_new ();
+  GstStructure *config = gst_buffer_pool_get_config (pool);
+  GstCaps *caps = gst_caps_new_empty_simple ("test/data");
+  GstBuffer *buffer = NULL;
+
+  /* unconfigured pool cannot be activated */
+  fail_if (gst_buffer_pool_set_active (pool, TRUE));
+
+  gst_buffer_pool_config_set_params (config, caps, 10, 10, 0);
+  fail_unless (gst_buffer_pool_set_config (pool, config));
+  fail_unless (gst_buffer_pool_set_active (pool, TRUE));
+
+  /* setting the same config on an active pool is ok */
+  config = gst_buffer_pool_get_config (pool);
+  fail_unless (gst_buffer_pool_set_config (pool, config));
+
+  /* setting a different config should deactivate the pool */
+  config = gst_buffer_pool_get_config (pool);
+  gst_buffer_pool_config_set_params (config, caps, 12, 10, 0);
+  fail_unless (gst_buffer_pool_set_config (pool, config));
+  fail_if (gst_buffer_pool_is_active (pool));
+
+  /* though it should fail if there is outstanding buffers */
+  gst_buffer_pool_set_active (pool, TRUE);
+  gst_buffer_pool_acquire_buffer (pool, &buffer, NULL);
+  fail_if (buffer == NULL);
+  config = gst_buffer_pool_get_config (pool);
+  gst_buffer_pool_config_set_params (config, caps, 10, 10, 0);
+  fail_if (gst_buffer_pool_set_config (pool, config));
+
+  /* and work when last buffer is back */
+  config = gst_buffer_pool_get_config (pool);
+  gst_buffer_pool_config_set_params (config, caps, 10, 10, 0);
+  gst_buffer_unref (buffer);
+  fail_unless (gst_buffer_pool_set_config (pool, config));
+  fail_unless (gst_buffer_pool_set_active (pool, TRUE));
+
+  gst_buffer_pool_set_active (pool, FALSE);
+  gst_object_unref (pool);
+  gst_caps_unref (caps);
+}
+
+GST_END_TEST;
+
 static Suite *
 gst_buffer_pool_suite (void)
 {
@@ -187,6 +235,7 @@ gst_buffer_pool_suite (void)
   tcase_add_test (tc_chain, test_pool_config_buffer_size);
   tcase_add_test (tc_chain, test_inactive_pool_returns_flushing);
   tcase_add_test (tc_chain, test_buffer_modify_discard);
+  tcase_add_test (tc_chain, test_pool_activation_and_config);
 
   return s;
 }