AcademySoftwareFoundation / OpenTimelineIO

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

[BUG] from_seconds/to_seconds & from_frames/to_frames do not produce matching values #1742

Open mark-oshea opened 2 months ago

mark-oshea commented 2 months ago

Required:


[X] I believe this isn't a duplicate topic
[X] This report is not related to an adapter

For adapter related issues, please go to the appropriate repository - likely in the OpenTimelineIO github organization. For general questions and help please use the Academy Software Foundation slack, #opentimelineio.

Select One:

[ ] Build problem
[X] Incorrect Functionality or bug
[ ] New feature or functionality

Description

Frames to Seconds conversion is not accurate on certain values at 25 FPS.

If I instance RationalTime.from_frames with a value of 29, when I call to_seconds() it gives me 1.16.

If I instance RationalTime.from_seconds with a value of 1.16, when I call to_frames() it gives me 28.

I believe this is due to the fact we always round down to nearest int rather than to closest int e.g the maths shows us it's just slightly below frame 29 and not at all close to frame 28:

>>> 1.16 * 25
28.999999999999996

This makes us unsure if we can trust OpenTimelineIO to always calculate correct values when converting between seconds/frames.

Optional


Environment

Operating System:
Python version if appropriate: Python 3.10.7

Reproduction Steps

Tested on v0.14.0, v0.15.0 & v0.16.0.

Example snippet:

>>> import opentimelineio as otio
>>> FRAME_NUM = 29
>>> RATE = 25
>>>
>>> rational_time = otio.opentime.from_frames(FRAME_NUM, RATE)
>>> seconds = rational_time.to_seconds()
>>> print(seconds)
1.16
>>>
>>> rational_time_from_sec = otio.opentime.from_seconds(seconds, RATE)
>>>
>>> print(rational_time)
RationalTime(29, 25)
>>> print(rational_time_from_sec)
RationalTime(29, 25)
>>> print(rational_time.to_frames())
29
>>> print(rational_time_from_sec.to_frames())
28
meshula commented 2 months ago

Not sure that closest is what is required. My first reaction is that we must be missing the bog standard add a half before rounding down somewhere.

phil-man-git-hub commented 2 months ago

From an outsider’s point of view it’s about including frame 0 or starting from frame 1.Respectfully PhilPhil @.+1 (818)-746-6131Sent from my iPhoneOn Apr 17, 2024, at 12:09, Nick Porcino @.> wrote: Not sure that closest is what is required. My first reaction is that we must be missing the bog standard add a half before rounding down somewhere.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: @.***>

meshula commented 2 months ago

@phil-man-git-hub Do you want to edit your message? I notice that your phone number has somehow got appended into the post.

I'm afraid I am not following how a start of 0 or 1 helps me decide between rounding or closest, an example would help me out a lot.

phil-man-git-hub commented 2 months ago

Sorry for the obtuse response.What I mean is that if you include frame 0 or time code 00:00:00:00, or 00:00:00;00, or 00:00:00+00 then your frame counts can be different than a count that starts on frame 1.So it’s not just about the calculation of out minus in divided by frame rate, but whether the calculation is inclusive or exclusive of frames.00:00:00+00 - 00:00:00+23 is either 23 frames long or 24 frames long.This can be complicated further when using key frames to denote value changes at temporal locations.e.g. If your animation curve starts at frame 1 but your timeline starts at frame 0 then which value is correct? Equally, if you are remapping frame 50 of your source to frame 25 on your timeline, do you count from frame one or from frame 0? (Both for the temporal location to change and the source blue to remap).There is definitely an opportunity to establish a rule here, but I expect it might be debated a bit.Thanks for the heads up about the phone number, although I can’t imagine that anyone might use or need it.Respectfully PhilPhil @. from my iPhoneOn Apr 17, 2024, at 13:26, Nick Porcino @.> wrote: @phil-man-git-hub Do you want to edit your message? I notice that your phone number has somehow got appended into the post. I'm afraid I am not following how a start of 0 or 1 helps me decide between rounding or closest, an example would help me out a lot.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

meshula commented 2 months ago

That makes sense, thank you. The interfaces for to/from seconds/frames could be a little less simplistic to help with those cases.