AcademySoftwareFoundation / OpenTimelineIO

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

EDL Adapter: should author and read timeline::global_start_time attribute rather than source_range.start_time of track #1385

Open reinecke opened 2 years ago

reinecke commented 2 years ago

To Reproduce

import opentimelineio as otio

# Read the screening example from the sample data
timeline = otio.adapters.read_from_file("tests/sample_data/screening_example.edl")

# Note that the source range on the track is offset to make the start timecode work - this assertion passes
assert timeline.tracks[0].source_range.start_time == otio.opentime.RationalTime(value=-86243, rate=24)

# This assertion should pass, but does not because global_start_time is None
assert timeline.global_start_time = otio.opentime.from_timecode("00:59:53:11", 24)

# We should be able to set global_start_time and make the first event record time start at the provided tc
timeline.global_start_time = otio.opentime.from_timecode("02:00:00:00", 24)

# The first event of the written out timeline should have a record start TC of 02:00:00:00
otio.adapters.from_name("cmx_3600").write_to_string(timeline)

Expected Behavior

I would expect that the first event's record start timecode would be set as the global_start_time on the timeline, and that the global_start_time on the timeline would be the first event record time - rather than relying on the source_range on tracks to set the time.

apetrynet commented 1 year ago

Hi TrevorAyl!

It seems your comments didn't make it into the comment stream on github. Would you mind summarizing your comments there so we have them recorded?

On Mon, Oct 23, 2023 at 12:47 AM TrevorAyl @.***> wrote:

Well I've had a little look - seems that the code is written assuming A mode. Not sure if explicitly setting the global_start_time to None is necessary but I added this when the timeline is initialized...

class EDLParser: def init(self, edl_string, rate=24, ignore_timecode_mismatch=False): self.timeline = schema.Timeline() self.timeline.global_start_time = None

Then added the following to the 'add_clip' function to set the global.start_time to be the first record_in the code comes across (I think, I'm not a coder).

` edl_rate = clip_handler.edl_rate record_in = opentime.from_timecode( clip_handler.record_tc_in, edl_rate )

# This should set the record_start to be the first record_in
# The whole code does seem to assume A mode EDL
# (i.e. so first event number would be first clip added and so earliest record in)
if self.timeline.global_start_time is None:
  # Set the sequence start time
  self.timeline.global_start_time = record_in
elif (record_in < self.timeline.global_start_time)
  raise EDLParseError(
      "Record In is less than Record Start: {} < {}"
      " for EDL event# {}".format(
          record_in,
          self.timeline.global_start_time,
          clip.num
      ))          

record_out = opentime.from_timecode(
    clip_handler.record_tc_out,
    edl_rate
)`

Apologies for not doing tests, submitting a pull request or whatever... I haven't learnt that bit yet...

Will do that next but probably won't get to it this week so if anyone more knowledgable can confirm I'm in the right ball park ? Thanks

— Reply to this email directly, view it on GitHub https://github.com/AcademySoftwareFoundation/OpenTimelineIO/issues/1385#issuecomment-1774220536, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIW7YKHMOTWQWFTOPRLNPLYAWPBVAVCNFSM57CH4B42U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCNZXGQZDEMBVGM3A . You are receiving this because you are subscribed to this thread.Message ID: <AcademySoftwareFoundation/OpenTimelineIO/issues/1385/1774220536@ github.com>

-- -Daniel

TrevorAyl commented 1 year ago

Hi Daniel

Sorry, I deleted them as I realised I'd run before I could walk, and thought I should ponder a little more before going in feet first. If that's not mixing too many metaphors.

I do think there's an underlying issue with the CMX3600 adapter - as it seems to store the track.source_range.start_time as a negative number: so an initial record in time of "23:00:00:00" is stored as -1.9872e06

                source_range=otio.opentime.TimeRange(
                    start_time=otio.opentime.RationalTime(value=-1.9872e06, rate=24),
                    duration=otio.opentime.RationalTime(value=6, rate=24),
                ),

I wrote a test for this:

    def test_record_in_translation(self):
        tl = otio.adapters.read_from_string(
            'TITLE: enable_test\n'
            '001  Clip001  V     C        00:00:00:00 00:00:00:03 23:00:00:00 23:00:00:03\n'
            '* FROM CLIP NAME:  Clip-001\n'
            '* OTIO TRUNCATED REEL NAME FROM: Clip-001\n'
            '002  Clip002  V     C        00:00:00:03 00:00:00:06 23:00:00:03 23:00:00:06\n'
            '* FROM CLIP NAME:  Clip-002\n'
            '* OTIO TRUNCATED REEL NAME FROM: Clip-002\n',
            adapter_name="cmx_3600", rate=24
        )
        print (tl.tracks[0].source_range.start_time)
        print (otio.opentime.from_timecode("23:00:00:00", 24))

which results in

RationalTime(-1.9872e+06, 24)
RationalTime(1.9872e+06, 24)

I amended cmx3600.py around line 186 as follows,

           if track.source_range is None:
                zero = opentime.RationalTime(0, edl_rate)
                track.source_range = opentime.TimeRange(
                    start_time=zero + record_in,
                    duration=zero
                )

            track_end = track.source_range.start_time + track.duration()

and the numbers from the test are then both positive

RationalTime(1.9872e+06, 24)
RationalTime(1.9872e+06, 24)

but other tests fail. Presumably as there is now a knock on effect down the line.

======================================================================
ERROR: test_edl_round_trip_disk2mem2disk (__main__.EDLAdapterTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/***/Documents/GitHub/OpenTimelineIO/tests/test_cmx_3600_adapter.py", line 325, in test_edl_round_trip_disk2mem2disk
    otio.adapters.write_to_file(timeline, tmp_path)
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/__init__.py", line 192, in write_to_file
    return adapter.write_to_file(
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/adapter.py", line 183, in write_to_file
    result = self.write_to_string(input_otio, **adapter_argument_map)
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/adapter.py", line 274, in write_to_string
    return self._execute_function(
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/plugins/python_plugin.py", line 142, in _execute_function
    return (getattr(self.module(), func_name)(**kwargs))
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/cmx_3600.py", line 832, in write_to_string
    return writer.get_content_for_track_at_index(0, title=input_otio.name)
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/cmx_3600.py", line 938, in get_content_for_track_at_index
    content += event.to_edl_format() + '\n'
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/cmx_3600.py", line 1041, in to_edl_format
    lines = [self.line.to_edl_format(self.edit_number)]
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/cmx_3600.py", line 1186, in to_edl_format
    'rec_in': opentime.to_timecode(self.record_in, self._rate),
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/opentime.py", line 46, in to_timecode
    else rt.to_timecode(rate, drop_frame)
ValueError: value cannot be negative here

======================================================================
ERROR: test_edl_round_trip_disk2mem2disk_speed_effects (__main__.EDLAdapterTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/***/Documents/GitHub/OpenTimelineIO/tests/test_cmx_3600_adapter.py", line 301, in test_edl_round_trip_disk2mem2disk_speed_effects
    otio.adapters.write_to_file(timeline, tmp_path)
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/__init__.py", line 192, in write_to_file
    return adapter.write_to_file(
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/adapter.py", line 183, in write_to_file
    result = self.write_to_string(input_otio, **adapter_argument_map)
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/adapter.py", line 274, in write_to_string
    return self._execute_function(
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/plugins/python_plugin.py", line 142, in _execute_function
    return (getattr(self.module(), func_name)(**kwargs))
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/cmx_3600.py", line 832, in write_to_string
    return writer.get_content_for_track_at_index(0, title=input_otio.name)
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/cmx_3600.py", line 938, in get_content_for_track_at_index
    content += event.to_edl_format() + '\n'
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/cmx_3600.py", line 1041, in to_edl_format
    lines = [self.line.to_edl_format(self.edit_number)]
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/cmx_3600.py", line 1186, in to_edl_format
    'rec_in': opentime.to_timecode(self.record_in, self._rate),
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/opentime.py", line 46, in to_timecode
    else rt.to_timecode(rate, drop_frame)
ValueError: value cannot be negative here

======================================================================
ERROR: test_edl_round_trip_with_transitions (__main__.EDLAdapterTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/***/Documents/GitHub/OpenTimelineIO/tests/test_cmx_3600_adapter.py", line 621, in test_edl_round_trip_with_transitions
    otio.adapters.write_to_file(timeline, tmp_path)
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/__init__.py", line 192, in write_to_file
    return adapter.write_to_file(
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/adapter.py", line 183, in write_to_file
    result = self.write_to_string(input_otio, **adapter_argument_map)
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/adapter.py", line 274, in write_to_string
    return self._execute_function(
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/plugins/python_plugin.py", line 142, in _execute_function
    return (getattr(self.module(), func_name)(**kwargs))
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/cmx_3600.py", line 832, in write_to_string
    return writer.get_content_for_track_at_index(0, title=input_otio.name)
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/cmx_3600.py", line 938, in get_content_for_track_at_index
    content += event.to_edl_format() + '\n'
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/cmx_3600.py", line 1041, in to_edl_format
    lines = [self.line.to_edl_format(self.edit_number)]
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/cmx_3600.py", line 1186, in to_edl_format
    'rec_in': opentime.to_timecode(self.record_in, self._rate),
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/opentime.py", line 46, in to_timecode
    else rt.to_timecode(rate, drop_frame)
ValueError: value cannot be negative here

======================================================================
ERROR: test_reels_edl_round_trip_string2mem2string (__main__.EDLAdapterTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/***/Documents/GitHub/OpenTimelineIO/tests/test_cmx_3600_adapter.py", line 888, in test_reels_edl_round_trip_string2mem2string
    otio_data = otio.adapters.write_to_string(timeline, adapter_name="cmx_3600",
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/__init__.py", line 213, in write_to_string
    return adapter.write_to_string(
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/adapter.py", line 274, in write_to_string
    return self._execute_function(
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/plugins/python_plugin.py", line 142, in _execute_function
    return (getattr(self.module(), func_name)(**kwargs))
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/cmx_3600.py", line 832, in write_to_string
    return writer.get_content_for_track_at_index(0, title=input_otio.name)
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/cmx_3600.py", line 938, in get_content_for_track_at_index
    content += event.to_edl_format() + '\n'
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/cmx_3600.py", line 1041, in to_edl_format
    lines = [self.line.to_edl_format(self.edit_number)]
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/adapters/cmx_3600.py", line 1186, in to_edl_format
    'rec_in': opentime.to_timecode(self.record_in, self._rate),
  File "/Users/***/Documents/GitHub/otioenv/lib/python3.9/site-packages/opentimelineio/opentime.py", line 46, in to_timecode
    else rt.to_timecode(rate, drop_frame)
ValueError: value cannot be negative here

Presumably the timeline.global_start_time should be the 'sequence start' for an NLE and the track[n].source_range.start_time should be the start time for a particular track in that timeline.

As far as EDLs go they can only have one video track per EDL (I guess keys are two tracks but I don't think they are used much or implemented in this adapter) - but sometimes each video track is exported as it's own EDL - and the sequence start would be the earliest record in from those EDLs.

I'll have a further look when I get the chance and see if I can figure out more. Hopefully without breaking too much or making a mess.

I'm very much learning here - so feel free to point out any errors or easier ways of doing things.

Trevor