Reviewed the whole crate (15.6 kloc) for bugs, safety, structure and comment truth.
Both compile gates are green: `scripts/xcheck.sh windows clippy` and
`cargo clippy -p pf-capture --all-targets --locked -- -D warnings` in the amd64 CI
image (the Linux half needs libpipewire, so it cannot ride xcheck).
Code defects, each one contradicted by a comment sitting next to it:
* `pipeline_depth` clamped to `OUT_RING` (3) while both `repeat_last` and `OUT_RING`
state the safe maximum is 2. `d` frames in flight need `d + 1` textures, so
`PUNKTFUNK_IDD_DEPTH=3` rotated onto the slot NVENC was still reading and the convert
overwrote it in place — torn frames, silently. Now `OUT_RING - 1`.
* The GDI cursor poller published `visible: true` for a NULL `hCursor` carrying
`CURSOR_SHOWING` — how an app hides the pointer for its own window. The last
rasterised arrow was then blended into a game that had hidden its cursor. Every
rasterise gate already tested `handle != 0`; the published verdict now agrees.
* The ETW event callback did `RING.lock().unwrap()`. That is an `extern "system"` fn, so
a poisoned lock panicked across an FFI boundary and ABORTED the host — a diagnostic
taking down capture. Poison-tolerant now, which also makes the poison unreachable.
* `ChannelBroker::send` bounded the ring with `debug_assert`, so a release build instead
panicked mid-`duplicate_and_deliver`, unwinding past the reap and leaking every handle
already planted in the driver's WUDFHost. Refuses before the first duplication.
* `set_active(false)` did not clear `stall_since`, so a pooled capturer carried a stale
stall clock into its next stream and reported capture loss microseconds in.
* `attach_gamescope_cursor` evaluated `spawn` before dropping the old source: two readers
published into one slot, and a failed spawn destroyed a working reader. Idempotent now.
* `PUNKTFUNK_FORCE_SHM` used a bare `== "1"` compare, silently ignoring `=true`/`=on`.
* `spa_meta_bitmap.offset == 0` is SPA's "no image data" signal, distinct from the
`bitmap_offset == 0` position-only case. Unhandled, it decoded the header's own words
as cursor pixels and cached them.
* A `VideoInfoRaw::parse` failure was swallowed, so a malformed Format pod surfaced as
the generic "no acceptable format" timeout. It is logged, and parsed once, not twice.
Comment corrections, all verified against the code they describe: four claims that a
failed open falls back to DDA (removed — the caller drops the keepalive under
"no fallback"); three comparisons to the removed WGC path; "we do NOT gate HDR on the
client's VIDEO_CAP_10BIT" (it does, in three places); the P010 sampler's "4 explicit
taps / 2x2 box" (two taps, left-cosited — the box was the bug it replaced); the cursor
meta cap quoted as 256x256 (1024, and 256 is the value that cost the whole Linux cursor
channel on-glass); the poller's "~60 Hz" (4 ms, ~250 Hz); "several minutes of coverage"
(~26 s); "8 frames in 400 ms >= 20 fps" (7 intervals, so 17.5); three "process-wide" HDR
latch claims (per-source, which is why HdrSource exists); a SAFETY proof claiming a view
is "unmapped never" (Drop unmaps it); the Linux module header describing a bounded
channel and BGRx-only frames (one-deep overwriting slot, several formats); and a doc
line stranded on `DisplayDescriptor` by an earlier split, restored to `IddPushCapturer`,
which had none.
enricobuehler
8b98d0b3ec