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.

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)
+        v = GObject.Value()
-        return res
+        v.init(pspec.value_type)
+        v.set_value(prop_value)
+
+        return GES.TimelineElement.set_child_property(self, prop_name, v)
 
 
 TimelineElement = override(TimelineElement)

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),
+      ges_timeline_element_add_child_property_full (GES_TIMELINE_ELEMENT
-          child_props[i], prop_child);
+          (container), child, child_props[i], prop_child);
       gst_object_unref (prop_child);
 
     }

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))))

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;
+  return ges_timeline_element_add_child_property_full (self, NULL, pspec,
-  ChildPropHandler *handler;
+      child);
-
-  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;
 }
 
 /**
@@ -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);
+  set_child_property_by_pspec (self, pspec, value);
-
-  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;
-  }
 }
 
 /**
@@ -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);
+  return set_child_property_by_pspec (self, pspec, value);
-  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;
 
 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;