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

Edit commands #1650

Closed
wants to merge 15 commits into from
Closed

Conversation

ggarra13
Copy link
Contributor

@ggarra13 ggarra13 commented Aug 17, 2023

Signed-off-by: Gonzalo Garramuño [email protected]

This commit is for:

Implements #1649.
Makes PR #1518 obsolete.

It implements edit commands in the otio::algo:: namespace. The commands implemented are those discussed in #711.

They are further explained more clearly in:
https://github.com/mccartnm/OpenTimelineIO/blob/openedit_design/docs/design/editorial_design.md

Summarize your change.

It implements the needed base commands for editing with otio, building on top of @darbyjohnston's PR.

@darbyjohnston 's PR #1518 was the base for it, but I debugged two subtle precision bugs in his code and moved it all to the algo:: namespace.

Implementation also modifies errorStatus.h/cpp to add NOT_A_GAP enum.

Implementation, as @darbyjohnston's, makes Composition's protected _index_of_child a now public member of the API as index_of_child and updates track.cpp accordingly.

Reference associated tests.

test_editAlgorithm.cpp is added as in @darbyjohnston, but with tests for all commands.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 17, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@codecov-commenter
Copy link

Codecov Report

Merging #1650 (b5667d8) into main (eaf8569) will decrease coverage by 0.01%.
The diff coverage is 23.07%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1650      +/-   ##
==========================================
- Coverage   79.91%   79.90%   -0.01%     
==========================================
  Files         197      197              
  Lines       21731    21733       +2     
  Branches     4339     4340       +1     
==========================================
  Hits        17366    17366              
- Misses       2213     2215       +2     
  Partials     2152     2152              
Flag Coverage Δ
py-unittests 79.90% <23.07%> (-0.01%) ⬇️

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

Files Changed Coverage Δ
src/opentimelineio/composition.h 41.37% <ø> (ø)
src/opentimelineio/errorStatus.cpp 28.33% <0.00%> (-0.98%) ⬇️
src/opentimelineio/errorStatus.h 86.95% <ø> (ø)
src/opentimelineio/track.cpp 52.85% <0.00%> (ø)
src/opentimelineio/composition.cpp 47.58% <30.00%> (ø)

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 eaf8569...b5667d8. Read the comment docs.

@meshula
Copy link
Collaborator

meshula commented Aug 18, 2023

This looks pretty cool! Have incorporated these commands into an app, or have they been exercised through the unit tests, so far?

@ggarra13
Copy link
Contributor Author

Have incorporated these commands into an app, or have they been exercised through the unit tests, so far?

For the time being, only the unit tests, which I tried to be thorough. Darby is working on adding a GUI front end to tlRender and he may be incorporating these commands.

@ggarra13 ggarra13 closed this Aug 18, 2023
@ggarra13 ggarra13 reopened this Aug 18, 2023
@ggarra13 ggarra13 closed this Aug 18, 2023
@ggarra13 ggarra13 deleted the edit_commands branch August 18, 2023 18:30
@ggarra13 ggarra13 restored the edit_commands branch August 18, 2023 18:31
@ggarra13 ggarra13 deleted the edit_commands branch August 18, 2023 18:31
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