From a93e8734028937eff8500cc2700ea214247cc1e3 Mon Sep 17 00:00:00 2001
From: Henry Wilkes <hwilkes@igalia.com>
Date: Wed, 25 Mar 2020 19:35:11 +0000
Subject: [PATCH] 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 | 53 ++++++++++++++++------------
ges/ges-uri-clip.c | 40 ++++++++++++----------
tests/check/ges/clip.c | 78 +++++++++++++++++++++++++++---------------
3 files changed, 103 insertions(+), 68 deletions(-)
diff --git a/ges/ges-clip.c b/ges/ges-clip.c
index 9bc86a31ee..7e6c295160 100644
--- a/ges/ges-clip.c
+++ b/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 (!ges_timeline_element_set_max_duration (child, maxduration)) {
- GST_ERROR_OBJECT ("Could not set the max-duration of child %"
- GES_FORMAT " to %" GST_TIME_FORMAT, GES_ARGS (child),
- GST_TIME_ARGS (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;
+ if (_IS_CORE_CHILD (child)) {
+ has_core = TRUE;
+ if (ges_track_element_has_internal_source (GES_TRACK_ELEMENT (child))) {
+ if (!ges_timeline_element_set_max_duration (child, maxduration))
+ GST_ERROR_OBJECT ("Could not set the max-duration of child %"
+ GES_FORMAT " to %" GST_TIME_FORMAT, GES_ARGS (child),
+ GST_TIME_ARGS (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
diff --git a/ges/ges-uri-clip.c b/ges/ges-uri-clip.c
index 39a6d7033f..7b0a535fc4 100644
--- a/ges/ges-uri-clip.c
+++ b/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) {
- GList *children = ges_container_get_children (GES_CONTAINER (self), TRUE);
+ children = ges_container_get_children (GES_CONTAINER (self), TRUE);
- for (tmp = children; tmp; tmp = tmp->next) {
- if (GES_IS_SOURCE (tmp->data)) {
- GESTrack *track = ges_track_element_get_track (tmp->data);
+ for (tmp = children; tmp; tmp = tmp->next) {
+ if (GES_IS_SOURCE (tmp->data)) {
+ GESTrack *track = ges_track_element_get_track (tmp->data);
- if (track->type == GES_TRACK_TYPE_AUDIO)
- audio_source = gst_object_ref (tmp->data);
- else if (track->type == GES_TRACK_TYPE_VIDEO)
- video_source = gst_object_ref (tmp->data);
+ if (track->type == GES_TRACK_TYPE_AUDIO)
+ audio_source = gst_object_ref (tmp->data);
+ else if (track->type == GES_TRACK_TYPE_VIDEO)
+ video_source = gst_object_ref (tmp->data);
- ges_track_remove_element (track, tmp->data);
- ges_container_remove (GES_CONTAINER (self), tmp->data);
- }
+ ges_track_remove_element (track, 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));
diff --git a/tests/check/ges/clip.c b/tests/check/ges/clip.c
index 2d9252dd5b..c231b11a4a 100644
--- a/tests/check/ges/clip.c
+++ b/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);