-
Notifications
You must be signed in to change notification settings - Fork 298
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
Added support for custom adapter hooks #1801
base: main
Are you sure you want to change the base?
Conversation
@ssteinbach I finally had some time to revive #1711. Please have a look when you find some time. Thank you! 🙏 |
3a5ff33
to
23b28a6
Compare
Codecov ReportAttention: Patch coverage is
❌ Your changes status has failed because you have indirect coverage changes. Learn more about Unexpected Coverage Changes and reasons for indirect coverage changes. Additional details and impacted files@@ Coverage Diff @@
## main #1801 +/- ##
==========================================
+ Coverage 84.11% 84.81% +0.69%
==========================================
Files 198 177 -21
Lines 22241 12779 -9462
Branches 4687 1186 -3501
==========================================
- Hits 18709 10839 -7870
+ Misses 2610 1760 -850
+ Partials 922 180 -742
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 126 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
This adds support for attributing custom hooks to adapters and executing them with `hook_function_argument_map` being passed along through the adapter IO functions. Signed-off-by: Tim Lehr <tim.lehr@disneyanimation.com>
23b28a6
to
87f0c74
Compare
@ssteinbach @jminor Finally rebased this again 👍 |
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.
The code looks good. This could use a note in the hook scripts documentation:
https://opentimelineio.readthedocs.io/en/latest/tutorials/write-a-hookscript.html
and adding it to the pluginfo script as well would be nice. I think all you need to do is add a "hooks" field to the plugin feature map:
def plugin_info_map(self): |
Summarize your change.
Rebased PR #1711
Required by OpenTimelineIO/otio-aaf-adapter#43
This adds support for attributing custom hooks to adapters and executing them with
hook_function_argument_map
being passed along through the adapter IO functions.Describe the reason for the change.
I added two custom hooks to the OTIO AAF adapter (OpenTimelineIO/otio-aaf-adapter#43), allowing for embedding of media essence into the resulting AAF. This was needed to facilitate just-in-time DNXHR transcoding of the media for the AAF creation and adding a certain level of control and flexibility to the feature.
These features to the core are required in order to properly pass the hook argument map along to potential custom hooks run by the adapter. I tried to work with
_FEATURE_MAP
instead of creating a new version of the Adapter schema in order to minimize the impact of this change, while adding the necessary changes to facilitate custom hooks for adapters.Reference associated tests.