Verify NVENC sub-frame readback in release, and stop trusting a driver that lied #400

Merged
enricobuehler merged 1 commits from worktree-nvenc-subframe-verify into main 2026-08-26 17:52:10 +00:00
Owner

The host half of the black-bands field report (#398 was the client half).

What the host was doing wrong

The chunked poll (poll_chunk, both direct-NVENC backends) ships each slice the moment the doNotWait lock publishes it, trusting that bitstreamSizeInBytes with n reported slices is a stable, byte-exact prefix of the AU the finishing blocking lock will return. The check for that trust — the ChunkState shadow comparison against the finished AU — was #[cfg(debug_assertions)], i.e. compiled out of every build that reaches a user. On a driver branch whose sub-frame accounting runs ahead of the flushed bytes (or back-patches published ones), release builds shipped not-yet-written buffer content straight to the wire: self-consistent tiling, wrong bytes, no counter anywhere moves. Zeros inside HEVC slices decode as black macroblock bands, the boundary lands somewhere new each frame — the "equalizer". The on-glass A/Bs all ran with the check alive (debug), which is exactly why the fleet lab never saw what the field does.

The fix

  • The shadow prefix check now runs in release. Cost is one AU-sized copy + compare per frame — noise next to the encode itself.
  • On divergence, latch subframe_broken and bail into the existing encode-stall recovery. The in-place rebuild re-resolves sub-frame with the latch and opens WITHOUT it, so a lying driver is disarmed after exactly one rebuild — not looped into MAX_ENCODER_RESETS, and not re-armed by the split arbiter either (both arbiters derive from the post-latch open state). The client sees the already-supported abandoned-AU path: partial frame ages out, the rebuild's IDR re-anchors. One logged ~1 s hiccup, once per session at most, instead of endless corruption.
  • The latch deliberately beats PUNKTFUNK_NVENC_SUBFRAME=1: it only sets after this session proved corruption, and it dies with the encoder, so a fresh session retests the driver.
  • Drive-by: the bail path no longer leaks the popped frame's input mapping (it was already out of pending, so teardown would never have unmapped it).

Both backends (Windows D3D11 nvenc.rs, Linux CUDA nvenc_cuda.rs) get the identical change — the Linux/AMD grey-smear reports are plausibly this same defect in RADV concealment dress.

Verification

  • cargo check + cargo clippy -p pf-encode in the punktfunk-rust-ci image (Linux leg).
  • windows-host.yml dispatched on this branch (PR CI never compiles the Windows leg).
  • The decision logic is a two-arm comparison feeding an existing, tested recovery path; the driver-facing behavior can only be proven on hardware — the 4090 reporter's session log should show the new warn once, then a clean stream.
The host half of the black-bands field report (#398 was the client half). ## What the host was doing wrong The chunked poll (`poll_chunk`, both direct-NVENC backends) ships each slice the moment the doNotWait lock publishes it, trusting that `bitstreamSizeInBytes` with `n` reported slices is a stable, byte-exact prefix of the AU the finishing blocking lock will return. The check for that trust — the `ChunkState` shadow comparison against the finished AU — was `#[cfg(debug_assertions)]`, i.e. compiled out of every build that reaches a user. On a driver branch whose sub-frame accounting runs ahead of the flushed bytes (or back-patches published ones), release builds shipped not-yet-written buffer content straight to the wire: **self-consistent tiling, wrong bytes, no counter anywhere moves**. Zeros inside HEVC slices decode as black macroblock bands, the boundary lands somewhere new each frame — the "equalizer". The on-glass A/Bs all ran with the check alive (debug), which is exactly why the fleet lab never saw what the field does. ## The fix - **The shadow prefix check now runs in release.** Cost is one AU-sized copy + compare per frame — noise next to the encode itself. - **On divergence, latch `subframe_broken` and bail into the existing encode-stall recovery.** The in-place rebuild re-resolves sub-frame with the latch and opens WITHOUT it, so a lying driver is disarmed after exactly one rebuild — not looped into `MAX_ENCODER_RESETS`, and not re-armed by the split arbiter either (both arbiters derive from the post-latch open state). The client sees the already-supported abandoned-AU path: partial frame ages out, the rebuild's IDR re-anchors. One logged ~1 s hiccup, once per session at most, instead of endless corruption. - The latch deliberately beats `PUNKTFUNK_NVENC_SUBFRAME=1`: it only sets after this session *proved* corruption, and it dies with the encoder, so a fresh session retests the driver. - Drive-by: the bail path no longer leaks the popped frame's input mapping (it was already out of `pending`, so `teardown` would never have unmapped it). Both backends (Windows D3D11 `nvenc.rs`, Linux CUDA `nvenc_cuda.rs`) get the identical change — the Linux/AMD grey-smear reports are plausibly this same defect in RADV concealment dress. ## Verification - `cargo check` + `cargo clippy -p pf-encode` in the punktfunk-rust-ci image (Linux leg). - `windows-host.yml` dispatched on this branch (PR CI never compiles the Windows leg). - The decision logic is a two-arm comparison feeding an existing, tested recovery path; the driver-facing behavior can only be proven on hardware — the 4090 reporter's session log should show the new `warn` once, then a clean stream.
enricobuehler added 1 commit 2026-08-26 16:46:41 +00:00
fix(encode): verify NVENC sub-frame readback in release, and stop trusting a driver that lied
ci / docs-drift (pull_request) Successful in 36s
ci / bun-nix (pull_request) Successful in 37s
ci / web (pull_request) Successful in 1m9s
ci / docs-site (pull_request) Successful in 1m41s
ci / rust-arm64 (pull_request) Successful in 1m46s
android / android (pull_request) Successful in 5m12s
ci / rust (pull_request) Successful in 5m34s
f79e9eb524
The chunked poll ships slice bytes the moment the doNotWait lock publishes
them, trusting that they are a byte-exact prefix of the AU the finishing
blocking lock returns. The check for that trust — the ChunkState shadow
comparison — was compiled out of release builds, so a driver branch whose
sub-frame accounting runs ahead of the flushed bytes shipped not-yet-written
buffer content straight to the wire: self-consistent tiling, wrong bytes,
no client counter moving. That is the black-band field report (Windows
client, 4090 host, identical across all three decoders, Android immune —
it never negotiates the slice wire), and most likely the Linux/AMD grey
smears too.

Make the shadow prefix check unconditional (one AU-sized copy + compare
per frame — noise next to the encode), and on divergence latch
subframe_broken before bailing into the existing encode-stall recovery:
the in-place rebuild now resolves sub-frame OFF for every later session
open on this encoder, so the driver that lied is disarmed after exactly
one rebuild instead of looping into MAX_ENCODER_RESETS. The client sees
the abandoned-AU path it already handles (ages the frame out, IDR
re-anchors) — a one-second hiccup, once, instead of endless corruption.
Both direct-NVENC backends (Windows D3D11, Linux CUDA) get the identical
change; the bail path also stops leaking the popped frame's input mapping.
enricobuehler merged commit 4dcc31dc3b into main 2026-08-26 17:52:10 +00:00
enricobuehler deleted branch worktree-nvenc-subframe-verify 2026-08-26 17:52:19 +00:00
Sign in to join this conversation.
No Reviewers
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: unom/punktfunk#400