Skip to content
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

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

Open
mark-oshea opened this issue Apr 17, 2024 · 5 comments

Comments

@mark-oshea
Copy link

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
Copy link
Collaborator

meshula commented Apr 17, 2024

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
Copy link

phil-man-git-hub commented Apr 17, 2024 via email

@meshula
Copy link
Collaborator

meshula commented Apr 17, 2024

@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
Copy link

phil-man-git-hub commented Apr 17, 2024 via email

@meshula
Copy link
Collaborator

meshula commented Apr 18, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants