From: Thibault Saunier Date: Fri, 15 Mar 2013 03:01:47 +0000 (-0300) Subject: Remove GESTrackElements from GESTracks when removing from a GESClip X-Git-Tag: 1.19.3~493^2~1961 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=aa740d86bd1fddb02741d94052585fca45f8ab0c;p=platform%2Fupstream%2Fgstreamer.git 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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;