Parser Rebuild

[reinecke]

Overview

EDL can be a tricky format to get right in the details - our original parser worked well in controlled situations, but also had issues like not adopting global_start_time support, not fully baked transition support, and brittleness when sources were in frame rates that didn't match the timeline rate.

This refactor separates the reader code into two separate disciplines:

1. A parser that yields an edl statement per line with parsed fields but does not resolve timing and represents the facts of exactly what's in the EDL as closely as possible.
2. The Reader which interprets the statements grouped by event into their resolved timing, converts them to OpenTimelineIO schema objects, and applies them to a timeline

Changes and Enhancements

Adapter Args

The adapter has been updated with new controls on parsing behavior based on our more current knowledge of the format:

• ignore_timecode_mismatch is still present, but is always hard-wired True and raises DeprecationWarning when used. Will be removed in a future release.
• (NEW!) ignore_invalid_timecode_errors - When a timecode can't be interpreted due to something like Frame rate mismatch. Timecode '19:13:21:28' has frames beyond 23, the start timecode in the source will be rounded up to zero frames in the next second. This helps address issues that can occur from doing things like using a 30 fps source in a 24 fps timeline without retimes.
• (NEW!) discard_unsupported_events - enables a mode where events that can't be interpreted (like KEY edits) are skipped and the event is noted in timeline.metadata["cmx_3600"]["unsupported_events"].

Note that setting ignore_invalid_timecode_errors and discard_unsupported_events to False (the default) will preserve the old behavior of erroring when correctness can't be trusted.

Parsing

• Transitions now resolve properly according to references in the CMX spec (AcademySoftwareFoundation/OpenTimelineIO#977) (AcademySoftwareFoundation/OpenTimelineIO#895) (maybe AcademySoftwareFoundation/OpenTimelineIO#137)
• Uses Timeline.global_start_time rather than Track.available_range to signal the timeline start time (#6) (AcademySoftwareFoundation/OpenTimelineIO#1385) (maybe AcademySoftwareFoundation/OpenTimelineIO#346)
• Timeline events are no longer merged if they aren't from the same reel or their source range is discontinuous (in the old parser clips adjacent to transitions could be merged even if they came from separate sources) (AcademySoftwareFoundation/OpenTimelineIO#1328) (AcademySoftwareFoundation/OpenTimelineIO#912)
• Includes more contextual information in parsed clips (event numbers, original EDL timecode values, etc)
• OTIO clip durations now primarily driven from timeline timecode rather than source timecode (this should avoid A/V sync issues due to potentially accumulating errors)
• Adds the ability to use a timing approximation when timecode doesn’t match the provided EDL rate (This is helpful when the source is at a different frame rate than the timeline rate)
• Most EDLParseException instances now include more contextual information
• Now attempts to fallback to ISO-8859-1 encoding in read_from_file when the operating system default string encoding fails - this has anecdotally been successful in handling some EDLs that would otherwise fail
• Video events that overwrite previous events in the timeline are now placed on a video track "above" the previous event's track to simulate an "overwrite". Overlapping audio events still raise an exception. Anecdotally this is most common in EDLs from Adobe Premiere.

TODOs

• [ ] Write tests around specific cited issues
• [ ] Update README documentation with new adapter args
• [ ] Flesh out docstrings in code
• [ ] Evaluate whether or not we should cull out parts of code targeted at SMPTE 258M EDL
• [ ] Evaluate if using a duration comparison based heuristic (interpret source timecode at all the known timecode rates until we find a source duration that is very close to the timeline duration) we can reliably infer mismatched source rates where we're currently using the ignore_invalid_timecode_errors behavior

[reinecke]

@douglascomet Thanks for the feedback!

The M2 issues are with M2 motion effects - see page 20 in the cmx spec.

A docstring pass is a good idea - I need to make sure I share some of the ideas I had behind this stuff to help future us.

The code density stuff is good feedback - we can think about driving sections of this toward being more approachable.

I'll try to carve some time for this to try and get it out in the coming weeks.