Remove GESTrackElements from GESTracks when removing from a GESClip

... Not the other way round.

  + Add and enhance debugging info on the way

The user should not be responsible for removing the GESTrackElements from
GESTracks, instead, removing it from a GESClip should imply removing
it from any GESTrack it is in.

This patch changes sensibly the behaviour when we remove a
GESTrackElement from a GESTrack, not remoing it from the GESClip it is
in. *But*, users should never remove a GESTrackElement from a GESTrack
anyway. The testsuite has been updated to that new behaviour.

diff --git a/ges/ges-clip.c b/ges/ges-clip.c
index 786eb56..62023aa 100644 (file)
--- a/ges/ges-clip.c
+++ b/ges/ges-clip.c
@@ -216,9 +216,12 @@ _ungroup (GESContainer * container, gboolean recursive)
 
     /* Move trackelement to the container it is supposed to land into */
     if (tmpclip != clip) {
+      /* We need to bump the refcount to avoid the object to be destroyed */
+      gst_object_ref (track_element);
       ges_container_remove (container, GES_TIMELINE_ELEMENT (track_element));
       ges_container_add (GES_CONTAINER (tmpclip),
           GES_TIMELINE_ELEMENT (track_element));
+      gst_object_unref (track_element);
     }
   }
   g_hash_table_foreach (_tracktype_clip, (GHFunc) add_tlobj_to_list, &ret);
@@ -354,8 +357,11 @@ _group (GList * containers)
         tmpelement = tmpelement->next) {
       GESTimelineElement *celement = GES_TIMELINE_ELEMENT (tmpelement->data);
 
+      /* We need to bump the refcount to avoid the object to be destroyed */
+      gst_object_ref (celement);
       ges_container_remove (GES_CONTAINER (cclip), celement);
       ges_container_add (ret, celement);
+      gst_object_unref (celement);
 
       supported_formats = supported_formats |
           ges_track_element_get_track_type (GES_TRACK_ELEMENT (celement));

diff --git a/ges/ges-container.c b/ges/ges-container.c
index fbae769..41e1d3a 100644 (file)
--- a/ges/ges-container.c
+++ b/ges/ges-container.c
@@ -560,8 +560,10 @@ ges_container_add (GESContainer * container, GESTimelineElement * child)
 
   priv->ignore_notifies = TRUE;
   if (class->add_child) {
-    if (class->add_child (container, child) == FALSE)
+    if (class->add_child (container, child) == FALSE) {
+      GST_WARNING_OBJECT (container, "Erreur adding child %p", child);
       return FALSE;
+    }
   }
   priv->ignore_notifies = FALSE;
 

diff --git a/ges/ges-timeline.c b/ges/ges-timeline.c
index 1837fbe..aacb886 100644 (file)
--- a/ges/ges-timeline.c
+++ b/ges/ges-timeline.c
@@ -1888,24 +1888,40 @@ clip_track_element_added_cb (GESClip * clip, GESTrackElement * track_element,
 }
 
 static void
+clip_track_element_removed_cb (GESClip * clip, GESTrackElement * track_element,
+    GESTimeline * timeline)
+{
+  GESTrack *track = ges_track_element_get_track (track_element);
+
+  if (track)
+    ges_track_remove_element (track, track_element);
+}
+
+static void
 layer_object_added_cb (GESTimelineLayer * layer, GESClip * clip,
     GESTimeline * timeline)
 {
+  /* We make sure not to be connected twice */
+  g_signal_handlers_disconnect_by_func (clip, clip_track_element_added_cb,
+      timeline);
+  g_signal_handlers_disconnect_by_func (clip, clip_track_element_removed_cb,
+      timeline);
+
+  /* And we connect to the object */
+  g_signal_connect (clip, "child-added",
+      G_CALLBACK (clip_track_element_added_cb), timeline);
+  g_signal_connect (clip, "child-removed",
+      G_CALLBACK (clip_track_element_removed_cb), timeline);
+
   if (ges_clip_is_moving_from_layer (clip)) {
-    GST_DEBUG ("Clip %p is moving from a layer to another, not doing"
-        " anything on it", clip);
+    GST_DEBUG ("Clip %p moving from one layer to another, not creating "
+        "TrackElement", clip);
     timeline->priv->movecontext.needs_move_ctx = TRUE;
-
     _create_transitions_on_layer (timeline, layer, NULL, NULL,
         _find_transition_from_auto_transitions);
-
     return;
   }
 
-  GST_DEBUG ("New Clip %p added to layer %p", clip, layer);
-  /* Here we connect to the child-added signal, and  */
-  g_signal_connect (clip, "child-added",
-      G_CALLBACK (clip_track_element_added_cb), timeline);
   add_object_to_tracks (timeline, clip, NULL);
   GST_DEBUG ("Done");
 }
@@ -1946,14 +1962,14 @@ layer_object_removed_cb (GESTimelineLayer * layer, GESClip * clip,
                 (GCompareFunc) custom_find_track))) {
       GST_DEBUG ("Belongs to one of the tracks we control");
 
-      ges_track_remove_element (ges_track_element_get_track (track_element),
-          track_element);
       ges_container_remove (GES_CONTAINER (clip),
           GES_TIMELINE_ELEMENT (track_element));
     }
   }
   g_signal_handlers_disconnect_by_func (clip, clip_track_element_added_cb,
       timeline);
+  g_signal_handlers_disconnect_by_func (clip, clip_track_element_removed_cb,
+      timeline);
 
   g_list_free_full (trackelements, gst_object_unref);
 

diff --git a/ges/ges-track-element.c b/ges/ges-track-element.c
index 0251967..0417147 100644 (file)
--- a/ges/ges-track-element.c
+++ b/ges/ges-track-element.c
@@ -185,12 +185,12 @@ ges_track_element_dispose (GObject * object)
     GstState cstate;
 
     if (priv->track != NULL) {
-      g_error ("Still in %p, this means that you forgot"
+      g_error ("%p Still in %p, this means that you forgot"
           " to remove it from the GESTrack it is contained in. You always need"
           " to remove a GESTrackElement from its track before dropping the last"
           " reference\n"
           "This problem may also be caused by a refcounting bug in"
-          " the application or GES itself.", priv->track);
+          " the application or GES itself.", object, priv->track);
       gst_element_get_state (priv->gnlobject, &cstate, NULL, 0);
       if (cstate != GST_STATE_NULL)
         gst_element_set_state (priv->gnlobject, GST_STATE_NULL);

diff --git a/ges/ges-track.c b/ges/ges-track.c
index 997cb2b..5cc015c 100644 (file)
--- a/ges/ges-track.c
+++ b/ges/ges-track.c
@@ -380,13 +380,6 @@ remove_object_internal (GESTrack * track, GESTrackElement * object)
 static void
 dispose_trackelements_foreach (GESTrackElement * trackelement, GESTrack * track)
 {
-  GESClip *clip;
-
-  clip = GES_CLIP (GES_TIMELINE_ELEMENT_PARENT (trackelement));
-
-  if (clip)
-    ges_container_remove (GES_CONTAINER (clip),
-        GES_TIMELINE_ELEMENT (trackelement));
   remove_object_internal (track, trackelement);
 }
 

diff --git a/tests/check/ges/basic.c b/tests/check/ges/basic.c
index f0e78b1..0d90ca6 100644 (file)
--- a/tests/check/ges/basic.c
+++ b/tests/check/ges/basic.c
@@ -510,15 +510,18 @@ GST_START_TEST (test_ges_timeline_remove_track)
   /* remove the track and check that the track elements have been released */
   fail_unless (ges_timeline_remove_track (timeline, track));
 
-  ASSERT_OBJECT_REFCOUNT (t1, "trackelement", 2);
-  ASSERT_OBJECT_REFCOUNT (t2, "trackelement", 2);
-  ASSERT_OBJECT_REFCOUNT (t3, "trackelement", 2);
+  ASSERT_OBJECT_REFCOUNT (t1, "trackelement", 3);
+  ASSERT_OBJECT_REFCOUNT (t2, "trackelement", 3);
+  ASSERT_OBJECT_REFCOUNT (t3, "trackelement", 3);
 
   g_object_unref (t1);
   g_object_unref (t2);
   g_object_unref (t3);
 
   g_object_unref (timeline);
+  ASSERT_OBJECT_REFCOUNT (t1, "trackelement", 0);
+  ASSERT_OBJECT_REFCOUNT (t2, "trackelement", 0);
+  ASSERT_OBJECT_REFCOUNT (t3, "trackelement", 0);
 }
 
 GST_END_TEST;