From 54b10dcbbf4413d5661cbef6b4dd025306b01a33 Mon Sep 17 00:00:00 2001 From: Thibault Saunier Date: Mon, 26 Aug 2013 23:31:14 -0400 Subject: [PATCH] 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 | 1 + ges/ges-base-effect.c | 98 --------------- ges/ges-effect.c | 4 + ges/ges-track-element.c | 244 +++++++++++++++++++++++++------------ ges/ges-track-element.h | 10 +- ges/ges-uri-source.c | 72 ++--------- 6 files changed, 189 insertions(+), 240 deletions(-) diff --git a/docs/libs/ges-sections.txt b/docs/libs/ges-sections.txt index b5b2da2e59..5c70451946 100644 --- a/docs/libs/ges-sections.txt +++ b/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 GESTrackElementAssetPrivate GES_TRACK_ELEMENT_ASSET diff --git a/ges/ges-base-effect.c b/ges/ges-base-effect.c index 4760b910c6..0ae0592d0b 100644 --- a/ges/ges-base-effect.c +++ b/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; -} diff --git a/ges/ges-effect.c b/ges/ges-effect.c index b9dc3c4667..0c22fc5890 100644 --- a/ges/ges-effect.c +++ b/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; } diff --git a/ges/ges-track-element.c b/ges/ges-track-element.c index e988337f5a..cf052e6749 100644 --- a/ges/ges-track-element.c +++ b/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; - - if (priv->properties_hashtable) - g_hash_table_destroy (priv->properties_hashtable); + GESTrackElement *element = GES_TRACK_ELEMENT (object); + GESTrackElementPrivate *priv = element->priv; + 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)) { - GST_WARNING ("The properties_hashtable hasn't been set"); - return; - } - - g_hash_table_foreach (object->priv->properties_hashtable, + g_hash_table_foreach (object->priv->children_props, (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) - goto prop_hash_not_set; - - element = g_hash_table_lookup (priv->properties_hashtable, pspec); + element = g_hash_table_lookup (object->priv->children_props, 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; - - if (!priv->properties_hashtable) - goto prop_hash_not_set; - - element = g_hash_table_lookup (priv->properties_hashtable, pspec); + element = g_hash_table_lookup (object->priv->children_props, 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) - goto prop_hash_not_set; - - *n_properties = g_hash_table_size (object->priv->properties_hashtable); + *n_properties = g_hash_table_size (object->priv->children_props); 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 diff --git a/ges/ges-track-element.h b/ges/ges-track-element.h index 19149d70e5..daeae276a3 100644 --- a/ges/ges-track-element.h +++ b/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 */ diff --git a/ges/ges-uri-source.c b/ges/ges-uri-source.c index 94fd8fbb85..a339681f99 100644 --- a/ges/ges-uri-source.c +++ b/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", - NULL); + ges_track_element_add_children_props (trksrc, volume, NULL, NULL, props); 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; }