container: Properly handle the case where we could not set parent
authorThibault Saunier <thibault.saunier@collabora.com>
Fri, 14 Mar 2014 19:04:33 +0000 (20:04 +0100)
committerThibault Saunier <tsaunier@gnome.org>
Sat, 15 Mar 2014 19:01:48 +0000 (20:01 +0100)
In this case we had a FIXME about reverting everything that was done,
implement that FIXME!

ges/ges-container.c
ges/ges-timeline-element.c
tests/check/ges/group.c

index 68e3680..126394f 100644 (file)
@@ -550,7 +550,10 @@ ges_container_add (GESContainer * container, GESTimelineElement * child)
 
   if (ges_timeline_element_set_parent (child, GES_TIMELINE_ELEMENT (container))
       == FALSE) {
-    GST_FIXME_OBJECT (container, "Revert everything that was done before!");
+
+    g_hash_table_remove (priv->mappings, child);
+    container->children = g_list_remove (container->children, child);
+    _ges_container_sort_children (container);
 
     return FALSE;
   }
index b4fe32a..db43af9 100644 (file)
@@ -261,7 +261,13 @@ ges_timeline_element_set_parent (GESTimelineElement * self,
   g_return_val_if_fail (GES_IS_TIMELINE_ELEMENT (self), FALSE);
   g_return_val_if_fail (parent == NULL
       || GES_IS_TIMELINE_ELEMENT (parent), FALSE);
-  g_return_val_if_fail (self != parent, FALSE);
+
+  if (self == parent) {
+    GST_INFO_OBJECT (self, "Trying to add %p in itself, not a good idea!",
+        self);
+
+    return FALSE;
+  }
 
   GST_DEBUG_OBJECT (self, "set parent to %" GST_PTR_FORMAT, parent);
 
index e60b61e..c7c0a2a 100644 (file)
@@ -487,6 +487,44 @@ GST_START_TEST (test_group_in_group)
 
 GST_END_TEST;
 
+GST_START_TEST (test_group_in_self)
+{
+  GESLayer *layer;
+  GESClip *c, *c1;
+  GESAsset *asset;
+  GESTimeline *timeline;
+  GESGroup *group;
+
+  GList *clips = NULL;
+
+  ges_init ();
+
+  timeline = ges_timeline_new_audio_video ();
+
+  layer = ges_timeline_append_layer (timeline);
+  asset = ges_asset_request (GES_TYPE_TEST_CLIP, NULL, NULL);
+
+  c = ges_layer_add_asset (layer, asset, 0, 0, 10, GES_TRACK_TYPE_UNKNOWN);
+  c1 = ges_layer_add_asset (layer, asset, 10, 0, 10, GES_TRACK_TYPE_UNKNOWN);
+  clips = g_list_prepend (clips, c);
+  clips = g_list_prepend (clips, c1);
+
+
+  group = GES_GROUP (ges_container_group (clips));
+  fail_unless (GES_TIMELINE_ELEMENT_TIMELINE (group) == timeline);
+  g_list_free (clips);
+
+  fail_if (ges_container_add (GES_CONTAINER (group),
+          GES_TIMELINE_ELEMENT (group)));
+  clips = ges_container_get_children (GES_CONTAINER (group), TRUE);
+  assert_equals_int (g_list_length (clips), 6);
+
+  gst_object_unref (timeline);
+  gst_object_unref (asset);
+}
+
+GST_END_TEST;
+
 static Suite *
 ges_suite (void)
 {
@@ -497,6 +535,7 @@ ges_suite (void)
 
   tcase_add_test (tc_chain, test_move_group);
   tcase_add_test (tc_chain, test_group_in_group);
+  tcase_add_test (tc_chain, test_group_in_self);
 
   return s;
 }