ges: Handle TimelineLayer and its contained TimelineObject priorities properly

GESTimelineObject.priority is now actually relative to its containing layer
priority.

Test it in the layer test-suite.

ges/ges-timeline-layer.c

@@ -166,7 +166,6 @@ ges_timeline_layer_init (GESTimelineLayer * self)
   self->priv = G_TYPE_INSTANCE_GET_PRIVATE (self,
       GES_TYPE_TIMELINE_LAYER, GESTimelineLayerPrivate);
 
-  /* TODO : Keep those 3 values in sync */
   self->priv->priority = 0;
   self->min_gnl_priority = 0;
   self->max_gnl_priority = 9;
@@ -230,6 +229,7 @@ ges_timeline_layer_add_object (GESTimelineLayer * layer,
     GESTimelineObject * object)
 {
   GESTimelineLayer *tl_obj_layer;
+  guint32 maxprio, minprio, prio;
 
   GST_DEBUG ("layer:%p, object:%p", layer, object);
 
@@ -256,13 +256,19 @@ ges_timeline_layer_add_object (GESTimelineLayer * layer,
       layer->min_gnl_priority, layer->max_gnl_priority);
 
   /* Set the priority. */
-  if (GES_TIMELINE_OBJECT_PRIORITY (object) > (layer->max_gnl_priority)) {
-    ges_timeline_object_set_priority (object, layer->max_gnl_priority);
+  maxprio = layer->max_gnl_priority;
+  minprio = layer->min_gnl_priority;
+  prio = GES_TIMELINE_OBJECT_PRIORITY (object);
+  if (minprio + prio > (maxprio)) {
+    GST_WARNING ("%p is out of the layer %p space, setting its priority to "
+        "setting its priority %d to the maximum priority of the layer %d",
+        object, layer, prio, maxprio - minprio);
+    ges_timeline_object_set_priority (object, LAYER_HEIGHT - 1);
   }
+  /* If the object has an acceptable priority, we just let it with its current
+   * priority */
 
-  else if (GES_TIMELINE_OBJECT_PRIORITY (object) < (layer->min_gnl_priority)) {
-    ges_timeline_object_set_priority (object, layer->min_gnl_priority);
-  }
+  ges_timeline_layer_resync_priorities (layer);
 
   /* emit 'object-added' */
   g_signal_emit (layer, ges_timeline_layer_signals[OBJECT_ADDED], 0, object);
@@ -330,17 +336,15 @@ gboolean
 ges_timeline_layer_resync_priorities (GESTimelineLayer * layer)
 {
   GSList *tmp;
+  GESTimelineObject *obj;
 
   /* TODO : Inhibit composition updates while doing this.
    * Ideally we want to do it from an even higher level, but here will
    * do in the meantime. */
 
-  /* TODO : This is the dumb version where we put everything linearly,
-   * will need to be adjusted for more complex usages (like with
-   * transitions).  */
   for (tmp = layer->priv->objects_start; tmp; tmp = tmp->next) {
-    ges_timeline_object_set_priority ((GESTimelineObject *) tmp->data,
-        layer->min_gnl_priority);
+    obj = GES_TIMELINE_OBJECT (tmp->data);
+    ges_timeline_object_set_priority (obj, GES_TIMELINE_OBJECT_PRIORITY (obj));
   }
 
   return TRUE;
@@ -366,7 +370,6 @@ ges_timeline_layer_set_priority (GESTimelineLayer * layer, guint priority)
     layer->min_gnl_priority = (priority * LAYER_HEIGHT);
     layer->max_gnl_priority = ((priority + 1) * LAYER_HEIGHT) - 1;
 
-    /* FIXME : Update controlled object's gnl priority accordingly */
     ges_timeline_layer_resync_priorities (layer);
   }
 }

ges/ges-timeline-object.c

@@ -58,6 +58,9 @@ static void update_height (GESTimelineObject * object);
 
 static gint sort_track_effects (gpointer a, gpointer b,
     GESTimelineObject * object);
+static void
+get_layer_priorities (GESTimelineLayer * layer, guint32 * layer_min_gnl_prio,
+    guint32 * layer_max_gnl_prio);
 
 static gboolean
 ges_timeline_object_set_start_internal (GESTimelineObject * object,
@@ -417,6 +420,7 @@ ges_timeline_object_add_track_object (GESTimelineObject * object, GESTrackObject
 {
   ObjectMapping *mapping;
   GList *tmp;
+  guint max_prio, min_prio;
   GESTimelineObjectPrivate *priv = object->priv;
   gboolean is_effect = GES_IS_TRACK_EFFECT (trobj);
   GESTimelineObjectClass *klass = GES_TIMELINE_OBJECT_GET_CLASS (object);
@@ -500,8 +504,9 @@ ges_timeline_object_add_track_object (GESTimelineObject * object, GESTrackObject
       g_signal_connect (G_OBJECT (trobj), "notify::priority",
       G_CALLBACK (track_object_priority_changed_cb), object);
 
-  ges_track_object_set_priority (trobj,
-      object->priority + mapping->priority_offset);
+  get_layer_priorities (priv->layer, &min_prio, &max_prio);
+  ges_track_object_set_priority (trobj, min_prio + object->priority
+      + mapping->priority_offset);
 
   GST_DEBUG ("Returning trobj:%p", trobj);
 
@@ -773,21 +778,39 @@ ges_timeline_object_set_priority_internal (GESTimelineObject * object,
   GESTrackObject *tr;
   ObjectMapping *map;
   GESTimelineObjectPrivate *priv = object->priv;
+  guint32 layer_min_gnl_prio, layer_max_gnl_prio;
 
   GST_DEBUG ("object:%p, priority:%" G_GUINT32_FORMAT, object, priority);
 
   priv->ignore_notifies = TRUE;
 
+  object->priv->ignore_notifies = TRUE;
+
+  get_layer_priorities (priv->layer, &layer_min_gnl_prio, &layer_max_gnl_prio);
+
   for (tmp = priv->trackobjects; tmp; tmp = g_list_next (tmp)) {
     tr = (GESTrackObject *) tmp->data;
     map = find_object_mapping (object, tr);
 
     if (ges_track_object_is_locked (tr)) {
+      guint32 real_tck_prio;
+
       /* Move the child... */
-      ges_track_object_set_priority (tr, priority + map->priority_offset);
+      real_tck_prio = layer_min_gnl_prio + priority + map->priority_offset;
+
+      if (real_tck_prio > layer_max_gnl_prio) {
+        GST_WARNING ("%p priority of %i, is outside of the its containing "
+            "layer space. (%d/%d) setting it to the maximum it can be", object,
+            priority, layer_min_gnl_prio, layer_max_gnl_prio);
+
+        real_tck_prio = layer_max_gnl_prio;
+      }
+
+      ges_track_object_set_priority (tr, real_tck_prio);
+
     } else {
       /* ... or update the offset */
-      map->priority_offset = priority - tr->priority;
+      map->priority_offset = layer_min_gnl_prio + priority - tr->priority;
     }
   }
 
@@ -1123,6 +1146,8 @@ track_object_priority_changed_cb (GESTrackObject * child,
     GParamSpec * arg G_GNUC_UNUSED, GESTimelineObject * object)
 {
   ObjectMapping *map;
+  guint32 layer_min_gnl_prio, layer_max_gnl_prio;
+
   guint tck_priority = ges_track_object_get_priority (child);
 
   GST_DEBUG ("Priority changed");
@@ -1132,17 +1157,53 @@ track_object_priority_changed_cb (GESTrackObject * child,
 
   update_height (object);
   map = find_object_mapping (object, child);
+  get_layer_priorities (object->priv->layer, &layer_min_gnl_prio,
+      &layer_max_gnl_prio);
+
   if (G_UNLIKELY (map == NULL))
     /* something massively screwed up if we get this */
     return;
 
   if (!ges_track_object_is_locked (child)) {
+    if (tck_priority < layer_min_gnl_prio || tck_priority > layer_max_gnl_prio) {
+      GST_WARNING ("%p priority of %i, is outside of its containing "
+          "layer space. (%d/%d). This is a bug in the program.", object,
+          tck_priority, layer_min_gnl_prio, layer_max_gnl_prio);
+    }
+
     /* Update the internal priority_offset */
-    map->priority_offset = object->priority - tck_priority;
-  } else if (tck_priority < object->priority) {
+    map->priority_offset =
+        (layer_min_gnl_prio + object->priority) - tck_priority;
+
+  } else if (tck_priority < layer_min_gnl_prio + object->priority) {
     /* Or update the parent priority, the object priority is always the
      * highest priority (smaller number) */
+    if (tck_priority - layer_min_gnl_prio < 0 ||
+        layer_max_gnl_prio - tck_priority < 0) {
+
+      GST_WARNING ("%p priority of %i, is outside of its containing "
+          "layer space. (%d/%d). This is a bug in the program.", object,
+          tck_priority, layer_min_gnl_prio, layer_max_gnl_prio);
+      return;
+    }
+
     ges_timeline_object_set_priority (object,
-        tck_priority + map->priority_offset);
+        tck_priority - layer_min_gnl_prio);
+  }
+
+  GST_DEBUG ("object %p priority %d child %p priority %d", object,
+      object->priority, child, ges_track_object_get_priority (child));
+}
+
+static void
+get_layer_priorities (GESTimelineLayer * layer, guint32 * layer_min_gnl_prio,
+    guint32 * layer_max_gnl_prio)
+{
+  if (layer) {
+    *layer_min_gnl_prio = layer->min_gnl_priority;
+    *layer_max_gnl_prio = layer->max_gnl_priority;
+  } else {
+    *layer_min_gnl_prio = 0;
+    *layer_max_gnl_prio = G_MAXUINT32;
   }
 }

tests/check/ges/layer.c

@@ -20,6 +20,8 @@
 #include <ges/ges.h>
 #include <gst/check/gstcheck.h>
 
+#define LAYER_HEIGHT 10
+
 static gboolean
 my_fill_track_func (GESTimelineObject * object,
     GESTrackObject * trobject, GstElement * gnlobj, gpointer user_data)
@@ -116,14 +118,14 @@ GST_START_TEST (test_layer_properties)
   /* Change the priority of the layer */
   g_object_set (layer, "priority", 1, NULL);
   assert_equals_int (ges_timeline_layer_get_priority (layer), 1);
-  assert_equals_uint64 (GES_TIMELINE_OBJECT_PRIORITY (object), 10);
+  assert_equals_uint64 (GES_TIMELINE_OBJECT_PRIORITY (object), 0);
   gnl_object_check (ges_track_object_get_gnlobject (trackobject), 42, 51, 12,
       51, 10, TRUE);
 
   /* Change it to an insanely high value */
   g_object_set (layer, "priority", 1000000, NULL);
   assert_equals_int (ges_timeline_layer_get_priority (layer), 1000000);
-  assert_equals_uint64 (GES_TIMELINE_OBJECT_PRIORITY (object), 10000000);
+  assert_equals_uint64 (GES_TIMELINE_OBJECT_PRIORITY (object), 0);
   gnl_object_check (ges_track_object_get_gnlobject (trackobject), 42, 51, 12,
       51, 10000000, TRUE);
 
@@ -143,6 +145,120 @@ GST_START_TEST (test_layer_properties)
 
 GST_END_TEST;
 
+GST_START_TEST (test_layer_priorities)
+{
+  GESTrack *track;
+  GESTimeline *timeline;
+  GESTimelineLayer *layer1, *layer2, *layer3;
+  GESTrackObject *tckobj1, *tckobj2, *tckobj3;
+  GESTimelineObject *object1, *object2, *object3;
+  GstElement *gnlobj1, *gnlobj2, *gnlobj3;
+  guint prio1, prio2, prio3;
+
+  ges_init ();
+
+  /* Timeline and 3 Layer */
+  timeline = ges_timeline_new ();
+  layer1 = (GESTimelineLayer *) ges_timeline_layer_new ();
+  layer2 = (GESTimelineLayer *) ges_timeline_layer_new ();
+  layer3 = (GESTimelineLayer *) ges_timeline_layer_new ();
+
+  ges_timeline_layer_set_priority (layer2, 1);
+  ges_timeline_layer_set_priority (layer3, 2);
+
+  fail_unless (ges_timeline_add_layer (timeline, layer1));
+  fail_unless (ges_timeline_add_layer (timeline, layer2));
+  fail_unless (ges_timeline_add_layer (timeline, layer3));
+  fail_unless_equals_int (ges_timeline_layer_get_priority (layer1), 0);
+  fail_unless_equals_int (ges_timeline_layer_get_priority (layer2), 1);
+  fail_unless_equals_int (ges_timeline_layer_get_priority (layer3), 2);
+
+  track = ges_track_video_raw_new ();
+  fail_unless (track != NULL);
+  fail_unless (ges_timeline_add_track (timeline, track));
+
+  object1 =
+      GES_TIMELINE_OBJECT (ges_custom_timeline_source_new (my_fill_track_func,
+          NULL));
+  object2 =
+      GES_TIMELINE_OBJECT (ges_custom_timeline_source_new (my_fill_track_func,
+          NULL));
+  object3 =
+      GES_TIMELINE_OBJECT (ges_custom_timeline_source_new (my_fill_track_func,
+          NULL));
+  fail_unless (object1 != NULL);
+  fail_unless (object2 != NULL);
+  fail_unless (object3 != NULL);
+
+  /* Set priorities on the objects */
+  g_object_set (object1, "priority", 0, NULL);
+  assert_equals_int (GES_TIMELINE_OBJECT_PRIORITY (object1), 0);
+  g_object_set (object2, "priority", 1, NULL);
+  assert_equals_int (GES_TIMELINE_OBJECT_PRIORITY (object2), 1);
+  g_object_set (object3, "priority", LAYER_HEIGHT + 1, NULL);
+  assert_equals_int (GES_TIMELINE_OBJECT_PRIORITY (object3), LAYER_HEIGHT + 1);
+
+  /* Add objects to the timeline */
+  fail_unless (ges_timeline_layer_add_object (layer1, object1));
+  tckobj1 = ges_timeline_object_find_track_object (object1, track, G_TYPE_NONE);
+  fail_unless (tckobj1 != NULL);
+
+  fail_unless (ges_timeline_layer_add_object (layer2, object2));
+  tckobj2 = ges_timeline_object_find_track_object (object2, track, G_TYPE_NONE);
+  fail_unless (tckobj2 != NULL);
+
+  fail_unless (ges_timeline_layer_add_object (layer3, object3));
+  tckobj3 = ges_timeline_object_find_track_object (object3, track, G_TYPE_NONE);
+  fail_unless (tckobj3 != NULL);
+
+  assert_equals_int (GES_TIMELINE_OBJECT_PRIORITY (object1), 0);
+  gnlobj1 = ges_track_object_get_gnlobject (tckobj1);
+  fail_unless (gnlobj1 != NULL);
+  g_object_get (gnlobj1, "priority", &prio1, NULL);
+  assert_equals_int (prio1, 0);
+
+  assert_equals_int (GES_TIMELINE_OBJECT_PRIORITY (object2), 1);
+  gnlobj2 = ges_track_object_get_gnlobject (tckobj2);
+  fail_unless (gnlobj2 != NULL);
+  g_object_get (gnlobj2, "priority", &prio2, NULL);
+  /* object2 is on the second layer and has a priority of 1 */
+  assert_equals_int (prio2, LAYER_HEIGHT + 1);
+
+  assert_equals_int (GES_TIMELINE_OBJECT_PRIORITY (object3), LAYER_HEIGHT - 1);
+  gnlobj3 = ges_track_object_get_gnlobject (tckobj3);
+  fail_unless (gnlobj3 != NULL);
+  /* object3 is on the third layer and has a priority of LAYER_HEIGHT + 1
+   * it priority must have the maximum priority of this layer*/
+  g_object_get (gnlobj3, "priority", &prio3, NULL);
+  assert_equals_int (prio3, LAYER_HEIGHT * 3 - 1);
+
+  /* Move layers around */
+  g_object_set (layer1, "priority", 2, NULL);
+  g_object_set (layer2, "priority", 0, NULL);
+  g_object_set (layer3, "priority", 1, NULL);
+
+  /* And check the new priorities */
+  assert_equals_int (ges_timeline_layer_get_priority (layer1), 2);
+  assert_equals_int (ges_timeline_layer_get_priority (layer2), 0);
+  assert_equals_int (ges_timeline_layer_get_priority (layer3), 1);
+  assert_equals_int (GES_TIMELINE_OBJECT_PRIORITY (object1), 0);
+  assert_equals_int (GES_TIMELINE_OBJECT_PRIORITY (object2), 1);
+  assert_equals_int (GES_TIMELINE_OBJECT_PRIORITY (object3), LAYER_HEIGHT - 1);
+  g_object_get (gnlobj1, "priority", &prio1, NULL);
+  g_object_get (gnlobj2, "priority", &prio2, NULL);
+  g_object_get (gnlobj3, "priority", &prio3, NULL);
+  assert_equals_int (prio1, 2 * LAYER_HEIGHT);
+  assert_equals_int (prio2, 1);
+  assert_equals_int (prio3, LAYER_HEIGHT * 2 - 1);
+
+  g_object_unref (tckobj1);
+  g_object_unref (tckobj2);
+  g_object_unref (tckobj3);
+  g_object_unref (timeline);
+}
+
+GST_END_TEST;
+
 static Suite *
 ges_suite (void)
 {
@@ -152,6 +268,7 @@ ges_suite (void)
   suite_add_tcase (s, tc_chain);
 
   tcase_add_test (tc_chain, test_layer_properties);
+  tcase_add_test (tc_chain, test_layer_priorities);
 
   return s;
 }

tests/check/ges/save_and_load.c

@@ -181,8 +181,7 @@ GST_START_TEST (test_keyfile_save)
   KEY ("Object3", "start", "5000000000");
   KEY ("Object3", "in-point", "0");
   KEY ("Object3", "duration", "1000000000");
-  /* The second layer's minimum priority will be 10 */
-  KEY ("Object3", "priority", "10");
+  KEY ("Object3", "priority", "0");
   KEY ("Object3", "mute", "false");
   KEY ("Object3", "text", "\"the\\\\ quick\\\\ brown\\\\ fox\"");
   KEY ("Object3", "font-desc", "\"Serif\\\\ 36\"");