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

Fix fundamental floating point error in Clip.iter_frames #1195

Merged
merged 5 commits into from
May 22, 2020

Conversation

tburrows13
Copy link
Collaborator

This PR has the same effect as #229, but with the (fixed) changes suggested in #210.

Closes #210, #229, #632, #646, #858.

Note that this supersedes half of #634, but at a deeper level. I will edit #634 to remove this half of the fix and leave it solely for the #631 supersample fix.

I'm quite confident in this PR so I will merge in a few days if there are no objections.

Explanation taken from #210:

clip duration = 0.56
fps = 12.5

When this gets written out to frames, we use (in Clip.py's iterframes):

        for t in np.arange(0, self.duration, 1.0/fps):

But numpy gives:

(Pdb) np.arange(0, .56, 1.0/12.5)
array([ 0.  ,  0.08,  0.16,  0.24,  0.32,  0.4 ,  0.48,  0.56])

This means that a composite video clip will try to generate an 8th frame at t=0.56. However, since the individual clips that make up the composite all have end=0.56, none of them is considered to be playing at that time. This results in a black final frame (just the background color, with no clips composited over it).

I don't think this is really a bug in the compositor so much as a result of numerical precision issues resulting in the extra t=0.56 frame being generated. Specifically, numpy says that arange generates a half-open interval [start, end), so in theory the arange array should end at t=0.48, but it doesn't, because the numbers 0.56 and 0.8 cannot be represented perfectly by floating point.


  • I have provided code that clearly demonstrates the bug and that only works correctly when applying this fix
  • I have added suitable tests demonstrating a fixed bug or new/changed feature to the test suite in tests/
  • I have properly documented new or changed features in the documentation or in the docstrings
  • I have properly explained unusual or unexpected code in the comments around it
  • I have formatted my code using black -t py36

@coveralls
Copy link

coveralls commented May 14, 2020

Coverage Status

Coverage decreased (-0.01%) to 67.327% when pulling 6058a5e on tburrows13:fix-concatenate-indexerror into b2ed1f6 on Zulko:master.

@tbermudez10
Copy link

Hello,

I'm a noob so I dont understand how this solves the problem, or maybe i just dont know how to pull it. Any help would be highly appreciated. Cheers!

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