clip: allow arbitrary max-duration when no core children

Before the max-duration could be set arbitrarily when the clip was empty,
to indicate what the max-duration would be once the core children were
created. Now, we can also do this whilst the clip only contains non-core
children.

ges/ges-clip.c

@@ -54,11 +54,15 @@
  * The #GESTimelineElement:in-point of the clip will control the
  * #GESTimelineElement:in-point of these core elements to be the same
  * value if their #GESTrackElement:has-internal-source is set to %TRUE.
+ *
  * The #GESTimelineElement:max-duration of the clip is the minimum
  * #GESTimelineElement:max-duration of its children. If you set its value
  * to anything other than its current value, this will also set the
  * #GESTimelineElement:max-duration of all its core children to the same
  * value if their #GESTrackElement:has-internal-source is set to %TRUE.
+ * As a special case, whilst a clip does not yet have any core children,
+ * its #GESTimelineElement:max-duration may be set to indicate what its
+ * value will be once they are created.
  *
  * ## Effects
  *
@@ -397,41 +401,45 @@ _set_max_duration (GESTimelineElement * element, GstClockTime maxduration)
   GList *tmp;
   GESClipPrivate *priv = GES_CLIP (element)->priv;
   GstClockTime new_min = GST_CLOCK_TIME_NONE;
+  gboolean has_core = FALSE;
 
   /* if we are setting based on a change in the minimum */
   if (priv->updating_max_duration)
     return TRUE;
 
-  if (!GES_CONTAINER_CHILDREN (element)) {
-    /* If any child added later on has a lower max duration, this max duration
-     * will be used instead anyway */
-    GST_INFO_OBJECT (element,
-        "Setting max duration %" GST_TIME_FORMAT " as %" GES_FORMAT
-        " doesn't have any child yet",
-        GST_TIME_ARGS (maxduration), GES_ARGS (element));
-    return TRUE;
-  }
-
   /* else, we set every core child to have the same max duration */
 
   priv->prevent_max_duration_update = TRUE;
   for (tmp = GES_CONTAINER_CHILDREN (element); tmp; tmp = tmp->next) {
     GESTimelineElement *child = tmp->data;
 
-    if (_IS_CORE_INTERNAL_SOURCE_CHILD (child)) {
+    if (_IS_CORE_CHILD (child)) {
-      if (!ges_timeline_element_set_max_duration (child, maxduration)) {
+      has_core = TRUE;
-        GST_ERROR_OBJECT ("Could not set the max-duration of child %"
+      if (ges_track_element_has_internal_source (GES_TRACK_ELEMENT (child))) {
-            GES_FORMAT " to %" GST_TIME_FORMAT, GES_ARGS (child),
+        if (!ges_timeline_element_set_max_duration (child, maxduration))
-            GST_TIME_ARGS (maxduration));
+          GST_ERROR_OBJECT ("Could not set the max-duration of child %"
-      }
+              GES_FORMAT " to %" GST_TIME_FORMAT, GES_ARGS (child),
-      if (GST_CLOCK_TIME_IS_VALID (child->maxduration)) {
+              GST_TIME_ARGS (maxduration));
-        new_min = GST_CLOCK_TIME_IS_VALID (new_min) ?
+
-            MIN (new_min, child->maxduration) : child->maxduration;
+        if (GST_CLOCK_TIME_IS_VALID (child->maxduration))
+          new_min = GST_CLOCK_TIME_IS_VALID (new_min) ?
+              MIN (new_min, child->maxduration) : child->maxduration;
       }
     }
   }
   priv->prevent_max_duration_update = FALSE;
 
+  if (!has_core) {
+    /* allow max-duration to be set arbitrarily when we have no
+     * core children, even though there is no actual minimum max-duration
+     * when it has no core children */
+    if (GST_CLOCK_TIME_IS_VALID (maxduration))
+      GST_INFO_OBJECT (element,
+          "Allowing max-duration of the clip to be set to %" GST_TIME_FORMAT
+          " because it has no core children", GST_TIME_ARGS (maxduration));
+    return TRUE;
+  }
+
   if (new_min != maxduration) {
     if (GST_CLOCK_TIME_IS_VALID (new_min))
       GST_WARNING_OBJECT (element, "Failed to set the max-duration of the "
@@ -703,7 +711,9 @@ _child_added (GESContainer * container, GESTimelineElement * element)
       G_CALLBACK (_child_has_internal_source_changed_cb), container);
 
   _child_priority_changed_cb (element, NULL, container);
-  _update_max_duration (container);
+
+  if (_IS_CORE_CHILD (element))
+    _update_max_duration (container);
 }
 
 static void
@@ -718,7 +728,8 @@ _child_removed (GESContainer * container, GESTimelineElement * element)
   g_signal_handlers_disconnect_by_func (element,
       _child_has_internal_source_changed_cb, container);
 
-  _update_max_duration (container);
+  if (_IS_CORE_CHILD (element))
+    _update_max_duration (container);
 }
 
 static void

ges/ges-uri-clip.c

@@ -227,12 +227,12 @@ extractable_get_id (GESExtractable * self)
 static gboolean
 extractable_set_asset (GESExtractable * self, GESAsset * asset)
 {
-  gboolean res = TRUE;
+  gboolean res = TRUE, contains_core;
   GESUriClip *uriclip = GES_URI_CLIP (self);
   GESUriClipAsset *uri_clip_asset;
   GESClip *clip = GES_CLIP (self);
   GESLayer *layer = ges_clip_get_layer (clip);
-  GList *tmp;
+  GList *tmp, *children;
   GESTimelineElement *audio_source = NULL, *video_source = NULL;
 
   g_return_val_if_fail (GES_IS_URI_CLIP_ASSET (asset), FALSE);
@@ -270,24 +270,26 @@ extractable_set_asset (GESExtractable * self, GESAsset * asset)
 
   GES_TIMELINE_ELEMENT (uriclip)->asset = asset;
 
-  if (layer) {
+  children = ges_container_get_children (GES_CONTAINER (self), TRUE);
-    GList *children = ges_container_get_children (GES_CONTAINER (self), TRUE);
 
-    for (tmp = children; tmp; tmp = tmp->next) {
+  for (tmp = children; tmp; tmp = tmp->next) {
-      if (GES_IS_SOURCE (tmp->data)) {
+    if (GES_IS_SOURCE (tmp->data)) {
-        GESTrack *track = ges_track_element_get_track (tmp->data);
+      GESTrack *track = ges_track_element_get_track (tmp->data);
 
-        if (track->type == GES_TRACK_TYPE_AUDIO)
+      if (track->type == GES_TRACK_TYPE_AUDIO)
-          audio_source = gst_object_ref (tmp->data);
+        audio_source = gst_object_ref (tmp->data);
-        else if (track->type == GES_TRACK_TYPE_VIDEO)
+      else if (track->type == GES_TRACK_TYPE_VIDEO)
-          video_source = gst_object_ref (tmp->data);
+        video_source = gst_object_ref (tmp->data);
 
-        ges_track_remove_element (track, tmp->data);
+      ges_track_remove_element (track, tmp->data);
-        ges_container_remove (GES_CONTAINER (self), tmp->data);
+      ges_container_remove (GES_CONTAINER (self), tmp->data);
-      }
     }
-    g_list_free_full (children, g_object_unref);
+  }
+  g_list_free_full (children, g_object_unref);
 
+  contains_core = FALSE;
+
+  if (layer) {
     gst_object_ref (clip);
 
     ges_layer_remove_clip (layer, clip);
@@ -296,6 +298,7 @@ extractable_set_asset (GESExtractable * self, GESAsset * asset)
     for (tmp = GES_CONTAINER_CHILDREN (self); tmp; tmp = tmp->next) {
       if (GES_IS_SOURCE (tmp->data)) {
         GESTrack *track = ges_track_element_get_track (tmp->data);
+        contains_core = TRUE;
 
         if (track->type == GES_TRACK_TYPE_AUDIO && audio_source) {
           ges_track_element_copy_properties (audio_source, tmp->data);
@@ -308,19 +311,18 @@ extractable_set_asset (GESExtractable * self, GESAsset * asset)
         }
       }
     }
-
-    g_clear_object (&audio_source);
-    g_clear_object (&video_source);
     gst_object_unref (clip);
     gst_object_unref (layer);
   }
+  g_clear_object (&audio_source);
+  g_clear_object (&video_source);
 
   if (res) {
     g_free (uriclip->priv->uri);
     uriclip->priv->uri = g_strdup (ges_asset_get_id (asset));
   }
 
-  if (!GES_CONTAINER_CHILDREN (uriclip))
+  if (!contains_core)
     ges_timeline_element_set_max_duration (GES_TIMELINE_ELEMENT (uriclip),
         ges_uri_clip_asset_get_max_duration (uri_clip_asset));
 

tests/check/ges/clip.c

@@ -1083,38 +1083,13 @@ GST_START_TEST (test_children_max_duration)
     fail_unless (ges_timeline_element_set_start (clip, 5));
     fail_unless (ges_timeline_element_set_duration (clip, 20));
     fail_unless (ges_timeline_element_set_inpoint (clip, 30));
-    /* can not the max duration the clip has no child */
+
+    /* can set the max duration the clip to anything whilst it has
+     * no core child */
     fail_unless (ges_timeline_element_set_max_duration (clip, 150));
 
     CHECK_OBJECT_PROPS_MAX (clip, 5, 30, 20, 150);
 
-    fail_unless (ges_layer_add_clip (layer, GES_CLIP (clip)));
-
-    /* clip now has children */
-    children = GES_CONTAINER_CHILDREN (clip);
-    fail_unless (children);
-    child0 = children->data;
-    fail_unless (children->next);
-    child1 = children->next->data;
-    fail_unless (children->next->next == NULL);
-
-    fail_unless (ges_track_element_has_internal_source (GES_TRACK_ELEMENT
-            (child0)));
-    fail_unless (ges_track_element_has_internal_source (GES_TRACK_ELEMENT
-            (child1)));
-
-    if (GES_IS_URI_CLIP (clip))
-      new_max = max_duration;
-    else
-      /* need a valid clock time that is not too large */
-      new_max = 500;
-
-    /* added children do not change the clip's max-duration, but will
-     * instead set it to the minimum value of its children */
-    CHECK_OBJECT_PROPS_MAX (clip, 5, 30, 20, max_duration);
-    CHECK_OBJECT_PROPS_MAX (child0, 5, 30, 20, max_duration);
-    CHECK_OBJECT_PROPS_MAX (child1, 5, 30, 20, max_duration);
-
     /* add a non-core element */
     effect = GES_TIMELINE_ELEMENT (ges_effect_new ("agingtv"));
     fail_if (ges_track_element_has_internal_source (GES_TRACK_ELEMENT
@@ -1130,6 +1105,53 @@ GST_START_TEST (test_children_max_duration)
      * max-duration (or in-point) */
     fail_unless (ges_container_add (GES_CONTAINER (clip), effect));
 
+    CHECK_OBJECT_PROPS_MAX (clip, 5, 30, 20, 150);
+    CHECK_OBJECT_PROPS_MAX (effect, 5, 0, 20, 400);
+
+    /* only non-core, so can still set the max-duration */
+    fail_unless (ges_timeline_element_set_max_duration (clip, 200));
+
+    CHECK_OBJECT_PROPS_MAX (clip, 5, 30, 20, 200);
+    CHECK_OBJECT_PROPS_MAX (effect, 5, 0, 20, 400);
+
+    /* removing should not change the max-duration we set on the clip */
+    gst_object_ref (effect);
+    fail_unless (ges_container_remove (GES_CONTAINER (clip), effect));
+
+    CHECK_OBJECT_PROPS_MAX (clip, 5, 30, 20, 200);
+    CHECK_OBJECT_PROPS_MAX (effect, 5, 0, 20, 400);
+
+    fail_unless (ges_container_add (GES_CONTAINER (clip), effect));
+    gst_object_unref (effect);
+
+    CHECK_OBJECT_PROPS_MAX (clip, 5, 30, 20, 200);
+    CHECK_OBJECT_PROPS_MAX (effect, 5, 0, 20, 400);
+
+    /* now add to a layer to create the core children */
+    fail_unless (ges_layer_add_clip (layer, GES_CLIP (clip)));
+
+    children = GES_CONTAINER_CHILDREN (clip);
+    fail_unless (children);
+    fail_unless (GES_TIMELINE_ELEMENT (children->data) == effect);
+    fail_unless (children->next);
+    child0 = children->next->data;
+    fail_unless (children->next->next);
+    child1 = children->next->next->data;
+    fail_unless (children->next->next->next == NULL);
+
+    fail_unless (ges_track_element_has_internal_source (GES_TRACK_ELEMENT
+            (child0)));
+    fail_unless (ges_track_element_has_internal_source (GES_TRACK_ELEMENT
+            (child1)));
+
+    if (GES_IS_URI_CLIP (clip))
+      new_max = max_duration;
+    else
+      /* need a valid clock time that is not too large */
+      new_max = 500;
+
+    /* added children do not change the clip's max-duration, but will
+     * instead set it to the minimum value of its children */
     CHECK_OBJECT_PROPS_MAX (clip, 5, 30, 20, max_duration);
     CHECK_OBJECT_PROPS_MAX (child0, 5, 30, 20, max_duration);
     CHECK_OBJECT_PROPS_MAX (child1, 5, 30, 20, max_duration);