unom/punktfunk
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases 30 Wiki Activity

fix(capture): stop stranding a PipeWire buffer on a caught panic; invalidate the PyroWave CSC on ring recreate

Browse Source 
Two medium findings from the round-1 sweep.

- The `.process` callback dequeued a buffer INSIDE its `catch_unwind`, and every
  requeue site was inside too. `newest` is a raw pointer with no Drop, so any
  caught panic (update_cursor_meta / consume_frame) unwound past all three
  requeues and permanently stranded that buffer. Once the stream's fixed pool
  drained, `dequeue_raw_buffer` returned null every call and capture silently
  wedged while still reporting negotiated/active — defeating the very
  catch_unwind that was meant to keep a panic survivable. The drain loop now runs
  OUTSIDE the catch (dequeue/queue are non-panicking C FFI pointer ops) and
  `newest` is requeued exactly once after it, on every path: normal,
  corrupted-skip, or caught panic.

- `recreate_ring` invalidated `video_conv` and `hdr_p010_conv` but not
  `pyro_conv`. That converter is mode-baked — BgraToYuvPlanes selects entirely
  different shaders and output formats for SDR (8-bit BT.709 → R8/R8G8) vs HDR
  (scRGB→PQ BT.2020 → R16/R16G16) — and `ensure_pyro_conv` only builds when None,
  so a display_hdr flip reused the stale SDR converter against a freshly
  HDR-formatted pyro ring, corrupting every frame. Reachable at the documented
  "Downgrade point D": a PyroWave session with client_10bit=true that opens on a
  box where HDR can't enable, then flips once the display comes up.

Linux .21: pf-capture 1/0. Windows .173: `cargo check -p pf-capture` clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Enrico Bühler
2026-07-19 23:16:25 +02:00
parent 26ff005e70
commit fe4af1761e
2 changed files with 43 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/pf-capture/src/linux/mod.rs
+24 -24
View File
@@ -2191,36 +2191,37 @@ mod pipewire {
}
})
.process(|stream, ud| {
// PipeWire dispatches this from a C trampoline with no catch_unwind; a
// panic crossing that FFI boundary would abort the whole host. Contain it.
let outcome = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
// Latest-frame-only (OBS pattern): Mutter delivers buffers in bursts and
// recycles its pool; an older queued buffer carries a STALE frame. Drain all
// queued buffers, requeue the older ones, keep only the newest.
// SAFETY: `stream` is the live stream PipeWire passes into this `.process` callback on
// the loop thread, where `pw_stream_dequeue_buffer` is the documented call. It returns
// a `*mut pw_buffer` owned by the stream (or null when the queue is drained),
// null-checked before any use. The loop is single-threaded, so no concurrent access.
// Latest-frame-only (OBS pattern): Mutter delivers buffers in bursts and recycles its
// pool; an older queued buffer carries a STALE frame. Drain all queued buffers, requeue
// the older ones, keep only the newest. This dequeue/requeue runs OUTSIDE the
// `catch_unwind` below — they are non-panicking C FFI pointer ops, and `newest` is
// requeued exactly once AFTER the panic-containing region. Previously the whole thing was
// inside the catch, so a caught panic (in `update_cursor_meta`/`consume_frame`) stranded
// `newest` forever, permanently shrinking the stream's fixed pool until capture wedged.
// SAFETY: `stream` is the live stream PipeWire passes into this `.process` callback on the
// loop thread; `dequeue_raw_buffer` returns a stream-owned `*mut pw_buffer` or null
// (null-checked), single-threaded so no concurrent access.
let mut newest = unsafe { stream.dequeue_raw_buffer() };
if newest.is_null() {
return;
}
let mut drained = 1u32;
loop {
// SAFETY: same stream/loop-thread contract as the dequeue above; each call returns
// the next stream-owned `*mut pw_buffer` or null (null-checked before use).
// SAFETY: same stream/loop-thread contract; returns the next stream-owned buffer or null.
let next = unsafe { stream.dequeue_raw_buffer() };
if next.is_null() {
break;
}
// SAFETY: `newest` is a non-null `*mut pw_buffer` previously dequeued from this same
// stream and not yet requeued; `pw_stream_queue_buffer` hands ownership back to the
// stream. We immediately overwrite `newest = next`, so the requeued pointer is never
// touched again (no use-after-requeue). Loop thread, single-threaded.
// SAFETY: `newest` was dequeued from this stream and not yet requeued; we immediately
// overwrite it, so the requeued pointer is never touched again.
unsafe { stream.queue_raw_buffer(newest) };
newest = next;
drained += 1;
}
// PipeWire dispatches from a C trampoline with no catch_unwind; a panic crossing that FFI
// boundary would abort the whole host. Contain the inspect/consume work — the only Rust
// code here that can panic — and requeue `newest` unconditionally after it.
let outcome = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
// SAFETY: `newest` is the non-null buffer we still own (dequeued, not requeued);
// `.buffer` is a `*mut spa_buffer` field libpipewire populated. This is a single field
// load through a valid pointer — no mutation or aliasing.
@@ -2299,19 +2300,18 @@ mod pipewire {
"capture: skipped a stale CORRUPTED/cursor buffer (GNOME)"
);
}
// SAFETY: `newest` is the non-null buffer we own (dequeued, never requeued on this
// skip path); hand it back to the stream exactly once and return without touching it
// again. Loop thread inside `.process`.
unsafe { stream.queue_raw_buffer(newest) };
// Skip this stale/cursor buffer — `newest` is requeued unconditionally below.
return;
}
consume_frame(ud, spa_buf);
// SAFETY: `consume_frame` has finished reading `spa_buf` (and the `datas` borrows derived
// from `newest`), so requeuing the owned `newest` exactly once here is sound — no
// use-after-requeue. Loop thread inside `.process`.
unsafe { stream.queue_raw_buffer(newest) };
}));
// Hand `newest` back to the stream exactly once, on EVERY path — normal, corrupted-skip,
// or a caught panic in the closure above. This single requeue is what keeps the fixed
// buffer pool from draining.
// SAFETY: all reads of `spa_buf`/`newest` (update_cursor_meta, consume_frame) completed
// inside the closure above; `newest` was dequeued from this stream and not yet requeued.
unsafe { stream.queue_raw_buffer(newest) };
if outcome.is_err() {
// In the per-frame `.process` callback: a deterministic panic (e.g. a bad
// format) would fire this every frame, so power-of-two throttle it — enough to
crates/pf-capture/src/windows/idd_push.rs
+6
View File
@@ -1341,6 +1341,12 @@ impl IddPushCapturer {
self.out_ring.clear(); // the output format changed → rebuild lazily at the new format
self.video_conv = None; // converters are sized + HDR-specific → rebuild at the new mode
self.hdr_p010_conv = None;
// The PyroWave CSC is mode-baked too (BgraToYuvPlanes picks different SDR vs HDR shaders
// and R8/R8G8 vs R16/R16G16 outputs). Without this, a display_hdr flip (Downgrade point D:
// client_10bit=true but HDR couldn't enable at open) reused the stale SDR converter against
// the freshly HDR-formatted pyro ring — every frame corrupted. `ensure_pyro_conv` only
// builds when None, so it must be reset here like its siblings.
self.pyro_conv = None;
self.pyro_ring.clear(); // PyroWave two-plane ring is sized → rebuild at the new mode
self.pyro_last = None;
self.out_idx = 0;