Skip to content

Conversation

rogernelson
Copy link
Collaborator

Fixes #771

This is a merge of the spatial_coordinates branch to main. The code remains unchanged from the branch besides resolving conflicts (mostly whitespace related) related to a clang-format that was run after the initial fork. I also ran clang-format on code coming from the spatial_coordinates branch to conform it to the new style.

I'll copy-paste the comments from the original merge in the spatial_coordinates that contained the bulk of the work below for convenience. Here's a link to it.

Summary

This change addresses the need to have spatial bounds expressed in the MediaReference. These are used to define a measurement-agnostic coordinate system representing the viewing area of the clip(s), which can be used by media players to determine how to display or scale/resize/center (if necessary) the media referenced by the Clips.

The Imath library is used to implement the spatial bounding box, specifically the Box class and its dependency Vec. Imath was selected since its already used by OpenExr, another ASWF project, and this helps establish a common standard between the two projects. Since the 2d template specializations (ie Box2d, a box using 2 dimensional doubles) made the most sense in this context, I only implemented support for it, and its V2d (a 2d vector) dependency. This, however, does not close the door to implementing support for more dimensions or other vector types if needed in the future. The Imath types are contained within a Bounds class which derives from SerializableObject. This encapsulation allows not only the serialization of bounds, but also for the concept to be extended in the future -- for example, support for additional Imath types or for a Transformation type to allow transformations between different coordinate systems.

For a more in-depth explanation of the idea and motivations please see the issue linked here:
#771

I have flagged as many potential discussion points that I think might arise with Note below.

Files changed

.gitmodules
Add Imath as a submodule

*.md files
Lists the additions to the schemas (automatically generated by Makefiles).

src/deps/CMakeLists.txt
Include Imath submodule and set the option to build it statically (without overriding the option for the base OTIO project). We want to build statically to keep the libraries self-contained so they can be made easily into Python wheels. Also use the EXCLUDE_FROM_ALL option if we are not installing dependencies so Imath doesn't write its header files into the installation directory.

src/opentime/CMakeLists.txt
Add new bounds class to compilation, link with static Imath library and remove OpenTimelineIOConfig export. Remove export of build tree (this is the same issue seen here: #911).

src/opentimelineio/bounds.cpp
Implement the Bounds class. For the time being it contains only an optional Imath::Box2d. The box is optional to avoid selecting default values for the Box. Since its optional we can simply have no box as the default.

src/opentimelineio/bounds.h
Add mutators and accessors to the public interface.
Python Bindings: Also added

src/opentimelineio/clip.cpp
Implement the bounds method. Simply returns the Bounds if it has been set on the clip or an error otherwise (conforming to the standard of the available_range method).

src/opentimelineio/clip.h
Adds the bounds method to the public interface.
Python Bindings: Also added

src/opentimelineio/composable.cpp
Implements the bounds method to return an error so that any derived classes that do not explicitly override the virtual method return the error (it is overriden by Clip, Stack and Track).

src/opentimelineio/composable.h
Adds the bounds method to the public interface
Python Bindings: Also added

src/opentimelineio/composition.cpp
Implements the has_clips method for composition, which returns true if the Composition contains at least one clip. This function is used by bounds method of the derived Stack class to exclude any tracks that do not contain any clips when it calculates the union of the bounds. Since the bounds default to width, height and center of {0, 0, (0, 0)} we want to prevent the default values from affecting the result and thus use the has_clips method to skip over any tracks that do not contain clips.

src/opentimelineio/composition.h
Adds the has_clips method to the public interface.
Python Bindings: Also added
Note: Even though this method is only needed by Stack, I thought it could be useful elsewhere so I made it public. If you would prefer I could make it protected so only Stack could access it. Or if you do not want it in the interface at all, I can also implement it as a lambda function (or in the anonymous namespace) inside of Stack::bounds.

src/opentimelineio/deserialization.cpp
Implements the de/serialization of Imath::Box2d Imath::V2d and Bounds, including optional support for Imath::Box2d. The code for serializing the Imath types follows the existing standard provided by Time and TimeRange.

src/opentimelineio/errorStatus.cpp
Adds a new error value for when the bounds cannot be determined. This occurs when the user asks for bounds on a Clip that does not contain bounds or asks for bounds on a Composition that has a Clip that does not contain bounds.

src/opentimelineio/externalReference.cpp, src/opentimelineio/generatorReference.cpp, src/opentimelineio/imageSequenceReference.cpp, src/opentimelineio/missingReference.h
Pass bounds constructor argument to MediaReference parent constructor

src/opentimelineio/externalReference.h, src/opentimelineio/generatorReference.h, src/opentimelineio/imageSequenceReference.h, src/opentimelineio/missingReference.h
Adds optional constructor argument for bounds.

src/opentimelineio/mediaReference.cpp
Initializes bounds member and add it to the read/write method for serialization. A Retainer is used to store Bounds so the memory can be managed in the Python bindings similar to the Effect class.

src/opentimelineio/mediaReference.h
Add bounds member and constructor argument as well as a mutator and accessor method.
Python Bindings: Also added (includes derived classes)

src/opentimelineio/safely_typed_any.cpp
Adds casting support for Box2d and V2d (required by serialization code).

src/opentimelineio/safely_typed_any.h
Adds casting methods for Box2d and V2d to public interface.

src/opentimelineio/serializableObject.h
Adds de/serialization support for Box2d and V2d found within the Bounds schema.

src/opentimelineio/serialization.cpp
Adds de/serialization code for Box2d and V2d.

src/opentimelineio/stack.cpp
Implements bounds method for a stack. We begin by finding the first bounding box, defined as the Box of the first Clip or of the first Composition that contains at least one Clip. After this is found, we then extend the box to contain each subsequent Clip or Composition containing at least one Clip. If no Clips are found or no Boxes are found, we return nullptr. If an error is found, we return the error (as a parameter). This follows the standard defined by the available_range method.

src/opentimelineio/stack.h
Adds bounds method to the public interface.
Python Bindings: Also added

src/opentimelineio/timeline.h
Adds bounds to the public interface and implements it by redirecting the call to it's Stack.
Python Bindings: Also added

src/opentimelineio/track.cpp
Implements bounds method for a track. Similarly to Stack, we begin by finding the first bounding box, however in this case we don't need to consider Compositions, only Clips. Once we have the bounding Box of the first clip, we extend it with the Box of each subsequent Clip. If no Clips or Boxes are found, we return nullptr. If an error is found, we return the error (as a parameter) and the default Box. This follows the standard defined by the available_range method.

src/opentimelineio/track.h
Adds bounds method to the public interface.
Python Bindings: Also added

src/opentimelineio/typeRegistry.cpp
Register Bounds as a SerializableObject.

src/py-opentimelineio/opentimelineio-bindings/CMakeLists.txt
Adds compilation of Python bindings for Imath types.

src/py-opentimelineio/opentimelineio-bindings/otio_bindings.cpp
Initialize Imath bindings

src/py-opentimelineio/opentimelineio-bindings/otio_bindings.h
Imath bindings function prototype

src/py-opentimelineio/opentimelineio-bindings/otio_imath.cpp
Create pybind bindings for Imath types.
Note: I did not use the existing Python bindings for Imath here because they are done using boost. Using them would create a dependency on boost that we want to avoid. We don't want to break the self-contained nature of the Python binding libraries by depending on a dynamic system library. Because of this if we want to use the Imath types in the Python OTIO library with the Imath Python library, we would need to create some kind bridge code.

src/py-opentimelineio/opentimelineio-bindings/otio_serializableObjects.cpp
Adds all added opentimelineio public c++ interface methods to the Python bindings
Note: These are denoted by Python Bindings in descriptions above.

src/py-opentimelineio/opentimelineio/core/mediaReference.py
Adds bounds to the repr and str method for MediaReference.

src/py-opentimelineio/opentimelineio/schema/init.py
Import schemas for Bounds, Box2d and V2d.

src/py-opentimelineio/opentimelineio/schema/bounds.py
Create a Bounds schema

src/py-opentimelineio/opentimelineio/schema/box2d.py
Create a Box schema

src/py-opentimelineio/opentimelineio/schema/image_sequence_reference.py
Adds bounds to the repr and str method for ImageSequence.

src/py-opentimelineio/opentimelineio/schema/v2d.py
Create a Vector schema

Tests

tests/baselines/*.json
Adds empty bounds to the json representations
Note: This is necessary even if the bounds don't exist in the metadata since the serialization code will add a null bounds member in this case. If you'd prefer I could make it exclude the bounds member entirely if it is not defined.

tests/sample_data/clip_example.otio
Added bounds to the example clip and associated test (test_documentation.py).

tests/test_bounds.py
Test all the public bounds methods. This is essentially just the schemas and getting and setting the box.

tests/test_box2d.py
Test all public Box2d methods.
Note: I did test each method but only on a basic level to ensure it was bound to the correct underlying C++ method. Imath has its own more rigorous unit testing to make sure the methods are working correctly.

tests/test_clip.py
Updated the str test to include the bounds. Also adds a bounding box to a clip and tests that they were set correctly using the contains methods to find the edges of the Box.

tests/test_composition.py
Tests the added has_clips method on empty stacks and tracks and ones containing no Clips (only Gaps). Also tests that the bounds are correctly applied to empty Stacks, single clip Stacks, multi-clip Stacks, Stacks with gaps, and Stacks containing Tracks containing Clips. Tracks are also tested for empty Tracks, gap-only Tracks, single Clip Tracks and multi-Clip Tracks.

tests/test_media_reference.py
Tests for new bounds in schema

tests/test_v2d.py
Test all public V2d methods.
Note: I did test each method but only on a basic level to ensure it was bound to the correct underlying C++ method. Imath has its own more rigorous unit testing to make sure the methods are working correctly.

tests/test_generator_reference.py, tests/test_image_sequence_reference.py, tests/test_media_reference.py
Ensures the bounds can be passed to the constructor and the same bounds are returned from the accessor.
ImageSequenceReference and MediaRefererence also test the str and repr methods return the expected string.

tests/test_timeline.py
Timeline bounds are tested to ensure a single track timeline has equal bounds between the Track and the Timeline.

@jminor jminor requested review from meshula and reinecke February 15, 2022 20:55
@meshula meshula merged commit 8f93157 into AcademySoftwareFoundation:main Feb 17, 2022
jminor pushed a commit that referenced this pull request May 2, 2022
* Spatial Bounds with Imath (#1022)

* Spatial Bounds with Imath
* Use Box2d as POD

* bounds -> available_image_bounds and remove imath inclusives (#1087)

* Add spatial coords doc draft (#1010)

* Updated spatial coordinates doc : available_image_bounds. (#1142)

Co-authored-by: Eric Desruisseaux <eric.desruisseaux@autodesk.com>

* Support available_image_bounds in the "Example OTIO Reader" RV plugin (#1136)

* fix compilation errors

* clang format

Co-authored-by: Stephan Steinbach <61017+ssteinbach@users.noreply.github.com>
Co-authored-by: Eric Desruisseaux <47835480+desruie@users.noreply.github.com>
Co-authored-by: Eric Desruisseaux <eric.desruisseaux@autodesk.com>
@ssteinbach ssteinbach added this to the Public Beta 15 milestone Sep 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MediaReference: spatial bounding box

5 participants