AcademySoftwareFoundation / OpenTimelineIO

Open Source API and interchange format for editorial timeline information.
http://opentimeline.io
Apache License 2.0
1.47k stars 293 forks source link

EDL with offset record time breaks trimmed_range_in_parent. #346

Open alatdneg opened 6 years ago

alatdneg commented 6 years ago

I'm assuming I've not done anything silly...

This EDLl causes this issue,

    parent_range = self.clip.trimmed_range_in_parent()
  File "/tools/SITE/python/opentimelineio/core/item.py", line 124, in trimmed_range_in_parent
    return self.parent().trimmed_range_of_child(self)
  File "/tools/SITE/python/opentimelineio/core/composition.py", line 379, in trimmed_range_of_child
    result_range.start_time
AttributeError: 'NoneType' object has no attribute 'start_time'
TITLE:   Ep101_090418_12b.Sub.01 
FCM: NON-DROP FRAME
000001  B007_C007_05245V_001             V     C        14:48:20:03 14:48:21:21 01:04:52:15 01:04:54:09 
*FROM CLIP NAME:  12B-09 (B) 
*LOC: 01:04:52:15 RED     TB_101_012_040 
*ASC_SOP (1.000000 1.000000 1.000000)(0.000000 0.000000 0.000000)(1.000000 1.000000 1.000000) 
*ASC_SAT 1.000000 
*SOURCE FILE: B007_C007_05245V_001
000002  B007_C007_05245V_001             V     C        14:48:41:19 14:48:49:11 01:05:10:12 01:05:18:04 
*FROM CLIP NAME:  12B-09 (B) 
*LOC: 01:05:10:12 RED     TB_101_012_070 
*ASC_SOP (1.000000 1.000000 1.000000)(0.000000 0.000000 0.000000)(1.000000 1.000000 1.000000) 
*ASC_SAT 1.000000 
*SOURCE FILE: B007_C007_05245V_001
alatdneg commented 6 years ago

It's looking like the EDL reader is setting the track source time to -01:04:52:15, I assume this is wrong and it should be +01:04:52:15 This is the source_range for the track. TimeRange(RationalTime(-93423, 23.976), RationalTime(613, 23.976))

alatdneg commented 6 years ago

This seems to be the problem, the clip source range is TimeRange(RationalTime(0, 23.976), RationalTime(42, 23.976)) Which causes it to be removed, as it falls outside the source range for the track, I'm not sure whats going on here, as the logic of the code makes no sense to me.

I thinks this code expects the child_range to already be relative to track.source_range, not 0

    def trim_child_range(self, child_range):
        if not self.source_range:
            return child_range

        # cropped out entirely
        if (
            self.source_range.start_time >= child_range.end_time_exclusive()
            or self.source_range.end_time_exclusive() <= child_range.start_time
        ):
            return None

        if child_range.start_time < self.source_range.start_time:
            child_range = opentime.range_from_start_end_time(
                self.source_range.start_time,
                child_range.end_time_exclusive()
            )

        if (
            child_range.end_time_exclusive() >
            self.source_range.end_time_exclusive()
        ):
            child_range = opentime.range_from_start_end_time(
                child_range.start_time,
                self.source_range.end_time_exclusive()
            )

        return child_range
ssteinbach commented 6 years ago

As you note, the clip is getting culled because it is outside the range of the track. The track has a massively negative start time which means all the clips are outside the range of the track.

here is a repro script:

#!/usr/bin/env python

import opentimelineio as otio

# original from al
"""
TITLE:   Ep101_090418_12b.Sub.01
FCM: NON-DROP FRAME
000001  B007_C007_05245V_001             V     C        14:48:20:03 14:48:21:21 01:04:52:15 01:04:54:09
*FROM CLIP NAME:  12B-09 (B)
*LOC: 01:04:52:15 RED     TB_101_012_040
*ASC_SOP (1.000000 1.000000 1.000000)(0.000000 0.000000 0.000000)(1.000000 1.000000 1.000000)
*ASC_SAT 1.000000
*SOURCE FILE: B007_C007_05245V_001
000002  B007_C007_05245V_001             V     C        14:48:41:19 14:48:49:11 01:05:10:12 01:05:18:04
*FROM CLIP NAME:  12B-09 (B)
*LOC: 01:05:10:12 RED     TB_101_012_070
*ASC_SOP (1.000000 1.000000 1.000000)(0.000000 0.000000 0.000000)(1.000000 1.000000 1.000000)
*ASC_SAT 1.000000
*SOURCE FILE: B007_C007_05245V_001
"""

# minimal error
EDL_STRING = """TITLE:   Ep101_090418_12b.Sub.01
FCM: NON-DROP FRAME
000001  B007_C007_05245V_001             V     C        14:48:20:03 14:48:21:21 01:04:52:15 01:04:54:09
*FROM CLIP NAME:  12B-09 (B)
*LOC: 01:04:52:15 RED     TB_101_012_040
*SOURCE FILE: B007_C007_05245V_001
000002  B007_C007_05245V_001             V     C        14:48:41:19 14:48:49:11 01:05:10:12 01:05:18:04
*FROM CLIP NAME:  12B-09 (B)
*LOC: 01:05:10:12 RED     TB_101_012_070
*SOURCE FILE: B007_C007_05245V_001
"""

def main():
    edl = otio.adapters.read_from_string(EDL_STRING, "cmx_3600", rate=23.976)

    # has a questionable source_range (-something big?)
    bad_track = edl.tracks[0]

    bad_clip = bad_track[0]

    # (-93423, 613)
    bad_range  = bad_track.range_of_child_at_index(0)
    # (0, 42)
    clip_range = bad_clip.source_range

    # for debugging
    import ipdb; ipdb.set_trace()

    # causes crash
    bad_clip.trimmed_range_in_parent()
    # bad_track.trim_child_range(clip_range)

if __name__ == '__main__':
    main()

I found a bug in trimmed_range_of_child that wasn't looking for a None in the right place, which causes your crash, so I have a fix for that. However, I think the bigger problem is the massively negative start_time, that seems wrong to me. As you note, it means that all the children are outside the range of the track.

The code in the EDL adapter that is setting the track source_range to be massively negative is this: https://github.com/PixarAnimationStudios/OpenTimelineIO/blob/9fc219539f93148dffc5a0ccf1044a7ffe4fb6df/opentimelineio/adapters/cmx_3600.py#L177

Am investigating further... will get back to you.

ssteinbach commented 6 years ago

This test also fails (if added to the cmx edl adapter unit tests):

    def test_children_inside_track(self):
        edl_path = SCREENING_EXAMPLE_PATH
        timeline = otio.adapters.read_from_file(edl_path)
        for cl in timeline.each_clip():
            self.assertIsNotNone(cl.trimmed_range_in_parent())
        self.assertTrue(timeline.tracks[0].source_range.start_time.value > 0)
ssteinbach commented 6 years ago

@alatdneg can you try this? https://github.com/PixarAnimationStudios/OpenTimelineIO/pull/348