timeline: fix adding track when layers contains clips

Made sure that adding a new track only uses select-tracks-for-object for
core children to determine whether a track elements should be added to the
new track or not, and *not* any other track. In particular, there should
be *no* change in the existing tracks of the timeline when adding another
track. Moreover, a new track should not invoke the creation of track
elements for other tracks.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-editing-services/-/merge_requests/160>
This commit is contained in:
Henry Wilkes 2020-04-13 11:40:55 +01:00 committed by GStreamer Merge Bot
parent dc9dbddbae
commit 4daa0ecba4
2 changed files with 222 additions and 40 deletions

View file

@ -151,6 +151,7 @@ struct _GESTimelinePrivate
gboolean needs_transitions_update;
GESTrack *auto_transition_track;
GESTrack *new_track;
/* While we are creating and adding the TrackElements for a clip, we need to
* ignore the child-added signal */
@ -348,6 +349,7 @@ ges_timeline_dispose (GObject * object)
gst_object_unref (priv->stream_collection);
gst_clear_object (&priv->auto_transition_track);
gst_clear_object (&priv->new_track);
G_OBJECT_CLASS (ges_timeline_parent_class)->dispose (object);
}
@ -1544,7 +1546,7 @@ static void
clip_track_element_added_cb (GESClip * clip,
GESTrackElement * track_element, GESTimeline * timeline)
{
GESTrack *auto_trans_track;
GESTrack *auto_trans_track, *new_track;
gboolean error = FALSE;
if (timeline->priv->track_elements_moving) {
@ -1566,6 +1568,8 @@ clip_track_element_added_cb (GESClip * clip,
* should be used exactly once! */
auto_trans_track = timeline->priv->auto_transition_track;
timeline->priv->auto_transition_track = NULL;
/* don't take ownership of new_track */
new_track = timeline->priv->new_track;
UNLOCK_DYN (timeline);
if (auto_trans_track) {
@ -1575,8 +1579,12 @@ clip_track_element_added_cb (GESClip * clip,
error = TRUE;
gst_object_unref (auto_trans_track);
} else {
if (!_add_track_element_to_tracks (timeline, clip, track_element))
error = TRUE;
if (new_track)
error =
!_try_add_track_element_to_track (timeline, clip, track_element,
new_track);
else
error = !_add_track_element_to_tracks (timeline, clip, track_element);
}
if (error)
@ -1639,7 +1647,8 @@ _add_clip_children_to_tracks (GESTimeline * timeline, GESClip * clip,
/* returns TRUE if no errors in adding to tracks */
static gboolean
add_object_to_tracks (GESTimeline * timeline, GESClip * clip, GESTrack * track)
add_object_to_tracks (GESTimeline * timeline, GESClip * clip,
GESTrack * new_track)
{
GList *tracks, *tmp, *list, *created, *just_added = NULL;
gboolean no_errors = TRUE;
@ -1652,10 +1661,13 @@ add_object_to_tracks (GESTimeline * timeline, GESClip * clip, GESTrack * track)
LOCK_DYN (timeline);
tracks =
g_list_copy_deep (timeline->tracks, (GCopyFunc) gst_object_ref, NULL);
timeline->priv->new_track = new_track ? gst_object_ref (new_track) : NULL;
UNLOCK_DYN (timeline);
/* create core elements */
for (tmp = tracks; tmp; tmp = tmp->next) {
GESTrack *track = GES_TRACK (tmp->data);
if (new_track && track != new_track)
continue;
list = ges_clip_create_track_elements (clip, track->type);
for (created = list; created; created = created->next) {
GESTimelineElement *el = created->data;
@ -1693,11 +1705,17 @@ add_object_to_tracks (GESTimeline * timeline, GESClip * clip, GESTrack * track)
* non-core can not be in a track by itself) */
/* include just_added as a blacklist to ensure we do not try the track
* selection a second time when track selection returns no tracks */
if (!_add_clip_children_to_tracks (timeline, clip, TRUE, track, just_added))
if (!_add_clip_children_to_tracks (timeline, clip, TRUE, new_track,
just_added))
no_errors = FALSE;
if (!_add_clip_children_to_tracks (timeline, clip, FALSE, track, just_added))
if (!_add_clip_children_to_tracks (timeline, clip, FALSE, new_track,
just_added))
no_errors = FALSE;
LOCK_DYN (timeline);
gst_clear_object (&timeline->priv->new_track);
UNLOCK_DYN (timeline);
g_list_free (just_added);
return no_errors;

View file

@ -256,13 +256,68 @@ GST_END_TEST;
/* this time we add the layer before we add the track. */
#define _assert_child_in_track(clip, child_type, track) \
{ \
GESTrackElement *el; \
GList *tmp = ges_clip_find_track_elements (clip, NULL, \
GES_TRACK_TYPE_UNKNOWN, child_type); \
fail_unless (g_list_length (tmp), 1); \
el = tmp->data; \
g_list_free_full (tmp, gst_object_unref); \
fail_unless (ges_track_element_get_track (el) == track); \
if (track) \
ASSERT_OBJECT_REFCOUNT (el, "1 clip + 1 track + 1 timeline", 3); \
else \
ASSERT_OBJECT_REFCOUNT (el, "1 clip", 1); \
}
#define _assert_no_child_in_track(clip, track) \
fail_if (ges_clip_find_track_elements (clip, track, \
GES_TRACK_TYPE_UNKNOWN, G_TYPE_NONE));
#define _assert_add_track(timeline, track) \
{ \
GList *tmp; \
GST_DEBUG ("Adding " #track " to the timeline"); \
fail_unless (ges_timeline_add_track (timeline, track)); \
ASSERT_OBJECT_REFCOUNT (track, #track, 1); \
fail_unless (ges_track_get_timeline (track) == timeline); \
fail_unless ((gpointer) GST_ELEMENT_PARENT (track) \
== (gpointer) timeline); \
tmp = ges_timeline_get_tracks (timeline); \
fail_unless (g_list_find (tmp, track), #track " not found in tracks"); \
g_list_free_full (tmp, gst_object_unref); \
}
#define _remove_sources(clip, expect_num) \
{ \
GList *tmp, *els; \
els = ges_clip_find_track_elements (clip, NULL, GES_TRACK_TYPE_UNKNOWN, \
GES_TYPE_SOURCE); \
assert_equals_int (g_list_length (els), expect_num); \
for (tmp = els; tmp; tmp = tmp->next) \
fail_unless (ges_container_remove (GES_CONTAINER (clip), tmp->data)); \
g_list_free_full (els, gst_object_unref); \
}
#define _remove_from_track(clip, track, expect_num) \
{ \
GList *tmp, *els; \
els = ges_clip_find_track_elements (clip, track, GES_TRACK_TYPE_UNKNOWN, \
G_TYPE_NONE); \
assert_equals_int (g_list_length (els), expect_num); \
for (tmp = els; tmp; tmp = tmp->next) \
fail_unless (ges_track_remove_element (track, tmp->data)); \
g_list_free_full (els, gst_object_unref); \
}
GST_START_TEST (test_ges_timeline_add_layer_first)
{
GESTimeline *timeline;
GESLayer *layer;
GESTrack *track;
GESTrack *track, *track1, *track2, *track3;
GESClip *s1, *s2, *s3;
GList *trackelements, *tmp, *layers;
GList *layers;
ges_init ();
@ -283,6 +338,12 @@ GST_START_TEST (test_ges_timeline_add_layer_first)
_CREATE_SOURCE (layer, s2, 20, 10);
_CREATE_SOURCE (layer, s3, 40, 10);
fail_unless (ges_container_add (GES_CONTAINER (s1),
GES_TIMELINE_ELEMENT (ges_effect_new ("agingtv"))));
assert_num_children (s1, 1);
assert_num_children (s2, 0);
assert_num_children (s3, 0);
GST_DEBUG ("Add the layer to the timeline");
fail_unless (ges_timeline_add_layer (timeline, layer));
/* The timeline steals our reference to the layer */
@ -292,42 +353,145 @@ GST_START_TEST (test_ges_timeline_add_layer_first)
fail_unless (g_list_find (layers, layer) != NULL);
g_list_free_full (layers, gst_object_unref);
GST_DEBUG ("Add the track to the timeline");
fail_unless (ges_timeline_add_track (timeline, track));
ASSERT_OBJECT_REFCOUNT (track, "track", 1);
fail_unless (ges_track_get_timeline (track) == timeline);
fail_unless ((gpointer) GST_ELEMENT_PARENT (track) == (gpointer) timeline);
/* core children not created yet since no tracks */
assert_num_children (s1, 1);
assert_num_children (s2, 0);
assert_num_children (s3, 0);
_assert_add_track (timeline, track);
/* 3 sources, 1 effect */
assert_num_in_track (track, 4);
/* Make sure the associated TrackElements are in the Track */
trackelements = GES_CONTAINER_CHILDREN (s1);
fail_unless (trackelements != NULL);
for (tmp = trackelements; tmp; tmp = tmp->next) {
/* Each object has 3 references:
* 1 by the clip
* 1 by the track
* 1 by the timeline */
ASSERT_OBJECT_REFCOUNT (GES_TRACK_ELEMENT (tmp->data), "trackelement", 3);
}
assert_num_children (s1, 2);
_assert_child_in_track (s1, GES_TYPE_EFFECT, track);
_assert_child_in_track (s1, GES_TYPE_VIDEO_TEST_SOURCE, track);
trackelements = GES_CONTAINER_CHILDREN (s2);
fail_unless (trackelements != NULL);
for (tmp = trackelements; tmp; tmp = tmp->next) {
/* Each object has 3 references:
* 1 by the clip
* 1 by the track
* 1 by the timeline */
ASSERT_OBJECT_REFCOUNT (GES_TRACK_ELEMENT (tmp->data), "trackelement", 3);
}
assert_num_children (s2, 1);
_assert_child_in_track (s2, GES_TYPE_VIDEO_TEST_SOURCE, track);
trackelements = GES_CONTAINER_CHILDREN (s3);
fail_unless (trackelements != NULL);
for (tmp = trackelements; tmp; tmp = tmp->next) {
/* Each object has 3 references:
* 1 by the clip
* 1 by the track
* 1 by the timeline */
ASSERT_OBJECT_REFCOUNT (GES_TRACK_ELEMENT (tmp->data), "trackelement", 3);
}
assert_num_children (s3, 1);
_assert_child_in_track (s3, GES_TYPE_VIDEO_TEST_SOURCE, track);
/* adding an audio track should create new audio sources */
track1 = GES_TRACK (ges_audio_track_new ());
_assert_add_track (timeline, track1);
/* other track stays the same */
assert_num_in_track (track, 4);
/* 3 sources */
assert_num_in_track (track1, 3);
/* one new core child */
assert_num_children (s1, 3);
_assert_child_in_track (s1, GES_TYPE_EFFECT, track);
_assert_child_in_track (s1, GES_TYPE_VIDEO_TEST_SOURCE, track);
_assert_child_in_track (s1, GES_TYPE_AUDIO_TEST_SOURCE, track1);
assert_num_children (s2, 2);
_assert_child_in_track (s2, GES_TYPE_VIDEO_TEST_SOURCE, track);
_assert_child_in_track (s2, GES_TYPE_AUDIO_TEST_SOURCE, track1);
assert_num_children (s3, 2);
_assert_child_in_track (s3, GES_TYPE_VIDEO_TEST_SOURCE, track);
_assert_child_in_track (s3, GES_TYPE_AUDIO_TEST_SOURCE, track1);
/* adding another track should not prompt the change anything
* unrelated to the new track */
/* remove the core children from s1 */
_remove_sources (s1, 2);
/* only have effect left, and not in any track */
assert_num_children (s1, 1);
/* effect is emptied from its track, since the corresponding core child
* was removed */
_assert_child_in_track (s1, GES_TYPE_EFFECT, NULL);
assert_num_in_track (track, 2);
assert_num_in_track (track1, 2);
track2 = GES_TRACK (ges_video_track_new ());
_assert_add_track (timeline, track2);
/* other tracks stay the same */
assert_num_in_track (track, 2);
assert_num_in_track (track1, 2);
/* 1 sources + 1 effect */
assert_num_in_track (track2, 2);
/* s1 only has a child created for the new track, not the other two */
assert_num_children (s1, 2);
_assert_child_in_track (s1, GES_TYPE_EFFECT, track2);
_assert_child_in_track (s1, GES_TYPE_VIDEO_TEST_SOURCE, track2);
_assert_no_child_in_track (s1, track);
_assert_no_child_in_track (s1, track1);
/* other clips stay the same since their children were already created
* with set tracks */
assert_num_children (s2, 2);
_assert_child_in_track (s2, GES_TYPE_VIDEO_TEST_SOURCE, track);
_assert_child_in_track (s2, GES_TYPE_AUDIO_TEST_SOURCE, track1);
_assert_no_child_in_track (s2, track2);
assert_num_children (s3, 2);
_assert_child_in_track (s3, GES_TYPE_VIDEO_TEST_SOURCE, track);
_assert_child_in_track (s3, GES_TYPE_AUDIO_TEST_SOURCE, track1);
_assert_no_child_in_track (s3, track2);
/* same with an audio track */
/* remove the core child from s1 */
_remove_sources (s1, 1);
assert_num_children (s1, 1);
_assert_child_in_track (s1, GES_TYPE_EFFECT, NULL);
assert_num_in_track (track, 2);
assert_num_in_track (track1, 2);
assert_num_in_track (track2, 0);
/* unset the core tracks for s2 */
_remove_from_track (s2, track, 1);
_remove_from_track (s2, track1, 1);
/* but keep children in clip */
assert_num_children (s2, 2);
assert_num_in_track (track, 1);
assert_num_in_track (track1, 1);
assert_num_in_track (track2, 0);
track3 = GES_TRACK (ges_audio_track_new ());
_assert_add_track (timeline, track3);
/* other tracks stay the same */
assert_num_in_track (track, 1);
assert_num_in_track (track1, 1);
assert_num_in_track (track2, 0);
/* 2 sources */
assert_num_in_track (track3, 2);
/* s1 creates core for the new track, but effect does not have a track
* set since the new track is not a video track */
assert_num_children (s1, 2);
_assert_child_in_track (s1, GES_TYPE_AUDIO_TEST_SOURCE, track3);
_assert_child_in_track (s1, GES_TYPE_EFFECT, NULL);
_assert_no_child_in_track (s1, track);
_assert_no_child_in_track (s1, track1);
_assert_no_child_in_track (s1, track2);
/* s2 audio core is in the new track, but video remains trackless */
assert_num_children (s2, 2);
_assert_child_in_track (s2, GES_TYPE_AUDIO_TEST_SOURCE, track3);
_assert_child_in_track (s2, GES_TYPE_VIDEO_TEST_SOURCE, NULL);
_assert_no_child_in_track (s1, track);
_assert_no_child_in_track (s1, track1);
_assert_no_child_in_track (s1, track2);
/* s3 remains the same since core already had tracks */
assert_num_children (s3, 2);
_assert_child_in_track (s3, GES_TYPE_VIDEO_TEST_SOURCE, track);
_assert_child_in_track (s3, GES_TYPE_AUDIO_TEST_SOURCE, track1);
_assert_no_child_in_track (s3, track2);
_assert_no_child_in_track (s3, track3);
/* theoretically this is all we need to do to ensure cleanup */
gst_object_unref (timeline);