From 5c546c6fe7c51d4949fd46f83d4622aa7a236bfa Mon Sep 17 00:00:00 2001 From: Henry Wilkes Date: Tue, 21 Apr 2020 12:55:34 +0100 Subject: [PATCH] clip: change order of split We first change the duration of the splitted clip, then we add the new clip to the layer and assign the tracks for its children. Normally, when a clip is added to a layer it will have its track elements created, if needed, and then assigned to their tracks. This will fail if any sources would fully or triple overlap existing sources in the same track. However, here we were adding the clip to the layer *and* avoiding the track assignment process and instead setting the tracks explicitly. In particular, the order was: + add new clip to layer with no tracks assigned + shrink the split clip + assign the tracks for the new clip This has been changed to: + shrink the split clip + add new clip to layer with no tracks assigned + assign the tracks for the new clip Thus, the order of events for any users connecting to object signals will be close to that of adding another clip to the layer. Part-of: --- ges/ges-clip.c | 15 +++-- tests/check/ges/clip.c | 144 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 7 deletions(-) diff --git a/ges/ges-clip.c b/ges/ges-clip.c index b93401cceb..1fbb6521de 100644 --- a/ges/ges-clip.c +++ b/ges/ges-clip.c @@ -2268,13 +2268,8 @@ ges_clip_split (GESClip * clip, guint64 position) inpoint + old_duration * media_duration_factor); _set_duration0 (GES_TIMELINE_ELEMENT (new_object), new_duration); - /* We do not want the timeline to create again TrackElement-s */ - ges_clip_set_moving_from_layer (new_object, TRUE); - /* adding to the same layer should not fail when moving */ - ges_layer_add_clip (clip->priv->layer, new_object); - ges_clip_set_moving_from_layer (new_object, FALSE); - - /* split binding before duration changes */ + /* split binding before duration changes since shrinking can destroy + * binding values */ track_for_copy = g_hash_table_new_full (NULL, NULL, gst_object_unref, gst_object_unref); /* _add_child will add core elements at the lowest priority and new @@ -2298,6 +2293,12 @@ ges_clip_split (GESClip * clip, guint64 position) _set_duration0 (GES_TIMELINE_ELEMENT (clip), old_duration); GES_TIMELINE_ELEMENT_UNSET_BEING_EDITED (clip); + /* We do not want the timeline to create again TrackElement-s */ + ges_clip_set_moving_from_layer (new_object, TRUE); + /* adding to the same layer should not fail when moving */ + ges_layer_add_clip (clip->priv->layer, new_object); + ges_clip_set_moving_from_layer (new_object, FALSE); + /* add to the track after the duration change so we don't overlap! */ for (tmp = GES_CONTAINER_CHILDREN (new_object); tmp; tmp = tmp->next) { GESTrackElement *copy = tmp->data; diff --git a/tests/check/ges/clip.c b/tests/check/ges/clip.c index a053c45840..1e7b29b7e0 100644 --- a/tests/check/ges/clip.c +++ b/tests/check/ges/clip.c @@ -518,6 +518,149 @@ GST_START_TEST (test_split_object) GST_END_TEST; +typedef struct +{ + gboolean duration_cb_called; + gboolean clip_added_cb_called; + gboolean track_selected_cb_called; + GESClip *clip; +} SplitOrderData; + +static void +_track_selected_cb (GESTimelineElement * el, GParamSpec * spec, + SplitOrderData * data) +{ + GESClip *clip = GES_CLIP (el->parent); + + fail_unless (data->clip == clip, "Parent is %" GES_FORMAT " rather than %" + GES_FORMAT, GES_ARGS (clip), GES_ARGS (data->clip)); + + fail_unless (data->duration_cb_called, "notify::duration not emitted " + "for neighbour of %" GES_FORMAT, GES_ARGS (data->clip)); + fail_unless (data->clip_added_cb_called, "child-added not emitted for %" + GES_FORMAT, GES_ARGS (data->clip)); + + data->track_selected_cb_called = TRUE; +} + +static void +_child_added_cb (GESClip * clip, GESTimelineElement * child, + SplitOrderData * data) +{ + fail_unless (data->clip == clip, "Received %" GES_FORMAT " rather than %" + GES_FORMAT, GES_ARGS (clip), GES_ARGS (data->clip)); + + g_signal_connect (child, "notify::track", G_CALLBACK (_track_selected_cb), + data); +} + +static void +_clip_added_cb (GESLayer * layer, GESClip * clip, SplitOrderData * data) +{ + GList *tmp; + + data->clip = clip; + + fail_unless (data->duration_cb_called, "notify::duration not emitted " + "for neighbour of %" GES_FORMAT, GES_ARGS (data->clip)); + /* only called once */ + fail_if (data->clip_added_cb_called, "clip-added already emitted for %" + GES_FORMAT, GES_ARGS (data->clip)); + fail_if (data->track_selected_cb_called, "track selection already " + "occurred for %" GES_FORMAT, GES_ARGS (data->clip)); + + data->clip_added_cb_called = TRUE; + + g_signal_connect (clip, "child-added", G_CALLBACK (_child_added_cb), data); + + for (tmp = GES_CONTAINER_CHILDREN (clip); tmp; tmp = tmp->next) + g_signal_connect (tmp->data, "notify::track", + G_CALLBACK (_track_selected_cb), data); +} + +static void +_disconnect_cbs (GESLayer * layer, GESClip * clip, SplitOrderData * data) +{ + GList *tmp; + g_signal_handlers_disconnect_by_func (clip, _child_added_cb, data); + for (tmp = GES_CONTAINER_CHILDREN (clip); tmp; tmp = tmp->next) + g_signal_handlers_disconnect_by_func (tmp->data, _track_selected_cb, data); +} + +static void +_duration_cb (GObject * object, GParamSpec * pspec, SplitOrderData * data) +{ + /* only called once */ + fail_if (data->duration_cb_called, "notify::duration of neighbour %" + GES_FORMAT " already emitted ", GES_ARGS (object)); + fail_if (data->clip_added_cb_called, "clip-added already emitted"); + fail_if (data->track_selected_cb_called, "track selection already " + "occurred"); + + data->duration_cb_called = TRUE; +} + +GST_START_TEST (test_split_ordering) +{ + GESTimeline *timeline; + GESLayer *layer; + GESClip *clip, *splitclip; + SplitOrderData data; + + ges_init (); + + timeline = ges_timeline_new_audio_video (); + + layer = ges_timeline_append_layer (timeline); + + clip = GES_CLIP (ges_test_clip_new ()); + assert_set_duration (clip, 10); + + /* test order when adding clip to a layer */ + /* don't care about duration yet */ + data.duration_cb_called = TRUE; + data.clip_added_cb_called = FALSE; + data.track_selected_cb_called = FALSE; + data.clip = NULL; + + g_signal_connect (layer, "clip-added", G_CALLBACK (_clip_added_cb), &data); + + fail_unless (ges_layer_add_clip (layer, clip)); + + fail_unless (data.duration_cb_called); + fail_unless (data.clip_added_cb_called); + fail_unless (data.track_selected_cb_called); + fail_unless (data.clip == clip); + + /* now check for the same ordering when splitting, which the original + * clip shrinking before the new one is added to the layer */ + data.duration_cb_called = FALSE; + data.clip_added_cb_called = FALSE; + data.track_selected_cb_called = FALSE; + data.clip = NULL; + + g_signal_connect (clip, "notify::duration", G_CALLBACK (_duration_cb), &data); + + splitclip = ges_clip_split (clip, 5); + + fail_unless (splitclip); + fail_unless (data.duration_cb_called); + fail_unless (data.clip_added_cb_called); + fail_unless (data.track_selected_cb_called); + fail_unless (data.clip == splitclip); + + /* disconnect since track of children will change when timeline is + * freed */ + _disconnect_cbs (layer, clip, &data); + _disconnect_cbs (layer, splitclip, &data); + + gst_object_unref (timeline); + + ges_deinit (); +} + +GST_END_TEST; + #define _assert_higher_priority(el, higher) \ { \ if (higher) { \ @@ -3183,6 +3326,7 @@ ges_suite (void) tcase_add_test (tc_chain, test_object_properties); tcase_add_test (tc_chain, test_split_object); + tcase_add_test (tc_chain, test_split_ordering); tcase_add_test (tc_chain, test_split_direct_bindings); tcase_add_test (tc_chain, test_split_direct_absolute_bindings); tcase_add_test (tc_chain, test_clip_group_ungroup);