unom/punktfunk
5
0
Fork 0
Code Issues Pull Requests 2 Actions 15 Packages Projects Releases 36 Wiki Activity

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

Browse Source 
AMF (user-LTR bitfield), QSV (mfxExtRefListCtrl) and Vulkan Video (the
app-owned DPB slot table) each hand-implemented the same loss-recovery
decision: distrust every reference encoded at-or-after the loss start,
anchor on the newest one strictly older. Three copies had already
diverged once — the fecbec2d taint sweep reached AMF/QSV a commit before
the Vulkan backend was carved out, and Vulkan shipped without it. The
decision now lives once in enc/rfi.rs, pure and unit-tested (this path
had zero coverage and is the loss-recovery path); every mechanism —
how a force is applied, how distrust is persisted — stays in its
backend.

Behavior-preservation notes, each critic-verified against the shipped
code:
- Callers feed only currently-trusted references; taint (>= loss) and
  anchor (< loss) predicates are disjoint, so pre-sweep-view pick ==
  post-sweep-table pick on every input. Tie-break preserved (first
  entry wins == the strict '>' all three used, ascending slot order).
- plan_slot_recovery delegates its pick half to pick_anchor, which
  keeps both items live on every leg (pick_anchor's only external
  caller is Linux+vulkan-encode; dead_code is an item lint) and makes
  'anchor chosen from the taint snapshot' structural.
- The decline arms are deliberately NOT uniform and stay put: AMF/QSV
  clear an un-consumed pending_force; Vulkan leaves pending_loss armed
  (a stale arm re-resolves at frame-build into the healing IDR) — now
  pinned in comments on both sides.
- Vulkan's frame-build site is a pick-only re-run by design (the arm
  carries the loss start, not the slot); pick_recovery_slot is deleted
  and its tests migrated 1:1 into the shared module.

Test-count deltas are expected: the rfi tests now run on every Windows
combo (amf.rs is featureless there) and on the Linux feature leg.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
enricobuehler
2026-07-26 00:08:03 +02:00
co-authored by Claude Opus 5
parent d28fb1282b
commit 9f1e648e4e
5 changed files with 271 additions and 151 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/pf-encode/src/enc/linux/vulkan_video.rs
+39 -101
View File
@@ -363,17 +363,15 @@ impl Default for BsPtr {
}
}
/// Newest resident DPB slot whose wire index is strictly older than the loss — the clean anchor.
fn pick_recovery_slot(slot_wire: &[i64], loss_first: i64) -> Option<usize> {
let mut best: Option<usize> = None;
let mut best_wire = -1i64;
for (i, &w) in slot_wire.iter().enumerate() {
if w >= 0 && w < loss_first && w > best_wire {
best = Some(i);
best_wire = w;
}
}
best
/// The trusted-reference view of `slot_wire` for the shared RFI policy ([`crate::rfi`]): resident
/// slots only — `-1` marks empty *or distrusted* (blanked by an earlier loss's taint sweep), and
/// excluding it here is exact because the loss start is validity-gated non-negative.
fn trusted_refs(slot_wire: &[i64]) -> Vec<(usize, i64)> {
slot_wire
.iter()
.enumerate()
.filter_map(|(s, &w)| (w >= 0).then_some((s, w)))
.collect()
}
/// The S0 (past-reference) half of an HEVC short-term RPS that **retains every resident DPB
@@ -385,7 +383,8 @@ fn pick_recovery_slot(slot_wire: &[i64], loss_first: i64) -> Option<usize> {
/// with a generated gray reference and every following frame chains off the corruption — exactly
/// at the moment the anchor claims the picture is clean. Listing all residents (with
/// `used_by_curr_pic` set only for the real reference) keeps the host and client DPBs in lockstep,
/// so any slot [`pick_recovery_slot`] can pick is decodable by construction.
/// so any slot the RFI anchor pick ([`crate::rfi::pick_anchor`]) can pick is decodable by
/// construction.
///
/// `setup_idx` — the slot this frame reconstructs into — is excluded: its old occupant dies with
/// this frame on the host, so the decoder must drop it too (also keeping the retained count at
@@ -1950,8 +1949,11 @@ impl VulkanVideoEncoder {
let mut recovery = false;
if let Some(lf) = self.pending_loss.take() {
if !is_idr {
match pick_recovery_slot(&self.slot_wire, lf) {
Some(s) => {
// Pick-only re-run of the shared policy: the taint sweep already happened in
// `invalidate_ref_frames`; the arm carries the loss start, not the slot, so the
// anchor is resolved against the table as it stands NOW.
match crate::rfi::pick_anchor(&trusted_refs(&self.slot_wire), lf) {
Some((s, _)) => {
ref_slot = s;
recovery = true;
tracing::debug!(
@@ -3705,29 +3707,31 @@ impl Encoder for VulkanVideoEncoder {
if first_frame < 0 || first_frame > last_frame {
return false;
}
// Taint sweep BEFORE picking the anchor (the fecbec2d fix AMF and QSV got; this backend was
// carved out one commit later and never received it). "Resident and older than THIS loss" is
// not the same as "the client decoded it": after an earlier loss [a,b] was recovered at wire
// r, everything in [a, r-1] is undecodable at the client — the lost frames plus every frame
// that predicted through the gap. Those wires stay valid anchor candidates here until the
// 8-slot ring rolls them out, so a LATER loss can anchor on one and ship corruption tagged
// `recovery_anchor` — which is the client's definitive re-anchor signal (reanchor.rs), so it
// lifts the post-loss freeze onto a picture built from a reference it never had.
// The taint-sweep + anchor-pick POLICY lives in `rfi::plan_slot_recovery` (one decision
// shared with AMF and QSV — the fecbec2d sweep reached those two a commit before this
// backend was carved out, and the hand-copy here shipped without it). Why the sweep:
// "resident and older than THIS loss" is not the same as "the client decoded it" — after
// an earlier loss [a,b] was recovered at wire r, everything in [a, r-1] is undecodable at
// the client, yet those wires stay anchor candidates until the 8-slot ring rolls them
// out, so a LATER loss could anchor on one and ship corruption tagged `recovery_anchor` —
// the client's definitive re-anchor signal (reanchor.rs).
//
// Blank `slot_wire` ONLY. `slot_poc` must keep naming every physically-resident DPB picture
// for `build_h265_rps_s0`, or a conforming decoder evicts them and the anchor references a
// picture the client already dropped. `slot_wire` is the RFI/loss domain; `slot_poc` is the
// reference-delta domain. `prev_slot` and the normal P-frame path are indices, not wires, so
// ordinary prediction is unaffected.
for w in self.slot_wire.iter_mut() {
if *w >= first_frame {
// This backend's mechanism: distrust = blank `slot_wire` ONLY. `slot_poc` must keep
// naming every physically-resident DPB picture for `build_h265_rps_s0`, or a conforming
// decoder evicts them and the anchor references a picture the client already dropped.
// `slot_wire` is the RFI/loss domain; `slot_poc` is the reference-delta domain.
// `prev_slot` and the normal P-frame path are indices, not wires, so ordinary prediction
// is unaffected.
let plan = crate::rfi::plan_slot_recovery(&trusted_refs(&self.slot_wire), first_frame);
for (s, w) in self.slot_wire.iter_mut().enumerate() {
if plan.tainted & (1 << s) != 0 {
*w = -1;
}
}
// Can we anchor a clean P-frame to a resident slot strictly older than the loss?
// (A sweep that empties every candidate yields `None` here and declines the RFI, matching
// `qsv_live_ltr_rfi_taint_sweep_declines`.)
match pick_recovery_slot(&self.slot_wire, first_frame) {
match plan.anchor {
Some(_) => {
self.pending_loss = Some(first_frame);
true
@@ -3736,6 +3740,10 @@ impl Encoder for VulkanVideoEncoder {
// Decline WITHOUT self-arming an IDR: the caller owns the fallback, and its
// keyframe path is cooldown-coalesced — arming `force_kf` here would bypass that
// and turn a storm of hopeless RFI requests into one full IDR per request.
// `pending_loss` is DELIBERATELY left armed (unlike AMF/QSV's pending_force
// clear): a stale arm is re-resolved at frame-build, where a failed re-pick
// forces the IDR that heals the stream — clearing it here would ship an untagged
// plain P during the caller's RFI-echo window instead.
tracing::debug!(
first_frame,
last_frame,
@@ -4855,80 +4863,10 @@ unsafe fn build_parameters_av1(
#[cfg(test)]
mod tests {
use super::{build_h265_rps_s0, parse_rgb_request, pick_recovery_slot, VulkanVideoEncoder};
use super::{build_h265_rps_s0, parse_rgb_request, VulkanVideoEncoder};
use crate::{Codec, Encoder};
use pf_frame::{CapturedFrame, FramePayload, PixelFormat};
/// The RFI anchor picker: newest resident wire strictly older than the loss; empty/newer
/// slots never qualify.
#[test]
fn recovery_slot_picks_newest_pre_loss() {
// slots hold wires 5..12 (ring position arbitrary); loss starts at 9 → anchor = wire 8.
let wires = [8i64, 9, 10, 11, 12, 5, 6, 7];
assert_eq!(pick_recovery_slot(&wires, 9), Some(0));
// loss older than everything resident → no anchor (caller keyframes).
assert_eq!(pick_recovery_slot(&wires, 5), None);
// empty slots (-1) are skipped.
assert_eq!(pick_recovery_slot(&[-1, 3, -1, 4], 5), Some(3));
assert_eq!(pick_recovery_slot(&[-1; 8], 5), None);
}
/// The taint sweep (fecbec2d's fix, ported here): a slot encoded inside an EARLIER, still
/// unrepaired loss window must not become the "known-good" anchor of a LATER loss. Without the
/// sweep, `pick_recovery_slot` accepts it — it is resident and its wire is below the second
/// loss start — and the frame ships tagged `recovery_anchor`, lifting the client's freeze onto
/// a reference it never decoded.
#[test]
fn taint_sweep_excludes_slots_from_an_earlier_loss() {
// Apply the sweep exactly as `invalidate_ref_frames` does.
fn sweep(wires: &mut [i64], loss_first: i64) {
for w in wires.iter_mut() {
if *w >= loss_first {
*w = -1;
}
}
}
// Slots hold wires 0..7. Loss 1 starts at wire 4, so wires 4..7 are undecodable at the
// client. A second loss report arrives at wire 6 while they are all still resident.
let tainted = [4i64, 5, 6, 7];
// WITHOUT the sweep this is the bug: the newest wire below 6 is wire 5 — squarely inside
// loss 1's unrepaired window — and it would be served as the "known-good" anchor.
let unswept = [0i64, 1, 2, 3, 4, 5, 6, 7];
let picked = pick_recovery_slot(&unswept, 6).expect("unswept picks something");
assert!(
tainted.contains(&unswept[picked]),
"precondition: without the sweep the anchor comes from the earlier loss window"
);
// WITH the sweep, loss 1 blanks 4..7, so loss 2 can only reach genuinely clean wires.
let mut wires = unswept;
sweep(&mut wires, 4);
assert_eq!(wires, [0, 1, 2, 3, -1, -1, -1, -1]);
let picked = pick_recovery_slot(&wires, 6).expect("clean wires remain");
assert_eq!(picked, 3, "newest clean survivor is wire 3");
assert!(!tainted.contains(&wires[picked]));
// Encoding resumes after recovery; wires 8..11 refill the swept slots and are clean. A
// later loss at wire 10 legitimately anchors on wire 9 — the sweep must not over-reject.
wires[4] = 8;
wires[5] = 9;
wires[6] = 10;
wires[7] = 11;
sweep(&mut wires, 10);
assert_eq!(
pick_recovery_slot(&wires, 10),
Some(5),
"wire 9 is post-recovery, clean"
);
// A loss covering every live wire leaves nothing clean → decline, caller serves an IDR.
let mut all = [5i64, 6, 7, 8, 9, 10, 11, 12];
sweep(&mut all, 5);
assert_eq!(pick_recovery_slot(&all, 5), None);
}
/// The full-retention RPS: every resident picture is listed (so the decoder keeps it), the
/// setup slot's dying occupant is not, and `used_by_curr_pic` marks exactly the real reference.
#[test]