Skip to content

Add support for Premiere styled EDLs to the CMX 3600 adapter #1199

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Nov 14, 2022
Merged

Add support for Premiere styled EDLs to the CMX 3600 adapter #1199

merged 4 commits into from
Nov 14, 2022

Conversation

douglascomet
Copy link
Contributor

@douglascomet douglascomet commented Jan 22, 2022 •
edited
Loading

Fixes

[1178] Premiere style for cmx3600

Summary.

Adds support for Premiere style for cmx3600 to limit the auto added comments that are added per clip when writing out an EDL.

Tests.

No new tests were added but the below is failing when running make test yet nearest_valid_timecode_rate seem like it should be available from the C++ class definition:

image

Additionally, when I attempt to run make lint I get this error even though pydocstyle is in my venv:
image

@douglascomet douglascomet changed the title Cmx 3600 premiere style CMX 3600 premiere style Jan 22, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #1199 (3609d52) into main (7f3b381) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1199      +/-   ##
==========================================
- Coverage   86.08%   86.07%   -0.01%     
==========================================
  Files         191      191              
  Lines       19228    19226       -2     
  Branches     2292     2292              
==========================================
- Hits        16552    16549       -3     
  Misses       2123     2123              
- Partials      553      554       +1
Flag Coverage Δ
unittests 86.07% <100.00%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
...opentimelineio/opentimelineio/adapters/cmx_3600.py 85.46% <100.00%> (-0.03%) ⬇️
...c/py-opentimelineio/opentimelineio/adapters/svg.py 90.19% <0.00%> (-0.22%) ⬇️

Continue to review full report at Codecov.

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

reinecke
reinecke approved these changes Feb 3, 2022
View reviewed changes
Copy link
Collaborator

@reinecke reinecke left a comment

Choose a reason for hiding this comment

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

I think we could merge this and be fine as is, I just had one minor thought. Thanks for the fix!

@reinecke
Copy link
Collaborator

reinecke commented Feb 3, 2022

Oops, sorry, one other note - would you mind adding a unittest to check the behavior?
Here is an example of the unittest for the nucoda style:

@meshula
Copy link
Collaborator

meshula commented Feb 23, 2022

@douglascomet This looks ready to merge, did you have any thoughts about @reinecke's request for a test?

reinecke
reinecke approved these changes Feb 28, 2022
View reviewed changes
Copy link
Collaborator

@reinecke reinecke left a comment

Choose a reason for hiding this comment

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

Looks good!

@apetrynet
Copy link
Contributor

Hi, @douglascomet !
Seems like this PR has been stuck on a failing test run for some time. In fact I'm unable to re-run it.
Would you mind re-basing your branch and pushing the updates again for the tests to update?

@douglascomet douglascomet force-pushed the cmx_3600_premiere_style branch from e684d8c to a415c43 Compare November 11, 2022 14:14
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 11, 2022 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@douglascomet douglascomet force-pushed the cmx_3600_premiere_style branch from 9939090 to a7cc58c Compare November 12, 2022 01:09
@douglascomet
Copy link
Contributor Author

@apetrynet, thanks to your help we have resolved all the CI issues this PR was having!

@apetrynet apetrynet added this to the Public Beta 16 milestone Nov 14, 2022
@apetrynet apetrynet changed the title CMX 3600 premiere style Add support for Premiere styled CMX 3600 EDLs Nov 14, 2022
@apetrynet apetrynet changed the title Add support for Premiere styled CMX 3600 EDLs Add support for Premiere styled EDLs to the CMX 3600 adapter Nov 14, 2022
@apetrynet apetrynet merged commit 0c80e91 into AcademySoftwareFoundation:main Nov 14, 2022
@apetrynet
Copy link
Contributor

Thank you for your contribution @douglascomet !

@apetrynet apetrynet mentioned this pull request Nov 14, 2022
Closed
MichaelPlug pushed a commit to MichaelPlug/OpenTimelineIO that referenced this pull request Aug 5, 2023
@douglascomet @MichaelPlug
Add support for Premiere styled EDLs to the CMX 3600 adapter (Academy…
6ef21be 
…SoftwareFoundation#1199)

* Converted VALID_EDL_STYLES into a dict to consolidate how a style is applied and support a null value for the added Premiere style
* Update style spec and added fallback OTIO comment if style does not have a spec
* Added sample edl for avid and premiere

Signed-off-by: Doug Halley <[email protected]>
Signed-off-by: Michele Spina <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reinecke reinecke reinecke approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Public Beta 16
Development

Successfully merging this pull request may close these issues.
5 participants
@douglascomet @codecov-commenter @reinecke @meshula @apetrynet