refactor(pf-encode): extract the range-family RFI recovery policy (WP7.2)

The two direct-NVENC backends carried hand-copied twins of the same
loss-recovery decision: range validity, covering-range dedup, DPB
window, clamp — ~30 duplicated lines each. The decision now lives once
as nvenc_core::plan_range_recovery (the range half of WP7.2; the slot
half is enc/rfi.rs), pure and unit-tested; each backend keeps its
session gate, its unsafe per-timestamp driver loop, and its state
stores.

The step order is load-bearing and now pinned by tests: the covering
dedup runs with the UNCLAMPED last and BEFORE the DPB window (a covered
re-ask never touches the driver even when the range has since aged out
of the DPB), the boundary at next_ts - RFI_DPB is inclusive, and the
Invalidate carries the CLAMPED last — which is also what the caller
records in last_rfi_range, exactly as the inline code stored it. A
driver failure mid-loop still returns false with NO range recorded and
no anchor armed. Decline deliberately clears nothing (neither twin
touched pending_anchor on decline — same shape as Vulkan's non-clear,
opposite of AMF/QSV; do not harmonize).

The exact-cover → Covered test records EXISTING behavior including that
a covered range survives a forced IDR with zero driver calls — a
recorded fact, not an endorsement. RFI_DPB's import leaves both twins:
its only per-backend use was the arithmetic that moved.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

crates/pf-encode/src/enc/linux/nvenc_cuda.rs

@@ -61,9 +61,9 @@
 #![deny(clippy::undocumented_unsafe_blocks)]
 
 use super::nvenc_core::{
-    apply_low_latency_config, build_init_params, cached_ceiling, codec_guid, resolve_slices,
-    resolve_split_mode, resolve_subframe, store_ceiling, CeilingKey, LowLatencyConfig, NvStatusExt,
-    RFI_DPB,
+    apply_low_latency_config, build_init_params, cached_ceiling, codec_guid, plan_range_recovery,
+    resolve_slices, resolve_split_mode, resolve_subframe, store_ceiling, CeilingKey,
+    LowLatencyConfig, NvStatusExt, RangePlan,
 };
 use super::nvenc_status;
 use super::{AuChunk, ChromaFormat, Codec, EncodedFrame, Encoder, EncoderCaps};
@@ -1847,48 +1847,46 @@ impl Encoder for NvencCudaEncoder {
     }
 
     fn invalidate_ref_frames(&mut self, first: i64, last: i64) -> bool {
-        if self.encoder.is_null() || !self.rfi_supported || first < 0 || first > last {
+        // Range validity, covering-range dedup, DPB window and clamp all live in
+        // `nvenc_core::plan_range_recovery` — one policy for both direct-NVENC backends; only the
+        // session gate and the driver loop are this backend's.
+        if self.encoder.is_null() || !self.rfi_supported {
             return false;
         }
-        // Already invalidated a covering range for this loss event — re-arm the anchor (the previous
-        // anchor AU may itself have been lost) but skip the driver calls.
-        if let Some((pf, pl)) = self.last_rfi_range {
-            if first >= pf && last <= pl {
+        match plan_range_recovery(first, last, self.frame_idx, self.last_rfi_range) {
+            // Already invalidated a covering range for this loss event — re-arm the anchor (the
+            // previous anchor AU may itself have been lost) but skip the driver calls.
+            RangePlan::Covered => {
                 self.pending_anchor = true;
-                return true;
+                true
             }
-        }
-        // The DPB holds `[frame_idx - RFI_DPB, frame_idx - 1]`; a lost frame older than that can't be
-        // invalidated, so the only correct recovery is an IDR.
-        let oldest_in_dpb = self.frame_idx - RFI_DPB as i64;
-        if first < oldest_in_dpb {
-            return false;
-        }
-        let last = last.min(self.frame_idx - 1);
-        if first > last {
-            return false;
-        }
-        // Each input's `inputTimeStamp` is the WIRE frame index (pinned by `submit_indexed`), so the
-        // client's lost-frame range maps 1:1 onto the timestamps NVENC invalidates here.
-        // SAFETY: `invalidate_ref_frames` is a function pointer from the runtime table; `self.encoder`
-        // was checked non-null and is the live session; this runs on the encode thread (no concurrent
-        // NVENC use). Each `ts` is clamped to `[oldest_in_dpb, frame_idx - 1]`, naming a frame still
-        // in the DPB; the call passes only that `u64` (no struct).
-        unsafe {
-            for ts in first..=last {
-                if (api().invalidate_ref_frames)(self.encoder, ts as u64)
-                    .nv_ok()
-                    .is_err()
-                {
-                    return false;
+            RangePlan::Decline => false,
+            RangePlan::Invalidate { first, last } => {
+                // Each input's `inputTimeStamp` is the WIRE frame index (pinned by
+                // `submit_indexed`), so the client's lost-frame range maps 1:1 onto the timestamps
+                // NVENC invalidates here.
+                // SAFETY: `invalidate_ref_frames` is a function pointer from the runtime table;
+                // `self.encoder` was checked non-null and is the live session; this runs on the
+                // encode thread (no concurrent NVENC use). The plan clamped each `ts` to
+                // `[oldest_in_dpb, frame_idx - 1]`, naming a frame still in the DPB; the call
+                // passes only that `u64` (no struct).
+                unsafe {
+                    for ts in first..=last {
+                        if (api().invalidate_ref_frames)(self.encoder, ts as u64)
+                            .nv_ok()
+                            .is_err()
+                        {
+                            return false;
+                        }
+                    }
                 }
+                self.last_rfi_range = Some((first, last));
+                // The next submitted frame is the clean re-anchor — arm the tag so its AU ships
+                // with `recovery_anchor` and the client lifts its post-loss freeze on it.
+                self.pending_anchor = true;
+                true
             }
         }
-        self.last_rfi_range = Some((first, last));
-        // The next submitted frame is the clean re-anchor — arm the tag so its AU ships with
-        // `recovery_anchor` and the client lifts its post-loss freeze on it.
-        self.pending_anchor = true;
-        true
     }
 
     fn poll(&mut self) -> Result<Option<EncodedFrame>> {

crates/pf-encode/src/enc/nvenc_core.rs

@@ -294,10 +294,158 @@ mod tests {
 
 /// Reference-frame DPB depth when RFI is supported (Apollo uses 5). A deeper DPB lets an invalidated
 /// reference fall back to an older still-valid frame instead of a full IDR; `numRefL0 = 1` keeps each
-/// P-frame single-reference for low latency. Also the window the backends' `invalidate_ref_frames`
-/// paths check against (`frame_idx - RFI_DPB` = the oldest frame still in the DPB).
+/// P-frame single-reference for low latency. Also the window [`plan_range_recovery`] checks against
+/// (`next_ts - RFI_DPB` = the oldest frame still in the DPB).
 pub(super) const RFI_DPB: u32 = 5;
 
+/// One loss event's recovery decision for the timestamp-range RFI both direct-NVENC backends run
+/// (the range half of WP7.2's policy extraction; the slot half — AMF/QSV/Vulkan — is
+/// `crate::rfi`). The mechanism (the per-timestamp `nvEncInvalidateRefFrames` loop, the
+/// `last_rfi_range`/`pending_anchor` stores, the null-handle/`rfi_supported` gate) stays in each
+/// backend.
+pub(super) enum RangePlan {
+    /// The last successful invalidation already covers this range — no new driver calls, no IDR.
+    /// The caller must still RE-ARM its recovery anchor: the client re-asking means the previous
+    /// anchor AU may itself have been lost, and the next frame is just as clean a re-anchor.
+    Covered,
+    /// Invalidate `first..=last` (the CLAMPED range — this is also what the caller must record in
+    /// `last_rfi_range` on success, exactly as the inline code stored the post-clamp values).
+    Invalidate { first: i64, last: i64 },
+    /// Recovery without an IDR is impossible (nonsense range, loss older than the DPB, or a range
+    /// entirely in the future) — the caller returns `false` and its (coalesced) keyframe path
+    /// recovers. Deliberately NOT paired with any state clearing: neither twin touches
+    /// `pending_anchor` on decline (matching Vulkan's decline, opposite of AMF/QSV's
+    /// `pending_force` clear — see `crate::rfi`'s module doc before "harmonizing").
+    Decline,
+}
+
+/// The range-RFI policy, extracted verbatim from the two backends' `invalidate_ref_frames` (they
+/// were hand-copied twins). Step order is load-bearing and pinned by tests:
+///
+/// 1. nonsense range (`first < 0 || first > last`) → [`RangePlan::Decline`];
+/// 2. covering-range dedup — checked with the UNCLAMPED `last`, BEFORE the DPB window, so a
+///    covered re-ask never touches the driver even when the range has since left the DPB;
+/// 3. DPB window: `first < next_ts - RFI_DPB` → Decline (a lost frame older than the DPB cannot
+///    be invalidated; the only correct recovery is an IDR);
+/// 4. clamp `last` to `next_ts - 1` (never invalidate a timestamp never assigned); an inverted
+///    range after the clamp (loss entirely in the future — a prediction desync) → Decline.
+///
+/// `next_ts` is the backend's `frame_idx`: the NEXT timestamp to assign, which `submit_indexed`
+/// pins to the wire frame index — so the client's lost-frame range maps 1:1 onto the timestamps
+/// the driver invalidates, across every rebuild/reset. Note `teardown()` clears `last_rfi_range`
+/// but NOT `frame_idx`, so a post-reset call legitimately sees a stale-high `next_ts` with a
+/// `None` range — the same view the inline code had.
+pub(super) fn plan_range_recovery(
+    first: i64,
+    last: i64,
+    next_ts: i64,
+    last_rfi_range: Option<(i64, i64)>,
+) -> RangePlan {
+    if first < 0 || first > last {
+        return RangePlan::Decline;
+    }
+    if let Some((pf, pl)) = last_rfi_range {
+        if first >= pf && last <= pl {
+            return RangePlan::Covered;
+        }
+    }
+    let oldest_in_dpb = next_ts - RFI_DPB as i64;
+    if first < oldest_in_dpb {
+        return RangePlan::Decline;
+    }
+    let last = last.min(next_ts - 1);
+    if first > last {
+        return RangePlan::Decline;
+    }
+    RangePlan::Invalidate { first, last }
+}
+
+#[cfg(test)]
+mod range_policy_tests {
+    use super::{plan_range_recovery, RangePlan, RFI_DPB};
+
+    /// Convenience: the plan with no prior invalidation recorded.
+    fn plan(first: i64, last: i64, next_ts: i64) -> RangePlan {
+        plan_range_recovery(first, last, next_ts, None)
+    }
+
+    #[test]
+    fn nonsense_ranges_decline() {
+        assert!(matches!(plan(-1, 5, 100), RangePlan::Decline));
+        assert!(matches!(plan(7, 5, 100), RangePlan::Decline));
+    }
+
+    #[test]
+    fn covering_range_dedups_partial_overlap_does_not() {
+        let prior = Some((90i64, 95i64));
+        // Exact cover and sub-range → Covered. This pins EXISTING behavior, including that a
+        // covered range survives a forced IDR with zero driver calls (nothing clears
+        // `last_rfi_range` on a keyframe) — a recorded fact, not an endorsement.
+        assert!(matches!(
+            plan_range_recovery(90, 95, 100, prior),
+            RangePlan::Covered
+        ));
+        assert!(matches!(
+            plan_range_recovery(92, 94, 100, prior),
+            RangePlan::Covered
+        ));
+        // Partial overlap re-invalidates the FULL new range (next_ts = 98 keeps the window open:
+        // oldest_in_dpb = 93; at next_ts = 100 the same range would age out and Decline instead).
+        assert!(matches!(
+            plan_range_recovery(93, 97, 98, prior),
+            RangePlan::Invalidate {
+                first: 93,
+                last: 97
+            }
+        ));
+    }
+
+    /// The covering check runs BEFORE the DPB window: a covered re-ask stays Covered (no driver
+    /// calls needed) even when the range has since aged out of the DPB.
+    #[test]
+    fn covered_is_checked_before_the_dpb_window() {
+        let prior = Some((10i64, 12i64));
+        assert!(matches!(
+            plan_range_recovery(10, 12, 100, prior),
+            RangePlan::Covered
+        ));
+        // ...whereas the same range with no prior invalidation is outside the window → Decline.
+        assert!(matches!(plan(10, 12, 100), RangePlan::Decline));
+    }
+
+    #[test]
+    fn dpb_window_boundary() {
+        let next_ts = 100i64;
+        let oldest = next_ts - RFI_DPB as i64; // 95: the oldest timestamp still in the DPB
+        assert!(matches!(
+            plan(oldest, oldest, next_ts),
+            RangePlan::Invalidate { .. }
+        ));
+        assert!(matches!(
+            plan(oldest - 1, oldest, next_ts),
+            RangePlan::Decline
+        ));
+    }
+
+    /// `last` clamps to `next_ts - 1` (the newest encoded frame); the Invalidate carries the
+    /// CLAMPED value — which is also what the caller records in `last_rfi_range`.
+    #[test]
+    fn clamps_to_newest_encoded() {
+        assert!(matches!(
+            plan(98, 150, 100),
+            RangePlan::Invalidate {
+                first: 98,
+                last: 99
+            }
+        ));
+        // A range entirely in the future inverts under the clamp → Decline (prediction desync).
+        assert!(matches!(plan(100, 150, 100), RangePlan::Decline));
+        // Fresh session (`frame_idx == 0`): window passes (oldest = -5) but the clamp gives
+        // last = -1 < first → Decline. The inline code behaved identically.
+        assert!(matches!(plan(0, 3, 0), RangePlan::Decline));
+    }
+}
+
 /// The per-session knobs both direct-NVENC backends feed [`apply_low_latency_config`]. `Copy` so the
 /// backend fills it from `self` at the call. The two input-format fields bridge the only real
 /// divergence between the CUDA and D3D11 paths (which surface formats can carry full chroma / 10-bit

crates/pf-encode/src/enc/windows/nvenc.rs

@@ -37,9 +37,9 @@
 #![deny(clippy::undocumented_unsafe_blocks)]
 
 use super::nvenc_core::{
-    apply_low_latency_config, build_init_params, cached_ceiling, codec_guid, resolve_slices,
-    resolve_split_mode, resolve_subframe, store_ceiling, CeilingKey, LowLatencyConfig, NvStatusExt,
-    RFI_DPB,
+    apply_low_latency_config, build_init_params, cached_ceiling, codec_guid, plan_range_recovery,
+    resolve_slices, resolve_split_mode, resolve_subframe, store_ceiling, CeilingKey,
+    LowLatencyConfig, NvStatusExt, RangePlan,
 };
 use super::nvenc_status;
 use super::{ChromaFormat, Codec, EncodedFrame, Encoder, EncoderCaps};
@@ -1679,61 +1679,56 @@ impl Encoder for NvencD3d11Encoder {
     }
 
     fn invalidate_ref_frames(&mut self, first: i64, last: i64) -> bool {
-        // No live session, the GPU can't invalidate, or a nonsense range → caller forces a full IDR.
-        // (NVENC handles are single-threaded; this runs on the encode thread, like submit/poll.)
-        if self.encoder.is_null() || !self.rfi_supported || first < 0 || first > last {
+        // No live session or the GPU can't invalidate → caller forces a full IDR. (NVENC handles
+        // are single-threaded; this runs on the encode thread, like submit/poll.) Everything else
+        // — range validity, covering-range dedup, the DPB window, the clamp — is
+        // `nvenc_core::plan_range_recovery`, one policy for both direct-NVENC backends.
+        if self.encoder.is_null() || !self.rfi_supported {
             return false;
         }
-        // Already invalidated a covering range for this loss event — no new driver calls needed,
-        // no IDR. RE-ARM the anchor though: the client re-asking means the previous recovery
-        // anchor AU may itself have been lost, and the next frame is just as clean a re-anchor
-        // (it too references only valid frames).
-        if let Some((pf, pl)) = self.last_rfi_range {
-            if first >= pf && last <= pl {
+        match plan_range_recovery(first, last, self.frame_idx, self.last_rfi_range) {
+            // Already invalidated a covering range for this loss event — no new driver calls
+            // needed, no IDR. RE-ARM the anchor though: the client re-asking means the previous
+            // recovery anchor AU may itself have been lost, and the next frame is just as clean a
+            // re-anchor (it too references only valid frames).
+            RangePlan::Covered => {
                 self.pending_anchor = true;
-                return true;
+                true
             }
-        }
-        // `frame_idx` is the NEXT timestamp to assign, so the last encoded frame is `frame_idx - 1`
-        // and the DPB holds `[frame_idx - RFI_DPB, frame_idx - 1]`. A lost frame older than that
-        // can't be invalidated, so the only correct recovery is an IDR.
-        let oldest_in_dpb = self.frame_idx - RFI_DPB as i64;
-        if first < oldest_in_dpb {
-            return false;
-        }
-        // Clamp to frames we've actually encoded (don't invalidate a timestamp we never assigned).
-        let last = last.min(self.frame_idx - 1);
-        if first > last {
-            return false;
-        }
-        // Each input's `inputTimeStamp` is `frame_idx`, which `submit_indexed` pins to the WIRE
-        // frame index the AU carries — so the client's lost-frame range maps 1:1 onto the
-        // timestamps NVENC invalidates here, and stays 1:1 across encoder rebuilds/resets (an
-        // internal counter would desync on the first adaptive-bitrate rebuild and RFI would then
-        // clamp every range into first > last, silently degrading to IDR-only forever).
-        // SAFETY: `invalidate_ref_frames` is a function pointer from the runtime-loaded `EncodeApi` table.
-        // `self.encoder` was checked non-null at the top of this fn and is the live session; this runs
-        // on the encode thread (like submit/poll), so there is no concurrent NVENC use. Each `ts` was
-        // clamped to `[oldest_in_dpb, frame_idx - 1]` above, so it names a frame still in the session's
-        // DPB; the call passes only that `u64` timestamp (no struct), so there is no struct-size or
-        // lifetime concern.
-        unsafe {
-            for ts in first..=last {
-                if (api().invalidate_ref_frames)(self.encoder, ts as u64)
-                    .nv_ok()
-                    .is_err()
-                {
-                    return false; // any failure → fall back to IDR
+            RangePlan::Decline => false,
+            RangePlan::Invalidate { first, last } => {
+                // Each input's `inputTimeStamp` is `frame_idx`, which `submit_indexed` pins to the
+                // WIRE frame index the AU carries — so the client's lost-frame range maps 1:1 onto
+                // the timestamps NVENC invalidates here, and stays 1:1 across encoder
+                // rebuilds/resets (an internal counter would desync on the first adaptive-bitrate
+                // rebuild and RFI would then clamp every range into first > last, silently
+                // degrading to IDR-only forever).
+                // SAFETY: `invalidate_ref_frames` is a function pointer from the runtime-loaded
+                // `EncodeApi` table. `self.encoder` was checked non-null at the top of this fn and
+                // is the live session; this runs on the encode thread (like submit/poll), so there
+                // is no concurrent NVENC use. The plan clamped each `ts` to
+                // `[oldest_in_dpb, frame_idx - 1]`, so it names a frame still in the session's
+                // DPB; the call passes only that `u64` timestamp (no struct), so there is no
+                // struct-size or lifetime concern.
+                unsafe {
+                    for ts in first..=last {
+                        if (api().invalidate_ref_frames)(self.encoder, ts as u64)
+                            .nv_ok()
+                            .is_err()
+                        {
+                            return false; // any failure → fall back to IDR
+                        }
+                    }
                 }
+                self.last_rfi_range = Some((first, last));
+                // The next submitted frame is the first one encoded after the invalidation — the
+                // clean re-anchor P-frame. Arm the tag so its AU ships with `recovery_anchor` and
+                // the client lifts its post-loss freeze on it (instead of waiting ~1 s for the
+                // cooldown-suppressed IDR fallback).
+                self.pending_anchor = true;
+                true
             }
         }
-        self.last_rfi_range = Some((first, last));
-        // The next submitted frame is the first one encoded after the invalidation — the clean
-        // re-anchor P-frame. Arm the tag so its AU ships with `recovery_anchor` and the client
-        // lifts its post-loss freeze on it (instead of waiting ~1 s for the cooldown-suppressed
-        // IDR fallback).
-        self.pending_anchor = true;
-        true
     }
 
     fn poll(&mut self) -> Result<Option<EncodedFrame>> {