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

#1677 otiotool verify ranges #1779

Merged

Conversation

semagnum
Copy link
Contributor

Fixes #1677

Summary

Adds --verify-ranges option for otiotool's inspection feature, verifies that the clip's source range doesn't start before the available start range or ends beyond it (if the available range exists).

Signed-off-by: Spencer Magnusson <[email protected]>
Signed-off-by: Spencer Magnusson <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Jul 10, 2024

Codecov Report

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

Project coverage is 81.60%. Comparing base (c0e97b0) to head (becc2a0).
Report is 18 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1779      +/-   ##
==========================================
- Coverage   84.11%   81.60%   -2.52%     
==========================================
  Files         198      176      -22     
  Lines       22241    12333    -9908     
  Branches     4687     3031    -1656     
==========================================
- Hits        18709    10064    -8645     
+ Misses       2610     1733     -877     
+ Partials      922      536     -386     
Flag Coverage Δ
py-unittests 81.60% <15.00%> (-2.52%) ⬇️

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

Files Coverage Δ
...-opentimelineio/opentimelineio/console/otiotool.py 63.37% <15.00%> (-2.93%) ⬇️

... and 65 files with indirect coverage changes


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 e10fdd2...becc2a0. 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

@jminor
Copy link
Collaborator

jminor commented Jul 15, 2024

This change looks good to me. One thing we should clarify though is that there are plenty of cases where it is valid for an OTIO Clip to reference media outside the available range of its media. Having this tool to check for the cases where that isn't okay is helpful for sure, but that isn't always an error case.

@semagnum
Copy link
Contributor Author

One thing we should clarify though is that there are plenty of cases where it is valid for an OTIO Clip to reference media outside the available range of its media. Having this tool to check for the cases where that isn't okay is helpful for sure, but that isn't always an error case.

Agreed. Are you wanting that clarified in the verify message, or in the argument help?

@jminor
Copy link
Collaborator

jminor commented Jul 16, 2024

I think something brief in the usage message for that option would be wonderful. Maybe it could say something like this?

"Verify that each clip in a timeline has a source range within the available range of media (acceptable in some use cases, not in others)"

That would be just enough of a hint to users that it depends on the use case.

Thanks :)

Signed-off-by: Spencer Magnusson <[email protected]>
@ssteinbach ssteinbach added this to the Public Beta 18 milestone Jul 17, 2024
Signed-off-by: Spencer Magnusson <[email protected]>
@jminor jminor merged commit 5184c36 into AcademySoftwareFoundation:main Aug 6, 2024
56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

otiotool --verify-ranges
5 participants