The OSD stage line stays a partition — an async decode figure is not one of its terms #211

Merged
enricobuehler merged 1 commits from worktree-decode-stat-overlap into main 2026-08-13 21:56:47 +00:00
1 Commits
Author SHA1 Message Date
enricobuehler 81022bcc80 fix(client/stats): keep the stage line a partition — an async decode figure is not one of its terms
ci / web (pull_request) Successful in 2m11s
ci / bun-nix (pull_request) Successful in 3m25s
ci / rust-arm64 (pull_request) Successful in 4m39s
android / android (pull_request) Successful in 6m20s
ci / docs-site (pull_request) Successful in 5m37s
ci / rust (pull_request) Successful in 6m40s
windows-client / client (arm64, --no-default-features, aarch64-pc-windows-msvc, C:\t-a64) (pull_request) Successful in 2m49s
windows-client / client (x64, , x86_64-pc-windows-msvc, C:\t) (pull_request) Successful in 6m34s
A 2026-08-13 field report read the OSD's stage line as a breakdown of e2e and
asked why the parts did not add up: `host 5.4 · net 0.3 · decode 6.6 ·
display 1.4` against `e2e 8.1/9.1`. Fair question, and the numbers are all
individually true. They add up without `decode`: 5.4 + 0.3 + 1.4 ≈ 8.1.

The stages ARE a per-frame partition of e2e — pts →(host+net)→ received
→(decode)→ decoded →(display)→ displayed — and that holds for as long as the
`decoded` stamp is a COMPLETION stamp. On the synchronous rungs it is. On the
native-Vulkan rung `receive_frame` returns at SUBMISSION (~0.1 ms) and the
stamp shipped to the presenter is taken there, so `display` is measured from
submit and the GPU decode happens INSIDE it. `host+net` and `display` already
tile e2e between them; the `decode` figure, measured received → fence-complete,
re-counts the GPU work `display` contains. Two figures, one overlap, printed
side by side as though they tiled.

So on that rung `decode` leaves the stage line and gets its own, carrying the
two caveats a reader needs before the number means anything: it is ONE sample
per window there, not the p50 every other figure on that line is, and it is
already inside `display` so adding it double-counts. The synchronous rungs are
untouched — `decode` is a real term there and stays inline.

Deliberately NOT changed: the one-sample-per-window design. `pf_client_core::
session` argues it at length — a per-frame fence wait serialises the decode
pipeline (an APU's 19 ms decode capping a 5120×1440 stream at ~51 fps), and M4
already re-examined and rejected polling, which quantises every sample up by a
frame interval (8.3 ms at 120 Hz against decodes of ~0.1-2 ms). That reasoning
still holds; the reporting around it was the defect. Making `decode` a genuine
per-frame term would need a completion stamp off the hot path — a waiter thread
on the timeline, which that comment already names as the remaining option — and
is a bigger change than this one.

Also not answered here: why the sampled frame read 6.6 ms when the sampling
comment expects 0.1-2 ms. It is a tail frame by construction (a frame that took
6.6 ms to decode also took ≥ 6.6 ms to display, against a 1.4 ms display p50),
but whether the first frame of a window is SYSTEMATICALLY a tail frame needs
instrumenting rather than guessing.

Verified in the linux/amd64 container: pf-presenter 47/47 (incl. the new case,
which pins both shapes and the timed-out-window zero), pf-client-core 188/188,
`clippy --all-targets -D warnings` clean on both, fmt clean. The pf-client-core
leg was proven non-vacuous with a planted compile_error! first.
2026-08-13 23:49:41 +02:00