timeline-tree: take time effects into account when trimming
authorHenry Wilkes <hwilkes@igalia.com>
Fri, 15 May 2020 13:47:15 +0000 (14:47 +0100)
committerHenry Wilkes <hwilkes@igalia.com>
Mon, 25 May 2020 10:20:38 +0000 (11:20 +0100)
When trimming the start of a clip, we want to set the in-point of its
children such that whatever data was at the timeline time T still
remains at the timeline time T after the trim, where
  T = MAX (prev_start, new_start)

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-editing-services/-/merge_requests/177>

ges/ges-clip.c
ges/ges-enums.h
ges/ges-internal.h
ges/ges-timeline-tree.c
tests/check/python/test_timeline.py

index 5b31b82..bffc36e 100644 (file)
@@ -581,6 +581,26 @@ _child_priority_changed (GESContainer * container, GESTimelineElement * child)
  *                    in-point                      *
  ****************************************************/
 
+GstClockTime
+ges_clip_duration_limit_with_new_children_inpoints (GESClip * clip,
+    GHashTable * child_inpoints)
+{
+  GHashTableIter iter;
+  gpointer key, value;
+  GList *child_data = NULL;
+
+  g_hash_table_iter_init (&iter, child_inpoints);
+  while (g_hash_table_iter_next (&iter, &key, &value)) {
+    GESTrackElement *child = key;
+    GstClockTime *inpoint_p = value;
+    DurationLimitData *data = _duration_limit_data_new (child);
+    data->inpoint = *inpoint_p;
+    child_data = g_list_prepend (child_data, data);
+  }
+
+  return _calculate_duration_limit (clip, child_data);
+}
+
 static gboolean
 _can_set_inpoint_of_core_children (GESClip * clip, GstClockTime inpoint,
     GError ** error)
@@ -3901,6 +3921,13 @@ _convert_core_time (GESClip * clip, GstClockTime time, gboolean to_timeline,
   return converted;
 }
 
+GstClockTime
+ges_clip_get_core_internal_time_from_timeline_time (GESClip * clip,
+    GstClockTime timeline_time, gboolean * no_core, GError ** error)
+{
+  return _convert_core_time (clip, timeline_time, FALSE, no_core, error);
+}
+
 /**
  * ges_clip_get_timeline_time_from_source_frame:
  * @clip: A #GESClip
index 554eabf..d3cca8b 100644 (file)
@@ -429,10 +429,10 @@ GType ges_pipeline_flags_get_type (void);
  * + START-TRIM: This cuts or grows the start of the element, whilst
  *   maintaining the time at which its internal content appears in the
  *   timeline data output. If the element is made shorter, the data that
- *   appeared at the edit position and later will still appear in the
- *   timeline at the same time. If the element is made longer, the data
- *   that appeared at the previous start of the element and later will
- *   still appear in the timeline at the same time.
+ *   appeared at the edit position will still appear in the timeline at
+ *   the same time. If the element is made longer, the data that appeared
+ *   at the previous start of the element will still appear in the
+ *   timeline at the same time.
  * + END-TRIM: Similar to START-TRIM, but the end of the element is cut or
  *   grown.
  *
index 5d5721c..f29498d 100644 (file)
@@ -417,6 +417,8 @@ G_GNUC_INTERNAL gboolean          ges_clip_can_set_active_of_child (GESClip * cl
 G_GNUC_INTERNAL gboolean          ges_clip_can_set_priority_of_child (GESClip * clip, GESTrackElement * child, guint32 priority, GError ** error);
 G_GNUC_INTERNAL gboolean          ges_clip_can_set_track_of_child (GESClip * clip, GESTrackElement * child, GESTrack * tack, GError ** error);
 G_GNUC_INTERNAL gboolean          ges_clip_can_set_time_property_of_child (GESClip * clip, GESTrackElement * child, GObject * prop_object, GParamSpec * pspec, const GValue * value, GError ** error);
+G_GNUC_INTERNAL GstClockTime      ges_clip_duration_limit_with_new_children_inpoints (GESClip * clip, GHashTable * child_inpoints);
+G_GNUC_INTERNAL GstClockTime      ges_clip_get_core_internal_time_from_timeline_time (GESClip * clip, GstClockTime timeline_time, gboolean * no_core, GError ** error);
 G_GNUC_INTERNAL void              ges_clip_empty_from_track       (GESClip * clip, GESTrack * track);
 
 /****************************************************
index edc9fbf..cb8d48c 100644 (file)
@@ -51,6 +51,7 @@ typedef enum
   EDIT_MOVE,
   EDIT_TRIM_START,
   EDIT_TRIM_END,
+  EDIT_TRIM_INPOINT_ONLY,
 } ElementEditMode;
 
 typedef struct _EditData
@@ -385,6 +386,9 @@ get_start_end_from_offset (GESTimelineElement * element, ElementEditMode mode,
         *negative_start = FALSE;
       new_end = _clock_time_minus_diff (current_end, offset, negative_end);
       break;
+    case EDIT_TRIM_INPOINT_ONLY:
+      GST_ERROR_OBJECT (element, "Trim in-point only not handled");
+      break;
   }
   if (start)
     *start = new_start;
@@ -547,6 +551,9 @@ timeline_tree_snap (GNode * root, GESTimelineElement * element,
       g_node_traverse (node, G_IN_ORDER, G_TRAVERSE_LEAVES, -1,
           (GNodeTraverseFunc) find_source_at_edge, &data);
       break;
+    case EDIT_TRIM_INPOINT_ONLY:
+      GST_ERROR_OBJECT (element, "Trim in-point only not handled");
+      goto done;
   }
 
   for (tmp = data.sources; tmp; tmp = tmp->next) {
@@ -588,6 +595,9 @@ timeline_tree_snap (GNode * root, GESTimelineElement * element,
         /* only snap the start of the source */
         find_snap_for_element (source, end, negative_end, &data);
         break;
+      case EDIT_TRIM_INPOINT_ONLY:
+        GST_ERROR_OBJECT (element, "Trim in-point only not handled");
+        goto done;
     }
   }
 
@@ -908,6 +918,18 @@ set_breaks_duration_limit_error (GError ** error, GESClip * clip,
       GST_TIME_ARGS (duration_limit));
 }
 
+static void
+set_inpoint_breaks_max_duration_error (GError ** error,
+    GESTimelineElement * element, GstClockTime inpoint,
+    GstClockTime max_duration)
+{
+  g_set_error (error, GES_ERROR, GES_ERROR_NOT_ENOUGH_INTERNAL_CONTENT,
+      "The element \"%s\" would have an in-point of %" GST_TIME_FORMAT
+      " that would break its max-duration of %" GST_TIME_FORMAT,
+      GES_TIMELINE_ELEMENT_NAME (element), GST_TIME_ARGS (inpoint),
+      GST_TIME_ARGS (max_duration));
+}
+
 static gboolean
 set_layer_priority (GESTimelineElement * element, EditData * data,
     GError ** error)
@@ -981,60 +1003,123 @@ set_edit_move_values (GESTimelineElement * element, EditData * data,
 }
 
 static gboolean
-set_edit_trim_start_inpoint_value (GESTimelineElement * element,
-    EditData * data, GError ** error)
+set_edit_trim_start_clip_inpoints (GESClip * clip, EditData * clip_data,
+    GHashTable * edit_table, GError ** error)
 {
-  gboolean negative = FALSE;
-  GstClockTime new_inpoint = _clock_time_minus_diff (element->inpoint,
-      data->offset, &negative);
-  if (negative || !GST_CLOCK_TIME_IS_VALID (new_inpoint)) {
-    GST_INFO_OBJECT (element, "Cannot trim start of %" GES_FORMAT
+  gboolean ret = FALSE;
+  GList *tmp;
+  GstClockTime duration_limit;
+  GstClockTime clip_inpoint;
+  GstClockTime new_start = clip_data->start;
+  gboolean no_core = FALSE;
+  GHashTable *child_inpoints;
+
+  child_inpoints = g_hash_table_new_full (NULL, NULL, gst_object_unref, g_free);
+
+  clip_inpoint = ges_clip_get_core_internal_time_from_timeline_time (clip,
+      new_start, &no_core, error);
+
+  if (no_core) {
+    GST_INFO_OBJECT (clip, "Clip %" GES_FORMAT " has no active core "
+        "children with an internal source. Not setting in-point during "
+        "trim to start", GES_ARGS (clip));
+    clip_inpoint = GES_TIMELINE_ELEMENT_INPOINT (clip);
+  } else if (!GST_CLOCK_TIME_IS_VALID (clip_inpoint)) {
+    GST_INFO_OBJECT (clip, "Cannot trim start of %" GES_FORMAT
         " with offset %" G_GINT64_FORMAT " because it would result in an "
-        "invalid in-point", GES_ARGS (element), data->offset);
-    if (negative)
-      set_negative_inpoint_error (error, element, new_inpoint);
-    return FALSE;
+        "invalid in-point for its core children", GES_ARGS (clip),
+        clip_data->offset);
+    goto done;
+  } else {
+    GST_INFO_OBJECT (clip, "Clip %" GES_FORMAT " will have its in-point "
+        " set to %" GST_TIME_FORMAT " because its start is being trimmed "
+        "to %" GST_TIME_FORMAT, GES_ARGS (clip),
+        GST_TIME_ARGS (clip_inpoint), GST_TIME_ARGS (new_start));
+    clip_data->inpoint = clip_inpoint;
   }
-  data->inpoint = new_inpoint;
-  return TRUE;
-}
-
-static gboolean
-set_edit_trim_start_non_core_children (GESTimelineElement * clip,
-    GstClockTimeDiff offset, GHashTable * edit_table, GError ** error)
-{
-  GList *tmp;
-  GESTimelineElement *child;
-  GESTrackElement *el;
-  EditData *data;
 
   /* need to set in-point of active non-core children to keep their
-   * internal content at the same timeline position. This also ensures
-   * the duration-limit will not be broken */
+   * internal content at the same timeline position */
   for (tmp = GES_CONTAINER_CHILDREN (clip); tmp; tmp = tmp->next) {
-    child = tmp->data;
-    el = tmp->data;
-    if (ges_track_element_has_internal_source (el)
-        && ges_track_element_is_active (el)
-        && !ges_track_element_is_core (el)) {
-
-      GST_INFO_OBJECT (child, "Setting track element %s to trim in-point "
-          "with offset %" G_GINT64_FORMAT " since the parent clip %"
-          GES_FORMAT " is being trimmed at its start", child->name, offset,
-          GES_ARGS (clip));
-
-      if (g_hash_table_contains (edit_table, child)) {
-        GST_ERROR_OBJECT (child, "Already set to be edited");
-        return FALSE;
+    GESTimelineElement *child = tmp->data;
+    GESTrackElement *el = tmp->data;
+    GstClockTime new_inpoint = child->inpoint;
+    GstClockTime *inpoint_p;
+
+    if (ges_track_element_has_internal_source (el)) {
+      if (ges_track_element_is_core (el)) {
+        new_inpoint = clip_inpoint;
+      } else if (ges_track_element_is_active (el)) {
+        EditData *data;
+
+        if (g_hash_table_contains (edit_table, child)) {
+          GST_ERROR_OBJECT (child, "Already set to be edited");
+          goto done;
+        }
+
+        new_inpoint = ges_clip_get_internal_time_from_timeline_time (clip, el,
+            new_start, error);
+
+        if (!GST_CLOCK_TIME_IS_VALID (new_inpoint)) {
+          GST_INFO_OBJECT (clip, "Cannot trim start of %" GES_FORMAT
+              " to %" GST_TIME_FORMAT " because it would result in an "
+              "invalid in-point for the non-core child %" GES_FORMAT,
+              GES_ARGS (clip), GST_TIME_ARGS (new_start), GES_ARGS (child));
+          goto done;
+        }
+
+        GST_INFO_OBJECT (child, "Setting track element %s to trim "
+            "in-point to %" GST_TIME_FORMAT " since the parent clip %"
+            GES_FORMAT " is being trimmed to start %" GST_TIME_FORMAT,
+            child->name, GST_TIME_ARGS (new_inpoint), GES_ARGS (clip),
+            GST_TIME_ARGS (new_start));
+
+        data = new_edit_data (EDIT_TRIM_INPOINT_ONLY, 0, 0);
+        data->inpoint = new_inpoint;
+        g_hash_table_insert (edit_table, child, data);
       }
+    }
 
-      data = new_edit_data (EDIT_TRIM_START, offset, 0);
-      g_hash_table_insert (edit_table, child, data);
-      if (!set_edit_trim_start_inpoint_value (child, data, error))
-        return FALSE;
+    if (GES_CLOCK_TIME_IS_LESS (child->maxduration, new_inpoint)) {
+      GST_INFO_OBJECT (clip, "Cannot trim start of %" GES_FORMAT
+          " to %" GST_TIME_FORMAT " because it would result in an "
+          "in-point of %" GST_TIME_FORMAT " for the child %" GES_FORMAT
+          ", which breaks its max-duration", GES_ARGS (clip),
+          GST_TIME_ARGS (new_start), GST_TIME_ARGS (new_inpoint),
+          GES_ARGS (child));
+
+      set_inpoint_breaks_max_duration_error (error, child, new_inpoint,
+          child->maxduration);
+      goto done;
     }
+
+    inpoint_p = g_new (GstClockTime, 1);
+    *inpoint_p = new_inpoint;
+    g_hash_table_insert (child_inpoints, gst_object_ref (child), inpoint_p);
   }
-  return TRUE;
+
+  duration_limit =
+      ges_clip_duration_limit_with_new_children_inpoints (clip, child_inpoints);
+
+  if (GES_CLOCK_TIME_IS_LESS (duration_limit, clip_data->duration)) {
+    GST_INFO_OBJECT (clip, "Cannot trim start of %" GES_FORMAT
+        " to %" GST_TIME_FORMAT " because it would result in a "
+        "duration of %" GST_TIME_FORMAT " that breaks its new "
+        "duration-limit of %" GST_TIME_FORMAT, GES_ARGS (clip),
+        GST_TIME_ARGS (new_start), GST_TIME_ARGS (clip_data->duration),
+        GST_TIME_ARGS (duration_limit));
+
+    set_breaks_duration_limit_error (error, clip, clip_data->duration,
+        duration_limit);
+    goto done;
+  }
+
+  ret = TRUE;
+
+done:
+  g_hash_table_unref (child_inpoints);
+
+  return ret;
 }
 
 /* trim the start of a clip or a track element */
@@ -1076,22 +1161,24 @@ set_edit_trim_start_values (GESTimelineElement * element, EditData * data,
     return TRUE;
 
   if (GES_IS_CLIP (element)) {
-    if (!set_edit_trim_start_inpoint_value (element, data, error))
-      return FALSE;
-    if (!set_edit_trim_start_non_core_children (element, data->offset,
+    if (!set_edit_trim_start_clip_inpoints (GES_CLIP (element), data,
             edit_table, error))
       return FALSE;
   } else if (GES_IS_TRACK_ELEMENT (element)
       && ges_track_element_has_internal_source (GES_TRACK_ELEMENT (element))) {
-    if (!set_edit_trim_start_inpoint_value (element, data, error))
+    GstClockTime new_inpoint =
+        _clock_time_minus_diff (element->inpoint, data->offset, &negative);
+
+    if (negative || !GST_CLOCK_TIME_IS_VALID (new_inpoint)) {
+      GST_INFO_OBJECT (element, "Cannot trim start of %" GES_FORMAT
+          " with offset %" G_GINT64_FORMAT " because it would result in "
+          "an invalid in-point", GES_ARGS (element), data->offset);
+      if (negative)
+        set_negative_inpoint_error (error, element, new_inpoint);
       return FALSE;
+    }
   }
 
-  /* NOTE: without time effects, the duration-limit will increase with
-   * a decrease in in-point by the same amount that duration increases,
-   * and vis-versa. So the new duration-limit should remain above the
-   * new duration */
-
   GST_INFO_OBJECT (element, "%s will trim start by setting start to %"
       GST_TIME_FORMAT ", in-point to %" GST_TIME_FORMAT " and duration "
       "to %" GST_TIME_FORMAT, element->name, GST_TIME_ARGS (data->start),
@@ -1155,6 +1242,9 @@ set_edit_values (GESTimelineElement * element, EditData * data,
       return set_edit_trim_start_values (element, data, edit_table, error);
     case EDIT_TRIM_END:
       return set_edit_trim_end_values (element, data, error);
+    case EDIT_TRIM_INPOINT_ONLY:
+      GST_ERROR_OBJECT (element, "Trim in-point only not handled");
+      return FALSE;
   }
   return FALSE;
 }
@@ -1504,6 +1594,9 @@ add_element_edit (GHashTable * edits, GESTimelineElement * element,
     case EDIT_TRIM_END:
       GST_LOG_OBJECT (element, "%s set to trim end", element->name);
       break;
+    case EDIT_TRIM_INPOINT_ONLY:
+      GST_ERROR_OBJECT (element, "%s set to trim in-point only", element->name);
+      return FALSE;
   }
 
   g_hash_table_insert (edits, element, new_edit_data (mode, 0, 0));
@@ -1674,6 +1767,11 @@ perform_element_edit (GESTimelineElement * element, EditData * edit)
           GST_TIME_ARGS (_END (element)),
           GST_TIME_ARGS (element->start + edit->duration));
       break;
+    case EDIT_TRIM_INPOINT_ONLY:
+      GST_INFO_OBJECT (element, "Trimming %s in-point from %"
+          GST_TIME_FORMAT " to %" GST_TIME_FORMAT, element->name,
+          GST_TIME_ARGS (element->inpoint), GST_TIME_ARGS (edit->inpoint));
+      break;
   }
 
   if (!GES_IS_CLIP (element) && !GES_IS_TRACK_ELEMENT (element)) {
index d438afa..d30df53 100644 (file)
@@ -577,7 +577,7 @@ class TestEditing(common.GESSimpleTimelineTest):
         self.assertEqual(effect3.inpoint, 20)
 
         self.assertTrue(
-            clip.edit([], -1, GES.EditMode.EDIT_TRIM, GES.Edge.EDGE_START, 5))
+            clip.edit_full(-1, GES.EditMode.EDIT_TRIM, GES.Edge.EDGE_START, 5))
 
         self.assertEqual(clip.start, 5)
         self.assertEqual(clip.duration, 15)
@@ -594,7 +594,7 @@ class TestEditing(common.GESSimpleTimelineTest):
         self.assertEqual(effect3.inpoint, 20)
 
         self.assertTrue(
-            clip.edit([], -1, GES.EditMode.EDIT_TRIM, GES.Edge.EDGE_START, 15))
+            clip.edit_full(-1, GES.EditMode.EDIT_TRIM, GES.Edge.EDGE_START, 15))
 
         self.assertEqual(clip.start, 15)
         self.assertEqual(clip.duration, 5)
@@ -610,6 +610,83 @@ class TestEditing(common.GESSimpleTimelineTest):
         self.assertEqual(effect2.inpoint, 13)
         self.assertEqual(effect3.inpoint, 20)
 
+    def test_trim_time_effects(self):
+        self.track_types = [GES.TrackType.VIDEO]
+        super().setUp()
+        clip = self.append_clip()
+        self.assertTrue(clip.set_inpoint(12))
+        self.assertTrue(clip.set_max_duration(30))
+        self.assertEqual(clip.get_duration_limit(), 18)
+
+        children = clip.get_children(False)
+        self.assertTrue(children)
+        self.assertEqual(len(children), 1)
+
+        source = children[0]
+        self.assertEqual(source.get_inpoint(), 12)
+        self.assertEqual(source.get_max_duration(), 30)
+
+        rate0 = GES.Effect.new("videorate rate=0.25")
+
+        overlay = GES.Effect.new("textoverlay")
+        overlay.set_has_internal_source(True)
+        self.assertTrue(overlay.set_inpoint(5))
+        self.assertTrue(overlay.set_max_duration(16))
+
+        rate1 = GES.Effect.new("videorate rate=2.0")
+
+        self.assertTrue(clip.add(rate0))
+        self.assertTrue(clip.add(overlay))
+        self.assertTrue(clip.add(rate1))
+
+        #                   source -> rate1 -> overlay -> rate0
+        # in-point/max-dur  12-30              5-16
+        # internal
+        # start/end         12-30     0-9      5-14       0-36
+        self.assertEqual(clip.get_duration_limit(), 36)
+        self.assertTrue(clip.set_start(40))
+        self.assertTrue(clip.set_duration(10))
+
+        # cannot trim to a 16 because overlay would have a negative in-point
+        error = None
+        try:
+            clip.edit_full(-1, GES.EditMode.EDIT_TRIM, GES.Edge.EDGE_START, 16)
+        except GLib.Error as err:
+            error = err
+        self.assertGESError(error, GES.Error.NEGATIVE_TIME)
+
+        self.assertEqual(clip.get_start(), 40)
+        self.assertEqual(clip.get_duration(), 10)
+        self.assertEqual(source.get_inpoint(), 12)
+        self.assertEqual(source.get_max_duration(), 30)
+        self.assertEqual(overlay.get_inpoint(), 5)
+        self.assertEqual(overlay.get_max_duration(), 16)
+
+        # trim backwards to 20
+        self.assertTrue(
+            clip.edit_full(-1, GES.EditMode.EDIT_TRIM, GES.Edge.EDGE_START, 20))
+
+        self.assertEqual(clip.get_start(), 20)
+        self.assertEqual(clip.get_duration(), 30)
+        # reduced by 10
+        self.assertEqual(source.get_inpoint(), 2)
+        self.assertEqual(source.get_max_duration(), 30)
+        # reduced by 5
+        self.assertEqual(overlay.get_inpoint(), 0)
+        self.assertEqual(overlay.get_max_duration(), 16)
+
+        # trim forwards to 28
+        self.assertTrue(
+            clip.edit_full(-1, GES.EditMode.EDIT_TRIM, GES.Edge.EDGE_START, 28))
+        self.assertEqual(clip.get_start(), 28)
+        self.assertEqual(clip.get_duration(), 22)
+        # increased by 4
+        self.assertEqual(source.get_inpoint(), 6)
+        self.assertEqual(source.get_max_duration(), 30)
+        # increased by 2
+        self.assertEqual(overlay.get_inpoint(), 2)
+        self.assertEqual(overlay.get_max_duration(), 16)
+
     def test_ripple_end(self):
         clip = self.append_clip()
         clip.set_max_duration(20)