Put a sound on the frame it is actually on - #39
Merged
Conversation
Found by making a piece end to end and reading the marker list that came out of it. Two of its nine rows disagreed with themselves: 00:00:02:08,2.300,69 timecode says frame 8, frame column says 69 00:00:07:08,7.300,219 which is frame 9, not 8 The timecode was built by taking the fraction of a second first and multiplying that by the rate. Binary floating point does not hold 2.3, so 2.3 minus 2 is 0.2999999999999998, and thirty of those floor to 8 rather than 9. The frame column rounded instead, so it was right, and the two columns were computed far enough apart that nobody noticed they had stopped agreeing. This is not an edge case. Sweeping every frame position over ten minutes at 30fps, 8374 of 18000 came out a frame early — 47%. It went unseen because a frame is a thirtieth of a second and either answer looks plausible. Counting frames from the start rather than from the second they sit in removes the subtraction. The tolerance is what makes a snapped time land on its own frame: a cue at frame 41 is held as 41/30 seconds, which multiplies back a hair either side of 41, and flooring the low side loses it again. A millionth of a frame is 33 nanoseconds at 30fps, far below anything the app can place. It still floors rather than rounds, because it also reads a playhead that is moving: two thirds through frame 68 is frame 68. The marker list had a second copy of the timecode arithmetic, which is how it drifted from the app's in the first place. That copy is gone, and both of its frame columns now come from one number, so they cannot disagree again. Rates that are not whole numbers count whole frame slots — 29.97 fills thirty of them and takes 1.001 seconds over it, which is what non-drop timecode is. Taking the remainder against 29.97 itself printed a fraction of a frame. Ten tests, verified by restoring the original arithmetic: four of them fail on it and the rest of the suite stays green. The sweep is the one that matters — every frame position rather than a few cases somebody thought to write down. 261 unit tests, 81 browser tests, build clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ux1kydvUkLRoMbHp82ofDi
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found by making a piece end to end — load a clip, find hits, accept them, export everything — and then reading the marker list that came out.
Two of its nine rows disagreed with themselves:
What was wrong
The timecode took the fraction of a second first and multiplied that by the rate:
Binary floating point does not hold
2.3.2.3 - 2is0.2999999999999998, and thirty of those floor to 8 rather than 9. The frame column rounded from the whole time instead, so it was right — and the two were computed far enough apart that nobody noticed they had stopped agreeing.It is not an edge case. Sweeping every frame position over ten minutes at 30fps:
It went unseen because a frame is a thirtieth of a second and either answer looks plausible on screen. But this app's entire job is landing a sound on the frame a cut happens on — its own comment for
refinePeakssays "which is what makes a suggestion land on the right frame rather than near it."The fix
Count frames from the start rather than from the second they sit in, which removes the subtraction entirely.
The tolerance is the part worth explaining. A cue at frame 41 is held as
41/30seconds, which multiplies back a hair either side of 41 depending on the value; flooring the low side loses the frame again. A millionth of a frame is 33 nanoseconds at 30fps — far below anything the app can place — so it absorbs the representation error without reaching a real difference. Verified exact for every frame over an hour at 24, 25, 30, 48, 50, 60, 23.976 and 29.97.It still floors rather than rounds, because the same function reads a playhead that is moving: two thirds of the way through frame 68 is frame 68, not 69.
Two other things fell out of it:
Checking
Ten tests, verified by restoring the original arithmetic: four fail on it and the rest of the suite stays green. The sweep is the one that matters — every frame position, rather than a handful of cases someone thought to write down.
261 unit tests (was 251), 81 browser tests, build clean.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Ux1kydvUkLRoMbHp82ofDi
Generated by Claude Code