From 6f52397342c72c7f95ed0daf11891ed937869677 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sun, 19 Jul 2026 21:42:53 +0200 Subject: [PATCH] fix(encode): bound NVENC async pipelining by the capturer's texture ring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Windows direct-NVENC backend registers and encodes the capturer's textures IN PLACE (no CopyResource), so how deep it may pipeline is a property of the CAPTURER, not of the encoder. It was bounded only by `async_inflight_cap()` — `PUNKTFUNK_NVENC_ASYNC_DEPTH`, default 4, clamped to the output-bitstream pool — which consults nothing about the capturer, while the comment at the backpressure loop claimed it "keep[s] in-flight depth within the capturer's texture ring". It never did. The IDD-push capturer rotates `OUT_RING = 3` per delivered frame with no regard for encode completion (its own invariant note says OUT_RING(3) > max pipeline_depth(2)). With the default async depth of 4 the encoder can therefore still be reading a texture the capturer has already handed out again and overwritten: torn or mixed frames. It is visual corruption rather than UB, so it fails silently and intermittently — the worst shape to diagnose from a field report. Adds `Encoder::set_input_ring_depth`, reported from `Capturer::pipeline_depth`, and bounds the async backpressure loop by `min(async_inflight_cap(), depth)`. For IDD-push that yields 2, matching the capturer's stated contract; backends that copy their input, or are synchronous, ignore it. Wired at ALL THREE encoder-creation sites (initial open, stall/resize rebuild, ABR rebuild) and forwarded through `TrackedEncoder` — this crate has a documented trap where an unforwarded defaulted trait method silently no-ops through that wrapper, which has already bitten the direct-NVENC work once and the wire-chunking probe once. Co-Authored-By: Claude Opus 4.8 --- crates/pf-encode/src/enc/codec.rs | 10 ++++++ crates/pf-encode/src/enc/windows/nvenc.rs | 38 +++++++++++++++++++--- crates/pf-encode/src/lib.rs | 5 +++ crates/punktfunk-host/src/native/stream.rs | 10 ++++++ 4 files changed, 58 insertions(+), 5 deletions(-) diff --git a/crates/pf-encode/src/enc/codec.rs b/crates/pf-encode/src/enc/codec.rs index 0be52010..5bd34720 100644 --- a/crates/pf-encode/src/enc/codec.rs +++ b/crates/pf-encode/src/enc/codec.rs @@ -293,6 +293,16 @@ pub trait Encoder: Send { /// flagged [`EncodedFrame::chunk_aligned`] and the session marks them on the wire. /// Default: no-op (the H.26x backends' bitstreams cannot be cut losslessly). fn set_wire_chunking(&mut self, _shard_payload: usize) {} + /// How many frames the CAPTURER guarantees the encoder may hold in flight before it starts + /// reusing an input texture (`Capturer::pipeline_depth`). Backends that encode the capturer's + /// textures IN PLACE — no `CopyResource` — must not pipeline deeper than this: the capturer + /// rotates its output ring per delivered frame with no regard for encode completion, so a + /// deeper pipeline lets it overwrite a texture mid-encode. That is visual corruption (torn or + /// mixed frames), not UB, so it fails silently and intermittently. + /// + /// Called once by the session glue after the capturer is known; a backend that copies its + /// input, or is synchronous, ignores it. Default: no-op. + fn set_input_ring_depth(&mut self, _depth: usize) {} /// Signal end-of-stream. After this, drain the remaining AUs with [`poll`](Self::poll) /// until it returns `None` — NVENC buffers frames internally even at `delay=0`. fn flush(&mut self) -> Result<()>; diff --git a/crates/pf-encode/src/enc/windows/nvenc.rs b/crates/pf-encode/src/enc/windows/nvenc.rs index 6c2cd7dc..ef3deb89 100644 --- a/crates/pf-encode/src/enc/windows/nvenc.rs +++ b/crates/pf-encode/src/enc/windows/nvenc.rs @@ -409,6 +409,12 @@ pub struct NvencD3d11Encoder { events: Vec, /// Async mode: the retrieve thread + its channels (`None` = classic same-thread sync retrieve). async_rt: Option, + /// The capturer's `pipeline_depth` (`set_input_ring_depth`). This backend encodes the + /// capturer's textures IN PLACE, so it is a HARD ceiling on async in-flight depth: the + /// capturer rotates its ring per delivered frame regardless of encode completion, so + /// pipelining deeper lets it overwrite a texture mid-encode (torn frames). `None` until the + /// session glue reports it — treated as "unknown, don't pipeline past the env cap". + input_ring_depth: Option, /// `NV_ENC_CAPS_ASYNC_ENCODE_SUPPORT` from the caps probe — gates the async retrieve mode. async_supported: bool, /// (bitstream, mapped input resource to unmap after retrieval, pts_ns, recovery-anchor) per @@ -505,6 +511,7 @@ impl NvencD3d11Encoder { bitstreams: Vec::new(), events: Vec::new(), async_rt: None, + input_ring_depth: None, async_supported: false, pending: VecDeque::new(), frame_idx: 0, @@ -1156,11 +1163,21 @@ impl Encoder for NvencD3d11Encoder { // index, which is non-zero on a mid-session encoder rebuild's first frame. let opening = self.next == 0; // Async backpressure: never hand NVENC an output bitstream that is still in flight, and - // keep in-flight depth within the capturer's texture ring (see `async_inflight_cap`). At - // the cap, block on the OLDEST completion (the retrieve thread is already waiting on its - // event) before submitting more — bounding depth exactly like the sync path's per-tick - // blocking poll, just `cap` deep instead of 1. - while self.async_rt.is_some() && self.pending.len() >= async_inflight_cap() { + // keep in-flight depth within the capturer's texture ring. At the cap, block on the OLDEST + // completion (the retrieve thread is already waiting on its event) before submitting more — + // bounding depth exactly like the sync path's per-tick blocking poll, just `cap` deep + // instead of 1. + // + // The ring term is the one that matters for correctness: `async_inflight_cap()` is only the + // output-bitstream-pool ceiling plus an env knob, and consults NOTHING about the capturer, + // despite this comment previously claiming otherwise. Since this backend encodes the + // capturer's textures in place, exceeding the capturer's declared `pipeline_depth` lets it + // rotate a texture out from under a live encode — torn frames, silently. + let cap = match self.input_ring_depth { + Some(d) => async_inflight_cap().min(d.max(1)), + None => async_inflight_cap(), + }; + while self.async_rt.is_some() && self.pending.len() >= cap { let done = { let rt = self.async_rt.as_mut().expect("checked in loop condition"); rt.done_rx @@ -1336,6 +1353,17 @@ impl Encoder for NvencD3d11Encoder { self.submit(frame) } + fn set_input_ring_depth(&mut self, depth: usize) { + // This backend registers and encodes the capturer's textures in place (no CopyResource), + // so the capturer's ring depth is a hard ceiling on how deep async may pipeline. + self.input_ring_depth = Some(depth); + tracing::debug!( + depth, + env_cap = async_inflight_cap(), + "NVENC: capturer input-ring depth reported — async in-flight bounded by the smaller" + ); + } + fn request_keyframe(&mut self) { self.force_kf = true; } diff --git a/crates/pf-encode/src/lib.rs b/crates/pf-encode/src/lib.rs index 292eb9db..7bea9d3c 100644 --- a/crates/pf-encode/src/lib.rs +++ b/crates/pf-encode/src/lib.rs @@ -209,6 +209,11 @@ impl Encoder for TrackedEncoder { fn set_wire_chunking(&mut self, shard_payload: usize) { self.inner.set_wire_chunking(shard_payload) } + // Forwarded for the same reason as `set_wire_chunking` above — an unforwarded default here + // would silently leave the in-place backends pipelining past the capturer's ring. + fn set_input_ring_depth(&mut self, depth: usize) { + self.inner.set_input_ring_depth(depth) + } fn poll(&mut self) -> Result> { self.inner.poll() } diff --git a/crates/punktfunk-host/src/native/stream.rs b/crates/punktfunk-host/src/native/stream.rs index 2fdb1390..ef739fff 100644 --- a/crates/punktfunk-host/src/native/stream.rs +++ b/crates/punktfunk-host/src/native/stream.rs @@ -1477,6 +1477,9 @@ pub(super) fn virtual_stream(ctx: SessionContext, prepared: Option