Merge pull request 'fix(stall): T2 amplification kill — resume-edge pacing + ABR starved-window guard' (#53) from worktree-stall-ride-through into main

crates/punktfunk-core/src/abr.rs

@@ -159,6 +159,17 @@ const CAP_REPROBE_WINDOWS_MAX: u32 = 128;
 /// choke again at the same place, and only backoffs at a climbed-to rate can agree within the
 /// band (a cascade's second backoff sits at ×0.7 of the first: outside it by construction).
 const DECODE_CAP_SIMILAR_DIV: u32 = 8;
+/// A deciding window that DELIVERED under `current / STARVED_DELIVERY_DIV` is STARVED: the
+/// stream barely flowed (a host-side capture stall, an outage, a mid-window pause), so whatever
+/// distress the window carries — a flush, a keyframe-ask burst — is starvation-shaped, not
+/// rate-shaped, and the decoder decoded almost nothing at the nominal rate. Such a window may
+/// still back off (real damage deserves the safe response) but must never be a decode-knee
+/// sample: latching `current_kbps` off a starved window teaches a phantom decoder cap at
+/// whatever rate the stall interrupted (the periodic-capture-stall field case: every 5 s cycle
+/// offers another pair of "backoffs" at the same rate — a bogus latch that then fights the
+/// re-probe ladder for minutes). Deliberately far below the ×¾ utilization bar climbs require:
+/// the band between them is ambiguous and keeps today's behavior.
+const STARVED_DELIVERY_DIV: u32 = 4;
 /// Rolling window (in 750 ms report windows, ~30 s) whose minimum mean is the OWD baseline.
 /// Long enough to remember the uncongested floor, short enough to follow genuine path changes.
 const BASELINE_WINDOWS: usize = 40;
@@ -697,6 +708,10 @@ impl BitrateController {
                 || self.streak_decode_windows >= BAD_WINDOWS_TO_DECREASE
                 || (recovery_kf >= RECOVERY_KF_BAD && loss_ppm < HEAVY_LOSS_PPM)
                 || (flushed && (decode_bad || decode_mean_us.is_none()));
+            // Starved deciding window (see [`STARVED_DELIVERY_DIV`]): the stream barely flowed,
+            // so the window says nothing about what the decoder can hold at this rate.
+            let starved =
+                (actual_kbps as u64) * (STARVED_DELIVERY_DIV as u64) < self.current_kbps as u64;
             if !self.climb_since_backoff {
                 // Still draining the previous backoff: the host acks a ×0.7 request in ~100 ms,
                 // so this window's rate is one the decoder never choked at while keeping up —
@@ -708,6 +723,17 @@ impl BitrateController {
                     "adaptive bitrate: backoff without an intervening climb — draining the \
                      previous choke, not a knee sample"
                 );
+            } else if starved {
+                // Same "not a knee sample either way" treatment as the draining arm: neither
+                // latch against a starved window nor let it erase the reference a real knee
+                // set — the next genuine choke at that rate must still find its pair.
+                tracing::debug!(
+                    at_kbps = self.current_kbps,
+                    actual_kbps,
+                    reference_kbps = self.decode_backoff_kbps,
+                    "adaptive bitrate: backoff in a starved window (delivery a fraction of \
+                     the target) — starvation-shaped distress, not a knee sample"
+                );
             } else if decode_evidence {
                 let rate = self.current_kbps;
                 let similar = self.decode_backoff_kbps > 0
@@ -2084,6 +2110,100 @@ mod tests {
         rate - rate / 16
     }
 
+    /// One capture-stall-shaped window at the current rate: almost nothing delivered
+    /// (current/10), nothing decoded, no loss — but a jump-to-live flush and a keyframe-ask
+    /// storm (the stall edge's damage signature). SEVERE, so it backs off; STARVED, so it must
+    /// never be a knee sample.
+    fn stall_choke(c: &mut BitrateController, start: Instant, tick: &mut u32) -> Option<u32> {
+        *tick += 2;
+        let r = c.on_window(
+            ticks(start, *tick),
+            0,
+            0,
+            None,
+            None,
+            None,
+            c.current_kbps / 10,
+            true,
+            RECOVERY_KF_SEVERE,
+        );
+        *tick += 1;
+        r
+    }
+
+    #[test]
+    fn capture_stall_windows_never_latch_a_decode_cap() {
+        // The periodic-capture-stall field case (RDNA4 standby-sink, 5 s cycle): every stall
+        // edge offers another flush + kf-storm "backoff" at the SAME rate — without the starved
+        // guard that pair latches a phantom decoder knee at whatever rate the display driver
+        // happened to interrupt, and the session then fights the re-probe ladder for minutes.
+        let mut c = BitrateController::new(240_000);
+        c.set_ceiling(900_000);
+        let start = Instant::now();
+        let mut t = 0;
+        for _ in 0..4 {
+            calm_window(&mut c, ticks(start, t));
+            t += 1;
+        }
+        climb_to(&mut c, start, &mut t, 400_000);
+        let at = c.current_kbps;
+        let r1 = stall_choke(&mut c, start, &mut t).expect("stall damage still backs off");
+        assert!(
+            c.decode_cap_kbps.is_none(),
+            "one starved window must not latch"
+        );
+        assert_eq!(
+            c.decode_backoff_kbps, 0,
+            "a starved window is not a knee sample — no reference recorded"
+        );
+        c.on_ack(r1);
+        climb_to(&mut c, start, &mut t, at - at / DECODE_CAP_SIMILAR_DIV);
+        let r2 = stall_choke(&mut c, start, &mut t).expect("second stall edge backs off too");
+        c.on_ack(r2);
+        assert!(
+            c.decode_cap_kbps.is_none(),
+            "a starved pair at the same rate must not latch a phantom knee"
+        );
+    }
+
+    #[test]
+    fn starved_window_preserves_the_knee_reference() {
+        // A REAL knee sample, then a stall edge, then the genuine re-climb choke: the starved
+        // window in the middle must neither latch nor ERASE the reference the real choke set —
+        // the genuine pair must still find each other around it.
+        let mut c = BitrateController::new(500_000);
+        c.set_ceiling(900_000);
+        let start = Instant::now();
+        let mut t = 0;
+        for _ in 0..4 {
+            calm_window(&mut c, ticks(start, t));
+            t += 1;
+        }
+        let knee = c.current_kbps;
+        let r1 = choke(&mut c, start, &mut t).expect("real choke backs off");
+        assert_eq!(
+            c.decode_backoff_kbps, knee,
+            "real choke records the reference"
+        );
+        c.on_ack(r1);
+        climb_to(&mut c, start, &mut t, knee - knee / DECODE_CAP_SIMILAR_DIV);
+        let r2 = stall_choke(&mut c, start, &mut t).expect("stall edge backs off");
+        assert_eq!(
+            c.decode_backoff_kbps, knee,
+            "the starved window must not erase the real reference"
+        );
+        assert!(c.decode_cap_kbps.is_none(), "and must not latch against it");
+        c.on_ack(r2);
+        climb_to(&mut c, start, &mut t, knee - knee / DECODE_CAP_SIMILAR_DIV);
+        let rate = c.current_kbps;
+        choke(&mut c, start, &mut t).expect("genuine re-climb choke backs off");
+        assert_eq!(
+            c.decode_cap_kbps,
+            Some(rate - rate / 16),
+            "the genuine pair still latches around the starved interruption"
+        );
+    }
+
     #[test]
     fn decode_cap_latches_when_the_reclimb_chokes_at_the_same_knee() {
         // The 1440p120 field sawtooth: a decoder knee (~500 Mbps) well under the (inflated)

crates/punktfunk-host/src/native/stream.rs

@@ -441,26 +441,27 @@ fn idd_adaptive_enabled() -> bool {
 /// Seal one access unit and send it with MICROBURST pacing (the shared
 /// [`send_pacing`](crate::send_pacing) policy, native parameterization): the first `burst_cap`
 /// bytes go out immediately (one absorbed burst the NIC / socket tx-buffer can swallow), and
-/// only the OVERFLOW beyond that is spread across `min(~90% of the time to deadline, the time
-/// the overflow needs at pace_rate_bps)` in ADAPTIVE chunks — 16 packets at today's rates,
-/// coarsening to at most 64 (the GSO-segment cap) once the rate would otherwise skip every
-/// sub-floor sleep, so ≥1 Gbps frames still pace instead of collapsing into an unpaced blast
-/// (plan Phase 1.2). `burst_cap` `None` = auto: `max(128 KB, this AU's wire bytes / 4)`, so
-/// the burst stays a bounded fraction of a high-rate frame instead of swallowing it whole
-/// (plan Phase 1.3); `Some` = PUNKTFUNK_PACE_BURST_KB pinned an absolute cap. So a
-/// normal-bitrate frame (≤ cap) leaves in one immediate burst at ~0 added latency, while a
-/// genuine IDR / sustained-high-bitrate frame (≫ cap) still spreads — keeping the freeze fix
-/// exactly where it's needed (an unpaced line-rate burst overruns the kernel tx buffer →
-/// EAGAIN drop → under infinite GOP, a freeze until the next keyframe). With no slack
-/// (encode ≈ interval) the budget collapses to 0 and even the overflow goes out immediately,
-/// so this is never slower than unpaced.
+/// only the OVERFLOW beyond that is spread across the time it needs at `pace_rate_bps` in
+/// ADAPTIVE chunks — 16 packets at today's rates, coarsening to at most 64 (the GSO-segment
+/// cap) once the rate would otherwise skip every sub-floor sleep, so ≥1 Gbps frames still pace
+/// instead of collapsing into an unpaced blast (plan Phase 1.2). `burst_cap` `None` = auto:
+/// `max(128 KB, this AU's wire bytes / 4)`, so the burst stays a bounded fraction of a
+/// high-rate frame instead of swallowing it whole (plan Phase 1.3); `Some` =
+/// PUNKTFUNK_PACE_BURST_KB pinned an absolute cap. So a normal-bitrate frame (≤ cap) leaves in
+/// one immediate burst at ~0 added latency, while a genuine IDR / sustained-high-bitrate frame
+/// (≫ cap) still spreads — keeping the freeze fix exactly where it's needed (an unpaced
+/// line-rate burst overruns the kernel tx buffer → EAGAIN drop → under infinite GOP, a freeze
+/// until the next keyframe).
 ///
-/// `pace_rate_bps` (latency plan T1.2) bounds the spread from above: the deadline term alone
-/// smears a big frame's tail across the whole remaining interval (~15 ms at 60 fps) even when
-/// the link could drain it in 2–3 ms. The caller passes ~3× the live encoder bitrate — a rate
-/// the link is proven to carry sustained, so the bounded excursion keeps the anti-freeze
-/// property while the tail leaves as soon as the link plausibly allows. `0` = uncapped
-/// (legacy smoothness-only spread, and the fallback when the bitrate isn't known yet).
+/// `pace_rate_bps` (latency plan T1.2; resume-safe form, stall program T2): the caller passes
+/// ~3× the live encoder bitrate — a rate the link is proven to carry sustained — and the
+/// overflow's wire time at that rate IS the pace budget ([`crate::send_pacing::native_budget`],
+/// [`crate::send_pacing::MAX_PACE_SPREAD`]-bounded). The frame deadline no longer under-cuts
+/// the spread: for a steady-state frame the rate term was the smaller one anyway (tail gone in
+/// a fraction of the interval), and for an oversized frame (stall-resume scene delta, cold
+/// IDR) the old deadline clamp was exactly the line-rate blast → tx-overrun → freeze path this
+/// module exists to prevent. `0` = uncapped legacy deadline-only spread
+/// (PUNKTFUNK_PACE_FACTOR=0, and the fallback when the bitrate isn't known yet).
 #[allow(clippy::too_many_arguments)]
 fn paced_submit(
     session: &mut Session,
@@ -498,34 +499,22 @@ fn pace_sealed(
         chunk: crate::send_pacing::ChunkPolicy::Adaptive { base: 16, max: 64 },
         sleep_floor: std::time::Duration::from_micros(500),
     };
-    // T1.2 rate cap: the overflow's wire time at `pace_rate_bps`. Only the bytes past the
-    // burst pace at all, so only they bound the budget.
+    // T1.2 rate cap, resume-safe form (stall program T2): the overflow's wire time at
+    // `pace_rate_bps` IS the budget — the deadline no longer under-cuts it, so an oversized
+    // frame (a stall-resume scene delta, a cold IDR) paces at the proven 3× rate instead of
+    // collapsing into a line-rate blast that overruns the socket buffer and loses the very
+    // frame that ends a freeze. See `send_pacing::native_budget` for the full argument.
     let overflow_bytes = wire_bytes.saturating_sub(burst_bytes) as u64;
-    let cap = if pace_rate_bps > 0 && overflow_bytes > 0 {
-        std::time::Duration::from_nanos(
-            (overflow_bytes * 8).saturating_mul(1_000_000_000) / pace_rate_bps,
-        )
-    } else {
-        std::time::Duration::MAX
-    };
+    let budget = crate::send_pacing::native_budget(deadline, pace_rate_bps, overflow_bytes);
     // Time the socket handoff per chunk and fold it into the session's SealPerf split — the
     // sleeps between chunks stay excluded, so sock_ns is pure send_gso/sendmmsg time.
     let mut sock_ns = 0u64;
-    let result = crate::send_pacing::pace_frame(
-        &refs,
-        crate::send_pacing::PaceBudget::UntilDeadline {
-            deadline,
-            fraction: 0.9,
-            cap,
-        },
-        &cfg,
-        |chunk| {
-            let t0 = std::time::Instant::now();
-            let r = session.send_sealed(chunk).map(|_| ());
-            sock_ns += t0.elapsed().as_nanos() as u64;
-            r
-        },
-    );
+    let result = crate::send_pacing::pace_frame(&refs, budget, &cfg, |chunk| {
+        let t0 = std::time::Instant::now();
+        let r = session.send_sealed(chunk).map(|_| ());
+        sock_ns += t0.elapsed().as_nanos() as u64;
+        r
+    });
     drop(refs); // release the borrow of `wires` so it can return to the seal pool
     session.reclaim_wires(wires);
     session.note_sock_ns(sock_ns);

crates/punktfunk-host/src/send_pacing.rs

@@ -55,7 +55,7 @@ pub(crate) enum ChunkPolicy {
 }
 
 /// The time the paced (post-burst) packets spread across.
-#[derive(Clone, Copy, Debug)]
+#[derive(Clone, Copy, Debug, PartialEq)]
 pub(crate) enum PaceBudget {
     /// `min((deadline − now-after-burst) × fraction, cap)`, collapsing to 0 with no slack
     /// (native: fraction 0.9). `cap` bounds the spread to the time the overflow actually needs
@@ -68,10 +68,53 @@ pub(crate) enum PaceBudget {
         fraction: f32,
         cap: Duration,
     },
-    /// A precomputed fixed budget (GameStream: ¾ of the frame interval).
+    /// A precomputed fixed budget (GameStream: ¾ of the frame interval; native: the rate-cap
+    /// spread from [`native_budget`]).
     Fixed(Duration),
 }
 
+/// Absolute ceiling on one frame's paced spread (native plane): a pathological frame must not
+/// park the send thread for longer than this, whatever the rate math says. At the ceiling the
+/// tail is late but delivered whole — still strictly better than the blast-loss → freeze →
+/// recovery-IDR round trip it replaces.
+pub(crate) const MAX_PACE_SPREAD: Duration = Duration::from_millis(100);
+
+/// The native plane's pace budget for one frame (pure — unit-tested): with the T1.2 rate cap
+/// active, the paced overflow spreads across exactly the time it needs at the pace rate
+/// (`cap`, bounded by [`MAX_PACE_SPREAD`]) and is NEVER under-cut by the frame deadline.
+///
+/// The old schedule took `min(0.9 × time-to-deadline, cap)`. For a steady-state frame the cap
+/// is the smaller term and nothing changes. But for an OVERSIZED frame — a stall-resume scene
+/// delta after seconds of frozen composition, a cold IDR — the overflow needs SEVERAL frame
+/// intervals at the pace rate, and the deadline term clamped that into the remainder of ONE:
+/// an instantaneous many-×-stream-rate blast that overruns the socket tx-buffer and loses the
+/// very frame that would have ended the freeze (field fingerprint: WSAENOBUFS 10055 +
+/// `loss_ppm` spikes at capture-stall edges, then a recovery-IDR round trip per retry). The
+/// pace rate is ~3× a rate the link demonstrably carries, so holding it past the deadline is
+/// safe by the same argument that introduced the cap — the deadline stays a *target*, not a
+/// license to blast.
+///
+/// `pace_rate_bps == 0` (PUNKTFUNK_PACE_FACTOR=0) or an overflow-free frame keeps the legacy
+/// deadline-only spread.
+pub(crate) fn native_budget(
+    deadline: Instant,
+    pace_rate_bps: u64,
+    overflow_bytes: u64,
+) -> PaceBudget {
+    if pace_rate_bps > 0 && overflow_bytes > 0 {
+        let cap = Duration::from_nanos(
+            (overflow_bytes * 8).saturating_mul(1_000_000_000) / pace_rate_bps,
+        );
+        PaceBudget::Fixed(cap.min(MAX_PACE_SPREAD))
+    } else {
+        PaceBudget::UntilDeadline {
+            deadline,
+            fraction: 0.9,
+            cap: Duration::MAX,
+        }
+    }
+}
+
 /// Per-plane pacing parameters. See the module doc for the two canonical values.
 #[derive(Clone, Copy, Debug)]
 pub(crate) struct PaceCfg {
@@ -598,6 +641,43 @@ mod tests {
         );
     }
 
+    /// [`native_budget`]: with the rate cap active the budget is the overflow's wire time at
+    /// the pace rate — a FIXED spread the deadline can no longer under-cut — bounded by
+    /// [`MAX_PACE_SPREAD`]; rate 0 / no overflow keep the legacy deadline-only schedule.
+    #[test]
+    fn native_budget_is_rate_bound_never_deadline_cut() {
+        // The stall-resume case the fix exists for: a 3 MB overflow at 3×240 Mbps needs
+        // ~33 ms — an IMMINENT deadline (the old min() made this a blast) must not shrink it.
+        let deadline = Instant::now() + Duration::from_millis(4); // 240 fps interval
+        let b = native_budget(deadline, 720_000_000, 3_000_000);
+        assert_eq!(b, PaceBudget::Fixed(Duration::from_nanos(33_333_333)));
+
+        // A steady-state frame: overflow 90 KB at 3×240 Mbps = 1 ms — identical to what the
+        // old min(slack, cap) chose (cap was the smaller term), so nothing regresses.
+        let b = native_budget(deadline, 720_000_000, 90_000);
+        assert_eq!(b, PaceBudget::Fixed(Duration::from_micros(1_000)));
+
+        // A crater-rate resume (ABR backed off to 20 Mbps, pace 60 Mbps): the raw rate math
+        // says 400 ms for 3 MB — the absolute ceiling bounds the send thread's stall.
+        let b = native_budget(deadline, 60_000_000, 3_000_000);
+        assert_eq!(b, PaceBudget::Fixed(MAX_PACE_SPREAD));
+
+        // Rate cap off (PUNKTFUNK_PACE_FACTOR=0): the legacy deadline-only spread, uncapped.
+        let b = native_budget(deadline, 0, 3_000_000);
+        assert!(matches!(
+            b,
+            PaceBudget::UntilDeadline {
+                fraction,
+                cap: Duration::MAX,
+                ..
+            } if fraction == 0.9
+        ));
+
+        // No overflow (the whole frame bursts): budget is never consulted — legacy shape.
+        let b = native_budget(deadline, 720_000_000, 0);
+        assert!(matches!(b, PaceBudget::UntilDeadline { .. }));
+    }
+
     /// `inject_video_drop` is a no-op when the knob is off (the default test env).
     #[test]
     fn drop_injection_off_by_default() {