Skip to content
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

Add implementation of to_nearest_timecode #1717

Merged

Conversation

vade
Copy link
Contributor

@vade vade commented Apr 4, 2024

Add implementation of to_nearest_timecode which makes approximate conversion of timecode from a rate using the closest valid time code rate.

This is intended to fix various downstream adaptor hiccups where source material might be variable frame rate, or close but not quite an exact SMPTE timecode.

Link the Issue(s) this Pull Request is related to.

#830

Summarize your change.

This PR does not directly fix the FCP_XML adaptor, but rather isolates changes to OTIO core and exposes a to_nearest_timecode with the same method signature as to_timecode.

This is intended to act as a entry point for changes to other adaptors if deemed a valid approach

Reference associated tests.

No new tests just yet.

@codecov-commenter
Copy link

codecov-commenter commented Apr 4, 2024

Codecov Report

Attention: Patch coverage is 19.04762% with 17 lines in your changes are missing coverage. Please review.

Project coverage is 79.89%. Comparing base (2b9c3a4) to head (1207631).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1717      +/-   ##
==========================================
- Coverage   79.95%   79.89%   -0.06%     
==========================================
  Files         197      197              
  Lines       21879    21900      +21     
  Branches     4342     4351       +9     
==========================================
+ Hits        17494    17498       +4     
- Misses       2252     2269      +17     
  Partials     2133     2133              
Flag Coverage Δ
py-unittests 79.89% <19.04%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/opentime/rationalTime.h 93.61% <ø> (ø)
src/py-opentimelineio/opentimelineio/opentime.py 95.00% <50.00%> (-5.00%) ⬇️
src/opentime/rationalTime.cpp 61.96% <0.00%> (-2.01%) ⬇️
...lineio/opentime-bindings/opentime_rationalTime.cpp 55.26% <27.27%> (-2.99%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b9c3a4...1207631. Read the comment docs.

Copy link
Collaborator

@meshula meshula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm; if you wouldn't mind doing the DCO steps indicated in the CI, that should clear up our ability to merge.

vade added 3 commits April 5, 2024 13:29
…version of timecode from a rate using the closest valid time code rate.

Signed-off-by: Anton Marini <[email protected]>
@vade vade force-pushed the fixes/to_nearest_timecode branch from 810b27b to 1207631 Compare April 5, 2024 17:30
@vade
Copy link
Contributor Author

vade commented Apr 5, 2024

Thanks for your patience. DCO Sign off passed. I'll try to keep that in mind in the future!

@meshula meshula merged commit 7755f31 into AcademySoftwareFoundation:main Apr 9, 2024
45 checks passed
@vade vade deleted the fixes/to_nearest_timecode branch April 11, 2024 17:39
@reinecke reinecke added this to the Public Beta 17 milestone Jun 24, 2024
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.

4 participants