From 7943bb510a706cb0f42b6a4d0e624adbf5ea5829 Mon Sep 17 00:00:00 2001 From: Thibault Saunier Date: Thu, 14 Mar 2013 12:53:25 -0400 Subject: [PATCH] ges: Make GESTimeline responsible for adding GESTrackElement to GESTrack + Fix tests as necessary (Do not use agingtv as it can be "applied" on any TrackType and is not representative of what happens IRL) We already had the infrastructure so the user can have the control over where to add the elements (through the "select-track-for-object" signal). We now make use of that signal everytime a GESClip is added to a GESTimelineLayer. This make user's life easier, and object responsability clearer. --- ges/ges-base-xml-formatter.c | 1 - ges/ges-clip.c | 11 +-- ges/ges-pitivi-formatter.c | 5 -- ges/ges-timeline.c | 160 +++++++++++++++++++++---------------- tests/check/ges/backgroundsource.c | 2 + tests/check/ges/effects.c | 56 ++++++++----- tests/examples/ges-ui.c | 13 +-- 7 files changed, 133 insertions(+), 115 deletions(-) diff --git a/ges/ges-base-xml-formatter.c b/ges/ges-base-xml-formatter.c index a8b9f6a..4100f11 100644 --- a/ges/ges-base-xml-formatter.c +++ b/ges/ges-base-xml-formatter.c @@ -453,7 +453,6 @@ _add_track_element (GESFormatter * self, GESClip * clip, " To : %" GST_PTR_FORMAT, trackelement, clip); ges_container_add (GES_CONTAINER (clip), GES_TIMELINE_ELEMENT (trackelement)); - ges_track_add_element (track, trackelement); gst_structure_foreach (children_properties, (GstStructureForeachFunc) _set_child_property, trackelement); } diff --git a/ges/ges-clip.c b/ges/ges-clip.c index 076897f..786eb56 100644 --- a/ges/ges-clip.c +++ b/ges/ges-clip.c @@ -566,6 +566,8 @@ ges_clip_create_track_elements (GESClip * clip, GESTrackType type) _set_priority0 (elem, min_prio + GES_TIMELINE_ELEMENT_PRIORITY (clip) + clip->priv->nb_effects); + + ges_container_add (GES_CONTAINER (clip), elem); } return result; @@ -1033,8 +1035,6 @@ ges_clip_split (GESClip * clip, guint64 position) * FIXME: Avoid setting it oureself reworking the API */ GES_TIMELINE_ELEMENT (clip)->duration = position - _START (clip); for (tmp = GES_CONTAINER_CHILDREN (clip); tmp; tmp = tmp->next) { - GESTrack *track; - GESTrackElement *new_trackelement, *trackelement = GES_TRACK_ELEMENT (tmp->data); @@ -1059,13 +1059,6 @@ ges_clip_split (GESClip * clip, guint64 position) continue; } - track = ges_track_element_get_track (trackelement); - if (track == NULL) - GST_DEBUG_OBJECT (trackelement, "Was not in a track, not adding %p to" - "any track", new_trackelement); - else - ges_track_add_element (track, new_trackelement); - /* Set 'new' track element timing propeties */ _set_start0 (GES_TIMELINE_ELEMENT (new_trackelement), position); _set_inpoint0 (GES_TIMELINE_ELEMENT (new_trackelement), diff --git a/ges/ges-pitivi-formatter.c b/ges/ges-pitivi-formatter.c index 20932a3..779d1f3 100644 --- a/ges/ges-pitivi-formatter.c +++ b/ges/ges-pitivi-formatter.c @@ -941,11 +941,6 @@ make_source (GESFormatter * self, GList * reflist, GHashTable * source_table) if (!g_strcmp0 (active, (gchar *) "(bool)False")) ges_track_element_set_active (GES_TRACK_ELEMENT (effect), FALSE); - if (video) - ges_track_add_element (priv->trackv, GES_TRACK_ELEMENT (effect)); - else - ges_track_add_element (priv->tracka, GES_TRACK_ELEMENT (effect)); - /* Set effect properties */ keys = g_hash_table_get_keys (effect_table); for (tmp_key = keys; tmp_key; tmp_key = tmp_key->next) { diff --git a/ges/ges-timeline.c b/ges/ges-timeline.c index 66eae95..1837fbe 100644 --- a/ges/ges-timeline.c +++ b/ges/ges-timeline.c @@ -157,6 +157,8 @@ struct _GESTimelinePrivate gboolean needs_transitions_update; gboolean updates_enabled; + + GESClip *ignore_track_element_added; }; /* private structure to contain our track-related information */ @@ -1743,26 +1745,6 @@ timeline_context_to_layer (GESTimeline * timeline, gint offset) return ret; } -static void -add_object_to_track (GESClip * clip, GESTrackElement * track_element, - GESTrack * track) -{ - if (!ges_container_add (GES_CONTAINER (clip), - GES_TIMELINE_ELEMENT (track_element))) { - GST_WARNING_OBJECT (clip, "Failed to add track element to clip"); - gst_object_unref (track_element); - return; - } - - if (!ges_track_add_element (track, track_element)) { - GST_WARNING_OBJECT (clip, "Failed to add track element to track"); - ges_container_remove (GES_CONTAINER (clip), - GES_TIMELINE_ELEMENT (track_element)); - gst_object_unref (track_element); - return; - } -} - static GPtrArray * select_tracks_for_object_default (GESTimeline * timeline, GESClip * clip, GESTrackElement * tr_object, gpointer user_data) @@ -1788,9 +1770,8 @@ static void add_object_to_tracks (GESTimeline * timeline, GESClip * clip, GESTrack * track) { gint i; - GPtrArray *tracks = NULL; + GList *tmp; GESTrackType types, visited_type = GES_TRACK_TYPE_UNKNOWN; - GList *tmp, *l, *track_elements; GST_DEBUG_OBJECT (timeline, "Creating %" GST_PTR_FORMAT " trackelements and adding them to our tracks", clip); @@ -1808,64 +1789,102 @@ add_object_to_tracks (GESTimeline * timeline, GESClip * clip, GESTrack * track) if (((track->type & types) == 0 || (track->type & visited_type))) continue; - track_elements = ges_clip_create_track_elements (clip, track->type); - for (l = track_elements; l; l = l->next) { - GESTrack *tmp_track; - GESTrackElement *track_element = l->data; + ges_clip_create_track_elements (clip, track->type); + } +} - GST_DEBUG_OBJECT (timeline, "Got trackelement: %" GST_PTR_FORMAT - "Asking to which track it should be added", track_element); +static void +layer_auto_transition_changed_cb (GESTimelineLayer * layer, + GParamSpec * arg G_GNUC_UNUSED, GESTimeline * timeline) +{ + _create_transitions_on_layer (timeline, layer, NULL, NULL, + _create_auto_transition_from_transitions); - g_signal_emit (G_OBJECT (timeline), - ges_timeline_signals[SELECT_TRACKS_FOR_OBJECT], 0, clip, - track_element, &tracks); +} - if (!tracks || tracks->len == 0) { - GST_DEBUG_OBJECT (timeline, "Got no Track to add %p (type %s)", - track_element, - ges_track_type_name (ges_track_element_get_track_type - (track_element))); - goto next_track_element; - } +static void +clip_track_element_added_cb (GESClip * clip, GESTrackElement * track_element, + GESTimeline * timeline) +{ + guint i; + GESTrack *track; + GPtrArray *tracks = NULL; - for (i = 0; i < tracks->len; i++) { - GESTrackElement *track_element_copy; + if (timeline->priv->ignore_track_element_added == clip) { + GST_DEBUG_OBJECT (timeline, "Ignoring element added (%" GST_PTR_FORMAT + " in %" GST_PTR_FORMAT, track_element, clip); - tmp_track = g_ptr_array_index (tracks, i); - if (track && tmp_track != track) { - GST_LOG_OBJECT (timeline, - "Not adding %" GST_PTR_FORMAT " to any track", track_element); - continue; - } + return; + } - track_element_copy = - GES_TRACK_ELEMENT (ges_timeline_element_copy (GES_TIMELINE_ELEMENT - (track_element), TRUE)); + if (ges_track_element_get_track (track_element)) { + GST_WARNING_OBJECT (track_element, "Already in a track"); - GST_LOG_OBJECT (timeline, "Trying to add %p to track %p", - track_element_copy, tmp_track); - add_object_to_track (clip, track_element_copy, tmp_track); + return; + } - gst_object_unref (tmp_track); - } + g_signal_emit (G_OBJECT (timeline), + ges_timeline_signals[SELECT_TRACKS_FOR_OBJECT], 0, clip, track_element, + &tracks); - next_track_element: - if (tracks) { - g_ptr_array_unref (tracks); - tracks = NULL; - } - gst_object_unref (track_element); - } + if (!tracks || tracks->len == 0) { + GST_WARNING_OBJECT (timeline, "Got no Track to add %p (type %s), removing" + " from clip", + track_element, ges_track_type_name (ges_track_element_get_track_type + (track_element))); + + if (tracks) + g_ptr_array_unref (tracks); + + ges_container_remove (GES_CONTAINER (clip), + GES_TIMELINE_ELEMENT (track_element)); + + return; } -} -static void -layer_auto_transition_changed_cb (GESTimelineLayer * layer, - GParamSpec * arg G_GNUC_UNUSED, GESTimeline * timeline) -{ - _create_transitions_on_layer (timeline, layer, NULL, NULL, - _create_auto_transition_from_transitions); + /* We add the current element to the first track */ + track = g_ptr_array_index (tracks, 0); + if (!ges_track_add_element (track, track_element)) { + GST_WARNING_OBJECT (clip, "Failed to add track element to track"); + ges_container_remove (GES_CONTAINER (clip), + GES_TIMELINE_ELEMENT (track_element)); + gst_object_unref (track_element); + return; + } + gst_object_unref (track); + + /* And create copies to add to other tracks */ + timeline->priv->ignore_track_element_added = clip; + for (i = 1; i < tracks->len; i++) { + GESTrack *track; + GESTrackElement *track_element_copy; + + track = g_ptr_array_index (tracks, i); + track_element_copy = + GES_TRACK_ELEMENT (ges_timeline_element_copy (GES_TIMELINE_ELEMENT + (track_element), TRUE)); + + GST_LOG_OBJECT (timeline, "Trying to add %p to track %p", + track_element_copy, track); + + if (!ges_container_add (GES_CONTAINER (clip), + GES_TIMELINE_ELEMENT (track_element_copy))) { + GST_WARNING_OBJECT (clip, "Failed to add track element to clip"); + gst_object_unref (track_element_copy); + return; + } + if (!ges_track_add_element (track, track_element_copy)) { + GST_WARNING_OBJECT (clip, "Failed to add track element to track"); + ges_container_remove (GES_CONTAINER (clip), + GES_TIMELINE_ELEMENT (track_element_copy)); + gst_object_unref (track_element_copy); + return; + } + + gst_object_unref (track); + } + timeline->priv->ignore_track_element_added = NULL; } static void @@ -1884,6 +1903,9 @@ layer_object_added_cb (GESTimelineLayer * layer, GESClip * clip, } 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"); } @@ -1930,6 +1952,8 @@ layer_object_removed_cb (GESTimelineLayer * layer, GESClip * clip, GES_TIMELINE_ELEMENT (track_element)); } } + g_signal_handlers_disconnect_by_func (clip, clip_track_element_added_cb, + timeline); g_list_free_full (trackelements, gst_object_unref); diff --git a/tests/check/ges/backgroundsource.c b/tests/check/ges/backgroundsource.c index c4877f0..65331d3 100644 --- a/tests/check/ges/backgroundsource.c +++ b/tests/check/ges/backgroundsource.c @@ -257,6 +257,8 @@ GST_START_TEST (test_gap_filling_basic) assert_equals_uint64 (_START (clip), 0); assert_equals_uint64 (_DURATION (clip), 5); + /* TODO Since the timeline is responsible for adding TrackElements to + * Tracks, this test should be refactored */ trackelement = ges_clip_create_track_element (clip, track->type); ges_container_add (GES_CONTAINER (clip), GES_TIMELINE_ELEMENT (trackelement)); diff --git a/tests/check/ges/effects.c b/tests/check/ges/effects.c index ddd43d3..bc598b3 100644 --- a/tests/check/ges/effects.c +++ b/tests/check/ges/effects.c @@ -71,7 +71,8 @@ GST_START_TEST (test_add_effect_to_clip) fail_unless (GES_IS_EFFECT (effect)); fail_unless (ges_container_add (GES_CONTAINER (source), GES_TIMELINE_ELEMENT (effect))); - fail_unless (ges_track_add_element (track_video, GES_TRACK_ELEMENT (effect))); + fail_unless (ges_track_element_get_track (GES_TRACK_ELEMENT (effect)) != + NULL); assert_equals_int (GES_TRACK_ELEMENT (effect)->active, TRUE); @@ -120,17 +121,18 @@ GST_START_TEST (test_get_effects_from_tl) fail_unless (ges_container_add (GES_CONTAINER (source), GES_TIMELINE_ELEMENT (effect))); - fail_unless (ges_track_add_element (track_video, GES_TRACK_ELEMENT (effect))); + fail_unless (ges_track_element_get_track (GES_TRACK_ELEMENT (effect)) == + track_video); fail_unless (ges_container_add (GES_CONTAINER (source), GES_TIMELINE_ELEMENT (effect1))); - fail_unless (ges_track_add_element (track_video, - GES_TRACK_ELEMENT (effect1))); + fail_unless (ges_track_element_get_track (GES_TRACK_ELEMENT (effect1)) == + track_video); fail_unless (ges_container_add (GES_CONTAINER (source), GES_TIMELINE_ELEMENT (effect2))); - fail_unless (ges_track_add_element (track_video, - GES_TRACK_ELEMENT (effect2))); + fail_unless (ges_track_element_get_track (GES_TRACK_ELEMENT (effect2)) == + track_video); assert_equals_int (GES_CONTAINER_HEIGHT (source), 4); effects = ges_clip_get_top_effects (GES_CLIP (source)); @@ -181,26 +183,29 @@ GST_START_TEST (test_effect_clip) ges_timeline_add_layer (timeline, layer); GST_DEBUG ("Create effect"); - effect_clip = ges_effect_clip_new ("identity", "identity"); + effect_clip = ges_effect_clip_new ("agingtv", "audiopanorama"); g_object_set (effect_clip, "duration", 25 * GST_SECOND, NULL); ges_simple_timeline_layer_add_object ((GESSimpleTimelineLayer *) (layer), (GESClip *) effect_clip, 0); - effect = ges_effect_new ("identity"); + effect = ges_effect_new ("agingtv"); fail_unless (ges_container_add (GES_CONTAINER (effect_clip), GES_TIMELINE_ELEMENT (effect))); - fail_unless (ges_track_add_element (track_video, GES_TRACK_ELEMENT (effect))); + fail_if (ges_track_add_element (track_video, GES_TRACK_ELEMENT (effect))); + fail_unless (ges_track_element_get_track (GES_TRACK_ELEMENT (effect)) == + track_video); g_object_get (effect_clip, "height", &clip_height, NULL); assert_equals_int (clip_height, 3); - effect1 = ges_effect_new ("identity"); + effect1 = ges_effect_new ("audiopanorama"); fail_unless (ges_container_add (GES_CONTAINER (effect_clip), GES_TIMELINE_ELEMENT (effect1))); - fail_unless (ges_track_add_element (track_audio, - GES_TRACK_ELEMENT (effect1))); + fail_if (ges_track_add_element (track_audio, GES_TRACK_ELEMENT (effect1))); + fail_unless (ges_track_element_get_track (GES_TRACK_ELEMENT (effect1)) == + track_audio); g_object_get (effect_clip, "height", &clip_height, NULL); assert_equals_int (clip_height, 4); @@ -211,8 +216,8 @@ GST_START_TEST (test_effect_clip) GES_BASE_EFFECT (tmp->data)); fail_unless (priority > effect_prio); fail_unless (GES_IS_EFFECT (tmp->data)); - fail_unless (ges_track_element_get_track (GES_TRACK_ELEMENT (tmp-> - data))->type == track_type[i]); + fail_unless (ges_track_element_get_track (GES_TRACK_ELEMENT (tmp->data))-> + type == track_type[i]); effect_prio = priority; g_object_unref (tmp->data); @@ -250,7 +255,7 @@ GST_START_TEST (test_priorities_clip) ges_timeline_add_layer (timeline, layer); GST_DEBUG ("Create effect"); - effect_clip = ges_effect_clip_new ("identity", "identity"); + effect_clip = ges_effect_clip_new ("agingtv", "audiopanorama"); g_object_set (effect_clip, "duration", 25 * GST_SECOND, NULL); @@ -275,19 +280,22 @@ GST_START_TEST (test_priorities_clip) assert_equals_int (_PRIORITY (video_effect), 1); assert_equals_int (GES_CONTAINER_HEIGHT (effect_clip), 2); - effect = ges_effect_new ("identity"); + effect = ges_effect_new ("agingtv"); GST_DEBUG ("Adding effect to the effect clip %" GST_PTR_FORMAT, effect); fail_unless (ges_container_add (GES_CONTAINER (effect_clip), GES_TIMELINE_ELEMENT (effect))); - fail_unless (ges_track_add_element (track_video, GES_TRACK_ELEMENT (effect))); + fail_if (ges_track_add_element (track_video, GES_TRACK_ELEMENT (effect))); + fail_unless (ges_track_element_get_track (GES_TRACK_ELEMENT (effect)) == + track_video); assert_equals_int (GES_CONTAINER_HEIGHT (effect_clip), 3); - effect1 = ges_effect_new ("identity"); + effect1 = ges_effect_new ("audiopanorama"); GST_DEBUG ("Adding effect1 to the effect clip %" GST_PTR_FORMAT, effect1); fail_unless (ges_container_add (GES_CONTAINER (effect_clip), GES_TIMELINE_ELEMENT (effect1))); - fail_unless (ges_track_add_element (track_audio, - GES_TRACK_ELEMENT (effect1))); + fail_if (ges_track_add_element (track_audio, GES_TRACK_ELEMENT (effect1))); + fail_unless (ges_track_element_get_track (GES_TRACK_ELEMENT (effect1)) == + track_audio); fail_unless (ges_clip_set_top_effect_priority (GES_CLIP (effect_clip), GES_BASE_EFFECT (effect1), 0)); @@ -353,7 +361,9 @@ GST_START_TEST (test_effect_set_properties) effect = GES_TRACK_ELEMENT (ges_effect_new ("agingtv")); fail_unless (ges_container_add (GES_CONTAINER (effect_clip), GES_TIMELINE_ELEMENT (effect))); - fail_unless (ges_track_add_element (track_video, effect)); + fail_if (ges_track_add_element (track_video, effect)); + fail_unless (ges_track_element_get_track (GES_TRACK_ELEMENT (effect)) == + track_video); ges_track_element_set_child_properties (effect, "GstAgingTV::scratch-lines", 17, "color-aging", FALSE, NULL); @@ -445,7 +455,9 @@ GST_START_TEST (test_clip_signals) fail_unless (effect_added); g_signal_handlers_disconnect_by_func (effect_clip, effect_added_cb, &effect_added); - fail_unless (ges_track_add_element (track_video, GES_TRACK_ELEMENT (effect))); + fail_if (ges_track_add_element (track_video, GES_TRACK_ELEMENT (effect))); + fail_unless (ges_track_element_get_track (GES_TRACK_ELEMENT (effect)) == + track_video); g_signal_connect (effect, "deep-notify", (GCallback) deep_prop_changed_cb, effect); diff --git a/tests/examples/ges-ui.c b/tests/examples/ges-ui.c index aa1c63e..74d63e9 100644 --- a/tests/examples/ges-ui.c +++ b/tests/examples/ges-ui.c @@ -1069,8 +1069,7 @@ app_move_selected_up (App * app) } static void -app_add_effect_on_selected_clips (App * app, const gchar * bin_desc, - GESTrackType type) +app_add_effect_on_selected_clips (App * app, const gchar * bin_desc) { GList *objects, *tmp; GESTrackElement *effect = NULL; @@ -1083,12 +1082,6 @@ app_add_effect_on_selected_clips (App * app, const gchar * bin_desc, effect = GES_TRACK_ELEMENT (ges_effect_new (bin_desc)); ges_container_add (GES_CONTAINER (tmp->data), GES_TIMELINE_ELEMENT (effect)); - - if (type == GES_TRACK_TYPE_VIDEO) - ges_track_add_element (app->video_track, effect); - else if (type == GES_TRACK_TYPE_AUDIO) - ges_track_add_element (app->audio_track, effect); - g_object_unref (tmp->data); } } @@ -1114,13 +1107,13 @@ on_apply_effect_cb (GtkButton * button, App * app) effect = gtk_entry_get_text (GTK_ENTRY (app->video_effect_entry)); if (g_strcmp0 (effect, "")) - app_add_effect_on_selected_clips (app, effect, GES_TRACK_TYPE_VIDEO); + app_add_effect_on_selected_clips (app, effect); gtk_entry_set_text (GTK_ENTRY (app->video_effect_entry), ""); effect = gtk_entry_get_text (GTK_ENTRY (app->audio_effect_entry)); if (g_strcmp0 (effect, "")) - app_add_effect_on_selected_clips (app, effect, GES_TRACK_TYPE_AUDIO); + app_add_effect_on_selected_clips (app, effect); gtk_entry_set_text (GTK_ENTRY (app->audio_effect_entry), ""); -- 2.7.4