-
Notifications
You must be signed in to change notification settings - Fork 289
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
EDL Adapter: should author and read timeline::global_start_time attribute rather than source_range.start_time of track #1385
Comments
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
<#1385 (comment)>,
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
|
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:
I wrote a test for this:
which results in
I amended cmx3600.py around line 186 as follows,
and the numbers from the test are then both positive
but other tests fail. Presumably as there is now a knock on effect down the line.
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 |
To Reproduce
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 theglobal_start_time
on the timeline would be the first event record time - rather than relying on thesource_range
on tracks to set the time.The text was updated successfully, but these errors were encountered: