From bd9d20291641c2595fe37ac71ea0ecdb41032db7 Mon Sep 17 00:00:00 2001 From: Yingjie Wang Date: Fri, 13 Jun 2025 17:03:22 -0700 Subject: [PATCH 1/5] moved find_clips function to superclass and added test case Signed-off-by: Yingjie Wang --- src/opentimelineio/composition.cpp | 9 +++++++++ src/opentimelineio/composition.h | 13 +++++++++++++ src/opentimelineio/stack.cpp | 9 --------- src/opentimelineio/stack.h | 11 ----------- src/opentimelineio/track.cpp | 9 --------- src/opentimelineio/track.h | 11 ----------- tests/test_stack_algo.cpp | 16 ++++++++++++++++ tests/test_track.cpp | 16 +++++++++++++++- 8 files changed, 53 insertions(+), 41 deletions(-) diff --git a/src/opentimelineio/composition.cpp b/src/opentimelineio/composition.cpp index 57dfc4c0b1..e6915ac2d9 100644 --- a/src/opentimelineio/composition.cpp +++ b/src/opentimelineio/composition.cpp @@ -709,4 +709,13 @@ Composition::has_clips() const return false; } +std::vector> +Composition::find_clips( + ErrorStatus* error_status, + std::optional const& search_range, + bool shallow_search) const +{ + return find_children(error_status, search_range, shallow_search); +} + }} // namespace opentimelineio::OPENTIMELINEIO_VERSION diff --git a/src/opentimelineio/composition.h b/src/opentimelineio/composition.h index b65936ab1b..6b6177dee8 100644 --- a/src/opentimelineio/composition.h +++ b/src/opentimelineio/composition.h @@ -9,6 +9,8 @@ namespace opentimelineio { namespace OPENTIMELINEIO_VERSION { +class Clip; + /// @brief Base class for an Item that contains Composables. /// /// Should be subclassed (for example by Track Stack), not used directly. @@ -157,6 +159,17 @@ class Composition : public Item ErrorStatus* error_status = nullptr, std::optional search_range = std::nullopt, bool shallow_search = false) const; + + /// @brief Find child clips. + /// + /// @param error_status The return status. + /// @param search_range An optional range to limit the search. + /// @param shallow_search The search is recursive unless shallow_search is + /// set to true. + std::vector> find_clips( + ErrorStatus* error_status = nullptr, + std::optional const& search_range = std::nullopt, + bool shallow_search = false) const; protected: virtual ~Composition(); diff --git a/src/opentimelineio/stack.cpp b/src/opentimelineio/stack.cpp index 070d8944e1..ee7ceeb378 100644 --- a/src/opentimelineio/stack.cpp +++ b/src/opentimelineio/stack.cpp @@ -133,15 +133,6 @@ Stack::available_range(ErrorStatus* error_status) const return TimeRange(RationalTime(0, duration.rate()), duration); } -std::vector> -Stack::find_clips( - ErrorStatus* error_status, - std::optional const& search_range, - bool shallow_search) const -{ - return find_children(error_status, search_range, shallow_search); -} - std::optional Stack::available_image_bounds(ErrorStatus* error_status) const { diff --git a/src/opentimelineio/stack.h b/src/opentimelineio/stack.h index 3894d91ad4..03be0d23af 100644 --- a/src/opentimelineio/stack.h +++ b/src/opentimelineio/stack.h @@ -58,17 +58,6 @@ class Stack : public Composition std::optional available_image_bounds(ErrorStatus* error_status) const override; - /// @brief Find child clips. - /// - /// @param error_status The return status. - /// @param search_range An optional range to limit the search. - /// @param shallow_search The search is recursive unless shallow_search is - /// set to true. - std::vector> find_clips( - ErrorStatus* error_status = nullptr, - std::optional const& search_range = std::nullopt, - bool shallow_search = false) const; - protected: virtual ~Stack(); diff --git a/src/opentimelineio/track.cpp b/src/opentimelineio/track.cpp index 3f81f27148..c0dcba1a9d 100644 --- a/src/opentimelineio/track.cpp +++ b/src/opentimelineio/track.cpp @@ -263,15 +263,6 @@ Track::range_of_all_children(ErrorStatus* error_status) const return result; } -std::vector> -Track::find_clips( - ErrorStatus* error_status, - std::optional const& search_range, - bool shallow_search) const -{ - return find_children(error_status, search_range, shallow_search); -} - std::optional Track::available_image_bounds(ErrorStatus* error_status) const { diff --git a/src/opentimelineio/track.h b/src/opentimelineio/track.h index 177a982a8d..cb7a01d2cc 100644 --- a/src/opentimelineio/track.h +++ b/src/opentimelineio/track.h @@ -81,17 +81,6 @@ class Track : public Composition std::optional available_image_bounds(ErrorStatus* error_status) const override; - /// @brief Find child clips. - /// - /// @param error_status The return status. - /// @param search_range An optional range to limit the search. - /// @param shallow_search The search is recursive unless shallow_search is - /// set to true. - std::vector> find_clips( - ErrorStatus* error_status = nullptr, - std::optional const& search_range = std::nullopt, - bool shallow_search = false) const; - protected: virtual ~Track(); diff --git a/tests/test_stack_algo.cpp b/tests/test_stack_algo.cpp index 39a953d73c..3edd7daa8f 100644 --- a/tests/test_stack_algo.cpp +++ b/tests/test_stack_algo.cpp @@ -213,6 +213,22 @@ main(int argc, char** argv) assertEqual(result->duration().value(), 300); }); + // test stack correctly calls find_clips from superclass + tests.add_test( + "test_find_clips", [] { + using namespace otio; + SerializableObject::Retainer stack = new Stack(); + SerializableObject::Retainer track = new Track; + SerializableObject::Retainer clip = new Clip; + stack->append_child(track); + track->append_child(clip); + + opentimelineio::v1_0::ErrorStatus err; + auto items = stack->find_clips(&err); + assertFalse(is_error(err)); + assertEqual(items.size(), 1); + }); + tests.run(argc, argv); return 0; } \ No newline at end of file diff --git a/tests/test_track.cpp b/tests/test_track.cpp index 00e3c7e147..1b0fb0264f 100644 --- a/tests/test_track.cpp +++ b/tests/test_track.cpp @@ -51,7 +51,7 @@ main(int argc, char** argv) opentimelineio::v1_0::ErrorStatus err; auto result = tr->find_children( &err, - TimeRange(RationalTime(0.0, 24.0), RationalTime(24.0, 24.0))); + TimeRange(RationalTime(0.0, 24.0), RationalTime(24.0, 24.0))); assertEqual(result.size(), 1); assertEqual(result[0].value, cl0.value); result = tr->find_children( @@ -166,6 +166,20 @@ main(int argc, char** argv) std::find(items.begin(), items.end(), clip.value) != items.end()); }); + // test track correctly calls find_clips from superclass + tests.add_test( + "test_find_clips", [] { + using namespace otio; + SerializableObject::Retainer track = new Track; + SerializableObject::Retainer clip = new Clip; + track->append_child(clip); + + opentimelineio::v1_0::ErrorStatus err; + auto items = track->find_clips(&err); + assertFalse(is_error(err)); + assertEqual(items.size(), 1); + }); + tests.run(argc, argv); return 0; } From 2954641dc2df6c2b18f2d12a0d33ffbf4ed01e81 Mon Sep 17 00:00:00 2001 From: Yingjie Wang Date: Tue, 17 Jun 2025 11:22:54 -0700 Subject: [PATCH 2/5] added composition class test script and moved tests from subclass to parent class test script Signed-off-by: Yingjie Wang --- .../otio_serializableObjects.cpp | 13 ++-- tests/CMakeLists.txt | 2 +- tests/test_composition.cpp | 59 +++++++++++++++++++ tests/test_stack_algo.cpp | 16 ----- tests/test_track.cpp | 16 +---- 5 files changed, 68 insertions(+), 38 deletions(-) create mode 100644 tests/test_composition.cpp diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp index 0b91a1641d..2288780649 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp @@ -513,6 +513,11 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u .def("find_children", [](Composition* c, py::object descended_from_type, std::optional const& search_range, bool shallow_search) { return find_children(c, descended_from_type, search_range, shallow_search); }, "descended_from_type"_a = py::none(), "search_range"_a = std::nullopt, "shallow_search"_a = false) + + .def("find_clips", [](Composition* c, std::optional const& search_range, bool shallow_search) { + return find_clips(c, search_range, shallow_search); + }, "search_range"_a = std::nullopt, "shallow_search"_a = false); + .def("handles_of_child", [](Composition* c, Composable* child) { auto result = c->handles_of_child(child, ErrorStatusHandler()); return py::make_tuple(py::cast(result.first), py::cast(result.second)); @@ -587,9 +592,7 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u auto result = t->neighbors_of(item, ErrorStatusHandler(), policy); return py::make_tuple(py::cast(result.first.take_value()), py::cast(result.second.take_value())); }, "item"_a, "policy"_a = Track::NeighborGapPolicy::never) - .def("find_clips", [](Track* t, std::optional const& search_range, bool shallow_search) { - return find_clips(t, search_range, shallow_search); - }, "search_range"_a = std::nullopt, "shallow_search"_a = false); + py::class_(track_class, "Kind") .def_property_readonly_static("Audio", [](py::object /* self */) { return Track::Kind::audio; }) @@ -622,9 +625,7 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u "markers"_a = py::none(), "effects"_a = py::none(), py::arg_v("metadata"_a = py::none())) - .def("find_clips", [](Stack* s, std::optional const& search_range, bool shallow_search) { - return find_clips(s, search_range, shallow_search); - }, "search_range"_a = std::nullopt, "shallow_search"_a = false); + py::class_>(m, "Timeline", py::dynamic_attr()) .def(py::init([](std::string name, diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index a17de5c8e8..e5eb0b4925 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_stack_algo 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 test_composition) foreach(test ${tests_opentimelineio}) add_executable(${test} utils.h utils.cpp ${test}.cpp) diff --git a/tests/test_composition.cpp b/tests/test_composition.cpp new file mode 100644 index 0000000000..d84eaac3d8 --- /dev/null +++ b/tests/test_composition.cpp @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright Contributors to the OpenTimelineIO project + +#include "utils.h" + +#include +#include +#include +#include +#include +#include + +#include + +namespace otime = opentime::OPENTIME_VERSION; +namespace otio = opentimelineio::OPENTIMELINEIO_VERSION; + +int +main(int argc, char** argv) +{ + Tests tests; + + // test a basic case of find_children + tests.add_test( + "test_find_children", [] { + using namespace otio; + SerializableObject::Retainer comp = new Composition; + SerializableObject::Retainer item = new Item; + + comp->append_child(item); + opentimelineio::v1_0::ErrorStatus err; + auto result = comp->find_children<>(&err); + assertEqual(result.size(), 1); + assertEqual(result[0].value, item.value); + }); + + // test stack and track correctly calls find_clips from composition parent class + tests.add_test( + "test_find_clips", [] { + using namespace otio; + SerializableObject::Retainer stack = new Stack(); + SerializableObject::Retainer track = new Track; + SerializableObject::Retainer clip = new Clip; + SerializableObject::Retainer transition = new Transition; + + stack->append_child(track); + track->append_child(transition); + track->append_child(clip); + + opentimelineio::v1_0::ErrorStatus err; + auto items = stack->find_clips(&err); + assertFalse(is_error(err)); + assertEqual(items.size(), 1); + assertEqual(items[0].value, clip.value); + }); + + tests.run(argc, argv); + return 0; +} diff --git a/tests/test_stack_algo.cpp b/tests/test_stack_algo.cpp index 3edd7daa8f..39a953d73c 100644 --- a/tests/test_stack_algo.cpp +++ b/tests/test_stack_algo.cpp @@ -213,22 +213,6 @@ main(int argc, char** argv) assertEqual(result->duration().value(), 300); }); - // test stack correctly calls find_clips from superclass - tests.add_test( - "test_find_clips", [] { - using namespace otio; - SerializableObject::Retainer stack = new Stack(); - SerializableObject::Retainer track = new Track; - SerializableObject::Retainer clip = new Clip; - stack->append_child(track); - track->append_child(clip); - - opentimelineio::v1_0::ErrorStatus err; - auto items = stack->find_clips(&err); - assertFalse(is_error(err)); - assertEqual(items.size(), 1); - }); - tests.run(argc, argv); return 0; } \ No newline at end of file diff --git a/tests/test_track.cpp b/tests/test_track.cpp index 1b0fb0264f..00e3c7e147 100644 --- a/tests/test_track.cpp +++ b/tests/test_track.cpp @@ -51,7 +51,7 @@ main(int argc, char** argv) opentimelineio::v1_0::ErrorStatus err; auto result = tr->find_children( &err, - TimeRange(RationalTime(0.0, 24.0), RationalTime(24.0, 24.0))); + TimeRange(RationalTime(0.0, 24.0), RationalTime(24.0, 24.0))); assertEqual(result.size(), 1); assertEqual(result[0].value, cl0.value); result = tr->find_children( @@ -166,20 +166,6 @@ main(int argc, char** argv) std::find(items.begin(), items.end(), clip.value) != items.end()); }); - // test track correctly calls find_clips from superclass - tests.add_test( - "test_find_clips", [] { - using namespace otio; - SerializableObject::Retainer track = new Track; - SerializableObject::Retainer clip = new Clip; - track->append_child(clip); - - opentimelineio::v1_0::ErrorStatus err; - auto items = track->find_clips(&err); - assertFalse(is_error(err)); - assertEqual(items.size(), 1); - }); - tests.run(argc, argv); return 0; } From f0f25e1950ea30e5492bdc61ff5711533231e0ca Mon Sep 17 00:00:00 2001 From: Yingjie Wang Date: Tue, 17 Jun 2025 11:46:29 -0700 Subject: [PATCH 3/5] fixed semicolons Signed-off-by: Yingjie Wang --- .../otio_serializableObjects.cpp | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp index 2288780649..6e533fc099 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp @@ -514,9 +514,11 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u return find_children(c, descended_from_type, search_range, shallow_search); }, "descended_from_type"_a = py::none(), "search_range"_a = std::nullopt, "shallow_search"_a = false) + // ToDo: add find_clips here .def("find_clips", [](Composition* c, std::optional const& search_range, bool shallow_search) { return find_clips(c, search_range, shallow_search); - }, "search_range"_a = std::nullopt, "shallow_search"_a = false); + }, "search_range"_a = std::nullopt, "shallow_search"_a = false) + // ========================== .def("handles_of_child", [](Composition* c, Composable* child) { auto result = c->handles_of_child(child, ErrorStatusHandler()); @@ -591,8 +593,10 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u .def("neighbors_of", [](Track* t, Composable* item, Track::NeighborGapPolicy policy) { auto result = t->neighbors_of(item, ErrorStatusHandler(), policy); return py::make_tuple(py::cast(result.first.take_value()), py::cast(result.second.take_value())); - }, "item"_a, "policy"_a = Track::NeighborGapPolicy::never) - + }, "item"_a, "policy"_a = Track::NeighborGapPolicy::never); + // .def("find_clips", [](Track* t, std::optional const& search_range, bool shallow_search) { + // return find_clips(t, search_range, shallow_search); + // }, "search_range"_a = std::nullopt, "shallow_search"_a = false); py::class_(track_class, "Kind") .def_property_readonly_static("Audio", [](py::object /* self */) { return Track::Kind::audio; }) @@ -624,8 +628,10 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u "source_range"_a = std::nullopt, "markers"_a = py::none(), "effects"_a = py::none(), - py::arg_v("metadata"_a = py::none())) - + py::arg_v("metadata"_a = py::none())); + // .def("find_clips", [](Stack* s, std::optional const& search_range, bool shallow_search) { + // return find_clips(s, search_range, shallow_search); + // }, "search_range"_a = std::nullopt, "shallow_search"_a = false); py::class_>(m, "Timeline", py::dynamic_attr()) .def(py::init([](std::string name, From 7c62facb557cdd3a15662e1331b3ebb53f234d7e Mon Sep 17 00:00:00 2001 From: Yingjie Wang Date: Tue, 17 Jun 2025 14:57:59 -0700 Subject: [PATCH 4/5] code cleanup Signed-off-by: Yingjie Wang --- .../otio_serializableObjects.cpp | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp b/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp index 6e533fc099..873690baa0 100644 --- a/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp +++ b/src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp @@ -513,13 +513,9 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u .def("find_children", [](Composition* c, py::object descended_from_type, std::optional const& search_range, bool shallow_search) { return find_children(c, descended_from_type, search_range, shallow_search); }, "descended_from_type"_a = py::none(), "search_range"_a = std::nullopt, "shallow_search"_a = false) - - // ToDo: add find_clips here .def("find_clips", [](Composition* c, std::optional const& search_range, bool shallow_search) { return find_clips(c, search_range, shallow_search); }, "search_range"_a = std::nullopt, "shallow_search"_a = false) - // ========================== - .def("handles_of_child", [](Composition* c, Composable* child) { auto result = c->handles_of_child(child, ErrorStatusHandler()); return py::make_tuple(py::cast(result.first), py::cast(result.second)); @@ -594,15 +590,11 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u auto result = t->neighbors_of(item, ErrorStatusHandler(), policy); return py::make_tuple(py::cast(result.first.take_value()), py::cast(result.second.take_value())); }, "item"_a, "policy"_a = Track::NeighborGapPolicy::never); - // .def("find_clips", [](Track* t, std::optional const& search_range, bool shallow_search) { - // return find_clips(t, search_range, shallow_search); - // }, "search_range"_a = std::nullopt, "shallow_search"_a = false); py::class_(track_class, "Kind") .def_property_readonly_static("Audio", [](py::object /* self */) { return Track::Kind::audio; }) .def_property_readonly_static("Video", [](py::object /* self */) { return Track::Kind::video; }); - py::class_>(m, "Stack", py::dynamic_attr()) .def(py::init([](std::string name, std::optional> children, @@ -629,9 +621,6 @@ Should be subclassed (for example by :class:`.Track` and :class:`.Stack`), not u "markers"_a = py::none(), "effects"_a = py::none(), py::arg_v("metadata"_a = py::none())); - // .def("find_clips", [](Stack* s, std::optional const& search_range, bool shallow_search) { - // return find_clips(s, search_range, shallow_search); - // }, "search_range"_a = std::nullopt, "shallow_search"_a = false); py::class_>(m, "Timeline", py::dynamic_attr()) .def(py::init([](std::string name, From 11ccc95eb3c4ce2cbe00ba862157d0b0a8bc2a88 Mon Sep 17 00:00:00 2001 From: Yingjie Wang Date: Wed, 18 Jun 2025 13:58:12 -0700 Subject: [PATCH 5/5] added check for track Signed-off-by: Yingjie Wang --- tests/test_composition.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_composition.cpp b/tests/test_composition.cpp index d84eaac3d8..65471c4790 100644 --- a/tests/test_composition.cpp +++ b/tests/test_composition.cpp @@ -52,6 +52,11 @@ main(int argc, char** argv) assertFalse(is_error(err)); assertEqual(items.size(), 1); assertEqual(items[0].value, clip.value); + + items = track->find_clips(&err); + assertFalse(is_error(err)); + assertEqual(items.size(), 1); + assertEqual(items[0].value, clip.value); }); tests.run(argc, argv);