diff --git a/crates/pf-bitstream/src/av1.rs b/crates/pf-bitstream/src/av1.rs index 2a59d881..7dee0185 100644 --- a/crates/pf-bitstream/src/av1.rs +++ b/crates/pf-bitstream/src/av1.rs @@ -277,6 +277,19 @@ pub struct PicturePlan { /// Colour signalling, per picture and never latched — the same rule the other two /// planners follow, because a host can switch an HDR desktop to PQ/BT.2020 in band. pub colour: ColourDescription, + /// Every picture this frame predicts from was itself decoded from a fully-available + /// reference chain — so a host claim that this frame is a clean re-anchor + /// (`USER_FLAG_RECOVERY_ANCHOR`) can be corroborated rather than taken on trust. + /// `true` for a key or intra-only frame (nothing to predict from) and for any + /// frame whose whole reference chain is clean; `false` from the moment this frame + /// — or anything it descends from — needed concealment. + /// + /// On a `show_existing_frame` this describes the picture being DISPLAYED, which is + /// the only thing such a frame puts on the screen: it decodes nothing of its own. + /// + /// Purely additive observation. See [`crate::clean`] for why it propagates and why + /// every rule errs toward `false`. + pub references_clean: bool, } /// One planned access unit. @@ -329,6 +342,35 @@ pub enum PlanWarning { TruncatedAu { offset: usize }, } +impl PlanWarning { + /// Does this warning mean the PICTURE is damaged? The AV1 twin of + /// [`crate::h264::PlanWarning::is_integrity`], and + /// `pf_vkdecode::is_integrity_warning_av1` delegates here. + /// + /// Every variant AV1 has IS damage, and that is a fact about the codec rather than + /// an oversight: AV1 puts nothing in this channel resembling h265's + /// `NonZeroReorder` or h264's `Mmco5Rebase`. It has no reorder envelope to report + /// (no bumping process, no `max_num_reorder_pics`) and no MMCO to rebase — the + /// frame header states the whole reference update outright — so the only things + /// left to warn about are pictures that went missing and an OBU walk that stopped + /// early. + /// + /// `MissingShowExisting` is the one that could be argued, and it is damage: a + /// `show_existing_frame` naming an empty slot means the picture the STREAM chose + /// to display was lost upstream. Nothing is displayed for that frame, so the + /// screen keeps the previous one — exactly the "silently stale picture" state a + /// re-anchor exists to end. + /// + /// Exhaustive with no wildcard, for the reason the H.264 twin spells out. + pub fn is_integrity(&self) -> bool { + match self { + PlanWarning::MissingReference { .. } + | PlanWarning::MissingShowExisting { .. } + | PlanWarning::TruncatedAu { .. } => true, + } + } +} + /// Why an access unit cannot be planned at all. #[derive(Debug, Clone, PartialEq, Eq)] pub enum PlanError { @@ -366,6 +408,10 @@ pub struct Av1Planner { slots: [Option; NUM_REF_SLOTS], next_id: PicId, sequence: Option>, + /// Which resident pictures came off a BROKEN reference chain — the fact behind + /// [`PicturePlan::references_clean`]. Empty on a healthy stream; see + /// [`crate::clean::CleanLedger`] for the propagation rules. + clean: crate::clean::CleanLedger, } impl Default for Av1Planner { @@ -381,6 +427,7 @@ impl Av1Planner { slots: [None; NUM_REF_SLOTS], next_id: 1, sequence: None, + clean: Default::default(), } } @@ -549,7 +596,22 @@ impl Av1Planner { } else { Vec::new() }; - let picture = picture_plan(&header, &sequence); + // A `show_existing_frame` decodes nothing, so the only picture it puts on + // the screen is the one it displays: report THAT picture's cleanliness. A + // slot that held nothing already warned above and shows nothing at all, + // which is damage in its own right — reporting it unclean keeps the two + // statements consistent. + let references_clean = match shown { + Some(pic) => self.clean.references_clean([pic.id]), + None => false, + }; + let picture = picture_plan(&header, &sequence, references_clean); + // A key-frame `show_existing_frame` rewrote every slot with the shown + // picture (7.20), so the ledger has to follow that aliasing: the refreshed + // slots all hold `pic.id`, whose mark already stands. Nothing new is + // stored, so there is no verdict to fold — only residency to re-bound. + self.clean + .retain_live(self.slots.iter().flatten().map(|p| p.id)); return Ok(AuPlan { picture, tiles, @@ -597,12 +659,34 @@ impl Av1Planner { } let removed = self.refresh_slots(header.refresh_frame_flags, id, RefState::of(&header)); - let picture = picture_plan(&header, &sequence); + // Was every picture this frame predicts from decoded off an intact chain? + // Over the resolved names only: a `None` hole is a lost reference, which has + // already pushed `MissingReference` and therefore condemns this frame through + // `concealed` below. A key or intra-only frame names nothing, so this is + // vacuously true for it (`CleanLedger::references_clean`). + let references_clean = self + .clean + .references_clean(refs.iter().flatten().map(|r| r.id)); + + let picture = picture_plan(&header, &sequence, references_clean); let outputs = if header.show_frame { vec![id] } else { Vec::new() }; + // Fold this frame's verdict, then bound the ledger to slot residency. After + // `refresh_slots`, so the live set reflects the writes this frame performed. + // `concealed` mirrors what a consumer conceals on, via the ONE classification + // (`PlanWarning::is_integrity`), so the ledger and the consumer can never + // disagree about whether this frame was damaged. + self.clean.note_stored( + id, + references_clean, + warnings.iter().any(PlanWarning::is_integrity), + ); + self.clean + .retain_live(self.slots.iter().flatten().map(|p| p.id)); + Ok(AuPlan { picture, tiles, @@ -655,7 +739,11 @@ impl Av1Planner { } } -fn picture_plan(header: &FrameHeaderObu, sequence: &SequenceHeaderObu) -> PicturePlan { +fn picture_plan( + header: &FrameHeaderObu, + sequence: &SequenceHeaderObu, + references_clean: bool, +) -> PicturePlan { let color = &sequence.color_config; let bit_depth = if color.high_bitdepth { if color.twelve_bit { @@ -698,6 +786,7 @@ fn picture_plan(header: &FrameHeaderObu, sequence: &SequenceHeaderObu) -> Pictur matrix_coefficients: color.matrix_coefficients as u8, video_full_range: color.color_range, }, + references_clean, } } #[cfg(test)] diff --git a/crates/pf-bitstream/src/clean.rs b/crates/pf-bitstream/src/clean.rs new file mode 100644 index 00000000..f0ffb64b --- /dev/null +++ b/crates/pf-bitstream/src/clean.rs @@ -0,0 +1,245 @@ +//! Which decoded pictures came off a fully-available reference chain — the fact a +//! client needs to CORROBORATE a host's claim that a frame is a clean re-anchor. +//! +//! # Why this exists +//! +//! After a loss the client freezes on its last good picture and lifts only on a proven +//! re-anchor. One of those proofs — `USER_FLAG_RECOVERY_ANCHOR`, the host's LTR-RFI +//! recovery frame — lifts the freeze on its FIRST occurrence, exactly like a real IDR, +//! because the host says the frame was coded against a known-good reference. +//! +//! The host's "known-good" is an inference from what the client RECEIVED. The client's +//! own DPB is the only place that knows what it actually DECODED, and it did not +//! previously record it: a picture the planner concealed (a reference the DPB could not +//! resolve, an AU that stopped early) entered the DPB looking exactly like a clean one. +//! So an anchor naming that picture lifted the freeze onto a gray plate, and every +//! frame after it chained off the corruption — the freeze gone, nothing left to +//! re-arm it, and the picture stayed broken until an unrelated signal forced an IDR. +//! +//! This ledger is the missing fact, and it is deliberately the SMALLEST one that +//! answers the question: a set of picture ids that are NOT clean. Membership is +//! per-picture, so it costs one `u64` per damaged picture and nothing at all on a +//! healthy stream — the overwhelmingly common case, where the set stays empty for the +//! life of the session. +//! +//! # Damage propagates; that is the whole point +//! +//! A picture is unclean when the AU that produced it needed concealment, OR when +//! ANYTHING it predicted from was unclean. Without the second half the ledger would be +//! useless: the concealed picture itself is rarely the one an anchor names — it is the +//! chain of ordinary P-frames DESCENDING from it, each of which planned perfectly and +//! raised no warning of its own, that carries the corruption forward. +//! +//! # It errs toward "unclean", never toward "clean" +//! +//! Every rule here is one-way. An id the ledger has forgotten (evicted from the DPB, +//! dropped at a flush) reads as clean, which is correct — a picture no longer in the +//! DPB cannot be referenced. An id it holds stays unclean until the picture leaves the +//! DPB. There is no path that clears the mark on a picture that is still resident, so +//! the ledger can only ever make a consumer MORE conservative: hold the freeze longer +//! and take an IDR it might not have needed. The opposite mistake — reporting a damaged +//! chain as clean — is the failure this exists to end, so the asymmetry is deliberate. + +use std::collections::BTreeSet; + +/// Per-picture "this came off a broken chain" marks for one planner. +/// +/// Keyed by the planner's own `PicId` (a `u64` in all three codecs), so this type is +/// codec-agnostic and the H.264, H.265 and AV1 planners share ONE implementation rather +/// than three hand-copies that can drift apart. +#[derive(Debug, Clone, Default)] +pub struct CleanLedger { + /// Ids of resident pictures that are NOT clean. Empty on a healthy stream — the + /// set only ever gains an entry when a plan needed concealment. + unclean: BTreeSet, +} + +impl CleanLedger { + pub fn new() -> Self { + Self::default() + } + + /// Is every id in `references` clean? — i.e. may a picture predicted from exactly + /// these be trusted? + /// + /// Vacuously true for an empty list, which is what makes an IRAP/IDR clean by + /// construction: it predicts from nothing, so there is nothing to distrust. + pub fn references_clean(&self, references: I) -> bool + where + I: IntoIterator, + { + // Short-circuits on the first unclean reference, and — because the set is + // empty on a healthy stream — degenerates to one `is_empty`-cheap lookup per + // reference in the case that matters for throughput. + self.unclean.is_empty() || !references.into_iter().any(|id| self.unclean.contains(&id)) + } + + /// Record the verdict for the picture this AU stored. + /// + /// `references_clean` is what [`Self::references_clean`] answered for this AU's + /// reference lists; `concealed` is whether the AU's own plan carried an integrity + /// warning. Either one being bad makes the stored picture unclean, and its + /// descendants inherit that through their own `references_clean` call. + pub fn note_stored(&mut self, id: u64, references_clean: bool, concealed: bool) { + if references_clean && !concealed { + // The common path. Nothing is inserted, so a healthy stream never allocates + // — and `remove` still runs below because an id can be REUSED after the + // planner recycles it, and a stale mark would then condemn a fresh picture. + self.unclean.remove(&id); + } else { + self.unclean.insert(id); + } + } + + /// Drop the marks of pictures that have left the DPB. + /// + /// Called with the ids still live after each plan. Bounding the set to DPB + /// residency is what keeps it from growing without limit across a long lossy + /// session, and it is safe precisely because a picture outside the DPB can never + /// appear in a later reference list. + pub fn retain_live(&mut self, live: I) + where + I: IntoIterator, + { + if self.unclean.is_empty() { + return; + } + let live: BTreeSet = live.into_iter().collect(); + self.unclean.retain(|id| live.contains(id)); + } + + /// Forget everything — the DPB was drained (a flush, a stream discontinuity), so no + /// mark describes a resident picture any more. + pub fn clear(&mut self) { + self.unclean.clear(); + } + + /// Is this picture known to have come off a broken chain? (Diagnostics and tests; + /// the plan path uses [`Self::references_clean`].) + pub fn is_unclean(&self, id: u64) -> bool { + self.unclean.contains(&id) + } + + /// How many resident pictures are marked unclean (diagnostics and tests). + pub fn unclean_count(&self) -> usize { + self.unclean.len() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The headline: damage propagates down the prediction chain. The concealed + /// picture is rarely the one an anchor names — it is the ordinary P-frames + /// descending from it, each of which planned perfectly and warned about nothing. + #[test] + fn damage_propagates_to_every_descendant() { + let mut led = CleanLedger::new(); + // An IDR: no references, no concealment. + assert!(led.references_clean([])); + led.note_stored(0, true, false); + assert!(!led.is_unclean(0)); + + // A clean P off it. + assert!(led.references_clean([0])); + led.note_stored(1, true, false); + + // Picture 2's plan needed concealment. + let refs_clean = led.references_clean([1]); + assert!(refs_clean, "its reference was still fine"); + led.note_stored(2, refs_clean, true); + assert!(led.is_unclean(2)); + + // …and picture 3 predicts from it, raising NO warning of its own. + let refs_clean = led.references_clean([2]); + assert!(!refs_clean, "the chain is broken from here down"); + led.note_stored(3, refs_clean, false); + assert!(led.is_unclean(3), "3 inherited 2's damage"); + + // The rot keeps travelling, arbitrarily far from the original loss. + let refs_clean = led.references_clean([3]); + assert!(!refs_clean); + led.note_stored(4, refs_clean, false); + assert!(led.is_unclean(4)); + } + + /// A picture that references BOTH a clean and an unclean predecessor is unclean — + /// one broken reference is enough to make the reconstruction wrong. + #[test] + fn one_unclean_reference_is_enough() { + let mut led = CleanLedger::new(); + led.note_stored(0, true, false); + led.note_stored(1, true, true); // damaged + assert!(!led.references_clean([0, 1])); + assert!(!led.references_clean([1, 0]), "order does not matter"); + assert!(led.references_clean([0])); + } + + /// An IDR predicts from nothing, so it is clean however broken the stream was + /// before it. This is the property that lets a real keyframe end a damaged run. + #[test] + fn a_picture_with_no_references_is_clean_however_bad_the_stream_was() { + let mut led = CleanLedger::new(); + led.note_stored(0, true, true); + led.note_stored(1, false, false); + assert_eq!(led.unclean_count(), 2); + // The IDR: an empty reference list is vacuously clean. + assert!(led.references_clean([])); + led.note_stored(2, true, false); + assert!(!led.is_unclean(2)); + } + + /// Marks are bounded by DPB residency: a picture that left the DPB can never be + /// referenced again, so keeping its mark would only grow the set forever. + #[test] + fn marks_are_dropped_when_their_picture_leaves_the_dpb() { + let mut led = CleanLedger::new(); + led.note_stored(7, true, true); + led.note_stored(8, false, false); + assert_eq!(led.unclean_count(), 2); + led.retain_live([8, 9]); + assert!(!led.is_unclean(7), "7 was evicted"); + assert!(led.is_unclean(8), "8 is still resident and still damaged"); + assert_eq!(led.unclean_count(), 1); + } + + /// A flush drains the whole DPB, so no mark describes anything resident. + #[test] + fn a_flush_forgets_every_mark() { + let mut led = CleanLedger::new(); + led.note_stored(1, true, true); + led.note_stored(2, false, false); + led.clear(); + assert_eq!(led.unclean_count(), 0); + assert!(led.references_clean([1, 2])); + } + + /// Planners hand out ids from a counter the flush path can rewind, so an id CAN be + /// reused. A stale mark must not condemn the fresh picture that inherits the id. + #[test] + fn a_reused_id_is_not_condemned_by_its_predecessors_mark() { + let mut led = CleanLedger::new(); + led.note_stored(5, true, true); + assert!(led.is_unclean(5)); + // The same id, planned cleanly this time. + led.note_stored(5, true, false); + assert!(!led.is_unclean(5)); + assert!(led.references_clean([5])); + } + + /// A healthy stream never marks anything, forever — the property that makes this + /// free to carry on every session that is working correctly. + #[test] + fn a_stream_without_loss_never_marks_a_picture() { + let mut led = CleanLedger::new(); + for id in 0..512u64 { + let refs = if id == 0 { vec![] } else { vec![id - 1] }; + let clean = led.references_clean(refs.iter().copied()); + assert!(clean, "picture {id} must read clean"); + led.note_stored(id, clean, false); + led.retain_live(id.saturating_sub(3)..=id); + } + assert_eq!(led.unclean_count(), 0); + } +} diff --git a/crates/pf-bitstream/src/h264.rs b/crates/pf-bitstream/src/h264.rs index 146555c9..646865de 100644 --- a/crates/pf-bitstream/src/h264.rs +++ b/crates/pf-bitstream/src/h264.rs @@ -149,6 +149,18 @@ pub struct PicturePlan { /// DPB size in frames per A.3.1 — backends size their slot pool from this. pub max_dpb_frames: usize, pub recovery_point: Option, + /// Every picture this AU predicts from was itself decoded from a fully-available + /// reference chain — so a host claim that this AU is a clean re-anchor + /// (`USER_FLAG_RECOVERY_ANCHOR`) can be corroborated rather than taken on trust. + /// `true` for an IDR (nothing to predict from) and for any picture whose whole + /// reference chain is clean; `false` from the moment this AU — or anything it + /// descends from — needed concealment. + /// + /// Purely additive observation: nothing in the plan, the warnings or the DPB + /// changes because of it, and on a stream that never loses a reference it is + /// `true` on every picture forever. See [`crate::clean`] for why it propagates and + /// why every rule errs toward `false`. + pub references_clean: bool, } /// The region of the coded picture that is actually displayed. @@ -252,6 +264,41 @@ pub enum PlanWarning { }, } +impl PlanWarning { + /// Does this warning mean the PICTURE is damaged — the plan was completed with a + /// SUBSTITUTE in place of something that was lost — rather than reporting a + /// spec-legal fact about the stream's envelope? + /// + /// The distinction decides two things that must never disagree: whether a consumer + /// releases the AU's output unshown and asks for a re-anchor, and whether the + /// picture enters [`crate::clean::CleanLedger`] as unclean. It lives HERE, on the + /// enum, because those two consumers sit in different crates and a second copy of + /// the list would let one of them conceal damage the other reports — the exact + /// shape of the invisible-corruption failure the native-decode program exists to + /// end. `pf_vkdecode::is_integrity_warning` delegates to this. + /// + /// `Mmco5Rebase` is not damage: the AU carried an MMCO 5 and this planner planned + /// it in full (the plan holds the pre-rebase 8.2.1 values; later AUs reference the + /// rebased ones). `LevelDerivedDpb` is not either: the picture is intact and fully + /// planned — it reports that the SPS never declared its DPB depth, so the plan had + /// to size from A.3.1's level ceiling, a property of the STREAM's signalling which + /// a backend answers by failing to open a session, not by showing a damaged frame. + /// + /// Written as an EXHAUSTIVE match with no wildcard, deliberately. A `matches!` (or + /// a `_ => false`) makes "damage" the opt-in and silence the default, so a variant + /// added later — by definition one nobody here has classified — would be reported + /// as clean and its picture shown. The compiler is the only reviewer guaranteed to + /// be present when that variant is written, so it gets the decision. + pub fn is_integrity(&self) -> bool { + match self { + PlanWarning::FrameNumGap { .. } + | PlanWarning::MissingReference { .. } + | PlanWarning::TruncatedAu { .. } => true, + PlanWarning::Mmco5Rebase | PlanWarning::LevelDerivedDpb { .. } => false, + } + } +} + /// The AU cannot be planned at all. #[derive(Debug, Clone, PartialEq, Eq)] pub enum PlanError { @@ -482,6 +529,10 @@ pub struct H264Planner { reported_live: BTreeSet, /// Set by [`Self::flush`]: planning resumes only at an IDR (upstream: `Reset`). awaiting_idr: bool, + /// Which resident pictures came off a BROKEN reference chain — the fact behind + /// [`PicturePlan::references_clean`]. Empty on a healthy stream; see + /// [`crate::clean::CleanLedger`] for the propagation rules. + clean: crate::clean::CleanLedger, } impl H264Planner { @@ -600,9 +651,20 @@ impl H264Planner { let cur = current .ok_or_else(|| PlanError::Parse("access unit contains no coded picture".into()))?; + // Was every picture this AU predicts from decoded off an intact chain? Asked + // over the SLICE reference lists rather than the DPB snapshot, because those + // are what this picture actually predicts from — a resident-but-unreferenced + // damaged picture says nothing about this one. An IDR references nothing, so + // this is vacuously true for it (`CleanLedger::references_clean`). + let references_clean = self.clean.references_clean( + slices + .iter() + .flat_map(|s: &SlicePlan| s.ref_list0.iter().chain(&s.ref_list1)) + .map(|r| r.id), + ); // Captured before finish_picture: MMCO5 rewrites the stored POC afterwards, but // backends submit the picture with its 8.2.1 values. - let picture = Self::picture_plan(&cur, recovery_point); + let picture = Self::picture_plan(&cur, recovery_point, references_clean); // The activated parameter sets ride out with the plan (AuPlan field docs); // cloned before finish_picture consumes `cur`. let pps = Rc::clone(&cur.first_slice_pps); @@ -619,6 +681,20 @@ impl H264Planner { let removed = previously_live.difference(&live_after).copied().collect(); self.reported_live = live_after; + // Fold this picture's verdict, then bound the ledger to DPB residency. Both + // AFTER `finish_picture`, so `stored` is the id the picture really got and + // `live_after` reflects the marking this AU performed — a mark written against + // a pre-marking view could survive an eviction it should have died with. + // `concealed` mirrors what a consumer conceals on, via the ONE classification + // (`PlanWarning::is_integrity`), so the ledger and the consumer can never + // disagree about whether this AU was damaged. + self.clean.note_stored( + stored, + references_clean, + warnings.iter().any(PlanWarning::is_integrity), + ); + self.clean.retain_live(self.reported_live.iter().copied()); + Ok(AuPlan { picture, slices, @@ -650,6 +726,9 @@ impl H264Planner { self.max_long_term_frame_idx = Default::default(); self.negotiation_info = Default::default(); self.awaiting_idr = true; + // The DPB is drained, so no mark describes a resident picture any more — and + // planning resumes at an IDR, which is clean by construction. + self.clean.clear(); DpbUpdate { stored: None, @@ -1747,7 +1826,11 @@ impl H264Planner { Ok(id) } - fn picture_plan(cur: &CurrentPicState, recovery_point: Option) -> PicturePlan { + fn picture_plan( + cur: &CurrentPicState, + recovery_point: Option, + references_clean: bool, + ) -> PicturePlan { let pic = &cur.pic; // The first slice's PPS defines the picture's parameters (upstream's // start_picture semantics); `cur.pps` may have drifted to a later slice's. @@ -1791,6 +1874,7 @@ impl H264Planner { chroma_format_idc: sps.chroma_format_idc, max_dpb_frames: dpb_limit(sps), recovery_point, + references_clean, } } } @@ -2343,6 +2427,110 @@ mod tests { assert!(missing_seen); } + /// The clean bit, end to end through the real planner: a `frame_num` gap + /// concealed one picture, and EVERY picture descending from it reports + /// `references_clean == false` even though their own plans are spotless. That + /// propagation is the whole point — the concealed picture is rarely the one a host + /// recovery anchor names; the ordinary P-frames after it are. + #[test] + fn a_concealed_picture_makes_every_descendant_report_unclean_references() { + let (sps, pps) = authored_sps_pps(); + let mut au0 = param_set_au(&sps, &pps); + au0.extend(write_idr_slice()); + + let mut planner = H264Planner::new(); + let p0 = planner.plan_au(&au0).unwrap(); + assert!( + p0.picture.references_clean, + "an IDR references nothing, so it is clean by construction" + ); + + // A healthy P off the IDR: still clean. + let p1 = planner.plan_au(&write_p_slice(1, 2, 1, 1, None)).unwrap(); + assert!(picture_warnings(&p1).is_empty()); + assert!(p1.picture.references_clean); + + // frame_num 2 never arrives — 8.2.5.2 fabricates a placeholder and the plan + // conceals. THIS picture's references were still intact; the damage is its own. + let p3 = planner.plan_au(&write_p_slice(3, 6, 1, 3, None)).unwrap(); + assert!(p3 + .warnings + .iter() + .any(|w| matches!(w, PlanWarning::FrameNumGap { .. }))); + + // …and every picture after it inherits the damage with a clean plan of its own. + let p4 = planner.plan_au(&write_p_slice(4, 8, 1, 1, None)).unwrap(); + assert!( + picture_warnings(&p4).is_empty(), + "p4's own plan raises nothing — which is exactly why the bit is needed" + ); + assert!( + !p4.picture.references_clean, + "p4 predicts from the concealed chain, so it must not read as clean" + ); + + let p5 = planner.plan_au(&write_p_slice(5, 10, 1, 1, None)).unwrap(); + assert!(picture_warnings(&p5).is_empty()); + assert!(!p5.picture.references_clean, "the rot keeps travelling"); + } + + /// An IDR ends a damaged run: it predicts from nothing, so it reads clean however + /// broken the stream was before it. Without this a session could never recover a + /// trustworthy anchor. + #[test] + fn an_idr_reports_clean_references_however_damaged_the_run_before_it() { + let (sps, pps) = authored_sps_pps(); + let mut au0 = param_set_au(&sps, &pps); + au0.extend(write_idr_slice()); + + let mut planner = H264Planner::new(); + planner.plan_au(&au0).unwrap(); + planner.plan_au(&write_p_slice(1, 2, 1, 1, None)).unwrap(); + // Gap: frame_num 2 lost. + let p3 = planner.plan_au(&write_p_slice(3, 6, 1, 3, None)).unwrap(); + assert!(p3 + .warnings + .iter() + .any(|w| matches!(w, PlanWarning::FrameNumGap { .. }))); + let p4 = planner.plan_au(&write_p_slice(4, 8, 1, 1, None)).unwrap(); + assert!(!p4.picture.references_clean); + + // A fresh IDR re-anchors, and the pictures after it are clean again. + let mut idr = param_set_au(&sps, &pps); + idr.extend(write_idr_slice()); + let p5 = planner.plan_au(&idr).unwrap(); + assert!(p5.picture.references_clean, "an IDR is always clean"); + let p6 = planner.plan_au(&write_p_slice(1, 2, 1, 1, None)).unwrap(); + assert!( + p6.picture.references_clean, + "the damaged chain died with the IDR's DPB flush" + ); + } + + /// A stream that never loses a reference reports `references_clean` on every + /// picture, forever — the property that makes this free to carry in production. + #[test] + fn a_healthy_stream_reports_clean_references_on_every_picture() { + let (sps, pps) = authored_sps_pps(); + let mut au0 = param_set_au(&sps, &pps); + au0.extend(write_idr_slice()); + + let mut planner = H264Planner::new(); + assert!(planner.plan_au(&au0).unwrap().picture.references_clean); + // log2_max_frame_num_minus4 = 0 and pic_order_cnt_lsb is u(4): both wrap at 16. + for n in 1..16u32 { + let plan = planner + .plan_au(&write_p_slice(n, (n * 2) % 16, 1, 1, None)) + .unwrap(); + assert!( + picture_warnings(&plan).is_empty(), + "frame {n} should plan cleanly: {:?}", + picture_warnings(&plan) + ); + assert!(plan.picture.references_clean, "frame {n} must read clean"); + } + } + #[test] fn a_gap_placeholder_inside_a_ref_list_is_substituted_in_place_not_compacted() { let (sps, pps) = authored_sps_pps(); diff --git a/crates/pf-bitstream/src/h265.rs b/crates/pf-bitstream/src/h265.rs index 49774d13..68da10ca 100644 --- a/crates/pf-bitstream/src/h265.rs +++ b/crates/pf-bitstream/src/h265.rs @@ -166,6 +166,18 @@ pub struct PicturePlan { /// came from the SPS by index) — Vulkan's `NumBitsForSTRefPicSetInSlice`. pub short_term_ref_pic_set_size_bits: u32, pub recovery_point: Option, + /// Every picture this AU predicts from was itself decoded from a fully-available + /// reference chain — so a host claim that this AU is a clean re-anchor + /// (`USER_FLAG_RECOVERY_ANCHOR`) can be corroborated rather than taken on trust. + /// `true` for an IRAP (nothing to predict from) and for any picture whose whole + /// reference chain is clean; `false` from the moment this AU — or anything it + /// descends from — needed concealment. + /// + /// Purely additive observation: nothing in the plan, the warnings or the DPB + /// changes because of it, and on a stream that never loses a reference it is + /// `true` on every picture forever. See [`crate::clean`] for why it propagates and + /// why every rule errs toward `false`. + pub references_clean: bool, } /// A reference list / RPS entry: the minimum every backend picparams format needs. @@ -232,6 +244,28 @@ pub enum PlanWarning { NonZeroReorder { max_num_reorder_pics: u8 }, } +impl PlanWarning { + /// Does this warning mean the PICTURE is damaged? The H.265 twin of + /// [`crate::h264::PlanWarning::is_integrity`] — the same one-list argument applies, + /// and `pf_vkdecode::is_integrity_warning_h265` delegates here. + /// + /// `NonZeroReorder` is NOT damage, and excluding it matters more here than the + /// H.264 exclusions do: it fires on the AU that ACTIVATES an SPS — the opening + /// IRAP, and the fresh IRAP at every ABR resolution change — so treating it as + /// concealment would cost a released-unshown frame plus a keyframe round trip at + /// every renegotiation, on a stream the planner says it planned correctly. It + /// would also poison the [`crate::clean::CleanLedger`] at exactly those IRAPs, + /// marking the one picture that is clean by construction as broken. + /// + /// Exhaustive with no wildcard, for the reason the H.264 twin spells out. + pub fn is_integrity(&self) -> bool { + match self { + PlanWarning::MissingReference { .. } | PlanWarning::TruncatedAu { .. } => true, + PlanWarning::NonZeroReorder { .. } => false, + } + } +} + /// The AU cannot be planned at all. #[derive(Debug, Clone, PartialEq, Eq)] pub enum PlanError { @@ -452,6 +486,10 @@ pub struct H265Planner { reported_live: BTreeSet, /// Set by [`Self::flush`]: planning resumes only at an IRAP (upstream: `Reset`). awaiting_idr: bool, + /// Which resident pictures came off a BROKEN reference chain — the fact behind + /// [`PicturePlan::references_clean`]. Empty on a healthy stream; see + /// [`crate::clean::CleanLedger`] for the propagation rules. + clean: crate::clean::CleanLedger, } impl Default for H265Planner { @@ -471,6 +509,7 @@ impl Default for H265Planner { pending_outputs: Vec::new(), reported_live: BTreeSet::new(), awaiting_idr: false, + clean: Default::default(), } } } @@ -690,7 +729,20 @@ impl H265Planner { let cur = current .ok_or_else(|| PlanError::Parse("access unit contains no coded picture".into()))?; - let picture = Self::picture_plan(&cur, recovery_point); + // Was every picture this AU predicts from decoded off an intact chain? Asked + // over the SLICE reference lists rather than the RPS or the DPB snapshot, + // because those are what this picture actually predicts from: 8.3.2 RETAINS + // pictures in the RPS that the current picture does not use + // (`used_by_curr_pic` clear), and a damaged one among those says nothing about + // this picture. An IRAP's lists are empty, so this is vacuously true for it + // (`CleanLedger::references_clean`). + let references_clean = self.clean.references_clean( + slices + .iter() + .flat_map(|s: &SlicePlan| s.ref_list0.iter().chain(&s.ref_list1)) + .map(|r| r.id), + ); + let picture = Self::picture_plan(&cur, recovery_point, references_clean); let rps = cur.rps_plan.clone(); let dpb_refs = cur.dpb_refs.clone(); // The activated parameter sets ride out with the plan (AuPlan field docs); @@ -708,6 +760,20 @@ impl H265Planner { let removed = previously_live.difference(&live_after).copied().collect(); self.reported_live = live_after; + // Fold this picture's verdict, then bound the ledger to DPB residency. Both + // AFTER `finish_picture`, so `stored` is the id the picture really got and + // `live_after` reflects the C.3.4/8.3.2 marking this AU performed — a mark + // written against a pre-marking view could survive an eviction it should have + // died with. `concealed` mirrors what a consumer conceals on, via the ONE + // classification (`PlanWarning::is_integrity`), so the ledger and the consumer + // can never disagree about whether this AU was damaged. + self.clean.note_stored( + stored, + references_clean, + warnings.iter().any(PlanWarning::is_integrity), + ); + self.clean.retain_live(self.reported_live.iter().copied()); + Ok(AuPlan { picture, rps, @@ -745,6 +811,9 @@ impl H265Planner { // re-entry sound. self.first_picture_after_eos = true; self.awaiting_idr = true; + // The DPB is drained, so no mark describes a resident picture any more — and + // planning resumes at an IRAP, which is clean by construction. + self.clean.clear(); DpbUpdate { stored: None, @@ -1452,6 +1521,7 @@ impl H265Planner { fn picture_plan( cur: &CurrentPicState, recovery_point: Option, + references_clean: bool, ) -> PicturePlan { let pic = &cur.pic; // The first slice's PPS defines the picture's parameters; `cur.pps` may have @@ -1498,6 +1568,7 @@ impl H265Planner { max_dpb_frames: dpb_limit(sps), short_term_ref_pic_set_size_bits: pic.short_term_ref_pic_set_size_bits, recovery_point, + references_clean, } } } @@ -1550,11 +1621,15 @@ mod tests { /// `NonZeroReorder` is excluded: the vendored conformance clips are general /// (reordering) encodes, and the planner deliberately plans them while flagging /// the envelope fact. + /// + /// Delegates rather than restating the list. This harness exists to prove the + /// planner conceals exactly where production conceals, so a second copy here + /// could drift and quietly prove the wrong thing — and a `matches!` in + /// particular reads any FUTURE variant as clean, which is the one answer a + /// damage predicate must never default to. [`PlanWarning::is_integrity`] is an + /// exhaustive match, so a new variant stops the compiler there instead. fn is_integrity_warning(w: &PlanWarning) -> bool { - matches!( - w, - PlanWarning::MissingReference { .. } | PlanWarning::TruncatedAu { .. } - ) + w.is_integrity() } /// Plan a whole vendored clip and assert the global invariants: every AU plans, @@ -1658,6 +1733,30 @@ mod tests { assert!(!bbb.is_empty()); } + /// The false-positive guard for [`PicturePlan::references_clean`], on REAL + /// bitstreams rather than authored ones: two conformance clips that lose nothing + /// must report every single picture clean. A regression that let the ledger mark a + /// healthy stream would refuse every host recovery anchor and force an IDR on + /// every loss — the cheap re-anchor path gone, silently. + /// + /// These clips carry B-slices and real reordering, so they also exercise the + /// "reference lists, not the RPS" reading: 8.3.2 retains pictures the current + /// picture does not use, and folding those in would condemn pictures at random. + #[test] + fn a_lossless_conformance_clip_reports_clean_references_on_every_picture() { + for (name, clip) in [("bear", TEST_BEAR), ("bbb", TEST_BBB)] { + let (_, plans) = plan_whole_clip(clip); + assert!(!plans.is_empty(), "{name} produced no plans"); + for (i, plan) in plans.iter().enumerate() { + assert!( + plan.picture.references_clean, + "{name} picture {i} (poc {}) must read clean on a lossless clip", + plan.picture.pic_order_cnt + ); + } + } + } + #[test] fn b_slices_get_a_future_led_list1_distinct_from_list0() { let aus = split_into_aus(TEST_64X64_I_P_B_P); diff --git a/crates/pf-bitstream/src/lib.rs b/crates/pf-bitstream/src/lib.rs index ee6936c3..12f37b52 100644 --- a/crates/pf-bitstream/src/lib.rs +++ b/crates/pf-bitstream/src/lib.rs @@ -21,6 +21,7 @@ #![forbid(unsafe_code)] pub mod av1; +pub mod clean; pub mod h264; pub mod h265; pub mod sei; diff --git a/crates/pf-client-core/src/session.rs b/crates/pf-client-core/src/session.rs index 309b2226..d527c1e5 100644 --- a/crates/pf-client-core/src/session.rs +++ b/crates/pf-client-core/src/session.rs @@ -1093,9 +1093,38 @@ fn pump( // `image.is_keyframe()` as the decoder's own IDR belt, applies the two-mark // rule + the mark-patience backstop, clears the no-output streak, and returns // whether to present this frame or withhold it as a post-loss concealment. - let present = - gate.on_decoded(frame.flags, image.is_keyframe(), Instant::now()) - == GateVerdict::Present; + // + // CORROBORATED (the grey-frame fix): the wire's RECOVERY_ANCHOR is the host + // asserting something about THIS decoder — "the picture I coded this + // P-frame against is one you still hold, intact" — and it lifts the freeze + // on the FIRST occurrence, no two-mark wait. The host derives that from + // bookkeeping that tracks what the client RECEIVED, not what it managed to + // DECODE, and when those diverge the anchor lifts the freeze onto a + // concealed picture and LEAVES it lifted: grey with motion painted on it + // until some later signal re-arms and the 500 ms backstop extracts a real + // IDR. A rung that planned the AU itself knows better, so it says so here. + // + // What a refusal costs is exactly one thing: the freeze keeps holding the + // last good picture until the backstop fires on its ORIGINAL deadline and + // forces the IDR the anchor failed to be. That is strictly the better half + // of the trade — the alternative is presenting a picture this client can + // prove is damaged — and it is the same direction every rule in the gate + // errs in. Every non-native lane reports `Unavailable` and is untouched. + let evidence = image.anchor_evidence(); + if evidence == punktfunk_core::reanchor::AnchorEvidence::ReferencesDamaged + && frame.flags & punktfunk_core::packet::USER_FLAG_RECOVERY_ANCHOR != 0 + { + tracing::debug!( + "refused a host recovery anchor: this AU predicts from a picture \ + this decoder had to conceal — holding for a real IDR" + ); + } + let present = gate.on_decoded_corroborated( + frame.flags, + image.is_keyframe(), + evidence, + Instant::now(), + ) == GateVerdict::Present; total_frames += 1; // ⚠ The `stats:` decode-path tag is a machine interface — // additive only. M10 removed the rungs whose tags were `vaapi`, diff --git a/crates/pf-client-core/src/video.rs b/crates/pf-client-core/src/video.rs index b95dc96b..9a4016b2 100644 --- a/crates/pf-client-core/src/video.rs +++ b/crates/pf-client-core/src/video.rs @@ -493,6 +493,17 @@ pub struct NativeVkFrame { /// the host, and it cannot be lost separately from the picture. Fed to /// [`ReanchorGate::on_local_recovery`](punktfunk_core::reanchor::ReanchorGate::on_local_recovery). pub recovery: punktfunk_core::reanchor::LocalRecovery, + /// Every picture this AU predicts from was itself decoded from a fully-available + /// reference chain (pf-vkdecode's `DecodedVkFrame::references_clean`). + /// + /// The corroboration for the host's `USER_FLAG_RECOVERY_ANCHOR`, which is a claim + /// about THIS decoder that only this decoder can check. The host derives its + /// anchor from slot bookkeeping that tracks what the client RECEIVED; this tracks + /// what the client managed to DECODE. When they disagree the anchor lifts the + /// post-loss freeze onto a concealed picture and leaves it lifted, which is the + /// grey-with-motion field report. `true` on every ordinary frame of a healthy + /// stream, so the flag is only ever load-bearing on the AU that carries an anchor. + pub references_clean: bool, /// This picture's position in DECODE order (pf-vkdecode's strictly increasing /// per-session ordinal). Delivery order is not decode order: after a failed AU /// the H.265 decoder flushes its DPB, handing back every buffered picture at @@ -559,6 +570,40 @@ impl DecodedImage { } } + /// What this lane can say about the host's re-anchor claim on this frame — the + /// corroboration for `USER_FLAG_RECOVERY_ANCHOR`, fed to + /// [`ReanchorGate::on_decoded_corroborated`](punktfunk_core::reanchor::ReanchorGate::on_decoded_corroborated). + /// + /// An anchor is the host asserting a fact about THIS decoder — *the picture I + /// coded this P-frame against is one you still hold, intact* — and the gate lifts + /// its post-loss freeze on the first one, no two-mark wait. Only a rung that + /// planned the AU itself knows which pictures it predicts from and whether each of + /// those decoded cleanly, so only such a rung can catch the host being wrong. + /// + /// The native Vulkan rung answers; everyone else reports + /// [`AnchorEvidence::Unavailable`](punktfunk_core::reanchor::AnchorEvidence::Unavailable) + /// and the gate treats them exactly as it did before this existed — silence is not + /// refutation, so no lane becomes stricter by accident. + /// + /// ⚠ The CPU rung's H.264 leg plans every AU with the same `H264Planner` and so + /// COULD answer; it does not yet, because its frame type carries no equivalent of + /// [`NativeVkFrame::references_clean`]. Reporting `Unavailable` there is the + /// conservative reading (today's behaviour), not a claim that its references are + /// fine. + pub fn anchor_evidence(&self) -> punktfunk_core::reanchor::AnchorEvidence { + use punktfunk_core::reanchor::AnchorEvidence; + match self { + DecodedImage::NativeVk(f) => { + if f.references_clean { + AnchorEvidence::ReferencesClean + } else { + AnchorEvidence::ReferencesDamaged + } + } + _ => AnchorEvidence::Unavailable, + } + } + /// This frame's position in DECODE order, where the lane knows one — see /// [`NativeVkFrame::decode_order`]. `None` everywhere else, which is what the /// pump reads as "this lane reports no local recovery either, so there is diff --git a/crates/pf-client-core/src/video_vaapi_native.rs b/crates/pf-client-core/src/video_vaapi_native.rs index 51130618..0d876cd9 100644 --- a/crates/pf-client-core/src/video_vaapi_native.rs +++ b/crates/pf-client-core/src/video_vaapi_native.rs @@ -3060,6 +3060,11 @@ mod tests { picture: pf_vaadec::PicturePlanAv1 { frame_type: pf_vaadec::FrameTypeAv1::KeyFrame, is_key: true, + // Vacuously true for a key frame: it predicts from nothing. This fixture + // exists to exercise the SIZING path (sequence max vs coded vs render), so + // the clean bit is incidental here — but it must state the honest value, + // because `false` is the answer that withholds a re-anchor. + references_clean: true, show_frame: true, showable_frame: false, order_hint: 0, diff --git a/crates/pf-client-core/src/video_vk_native.rs b/crates/pf-client-core/src/video_vk_native.rs index d6a5598d..5ab54bce 100644 --- a/crates/pf-client-core/src/video_vk_native.rs +++ b/crates/pf-client-core/src/video_vk_native.rs @@ -741,6 +741,12 @@ fn project_frame(frame: &DecodedVkFrame, guard: NativeReleaseGuard) -> NativeVkF sei_here: frame.recovery.sei_here, is_recovery_point: frame.recovery.is_recovery_point, }, + // Whether this picture's own references decoded cleanly — the corroboration + // the shared gate weighs a host `USER_FLAG_RECOVERY_ANCHOR` against. The + // planner already knows it (it is the one party that resolved this AU's + // reference lists), and it is the only thing that can catch the host + // asserting a re-anchor over a picture THIS decoder had to conceal. + references_clean: frame.references_clean, // Which side of a loss this picture was DECODED on. Carried beside the // recovery mark because the mark is worthless without it: a post-failure // DPB flush delivers pre-loss pictures after the loss, and their marks @@ -1690,6 +1696,11 @@ mod tests { sei_here: true, is_recovery_point: true, }, + // SET, per this fixture's no-boolean-is-false rule — and it earns the + // rule: a projection that dropped this would default it to `false`, + // which reads as "this picture's references were concealed" and would + // make the gate refuse EVERY host recovery anchor on a healthy stream. + references_clean: true, // Distinct from every other number here for the same reason: a // projection that dropped the decode ordinal would make every frame // look pre-loss (0) and silently disable the local-recovery path. @@ -1792,6 +1803,7 @@ mod tests { keyframe, poc, recovery, + references_clean, decode_order, guard: _, } = p; @@ -1838,6 +1850,12 @@ mod tests { "the recovery point SEI's verdict reaches the gate — it is the ONLY \ clean point an intra-refresh session has" ); + assert!( + references_clean, + "the reference-cleanliness verdict rides along — without it the gate \ + cannot refute a host recovery anchor that names a picture this decoder \ + had to conceal, which is the grey-with-motion field report" + ); assert_eq!( decode_order, 17, "the decode ordinal rides along — without it the pump cannot tell a \ @@ -2150,6 +2168,7 @@ mod tests { keyframe: true, poc: 0, recovery: punktfunk_core::reanchor::LocalRecovery::NONE, + references_clean: true, decode_order: 1, guard: NativeReleaseGuard::new( tx, diff --git a/crates/pf-encode/src/enc/codec.rs b/crates/pf-encode/src/enc/codec.rs index 61188a3f..5ee3fbc2 100644 --- a/crates/pf-encode/src/enc/codec.rs +++ b/crates/pf-encode/src/enc/codec.rs @@ -368,6 +368,32 @@ pub trait Encoder: Send { fn invalidate_ref_frames(&mut self, _first_frame: i64, _last_frame: i64) -> bool { false } + /// Mark every resident reference UNTRUSTED FOR RFI ANCHORING — the answer to "the client told + /// us it has damage and we did NOT repair it". + /// + /// Why this exists at all. The slot-family RFI trust domain is the WIRE index each reference + /// holds, which answers *did the client receive this frame*; what an anchor pick actually needs + /// is *did the client DECODE it intact*. [`super::rfi`]'s taint sweep bridges that gap, but it + /// only runs inside [`invalidate_ref_frames`] — reachable from exactly ONE of the client's five + /// damage signals (the frame-index gap, which carries a loss RANGE). The other four report + /// through [`request_keyframe`](Self::request_keyframe), which carries no range and so cannot + /// sweep anything. That is self-healing while the IDR is actually emitted — an IDR flushes the + /// DPB and rebuilds trust from scratch — but the host coalesces those requests (a keyframe + /// storm is a 20-40× spike that deepens the very loss it recovers), and a coalesced request + /// leaves the client's damage unrepaired AND unrecorded. Those references stay anchor + /// candidates, and the next loss is answered with one of them tagged `recovery_anchor` — the + /// client's *definitive* clean re-anchor signal, which lifts its post-loss freeze on the first + /// occurrence. Grey frames, presented, with the freeze lifted. + /// + /// Distrust is deliberately NOT "unusable": ordinary prediction runs off the backend's own slot + /// INDEX, never the wire domain, so this costs nothing but the next anchor pick — which + /// declines and falls through to the (still coalesced, so still non-storming) keyframe path. + /// It is also self-correcting on all three backends: a slot re-marked with a fresh frame, or an + /// IDR flushing the DPB, restores trust within a few frames. So this can suppress RFI briefly, + /// never permanently. + /// + /// Default: no-op — the backends with no reference bookkeeping have no trust to withdraw. + fn distrust_references(&mut self) {} /// Escalate into a pipelined (two-thread) retrieve mode under sustained GPU contention — the /// encoder analog of the capturer depth escalation: AUs ride ~one loop tick behind (`poll` /// may return `None` while an encode is in flight) in exchange for capture/submit no longer diff --git a/crates/pf-encode/src/enc/linux/vulkan_video.rs b/crates/pf-encode/src/enc/linux/vulkan_video.rs index 36d48ad2..732eb5fe 100644 --- a/crates/pf-encode/src/enc/linux/vulkan_video.rs +++ b/crates/pf-encode/src/enc/linux/vulkan_video.rs @@ -3991,6 +3991,35 @@ impl Encoder for VulkanVideoEncoder { } } + /// Withdraw anchor trust from every resident reference (trait docs carry the why). + /// + /// The mechanism is this backend's half of the split `enc::rfi` documents: blank `slot_wire` + /// ONLY. `slot_poc` MUST keep naming every physically-resident DPB picture — it is what + /// [`build_h265_rps_s0`] retains the RPS from, and an RPS that stops naming a resident lets a + /// conforming decoder mark it "unused for reference" and reclaim it (8.3.2), so a later anchor + /// would reference a picture the client has already dropped. That is its own grey-screen bug, + /// documented on `build_h265_rps_s0`, and it is the exact failure this method exists to + /// prevent — so getting the two domains the wrong way round here would trade one for the other. + /// `slot_wire` is the RFI/loss domain; `slot_poc` is the reference-delta domain. + /// + /// `pending_loss` is deliberately left armed, matching this backend's decline arm: a stale arm + /// is re-resolved at frame-build, where the re-pick now finds nothing trusted and forces the + /// IDR that heals the stream. Clearing it here would ship an untagged plain P instead. + /// + /// Ordinary prediction is untouched — it runs off `prev_slot`, an index, not a wire. + fn distrust_references(&mut self) { + let trusted = self.slot_wire.iter().filter(|&&w| w >= 0).count(); + if trusted == 0 { + return; // already fully distrusted — nothing to log or clear + } + self.slot_wire.iter_mut().for_each(|w| *w = -1); + tracing::debug!( + trusted, + "vulkan-encode: client reported unrepaired damage — withdrawing RFI anchor trust from \ + every resident reference (prediction and the RPS are unaffected)" + ); + } + fn poll(&mut self) -> Result> { // Backpressure-drained frames (already read, oldest) come out first, then the oldest slot // still in flight — both in submission order. BLOCKING, per the depth-1 pump contract diff --git a/crates/pf-encode/src/enc/rfi.rs b/crates/pf-encode/src/enc/rfi.rs index 214a3543..048d24ee 100644 --- a/crates/pf-encode/src/enc/rfi.rs +++ b/crates/pf-encode/src/enc/rfi.rs @@ -175,4 +175,56 @@ mod tests { apply(&mut all, plan.tainted); assert_eq!(pick_anchor(&view(&all), 5), None); } + + /// `Encoder::distrust_references` — the OTHER way trust is withdrawn, and the one that needs no + /// loss range. The host calls it when the client reports damage the host did not repair (a + /// coalesced keyframe request, or an RFI anchor the client kept asking past): the sweep cannot + /// run there because a keyframe request carries no range, so every resident reference is + /// withdrawn wholesale instead. All three backends persist that through their own marker; what + /// the shared policy must guarantee is the consequence — the next pick finds nothing and + /// declines, so the caller keyframes instead of serving an anchor over unrepaired damage. + #[test] + fn distrusting_every_reference_makes_the_next_anchor_pick_decline() { + // A table with plenty of pre-loss candidates: without the withdrawal, wire 7 anchors. + let mut wires = [4i64, 5, 6, 7, -1, -1, -1, -1]; + assert_eq!( + pick_anchor(&view(&wires), 9), + Some((3, 7)), + "precondition: this table would happily anchor" + ); + + // The Vulkan mechanism (blank the wire) stands in for all three: AMF clears its mirror and + // QSV raises `ltr_tainted`, but each is filtered out of the trusted view identically — + // which is exactly what makes one pure policy serve three persistence schemes. + apply(&mut wires, u32::MAX); + assert_eq!( + pick_anchor(&view(&wires), 9), + None, + "every reference withdrawn → no anchor, caller falls through to its keyframe path" + ); + // And it holds for ANY later loss, not just this one — the point of persisting distrust. + assert_eq!(pick_anchor(&view(&wires), 100), None); + } + + /// The withdrawal must be temporary, or one coalesced keyframe request would cost a session its + /// RFI recovery for good and every later loss would ride the 20-40× IDR path. Each backend + /// restores trust the same way it always did — a slot re-marked with a fresh frame (and an IDR + /// flush, which empties the table first) — so a refilled slot anchors again. + #[test] + fn a_re_marked_slot_restores_anchor_trust_after_a_full_withdrawal() { + let mut wires = [4i64, 5, 6, 7, -1, -1, -1, -1]; + apply(&mut wires, u32::MAX); + assert_eq!(pick_anchor(&view(&wires), 20), None); + + // Encoding continues; the ring refills two slots with post-withdrawal frames. Those really + // are clean — the client's damage was repaired by the IDR the withdrawal forced — so they + // are legitimate anchors and the sweep must not keep rejecting them. + wires[0] = 14; + wires[1] = 15; + assert_eq!( + pick_anchor(&view(&wires), 20), + Some((1, 15)), + "a re-marked slot is trusted again — the suppression is a few frames, not the session" + ); + } } diff --git a/crates/pf-encode/src/enc/windows/amf.rs b/crates/pf-encode/src/enc/windows/amf.rs index a11e2c56..879451c8 100644 --- a/crates/pf-encode/src/enc/windows/amf.rs +++ b/crates/pf-encode/src/enc/windows/amf.rs @@ -2010,6 +2010,30 @@ impl Encoder for AmfEncoder { } } + /// Withdraw anchor trust from every live LTR (trait docs carry the why). + /// + /// This backend's mechanism, unchanged from the sweep's: distrust = clear the mirror slot. + /// Dropped slots stay dropped and the marking cadence re-marks a clean frame within ~1/4 s, so + /// the suppression is brief by construction. + /// + /// `pending_force` is cleared with them, matching the decline arm above: an un-consumed force + /// would otherwise point at a slot this call just distrusted, and the next submit would + /// force-reference it anyway — shipping the corruption tagged `recovery_anchor`, which is the + /// whole failure being closed. + fn distrust_references(&mut self) { + let live = self.ltr_slots.iter().filter(|m| m.is_some()).count(); + if live == 0 && self.pending_force.is_none() { + return; + } + self.ltr_slots = [None; NUM_LTR_SLOTS]; + self.pending_force = None; + tracing::debug!( + live, + "AMF LTR-RFI: client reported unrepaired damage — withdrawing anchor trust from every \ + live LTR (the marking cadence re-marks a clean frame within ~1/4 s)" + ); + } + fn caps(&self) -> EncoderCaps { EncoderCaps { // As Windows NVENC: the capturer composites; this backend never reads `frame.cursor`. diff --git a/crates/pf-encode/src/enc/windows/qsv.rs b/crates/pf-encode/src/enc/windows/qsv.rs index d2448dfe..19e9f719 100644 --- a/crates/pf-encode/src/enc/windows/qsv.rs +++ b/crates/pf-encode/src/enc/windows/qsv.rs @@ -1467,6 +1467,39 @@ impl Encoder for QsvEncoder { } } + /// Withdraw anchor trust from every live LTR (trait docs carry the why). + /// + /// This backend's mechanism, unchanged from the sweep's: distrust is the SEPARATE + /// `ltr_tainted` flag, never a cleared mirror slot. `ltr_slots` mirrors the HARDWARE DPB and + /// nulling an entry issues no VPL call, so the frame stays marked long-term in the encoder — + /// and the RejectedRefList built at submit only names `Some` slots, so a cleared mirror would + /// silently SKIP the very entry being distrusted and the recovery frame could still predict + /// from it. Taint keeps the mirror intact and the rejection reachable. + /// + /// The taint lifts itself: an IDR flush and a re-mark both clear it, so this suppresses RFI + /// for a few frames, never for the session. + /// + /// `pending_force` is cleared for the same reason as the decline arm above — an un-consumed + /// force would point at a slot this call just distrusted. + fn distrust_references(&mut self) { + let live = self + .ltr_slots + .iter() + .enumerate() + .filter(|&(slot, m)| m.is_some() && !self.ltr_tainted[slot]) + .count(); + if live == 0 && self.pending_force.is_none() { + return; + } + self.ltr_tainted = [true; NUM_LTR_SLOTS]; + self.pending_force = None; + tracing::debug!( + live, + "QSV LTR-RFI: client reported unrepaired damage — withdrawing anchor trust from every \ + live LTR (cleared by the next re-mark or IDR flush)" + ); + } + fn caps(&self) -> EncoderCaps { EncoderCaps { // As Windows NVENC: the capturer composites; this backend never reads `frame.cursor`. diff --git a/crates/pf-encode/src/lib.rs b/crates/pf-encode/src/lib.rs index 918aefb8..5e6d89eb 100644 --- a/crates/pf-encode/src/lib.rs +++ b/crates/pf-encode/src/lib.rs @@ -274,6 +274,14 @@ impl Encoder for TrackedEncoder { fn invalidate_ref_frames(&mut self, first_frame: i64, last_frame: i64) -> bool { self.inner.invalidate_ref_frames(first_frame, last_frame) } + // Same trap class as `set_wire_chunking`, and the one where it would hurt most: unforwarded, + // the default no-op would leave every session serving RFI anchors over damage the client + // reported and the host never repaired — the failure this method exists to close, silently + // reintroduced by the wrapper. (The `every_encoder_method_is_forwarded` guard below catches + // it, which is exactly why that guard is there.) + fn distrust_references(&mut self) { + self.inner.distrust_references() + } // Forwarded for the same reason as `set_wire_chunking` below — the unforwarded default // (`false` = "backend can't pipeline, stop asking") silently killed the §7 LN3 contention // escalation for every session, since the host loop only ever holds the wrapped box. diff --git a/crates/pf-vkdecode/src/decoder.rs b/crates/pf-vkdecode/src/decoder.rs index bf0854b8..462947ae 100644 --- a/crates/pf-vkdecode/src/decoder.rs +++ b/crates/pf-vkdecode/src/decoder.rs @@ -185,6 +185,24 @@ pub struct DecodedVkFrame { /// the loss, and lift a freeze on a wave that completed before it. Comparing /// this ordinal against the one current at the arm is what tells them apart. pub decode_order: u64, + /// Every picture this one was predicted from came off a fully-available reference + /// chain ([`pf_bitstream::h264::PicturePlan::references_clean`] and its two twins). + /// `true` for an IDR/IRAP/key frame and for any picture whose whole chain is clean; + /// `false` from the moment this AU — or anything it descends from — needed + /// concealment. + /// + /// It exists to let a consumer CORROBORATE a host's claim that a frame is a clean + /// re-anchor. `USER_FLAG_RECOVERY_ANCHOR` — the host's LTR-RFI recovery frame — + /// lifts a post-loss freeze on its FIRST occurrence, exactly like a real IDR, + /// because the host says the frame was coded against a known-good reference. The + /// host's "known-good" is an inference from what the client RECEIVED; this is what + /// the client actually DECODED. Where they disagree the freeze used to lift onto a + /// gray plate and every frame after it chained off the corruption, with nothing + /// left to re-arm the gate. + /// + /// A consumer with no such claim to check can ignore it: it is an observation + /// about the stream, and nothing in this crate's own behaviour reads it. + pub references_clean: bool, /// The decode op's slot in the status query pool. pub query_slot: u32, /// The decode op's submission ordinal (validates the query slot has not been @@ -281,12 +299,24 @@ pub enum VkDecodeError { /// correct consumer can never hit this). The AU was planned but NOT decoded; /// release frames and request a keyframe. NoFreeSlot, - /// A DPB slot this AU references holds no bound image. H.265 only, and fatal - /// rather than skippable: `StdVideoDecodeH265PictureInfo`'s RPS arrays are - /// INDICES into `pReferenceSlots`, so dropping one entry would silently - /// re-point every later index at the wrong picture — the exact class of - /// plausible-looking corruption this crate refuses to produce. (H.264 carries - /// no such index arrays and only traces the case.) + /// A DPB slot this AU references holds no bound image. Fatal on all three codecs + /// rather than skippable. + /// + /// H.265 and AV1 have a structural argument: their picture info names DPB slots by + /// INDEX (the H.265 RPS arrays, AV1's name-indexed `refs`), so dropping one entry + /// silently re-points a later index at the wrong picture. H.264 has no such index + /// arrays, and used to skip the case with a `trace!` on exactly that reasoning — + /// but the reasoning was about the STRUCTURE, not the output. The hardware still + /// decodes a P-picture against a reference that was never bound, and on the + /// DPB-and-output-COINCIDE path that is a gray plate with the new frame's motion + /// painted over it. Nothing warned: the planner's DPB genuinely holds the picture + /// (the breakage is this crate's slot→image ledger), so the frame was shipped, + /// presented, and cleared the consumer's demotion streak on the way past — + /// invisible damage, which is the one outcome this crate exists to make impossible. + /// + /// Failing closed is only safe because it is paired with recovery: the latch + /// ([`crate::decoder_h265::RecoveryLatch`]) flushes to the next IRAP/IDR rather + /// than leaving the stream wedged on a slot nothing can honour. UnboundReferenceSlot { slot: u8 }, /// The frame belongs to a generation whose retired pool is already gone /// (double release, or a frame outliving its graveyard entry). @@ -615,6 +645,11 @@ pub(crate) struct PendingPic { pub(crate) recovery: crate::recovery::RecoveryMark, /// See [`DecodedVkFrame::decode_order`]. pub(crate) decode_order: u64, + /// Read off the plan at DECODE time and carried here for the same reason + /// [`Self::recovery`] is: display order is not decode order, and this describes + /// the picture rather than the moment it is delivered. See + /// [`DecodedVkFrame::references_clean`]. + pub(crate) references_clean: bool, } /// A retired generation's picture pool: images the presenter still holds live @@ -678,7 +713,15 @@ pub struct VkH264Decoder { /// The outstanding recovery point SEI, if any — see [`crate::recovery`]. /// Survives session rebuilds on purpose: it is a fact about the STREAM's /// prediction structure, not about this decoder's Vulkan objects. + /// + /// Named apart from [`Self::recovery`], which is this decoder's DPB-recovery + /// latch: the two are unrelated (one is a fact about the stream's prediction + /// structure, the other about this decoder's own wedged state). recovery_watch: crate::recovery::RecoveryWatch, + /// Post-failure DPB recovery owed — see + /// [`crate::decoder_h265::RecoveryLatch`], whose docs carry the whole + /// fail-closed/recover argument for all three codecs. + recovery: crate::decoder_h265::RecoveryLatch, /// Pictures planned so far — stamped onto each one as /// [`DecodedVkFrame::decode_order`]. Survives session rebuilds for the same /// reason the watch does. @@ -724,6 +767,7 @@ impl VkH264Decoder { graveyard: Vec::new(), last_warnings: Vec::new(), recovery_watch: crate::recovery::RecoveryWatch::new(), + recovery: Default::default(), decoded: 0, generation: 0, device_lost: false, @@ -748,6 +792,12 @@ impl VkH264Decoder { } fn decode_inner(&mut self, au: &[u8]) -> Result, VkDecodeError> { + // A previous AU failed after its planning had advanced: clear the stale + // DPB residency BEFORE planning this one, or every AU referencing the + // stranded picture fails forever ([`RecoveryLatch`] docs). + if self.recovery.take() { + self.recover_dpb(); + } // `take_warnings` promises "cleared by the next decode", and this IS a // decode: clear BEFORE planning, so an AU that fails to plan at all cannot // leave the previous AU's warnings behind to be re-read as damage on the @@ -784,7 +834,38 @@ impl VkH264Decoder { ); } - self.ensure_state(&plan)?; + // From here the PLANNER has already advanced past this AU — its DPB holds + // the picture whatever happens next — so any failure below leaves the + // planner's DPB and this decoder's slot/image ledgers able to disagree. + // Latch the recovery for the next decode rather than returning into a + // permanently wedged state. (Deliberately wider than the paths that mutate + // the SlotMap: a failure BEFORE `plan_to_vk` mutates it — an `ensure_state` + // refusal, a `NoFreeSlot` — strands the picture the other way round, + // planner-resident with no slot at all, and wedges just as hard. One flush + // cures both.) The H.265 twin, for the same reason: `decoder_h265`'s + // `RecoveryLatch` docs carry the whole argument. + let result = self.decode_planned(&plan, au, recovery, decode_order); + if result.is_err() { + self.recovery.latch(); + } + result + } + + /// The submission half of one decode, from the point the planner has already + /// advanced. Split out so [`Self::decode_inner`] can latch recovery on ANY + /// failure past that line without threading a flag through every exit. + /// `au` is the same buffer `plan`'s slice ranges index into; `recovery` is the + /// recovery-point verdict already folded for this AU and `decode_order` its + /// decode-order ordinal (both advance in decode order, so neither can be + /// derived here — this path is not reached for every planned AU). + fn decode_planned( + &mut self, + plan: &AuPlan, + au: &[u8], + recovery: crate::recovery::RecoveryMark, + decode_order: u64, + ) -> Result, VkDecodeError> { + self.ensure_state(plan)?; let sps_id = plan.sps.seq_parameter_set_id; // Convert, with ONE rebuild retry on CapacityMismatch — the designed @@ -810,7 +891,7 @@ impl VkH264Decoder { // satisfies ensure_parameters' Recreate contract, and Current/Add // touch nothing a submitted decode reads. unsafe { state.session.ensure_parameters(&plan.sps, &plan.pps)? }; - match plan_to_vk(&plan, &mut state.slots, sps_id) { + match plan_to_vk(plan, &mut state.slots, sps_id) { Ok(converted) => { vk_plan = Some(converted); break; @@ -820,7 +901,7 @@ impl VkH264Decoder { required, capacity, "DPB depth renegotiated — rebuilding session" ); - self.rebuild_state(&plan)?; + self.rebuild_state(plan)?; } Err(e) => return Err(VkDecodeError::Convert(e)), } @@ -1002,6 +1083,7 @@ impl VkH264Decoder { is_idr: plan.picture.is_idr, recovery, decode_order, + references_clean: plan.picture.references_clean, }, ); Ok(()) @@ -1358,6 +1440,40 @@ impl VkH264Decoder { } } + /// Clear the DPB state a failed AU left behind, so planning resumes at the + /// next IDR instead of erroring on residency nothing can honour. + /// + /// Three ledgers have to agree and, after a post-planning failure, do not: + /// the PLANNER's DPB, this decoder's [`SlotMap`], and the slot→image + /// bindings. [`Self::flush`] settles the first (and hands back any picture + /// that did reach output — those frames are real and are still delivered), + /// then [`crate::decoder_h265::reset_slot_bindings`] empties the other two. + /// Pool images the stale bindings pinned go back on the free list; images a + /// consumer still HOLDS stay pinned by their own `held` counts, exactly as + /// they would across a session rebuild. + /// + /// Deliberately not a session rebuild: the session, pools and ring are all + /// still valid — only the DPB bookkeeping is stale — and a rebuild would + /// churn every image allocation for a condition an IDR fixes anyway. + /// + /// The H.265 twin (`decoder_h265::recover_dpb`) is the same function one codec + /// over; the two share `reset_slot_bindings` rather than the whole body because + /// each has to call its OWN `flush`, which settles its own planner's DPB. + fn recover_dpb(&mut self) { + debug!("recovering from a failed AU — flushing the H.264 DPB to the next IDR"); + self.flush(); + if let Some(state) = &mut self.state { + let unbound = crate::decoder_h265::reset_slot_bindings( + &mut state.slots, + &mut state.slot_image, + &mut state.slot_refs, + ); + for picture in unbound { + state.pool.pictures[picture].bound = false; + } + } + } + /// Session/caps for THIS plan exist and match its extent + profile, and the /// stream sits inside the device's level ceiling. DPB-depth mismatches /// surface later as `plan_to_vk`'s `CapacityMismatch` (the designed trigger) @@ -1657,6 +1773,7 @@ pub(crate) fn build_frame( is_idr: entry.is_idr, recovery: entry.recovery, decode_order: entry.decode_order, + references_clean: entry.references_clean, query_slot: entry.query_slot, submission: entry.submission, picture: entry.image as u32, @@ -1848,37 +1965,9 @@ unsafe fn record_and_submit( } // ---- bound-slot staging ---- - // Scope list: this AU's references first, then every other still-held slot - // (their resources must stay bound for their associations to persist), then - // the setup slot as the ACTIVATION entry (slot index -1 binds its resource - // without a current association; the decode op's setup slot then claims it). - let mut scope: Vec<(i32, vk::ImageView, hh::StdVideoDecodeH264ReferenceInfo)> = Vec::new(); - for r in &vk_plan.refs { - match slot_view(state, r.slot) { - Some(view) => scope.push((i32::from(r.slot), view, r.std)), - None => trace!(slot = r.slot, "referenced slot without a bound image"), - } - } - for (slot, _id) in state.slots.held() { - if slot == vk_plan.setup_slot - || scope - .iter() - .any(|&(index, _, _)| index >= 0 && index as u8 == slot) - { - continue; - } - match (state.slot_refs[usize::from(slot)], slot_view(state, slot)) { - (Some(std), Some(view)) => scope.push((i32::from(slot), view, std)), - // Unreachable in practice: every held slot was a setup slot once. - _ => trace!( - slot, - "held slot without reference info/binding — left unbound" - ), - } - } - let reference_count = vk_plan.refs.len().min(scope.len()); // The setup/dst resource: the fresh pool image (coincide) or the DPB layer - // (distinct — the pool image is the separate decode output). + // (distinct — the pool image is the separate decode output). Resolved before the + // scope is built, because it is the scope's last entry. let setup_view = if coincide { state.pool.pictures[dst].view } else { @@ -1888,31 +1977,48 @@ unsafe fn record_and_submit( .expect("distinct mode") .dpb_view(vk_plan.setup_slot) }; - scope.push((-1, setup_view, vk_plan.setup_ref)); + // Scope list: this AU's references first, then every other still-held slot + // (their resources must stay bound for their associations to persist), then + // the setup slot as the ACTIVATION entry (slot index -1 binds its resource + // without a current association; the decode op's setup slot then claims it). + // + // Shared with H.265 (`decoder_h265::build_scope`): the two codecs' layout, + // fail-closed rule and reference-count derivation are the same algorithm over a + // different `StdVideo*` type, and this function's whole job is refusing to guess — + // the property least tolerant of two copies drifting apart. + let held: Vec = state.slots.held().map(|(slot, _id)| slot).collect(); + let (scope, reference_count) = crate::decoder_h265::build_scope( + &vk_plan.refs, + held.into_iter(), + vk_plan.setup_slot, + setup_view, + vk_plan.setup_ref, + &state.slot_refs, + |slot| slot_view(state, slot), + )?; // Staged arrays: resources → std infos → codec slot infos → slot infos. Each // vector is fully built before the next borrows it, so nothing reallocates // under a stored pointer. let resources: Vec> = scope .iter() - .map(|&(_, view, _)| { + .map(|e| { vk::VideoPictureResourceInfoKHR::default() .coded_extent(coded_extent) .base_array_layer(0) - .image_view_binding(view) + .image_view_binding(e.view) }) .collect(); - let std_refs: Vec = - scope.iter().map(|&(_, _, std)| std).collect(); + let std_refs: Vec = scope.iter().map(|e| e.std).collect(); let mut dpb_infos: Vec> = std_refs .iter() .map(|std| vk::VideoDecodeH264DpbSlotInfoKHR::default().std_reference_info(std)) .collect(); let mut begin_slots: Vec> = Vec::with_capacity(scope.len()); - for (index, &(slot_index, _, _)) in scope.iter().enumerate() { + for (index, entry) in scope.iter().enumerate() { begin_slots.push( vk::VideoReferenceSlotInfoKHR::default() - .slot_index(slot_index) + .slot_index(entry.slot_index) .picture_resource(&resources[index]), ); } @@ -2038,8 +2144,96 @@ unsafe fn record_and_submit( #[cfg(test)] mod tests { + use ash::vk::Handle as _; + use super::*; + /// A fake, never-dereferenced view handle keyed by slot, so a scope's bindings + /// can be checked without a device (the H.265 tests' idiom, one codec over). + fn fake_view(slot: u8) -> vk::ImageView { + vk::ImageView::from_raw(u64::from(slot) + 1) + } + + /// A reference-info value carrying just the field the assertions read. + fn h264_std_ref(frame_num: u16) -> hh::StdVideoDecodeH264ReferenceInfo { + // SAFETY: StdVideoDecodeH264ReferenceInfo is a plain-C bindgen struct of a + // bitfield word and integers; all-zero is valid for every field. + let mut std: hh::StdVideoDecodeH264ReferenceInfo = unsafe { std::mem::zeroed() }; + std.FrameNum = frame_num; + std + } + + fn h264_ref(slot: u8, frame_num: u16) -> crate::pic::VkRef { + crate::pic::VkRef { + slot, + std: h264_std_ref(frame_num), + id: u64::from(slot), + } + } + + /// The H.264 leg of the fail-closed rule. It used to trace-and-continue here, on + /// the grounds that H.264 carries no RPS index arrays — but the hardware still + /// decoded the picture against a reference that was never bound, which is a gray + /// plate with motion on it, shipped with no warning attached. Fail closed. + #[test] + fn an_h264_reference_slot_without_a_bound_image_fails_the_whole_op() { + let refs = vec![h264_ref(1, 10), h264_ref(3, 20)]; + let slot_refs = vec![Some(h264_std_ref(0)); 8]; + let err = crate::decoder_h265::build_scope( + &refs, + [1u8, 3].into_iter(), + 0, + fake_view(0), + h264_std_ref(30), + &slot_refs, + |slot| (slot != 3).then(|| fake_view(slot)), + ) + .unwrap_err(); + assert!( + matches!(err, VkDecodeError::UnboundReferenceSlot { slot: 3 }), + "{err}" + ); + } + + /// `reference_count` must be the number of THIS AU's references and nothing else. + /// The old H.264 form (`refs.len().min(scope.len())`) was taken AFTER the + /// held-slot pass appended to the same vector, so a short refs list let the decode + /// op's reference array run past the references into unrelated held slots — a + /// picture predicted from something the stream never named. + #[test] + fn the_h264_reference_count_covers_the_references_and_never_a_held_slot() { + // Two references (slots 1, 3); slots 5 and 6 are held but NOT referenced. + let refs = vec![h264_ref(1, 10), h264_ref(3, 20)]; + let slot_refs = vec![Some(h264_std_ref(77)); 8]; + let (scope, reference_count) = crate::decoder_h265::build_scope( + &refs, + [1u8, 3, 5, 6].into_iter(), + 0, + fake_view(0), + h264_std_ref(30), + &slot_refs, + |slot| Some(fake_view(slot)), + ) + .unwrap(); + + assert_eq!(reference_count, 2, "exactly this AU's references"); + assert_eq!( + scope[..reference_count] + .iter() + .map(|e| e.slot_index) + .collect::>(), + vec![1, 3], + "the decode op's reference prefix is the references, in order" + ); + // The rest of the scope keeps the other slots bound (so their associations + // survive) and ends on the setup activation entry — but none of that is a + // reference of this AU. + assert_eq!( + scope.iter().map(|e| e.slot_index).collect::>(), + vec![1, 3, 5, 6, -1] + ); + } + #[test] fn settle_dpb_readies_outputs_in_order_and_returns_never_output_removals() { let mut pending: BTreeMap = BTreeMap::new(); diff --git a/crates/pf-vkdecode/src/decoder_av1.rs b/crates/pf-vkdecode/src/decoder_av1.rs index b106917b..88add75a 100644 --- a/crates/pf-vkdecode/src/decoder_av1.rs +++ b/crates/pf-vkdecode/src/decoder_av1.rs @@ -1114,6 +1114,7 @@ impl VkAv1Decoder { is_idr: plan.picture.is_key, recovery: crate::recovery::RecoveryMark::NONE, decode_order, + references_clean: plan.picture.references_clean, }, ); diff --git a/crates/pf-vkdecode/src/decoder_h265.rs b/crates/pf-vkdecode/src/decoder_h265.rs index a3713085..b1f338f2 100644 --- a/crates/pf-vkdecode/src/decoder_h265.rs +++ b/crates/pf-vkdecode/src/decoder_h265.rs @@ -139,13 +139,20 @@ struct SessionStateH265 { /// /// This decoder FAILS CLOSED, and that stays: when an AU cannot be carried /// through to a submitted decode, it returns an error rather than substituting a -/// reference or decoding against a slot whose image is gone. H.264's -/// soft-degrade (trace the missing binding, drop that reference, decode anyway) -/// is not available here because `StdVideoDecodeH265PictureInfo`'s +/// reference or decoding against a slot whose image is gone. The structural argument +/// is that `StdVideoDecodeH265PictureInfo`'s /// `RefPicSetStCurrBefore`/`StCurrAfter`/`LtCurr` arrays hold INDICES into the /// decode op's reference array — dropping one entry re-points every later index /// at the wrong picture, which is the corruption-hiding class this crate refuses. /// +/// ⚠ H.264 used to soft-degrade here (trace the missing binding, drop that reference, +/// decode anyway) on the grounds that it carries no such index arrays. It now fails +/// closed and carries this same latch: the arrays were never the point, the OUTPUT +/// was. A P-picture decoded against a reference that was never bound is a gray plate +/// with motion painted over it, and because the planner raises no warning for it, that +/// frame reached the screen and cleared the consumer's demotion streak. Both codecs +/// now fail closed, and both recover through this latch rather than wedging. +/// /// But failing closed once must not wedge the stream FOREVER, and without this /// latch it did: by the time an AU reaches a failure exit, `plan_to_vk_h265` has /// already mutated the [`SlotMap`] (releases + the setup assignment) and the @@ -636,6 +643,7 @@ impl VkH265Decoder { is_idr: plan.picture.is_idr, recovery, decode_order, + references_clean: plan.picture.references_clean, }, ); @@ -1247,10 +1255,15 @@ fn profile_key_for(plan: &AuPlan) -> Result { /// let [`build_scope`] bind a slot the planner no longer knows about, which is the /// same "plausible-looking picture in the wrong place" the unbound-reference /// refusal exists to prevent. -fn reset_slot_bindings( +/// +/// Generic over the cached reference-info type so H.264's recovery uses this exact +/// code rather than a twin: the three ledgers and the "empty them together" rule are +/// codec-independent (`SlotMap` is already shared), and only the `StdVideo*` type in +/// `slot_refs` differs. +pub(crate) fn reset_slot_bindings( slots: &mut SlotMap, slot_image: &mut [Option], - slot_refs: &mut [Option], + slot_refs: &mut [Option], ) -> Vec { // `release` is the only way a slot is freed (SlotMap docs); the collect is // because `held` borrows the map the releases mutate. @@ -1279,10 +1292,55 @@ fn slot_view(state: &SessionStateH265, slot: u8) -> Option { /// (No derived equality: `StdVideoDecodeH265ReferenceInfo` is a plain-C bindgen /// struct without it. Assertions compare the fields that carry meaning.) #[derive(Debug, Clone, Copy)] -struct ScopeEntry { - slot_index: i32, - view: vk::ImageView, - std: hh::StdVideoDecodeH265ReferenceInfo, +pub(crate) struct ScopeEntry { + pub(crate) slot_index: i32, + pub(crate) view: vk::ImageView, + pub(crate) std: S, +} + +/// One of this AU's references, as [`build_scope`] needs to see it: a DPB slot and +/// the codec reference info to bind with it. +/// +/// It exists so H.264 and H.265 share ONE scope builder instead of two hand-copies of +/// a function whose whole job is refusing to guess — the property most in need of a +/// single implementation. Their `VkRef`/`VkRefH265` differ only in the `StdVideo*` +/// type they carry, so the shape generalises exactly. +/// +/// ⚠ AV1 deliberately keeps its own ([`crate::decoder_av1`]'s `build_scope_av1`): its +/// reference array is indexed by reference NAME and may hold HOLES, so its walk is a +/// different algorithm rather than the same one over a different Std type. Folding it +/// in here would mean a builder with a mode flag, which is how the two would drift. +pub(crate) trait ScopeRef { + /// The codec's `StdVideoDecode*ReferenceInfo`. + type Std: Copy; + /// The DPB slot this reference is bound in. + fn slot(&self) -> u8; + fn std(&self) -> Self::Std; +} + +/// [`build_scope`]'s answer: the bound-slot list, and how many of its LEADING entries +/// are this AU's own references (the prefix the decode op takes as its reference +/// array — see the ordering note in `build_scope`'s docs). +pub(crate) type Scope = (Vec::Std>>, usize); + +impl ScopeRef for crate::pic_h265::VkRefH265 { + type Std = hh::StdVideoDecodeH265ReferenceInfo; + fn slot(&self) -> u8 { + self.slot + } + fn std(&self) -> Self::Std { + self.std + } +} + +impl ScopeRef for crate::pic::VkRef { + type Std = ash::vk::native::StdVideoDecodeH264ReferenceInfo; + fn slot(&self) -> u8 { + self.slot + } + fn std(&self) -> Self::Std { + self.std + } } /// Build the coding scope's bound-slot list and say how many leading entries are @@ -1295,36 +1353,51 @@ struct ScopeEntry { /// resources must stay bound even when this AU does not reference them); /// 3. the setup slot as the activation entry, slot index `-1`. /// -/// A reference whose slot binds no image is a hard error, never a skip: -/// `StdVideoDecodeH265PictureInfo`'s `RefPicSetStCurrBefore`/`StCurrAfter`/ -/// `LtCurr` arrays name DPB slots, and every slot they name is one of `refs`' -/// ([`crate::pic_h265`]) — so dropping an entry leaves the hardware with a named -/// slot this op never bound, which it can only answer by guessing or failing. -/// Output that looks plausible and is wrong is the outcome this refusal exists to -/// prevent. -fn build_scope( - refs: &[crate::pic_h265::VkRefH265], +/// A reference whose slot binds no image is a hard error, never a skip. For H.265 the +/// argument is `StdVideoDecodeH265PictureInfo`'s `RefPicSetStCurrBefore`/`StCurrAfter`/ +/// `LtCurr` arrays: they name DPB slots, every slot they name is one of `refs`' +/// ([`crate::pic_h265`]), so dropping an entry leaves the hardware with a named slot +/// this op never bound — which it can only answer by guessing or failing. +/// +/// H.264 has no such index arrays, and it used to skip the case with a `trace!` on +/// exactly that reasoning. The reasoning was wrong about the OUTPUT: the hardware +/// still decodes a P-picture against a reference that was never bound, which on the +/// DPB-and-output-COINCIDE path is a gray plate with the new frame's motion painted +/// over it — and because the planner raised no warning (its DPB genuinely holds the +/// picture; the breakage is in this ledger), the frame was shipped, presented, and +/// cleared the consumer's demotion streak on its way past. Both codecs fail closed +/// here now; the recovery latch is what keeps failing closed from wedging the stream. +/// +/// `reference_count` is captured the instant the `refs` loop ends, BEFORE the +/// held-slot pass appends anything. That ordering is load-bearing: the decode op takes +/// `scope[..reference_count]` as its reference list, so a count computed after the +/// second pass could hand it a still-held slot that this AU does not reference, in +/// place of one that failed to resolve. (Fail-closed above makes that unreachable — +/// but the construction must be correct on its own, not by depending on a check +/// somewhere else.) +pub(crate) fn build_scope( + refs: &[R], held_slots: impl Iterator, setup_slot: u8, setup_view: vk::ImageView, - setup_ref: hh::StdVideoDecodeH265ReferenceInfo, - slot_refs: &[Option], + setup_ref: R::Std, + slot_refs: &[Option], view_of: impl Fn(u8) -> Option, -) -> Result<(Vec, usize), VkDecodeError> { - let mut scope: Vec = Vec::with_capacity(refs.len() + slot_refs.len() + 1); +) -> Result, VkDecodeError> { + let mut scope: Vec> = Vec::with_capacity(refs.len() + slot_refs.len() + 1); for r in refs { - match view_of(r.slot) { + match view_of(r.slot()) { Some(view) => scope.push(ScopeEntry { - slot_index: i32::from(r.slot), + slot_index: i32::from(r.slot()), view, - std: r.std, + std: r.std(), }), - None => return Err(VkDecodeError::UnboundReferenceSlot { slot: r.slot }), + None => return Err(VkDecodeError::UnboundReferenceSlot { slot: r.slot() }), } } let reference_count = scope.len(); for slot in held_slots { - if slot == setup_slot || refs.iter().any(|r| r.slot == slot) { + if slot == setup_slot || refs.iter().any(|r| r.slot() == slot) { continue; } match ( @@ -1862,8 +1935,11 @@ mod tests { let setup_slot = slots.assign(400).unwrap(); assert_eq!(setup_slot, 0, "the freed slots are assignable again"); slot_image[usize::from(setup_slot)] = Some(9); + // The empty slice needs its element type named now that `build_scope` is + // generic over the two codecs' reference types. + let no_refs: [VkRefH265; 0] = []; let (scope, reference_count) = build_scope( - &[], + &no_refs, slots.held().map(|(slot, _id)| slot), setup_slot, fake_view(setup_slot), diff --git a/crates/pf-vkdecode/src/integrity.rs b/crates/pf-vkdecode/src/integrity.rs index a5f493a3..979669c9 100644 --- a/crates/pf-vkdecode/src/integrity.rs +++ b/crates/pf-vkdecode/src/integrity.rs @@ -38,19 +38,20 @@ use crate::{Av1PlanWarning, H265PlanWarning, PlanWarning}; /// property of the STREAM's signalling, which the decoder answers by failing to open /// a session, not by showing a damaged frame. /// -/// Written as an EXHAUSTIVE match with no wildcard, deliberately. A `matches!` (or -/// a `_ => false`) makes "damage" the opt-in and silence the default, so a -/// `PlanWarning` added later — by definition one nobody here has classified — -/// would be reported as clean and its picture shown. Invisible damage is the bug -/// this whole program exists to end; the compiler is the only reviewer guaranteed -/// to be present when that variant is written, so it gets the decision. +/// ⚠ The classification itself now lives on the warning enum, in pf-bitstream +/// ([`PlanWarning::is_integrity`]), and this function delegates. It moved there when +/// the planners gained the per-picture clean bit +/// ([`pf_bitstream::h264::PicturePlan::references_clean`]): that ledger has to mark a +/// picture damaged on exactly the warnings a consumer conceals on, and it lives one +/// crate DOWN from here. A copy of the list in each crate would let the two disagree — +/// the planner recording a picture as clean while the client concealed it, or the +/// reverse — which is the same invisible-damage failure the single-list rule below was +/// written to prevent, one layer lower. One list, in the crate that owns the enum. +/// +/// This function stays as the crate's public spelling of the question (the fault +/// harness, the client and the tests all name it) and keeps its exact semantics. pub fn is_integrity_warning(w: &PlanWarning) -> bool { - match w { - PlanWarning::FrameNumGap { .. } - | PlanWarning::MissingReference { .. } - | PlanWarning::TruncatedAu { .. } => true, - PlanWarning::Mmco5Rebase | PlanWarning::LevelDerivedDpb { .. } => false, - } + w.is_integrity() } /// The H.265 twin — the same set pf-bitstream's own `h265` conformance harness @@ -59,10 +60,7 @@ pub fn is_integrity_warning(w: &PlanWarning) -> bool { /// Exhaustive for the same reason as [`is_integrity_warning`]: a new H.265 warning /// must not be able to mean "damaged" and read as clean. pub fn is_integrity_warning_h265(w: &H265PlanWarning) -> bool { - match w { - H265PlanWarning::MissingReference { .. } | H265PlanWarning::TruncatedAu { .. } => true, - H265PlanWarning::NonZeroReorder { .. } => false, - } + w.is_integrity() } /// The AV1 twin (M7). Every variant the AV1 planner has today IS damage, and that @@ -93,11 +91,7 @@ pub fn is_integrity_warning_h265(w: &H265PlanWarning) -> bool { /// Exhaustive for the same reason as [`is_integrity_warning`]: a new AV1 warning /// must not be able to mean "damaged" and read as clean. pub fn is_integrity_warning_av1(w: &Av1PlanWarning) -> bool { - match w { - Av1PlanWarning::MissingReference { .. } - | Av1PlanWarning::MissingShowExisting { .. } - | Av1PlanWarning::TruncatedAu { .. } => true, - } + w.is_integrity() } #[cfg(test)] diff --git a/crates/pf-vkdecode/src/pic_h265.rs b/crates/pf-vkdecode/src/pic_h265.rs index d6fe2c1e..1a4b485b 100644 --- a/crates/pf-vkdecode/src/pic_h265.rs +++ b/crates/pf-vkdecode/src/pic_h265.rs @@ -899,6 +899,9 @@ mod tests { max_dpb_frames, short_term_ref_pic_set_size_bits: 0, recovery_point: None, + // These fixtures model a healthy stream; the clean bit is the planner's + // observation and nothing in this conversion layer reads it. + references_clean: true, } } diff --git a/crates/punktfunk-core/src/abi.rs b/crates/punktfunk-core/src/abi.rs index 31f6d972..b57fc50b 100644 --- a/crates/punktfunk-core/src/abi.rs +++ b/crates/punktfunk-core/src/abi.rs @@ -5167,6 +5167,15 @@ pub unsafe extern "C" fn punktfunk_reanchor_gate_arm_expecting_drops( /// `USER_FLAG_RECOVERY_POINT`. Pass `decoder_keyframe = false` where the platform decoder doesn't flag /// IDRs (VideoToolbox/MediaCodec) — the wire `FLAG_SOF` covers it. /// +/// This is the uncorroborated entry point and deliberately stays that way. Rust embedders whose +/// decoder parses the bitstream call [`ReanchorGate::on_decoded_corroborated`] to let their own +/// parser refute a `USER_FLAG_RECOVERY_ANCHOR` that names a picture they had to conceal; every +/// client reachable through THIS surface (Apple VideoToolbox, Android MediaCodec) uses a platform +/// decoder that surfaces no such fact, so it would have nothing to pass but +/// `AnchorEvidence::Unavailable` — which is exactly what this wrapper already means. Growing a +/// second export for a corroboration no C caller can supply would spend an ABI version bump on +/// dead surface. +/// /// # Safety /// `g` is a valid gate handle; `out_present` is writable or NULL. #[unsafe(no_mangle)] diff --git a/crates/punktfunk-core/src/reanchor.rs b/crates/punktfunk-core/src/reanchor.rs index b5977d1f..f6b27f11 100644 --- a/crates/punktfunk-core/src/reanchor.rs +++ b/crates/punktfunk-core/src/reanchor.rs @@ -20,6 +20,30 @@ //! VideoToolbox, every FFmpeg rung, which exposes no SEI) simply never calls it and every wire //! behaviour above is bit-for-bit unchanged. //! +//! # The one claim a client can REFUTE +//! +//! Of the three lifts, two are self-evident to the client and one is pure hearsay. An IDR predicts +//! from nothing, so "this re-anchors decode" is a property of the picture itself. A recovery mark is +//! only *half* a re-anchor and the gate says so by requiring two. But +//! [`USER_FLAG_RECOVERY_ANCHOR`] is the HOST asserting a fact about the CLIENT's decoder — *the +//! picture I coded this P-frame against is one you still hold, intact* — and until +//! [`AnchorEvidence`] existed the client took it on faith, on the first occurrence, with no +//! scrutiny at all. +//! +//! When that assertion is wrong the failure is the worst-shaped one in this module: the anchor lifts +//! the freeze onto a picture predicted from a reference the client had to conceal, so the gray plate +//! reaches the screen AND the gate stops holding, which means it keeps reaching the screen until +//! some later signal re-arms. A re-anchor claim the client can refute is therefore worse than no +//! claim at all — no claim merely holds the last good frame until the backstop. +//! +//! So a client whose decoder parses the bitstream corroborates it: it already knows which pictures +//! this AU predicts from and whether each of those decoded from a complete reference chain, and +//! [`on_decoded_corroborated`](ReanchorGate::on_decoded_corroborated) refuses an anchor whose +//! references it can prove were damaged. Refusing can only ever make the gate hold LONGER — the +//! freeze stays up, the backstop fires on its ORIGINAL deadline, and the client escalates to a real +//! IDR — which is the direction every other rule here errs in, deliberately. Lanes that cannot +//! answer pass [`AnchorEvidence::Unavailable`] and behave exactly as they always have. +//! //! [`USER_FLAG_RECOVERY_POINT`]: crate::packet::USER_FLAG_RECOVERY_POINT //! [`USER_FLAG_RECOVERY_ANCHOR`]: crate::packet::USER_FLAG_RECOVERY_ANCHOR @@ -97,8 +121,9 @@ pub fn index_gap(expected: u32, got: u32) -> Option { /// Fold one decoded frame into the re-anchor state and decide whether it lifts the post-loss freeze. /// /// `is_keyframe` — a real IDR (always a clean re-anchor). `has_anchor` — this AU carried -/// [`USER_FLAG_RECOVERY_ANCHOR`](crate::packet::USER_FLAG_RECOVERY_ANCHOR), the host's definitive -/// single-frame re-anchor from an LTR-RFI recovery (a clean P-frame coded against a known-good +/// [`USER_FLAG_RECOVERY_ANCHOR`](crate::packet::USER_FLAG_RECOVERY_ANCHOR) **and the caller did not +/// refute it** ([`AnchorEvidence`]), the host's definitive single-frame re-anchor from an LTR-RFI +/// recovery (a clean P-frame coded against a known-good /// reference), so it lifts on the FIRST occurrence exactly like an IDR — no two-mark wait. `has_mark` — /// this AU carried [`USER_FLAG_RECOVERY_POINT`](crate::packet::USER_FLAG_RECOVERY_POINT), a /// host-signalled intra-refresh wave boundary (only *half* a re-anchor). `marks` — recovery marks seen @@ -158,6 +183,44 @@ impl LocalRecovery { }; } +/// What a client's OWN parser can say about the host's re-anchor claim on one decoded frame — the +/// corroboration for [`USER_FLAG_RECOVERY_ANCHOR`](crate::packet::USER_FLAG_RECOVERY_ANCHOR). +/// +/// An anchor is the host asserting something about the CLIENT's decoder: *this P-frame is coded +/// against a picture you still hold, intact, so decoding it re-anchors you*. The host derives that +/// from its own slot bookkeeping — which tracks whether the client RECEIVED a frame, not whether it +/// DECODED that frame from a complete reference chain. Those two differ exactly when the client had +/// to conceal, and the gap between them is what puts a gray plate on screen with the freeze lifted. +/// +/// Three states rather than a bool, for the same reason [`LocalRecovery`] is two facts: a lane that +/// *cannot* answer must be able to say so instead of being folded into "nothing wrong here". Only +/// [`Self::ReferencesDamaged`] changes any behaviour; the other two are indistinguishable to the +/// gate and differ only in what they claim at the call site. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum AnchorEvidence { + /// This lane has no local bitstream parser, so it cannot corroborate or refute anything — the + /// host's claim stands, exactly as it always has. Android MediaCodec, Apple VideoToolbox and + /// every lane reached over the C ABI pass this, and their behaviour is bit-for-bit unchanged. + #[default] + Unavailable, + /// Corroborated: every picture this AU predicts from was itself decoded from a fully-available + /// reference chain, so the host's claim is consistent with what this decoder actually holds. + ReferencesClean, + /// Refuted: this AU predicts from a picture that needed concealment. Whatever the host believes, + /// decoding this frame cannot re-anchor a decoder whose reference for it is already damaged, so + /// the anchor does not lift the freeze. + ReferencesDamaged, +} + +impl AnchorEvidence { + /// May an [`USER_FLAG_RECOVERY_ANCHOR`](crate::packet::USER_FLAG_RECOVERY_ANCHOR) on this frame + /// be honoured? Only an outright refutation withholds it — silence is not refutation, so a lane + /// that cannot corroborate never becomes *stricter* than it was. + fn honours_anchor(self) -> bool { + !matches!(self, AnchorEvidence::ReferencesDamaged) + } +} + /// Whether a decoded frame should be shown or withheld while the gate is (or isn't) frozen. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum GateVerdict { @@ -333,6 +396,11 @@ impl ReanchorGate { /// A decoded frame always clears the no-output streak. When frozen, a live mark stream pushes the /// backstop out ([`RECOVERY_MARK_PATIENCE`]) so a healing wave isn't pre-empted by a mid-heal IDR. /// + /// This is the whole-hearsay entry point: it believes an anchor on sight. A client whose decoder + /// parses the bitstream should call + /// [`on_decoded_corroborated`](Self::on_decoded_corroborated) instead and let its own parser + /// check the host's claim. + /// /// [`USER_FLAG_RECOVERY_ANCHOR`]: crate::packet::USER_FLAG_RECOVERY_ANCHOR /// [`USER_FLAG_RECOVERY_POINT`]: crate::packet::USER_FLAG_RECOVERY_POINT pub fn on_decoded( @@ -340,10 +408,49 @@ impl ReanchorGate { wire_flags: u32, decoder_keyframe: bool, now: Instant, + ) -> GateVerdict { + self.on_decoded_corroborated( + wire_flags, + decoder_keyframe, + AnchorEvidence::Unavailable, + now, + ) + } + + /// [`on_decoded`](Self::on_decoded) for a client that can CHECK the host's re-anchor claim + /// against its own decoder — the native-decode lanes, which parse every AU themselves and so + /// know both which pictures this one predicts from and whether each of those decoded cleanly. + /// + /// `evidence` is consulted for exactly one thing: whether a + /// [`USER_FLAG_RECOVERY_ANCHOR`](crate::packet::USER_FLAG_RECOVERY_ANCHOR) on THIS frame may + /// lift the freeze. [`AnchorEvidence::ReferencesDamaged`] withholds that lift and nothing else, + /// and the two exclusions are as deliberate as the rule itself: + /// + /// * **A real IDR still lifts.** It predicts from nothing, so no evidence about its references + /// can bear on it — and the IDR is precisely the escalation a refused anchor is trying to + /// provoke. Refusing it would turn the fix into the permanent freeze it exists to avoid. + /// * **The two-mark [`USER_FLAG_RECOVERY_POINT`](crate::packet::USER_FLAG_RECOVERY_POINT) rule + /// is untouched**, including its [`RECOVERY_MARK_PATIENCE`] deadline push. An intra-refresh + /// wave heals by overwriting stripes rather than by predicting from one named picture, so + /// "this frame's references were damaged" says nothing about whether the wave completed. + /// + /// A refused anchor also leaves the backstop deadline exactly where the arm put it. That is the + /// point rather than an omission: the freeze becomes overdue on its ORIGINAL schedule, [`poll`](Self::poll) + /// re-asks, and the client escalates to a real IDR — the recovery the host's anchor failed to + /// deliver. Pushing the deadline out on a refusal would reward a host whose anchors do not work + /// with a longer wait. + pub fn on_decoded_corroborated( + &mut self, + wire_flags: u32, + decoder_keyframe: bool, + evidence: AnchorEvidence, + now: Instant, ) -> GateVerdict { self.no_output_streak = 0; let is_keyframe = decoder_keyframe || (wire_flags & FLAG_SOF as u32 != 0); - let has_anchor = wire_flags & USER_FLAG_RECOVERY_ANCHOR != 0; + // An anchor the client's own parser refutes is not an anchor. Folded in HERE rather than + // inside `reanchor_after_frame` so that function stays a pure statement of the wire rules. + let has_anchor = wire_flags & USER_FLAG_RECOVERY_ANCHOR != 0 && evidence.honours_anchor(); let has_mark = wire_flags & USER_FLAG_RECOVERY_POINT != 0; if has_mark && self.awaiting { self.deadline = Some(now + RECOVERY_MARK_PATIENCE); @@ -888,4 +995,188 @@ mod tests { assert!(!g.poll(0, t + Duration::from_millis(1))); assert!(g.is_holding()); } + + // ---- the corroborated-anchor path (AnchorEvidence) ---- + + use AnchorEvidence::{ReferencesClean, ReferencesDamaged, Unavailable}; + + /// The headline. The host says "this P-frame re-anchors you"; the client's own parser says the + /// picture it predicts from is one IT had to conceal. Both cannot be true, and the client's + /// statement is about its OWN decoder — so the anchor does not lift and the gray plate the + /// anchor would have presented never reaches the screen. + #[test] + fn an_anchor_whose_references_the_decoder_concealed_does_not_lift() { + let mut g = ReanchorGate::new(0); + let now = t0(); + g.arm(now); + assert_eq!( + g.on_decoded_corroborated(ANCHOR, false, ReferencesDamaged, now), + GateVerdict::Hold, + "a refuted anchor is not a re-anchor" + ); + assert!(g.is_holding(), "and the freeze stays up"); + // Repeating it changes nothing — a host that keeps sending anchors it cannot honour never + // talks its way past the gate. + assert_eq!( + g.on_decoded_corroborated(ANCHOR, false, ReferencesDamaged, now), + GateVerdict::Hold + ); + assert!(g.is_holding()); + } + + /// The escalation a refusal exists to provoke must still work. An IDR predicts from nothing, so + /// no evidence about damaged references can bear on it — refusing it too would convert this fix + /// into the permanent freeze it is meant to avoid. + #[test] + fn a_real_idr_lifts_even_while_the_evidence_refutes_anchors() { + // The decoder's own keyframe flag... + let mut g = ReanchorGate::new(0); + let now = t0(); + g.arm(now); + assert_eq!( + g.on_decoded_corroborated(ANCHOR, false, ReferencesDamaged, now), + GateVerdict::Hold + ); + assert_eq!( + g.on_decoded_corroborated(0, true, ReferencesDamaged, now), + GateVerdict::Present, + "the IDR re-anchors regardless of what the anchor evidence says" + ); + assert!(!g.is_holding()); + + // ...and the wire's FLAG_SOF, for the lanes whose decoder does not flag IDRs. + let mut g = ReanchorGate::new(0); + g.arm(now); + assert_eq!( + g.on_decoded_corroborated(SOF, false, ReferencesDamaged, now), + GateVerdict::Present + ); + assert!(!g.is_holding()); + } + + /// A corroborated anchor is still an anchor: the whole point is to refuse the ones the client + /// can disprove, not to stop honouring the mechanism. + #[test] + fn a_corroborated_anchor_lifts_on_the_first_occurrence() { + let mut g = ReanchorGate::new(0); + let now = t0(); + g.arm(now); + assert_eq!( + g.on_decoded_corroborated(0, false, ReferencesClean, now), + GateVerdict::Hold, + "an ordinary frame is still withheld" + ); + assert_eq!( + g.on_decoded_corroborated(ANCHOR, false, ReferencesClean, now), + GateVerdict::Present + ); + assert!(!g.is_holding()); + } + + /// `Unavailable` is the promise made to every lane without a local parser: silence is not + /// refutation. This walks the same sequences the wire-path tests above assert and requires the + /// identical verdicts through the corroborated entry point. + #[test] + fn an_uncorroborated_lane_behaves_exactly_as_it_always_has() { + // The anchor lift, byte for byte the `a_gap_lifts_on_the_first_rfi_anchor` contract. + let mut g = ReanchorGate::new(0); + let now = t0(); + g.arm(now); + assert_eq!( + g.on_decoded_corroborated(0, false, Unavailable, now), + GateVerdict::Hold + ); + assert_eq!( + g.on_decoded_corroborated(ANCHOR, false, Unavailable, now), + GateVerdict::Present + ); + assert!(!g.is_holding()); + + // And `on_decoded` — which every such lane actually calls — must agree with it exactly. + let mut wire = ReanchorGate::new(0); + let mut corroborated = ReanchorGate::new(0); + wire.arm(now); + corroborated.arm(now); + for flags in [0, POINT, 0, ANCHOR, SOF, 0] { + assert_eq!( + wire.on_decoded(flags, false, now), + corroborated.on_decoded_corroborated(flags, false, Unavailable, now), + "flags {flags:#x} diverged between the two entry points" + ); + assert_eq!(wire.is_holding(), corroborated.is_holding()); + } + } + + /// A refused anchor must not buy the host time. The freeze becomes overdue on the deadline the + /// ARM set — not one pushed out by the refusal — so the client escalates to the real IDR that + /// the failed anchor did not deliver. + #[test] + fn a_refused_anchor_leaves_the_backstop_on_its_original_deadline() { + let mut g = ReanchorGate::new(0); + let start = t0(); + g.arm(start); + // Anchors keep arriving and keep being refused, right up to the deadline. + for ms in [10, 100, 300, 490] { + assert_eq!( + g.on_decoded_corroborated( + ANCHOR, + false, + ReferencesDamaged, + start + Duration::from_millis(ms) + ), + GateVerdict::Hold + ); + assert!(!g.poll(0, start + Duration::from_millis(ms)), "not yet due"); + } + let overdue = start + REANCHOR_FREEZE_MAX + Duration::from_millis(1); + assert!( + g.poll(0, overdue), + "the backstop fires on the arm's own deadline — the refusals did not extend it" + ); + assert!( + g.is_holding(), + "and it keeps holding, never resuming to gray" + ); + } + + /// Refuting an anchor says nothing about an intra-refresh wave: a wave heals by overwriting + /// stripes rather than by predicting from one named picture, so the two-mark rule and its + /// patience deadline must be untouched by the evidence. + #[test] + fn refuted_anchors_do_not_disturb_the_two_mark_rule() { + let mut g = ReanchorGate::new(0); + let now = t0(); + g.arm(now); + assert_eq!( + g.on_decoded_corroborated(POINT, false, ReferencesDamaged, now), + GateVerdict::Hold, + "mark #1 is still only half a re-anchor" + ); + // An anchor in between is refused and must not consume or reset the mark count. + assert_eq!( + g.on_decoded_corroborated(ANCHOR, false, ReferencesDamaged, now), + GateVerdict::Hold + ); + assert_eq!( + g.on_decoded_corroborated(POINT, false, ReferencesDamaged, now), + GateVerdict::Present, + "mark #2 lifts exactly as it does on the wire path" + ); + assert!(!g.is_holding()); + } + + /// The evidence is consulted only while an anchor flag is actually present — a refutation on an + /// ordinary frame must not become a second, sticky reason to hold. + #[test] + fn damaged_evidence_alone_neither_holds_nor_arms_an_unfrozen_gate() { + let mut g = ReanchorGate::new(0); + let now = t0(); + assert_eq!( + g.on_decoded_corroborated(0, false, ReferencesDamaged, now), + GateVerdict::Present, + "an unfrozen gate presents; the evidence is about anchors, not about frames" + ); + assert!(!g.is_holding()); + assert!(!g.poll(0, now)); + } } diff --git a/crates/punktfunk-host/src/native/stream.rs b/crates/punktfunk-host/src/native/stream.rs index 2bade60a..98a1a62a 100644 --- a/crates/punktfunk-host/src/native/stream.rs +++ b/crates/punktfunk-host/src/native/stream.rs @@ -2893,15 +2893,46 @@ pub(super) fn virtual_stream(ctx: SessionContext, prepared: Option 0; + tracing::debug!(rfi_unhealed, "forcing keyframe (client decode recovery)"); + if rfi_unhealed { + enc.distrust_references(); + } enc.request_keyframe(); last_forced_idr = Some(now); rfi_echo_swallowed = 0; // the IDR resets the episode — echoes of IT coalesce via the cooldown diff --git a/include/punktfunk_core.h b/include/punktfunk_core.h index 3d5b4acc..47d76c81 100644 --- a/include/punktfunk_core.h +++ b/include/punktfunk_core.h @@ -3774,6 +3774,15 @@ void punktfunk_reanchor_gate_arm_expecting_drops(ReanchorGate *g, uint64_t expec // `USER_FLAG_RECOVERY_POINT`. Pass `decoder_keyframe = false` where the platform decoder doesn't flag // IDRs (VideoToolbox/MediaCodec) — the wire `FLAG_SOF` covers it. // +// This is the uncorroborated entry point and deliberately stays that way. Rust embedders whose +// decoder parses the bitstream call [`ReanchorGate::on_decoded_corroborated`] to let their own +// parser refute a `USER_FLAG_RECOVERY_ANCHOR` that names a picture they had to conceal; every +// client reachable through THIS surface (Apple VideoToolbox, Android MediaCodec) uses a platform +// decoder that surfaces no such fact, so it would have nothing to pass but +// `AnchorEvidence::Unavailable` — which is exactly what this wrapper already means. Growing a +// second export for a corroboration no C caller can supply would spend an ABI version bump on +// dead surface. +// // # Safety // `g` is a valid gate handle; `out_present` is writable or NULL. PunktfunkStatus punktfunk_reanchor_gate_on_decoded(ReanchorGate *g,