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

@@ -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);
 }

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

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

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,54 +1789,7 @@ 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);
+    ges_clip_create_track_elements (clip, track->type);
-    for (l = track_elements; l; l = l->next) {
-      GESTrack *tmp_track;
-      GESTrackElement *track_element = l->data;
-
-      GST_DEBUG_OBJECT (timeline, "Got trackelement: %" GST_PTR_FORMAT
-          "Asking to which track it should be added", track_element);
-
-      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;
-      }
-
-      for (i = 0; i < tracks->len; i++) {
-        GESTrackElement *track_element_copy;
-
-        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;
-        }
-
-        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, tmp_track);
-        add_object_to_track (clip, track_element_copy, tmp_track);
-
-        gst_object_unref (tmp_track);
-      }
-
-    next_track_element:
-      if (tracks) {
-        g_ptr_array_unref (tracks);
-        tracks = NULL;
-      }
-      gst_object_unref (track_element);
-    }
   }
 }
 
@@ -1868,6 +1802,91 @@ layer_auto_transition_changed_cb (GESTimelineLayer * layer,
 
 }
 
+static void
+clip_track_element_added_cb (GESClip * clip, GESTrackElement * track_element,
+    GESTimeline * timeline)
+{
+  guint i;
+  GESTrack *track;
+  GPtrArray *tracks = NULL;
+
+  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);
+
+    return;
+  }
+
+  if (ges_track_element_get_track (track_element)) {
+    GST_WARNING_OBJECT (track_element, "Already in a track");
+
+    return;
+  }
+
+  g_signal_emit (G_OBJECT (timeline),
+      ges_timeline_signals[SELECT_TRACKS_FOR_OBJECT], 0, clip, track_element,
+      &tracks);
+
+  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;
+  }
+
+  /* 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
 layer_object_added_cb (GESTimelineLayer * layer, GESClip * clip,
     GESTimeline * timeline)
@@ -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);
 

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

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,
+  fail_unless (ges_track_element_get_track (GES_TRACK_ELEMENT (effect1)) ==
-          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,
+  fail_unless (ges_track_element_get_track (GES_TRACK_ELEMENT (effect2)) ==
-          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,
+  fail_if (ges_track_add_element (track_audio, GES_TRACK_ELEMENT (effect1)));
-          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->
+    fail_unless (ges_track_element_get_track (GES_TRACK_ELEMENT (tmp->data))->
-                data))->type == track_type[i]);
+        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,
+  fail_if (ges_track_add_element (track_audio, GES_TRACK_ELEMENT (effect1)));
-          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);
 

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,
+app_add_effect_on_selected_clips (App * app, const gchar * bin_desc)
-    GESTrackType type)
 {
   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), "");