controller: factor out duplicated code and add a description for it.

Also fix typo in the tests while reviewing them.

2 files changed, 56 insertions, 60 deletions

diff --git a/libs/gst/controller/gstcontroller.c b/libs/gst/controller/gstcontroller.c
index 58f22d8ad0..c9c561c6f9 100644
--- a/libs/gst/controller/gstcontroller.c
+++ b/libs/gst/controller/gstcontroller.c
@@ -201,6 +201,58 @@ gst_controller_find_controlled_property (GstController * self,
   return NULL;
 }
 
+/*
+ * gst_controller_add_property:
+ * @self: the controller object or %NULL if none yet exists
+ * @object: object to bind the property
+ * @name: name of projecty in @object
+ * @ref_existing: pointer to flag that tracks if we need to ref an existng
+ *   controller still
+ *
+ * Creates a new #GstControlledProperty if there is none for property @name yet.
+ * In case this is the first controlled propery, it creates the controller as
+ * well.
+ *
+ * Returns: a newly created controller object or reffed existing one with the
+ * given property bound.
+ */
+static GstController *
+gst_controller_add_property (GstController * self, GObject * object,
+    gchar * name, gboolean * ref_existing)
+{
+  /* test if this property isn't yet controlled */
+  if (!self || !gst_controller_find_controlled_property (self, name)) {
+    GstControlledProperty *prop;
+
+    /* create GstControlledProperty and add to self->propeties List */
+    if ((prop = gst_controlled_property_new (object, name))) {
+      /* if we don't have a controller object yet, now is the time to create one */
+      if (!self) {
+        self = g_object_new (GST_TYPE_CONTROLLER, NULL);
+        self->object = g_object_ref (object);
+        /* store the controller */
+        g_object_set_qdata (object, priv_gst_controller_key, self);
+        *ref_existing = FALSE;
+      } else {
+        /* only want one single _ref(), even for multiple properties */
+        if (*ref_existing) {
+          g_object_ref (self);
+          *ref_existing = FALSE;
+          GST_INFO ("returning existing controller");
+        }
+      }
+      self->properties = g_list_prepend (self->properties, prop);
+    }
+  } else {
+    GST_WARNING ("trying to control property again");
+    if (*ref_existing) {
+      g_object_ref (self);
+      *ref_existing = FALSE;
+    }
+  }
+  return self;
+}
+
 /* methods */
 
 /**
@@ -216,7 +268,6 @@ GstController *
 gst_controller_new_valist (GObject * object, va_list var_args)
 {
   GstController *self;
-  GstControlledProperty *prop;
   gboolean ref_existing = TRUE;
   gchar *name;
 
@@ -227,34 +278,7 @@ gst_controller_new_valist (GObject * object, va_list var_args)
   self = g_object_get_qdata (object, priv_gst_controller_key);
   /* create GstControlledProperty for each property */
   while ((name = va_arg (var_args, gchar *))) {
-    /* test if this property isn't yet controlled */
-    if (!self || !gst_controller_find_controlled_property (self, name)) {
-      /* create GstControlledProperty and add to self->propeties List */
-      if ((prop = gst_controlled_property_new (object, name))) {
-        /* if we don't have a controller object yet, now is the time to create one */
-        if (!self) {
-          self = g_object_new (GST_TYPE_CONTROLLER, NULL);
-          self->object = g_object_ref (object);
-          /* store the controller */
-          g_object_set_qdata (object, priv_gst_controller_key, self);
-          ref_existing = FALSE;
-        } else {
-          /* only want one single _ref(), even for multiple properties */
-          if (ref_existing) {
-            g_object_ref (self);
-            ref_existing = FALSE;
-            GST_INFO ("returning existing controller");
-          }
-        }
-        self->properties = g_list_prepend (self->properties, prop);
-      }
-    } else {
-      GST_WARNING ("trying to control property again");
-      if (ref_existing) {
-        g_object_ref (self);
-        ref_existing = FALSE;
-      }
-    }
+    self = gst_controller_add_property (self, object, name, &ref_existing);
   }
   va_end (var_args);
 
@@ -276,7 +300,6 @@ GstController *
 gst_controller_new_list (GObject * object, GList * list)
 {
   GstController *self;
-  GstControlledProperty *prop;
   gboolean ref_existing = TRUE;
   gchar *name;
   GList *node;
@@ -289,34 +312,7 @@ gst_controller_new_list (GObject * object, GList * list)
   /* create GstControlledProperty for each property */
   for (node = list; node; node = g_list_next (node)) {
     name = (gchar *) node->data;
-    /* test if this property isn't yet controlled */
-    if (!self || !gst_controller_find_controlled_property (self, name)) {
-      /* create GstControlledProperty and add to self->propeties List */
-      if ((prop = gst_controlled_property_new (object, name))) {
-        /* if we don't have a controller object yet, now is the time to create one */
-        if (!self) {
-          self = g_object_new (GST_TYPE_CONTROLLER, NULL);
-          self->object = g_object_ref (object);
-          /* store the controller */
-          g_object_set_qdata (object, priv_gst_controller_key, self);
-          ref_existing = FALSE;
-        } else {
-          /* only want one single _ref(), even for multiple properties */
-          if (ref_existing) {
-            g_object_ref (self);
-            ref_existing = FALSE;
-            GST_INFO ("returning existing controller");
-          }
-        }
-        self->properties = g_list_prepend (self->properties, prop);
-      }
-    } else {
-      GST_WARNING ("trying to control property again");
-      if (ref_existing) {
-        g_object_ref (self);
-        ref_existing = FALSE;
-      }
-    }
+    self = gst_controller_add_property (self, object, name, &ref_existing);
   }
 
   if (self)
diff --git a/tests/check/libs/controller.c b/tests/check/libs/controller.c
index fa3998b7b1..6baf434dc0 100644
--- a/tests/check/libs/controller.c
+++ b/tests/check/libs/controller.c
@@ -319,7 +319,7 @@ GST_START_TEST (controller_new_fail4)
 
 GST_END_TEST;
 
-/* tests for static params */
+/* tests for construct-only params */
 GST_START_TEST (controller_new_fail5)
 {
   GstController *ctrl;
@@ -456,7 +456,7 @@ GST_START_TEST (controller_param_twice)
   res = gst_controller_remove_properties (ctrl, "ulong", NULL);
   fail_unless (res, NULL);
 
-  /* removing it agian should not work */
+  /* removing it again should not work */
   res = gst_controller_remove_properties (ctrl, "ulong", NULL);
   fail_unless (!res, NULL);