-
Notifications
You must be signed in to change notification settings - Fork 293
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Media Multi-Reference Feature #1241
Media Multi-Reference Feature #1241
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment, the ergonomics look like they'd be pretty good on this!
I just had a few comments for discussion about having a standard set of reference keys and bike shedding the name of `active_media_reference.
@rogernelson This proposal has iterated greatly in a good direction, since the original document https://docs.google.com/document/d/13jrl2qtWBTSFqMTVQh28LWc16uYvQ79w-7TfX92JaIM/edit?usp=sharing and a great many decisions have been taken since then, making it hard to parse the implementation versus the design intention. As I read through the comments and changes, I keep thinking I wish I could refer the current state against a technical design document. Could the current state of the design and its behaviors be added to the existing document? I think a paragraph summary of the addition to the schema, and the current state of the design would suffice. I added a Current Status block at the document, pointing to this PR, as a place where I thought it could go. |
Done. I listed the API additions as well as describing their intended usage. I also tried to summarize the various points of discussion that have come up here. Please let me know if there's anything else you think should be in there. |
The python 2.7 build is failing because of this:
The difference is str vs. unicode I build the python bindings with python3, so that probably explains why 2.7 is failing. But I am curious, why is it putting in the function signature for |
Codecov Report
@@ Coverage Diff @@
## main #1241 +/- ##
==========================================
+ Coverage 86.15% 86.18% +0.03%
==========================================
Files 196 196
Lines 19632 19716 +84
Branches 2302 2302
==========================================
+ Hits 16913 16992 +79
- Misses 2161 2166 +5
Partials 558 558
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
src/py-opentimelineio/opentimelineio/console/autogen_serialized_datamodel.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, I left some questions about corner cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the last minute touch ups! lgtm
Thanks for the contribution @rogernelson !!! |
* Upgraded the Clip API and schema to support multiple media references in a single Clip. * New API is introduced to allow for setting a dictionary of media references indexed by a string key, and API for choosing which of those is active. The old API for getting/setting a single media reference is retained for backwards compatibility, and for use cases where only a single media reference is needed. * Note that the Clip schema version is updated to Clip.2, which is not supported by older versions of OTIO.
Fixes #630
Summarize your change.
This branch is an initial prototype for the changes and discussion initially proposed in this working document.
To summarize, there are often uses cases when a single conceptual clip can have multiple different media representations. A common example is high-resolution and proxy-resolution representations of the same media where the high-resolution is used when users have access to the file path and hardware appropriate for playback and proxy-resolution for when they do not. In these situations it's convenient for the Clip to have a reference to each media representation so we can easily toggle between them based on our needs.
To accommodate these workflows, this branch transforms the single media reference contained within a clip into a map/dictionary of media references. Below is what the difference looks like when serialized.
before:
after:
As can be seen above, the later format is extensible allowing new media references to be added to the
media_references
map. The key set in theactive_media_reference
will be the media reference returned using the existingmedia_reference
properties. To change the active reference, the user can simply just pass the name of the key of the new active reference to the newactive_media_reference
property.Reference associated tests.
test_clip.{py,cpp} were modified to test the new functionality. All other existing tests access that media_reference (test_clip, test_track, test_stack, etc) ensure the existing functionality continues to work.