ges: Call the right ->set_child_property vmethod

We used to always call the `->set_child_property` virtual method
of the object that `ges_timeline_element_set_child_property` was called
from, but that means that, in the case of referencing GESContainer
children properties from its children, the children wouldn't know
what child property have been set, and the children override wouldn't
be takent into account, in turns, it means that the behaviour could be
different in the setter depending on parent the method was called,
which is totally unexpected.

We now make sure that the vmethod from the element that introduced the
child property is called whatever parent method is called, making the
behaviour more uniform.

Fix the python override to make sure that new behaviour is respected.

diff --git a/bindings/python/gi/overrides/GES.py b/bindings/python/gi/overrides/GES.py
index fa09f7c..1bd170e 100644 (file)
--- a/bindings/python/gi/overrides/GES.py
+++ b/bindings/python/gi/overrides/GES.py
@@ -27,6 +27,7 @@
 import sys
 from ..overrides import override
 from ..importer import modules
+from gi.repository import GObject
 
 
 if sys.version_info >= (3, 0):
@@ -60,12 +61,15 @@ class TimelineElement(GES.TimelineElement):
         )
 
     def set_child_property(self, prop_name, prop_value):
-        res, child, unused_pspec = GES.TimelineElement.lookup_child(self, prop_name)
+        res, _, pspec = GES.TimelineElement.lookup_child(self, prop_name)
         if not res:
             return res
 
-        child.set_property(prop_name, prop_value)
-        return res
+        v = GObject.Value()
+        v.init(pspec.value_type)
+        v.set_value(prop_value)
+
+        return GES.TimelineElement.set_child_property(self, prop_name, v)
 
 
 TimelineElement = override(TimelineElement)

diff --git a/ges/ges-container.c b/ges/ges-container.c
index bf7e8c7..81e54d3 100644 (file)
--- a/ges/ges-container.c
+++ b/ges/ges-container.c
@@ -220,8 +220,8 @@ _ges_container_add_child_properties (GESContainer * container,
         child_props[i]->name);
 
     if (ges_timeline_element_lookup_child (child, prop_name, &prop_child, NULL)) {
-      ges_timeline_element_add_child_property (GES_TIMELINE_ELEMENT (container),
-          child_props[i], prop_child);
+      ges_timeline_element_add_child_property_full (GES_TIMELINE_ELEMENT
+          (container), child, child_props[i], prop_child);
       gst_object_unref (prop_child);
 
     }

diff --git a/ges/ges-internal.h b/ges/ges-internal.h
index 0141bc2..8afde95 100644 (file)
--- a/ges/ges-internal.h
+++ b/ges/ges-internal.h
@@ -437,6 +437,10 @@ G_GNUC_INTERNAL gdouble ges_timeline_element_get_media_duration_factor(GESTimeli
 G_GNUC_INTERNAL GESTimelineElement * ges_timeline_element_get_copied_from (GESTimelineElement *self);
 G_GNUC_INTERNAL GESTimelineElementFlags ges_timeline_element_flags (GESTimelineElement *self);
 G_GNUC_INTERNAL void                ges_timeline_element_set_flags (GESTimelineElement *self, GESTimelineElementFlags flags);
+G_GNUC_INTERNAL gboolean             ges_timeline_element_add_child_property_full (GESTimelineElement *self,
+                                                                                   GESTimelineElement *owner,
+                                                                                   GParamSpec *pspec,
+                                                                                   GObject *child);
 
 #define ELEMENT_FLAGS(obj)             (ges_timeline_element_flags (GES_TIMELINE_ELEMENT(obj)))
 #define ELEMENT_SET_FLAG(obj,flag)     (ges_timeline_element_set_flags(GES_TIMELINE_ELEMENT(obj), (ELEMENT_FLAGS(obj) | (flag))))

diff --git a/ges/ges-timeline-element.c b/ges/ges-timeline-element.c
index 21ff79f..30c7bdd 100644 (file)
--- a/ges/ges-timeline-element.c
+++ b/ges/ges-timeline-element.c
@@ -83,6 +83,7 @@ static GParamSpec *properties[PROP_LAST] = { NULL, };
 typedef struct
 {
   GObject *child;
+  GESTimelineElement *owner;
   gulong handler_id;
 } ChildPropHandler;
 
@@ -593,6 +594,99 @@ ges_timeline_element_set_flags (GESTimelineElement * self,
 
 }
 
+static gboolean
+emit_deep_notify_in_idle (EmitDeepNotifyInIdleData * data)
+{
+  g_signal_emit (data->self, ges_timeline_element_signals[DEEP_NOTIFY], 0,
+      data->child, data->arg);
+
+  gst_object_unref (data->child);
+  g_param_spec_unref (data->arg);
+  gst_object_unref (data->self);
+  g_slice_free (EmitDeepNotifyInIdleData, data);
+
+  return FALSE;
+}
+
+static void
+child_prop_changed_cb (GObject * child, GParamSpec * arg
+    G_GNUC_UNUSED, GESTimelineElement * self)
+{
+  EmitDeepNotifyInIdleData *data;
+
+  /* Emit "deep-notify" right away if in main thread */
+  if (g_main_context_acquire (g_main_context_default ())) {
+    g_main_context_release (g_main_context_default ());
+    g_signal_emit (self, ges_timeline_element_signals[DEEP_NOTIFY], 0,
+        child, arg);
+    return;
+  }
+
+  data = g_slice_new (EmitDeepNotifyInIdleData);
+
+  data->child = gst_object_ref (child);
+  data->arg = g_param_spec_ref (arg);
+  data->self = gst_object_ref (self);
+
+  ges_idle_add ((GSourceFunc) emit_deep_notify_in_idle, data, NULL);
+}
+
+static gboolean
+set_child_property_by_pspec (GESTimelineElement * self,
+    GParamSpec * pspec, const GValue * value)
+{
+  GESTimelineElementClass *klass;
+  GESTimelineElement *setter = self;
+  ChildPropHandler *handler =
+      g_hash_table_lookup (self->priv->children_props, pspec);
+
+  if (!handler) {
+    GST_ERROR_OBJECT (self, "The %s property doesn't exist", pspec->name);
+    return FALSE;
+  }
+
+  if (handler->owner) {
+    klass = GES_TIMELINE_ELEMENT_GET_CLASS (handler->owner);
+    setter = handler->owner;
+  } else {
+    klass = GES_TIMELINE_ELEMENT_GET_CLASS (self);
+  }
+
+  g_assert (klass->set_child_property);
+  klass->set_child_property (setter, handler->child, pspec, (GValue *) value);
+
+  return TRUE;
+}
+
+gboolean
+ges_timeline_element_add_child_property_full (GESTimelineElement * self,
+    GESTimelineElement * owner, GParamSpec * pspec, GObject * child)
+{
+  gchar *signame;
+  ChildPropHandler *handler;
+
+  if (g_hash_table_contains (self->priv->children_props, pspec)) {
+    GST_INFO_OBJECT (self, "Child property already exists: %s", pspec->name);
+    return FALSE;
+  }
+
+  GST_DEBUG_OBJECT (self, "Adding child property: %" GST_PTR_FORMAT "::%s",
+      child, pspec->name);
+
+  signame = g_strconcat ("notify::", pspec->name, NULL);
+  handler = (ChildPropHandler *) g_slice_new0 (ChildPropHandler);
+  handler->child = gst_object_ref (child);
+  handler->owner = owner;
+  handler->handler_id =
+      g_signal_connect (child, signame, G_CALLBACK (child_prop_changed_cb),
+      self);
+  g_hash_table_insert (self->priv->children_props, g_param_spec_ref (pspec),
+      handler);
+
+  g_free (signame);
+  return TRUE;
+}
+
 /*********************************************
  *            API implementation             *
  *********************************************/
@@ -1390,70 +1484,12 @@ had_timeline:
   }
 }
 
-static gboolean
-emit_deep_notify_in_idle (EmitDeepNotifyInIdleData * data)
-{
-  g_signal_emit (data->self, ges_timeline_element_signals[DEEP_NOTIFY], 0,
-      data->child, data->arg);
-
-  gst_object_unref (data->child);
-  g_param_spec_unref (data->arg);
-  gst_object_unref (data->self);
-  g_slice_free (EmitDeepNotifyInIdleData, data);
-
-  return FALSE;
-}
-
-static void
-child_prop_changed_cb (GObject * child, GParamSpec * arg
-    G_GNUC_UNUSED, GESTimelineElement * self)
-{
-  EmitDeepNotifyInIdleData *data;
-
-  /* Emit "deep-notify" right away if in main thread */
-  if (g_main_context_acquire (g_main_context_default ())) {
-    g_main_context_release (g_main_context_default ());
-    g_signal_emit (self, ges_timeline_element_signals[DEEP_NOTIFY], 0,
-        child, arg);
-    return;
-  }
-
-  data = g_slice_new (EmitDeepNotifyInIdleData);
-
-  data->child = gst_object_ref (child);
-  data->arg = g_param_spec_ref (arg);
-  data->self = gst_object_ref (self);
-
-  ges_idle_add ((GSourceFunc) emit_deep_notify_in_idle, data, NULL);
-}
-
 gboolean
 ges_timeline_element_add_child_property (GESTimelineElement * self,
     GParamSpec * pspec, GObject * child)
 {
-  gchar *signame;
-  ChildPropHandler *handler;
-
-  if (g_hash_table_contains (self->priv->children_props, pspec)) {
-    GST_INFO_OBJECT (self, "Child property already exists: %s", pspec->name);
-    return FALSE;
-  }
-
-  GST_DEBUG_OBJECT (self, "Adding child property: %" GST_PTR_FORMAT "::%s",
-      child, pspec->name);
-
-  signame = g_strconcat ("notify::", pspec->name, NULL);
-  handler = (ChildPropHandler *) g_slice_new0 (ChildPropHandler);
-  handler->child = gst_object_ref (child);
-  handler->handler_id =
-      g_signal_connect (child, signame, G_CALLBACK (child_prop_changed_cb),
-      self);
-  g_hash_table_insert (self->priv->children_props, g_param_spec_ref (pspec),
-      handler);
-
-  g_free (signame);
-
-  return TRUE;
+  return ges_timeline_element_add_child_property_full (self, NULL, pspec,
+      child);
 }
 
 /**
@@ -1499,27 +1535,9 @@ void
 ges_timeline_element_set_child_property_by_pspec (GESTimelineElement * self,
     GParamSpec * pspec, const GValue * value)
 {
-  ChildPropHandler *handler;
-  GESTimelineElementClass *klass;
-
   g_return_if_fail (GES_IS_TRACK_ELEMENT (self));
 
-  handler = g_hash_table_lookup (self->priv->children_props, pspec);
-
-  if (!handler)
-    goto not_found;
-
-  klass = GES_TIMELINE_ELEMENT_GET_CLASS (self);
-  g_assert (klass->set_child_property);
-  klass->set_child_property (self, handler->child, pspec, (GValue *) value);
-
-  return;
-
-not_found:
-  {
-    GST_ERROR ("The %s property doesn't exist", pspec->name);
-    return;
-  }
+  set_child_property_by_pspec (self, pspec, value);
 }
 
 /**
@@ -1541,7 +1559,6 @@ ges_timeline_element_set_child_property (GESTimelineElement * self,
     const gchar * property_name, const GValue * value)
 {
   GParamSpec *pspec;
-  GESTimelineElementClass *klass;
   GObject *child;
 
   g_return_val_if_fail (GES_IS_TIMELINE_ELEMENT (self), FALSE);
@@ -1549,14 +1566,7 @@ ges_timeline_element_set_child_property (GESTimelineElement * self,
   if (!ges_timeline_element_lookup_child (self, property_name, &child, &pspec))
     goto not_found;
 
-  klass = GES_TIMELINE_ELEMENT_GET_CLASS (self);
-  g_assert (klass->set_child_property);
-  klass->set_child_property (self, child, pspec, (GValue *) value);
-
-  gst_object_unref (child);
-  g_param_spec_unref (pspec);
-
-  return TRUE;
+  return set_child_property_by_pspec (self, pspec, value);
 
 not_found:
   {
@@ -1667,7 +1677,6 @@ ges_timeline_element_set_child_property_valist (GESTimelineElement * self,
 {
   const gchar *name;
   GParamSpec *pspec;
-  GObject *child;
 
   gchar *error = NULL;
   GValue value = { 0, };
@@ -1681,7 +1690,7 @@ ges_timeline_element_set_child_property_valist (GESTimelineElement * self,
 
   /* iterate over pairs */
   while (name) {
-    if (!ges_timeline_element_lookup_child (self, name, &child, &pspec))
+    if (!ges_timeline_element_lookup_child (self, name, NULL, &pspec))
       goto not_found;
 
     G_VALUE_COLLECT_INIT (&value, pspec->value_type, var_args,
@@ -1690,9 +1699,8 @@ ges_timeline_element_set_child_property_valist (GESTimelineElement * self,
     if (error)
       goto cant_copy;
 
-    g_object_set_property (child, pspec->name, &value);
+    set_child_property_by_pspec (self, pspec, &value);
 
-    gst_object_unref (child);
     g_param_spec_unref (pspec);
     g_value_unset (&value);
 
@@ -1710,7 +1718,6 @@ cant_copy:
     GST_WARNING_OBJECT (self, "error copying value %s in %p: %s", pspec->name,
         self, error);
 
-    gst_object_unref (child);
     g_param_spec_unref (pspec);
     g_value_unset (&value);
     return;