surface: give the playout margin a refresh of headroom, so a clean 60 fps stream stops dropping frames - #263
Merged
Merged
Conversation
… fps stream stops dropping frames The presenter scheduled each frame to become due at the 0.95 quantile of observed arrival offsets. That is a jitter estimate, not a due time: a frame can only be painted on a vsync, so aiming at the quantile leaves it a fraction of a refresh to land, and by construction 5% of a steady stream arrives after its own due time. Each one that does costs two frames — the refresh that repeats its predecessor, and itself, skipped as overdue by presentIndex. Measured against the server delivering a clean 60 fps (p99 inter-frame gap 19-26 ms at the socket, no stalls) to a 60 Hz display: the margin settled at 4-15 ms, under one refresh, and 3-19% of decoded frames never reached the canvas. recv=300 decoded=300 output=300 presented=264 dropped=37 in a 5 s window, with zero long tasks and rAF p99 of 17 ms — two to six visible hitches a second that nothing on the server side or the main thread accounts for. Adding one display refresh to the target buys the headroom out of the latency PRESENT_DELAY_MAX_MS was already budgeting for. Paired runs through the same harness: 10.4% -> 4.2% dropped, draw gaps over 25 ms 230 -> 99, p95 draw gap 33.4 -> 20.5 ms, for +19 ms of playout latency. Co-Authored-By: Claude <noreply@anthropic.com>
|
Coverage
|
|
🔗 Preview: https://blit-6nmqijfgl-indent.vercel.app |
![indent[bot]](https://avatars.githubusercontent.com/in/4161513?s=60&v=4){kind=small}
…rebase the tests on the new baseline Review catch, both parts fair. Seeding `presentOffsetMs` directly from `target` was fine while the target was the bare jitter quantile — near zero on a clean stream, so the step was invisible. With a refresh of headroom folded in, that same seed became a one-step latency jump at the moment smoothing engages, and a step in the offset is a hole of exactly that size in the output: three refreshes of nothing, once, at the start of every stream. It tripped the one invariant the newest-wins control block exists to protect. Seed at the jitter quantile alone and let the existing grow slew walk the headroom in over ~8 frames, which is what MARGIN_GROW_MS is for. The other two failures were tests encoding the old "clean stream settles near zero margin" assumption, which this series deliberately changes. Rebased on the new baseline — one refresh, expressed as REFRESH rather than a bare constant so they keep meaning what they meant. js/core: 750 passed, 10 skipped, 32 files. Co-Authored-By: Claude <noreply@anthropic.com>
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
1 participant
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.
What
One line in
updateSchedule: the presentation offset now targets the 0.95 arrival-offset quantile plus one display refresh, instead of the quantile alone. Two doc comments updated to say why.Why
Chasing a "blit misses frames / micro-hangs" report on surfaces. The server turned out to be fine; the client was discarding frames it had already decoded.
Server delivers cleanly. Recording surface 3 straight off the server's unix socket — 30 s, AV1, 1924x1258:
Zero gaps above 25 ms.
The browser drops 3-19% of decoded frames. Same server, overlapping windows, real UI in Chromium. blit's own counter:
rAF p50 16.7 / p99 17 ms, zero long tasks. Nothing on the main thread is janking and nothing upstream is stalling — the frames arrive, decode, and are thrown away at present time. That is two to six visible hitches a second.
Mechanism. A frame's due time was the 0.95 quantile of observed arrival offsets. That is a jitter estimate, not a due time: a frame can only be painted on a vsync, so aiming at the quantile leaves it a fraction of a refresh to actually land, and by construction 5% of a steady stream arrives after its own due time. Each one that does costs two frames — the refresh that repeats its predecessor, and itself, skipped as overdue by
presentIndex. Measured margin settled at 4-15 ms, under one 16.7 ms refresh.The old comment on
PRESENT_QUANTILEasserted the tail "costs nothing and is what should happen to a rare outlier". The first half is wrong, and at 5% of a 60 fps stream it is not rare. Both comments corrected.Measurement
Paired runs through the same harness (vite dev -> gateway -> live server, same surface, minutes apart):
The +19 ms comes out of the budget
PRESENT_DELAY_MAX_MS(50 ms) already reserved for exactly this, so the existing latency ceiling is unchanged.Method: Playwright-driven Chromium against the real UI, hooking
EncodedVideoChunkconstruction (arrival),CanvasRenderingContext2D.drawImagewith aVideoFramesource (present), and rAF deltas. A frame counts as dropped when its PTS arrived and was never drawn.Reviewer notes
#s=local:<id>did not pin the view, so arrival counts swung 450-1800 between runs. The patched arm won every round (13.8/19/8.6% -> 0.5/10.6/0.3%) but the magnitudes from those runs are not trustworthy. The table above is the one clean paired comparison. Worth re-running on a quiet box before trusting the exact numbers.this.refreshMsis the measured display period (noteRafInterval), so this scales with the panel rather than assuming 60 Hz. It falls back to 1/60 before any rAF delta is seen.tsc --noEmit -p js/core). No behavioural test added — the effect is statistical over hundreds of frames and needs a real display; there is no unit-test shape for it.Out of scope, found on the way
lib.rs:7473sleeps 50 ms per exited PTY inside the delivery loop, andlib.rs:7822can block for seconds on an audio-pipeline restart while holding the session mutex.