From 0030b0833b1f027b4fe9b4a0d5dd9beb473c140f Mon Sep 17 00:00:00 2001 From: Thibault Saunier Date: Thu, 3 Sep 2020 23:32:23 -0400 Subject: [PATCH] ges: Do not recreate auto-transitions when changing clip assets Otherwise we loose the configuration of the auto transition, and it is not required at all in any case. Fixes https://gitlab.gnome.org/GNOME/pitivi/-/issues/2380 Part-of: --- ges/ges-auto-transition.c | 15 +++- ges/ges-clip.c | 5 +- ges/ges-internal.h | 5 +- ges/ges-timeline-tree.c | 5 +- ges/ges-timeline.c | 9 ++- ges/ges-uri-clip.c | 103 +++++++++++++++++++++------- tests/check/python/common.py | 34 ++++++++- tests/check/python/test_assets.py | 51 +++++--------- tests/check/python/test_timeline.py | 25 ++++++- 9 files changed, 178 insertions(+), 74 deletions(-) diff --git a/ges/ges-auto-transition.c b/ges/ges-auto-transition.c index e36955bb1f..e2e083d758 100644 --- a/ges/ges-auto-transition.c +++ b/ges/ges-auto-transition.c @@ -104,6 +104,11 @@ static void _track_changed_cb (GESTrackElement * track_element, GParamSpec * arg G_GNUC_UNUSED, GESAutoTransition * self) { + if (self->frozen) { + GST_LOG_OBJECT (self, "Not updating because frozen"); + return; + } + if (ges_track_element_get_track (track_element) == NULL) { GST_DEBUG_OBJECT (self, "Neighboor %" GST_PTR_FORMAT " removed from track ... auto destructing", track_element); @@ -134,12 +139,16 @@ _disconnect_from_source (GESAutoTransition * self, GESTrackElement * source) } void -ges_auto_transition_set_previous_source (GESAutoTransition * self, - GESTrackElement * source) +ges_auto_transition_set_source (GESAutoTransition * self, + GESTrackElement * source, GESEdge edge) { _disconnect_from_source (self, self->previous_source); _connect_to_source (self, source); - self->previous_source = source; + + if (edge == GES_EDGE_END) + self->next_source = source; + else + self->previous_source = source; } static void diff --git a/ges/ges-clip.c b/ges/ges-clip.c index 8e17584a45..fbd4f47118 100644 --- a/ges/ges-clip.c +++ b/ges/ges-clip.c @@ -3491,11 +3491,12 @@ ges_clip_split_full (GESClip * clip, guint64 position, GError ** error) gst_object_ref (track)); trans = timeline ? - ges_timeline_get_auto_transition_at_end (timeline, orig) : NULL; + ges_timeline_get_auto_transition_at_edge (timeline, orig, + GES_EDGE_END) : NULL; if (trans) { trans->frozen = TRUE; - ges_auto_transition_set_previous_source (trans, copy); + ges_auto_transition_set_source (trans, copy, GES_EDGE_START); transitions = g_list_append (transitions, trans); } } diff --git a/ges/ges-internal.h b/ges/ges-internal.h index 186da684d7..692affcbb0 100644 --- a/ges/ges-internal.h +++ b/ges/ges-internal.h @@ -112,7 +112,8 @@ G_GNUC_INTERNAL void ges_timeline_freeze_auto_transitions (GESTimeline * timeline, gboolean freeze); G_GNUC_INTERNAL GESAutoTransition * -ges_timeline_get_auto_transition_at_end (GESTimeline * timeline, GESTrackElement * source); +ges_timeline_get_auto_transition_at_edge (GESTimeline * timeline, GESTrackElement * source, + GESEdge edge); G_GNUC_INTERNAL gboolean ges_timeline_is_disposed (GESTimeline* timeline); @@ -183,7 +184,7 @@ G_GNUC_INTERNAL gboolean ges_timeline_get_smart_rendering (GESTimeline *timeline); G_GNUC_INTERNAL void -ges_auto_transition_set_previous_source (GESAutoTransition * self, GESTrackElement * source); +ges_auto_transition_set_source (GESAutoTransition * self, GESTrackElement * source, GESEdge edge); diff --git a/ges/ges-timeline-tree.c b/ges/ges-timeline-tree.c index 8f92d23c05..ece5a5bf34 100644 --- a/ges/ges-timeline-tree.c +++ b/ges/ges-timeline-tree.c @@ -2368,9 +2368,8 @@ create_transition_if_needed (GESTimeline * timeline, GESTrackElement * prev, ges_timeline_create_transition (timeline, prev, next, NULL, layer, _START (next), duration); } else { - GST_INFO ("Already have transition %" GES_FORMAT " between %" GES_FORMAT - " and %" GES_FORMAT, GES_ARGS (trans), GES_ARGS (prev), - GES_ARGS (next)); + GST_INFO ("Already have transition %" GST_PTR_FORMAT " between %" GES_FORMAT + " and %" GES_FORMAT, trans, GES_ARGS (prev), GES_ARGS (next)); } } diff --git a/ges/ges-timeline.c b/ges/ges-timeline.c index 2f49e2189a..bb675adc01 100644 --- a/ges/ges-timeline.c +++ b/ges/ges-timeline.c @@ -1074,8 +1074,8 @@ ges_timeline_find_auto_transition (GESTimeline * timeline, } GESAutoTransition * -ges_timeline_get_auto_transition_at_end (GESTimeline * timeline, - GESTrackElement * source) +ges_timeline_get_auto_transition_at_edge (GESTimeline * timeline, + GESTrackElement * source, GESEdge edge) { GList *tmp, *auto_transitions; GESAutoTransition *ret = NULL; @@ -1090,7 +1090,10 @@ ges_timeline_get_auto_transition_at_end (GESTimeline * timeline, /* We already have a transition linked to one of the elements we want to * find a transition for */ - if (auto_trans->previous_source == source) { + if (edge == GES_EDGE_END && auto_trans->previous_source == source) { + ret = gst_object_ref (auto_trans); + break; + } else if (edge == GES_EDGE_START && auto_trans->next_source == source) { ret = gst_object_ref (auto_trans); break; } diff --git a/ges/ges-uri-clip.c b/ges/ges-uri-clip.c index 12bb09436b..1c1d14cf89 100644 --- a/ges/ges-uri-clip.c +++ b/ges/ges-uri-clip.c @@ -224,6 +224,27 @@ extractable_get_id (GESExtractable * self) return g_strdup (GES_URI_CLIP (self)->priv->uri); } +static GList * +get_auto_transitions_around_source (GESTrackElement * child) +{ + GList *transitions = NULL; + GESTimeline *timeline = GES_TIMELINE_ELEMENT_TIMELINE (child); + gint i; + GESEdge edges[] = { GES_EDGE_START, GES_EDGE_END }; + + if (!timeline) + return NULL; + + for (i = 0; i < G_N_ELEMENTS (edges); i++) { + GESAutoTransition *transition = + ges_timeline_get_auto_transition_at_edge (timeline, child, edges[i]); + if (transition) + transitions = g_list_prepend (transitions, transition); + } + + return transitions; +} + static gboolean extractable_set_asset (GESExtractable * self, GESAsset * asset) { @@ -235,9 +256,11 @@ extractable_set_asset (GESExtractable * self, GESAsset * asset) GESTimelineElement *element = GES_TIMELINE_ELEMENT (self); GESLayer *layer = ges_clip_get_layer (clip); GList *tmp, *children; - GHashTable *source_by_track; + GHashTable *source_by_track, *auto_transitions_on_sources; GstClockTime max_duration; GESAsset *prev_asset; + GList *transitions = NULL; + GESTimeline *timeline = GES_TIMELINE_ELEMENT_TIMELINE (self); g_return_val_if_fail (GES_IS_URI_CLIP_ASSET (asset), FALSE); @@ -300,23 +323,36 @@ extractable_set_asset (GESExtractable * self, GESAsset * asset) source_by_track = g_hash_table_new_full (NULL, NULL, gst_object_unref, gst_object_unref); - children = ges_container_get_children (container, FALSE); + auto_transitions_on_sources = g_hash_table_new_full (NULL, NULL, + gst_object_unref, (GDestroyNotify) g_list_free); + if (timeline) + ges_timeline_freeze_auto_transitions (timeline, TRUE); + + children = ges_container_get_children (container, FALSE); for (tmp = children; tmp; tmp = tmp->next) { GESTrackElement *child = tmp->data; - GESTrack *track = ges_track_element_get_track (child); - /* remove our core children */ - if (ges_track_element_is_core (child)) { - if (track) - g_hash_table_insert (source_by_track, gst_object_ref (track), - gst_object_ref (child)); + GESTrack *track; - /* removing the track element from its clip whilst it is in a - * timeline will remove it from its track */ - /* removing the core element will also empty its non-core siblings - * from the same track */ - ges_container_remove (container, GES_TIMELINE_ELEMENT (child)); - } + /* remove our core children */ + if (!ges_track_element_is_core (child)) + continue; + + track = ges_track_element_get_track (child); + if (track) + g_hash_table_insert (source_by_track, gst_object_ref (track), + gst_object_ref (child)); + + transitions = get_auto_transitions_around_source (child); + if (transitions) + g_hash_table_insert (auto_transitions_on_sources, gst_object_ref (child), + transitions); + + /* removing the track element from its clip whilst it is in a + * timeline will remove it from its track */ + /* removing the core element will also empty its non-core siblings + * from the same track */ + ges_container_remove (container, GES_TIMELINE_ELEMENT (child)); } g_list_free_full (children, g_object_unref); @@ -343,17 +379,32 @@ extractable_set_asset (GESExtractable * self, GESAsset * asset) * the same source! */ for (tmp = container->children; tmp; tmp = tmp->next) { GESTrackElement *child = tmp->data; - if (ges_track_element_is_core (child)) { - GESTrackElement *orig_source = g_hash_table_lookup (source_by_track, - ges_track_element_get_track (child)); - contains_core = TRUE; + GESTrackElement *orig_source; - if (orig_source) { - ges_track_element_copy_properties (GES_TIMELINE_ELEMENT - (orig_source), GES_TIMELINE_ELEMENT (child)); - ges_track_element_copy_bindings (orig_source, child, - GST_CLOCK_TIME_NONE); - } + if (!ges_track_element_is_core (child)) + continue; + + contains_core = TRUE; + orig_source = g_hash_table_lookup (source_by_track, + ges_track_element_get_track (child)); + + if (!orig_source) + continue; + + ges_track_element_copy_properties (GES_TIMELINE_ELEMENT + (orig_source), GES_TIMELINE_ELEMENT (child)); + ges_track_element_copy_bindings (orig_source, child, + GST_CLOCK_TIME_NONE); + + transitions = + g_hash_table_lookup (auto_transitions_on_sources, orig_source); + for (; transitions; transitions = transitions->next) { + GESAutoTransition *transition = transitions->data; + + if (transition->previous_source == orig_source) + ges_auto_transition_set_source (transition, child, GES_EDGE_START); + else if (transition->next_source == orig_source) + ges_auto_transition_set_source (transition, child, GES_EDGE_END); } } } else { @@ -363,6 +414,10 @@ extractable_set_asset (GESExtractable * self, GESAsset * asset) gst_object_unref (layer); } g_hash_table_unref (source_by_track); + g_hash_table_unref (auto_transitions_on_sources); + + if (timeline) + ges_timeline_freeze_auto_transitions (timeline, FALSE); if (res) { g_free (uriclip->priv->uri); diff --git a/tests/check/python/common.py b/tests/check/python/common.py index d46a21c0fd..1593d68982 100644 --- a/tests/check/python/common.py +++ b/tests/check/python/common.py @@ -32,6 +32,12 @@ import os #noqa import unittest # noqa import tempfile # noqa +try: + gi.require_version("GstTranscoder", "1.0") + from gi.repository import GstTranscoder +except ValueError: + GstTranscoder = None + Gst.init(None) GES.init() @@ -99,6 +105,28 @@ def created_project_file(xges): os.remove(xges_path) +def can_generate_assets(): + if GstTranscoder is None: + return False, "GstTranscoder is not available" + + if not Gst.ElementFactory.make("testsrcbin"): + return False, "testbinsrc is not available" + + return True, None + + +@contextlib.contextmanager +def created_video_asset(uri=None, num_bufs=30): + with tempfile.NamedTemporaryFile(suffix=".ogg") as f: + if not uri: + uri = Gst.filename_to_uri(f.name) + transcoder = GstTranscoder.Transcoder.new("testbin://video,num-buffers=%s" % num_bufs, + uri, "application/ogg:video/x-theora:audio/x-vorbis") + transcoder.run() + + yield uri + + def get_asset_uri(name): python_tests_dir = os.path.dirname(os.path.abspath(__file__)) assets_dir = os.path.join(python_tests_dir, "..", "assets") @@ -217,7 +245,11 @@ class GESSimpleTimelineTest(GESTest): while len(self.timeline.get_layers()) < layer + 1: self.timeline.append_layer() layer = self.timeline.get_layers()[layer] - clip = GES.Asset.request(asset_type, asset_id).extract() + if asset_type == GES.UriClip: + asset = GES.UriClipAsset.request_sync(asset_id) + else: + asset = GES.Asset.request(asset_type, asset_id) + clip = asset.extract() clip.props.start = layer.get_duration() clip.props.duration = 10 self.assertTrue(layer.add_clip(clip)) diff --git a/tests/check/python/test_assets.py b/tests/check/python/test_assets.py index 4531dbb6d4..e33c623009 100644 --- a/tests/check/python/test_assets.py +++ b/tests/check/python/test_assets.py @@ -32,12 +32,6 @@ from gi.repository import GES # noqa import unittest # noqa from unittest import mock -try: - gi.require_version("GstTranscoder", "1.0") - from gi.repository import GstTranscoder -except ValueError: - GstTranscoder = None - from . import common from .common import GESSimpleTimelineTest # noqa @@ -66,40 +60,29 @@ class TestTimeline(GESSimpleTimelineTest): asset = proj.create_asset_sync("file:///png.png", GES.UriClip) self.assertIsNotNone(asset) - @unittest.skipIf(GstTranscoder is None, "GstTranscoder is not available") - @unittest.skipIf(Gst.ElementFactory.make("testsrcbin") is None, "testbinsrc is not available") + @unittest.skipUnless(*common.can_generate_assets()) def test_reload_asset(self): - with tempfile.NamedTemporaryFile(suffix=".ogg") as f: - uri = Gst.filename_to_uri(f.name) - transcoder = GstTranscoder.Transcoder.new("testbin://video,num-buffers=30", - uri, "application/ogg:video/x-theora:audio/x-vorbis") - transcoder.run() - + with common.created_video_asset() as uri: asset0 = GES.UriClipAsset.request_sync(uri) self.assertEqual(asset0.props.duration, Gst.SECOND) - transcoder = GstTranscoder.Transcoder.new("testbin://video,num-buffers=60", - uri, "application/ogg:video/x-theora:audio/x-vorbis") - transcoder.run() + with common.created_video_asset(uri, 60) as uri: + GES.Asset.needs_reload(GES.UriClip, uri) + asset1 = GES.UriClipAsset.request_sync(uri) + self.assertEqual(asset1.props.duration, 2 * Gst.SECOND) + self.assertEqual(asset1, asset0) - GES.Asset.needs_reload(GES.UriClip, uri) - asset1 = GES.UriClipAsset.request_sync(uri) - self.assertEqual(asset1.props.duration, 2 * Gst.SECOND) - self.assertEqual(asset1, asset0) + with common.created_video_asset(uri, 90) as uri: + mainloop = common.create_main_loop() + def asset_loaded_cb(_, res, mainloop): + asset2 = GES.Asset.request_finish(res) + self.assertEqual(asset2.props.duration, 3 * Gst.SECOND) + self.assertEqual(asset2, asset0) + mainloop.quit() - transcoder = GstTranscoder.Transcoder.new("testbin://video,num-buffers=90", - uri, "application/ogg:video/x-theora:audio/x-vorbis") - transcoder.run() - mainloop = common.create_main_loop() - def asset_loaded_cb(_, res, mainloop): - asset2 = GES.Asset.request_finish(res) - self.assertEqual(asset2.props.duration, 3 * Gst.SECOND) - self.assertEqual(asset2, asset0) - mainloop.quit() - - GES.Asset.needs_reload(GES.UriClip, uri) - GES.Asset.request_async(GES.UriClip, uri, None, asset_loaded_cb, mainloop) - mainloop.run() + GES.Asset.needs_reload(GES.UriClip, uri) + GES.Asset.request_async(GES.UriClip, uri, None, asset_loaded_cb, mainloop) + mainloop.run() def test_asset_metadata_on_reload(self): mainloop = GLib.MainLoop() diff --git a/tests/check/python/test_timeline.py b/tests/check/python/test_timeline.py index 6187d039f3..e28607bcf5 100644 --- a/tests/check/python/test_timeline.py +++ b/tests/check/python/test_timeline.py @@ -201,6 +201,29 @@ class TestTimeline(common.GESSimpleTimelineTest): ] ]) + @unittest.skipUnless(*common.can_generate_assets()) + def test_auto_transition_type_after_setting_proxy_asset(self): + self.track_types = [GES.TrackType.VIDEO] + super().setUp() + + self.timeline.props.auto_transition = True + with common.created_video_asset() as uri: + self.append_clip(asset_type=GES.UriClip, asset_id=uri) + self.append_clip(asset_type=GES.UriClip, asset_id=uri).props.start = 5 + clip1, transition, clip2 = self.layer.get_clips() + video_transition, = transition.get_children(True) + video_transition.set_transition_type(GES.VideoStandardTransitionType.BAR_WIPE_LR) + self.assertEqual(video_transition.get_transition_type(), GES.VideoStandardTransitionType.BAR_WIPE_LR) + + with common.created_video_asset() as uri2: + proxy_asset = GES.UriClipAsset.request_sync(uri2) + clip1.set_asset(proxy_asset) + clip1, transition1, clip2 = self.layer.get_clips() + + video_transition1, = transition1.get_children(True) + self.assertEqual(video_transition, video_transition1) + self.assertEqual(video_transition.get_transition_type(), GES.VideoStandardTransitionType.BAR_WIPE_LR) + def test_frame_info(self): self.track_types = [GES.TrackType.VIDEO] super().setUp() @@ -345,12 +368,10 @@ class TestEditing(common.GESSimpleTimelineTest): def _snapped_cb(timeline, elem1, elem2, position): self.snapped_at.append(position) - Gst.error('%s' % position) def _snapped_end_cb(timeline, elem1, elem2, position): if self.snapped_at: # Ignoring first snap end. self.snapped_at.append(Gst.CLOCK_TIME_NONE) - Gst.error('%s' % position) self.timeline.connect("snapping-started", _snapped_cb) self.timeline.connect("snapping-ended", _snapped_end_cb)