ges: Recompute Group priority when one of its clip.layer change priority

And add a unit test for that case where it was previously failing

ges/ges-group.c

@@ -112,13 +112,49 @@ _update_our_values (GESGroup * group)
       max_layer_prio - min_layer_prio + 1);
 }
 
+static void
+_child_priority_changed_cb (GESLayer * layer,
+    GParamSpec * arg G_GNUC_UNUSED, GESTimelineElement * clip)
+{
+  GESContainer *container = GES_CONTAINER (GES_TIMELINE_ELEMENT_PARENT (clip));
+
+  gint layer_prio = ges_layer_get_priority (layer);
+  gint offset = _ges_container_get_priority_offset (container, clip);
+
+  if (container->children_control_mode != GES_CHILDREN_UPDATE) {
+    GST_DEBUG_OBJECT (container, "Ignoring updated");
+    return;
+  }
+
+  if (layer_prio + offset == _PRIORITY (container))
+    return;
+
+  container->initiated_move = clip;
+  _set_priority0 (GES_TIMELINE_ELEMENT (container), layer_prio + offset);
+  container->initiated_move = NULL;
+}
+
 static void
 _child_clip_changed_layer_cb (GESTimelineElement * clip,
     GParamSpec * arg G_GNUC_UNUSED, GESGroup * group)
 {
+  GESLayer *old_layer, *new_layer;
   gint offset, layer_prio = ges_clip_get_layer_priority (GES_CLIP (clip));
   GESContainer *container = GES_CONTAINER (group);
 
+  offset = _ges_container_get_priority_offset (container, clip);
+  old_layer = g_list_nth_data (GES_TIMELINE_ELEMENT_TIMELINE (group)->layers,
+      _PRIORITY (group) - offset);
+
+  if (old_layer)
+    g_signal_handlers_disconnect_by_func (old_layer, _child_priority_changed_cb,
+        clip);
+
+  new_layer = ges_clip_get_layer (GES_CLIP (clip));
+  if (new_layer)
+    g_signal_connect (new_layer, "notify::priority",
+        (GCallback) _child_priority_changed_cb, clip);
+
   if (container->children_control_mode != GES_CHILDREN_UPDATE) {
     if (container->children_control_mode == GES_CHILDREN_INIBIT_SIGNAL_EMISSION) {
       container->children_control_mode = GES_CHILDREN_UPDATE;
@@ -127,15 +163,10 @@ _child_clip_changed_layer_cb (GESTimelineElement * clip,
     return;
   }
 
-  offset = _ges_container_get_priority_offset (container, clip);
-
   if (layer_prio + offset < 0 ||
       (GES_TIMELINE_ELEMENT_TIMELINE (group) &&
           layer_prio + offset + GES_CONTAINER_HEIGHT (group) - 1 >
           g_list_length (GES_TIMELINE_ELEMENT_TIMELINE (group)->layers))) {
-    GESLayer *old_layer =
-        g_list_nth_data (GES_TIMELINE_ELEMENT_TIMELINE (group)->layers,
-        _PRIORITY (group) - offset);
 
     GST_INFO_OBJECT (container, "Trying to move to a layer outside of"
         "the timeline layers, moving back to old layer (prio %i)",
@@ -418,6 +449,8 @@ _child_added (GESContainer * group, GESTimelineElement * child)
   if (GES_IS_CLIP (child)) {
     g_signal_connect (child, "notify::layer",
         (GCallback) _child_clip_changed_layer_cb, group);
+    g_signal_connect (ges_clip_get_layer (GES_CLIP (child)),
+        "notify::priority", (GCallback) _child_priority_changed_cb, child);
   } else if (GES_IS_GROUP (child), group) {
     g_signal_connect (child, "notify::priority",
         (GCallback) _child_group_priority_changed, group);
@@ -435,10 +468,12 @@ _child_removed (GESContainer * group, GESTimelineElement * child)
 
   children = GES_CONTAINER_CHILDREN (group);
 
-  if (GES_IS_CLIP (child))
+  if (GES_IS_CLIP (child)) {
+    g_signal_handlers_disconnect_by_func (ges_clip_get_layer (GES_CLIP (child)),
+        _child_priority_changed_cb, child);
     g_signal_handlers_disconnect_by_func (child, _child_clip_changed_layer_cb,
         group);
-  else if (GES_IS_GROUP (child), group)
+  } else if (GES_IS_GROUP (child), group)
     g_signal_handlers_disconnect_by_func (child, _child_group_priority_changed,
         group);
 

ges/ges-timeline.c

@@ -2697,6 +2697,8 @@ ges_timeline_add_layer (GESTimeline * timeline, GESLayer * layer)
   }
   g_list_free (objects);
 
+  timeline->priv->movecontext.needs_move_ctx = TRUE;
+
   return TRUE;
 }
 
@@ -2751,6 +2753,7 @@ ges_timeline_remove_layer (GESTimeline * timeline, GESLayer * layer)
   g_signal_emit (timeline, ges_timeline_signals[LAYER_REMOVED], 0, layer);
 
   gst_object_unref (layer);
+  timeline->priv->movecontext.needs_move_ctx = TRUE;
 
   return TRUE;
 }

tests/check/ges/group.c

@@ -487,6 +487,144 @@ GST_START_TEST (test_group_in_group)
 
 GST_END_TEST;
 
+static guint32
+ges_clip_get_layer_priority (GESClip * clip)
+{
+  GESLayer *layer = ges_clip_get_layer (clip);
+  if (layer == NULL)
+    return -1;
+
+  return ges_layer_get_priority (layer);
+}
+
+GST_START_TEST (test_group_in_group_layer_moving)
+{
+  GESAsset *asset;
+  GESTimeline *timeline;
+  GESGroup *group;
+  GESLayer *layer, *layer1, *layer2, *layer3;
+  GESClip *c, *c1;
+
+  GList *clips = NULL;
+
+  ges_init ();
+
+  timeline = ges_timeline_new_audio_video ();
+
+  /* Our timeline
+   *
+   *    --0------------10-Group-----20
+   *      | +-----------+           |
+   * L    | |    C      |           |
+   *      | +-----------+           |
+   *    --|--------------------------
+   *      |            +------------+
+   * L1   |            |     C1     |
+   *      |            +------------+
+   *    -----------------------------
+   */
+
+  layer = ges_timeline_append_layer (timeline);
+  layer1 = ges_timeline_append_layer (timeline);
+  layer2 = ges_timeline_append_layer (timeline);
+  layer3 = ges_timeline_append_layer (timeline);
+  fail_unless (layer2 && layer3);
+  assert_equals_int (ges_layer_get_priority (layer3), 3);
+  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 (layer1, 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_unless (GES_IS_GROUP (group));
+  CHECK_OBJECT_PROPS (c, 0, 0, 10);
+  CHECK_OBJECT_PROPS (c1, 10, 0, 10);
+  CHECK_OBJECT_PROPS (group, 0, 0, 20);
+
+  /* Our timeline
+   *
+   *    --0--------10-----------20-Group----30
+   *      |         +-----------+           |
+   * L    |         |    C      |           |
+   *      |         +-----------+           |
+   *    --|-----------------------------------
+   *      |                    +------------+
+   * L1   |                    |     C1     |
+   *      |                    +------------+
+   *    -------------------------------------
+   */
+  fail_unless (ges_container_edit (GES_CONTAINER (c), NULL,
+          -1, GES_EDIT_MODE_NORMAL, GES_EDGE_NONE, 10));
+
+  CHECK_OBJECT_PROPS (c, 10, 0, 10);
+  CHECK_OBJECT_PROPS (c1, 20, 0, 10);
+  CHECK_OBJECT_PROPS (group, 10, 0, 20);
+  assert_equals_int (ges_clip_get_layer_priority (c), 0);
+  assert_equals_int (ges_clip_get_layer_priority (c1), 1);
+
+  ges_layer_set_priority (layer2, 0);
+  ges_layer_set_priority (layer, 1);
+  ges_layer_set_priority (layer1, 2);
+
+  assert_equals_int (ges_clip_get_layer_priority (c), 1);
+  assert_equals_int (ges_clip_get_layer_priority (c1), 2);
+
+  /* Our timeline
+   *
+   *    --0--------10-----------20-Group----30
+   *      |         +-----------+           |
+   * L2   |         |    C      |           |
+   *      |         +-----------+           |
+   *    --|-----------------------------------
+   *      |                    +------------+
+   * L    |                    |     C1     |
+   *      |                    +------------+
+   *    -------------------------------------
+   *
+   * L1
+   *    -------------------------------------
+   */
+  fail_unless (ges_container_edit (GES_CONTAINER (c), NULL,
+          0, GES_EDIT_MODE_NORMAL, GES_EDGE_NONE, 10));
+  CHECK_OBJECT_PROPS (c, 10, 0, 10);
+  CHECK_OBJECT_PROPS (c1, 20, 0, 10);
+  CHECK_OBJECT_PROPS (group, 10, 0, 20);
+  assert_equals_int (ges_clip_get_layer_priority (c), 0);
+  assert_equals_int (ges_clip_get_layer_priority (c1), 1);
+
+  /* Our timeline
+   *
+   *    --0--------10-----------20-Group----30
+   * L2   |                                 |
+   *   --------------------------------------
+   *      |         +-----------+           |
+   * L    |         |    C      |           |
+   *      |         +-----------+           |
+   *    --|-----------------------------------
+   *      |                    +------------+
+   * L1   |                    |     C1     |
+   *      |                    +------------+
+   *    -------------------------------------
+   */
+  fail_unless (ges_container_edit (GES_CONTAINER (c), NULL,
+          1, GES_EDIT_MODE_NORMAL, GES_EDGE_NONE, 10));
+  CHECK_OBJECT_PROPS (c, 10, 0, 10);
+  CHECK_OBJECT_PROPS (c1, 20, 0, 10);
+  CHECK_OBJECT_PROPS (group, 10, 0, 20);
+  assert_equals_int (ges_clip_get_layer_priority (c), 1);
+  assert_equals_int (ges_clip_get_layer_priority (c1), 2);
+
+  gst_object_unref (timeline);
+  gst_object_unref (asset);
+}
+
+GST_END_TEST;
+
+
 GST_START_TEST (test_group_in_self)
 {
   GESLayer *layer;
@@ -624,6 +762,7 @@ ges_suite (void)
   tcase_add_test (tc_chain, test_group_in_group);
   tcase_add_test (tc_chain, test_group_in_self);
   tcase_add_test (tc_chain, test_group_serialization);
+  tcase_add_test (tc_chain, test_group_in_group_layer_moving);
 
   return s;
 }