fixed image bounds error status null bug + test case (#1896)

* fixed image bounds error status null bug + test case

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

Author: cato-o

src/opentimelineio/clip.cpp

@@ -190,21 +190,32 @@ std::optional<IMATH_NAMESPACE::Box2d>
 Clip::available_image_bounds(ErrorStatus* error_status) const
 {
     auto active_media = media_reference();
+    
+    //this code path most likely never runs since a null or empty media_reference gets
+    //replaced with a placeholder value when instantiated
     if (!active_media)
     {
-        *error_status = ErrorStatus(
-            ErrorStatus::CANNOT_COMPUTE_BOUNDS,
-            "No image bounds set on clip",
-            this);
+        if(error_status)
+        {
+            *error_status = ErrorStatus(
+                ErrorStatus::CANNOT_COMPUTE_BOUNDS,
+                "No image bounds set on clip",
+                this);
+        }
+
         return std::optional<IMATH_NAMESPACE::Box2d>();
     }
 
     if (!active_media->available_image_bounds())
     {
-        *error_status = ErrorStatus(
-            ErrorStatus::CANNOT_COMPUTE_BOUNDS,
-            "No image bounds set on media reference on clip",
-            this);
+        if(error_status)
+        {
+            *error_status = ErrorStatus(
+                ErrorStatus::CANNOT_COMPUTE_BOUNDS,
+                "No image bounds set on media reference on clip",
+                this);
+        }
+
         return std::optional<IMATH_NAMESPACE::Box2d>();
     }
 

src/opentimelineio/clip.h

@@ -84,7 +84,7 @@ class Clip : public Item
     available_range(ErrorStatus* error_status = nullptr) const override;
 
     std::optional<IMATH_NAMESPACE::Box2d>
-    available_image_bounds(ErrorStatus* error_status) const override;
+    available_image_bounds(ErrorStatus* error_status = nullptr) const override;
 
 protected:
     virtual ~Clip();

src/opentimelineio/composable.h

@@ -47,7 +47,7 @@ class Composable : public SerializableObjectWithMetadata
 
     /// @brief Return the available image bounds.
     virtual std::optional<IMATH_NAMESPACE::Box2d>
-    available_image_bounds(ErrorStatus* error_status) const;
+    available_image_bounds(ErrorStatus* error_status = nullptr) const;
 
 protected:
     bool        _set_parent(Composition*) noexcept;

tests/test_clip.cpp

@@ -260,6 +260,59 @@ main(int argc, char** argv)
         assertEqual(marker->color().c_str(), red);
     });
 
+    // test to ensure null error_status pointers are correctly handled
+    tests.add_test("test_error_ptr_null", [] {
+
+        using namespace otio;
+
+        // tests for no image bounds on media reference on clip
+        SerializableObject::Retainer<Clip> clip(new Clip);
+
+        // check that there is an error, and that it's the correct error
+        otio::ErrorStatus mr_bounds_error;
+        clip->available_image_bounds(&mr_bounds_error);
+        assertTrue(otio::is_error(mr_bounds_error));
+        assertEqual(
+            mr_bounds_error.outcome,
+            otio::ErrorStatus::CANNOT_COMPUTE_BOUNDS);
+
+        // check that if null ptr, nothing happens
+        otio::ErrorStatus* null_test = nullptr;
+
+        assertEqual(
+            clip->available_image_bounds(null_test), std::optional<IMATH_NAMESPACE::Box2d>()
+        );
+    });
+
+    // test to ensure null error_status pointers are correctly handled
+    // when there's no media reference
+    tests.add_test("test_error_ptr_null_no_media", [] {
+        using namespace otio;
+
+        SerializableObject::Retainer<Clip> clip(new Clip);
+
+        // set media reference to empty
+        Clip::MediaReferences empty_mrs;
+        empty_mrs["empty"] = nullptr;
+        clip->set_media_references(empty_mrs, "empty");
+
+        otio::ErrorStatus bounds_error_no_mr;
+        clip->available_image_bounds(&bounds_error_no_mr);
+        assertTrue(otio::is_error(bounds_error_no_mr));
+
+        assertEqual(
+            bounds_error_no_mr.outcome,
+            otio::ErrorStatus::CANNOT_COMPUTE_BOUNDS);
+
+        // std::cout<< "bounds error details: " << bounds_error_no_mr.details << std::endl;
+
+        otio::ErrorStatus* null_test_no_mr = nullptr;
+
+        assertEqual(
+            clip->available_image_bounds(null_test_no_mr), std::optional<IMATH_NAMESPACE::Box2d>()
+        );
+    });
+
     tests.run(argc, argv);
     return 0;
 }