OpenTimelineIO / otio-aaf-adapter

OpenTimelineIO Advanced Authoring Format (AAF) Adapter
Apache License 2.0
16 stars 6 forks source link

Fix Transition Marker Issues #20

Closed markreidvfx closed 8 months ago

markreidvfx commented 1 year ago

This pull request fixes few issues I've discovered with the adapter relating to markers and transitions.

Before: image

After:

image

Reference:

image

There are 2 main issues happening here. The first big issue is marker positions are not stored with transition offsets. The fix is to attach the marker after the transition lengths have been corrected. This does move the _fix_transitions to happen sooner in the transcribe process but I haven't noticed any issues doing this and all the tests still pass.

The second issue is markers that land on top of transitions are getting lost. This is because transition don't have a markers attr. The fix is to find and attach the marker to the clip underneath.

The added test verifies the marker timings exactly against the timings exported by Media Composer. Both the absolute offsets and relative to clip offsets.

codecov[bot] commented 8 months ago

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (72a811f) 90.15% compared to head (6140cc2) 90.00%.

Files Patch % Lines
..._aaf_adapter/adapters/advanced_authoring_format.py 76.19% 5 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #20 +/- ## ========================================== - Coverage 90.15% 90.00% -0.15% ========================================== Files 3 3 Lines 1107 1121 +14 ========================================== + Hits 998 1009 +11 - Misses 109 112 +3 ``` | [Flag](https://app.codecov.io/gh/OpenTimelineIO/otio-aaf-adapter/pull/20/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OpenTimelineIO) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/OpenTimelineIO/otio-aaf-adapter/pull/20/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OpenTimelineIO) | `90.00% <76.19%> (-0.15%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OpenTimelineIO#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

jminor commented 8 months ago

We're not sure why codecov is unhappy with this. Will explore in more detail...

jminor commented 8 months ago

I found the codecov settings. https://github.com/OpenTimelineIO/otio-aaf-adapter/pull/30 should make the codecov checks "informational" instead of blocking for now - we can adjust them more as we get used to them.