From 46d09ed97373f60d64802eb68e873ee88e40c48a Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Tue, 21 Jul 2026 10:30:17 +0200 Subject: [PATCH] fix(host/linux): stop burning retry budget on doomed pipeline builds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two session-transition stalls found live on a SteamOS Deck host, one cause: the pipeline retry loop couldn't tell a wait-it-out failure from a retry-now-and-it-works one. - Gamescope first-frame race: a PipeWire stream connected while gamescope re-initializes its headless takeover negotiates a format, reaches Streaming, and never receives a buffer — while a fresh connect delivers within ~0.5 s. Every gamescope bring-up ate the full 10 s first-frame budget on attempt 1 (17 s bring-ups; KWin: 1.2 s). The retry loop's FIRST attempt now waits 2.5 s (Capturer::next_frame_within), so the reconnect that fixes the race happens at ~3 s. Later attempts keep the patient 10 s — the documented 30-60 s Big Picture cold start still fits the budget. - Capture-loss rebuild vs session switch: the rebuild loop re-detects the active session between build_pipeline_with_retry calls, but each call burned 8 attempts (~13 s) against a compositor that no longer exists (a Desktop→Gaming switch spent 13 of its 27 s retrying gone-KWin). The capture-loss path now passes max_attempts=2, turning the outer loop into ~1 s detect-and-retry cycles that follow the box to the new session. The resize path's direct build_pipeline call keeps the default budget (no retry wrapper there to absorb an early bail). Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/pf-capture/src/lib.rs | 10 +++ crates/pf-capture/src/linux/mod.rs | 90 +++++++++++++--------- crates/punktfunk-host/src/native/stream.rs | 71 ++++++++++++++--- 3 files changed, 123 insertions(+), 48 deletions(-) diff --git a/crates/pf-capture/src/lib.rs b/crates/pf-capture/src/lib.rs index 2157370f..d18f256c 100644 --- a/crates/pf-capture/src/lib.rs +++ b/crates/pf-capture/src/lib.rs @@ -24,6 +24,16 @@ use pf_frame::DmabufFrame; pub trait Capturer: Send { fn next_frame(&mut self) -> Result; + /// [`next_frame`](Self::next_frame) with a caller-chosen first-frame budget instead of the + /// backend's default. The pipeline retry loop shortens its FIRST attempt's wait: a PipeWire + /// stream connected while gamescope re-inits its headless takeover can negotiate a format, + /// reach `Streaming`, and still never receive a buffer — a fresh connect then delivers within + /// ~0.5 s, so waiting out the full default budget on a doomed stream just delays the retry + /// that fixes it. Backends without an internal wait budget ignore it (the default delegates). + fn next_frame_within(&mut self, _budget: std::time::Duration) -> Result { + self.next_frame() + } + /// Non-blocking: the freshest frame available since the last call, or `None` if none has /// arrived (the caller reuses its last frame to hold a steady output rate). The default /// just produces a frame each call — fine for instant synthetic sources; the portal diff --git a/crates/pf-capture/src/linux/mod.rs b/crates/pf-capture/src/linux/mod.rs index a5d68e91..eca8f323 100644 --- a/crates/pf-capture/src/linux/mod.rs +++ b/crates/pf-capture/src/linux/mod.rs @@ -299,29 +299,11 @@ fn spawn_pipewire( impl Capturer for PortalCapturer { fn next_frame(&mut self) -> Result { - // First frame can lag behind format negotiation; later frames arrive at ~fps. Wait in - // short slices so a GPU-import poison (worker death) fails the capture within ~0.5 s - // instead of sitting out the full first-frame budget. - let deadline = std::time::Instant::now() + Duration::from_secs(10); - loop { - if self.broken.load(Ordering::Relaxed) { - return Err(anyhow!( - "zero-copy GPU import lost (node {}): the import worker died or tiled imports \ - failed repeatedly — rebuilding capture", - self.node_id - )); - } - if let Some(f) = self.pending.take() { - return Ok(f); // a wait_arrival stash outranks the channel (it's older) - } - let slice = Duration::from_millis(500) - .min(deadline.saturating_duration_since(std::time::Instant::now())); - match self.frames.recv_timeout(slice) { - Ok(frame) => return Ok(frame), - Err(RecvTimeoutError::Timeout) if std::time::Instant::now() < deadline => continue, - Err(e) => return self.next_frame_timed_out(e), - } - } + self.frame_within(Duration::from_secs(10)) + } + + fn next_frame_within(&mut self, budget: Duration) -> Result { + self.frame_within(budget) } fn supports_arrival_wait(&self) -> bool { @@ -417,9 +399,41 @@ impl Capturer for PortalCapturer { } impl PortalCapturer { - /// The [`Capturer::next_frame`] budget expired (or the thread ended) — turn it into the - /// diagnosis-bearing error. Split out of the slicing loop above; behavior unchanged. - fn next_frame_timed_out(&self, err: RecvTimeoutError) -> Result { + /// The blocking first-frame wait behind [`Capturer::next_frame`] / + /// [`Capturer::next_frame_within`]. First frame can lag behind format negotiation; later + /// frames arrive at ~fps. Wait in short slices so a GPU-import poison (worker death) fails + /// the capture within ~0.5 s instead of sitting out the full first-frame budget. + fn frame_within(&mut self, budget: Duration) -> Result { + let deadline = std::time::Instant::now() + budget; + loop { + if self.broken.load(Ordering::Relaxed) { + return Err(anyhow!( + "zero-copy GPU import lost (node {}): the import worker died or tiled imports \ + failed repeatedly — rebuilding capture", + self.node_id + )); + } + if let Some(f) = self.pending.take() { + return Ok(f); // a wait_arrival stash outranks the channel (it's older) + } + let slice = Duration::from_millis(500) + .min(deadline.saturating_duration_since(std::time::Instant::now())); + match self.frames.recv_timeout(slice) { + Ok(frame) => return Ok(frame), + Err(RecvTimeoutError::Timeout) if std::time::Instant::now() < deadline => continue, + Err(e) => return self.next_frame_timed_out(e, budget), + } + } + } + + /// The [`frame_within`](Self::frame_within) budget expired (or the thread ended) — turn it + /// into the diagnosis-bearing error. Split out of the slicing loop above; behavior unchanged. + fn next_frame_timed_out( + &self, + err: RecvTimeoutError, + budget: Duration, + ) -> Result { + let within = budget.as_secs_f32(); match err { RecvTimeoutError::Timeout => { // Split the two black-screen root causes apart so the operator gets a cause, not @@ -427,9 +441,10 @@ impl PortalCapturer { // not (no acceptable format / node never emitted a param)? if self.negotiated.load(Ordering::Relaxed) { Err(anyhow!( - "no PipeWire frame within 10s (node {}): format negotiated but no buffers \ - arrived — the compositor produced no frames (virtual output idle/unmapped, \ - or capture never started)", + "no PipeWire frame within {within}s (node {}): format negotiated but no \ + buffers arrived — the compositor produced no frames (virtual output \ + idle/unmapped, capture never started, or a stream bound during a \ + compositor (re)start that will never deliver — a reconnect fixes that)", self.node_id )) } else if self.hdr_offer { @@ -440,10 +455,10 @@ impl PortalCapturer { // auto-reconnects) negotiates SDR instead of re-running this same timeout. super::note_hdr_capture_failed(); Err(anyhow!( - "no PipeWire frame within 10s (node {}): the compositor never accepted \ - the HDR (10-bit PQ/BT.2020 dmabuf) offer — is the mirrored monitor in \ - HDR mode on GNOME 50+? Downgrading this host to SDR capture; reconnect \ - to stream SDR", + "no PipeWire frame within {within}s (node {}): the compositor never \ + accepted the HDR (10-bit PQ/BT.2020 dmabuf) offer — is the mirrored \ + monitor in HDR mode on GNOME 50+? Downgrading this host to SDR capture; \ + reconnect to stream SDR", self.node_id )) } else if self.vaapi_dmabuf && !pf_zerocopy::vaapi_dmabuf_forced() { @@ -452,14 +467,15 @@ impl PortalCapturer { // retries on the CPU offer instead of failing this same negotiation forever. pf_zerocopy::note_vaapi_dmabuf_failed(); Err(anyhow!( - "no PipeWire frame within 10s (node {}): the compositor never accepted \ - the LINEAR-dmabuf offer (VAAPI zero-copy) — downgrading this host to the \ - CPU capture path; the pipeline rebuild will renegotiate without dmabuf", + "no PipeWire frame within {within}s (node {}): the compositor never \ + accepted the LINEAR-dmabuf offer (VAAPI zero-copy) — downgrading this \ + host to the CPU capture path; the pipeline rebuild will renegotiate \ + without dmabuf", self.node_id )) } else { Err(anyhow!( - "no PipeWire frame within 10s (node {}): format negotiation never \ + "no PipeWire frame within {within}s (node {}): format negotiation never \ completed — the compositor offered no format this consumer accepts \ (pixel-format/modifier mismatch) or the node never emitted a Format param", self.node_id diff --git a/crates/punktfunk-host/src/native/stream.rs b/crates/punktfunk-host/src/native/stream.rs index 9c5ce530..97fd9c3c 100644 --- a/crates/punktfunk-host/src/native/stream.rs +++ b/crates/punktfunk-host/src/native/stream.rs @@ -1111,6 +1111,7 @@ pub(super) fn virtual_stream(ctx: SessionContext, prepared: Option { @@ -1920,6 +1926,16 @@ pub(super) fn virtual_stream(ctx: SessionContext, prepared: Option break p, @@ -2655,6 +2671,7 @@ pub(super) fn prepare_display( plan, quit, stop, + 8, Some(trace), )?; Ok(PreparedDisplay { vd, pipeline }) @@ -2679,15 +2696,22 @@ fn build_pipeline_with_retry( plan: crate::session_plan::SessionPlan, quit: &Arc, stop: &Arc, + // Retry budget: 8 everywhere EXCEPT the capture-loss rebuild (2). That path wraps this call + // in its own outer loop that RE-DETECTS the active session between calls — during a + // Gaming↔Desktop switch the old compositor is simply gone, so burning 8 attempts (~13 s) + // against its dead socket only delays following the box to the session that replaced it + // (observed live: a Desktop→Gaming switch spent 13 of its 27 s retrying gone-KWin). + max_attempts: u32, // Transition trace (P0.1): `Some` for the traced builds (bring-up, resize); each stage stamps // once (first crossing) so the retry loop can pass it through unconditionally. trace: Option<&crate::bringup::Trace>, ) -> Result { - // ~10s first-frame wait per attempt. 8 gives a ~90s budget for the SLOW case: a host-managed - // gamescope session cold-starting Steam Big Picture (the SteamOS/Bazzite takeover) can take - // 30-60s to produce its first frame, and a first-connect timeout would tear down the warm - // session (forcing another cold start on reconnect). A genuinely permanent failure still fails - // fast via `is_permanent_build_error`; only transient "no frame yet" retries consume the budget. + // ~10s first-frame wait per attempt (attempt 1: see FIRST_ATTEMPT_FRAME_BUDGET below). 8 + // gives a ~80s budget for the SLOW case: a host-managed gamescope session cold-starting Steam + // Big Picture (the SteamOS/Bazzite takeover) can take 30-60s to produce its first frame, and + // a first-connect timeout would tear down the warm session (forcing another cold start on + // reconnect). A genuinely permanent failure still fails fast via `is_permanent_build_error`; + // only transient "no frame yet" retries consume the budget. // IDD-push only: HOLD one monitor lease across all build attempts. A failed attempt's capturer // drop releases ITS lease, but this held lease keeps the shared monitor Active (refs >= 1), so the // next attempt's `vd.create` JOINS it (refcount++) instead of finding it Lingering and tripping the @@ -2705,9 +2729,16 @@ fn build_pipeline_with_retry( } else { None }; - const MAX_ATTEMPTS: u32 = 8; + // Attempt 1 waits only briefly for the first frame: a PipeWire stream connected while + // gamescope re-initializes its headless takeover negotiates a format and reaches `Streaming` + // but never receives a buffer — a FRESH connect then delivers within ~0.5 s (observed on + // SteamOS: every gamescope bring-up burned the full 10 s on attempt 1, then attempt 2 got + // frames instantly → 17 s bring-ups). Healthy compositors deliver the first frame well inside + // this window (KWin ~0.3 s), and the genuinely-slow cold start above still gets the patient + // 10 s window on every later attempt. + const FIRST_ATTEMPT_FRAME_BUDGET: std::time::Duration = std::time::Duration::from_millis(2500); let mut backoff = std::time::Duration::from_millis(500); - for attempt in 1..=MAX_ATTEMPTS { + for attempt in 1..=max_attempts { // The client is gone (connection closed → `stop`): every further attempt only churns the // box for a session no one is watching — on a Bazzite takeover that means SIGKILLing and // relaunching the box's Steam session once per attempt for minutes (the .181 storm @@ -2719,7 +2750,17 @@ fn build_pipeline_with_retry( attempt - 1 ); } - match build_pipeline(vd, mode, bitrate_kbps, bit_depth, plan, quit, trace) { + let first_frame_budget = (attempt == 1).then_some(FIRST_ATTEMPT_FRAME_BUDGET); + match build_pipeline( + vd, + mode, + bitrate_kbps, + bit_depth, + plan, + quit, + first_frame_budget, + trace, + ) { Ok(pipe) => { if attempt > 1 { tracing::info!(attempt, "pipeline up after retry"); @@ -2729,7 +2770,7 @@ fn build_pipeline_with_retry( Err(e) => { let chain = format!("{e:#}"); let permanent = is_permanent_build_error(&chain); - if permanent || attempt == MAX_ATTEMPTS { + if permanent || attempt == max_attempts { let why = if permanent { "permanent" } else { @@ -2741,7 +2782,7 @@ fn build_pipeline_with_retry( } tracing::warn!( attempt, - max = MAX_ATTEMPTS, + max = max_attempts, backoff_ms = backoff.as_millis() as u64, error = %chain, "pipeline build failed — retrying" @@ -2802,6 +2843,10 @@ fn build_pipeline( bit_depth: u8, plan: crate::session_plan::SessionPlan, quit: &Arc, + // First-frame wait override (`None` = the backend's default 10 s): the retry loop shortens + // its FIRST attempt so a stream stuck in the gamescope takeover race fails over to the + // reconnect that fixes it (see FIRST_ATTEMPT_FRAME_BUDGET in `build_pipeline_with_retry`). + first_frame_budget: Option, // Transition trace (P0.1): stamps the build's stages (display acquire, capture attach, first // frame, encoder open) into the bring-up/resize timeline. `None` on untraced rebuilds. trace: Option<&crate::bringup::Trace>, @@ -2857,7 +2902,11 @@ fn build_pipeline( t.mark("capture_attached"); } capturer.set_active(true); - let frame = match capturer.next_frame().context("first frame") { + let first = match first_frame_budget { + Some(budget) => capturer.next_frame_within(budget), + None => capturer.next_frame(), + }; + let frame = match first.context("first frame") { Ok(f) => f, Err(e) => { // A reused kept display was dead — invalidate it so the next attempt creates fresh (A2).