moved find_clips function to superclass and added test case (#1902)

* moved find_clips function to superclass and added test case

Signed-off-by: Yingjie Wang <[email protected]>

Author: cato-o

src/opentimelineio/composition.cpp

@@ -709,4 +709,13 @@ Composition::has_clips() const
     return false;
 }
 
+std::vector<SerializableObject::Retainer<Clip>>
+Composition::find_clips(
+    ErrorStatus*                    error_status,
+    std::optional<TimeRange> const& search_range,
+    bool                            shallow_search) const
+{
+    return find_children<Clip>(error_status, search_range, shallow_search);
+}
+
 }} // namespace opentimelineio::OPENTIMELINEIO_VERSION

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<TimeRange> 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<Retainer<Clip>> find_clips(
+        ErrorStatus*                    error_status   = nullptr,
+        std::optional<TimeRange> const& search_range   = std::nullopt,
+        bool                            shallow_search = false) const;
 
 protected:
     virtual ~Composition();

src/opentimelineio/stack.cpp

@@ -133,15 +133,6 @@ Stack::available_range(ErrorStatus* error_status) const
     return TimeRange(RationalTime(0, duration.rate()), duration);
 }
 
-std::vector<SerializableObject::Retainer<Clip>>
-Stack::find_clips(
-    ErrorStatus*                    error_status,
-    std::optional<TimeRange> const& search_range,
-    bool                            shallow_search) const
-{
-    return find_children<Clip>(error_status, search_range, shallow_search);
-}
-
 std::optional<IMATH_NAMESPACE::Box2d>
 Stack::available_image_bounds(ErrorStatus* error_status) const
 {

src/opentimelineio/stack.h

@@ -58,17 +58,6 @@ class Stack : public Composition
     std::optional<IMATH_NAMESPACE::Box2d>
     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<Retainer<Clip>> find_clips(
-        ErrorStatus*                    error_status   = nullptr,
-        std::optional<TimeRange> const& search_range   = std::nullopt,
-        bool                            shallow_search = false) const;
-
 protected:
     virtual ~Stack();
 

src/opentimelineio/track.cpp

@@ -263,15 +263,6 @@ Track::range_of_all_children(ErrorStatus* error_status) const
     return result;
 }
 
-std::vector<SerializableObject::Retainer<Clip>>
-Track::find_clips(
-    ErrorStatus*                    error_status,
-    std::optional<TimeRange> const& search_range,
-    bool                            shallow_search) const
-{
-    return find_children<Clip>(error_status, search_range, shallow_search);
-}
-
 std::optional<IMATH_NAMESPACE::Box2d>
 Track::available_image_bounds(ErrorStatus* error_status) const
 {

src/opentimelineio/track.h

@@ -81,17 +81,6 @@ class Track : public Composition
     std::optional<IMATH_NAMESPACE::Box2d>
     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<Retainer<Clip>> find_clips(
-        ErrorStatus*                    error_status   = nullptr,
-        std::optional<TimeRange> const& search_range   = std::nullopt,
-        bool                            shallow_search = false) const;
-
 protected:
     virtual ~Track();
 

src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp

@@ -513,6 +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<TimeRange> 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<TimeRange> 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));
@@ -586,16 +589,12 @@ 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)
-        .def("find_clips", [](Track* t, std::optional<TimeRange> const& search_range, bool shallow_search) {
-                return find_clips(t, search_range, shallow_search);
-            }, "search_range"_a = std::nullopt, "shallow_search"_a = false);
+            }, "item"_a, "policy"_a = Track::NeighborGapPolicy::never);
 
     py::class_<Track::Kind>(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_<Stack, Composition, managing_ptr<Stack>>(m, "Stack", py::dynamic_attr())
         .def(py::init([](std::string name,
                          std::optional<std::vector<Composable*>> children,
@@ -621,10 +620,7 @@ 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()))
-        .def("find_clips", [](Stack* s, std::optional<TimeRange> const& search_range, bool shallow_search) {
-                return find_clips(s, search_range, shallow_search);
-            }, "search_range"_a = std::nullopt, "shallow_search"_a = false);
+             py::arg_v("metadata"_a = py::none()));
 
     py::class_<Timeline, SerializableObjectWithMetadata, managing_ptr<Timeline>>(m, "Timeline", py::dynamic_attr())
         .def(py::init([](std::string name,

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)
 

tests/test_composition.cpp

@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright Contributors to the OpenTimelineIO project
+
+#include "utils.h"
+
+#include <opentimelineio/composition.h>
+#include <opentimelineio/item.h>
+#include <opentimelineio/clip.h>
+#include <opentimelineio/stack.h>
+#include <opentimelineio/track.h>
+#include <opentimelineio/transition.h>
+
+#include <iostream>
+
+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<Composition>  comp  = new Composition;
+        SerializableObject::Retainer<Item>  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> stack = new Stack();
+        SerializableObject::Retainer<Track> track = new Track;
+        SerializableObject::Retainer<Clip>  clip  = new Clip;
+        SerializableObject::Retainer<Transition> 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);
+
+        items = track->find_clips(&err);
+        assertFalse(is_error(err));
+        assertEqual(items.size(), 1);
+        assertEqual(items[0].value, clip.value);
+    });
+
+    tests.run(argc, argv);
+    return 0;
+}