trackelement: Simplify the way we handle children properties

So subclass do not have to implement a new logic all the time, but
instead can use a simple method to add properties as needed.

docs/libs/ges-sections.txt

@@ -1042,6 +1042,7 @@ GESTrackElementAsset
 ges_track_element_asset_get_type
 ges_track_element_asset_get_track_type
 ges_track_element_asset_set_track_type
+ges_track_element_add_children_props
 <SUBSECTION Standard>
 GESTrackElementAssetPrivate
 GES_TRACK_ELEMENT_ASSET

ges/ges-base-effect.c

@@ -32,8 +32,6 @@
 
 G_DEFINE_ABSTRACT_TYPE (GESBaseEffect, ges_base_effect, GES_TYPE_OPERATION);
 
-static GHashTable *ges_base_effect_get_props_hashtable (GESTrackElement * self);
-
 struct _GESBaseEffectPrivate
 {
   void *nothing;
@@ -42,11 +40,7 @@ struct _GESBaseEffectPrivate
 static void
 ges_base_effect_class_init (GESBaseEffectClass * klass)
 {
-  GESTrackElementClass *obj_bg_class = GES_TRACK_ELEMENT_CLASS (klass);
-
   g_type_class_add_private (klass, sizeof (GESBaseEffectPrivate));
-
-  obj_bg_class->get_props_hastable = ges_base_effect_get_props_hashtable;
 }
 
 static void
@@ -56,95 +50,3 @@ ges_base_effect_init (GESBaseEffect * self)
       G_TYPE_INSTANCE_GET_PRIVATE (self, GES_TYPE_BASE_EFFECT,
       GESBaseEffectPrivate);
 }
-
-/*  Virtual methods */
-static GHashTable *
-ges_base_effect_get_props_hashtable (GESTrackElement * self)
-{
-  GValue item = { 0, };
-  GstIterator *it;
-  GParamSpec **parray;
-  GObjectClass *class;
-  const gchar *klass;
-  GstElementFactory *factory;
-  GstElement *element;
-  gboolean done = FALSE;
-  GHashTable *ret = NULL;
-
-  element = ges_track_element_get_element (self);
-  if (!element) {
-    GST_DEBUG
-        ("Can't build the property hashtable until the gnlobject is created");
-    return NULL;
-  }
-
-  ret = g_hash_table_new_full ((GHashFunc) pspec_hash, pspec_equal,
-      (GDestroyNotify) g_param_spec_unref, gst_object_unref);
-
-  /*  We go over child elements recursivly, and add writable properties to the
-   *  hashtable
-   *  FIXME: Add a blacklist of properties */
-  it = gst_bin_iterate_recurse (GST_BIN (element));
-
-  while (!done) {
-    switch (gst_iterator_next (it, &item)) {
-      case GST_ITERATOR_OK:
-      {
-        gchar **categories;
-        guint category;
-        GstElement *child = g_value_get_object (&item);
-
-        factory = gst_element_get_factory (child);
-        klass = gst_element_factory_get_metadata (factory,
-            GST_ELEMENT_METADATA_KLASS);
-
-        GST_DEBUG ("Looking at element '%s' of klass '%s'",
-            GST_ELEMENT_NAME (child), klass);
-
-        categories = g_strsplit (klass, "/", 0);
-
-        for (category = 0; categories[category]; category++) {
-          if (g_strcmp0 (categories[category], "Effect") == 0) {
-            guint i, nb_specs;
-
-            class = G_OBJECT_GET_CLASS (child);
-            parray = g_object_class_list_properties (class, &nb_specs);
-            for (i = 0; i < nb_specs; i++) {
-              if (parray[i]->flags & G_PARAM_WRITABLE) {
-                g_hash_table_insert (ret, g_param_spec_ref (parray[i]),
-                    gst_object_ref (child));
-              }
-            }
-            g_free (parray);
-
-            GST_DEBUG
-                ("%d configurable properties of '%s' added to property hashtable",
-                nb_specs, GST_ELEMENT_NAME (child));
-            break;
-          }
-        }
-
-        g_strfreev (categories);
-        g_value_reset (&item);
-        break;
-      }
-      case GST_ITERATOR_RESYNC:
-        /* FIXME, properly restart the process */
-        GST_DEBUG ("iterator resync");
-        gst_iterator_resync (it);
-        break;
-
-      case GST_ITERATOR_DONE:
-        GST_DEBUG ("iterator done");
-        done = TRUE;
-        break;
-
-      default:
-        break;
-    }
-  }
-  g_value_unset (&item);
-  gst_iterator_free (it);
-
-  return ret;
-}

ges/ges-effect.c

@@ -189,6 +189,7 @@ ges_effect_create_element (GESTrackElement * object)
   GError *error = NULL;
   GESEffect *self = GES_EFFECT (object);
   GESTrack *track = ges_track_element_get_track (object);
+  const gchar *wanted_categories[] = { "Effect", NULL };
 
   if (!track) {
     GST_WARNING
@@ -223,6 +224,9 @@ ges_effect_create_element (GESTrackElement * object)
 
   GST_DEBUG ("Created effect %p", effect);
 
+  ges_track_element_add_children_props (object, effect, wanted_categories,
+      NULL, NULL);
+
   return effect;
 }
 

ges/ges-track-element.c

@@ -29,7 +29,7 @@
  * its container, like the start position, the inpoint, the duration and the
  * priority.
  */
-
+#include "ges-utils.h"
 #include "ges-internal.h"
 #include "ges-extractable.h"
 #include "ges-track-element.h"
@@ -57,7 +57,7 @@ struct _GESTrackElementPrivate
   /* We keep a link between properties name and elements internally
    * The hashtable should look like
    * {GParamaSpec ---> element,}*/
-  GHashTable *properties_hashtable;
+  GHashTable *children_props;
 
   GESTrack *track;
 
@@ -162,11 +162,10 @@ ges_track_element_set_property (GObject * object, guint property_id,
 static void
 ges_track_element_dispose (GObject * object)
 {
-  GESTrackElementPrivate *priv = GES_TRACK_ELEMENT (object)->priv;
+  GESTrackElement *element = GES_TRACK_ELEMENT (object);
-
+  GESTrackElementPrivate *priv = element->priv;
-  if (priv->properties_hashtable)
-    g_hash_table_destroy (priv->properties_hashtable);
 
+  g_hash_table_destroy (priv->children_props);
   if (priv->bindings_hashtable)
     g_hash_table_destroy (priv->bindings_hashtable);
 
@@ -262,8 +261,6 @@ ges_track_element_class_init (GESTrackElementClass * klass)
   element_class->deep_copy = ges_track_element_copy_properties;
 
   klass->create_gnl_object = ges_track_element_create_gnl_object_func;
-  /*  There is no 'get_props_hashtable' default implementation */
-  klass->get_props_hastable = NULL;
   klass->list_children_properties = default_list_children_properties;
 }
 
@@ -279,9 +276,12 @@ ges_track_element_init (GESTrackElement * self)
   priv->pending_duration = GST_SECOND;
   priv->pending_priority = MIN_GNL_PRIO;
   priv->pending_active = TRUE;
-  priv->properties_hashtable = NULL;
   priv->bindings_hashtable = g_hash_table_new_full (g_str_hash, g_str_equal,
       g_free, NULL);
+  priv->children_props =
+      g_hash_table_new_full ((GHashFunc) pspec_hash, pspec_equal,
+      (GDestroyNotify) g_param_spec_unref, gst_object_unref);
+
 }
 
 static gfloat
@@ -554,14 +554,8 @@ connect_signal (gpointer key, gpointer value, gpointer user_data)
 static void
 connect_properties_signals (GESTrackElement * object)
 {
-  if (G_UNLIKELY (!object->priv->properties_hashtable)) {
+  g_hash_table_foreach (object->priv->children_props,
-    GST_WARNING ("The properties_hashtable hasn't been set");
-    return;
-  }
-
-  g_hash_table_foreach (object->priv->properties_hashtable,
       (GHFunc) connect_signal, object);
-
 }
 
 /* default 'create_gnl_object' virtual method implementation */
@@ -649,7 +643,6 @@ ensure_gnl_object (GESTrackElement * object)
 {
   GESTrackElementClass *class;
   GstElement *gnlobject;
-  GHashTable *props_hash;
   gboolean res = TRUE;
 
   if (object->priv->gnlobject && object->priv->valid)
@@ -705,18 +698,6 @@ ensure_gnl_object (GESTrackElement * object)
       g_object_set (object->priv->gnlobject,
           "caps", ges_track_get_caps (object->priv->track), NULL);
 
-    /*  We feed up the props_hashtable if possible */
-    if (class->get_props_hastable) {
-      props_hash = class->get_props_hastable (object);
-
-      if (props_hash == NULL) {
-        GST_DEBUG ("'get_props_hastable' implementation returned TRUE but no"
-            "properties_hashtable is available");
-      } else {
-        object->priv->properties_hashtable = props_hash;
-        connect_properties_signals (object);
-      }
-    }
   }
 
 done:
@@ -727,6 +708,157 @@ done:
   return res;
 }
 
+static gboolean
+strv_find_str (const gchar ** strv, const char *str)
+{
+  guint i;
+
+  if (strv == NULL)
+    return FALSE;
+
+  for (i = 0; strv[i]; i++) {
+    if (g_strcmp0 (strv[i], str) == 0)
+      return TRUE;
+  }
+
+  return FALSE;
+}
+
+/**
+ * ges_track_element_add_children_props:
+ * @self: The #GESTrackElement to set chidlren props on
+ * @element: The GstElement to retrieve properties from
+ * @wanted_categories: (array zero-terminated=1) (transfer none) (allow-none):
+ * An array of categories of GstElement to
+ * take into account (as defined in the factory meta "klass" field)
+ * @blacklist: (array zero-terminated=1) (transfer none) (allow-none): A
+ * blacklist of elements factory names to not take into account
+ * @witelist: (array zero-terminated=1) (transfer none) (allow-none): A list
+ * of propery names to add as children properties
+ *
+ * Looks for the properties defines with the various parametters and add
+ * them to the hashtable of children properties.
+ *
+ * To be used by subclasses only
+ */
+void
+ges_track_element_add_children_props (GESTrackElement * self,
+    GstElement * element, const gchar ** wanted_categories,
+    const gchar ** blacklist, const gchar ** whitelist)
+{
+  GValue item = { 0, };
+  GstIterator *it;
+  GParamSpec **parray;
+  GObjectClass *class;
+  const gchar *klass;
+  GstElementFactory *factory;
+  gboolean done = FALSE;
+
+  if (!GST_IS_BIN (element)) {
+    guint i;
+    GParamSpec *pspec;
+
+    GObjectClass *class = G_OBJECT_GET_CLASS (element);
+
+    for (i = 0; whitelist[i]; i++) {
+
+      pspec = g_object_class_find_property (class, whitelist[i]);
+      if (!pspec) {
+        GST_WARNING ("no such property : %s in element : %s", whitelist[i],
+            gst_element_get_name (element));
+        continue;
+      }
+
+      if (pspec->flags & G_PARAM_WRITABLE) {
+        g_hash_table_insert (self->priv->children_props,
+            g_param_spec_ref (pspec), gst_object_ref (element));
+        GST_LOG_OBJECT (self,
+            "added property %s to controllable properties successfully !",
+            whitelist[i]);
+      } else
+        GST_WARNING
+            ("the property %s for element %s exists but is not writable",
+            whitelist[i], gst_element_get_name (element));
+
+    }
+
+    connect_properties_signals (self);
+    return;
+  }
+
+  /*  We go over child elements recursivly, and add writable properties to the
+   *  hashtable */
+  it = gst_bin_iterate_recurse (GST_BIN (element));
+  while (!done) {
+    switch (gst_iterator_next (it, &item)) {
+      case GST_ITERATOR_OK:
+      {
+        gchar **categories;
+        guint i;
+        GstElement *child = g_value_get_object (&item);
+
+        factory = gst_element_get_factory (child);
+        klass = gst_element_factory_get_metadata (factory,
+            GST_ELEMENT_METADATA_KLASS);
+
+        if (strv_find_str (blacklist, GST_OBJECT_NAME (factory))) {
+          GST_DEBUG_OBJECT (self, "%s blacklisted", GST_OBJECT_NAME (factory));
+          continue;
+        }
+
+        GST_DEBUG ("Looking at element '%s' of klass '%s'",
+            GST_ELEMENT_NAME (child), klass);
+
+        categories = g_strsplit (klass, "/", 0);
+
+        for (i = 0; categories[i]; i++) {
+          if ((!wanted_categories ||
+                  strv_find_str (wanted_categories, categories[i]))) {
+            guint i, nb_specs;
+
+            class = G_OBJECT_GET_CLASS (child);
+            parray = g_object_class_list_properties (class, &nb_specs);
+            for (i = 0; i < nb_specs; i++) {
+              if ((parray[i]->flags & G_PARAM_WRITABLE) &&
+                  (!whitelist || strv_find_str (whitelist, parray[i]->name))) {
+                g_hash_table_insert (self->priv->children_props,
+                    g_param_spec_ref (parray[i]), gst_object_ref (child));
+              }
+            }
+            g_free (parray);
+
+            GST_DEBUG
+                ("%d configurable properties of '%s' added to property hashtable",
+                nb_specs, GST_ELEMENT_NAME (child));
+            break;
+          }
+        }
+
+        g_strfreev (categories);
+        g_value_reset (&item);
+        break;
+      }
+      case GST_ITERATOR_RESYNC:
+        /* FIXME, properly restart the process */
+        GST_DEBUG ("iterator resync");
+        gst_iterator_resync (it);
+        break;
+
+      case GST_ITERATOR_DONE:
+        GST_DEBUG ("iterator done");
+        done = TRUE;
+        break;
+
+      default:
+        break;
+    }
+    g_value_unset (&item);
+  }
+  gst_iterator_free (it);
+
+  connect_properties_signals (self);
+}
+
 /* INTERNAL USAGE */
 static void
 _free_pending_binding (PendingBinding * pend)
@@ -894,15 +1026,9 @@ ges_track_element_lookup_child (GESTrackElement * object,
   gpointer key, value;
   gchar **names, *name, *classename;
   gboolean res;
-  GESTrackElementPrivate *priv;
 
   g_return_val_if_fail (GES_IS_TRACK_ELEMENT (object), FALSE);
 
-  priv = object->priv;
-
-  if (!priv->properties_hashtable)
-    goto prop_hash_not_set;
-
   classename = NULL;
   res = FALSE;
 
@@ -913,7 +1039,7 @@ ges_track_element_lookup_child (GESTrackElement * object,
   } else
     name = names[0];
 
-  g_hash_table_iter_init (&iter, priv->properties_hashtable);
+  g_hash_table_iter_init (&iter, object->priv->children_props);
   while (g_hash_table_iter_next (&iter, &key, &value)) {
     if (g_strcmp0 (G_PARAM_SPEC (key)->name, name) == 0) {
       if (classename == NULL ||
@@ -931,12 +1057,6 @@ ges_track_element_lookup_child (GESTrackElement * object,
   g_strfreev (names);
 
   return res;
-
-prop_hash_not_set:
-  {
-    GST_WARNING_OBJECT (object, "The child properties haven't been set yet");
-    return FALSE;
-  }
 }
 
 /**
@@ -954,16 +1074,10 @@ ges_track_element_set_child_property_by_pspec (GESTrackElement * object,
     GParamSpec * pspec, GValue * value)
 {
   GstElement *element;
-  GESTrackElementPrivate *priv;
-
   g_return_if_fail (GES_IS_TRACK_ELEMENT (object));
 
-  priv = object->priv;
 
-  if (!priv->properties_hashtable)
+  element = g_hash_table_lookup (object->priv->children_props, pspec);
-    goto prop_hash_not_set;
-
-  element = g_hash_table_lookup (priv->properties_hashtable, pspec);
   if (!element)
     goto not_found;
 
@@ -976,11 +1090,6 @@ not_found:
     GST_ERROR ("The %s property doesn't exist", pspec->name);
     return;
   }
-prop_hash_not_set:
-  {
-    GST_DEBUG ("The child properties haven't been set on %p", object);
-    return;
-  }
 }
 
 /**
@@ -1207,16 +1316,10 @@ ges_track_element_get_child_property_by_pspec (GESTrackElement * object,
     GParamSpec * pspec, GValue * value)
 {
   GstElement *element;
-  GESTrackElementPrivate *priv;
 
   g_return_if_fail (GES_IS_TRACK_ELEMENT (object));
 
-  priv = object->priv;
+  element = g_hash_table_lookup (object->priv->children_props, pspec);
-
-  if (!priv->properties_hashtable)
-    goto prop_hash_not_set;
-
-  element = g_hash_table_lookup (priv->properties_hashtable, pspec);
   if (!element)
     goto not_found;
 
@@ -1229,11 +1332,6 @@ not_found:
     GST_ERROR ("The %s property doesn't exist", pspec->name);
     return;
   }
-prop_hash_not_set:
-  {
-    GST_ERROR ("The child properties haven't been set on %p", object);
-    return;
-  }
 }
 
 /**
@@ -1336,13 +1434,10 @@ default_list_children_properties (GESTrackElement * object,
 
   guint i = 0;
 
-  if (!object->priv->properties_hashtable)
+  *n_properties = g_hash_table_size (object->priv->children_props);
-    goto prop_hash_not_set;
-
-  *n_properties = g_hash_table_size (object->priv->properties_hashtable);
   pspec = g_new (GParamSpec *, *n_properties);
 
-  g_hash_table_iter_init (&iter, object->priv->properties_hashtable);
+  g_hash_table_iter_init (&iter, object->priv->children_props);
   while (g_hash_table_iter_next (&iter, &key, &value)) {
     spec = G_PARAM_SPEC (key);
     pspec[i] = g_param_spec_ref (spec);
@@ -1350,13 +1445,6 @@ default_list_children_properties (GESTrackElement * object,
   }
 
   return pspec;
-
-prop_hash_not_set:
-  {
-    *n_properties = 0;
-    GST_DEBUG_OBJECT (object, "No child properties have been set yet");
-    return NULL;
-  }
 }
 
 void

ges/ges-track-element.h

@@ -75,8 +75,6 @@ struct _GESTrackElement {
  * @create_gnl_object: method to create the GNonLin container object.
  * @create_element: method to return the GstElement to put in the gnlobject.
  * @active_changed: active property of gnlobject has changed
- * @get_props_hastable: method to list children properties that user could like
- *                      to configure. Since: 0.10.2
  * @list_children_properties: method to get children properties that user could
  *                            like to configure.
  *                            The default implementation will create an object
@@ -104,7 +102,6 @@ struct _GESTrackElementClass {
 
   /*< public >*/
   /* virtual methods for subclasses */
-  GHashTable*  (*get_props_hastable)       (GESTrackElement * object);
   GParamSpec** (*list_children_properties) (GESTrackElement * object,
               guint *n_properties);
   /*< private >*/
@@ -188,6 +185,11 @@ ges_track_element_set_control_source          (GESTrackElement *object,
 GstControlBinding *
 ges_track_element_get_control_binding         (GESTrackElement *object,
                                                const gchar *property_name);
-
+void
+ges_track_element_add_children_props          (GESTrackElement *self,
+                                               GstElement *element,
+                                               const gchar ** wanted_categories,
+                                               const gchar **blacklist,
+                                               const gchar **whitelist);
 G_END_DECLS
 #endif /* _GES_TRACK_ELEMENT */

ges/ges-uri-source.c

@@ -37,7 +37,6 @@
 
 struct _GESUriSourcePrivate
 {
-  GHashTable *props_hashtable;
   GstFramePositionner *positionner;
 };
 
@@ -119,46 +118,6 @@ _create_bin (const gchar * bin_name, GstElement * decodebin, ...)
   return bin;
 }
 
-static void
-_add_element_properties_to_hashtable (GESUriSource * self, GstElement * element,
-    ...)
-{
-  GObjectClass *class;
-  GParamSpec *pspec;
-  va_list argp;
-  const gchar *propname;
-
-  class = G_OBJECT_GET_CLASS (element);
-  va_start (argp, element);
-
-  while ((propname = va_arg (argp, const gchar *)) != NULL)
-  {
-    pspec = g_object_class_find_property (class, propname);
-    if (!pspec) {
-      GST_WARNING ("no such property : %s in element : %s", propname,
-          gst_element_get_name (element));
-      continue;
-    }
-
-    if (self->priv->props_hashtable == NULL)
-      self->priv->props_hashtable =
-          g_hash_table_new_full ((GHashFunc) pspec_hash, pspec_equal,
-          (GDestroyNotify) g_param_spec_unref, gst_object_unref);
-
-    if (pspec->flags & G_PARAM_WRITABLE) {
-      g_hash_table_insert (self->priv->props_hashtable,
-          g_param_spec_ref (pspec), gst_object_ref (element));
-      GST_LOG_OBJECT (self,
-          "added property %s to controllable properties successfully !",
-          propname);
-    } else
-      GST_WARNING ("the property %s for element %s exists but is not writable",
-          propname, gst_element_get_name (element));
-  }
-
-  va_end (argp);
-}
-
 static void
 _sync_element_to_layer_property_float (GESTrackElement * trksrc,
     GstElement * element, const gchar * meta, const gchar * propname)
@@ -220,6 +179,9 @@ ges_uri_source_create_element (GESTrackElement * trksrc)
 
   switch (track->type) {
     case GES_TRACK_TYPE_AUDIO:
+    {
+      const gchar *props[] = { "volume", "mute", NULL };
+
       GST_DEBUG_OBJECT (trksrc, "Creating a bin uridecodebin ! volume");
 
       decodebin = gst_element_factory_make ("uridecodebin", NULL);
@@ -229,18 +191,22 @@ ges_uri_source_create_element (GESTrackElement * trksrc)
 
       _sync_element_to_layer_property_float (trksrc, volume, GES_META_VOLUME,
           "volume");
-      _add_element_properties_to_hashtable (self, volume, "volume", "mute",
+      ges_track_element_add_children_props (trksrc, volume, NULL, NULL, props);
-          NULL);
       break;
+    }
     case GES_TRACK_TYPE_VIDEO:
+    {
+      const gchar *props[] = { "alpha", "posx", "posy", NULL };
+
       decodebin = gst_element_factory_make ("uridecodebin", NULL);
 
       /* That positionner will add metadata to buffers according to its
          properties, acting like a proxy for our smart-mixer dynamic pads. */
       positionner =
           gst_element_factory_make ("framepositionner", "frame_tagger");
-      _add_element_properties_to_hashtable (self, positionner, "alpha", "posx",
+
-          "posy", NULL);
+      ges_track_element_add_children_props (trksrc, positionner, NULL, NULL,
+          props);
       topbin = _create_bin ("video-src-bin", decodebin, positionner, NULL);
       parent = ges_timeline_element_get_parent (GES_TIMELINE_ELEMENT (trksrc));
       if (parent) {
@@ -253,6 +219,7 @@ ges_uri_source_create_element (GESTrackElement * trksrc)
         GST_WARNING ("No parent timeline element, SHOULD NOT HAPPEN");
       }
       break;
+    }
     default:
       decodebin = gst_element_factory_make ("uridecodebin", NULL);
       topbin = _create_bin ("video-src-bin", decodebin, NULL);
@@ -265,19 +232,6 @@ ges_uri_source_create_element (GESTrackElement * trksrc)
   return topbin;
 }
 
-static GHashTable *
-ges_uri_source_get_props_hashtable (GESTrackElement * element)
-{
-  GESUriSource *self = (GESUriSource *) element;
-
-  if (self->priv->props_hashtable == NULL)
-    self->priv->props_hashtable =
-        g_hash_table_new_full ((GHashFunc) pspec_hash, pspec_equal,
-        (GDestroyNotify) g_param_spec_unref, gst_object_unref);
-
-  return self->priv->props_hashtable;
-}
-
 /* Extractable interface implementation */
 
 static gchar *
@@ -383,7 +337,6 @@ ges_track_filesource_class_init (GESUriSourceClass * klass)
           NULL, G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY));
 
   track_class->create_element = ges_uri_source_create_element;
-  track_class->get_props_hastable = ges_uri_source_get_props_hashtable;
 }
 
 static void
@@ -391,7 +344,6 @@ ges_track_filesource_init (GESUriSource * self)
 {
   self->priv = G_TYPE_INSTANCE_GET_PRIVATE (self,
       GES_TYPE_URI_SOURCE, GESUriSourcePrivate);
-  self->priv->props_hashtable = NULL;
   self->priv->positionner = NULL;
 }