Issue 912: cmx_3600 adapter incorrectly processing three-clip transitions

[JoshBurnell]

the issue is actually in how the adapter is adjusting the duration of the clip after the transition. Specifically, in how it's defining the "next clip."

https://github.com/PixarAnimationStudios/OpenTimelineIO/blob/84af5e5a30e4d14cf3434d6d055631e115ac1ee5/src/py-opentimelineio/opentimelineio/adapters/cmx_3600.py#L731-L749

To explain, here's an EDL with a simple dissolve:

001  ABC0010. V     C        01:00:06:15 01:00:08:18 01:04:12:23 01:04:15:02
002  ABC0010. V     C        01:00:08:18 01:00:08:18 01:04:15:02 01:04:15:02
002  ABC0020. V     D    035 01:00:06:22 01:00:10:07 01:04:15:02 01:04:18:11
* BLEND, DISSOLVE

In this case, when the adapter gets to line three, it hits the above code. It adds the Transition and adds half of the transition's runtime to the "next clip", which is also line 3.

However, in the case of an A->B->C transition, the adapter gets confused. It sees line 3 and line 4 as continuous and thus, it adds the Transition length to line 4, which is a 0-length transition marker.

001  ABC0010. V     C        01:00:06:15 01:00:08:18 01:04:12:23 01:04:15:02
002  ABC0010. V     C        01:00:08:18 01:00:08:18 01:04:15:02 01:04:15:02
002  ABC0020. V     D    035 01:00:06:22 01:00:10:07 01:04:15:02 01:04:18:11
* BLEND, DISSOLVE
003  ABC0020. V     C        01:00:10:07 01:00:10:07 01:04:18:11 01:04:18:11
003  ABC0030. V     D    064 01:00:06:10 01:00:09:22 01:04:18:11 01:04:21:23
* BLEND, DISSOLVE

In the next loop, the adapter then removes line 4 (because it's just a transition marker) and we lose half the duration of the transition.

I fixed it by tweaking the "next_clip" logic so that, rather than assuming the next clip is "whatever's next," it defines the next clip as "the next clip with a duration."

def _get_next_clip(start_index, track):
    # Iterate over the following clips and return the first "real" one
    for index in range(start_index+1, len(track)):
        clip = track[index]
        if clip.duration().value > 0:
            return clip

    return None

(This also led me to simplify the loop in _expand_transitions() and remove the iter().)

This works for me and all the tests are passing now, however the entirety of _expand_transitions() feels a little overcomplicated to me, and could possibly be simplified. Unfortunately, in attempting to simplify it, I broke other tests in the test suite, so I don't think I fully grok the entirety of what it's doing.

[codecov-io]

Codecov Report

Merging #919 (83534ac) into master (600a9d7) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #919      +/-   ##
==========================================
+ Coverage   85.70%   85.71%   +0.01%     
==========================================
  Files         184      184              
  Lines       17723    17737      +14     
  Branches     1993     1995       +2     
==========================================
+ Hits        15190    15204      +14     
  Misses       2024     2024              
  Partials      509      509              

Flag	Coverage Δ
unittests	85.71% <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.55% <100.00%> (ø)
tests/test_cmx_3600_adapter.py	98.95% <100.00%> (+0.04%)	⬆️

---

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 600a9d7...83534ac. Read the comment docs.

[ssteinbach]

One small suggestion, otherwise LGTM.

[ssteinbach]

Looks great, thanks!