unom/punktfunk
6
0
Fork 1
Code Issues 3 Pull Requests 2 Actions 1 Packages Projects Releases 47 Wiki Activity

feat(vkdecode): M7's Vulkan AV1 rung — GPU half, and the review that saved it

Browse Source 
caps_av1 / session_av1 / decoder_av1, over the CPU half already committed,
sharing the picture pool, bitstream ring, op ring, DPB settling and frame
delivery with H.264 and H.265 rather than forking them. AV1 session
parameters carry exactly one sequence header — no PPS, no VPS — so the
parameters ledger is two-state: current, or recreate.

The GPU plumbing came through review clean. The damage was all in the
conversion committed two rounds ago, which nothing tested against a
reference, and none of it would have failed a gate: clippy was clean, the
tests were green, and the rung would have decoded its own conformance vector
wrong on essentially every frame on AMD, silently.

Four blocking defects, each measured on the vendored vector rather than
argued:

Nine StdVideoDecodeAV1PictureInfo flags were never set. Four change
reconstruction — allow_screen_content_tools on 274 frames of 274,
allow_warped_motion on 273, is_filter_switchable on 172, force_integer_mv on
1 — and RADV reads three of them directly. The block already set
allow_intrabc, which is only codeable when screen-content tools are on, so
it contradicted itself.

LoopRestorationSize sent the pixel size where the field is log2(size) - 5.
cros-codecs stores 64/128/256; RADV names its destination
log2_restoration_size_minus5 and reads 1/2/3. Nothing truncates, nothing
errors, and every frame with loop restoration reconstructs against a
nonsense unit size.

Per-reference Std info answered questions about the wrong picture: every
reference carried the CURRENT frame's type, and RefFrameSignBias was never
set at all. Sign bias is what tells a decoder a reference lies in the
future, and this vector is the hidden-ALTREF one, so all-zero meant every
reference was treated as past. Fixed at the source: pf-bitstream now records
a RefState when a picture is stored — its own frame type, sign-bias mask,
saved order hints — and carries it on the slot, so all three backends get
answers about the reference rather than about the frame reading it.

Film grain's six chroma-scaling fields were zero, which defeats the profile
machinery that exists to refuse devices unable to synthesise grain.

The reference-name compaction is fixed in the PLANNER, once. AuPlan::refs is
now name-indexed with holes preserved, so a lost reference can no longer
renumber every later AV1 reference name — a class that was live in both
conversions and armed for the VAAPI rung that does not exist yet. The DXVA
twin had a second name-versus-slot confusion: it read global motion by DPB
slot from an array the spec indexes by reference name, and slot 0's matrix
is all-zero rather than identity, so 273 references were given a zero warp.

Also closed: pTileOffsets/pTileSizes were sized to tileCount while RADV
reads AV1_MAX_NUM_TILES entries unconditionally — a 4-byte allocation read a
kilobyte deep — now fixed 256-entry arrays with zeroed tails. And the test
guarding the lost-reference refusal re-implemented the predicate inline, so
deleting the guard left it green; both now call one named function.

The bitstream layout now matches libavcodec: raw tile payloads only,
frameHeaderOffset 0. The review established the spec-literal layout was NOT
wrong — AV1 has no start-code scanning, so the 3-versus-4-byte and
slices-only scars do not transfer, and no driver in the fleet reads
frameHeaderOffset — but matching the validated reference deletes code,
uploads 5835 fewer bytes over the vector, and removes the untested-driver
tail.

Upstream, and the third of its kind: the vendored parser writes
ref_frame_sign_bias[i] in the same loop body where it writes
order_hints[LAST_FRAME + i], so its array is shifted one down and index 7 is
never written. Corrected in RefState::of with the shift documented, the
vendored tree untouched, and pinned by a test that recomputes the bias from
order_hints through the parser's own get_relative_dist.

Gates: macOS fmt/clippy/tests, container clippy -D warnings over six crates,
845 tests, workspace check. No hardware: nothing here has reached a driver.
This commit is contained in:
enricobuehler
2026-08-06 21:15:39 +02:00
parent eecd04247f
commit cab3aa1726
12 changed files with 5065 additions and 82 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/pf-vkdecode/src/decoder.rs
+71 -6
View File
@@ -64,10 +64,12 @@ use crate::images::PicturePool;
use crate::images::HOLD_HEADROOM;
use crate::params::level_to_std;
use crate::params::ParamsError;
use crate::params_av1::ParamsAv1Error;
use crate::params_h265::H265ParamsError;
use crate::pic::plan_to_vk;
use crate::pic::DecodePlanVk;
use crate::pic::PlanToVkError;
use crate::pic_av1::PlanToVkAv1Error;
use crate::pic_h265::PlanToVkH265Error;
use crate::ring::pack_slices;
use crate::ring::BitstreamRing;
@@ -210,6 +212,28 @@ pub enum VkDecodeError {
/// outside the H.265 decode envelope (chroma format / bit depth / profile) —
/// a stream-integrity failure, refused rather than half-converted.
ParamsH265(H265ParamsError),
/// [`VkDecodeError::Plan`]'s AV1 counterpart.
PlanAv1(pf_bitstream::av1::PlanError),
/// [`VkDecodeError::Convert`]'s AV1 counterpart.
ConvertAv1(PlanToVkAv1Error),
/// An AV1 sequence header has no Std representation, or the stream sits
/// outside the AV1 decode envelope (sampling / bit depth / profile).
ParamsAv1(ParamsAv1Error),
/// The AV1 access unit's tile groups could not be split into the per-tile
/// byte ranges `VkVideoDecodeAV1PictureInfoKHR::pTileOffsets` wants — a
/// malformed or unexpected OBU. Refused rather than submitted with the whole
/// OBU standing in for its tiles ([`crate::decoder_av1`]).
TilesAv1(crate::decoder_av1::Av1TileError),
/// An AV1 frame named a reference slot the planner's store no longer holds.
///
/// Fatal rather than degraded, and for a sharper reason than "the picture
/// would be wrong": the planner COMPACTS the surviving references into
/// `AuPlan::refs`, so the seven AV1 reference NAMES stop lining up with that
/// list the moment one is lost — every later name would resolve to the wrong
/// picture, which is the plausible-looking corruption this crate refuses to
/// produce. `ref_index` is the AV1 reference name (`LAST_FRAME` = 0 through
/// `ALTREF_FRAME` = 6), `slot` the reference slot it pointed at.
MissingReferenceAv1 { slot: u8, ref_index: u8 },
/// Plan-to-Vulkan conversion failed (caller/session bugs; `CapacityMismatch`
/// is consumed internally by the rebuild path and only surfaces if the rebuilt
/// session STILL mismatches).
@@ -265,6 +289,19 @@ impl std::fmt::Display for VkDecodeError {
VkDecodeError::ParamsH265(e) => {
write!(f, "H.265 parameter-set conversion failed: {e}")
}
VkDecodeError::PlanAv1(e) => write!(f, "AV1 AU planning failed: {e}"),
VkDecodeError::ConvertAv1(e) => write!(f, "AV1 plan conversion failed: {e}"),
VkDecodeError::ParamsAv1(e) => {
write!(f, "AV1 sequence-header conversion failed: {e}")
}
VkDecodeError::TilesAv1(e) => write!(f, "AV1 tile split failed: {e}"),
VkDecodeError::MissingReferenceAv1 { slot, ref_index } => {
write!(
f,
"AV1 reference name {ref_index} points at slot {slot}, which holds \
no picture — the surviving references would renumber"
)
}
VkDecodeError::Convert(e) => write!(f, "plan conversion failed: {e}"),
VkDecodeError::ConvertH265(e) => write!(f, "H.265 plan conversion failed: {e}"),
VkDecodeError::Caps(e) => write!(f, "decode capabilities unusable: {e}"),
@@ -318,6 +355,10 @@ impl std::error::Error for VkDecodeError {
VkDecodeError::ParamsH265(e) => Some(e),
VkDecodeError::Convert(e) => Some(e),
VkDecodeError::ConvertH265(e) => Some(e),
VkDecodeError::PlanAv1(e) => Some(e),
VkDecodeError::ConvertAv1(e) => Some(e),
VkDecodeError::ParamsAv1(e) => Some(e),
VkDecodeError::TilesAv1(e) => Some(e),
VkDecodeError::Caps(e) => Some(e),
VkDecodeError::Device(e) => Some(e),
_ => None,
@@ -353,6 +394,18 @@ impl From<H265ParamsError> for VkDecodeError {
}
}
impl From<ParamsAv1Error> for VkDecodeError {
fn from(e: ParamsAv1Error) -> Self {
VkDecodeError::ParamsAv1(e)
}
}
impl From<PlanToVkAv1Error> for VkDecodeError {
fn from(e: PlanToVkAv1Error) -> Self {
VkDecodeError::ConvertAv1(e)
}
}
impl From<CapsError> for VkDecodeError {
fn from(e: CapsError) -> Self {
VkDecodeError::Caps(e)
@@ -371,6 +424,7 @@ impl From<SessionError> for VkDecodeError {
SessionError::Vk(r) => VkDecodeError::from(r),
SessionError::Params(p) => VkDecodeError::Params(p),
SessionError::ParamsH265(p) => VkDecodeError::ParamsH265(p),
SessionError::ParamsAv1(p) => VkDecodeError::ParamsAv1(p),
SessionError::NoMemoryType { type_bits, flags } => {
VkDecodeError::NoMemoryType { type_bits, flags }
}
@@ -1524,8 +1578,23 @@ pub(crate) fn build_frame(
/// generic for testability — and codec-agnostic (H.265 plans carry the very same
/// [`DpbUpdate`] type), so both decoders settle through this one function.
pub(crate) fn settle_dpb<F>(pending: &mut BTreeMap<PicId, F>, dpb: &DpbUpdate) -> (Vec<F>, Vec<F>) {
settle_dpb_ids(pending, &dpb.outputs, &dpb.removed)
}
/// [`settle_dpb`] over the two id lists directly.
///
/// It exists because AV1's planner declares its OWN `DpbUpdate`
/// ([`pf_bitstream::av1::DpbUpdate`]) rather than re-using the H.264 one the way
/// H.265 does — structurally identical, a distinct type. Splitting the settle at
/// the id lists is what lets all three codecs share ONE implementation of the
/// output/free bookkeeping instead of the AV1 rung growing a copy that could drift.
pub(crate) fn settle_dpb_ids<F>(
pending: &mut BTreeMap<PicId, F>,
outputs: &[PicId],
removed: &[PicId],
) -> (Vec<F>, Vec<F>) {
let mut ready = Vec::new();
for id in &dpb.outputs {
for id in outputs {
match pending.remove(id) {
Some(entry) => ready.push(entry),
// Ids planned before this decoder existed (post-recovery), or
@@ -1533,11 +1602,7 @@ pub(crate) fn settle_dpb<F>(pending: &mut BTreeMap<PicId, F>, dpb: &DpbUpdate) -
None => trace!(id, "output id without a pending picture"),
}
}
let dropped = dpb
.removed
.iter()
.filter_map(|id| pending.remove(id))
.collect();
let dropped = removed.iter().filter_map(|id| pending.remove(id)).collect();
(ready, dropped)
}