From 54374b46f5e76c327557418cd61069b657d31932 Mon Sep 17 00:00:00 2001 From: Mark Reid <mindmark@gmail.com> Date: Sat, 17 Feb 2024 23:05:00 -0800 Subject: [PATCH 1/2] Normalize track lengths with gaps before flatten stacks fixes #1430 Signed-off-by: Mark Reid <mindmark@gmail.com> --- src/opentimelineio/stackAlgorithm.cpp | 63 +++++++- tests/CMakeLists.txt | 2 +- tests/test_stack_algo.cpp | 218 ++++++++++++++++++++++++++ 3 files changed, 281 insertions(+), 2 deletions(-) create mode 100644 tests/test_stack_algo.cpp diff --git a/src/opentimelineio/stackAlgorithm.cpp b/src/opentimelineio/stackAlgorithm.cpp index 6800cbc2de..77129c62eb 100644 --- a/src/opentimelineio/stackAlgorithm.cpp +++ b/src/opentimelineio/stackAlgorithm.cpp @@ -5,10 +5,12 @@ #include "opentimelineio/track.h" #include "opentimelineio/trackAlgorithm.h" #include "opentimelineio/transition.h" +#include "opentimelineio/gap.h" namespace opentimelineio { namespace OPENTIMELINEIO_VERSION { typedef std::map<Track*, std::map<Composable*, TimeRange>> RangeTrackMap; +typedef std::vector<SerializableObject::Retainer<Track>> TrackRetainerVector; static void _flatten_next_item( @@ -123,10 +125,49 @@ _flatten_next_item( } } +// make all tracks the same length by adding a gap to the end if they are shorter +static void +_normalize_tracks_lengths(std::vector<Track*>& tracks, + TrackRetainerVector& tracks_retainer, + ErrorStatus* error_status) +{ + RationalTime duration; + for (auto track: tracks) + { + duration = std::max(duration, track->duration(error_status)); + if (is_error(error_status)) + { + return; + } + } + + for(int i = 0; i < tracks.size(); i++) + { + Track *track = tracks[i]; + RationalTime track_duration = track->duration(error_status); + if (track_duration < duration) + { + track = static_cast<Track*>(track->clone(error_status)); + if (is_error(error_status)) + { + return; + } + tracks_retainer.push_back(SerializableObject::Retainer<Track>(track)); + track->append_child(new Gap(duration - track_duration), error_status); + if (is_error(error_status)) + { + return; + } + tracks[i] = track; + } + } +} + Track* flatten_stack(Stack* in_stack, ErrorStatus* error_status) { std::vector<Track*> tracks; + TrackRetainerVector tracks_retainer; tracks.reserve(in_stack->children().size()); for (auto c: in_stack->children()) @@ -151,6 +192,12 @@ flatten_stack(Stack* in_stack, ErrorStatus* error_status) } } + _normalize_tracks_lengths(tracks, tracks_retainer, error_status); + if (is_error(error_status)) + { + return nullptr; + } + Track* flat_track = new Track; flat_track->set_name("Flattened"); @@ -168,6 +215,20 @@ flatten_stack(Stack* in_stack, ErrorStatus* error_status) Track* flatten_stack(std::vector<Track*> const& tracks, ErrorStatus* error_status) { + std::vector<Track*> flat_tracks; + TrackRetainerVector tracks_retainer; + flat_tracks.reserve(tracks.size()); + for (auto track : tracks) + { + flat_tracks.push_back(track); + } + + _normalize_tracks_lengths(flat_tracks, tracks_retainer, error_status); + if (is_error(error_status)) + { + return nullptr; + } + Track* flat_track = new Track; flat_track->set_name("Flattened"); @@ -175,7 +236,7 @@ flatten_stack(std::vector<Track*> const& tracks, ErrorStatus* error_status) _flatten_next_item( range_track_map, flat_track, - tracks, + flat_tracks, -1, nullopt, error_status); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e206fb8cf7..85cb216f86 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -15,7 +15,7 @@ foreach(test ${tests_opentime}) WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}) endforeach() -list(APPEND tests_opentimelineio test_clip test_serialization test_serializableCollection test_timeline test_track test_editAlgorithm) +list(APPEND tests_opentimelineio test_clip test_serialization test_serializableCollection test_stack_algo test_timeline test_track test_editAlgorithm) foreach(test ${tests_opentimelineio}) add_executable(${test} utils.h utils.cpp ${test}.cpp) diff --git a/tests/test_stack_algo.cpp b/tests/test_stack_algo.cpp new file mode 100644 index 0000000000..39a953d73c --- /dev/null +++ b/tests/test_stack_algo.cpp @@ -0,0 +1,218 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Contributors to the OpenTimelineIO project + +#include "opentime/timeRange.h" +#include "utils.h" + +#include <opentimelineio/clip.h> +#include <opentimelineio/stack.h> +#include <opentimelineio/track.h> +#include <opentimelineio/stackAlgorithm.h> + +#include <iostream> + +namespace otime = opentime::OPENTIME_VERSION; +namespace otio = opentimelineio::OPENTIMELINEIO_VERSION; + +int +main(int argc, char** argv) +{ + Tests tests; + + tests.add_test( + "test_flatten_stack_01", [] { + using namespace otio; + + otio::RationalTime rt_0_24{0, 24}; + otio::RationalTime rt_150_24{150, 24}; + otio::TimeRange tr_0_150_24{rt_0_24, rt_150_24}; + + // all three clips are identical, but placed such that A is over B and + // has no gap or end over C + // 0 150 300 + // [ A ] + // [ B | C ] + // + // should flatten to: + // [ A | C ] + + otio::SerializableObject::Retainer<otio::Clip> cl_A = + new otio::Clip("track1_A", nullptr, tr_0_150_24); + otio::SerializableObject::Retainer<otio::Clip> cl_B = + new otio::Clip("track1_B", nullptr, tr_0_150_24); + otio::SerializableObject::Retainer<otio::Clip> cl_C = + new otio::Clip("track1_C", nullptr, tr_0_150_24); + + otio::SerializableObject::Retainer<otio::Track> tr_over = + new otio::Track(); + tr_over->append_child(cl_A); + + otio::SerializableObject::Retainer<otio::Track> tr_under = + new otio::Track(); + tr_under->append_child(cl_B); + tr_under->append_child(cl_C); + + otio::SerializableObject::Retainer<otio::Stack> st = + new otio::Stack(); + st->append_child(tr_under); + st->append_child(tr_over); + + auto result = flatten_stack(st); + + assertEqual(result->children()[0]->name(), std::string("track1_A")); + assertEqual(result->children().size(), 2); + assertEqual(result->duration().value(), 300); + }); + + tests.add_test( + "test_flatten_stack_02", [] { + using namespace otio; + + otio::RationalTime rt_0_24{0, 24}; + otio::RationalTime rt_150_24{150, 24}; + otio::TimeRange tr_0_150_24{rt_0_24, rt_150_24}; + + // all four clips are identical, but placed such that A is over B and + // has no gap or end over C. The bottom track is also shorter. + // 0 150 300 + // [ A ] + // [ B | C ] + // [ D ] + // + // should flatten to: + // [ A | C ] + + otio::SerializableObject::Retainer<otio::Clip> cl_A = + new otio::Clip("track1_A", nullptr, tr_0_150_24); + otio::SerializableObject::Retainer<otio::Clip> cl_B = + new otio::Clip("track1_B", nullptr, tr_0_150_24); + otio::SerializableObject::Retainer<otio::Clip> cl_C = + new otio::Clip("track1_C", nullptr, tr_0_150_24); + otio::SerializableObject::Retainer<otio::Clip> cl_D = + new otio::Clip("track1_D", nullptr, tr_0_150_24); + + otio::SerializableObject::Retainer<otio::Track> tr_top = + new otio::Track(); + tr_top->append_child(cl_A); + + otio::SerializableObject::Retainer<otio::Track> tr_middle = + new otio::Track(); + tr_middle->append_child(cl_B); + tr_middle->append_child(cl_C); + + otio::SerializableObject::Retainer<otio::Track> tr_bottom = + new otio::Track(); + + tr_bottom->append_child(cl_D); + + otio::SerializableObject::Retainer<otio::Stack> st = + new otio::Stack(); + st->append_child(tr_bottom); + st->append_child(tr_middle); + st->append_child(tr_top); + + auto result = flatten_stack(st); + + assertEqual(result->children()[0]->name(), std::string("track1_A")); + assertEqual(result->children().size(), 2); + assertEqual(result->duration().value(), 300); + }); + + tests.add_test( + "test_flatten_stack_03", [] { + using namespace otio; + + otio::RationalTime rt_0_24{0, 24}; + otio::RationalTime rt_150_24{150, 24}; + otio::TimeRange tr_0_150_24{rt_0_24, rt_150_24}; + + // all three clips are identical but the middle track is empty + // 0 150 300 + // [ A ] + // [] + // [ B | C ] + // + // should flatten to: + // [ A | C ] + + otio::SerializableObject::Retainer<otio::Clip> cl_A = + new otio::Clip("track1_A", nullptr, tr_0_150_24); + otio::SerializableObject::Retainer<otio::Clip> cl_B = + new otio::Clip("track1_B", nullptr, tr_0_150_24); + otio::SerializableObject::Retainer<otio::Clip> cl_C = + new otio::Clip("track1_C", nullptr, tr_0_150_24); + + otio::SerializableObject::Retainer<otio::Track> tr_top = + new otio::Track(); + tr_top->append_child(cl_A); + + otio::SerializableObject::Retainer<otio::Track> tr_middle = + new otio::Track(); + + otio::SerializableObject::Retainer<otio::Track> tr_bottom = + new otio::Track(); + + tr_bottom->append_child(cl_B); + tr_bottom->append_child(cl_C); + + + otio::SerializableObject::Retainer<otio::Stack> st = + new otio::Stack(); + st->append_child(tr_bottom); + st->append_child(tr_middle); + st->append_child(tr_top); + + auto result = flatten_stack(st); + + assertEqual(result->children()[0]->name(), std::string("track1_A")); + assertEqual(result->children().size(), 2); + assertEqual(result->duration().value(), 300); + }); + + tests.add_test( + "test_flatten_vector_01", [] { + using namespace otio; + + otio::RationalTime rt_0_24{0, 24}; + otio::RationalTime rt_150_24{150, 24}; + otio::TimeRange tr_0_150_24{rt_0_24, rt_150_24}; + + // all three clips are identical, but placed such that A is over B and + // has no gap or end over C, tests vector version + // 0 150 300 + // [ A ] + // [ B | C ] + // + // should flatten to: + // [ A | C ] + + otio::SerializableObject::Retainer<otio::Clip> cl_A = + new otio::Clip("track1_A", nullptr, tr_0_150_24); + otio::SerializableObject::Retainer<otio::Clip> cl_B = + new otio::Clip("track1_B", nullptr, tr_0_150_24); + otio::SerializableObject::Retainer<otio::Clip> cl_C = + new otio::Clip("track1_C", nullptr, tr_0_150_24); + + otio::SerializableObject::Retainer<otio::Track> tr_over = + new otio::Track(); + tr_over->append_child(cl_A); + + otio::SerializableObject::Retainer<otio::Track> tr_under = + new otio::Track(); + tr_under->append_child(cl_B); + tr_under->append_child(cl_C); + + std::vector<Track *> st; + st.push_back(tr_under); + st.push_back(tr_over); + + auto result = flatten_stack(st); + + assertEqual(result->children()[0]->name(), std::string("track1_A")); + assertEqual(result->children().size(), 2); + assertEqual(result->duration().value(), 300); + }); + + tests.run(argc, argv); + return 0; +} \ No newline at end of file From d5430bd1929c7ae787f8f49e33eea74eb4192182 Mon Sep 17 00:00:00 2001 From: Mark Reid <mindmark@gmail.com> Date: Thu, 14 Mar 2024 22:15:35 -0700 Subject: [PATCH 2/2] Added more comments about the purpose the track retainer Renamed track variables in normalized_tracks_lengths to be more clear. Signed-off-by: Mark Reid <mindmark@gmail.com> --- src/opentimelineio/stackAlgorithm.cpp | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/opentimelineio/stackAlgorithm.cpp b/src/opentimelineio/stackAlgorithm.cpp index 77129c62eb..56f65bbda1 100644 --- a/src/opentimelineio/stackAlgorithm.cpp +++ b/src/opentimelineio/stackAlgorithm.cpp @@ -125,7 +125,9 @@ _flatten_next_item( } } -// make all tracks the same length by adding a gap to the end if they are shorter +// add a gap to end of a track if it is shorter then the longest track. +// shorter tracks are clones and get added to the tracks_retainer. +// a new track will replace the original pointer in the track vector. static void _normalize_tracks_lengths(std::vector<Track*>& tracks, TrackRetainerVector& tracks_retainer, @@ -143,22 +145,23 @@ _normalize_tracks_lengths(std::vector<Track*>& tracks, for(int i = 0; i < tracks.size(); i++) { - Track *track = tracks[i]; - RationalTime track_duration = track->duration(error_status); + Track *old_track = tracks[i]; + RationalTime track_duration = old_track->duration(error_status); if (track_duration < duration) { - track = static_cast<Track*>(track->clone(error_status)); + Track *new_track = static_cast<Track*>(old_track->clone(error_status)); if (is_error(error_status)) { return; } - tracks_retainer.push_back(SerializableObject::Retainer<Track>(track)); - track->append_child(new Gap(duration - track_duration), error_status); + // add track to retainer so it can be freed later + tracks_retainer.push_back(SerializableObject::Retainer<Track>(new_track)); + new_track->append_child(new Gap(duration - track_duration), error_status); if (is_error(error_status)) { return; } - tracks[i] = track; + tracks[i] = new_track; } } } @@ -167,6 +170,9 @@ Track* flatten_stack(Stack* in_stack, ErrorStatus* error_status) { std::vector<Track*> tracks; + // tracks are cloned if they need to be normalized + // they get added to this retainer so they can be + // freed when the algorithm is complete TrackRetainerVector tracks_retainer; tracks.reserve(in_stack->children().size()); @@ -216,6 +222,9 @@ Track* flatten_stack(std::vector<Track*> const& tracks, ErrorStatus* error_status) { std::vector<Track*> flat_tracks; + // tracks are cloned if they need to be normalized + // they get added to this retainer so they can be + // freed when the algorithm is complete TrackRetainerVector tracks_retainer; flat_tracks.reserve(tracks.size()); for (auto track : tracks)