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

EDL CMX 3600 Adapter: Fix Source Out calculation when a TimeWarp retiming effect is present #1459

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ghost-luck
Copy link

Signed-off-by: Nikita Glukhov [email protected]

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

Fixes #1458

Summarize your change.

Fixed an error in calculating the Source Out timecode value.

The division is replaced by multiplication. Fixed math of clip length summation.

Signed-off-by: Nikita Glukhov <[email protected]>
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 17, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

Copy link
Collaborator

@ssteinbach ssteinbach left a comment

Choose a reason for hiding this comment

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

Can you please include a unit test? The EDL you included in the issue would be perfect. The unit tests are here: https://github.com/AcademySoftwareFoundation/OpenTimelineIO/blob/main/tests/test_cmx_3600_adapter.py

src/py-opentimelineio/opentimelineio/adapters/cmx_3600.py Outdated Show resolved Hide resolved
@ssteinbach ssteinbach changed the title Fixes the 1458 issue. EDL CMX 3600 Adapter: Fix sourceOut calculation when a TimeWarp retiming effect is present Oct 18, 2022
@ssteinbach ssteinbach added this to the Public Beta 16 milestone Oct 18, 2022
@ssteinbach ssteinbach changed the title EDL CMX 3600 Adapter: Fix sourceOut calculation when a TimeWarp retiming effect is present EDL CMX 3600 Adapter: Fix Source Out calculation when a TimeWarp retiming effect is present Oct 18, 2022
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 31, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

@ghost-luck
Copy link
Author

ghost-luck commented Oct 31, 2022

Can you please include a unit test?

Added.

ghost-luck and others added 2 commits November 3, 2022 20:09
Co-authored-by: Stephan Steinbach <[email protected]>
Signed-off-by: Nikita Glukhov <[email protected]>
@reinecke
Copy link
Collaborator

reinecke commented Jun 8, 2023

Going to close and re-open to trigger CI

@reinecke reinecke closed this Jun 8, 2023
@reinecke reinecke reopened this Jun 8, 2023
@meshula
Copy link
Collaborator

meshula commented Jun 8, 2023

CI is failing for a reason that seems unrelated to this PR, I can't see the trailing whitespace. line 1003 is not in the PR, nor does it have 84 characters on it???? @ghost-luck @reinecke can you see it?

lists of files in version control and sdist match
check-manifest succeeded
./src/py-opentimelineio/opentimelineio/adapters/cmx_3600.py:1003:84: W291 trailing whitespace
make: *** [Makefile:159: lint] Error 1
Error: Process completed with exit code 2.

@reinecke
Copy link
Collaborator

@ghost-luck So sorry to have lost track of this item. I've been doing a re-build of the CMX reader from scratch to address a collection of these kinds of issues. Would you be able to take a look at that branch and let me know if it addresses this issue?

When I ran your sample test case through my branch, I got this error:

E           AssertionError: 'TITL[19 chars]ARP\n\n001  B113RPTW V     C        00:32:02:0[100 chars]01\n' != 'TITL[19 chars]ARP\n001  B113RPTW V     C        00:32:02:06 [98 chars]01\n'
E             TITLE: SOURCE_OUT_TIMEWARP
E           -
E           - 001  B113RPTW V     C        00:32:02:06 00:32:06:20 01:34:53:09 01:34:57:13
E           ?                                                   ^^
E           + 001  B113RPTW V     C        00:32:02:06 00:32:06:19 01:34:53:09 01:34:57:13
E           ?                                                   ^^
E             M2   000001		26.4			00:32:02:06
E             * FROM CLIP NAME:  000001

tests/test_cmx_3600_adapter.py:1429: AssertionError

Is it possible this off-by-one is a rounding issue?

@reinecke reinecke removed this from the Public Beta 16 milestone Mar 25, 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.

EDL CMX 3600 Adapter: EDL files with TimeWarp retiming effects do not round trip correctly
4 participants