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

Revisited how transitions are handled by the cmx 3600 adapter. #1341

Merged
merged 1 commit into from
Jul 13, 2022

Conversation

gplsteph
Copy link
Contributor

The code handling transitions in the cmx 3600 adapter was hard to follow and was introducing problems. These changes
fix these problems and make the code easier to maintain.

  • Transitions lines are now collected when parsing EDL lines and treated by the ClipHandler.
  • The Clip source and record points are replaced with the transition values when the line is parsed.
  • The transition in_offset is now 0 and the out_offset is the transition duration, matching in the timeline what would be done if the editing was read from another format.
  • The transition is added just before the clip when it is added to a track, and that's it.
  • Adjusted tests for the new way of handling transitions, added some other checks.
  • Added an EDL read/write/read roundtrip to check that the reader and writer are compatible.
  • Removed code which was not needed anymore.

Fixes #1328
Fixes #912
Fixes #977

Now the transitions read from an EDL appear at the right time in the timeline, and the timeline has the right overall length.
EDL transitions

Associated tests.
These changes are covered by the test_cmx_3600_adapter tests. A read/write/read roundtrip test was added to ensure they are compatible with the writer.

commit beac098
Merge: 98a85b8 d48163e
Author: Stephane Deverly <[email protected]>
Date:   Thu Jun 23 18:30:44 2022 +0200

    Merge remote-tracking branch 'asf/main'

commit 98a85b8
Author: Stéphane Deverly <[email protected]>
Date:   Thu Jun 23 17:12:27 2022 +0200

    8956 cmx transitions (#1)

    Revisited how transitions are handled by the cmx 3600 adapter.

    - Transitions lines are now collected when parsing EDL lines and treated by the ClipHandler.
    - The clip source and record points are replaced with the transition values when the line is parsed.
    - The transition in_offset is now 0 and the out_offset is the transition duration, matching in the timeline what would be done if the editing was read from another format.
    - The transition is added just before the clip when it is added to a track, and that's it.
    - Adjusted tests for the new way of handling transitions, added some other checks.
    - Added an EDL read/write/read roundtrip to check that the reader and writer are compatible.
    - Removed code which was not needed anymore.

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

linux-foundation-easycla bot commented Jun 27, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: gplsteph / name: Stéphane Deverly (23dfc87)

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.

This is a clever way to solve the awkward parsing problem CMX transitions pose!
My only reservation is that the result doesn't exactly look like a user would expect it to, but the parsing is much more robust in this case so I think it's a good trade-off. As long as consumers are properly handling "imbalanced" transition overlaps, I don't think this should cause an issue.
Thanks for the contribution!

@reinecke reinecke merged commit f54569f into AcademySoftwareFoundation:main Jul 13, 2022
@ssteinbach ssteinbach added this to the Public Beta 15 milestone Aug 29, 2022
MichaelPlug pushed a commit to MichaelPlug/OpenTimelineIO that referenced this pull request Aug 5, 2023
…ySoftwareFoundation#1341)

Author: Stéphane Deverly <[email protected]>

    Revisited how transitions are handled by the cmx 3600 adapter.

    - Transitions lines are now collected when parsing EDL lines and treated by the ClipHandler.
    - The clip source and record points are replaced with the transition values when the line is parsed.
    - The transition in_offset is now 0 and the out_offset is the transition duration, matching in the timeline what would be done if the editing was read from another format.
    - The transition is added just before the clip when it is added to a track, and that's it.
    - Adjusted tests for the new way of handling transitions, added some other checks.
    - Added an EDL read/write/read roundtrip to check that the reader and writer are compatible.
    - Removed code which was not needed anymore.

Signed-off-by: Stephane Deverly <[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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants