From 9b050a54cdf060df5d9d32eb5a3ca58971981bcd Mon Sep 17 00:00:00 2001
From: Thibault Saunier <thibault.saunier@collabora.com>
Date: Fri, 14 Mar 2014 20:04:33 +0100
Subject: [PATCH] container: Properly handle the case where we could not set
 parent

In this case we had a FIXME about reverting everything that was done,
implement that FIXME!
---
 ges/ges-container.c        |  5 ++++-
 ges/ges-timeline-element.c |  8 +++++++-
 tests/check/ges/group.c    | 39 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/ges/ges-container.c b/ges/ges-container.c
index 68e3680..126394f 100644
--- a/ges/ges-container.c
+++ b/ges/ges-container.c
@@ -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;
   }
diff --git a/ges/ges-timeline-element.c b/ges/ges-timeline-element.c
index b4fe32a..db43af9 100644
--- a/ges/ges-timeline-element.c
+++ b/ges/ges-timeline-element.c
@@ -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);
 
diff --git a/tests/check/ges/group.c b/tests/check/ges/group.c
index e60b61e..c7c0a2a 100644
--- a/tests/check/ges/group.c
+++ b/tests/check/ges/group.c
@@ -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;
 }
-- 
2.7.4