group: Make deep copying actually copy deep

Allowing pasting groups paste exactly what had been copied

And not the new version of the contained objects

This technically breaks the C API but this is a new API and I believe
and hope nobody is using it right now.

Reviewed-by: Thibault Saunier <thibault.saunier@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D616

ges/ges-audio-source.c

@@ -71,6 +71,12 @@ _sync_element_to_layer_property_float (GESTrackElement * trksrc,
   gfloat value;
 
   parent = ges_timeline_element_get_parent (GES_TIMELINE_ELEMENT (trksrc));
+  if (!parent) {
+      GST_DEBUG_OBJECT (trksrc, "Not in a clip... doing nothing");
+
+      return;
+  }
+
   layer = ges_clip_get_layer (GES_CLIP (parent));
 
   gst_object_unref (parent);
@@ -81,12 +87,12 @@ _sync_element_to_layer_property_float (GESTrackElement * trksrc,
     g_object_set (element, propname, value, NULL);
     GST_DEBUG_OBJECT (trksrc, "Setting %s to %f", propname, value);
 
+
+    gst_object_unref (layer);
   } else {
 
     GST_DEBUG_OBJECT (trksrc, "NOT setting the %s", propname);
   }
-
-  gst_object_unref (layer);
 }
 
 static void

ges/ges-audio-uri-source.c

@@ -48,6 +48,7 @@ ges_audio_uri_source_create_source (GESTrackElement * trksrc)
   GESAudioUriSource *self;
   GESTrack *track;
   GstElement *decodebin;
+  const GstCaps *caps = NULL;
 
   self = (GESAudioUriSource *) trksrc;
 
@@ -55,7 +56,10 @@ ges_audio_uri_source_create_source (GESTrackElement * trksrc)
 
   decodebin = gst_element_factory_make ("uridecodebin", NULL);
 
-  g_object_set (decodebin, "caps", ges_track_get_caps (track),
+  if (track)
+    caps = ges_track_get_caps (track);
+
+  g_object_set (decodebin, "caps", caps,
       "expose-all-streams", FALSE, "uri", self->uri, NULL);
 
   return decodebin;

ges/ges-clip.c

@@ -61,6 +61,9 @@ struct _GESClipPrivate
 
   guint nb_effects;
 
+  GList *copied_track_elements;
+  GESLayer *copied_layer;
+
   /* The formats supported by this Clip */
   GESTrackType supportedformats;
 };
@@ -625,21 +628,43 @@ _edit (GESContainer * container, GList * layers,
   return ret;
 }
 
-static gboolean
+static void
+_deep_copy (GESTimelineElement * element, GESTimelineElement * copy)
+{
+  GList *tmp;
+  GESClip *self = GES_CLIP (element), *ccopy = GES_CLIP (copy);
+
+  for (tmp = GES_CONTAINER_CHILDREN (element); tmp; tmp = tmp->next) {
+    ccopy->priv->copied_track_elements =
+        g_list_append (ccopy->priv->copied_track_elements,
+        ges_timeline_element_copy (tmp->data, TRUE));
+  }
+
+  if (self->priv->copied_layer)
+    ccopy->priv->copied_layer = g_object_ref (self->priv->copied_layer);
+  else if (self->priv->layer)
+    ccopy->priv->copied_layer = g_object_ref (self->priv->layer);
+}
+
+static GESTimelineElement *
 _paste (GESTimelineElement * element, GESTimelineElement * ref,
     GstClockTime paste_position)
 {
   GList *tmp;
   GESClip *self = GES_CLIP (element);
-  GESClip *refclip = GES_CLIP (ref);
+  GESClip *nclip = GES_CLIP (ges_timeline_element_copy (element, FALSE));
 
-  ges_clip_set_moving_from_layer (self, TRUE);
-  ges_layer_add_clip (refclip->priv->layer, self);
-  ges_clip_set_moving_from_layer (self, FALSE);
+  if (self->priv->copied_layer)
+    nclip->priv->copied_layer = g_object_ref (self->priv->copied_layer);
 
-  ges_timeline_element_set_start (GES_TIMELINE_ELEMENT (self), paste_position);
+  ges_clip_set_moving_from_layer (nclip, TRUE);
+  if (self->priv->copied_layer)
+    ges_layer_add_clip (self->priv->copied_layer, nclip);
+  ges_clip_set_moving_from_layer (nclip, FALSE);
 
-  for (tmp = GES_CONTAINER_CHILDREN (refclip); tmp; tmp = tmp->next) {
+  ges_timeline_element_set_start (GES_TIMELINE_ELEMENT (nclip), paste_position);
+
+  for (tmp = self->priv->copied_track_elements; tmp; tmp = tmp->next) {
     GESTrackElement *new_trackelement, *trackelement =
         GES_TRACK_ELEMENT (tmp->data);
 
@@ -651,7 +676,7 @@ _paste (GESTimelineElement * element, GESTimelineElement * ref,
       continue;
     }
 
-    ges_container_add (GES_CONTAINER (self),
+    ges_container_add (GES_CONTAINER (nclip),
         GES_TIMELINE_ELEMENT (new_trackelement));
 
     ges_track_element_copy_properties (GES_TIMELINE_ELEMENT (trackelement),
@@ -661,7 +686,7 @@ _paste (GESTimelineElement * element, GESTimelineElement * ref,
         GST_CLOCK_TIME_NONE);
   }
 
-  return TRUE;
+  return GES_TIMELINE_ELEMENT (nclip);
 }
 
 
@@ -703,6 +728,16 @@ ges_clip_set_property (GObject * object, guint property_id,
   }
 }
 
+static void
+ges_clip_finalize (GObject * object)
+{
+  GESClip *self = GES_CLIP (object);
+
+  g_list_free_full (self->priv->copied_track_elements, g_object_unref);
+
+  G_OBJECT_CLASS (ges_clip_parent_class)->finalize (object);
+}
+
 static void
 ges_clip_class_init (GESClipClass * klass)
 {
@@ -714,6 +749,7 @@ ges_clip_class_init (GESClipClass * klass)
 
   object_class->get_property = ges_clip_get_property;
   object_class->set_property = ges_clip_set_property;
+  object_class->finalize = ges_clip_finalize;
   klass->create_track_elements = ges_clip_create_track_elements_func;
   klass->create_track_element = NULL;
 
@@ -754,6 +790,7 @@ ges_clip_class_init (GESClipClass * klass)
   element_class->set_priority = _set_priority;
   element_class->set_max_duration = _set_max_duration;
   element_class->paste = _paste;
+  element_class->deep_copy = _deep_copy;
 
   container_class->add_child = _add_child;
   container_class->remove_child = _remove_child;

ges/ges-container.c

@@ -81,6 +81,8 @@ struct _GESContainerPrivate
   /* List of GESTimelineElement being in the "child-added" signal
    * emission stage */
   GList *adding_children;
+
+  GList *copied_children;
 };
 
 enum
@@ -101,9 +103,12 @@ _free_mapping (ChildMapping * mapping)
   GESTimelineElement *child = mapping->child;
 
   /* Disconnect all notify listeners */
-  g_signal_handler_disconnect (child, mapping->start_notifyid);
-  g_signal_handler_disconnect (child, mapping->duration_notifyid);
-  g_signal_handler_disconnect (child, mapping->inpoint_notifyid);
+  if (mapping->start_notifyid)
+    g_signal_handler_disconnect (child, mapping->start_notifyid);
+  if (mapping->duration_notifyid)
+    g_signal_handler_disconnect (child, mapping->duration_notifyid);
+  if (mapping->inpoint_notifyid)
+    g_signal_handler_disconnect (child, mapping->inpoint_notifyid);
 
   ges_timeline_element_set_parent (child, NULL);
   g_slice_free (ChildMapping, mapping);
@@ -287,28 +292,51 @@ _get_track_types (GESTimelineElement * object)
   return types ^ GES_TRACK_TYPE_UNKNOWN;
 }
 
-static gboolean
+static void
+_deep_copy (GESTimelineElement * element, GESTimelineElement * copy)
+{
+  GList *tmp;
+  GESContainer *self = GES_CONTAINER (element), *ccopy = GES_CONTAINER (copy);
+
+  for (tmp = GES_CONTAINER_CHILDREN (element); tmp; tmp = tmp->next) {
+    ChildMapping *map;
+
+    map =
+        g_slice_dup (ChildMapping, g_hash_table_lookup (self->priv->mappings,
+            tmp->data));
+    map->child = ges_timeline_element_copy (tmp->data, TRUE);
+    map->start_notifyid = 0;
+    map->inpoint_notifyid = 0;
+    map->duration_notifyid = 0;
+
+    ccopy->priv->copied_children = g_list_prepend (ccopy->priv->copied_children,
+        map);
+  }
+}
+
+static GESTimelineElement *
 _paste (GESTimelineElement * element, GESTimelineElement * ref,
     GstClockTime paste_position)
 {
   GList *tmp;
+  ChildMapping *map;
+  GESContainer *ncontainer =
+      GES_CONTAINER (ges_timeline_element_copy (element, FALSE));
   GESContainer *self = GES_CONTAINER (element);
-  GESContainer *refcontainer = GES_CONTAINER (ref);
 
-  for (tmp = GES_CONTAINER_CHILDREN (refcontainer); tmp; tmp = tmp->next) {
-    ChildMapping *map;
-    GESTimelineElement *child, *refchild = GES_TIMELINE_ELEMENT (tmp->data);
+  for (tmp = self->priv->copied_children; tmp; tmp = tmp->next) {
+    GESTimelineElement *nchild;
 
-    map = g_hash_table_lookup (refcontainer->priv->mappings, refchild);
-    child = ges_timeline_element_copy (GES_TIMELINE_ELEMENT (refchild), TRUE);
-
-    ges_timeline_element_paste (child, paste_position + map->start_offset);
-    ges_timeline_element_set_timeline (element,
+    map = tmp->data;
+    nchild =
+        ges_timeline_element_paste (map->child,
+        paste_position - map->start_offset);
+    ges_timeline_element_set_timeline (GES_TIMELINE_ELEMENT (ncontainer),
         GES_TIMELINE_ELEMENT_TIMELINE (ref));
-    ges_container_add (self, child);
+    ges_container_add (ncontainer, nchild);
   }
 
-  return TRUE;
+  return GES_TIMELINE_ELEMENT (ncontainer);
 }
 
 
@@ -327,6 +355,17 @@ _dispose (GObject * object)
   G_OBJECT_CLASS (ges_container_parent_class)->dispose (object);
 }
 
+static void
+_finalize (GObject * object)
+{
+  GESContainer *self = GES_CONTAINER (object);
+
+  g_list_free_full (self->priv->copied_children,
+      (GDestroyNotify) _free_mapping);
+
+  G_OBJECT_CLASS (ges_container_parent_class)->dispose (object);
+}
+
 static void
 _get_property (GObject * container, guint property_id,
     GValue * value, GParamSpec * pspec)
@@ -366,6 +405,7 @@ ges_container_class_init (GESContainerClass * klass)
   object_class->get_property = _get_property;
   object_class->set_property = _set_property;
   object_class->dispose = _dispose;
+  object_class->finalize = _finalize;
 
   /**
    * GESContainer:height:
@@ -415,6 +455,7 @@ ges_container_class_init (GESContainerClass * klass)
   element_class->lookup_child = _lookup_child;
   element_class->get_track_types = _get_track_types;
   element_class->paste = _paste;
+  element_class->deep_copy = _deep_copy;
 
   /* No default implementations */
   klass->remove_child = NULL;

ges/ges-group.c

@@ -598,21 +598,21 @@ _group (GList * containers)
   return ret;
 }
 
-static gboolean
+static GESTimelineElement *
 _paste (GESTimelineElement * element, GESTimelineElement * ref,
     GstClockTime paste_position)
 {
-  if (GES_TIMELINE_ELEMENT_CLASS (parent_class)->paste (element,
-          ref, paste_position)) {
+  GESTimelineElement *ngroup =
+      GES_TIMELINE_ELEMENT_CLASS (parent_class)->paste (element, ref,
+      paste_position);
 
-    if (GES_CONTAINER_CHILDREN (element))
+  if (ngroup) {
+    if (GES_CONTAINER_CHILDREN (ngroup))
       timeline_add_group (GES_TIMELINE_ELEMENT_TIMELINE (GES_CONTAINER_CHILDREN
-              (element)->data), GES_GROUP (element));
-
-    return TRUE;
+              (ngroup)->data), GES_GROUP (element));
   }
 
-  return FALSE;
+  return ngroup;
 }
 
 

ges/ges-timeline-element.c

@@ -1697,25 +1697,27 @@ ges_timeline_element_get_track_types (GESTimelineElement * self)
  * using ges_timeline_element_copy with recurse=TRUE set,
  * otherwise it will fail.
  *
+ * Returns: (transfer none): Paste @self copying the element
+ *
  * Since: 1.6.0
  */
-gboolean
+GESTimelineElement *
 ges_timeline_element_paste (GESTimelineElement * self,
     GstClockTime paste_position)
 {
-  gboolean res;
+  GESTimelineElement *res;
   g_return_val_if_fail (GES_IS_TIMELINE_ELEMENT (self), FALSE);
 
   if (!self->priv->copied_from) {
     GST_ERROR_OBJECT (self, "Is not being 'deeply' copied!");
 
-    return FALSE;
+    return NULL;
   }
 
   if (!GES_TIMELINE_ELEMENT_GET_CLASS (self)->paste) {
     GST_ERROR_OBJECT (self, "No paste vmethod implemented");
 
-    return FALSE;
+    return NULL;
   }
 
   res = GES_TIMELINE_ELEMENT_GET_CLASS (self)->paste (self,
@@ -1723,5 +1725,5 @@ ges_timeline_element_paste (GESTimelineElement * self,
 
   g_clear_object (&self->priv->copied_from);
 
-  return res;
+  return g_object_ref (res);
 }

ges/ges-timeline-element.h

@@ -186,8 +186,10 @@ struct _GESTimelineElementClass
   gboolean (*roll_end)         (GESTimelineElement *self, guint64  end);
   gboolean (*trim)             (GESTimelineElement *self, guint64  start);
   void     (*deep_copy)        (GESTimelineElement *self, GESTimelineElement *copy);
-  gboolean (*paste)            (GESTimelineElement *self, GESTimelineElement *ref_element,
-                                GstClockTime paste_position);
+
+  GESTimelineElement * (*paste) (GESTimelineElement *self,
+                                   GESTimelineElement *ref_element,
+                                   GstClockTime paste_position);
 
   GParamSpec** (*list_children_properties) (GESTimelineElement * self, guint *n_properties);
   gboolean (*lookup_child)                 (GESTimelineElement *self, const gchar *prop_name,
@@ -279,7 +281,7 @@ gboolean ges_timeline_element_add_child_property   (GESTimelineElement * self,
 gboolean ges_timeline_element_remove_child_property(GESTimelineElement * self,
                                                     GParamSpec *pspec);
 
-gboolean ges_timeline_element_paste                (GESTimelineElement * self,
+GESTimelineElement * ges_timeline_element_paste    (GESTimelineElement * self,
                                                     GstClockTime paste_position);
 
 GESTrackType ges_timeline_element_get_track_types  (GESTimelineElement * self);

ges/ges-video-source.c

@@ -124,7 +124,6 @@ ges_video_source_create_element (GESTrackElement * trksrc)
   GstElement *positionner, *videoscale, *videorate, *capsfilter, *videoconvert,
       *deinterlace;
   const gchar *props[] = { "alpha", "posx", "posy", "width", "height", NULL };
-  GESTimelineElement *parent;
 
   if (!source_class->create_source)
     return NULL;
@@ -172,14 +171,7 @@ ges_video_source_create_element (GESTrackElement * trksrc)
         capsfilter, NULL);
   }
 
-  parent = ges_timeline_element_get_parent (GES_TIMELINE_ELEMENT (trksrc));
-  if (parent) {
-    self->priv->positionner = GST_FRAME_POSITIONNER (positionner);
-    gst_object_unref (parent);
-  } else {
-    GST_ERROR ("No parent timeline element, SHOULD NOT HAPPEN");
-  }
-
+  self->priv->positionner = GST_FRAME_POSITIONNER (positionner);
   self->priv->capsfilter = capsfilter;
 
   return topbin;

ges/ges-video-uri-source.c

@@ -50,14 +50,17 @@ ges_video_uri_source_create_source (GESTrackElement * trksrc)
   GESVideoUriSource *self;
   GESTrack *track;
   GstElement *decodebin;
+  const GstCaps *caps = NULL;
 
   self = (GESVideoUriSource *) trksrc;
 
   track = ges_track_element_get_track (trksrc);
+  if (track)
+    caps = ges_track_get_caps (track);
 
   decodebin = gst_element_factory_make ("uridecodebin", NULL);
 
-  g_object_set (decodebin, "caps", ges_track_get_caps (track),
+  g_object_set (decodebin, "caps", caps,
       "expose-all-streams", FALSE, "uri", self->uri, NULL);
 
   return decodebin;

ges/gstframepositionner.c

@@ -198,6 +198,12 @@ ges_frame_positionner_set_source_and_filter (GstFramePositionner * pos,
   pos->capsfilter = capsfilter;
   pos->current_track = ges_track_element_get_track (trksrc);
 
+  if (!pos->current_track) {
+    GST_INFO_OBJECT (pos, "No track set, won't be usable");
+
+    return;
+  }
+
   g_object_add_weak_pointer (G_OBJECT (pos->track_source),
       ((gpointer *) & pos->track_source));
   g_object_weak_ref (G_OBJECT (pos->current_track),

tests/check/python/test_clip.py

@@ -0,0 +1,54 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright (c) 2015, Thibault Saunier
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this program; if not, write to the
+# Free Software Foundation, Inc., 51 Franklin St, Fifth Floor,
+# Boston, MA 02110-1301, USA.
+
+import gi
+
+gi.require_version("Gst", "1.0")
+gi.require_version("GES", "1.0")
+
+from gi.repository import Gst  # noqa
+from gi.repository import GES  # noqa
+import unittest  # noqa
+
+Gst.init(None)
+GES.init()
+
+
+class TestCopyPaste(unittest.TestCase):
+
+    def setUp(self):
+        self.timeline = GES.Timeline.new_audio_video()
+        self.assertEqual(len(self.timeline.get_tracks()), 2)
+        self.layer = self.timeline.append_layer()
+
+    def testCopyClipRemoveAndPaste(self):
+        clip1 = GES.TestClip.new()
+        clip1.props.duration = 10
+
+        self.layer.add_clip(clip1)
+
+        self.assertEqual(len(clip1.get_children(False)), 2)
+
+        copy = clip1.copy(True)
+        self.assertEqual(len(self.layer.get_clips()), 1)
+
+        self.layer.remove_clip(clip1)
+
+        copy.paste(10)
+        self.assertEqual(len(self.layer.get_clips()), 1)

tests/check/python/test_group.py

@@ -61,3 +61,63 @@ class TestCopyPaste(unittest.TestCase):
         clips[1].edit([], 1, GES.EditMode.EDIT_NORMAL, GES.Edge.EDGE_NONE, 10)
         clips = self.layer.get_clips()
         self.assertEqual(len(clips), 1)
+
+    def testPasteChangedGroup(self):
+        clip1 = GES.TestClip.new()
+        clip1.props.duration = 10
+
+        clip2 = GES.TestClip.new()
+        clip2.props.start = 20
+        clip2.props.duration = 10
+
+        self.layer.add_clip(clip1)
+        self.layer.add_clip(clip2)
+
+        self.assertEqual(len(clip1.get_children(False)), 2)
+
+        group = GES.Group.new()
+        self.assertTrue(group.add(clip1))
+
+        self.assertEqual(len(group.get_children(False)), 1)
+
+        group_copy = group.copy(True)
+        self.assertEqual(len(group_copy.get_children(False)), 0)
+
+        self.assertTrue(group.add(clip2))
+        self.assertEqual(len(group.get_children(False)), 2)
+        self.assertEqual(len(group_copy.get_children(False)), 0)
+
+        self.assertTrue(group_copy.paste(10))
+        clips = self.layer.get_clips()
+        self.assertEqual(len(clips), 3)
+        self.assertEqual(clips[1].props.start, 10)
+
+    def testPasteChangedGroup(self):
+        clip1 = GES.TestClip.new()
+        clip1.props.duration = 10
+
+        clip2 = GES.TestClip.new()
+        clip2.props.start = 20
+        clip2.props.duration = 10
+
+        self.layer.add_clip(clip1)
+        self.layer.add_clip(clip2)
+
+        self.assertEqual(len(clip1.get_children(False)), 2)
+
+        group = GES.Group.new()
+        self.assertTrue(group.add(clip1))
+
+        self.assertEqual(len(group.get_children(False)), 1)
+
+        group_copy = group.copy(True)
+        self.assertEqual(len(group_copy.get_children(False)), 0)
+
+        self.assertTrue(group.add(clip2))
+        self.assertEqual(len(group.get_children(False)), 2)
+        self.assertEqual(len(group_copy.get_children(False)), 0)
+
+        self.assertTrue(group_copy.paste(10))
+        clips = self.layer.get_clips()
+        self.assertEqual(len(clips), 3)
+        self.assertEqual(clips[1].props.start, 10)