Commit Graph
5 Commits
Author SHA1 Message Date
enricobuehler a02014ec19 fix(pf-vkdecode): treat an over-declared stream level as a clamp, not a refusal
A 2026-08-12 field report (RTX 5060 client): every HEVC session demoted to
D3D11VA with 81 "outside device caps: stream level (Std code point 12) above
the device's maxLevelIdc (H.265 Std level 11)" refusals — the host's AMF
encoder stamps general_level_idc 6.2 (the codec maximum) on a 4K120 stream
that needs 5.2, and NVIDIA's driver caps H.265 decode at 6.1. The hardware
decodes the actual stream trivially; only the declaration was oversized.
AV1 passed the same gate, which is why "native-vulkan runs only with AV1".

The declared level is a claim, and the stream's real demands are enforced
where they are physical facts — coded extent and DPB depth, both checked at
session build. So the up-front level gate (H.264 + H.265) now warns once and
proceeds, and every SPS/VPS handed to the Vulkan parameters object has its
level clamped to the device ceiling (a set above maxLevelIdc is invalid
usage). AV1's gate is untouched: its code space is the bitstream's own and
no over-declaration has been seen in the field.

Verified on .173 (RTX 4090, driver 610.88): HEVC and AV1 both decode on the
native Vulkan rung at 60 fps against an NVENC host; unit tests pin the clamp
(lowers, only lowers, mutates the driver-visible block in place).
2026-08-12 18:46:57 +02:00
enricobuehler dee97e893c fix(vkdecode): the address the driver keeps is now the address we keep
The AV1 use-after-free fix (cdd1f3ef) stabilised the wrong half. NVIDIA was
measured retaining pColorConfig, so StoredParamsAv1 boxed the colour and timing
blocks — but OwnedStdAv1SequenceHeader kept the Std struct ITSELF inline, so the
pStdSequenceHeader we handed vkCreateVideoSessionParametersKHR was a stack
address inside ensure_parameters, dead the moment it returned. The fix worked
because of WHICH pointer that driver happened to hold. A driver retaining the
outer one instead — no more of a spec violation than retaining pColorConfig was —
reproduces the original bug exactly: plausible pictures, wrong content, no error
and no counter moved.

The same shape was in the shipping codecs, one step further from evidence: the
H.264 and H.265 create paths pointed pStdSPSs/pStdPPSs/pStdVPSs at function-local
Vecs, and both Add paths handed over the wrapper's inline std field and then moved
the wrapper. Those are spec-legal — the object stores copies — and have never
misbehaved on the fleet. They are fixed anyway, because that is precisely what was
true of H.264/H.265 before the same class of bug was found in them, and a
correctness argument that reduces to which vendor we tested is not one.

So: the Std struct is boxed inside each owning wrapper (one level out from what
_color_backing already did), and the contiguous create-time arrays are now fields
of the stored parameters, assembled at their final address. Identical bytes at
identical offsets — only where they live changed.

The line drawn deliberately, in prose at session.rs:29: Std DATA is pinned; the
VkVideoSessionParametersCreateInfoKHR chain itself is not. Retention there would
be a different and far more extreme class of driver bug, and pinning it needs a
self-referential struct over lifetime-parameterised builders.

⚠ NOT hardware-verified. No GPU has run this — the fleet is unreachable and the
250/250 parity that proved this code bit-exact cannot be re-run. That is why the
change is constrained to address stability alone, and why it ships five CPU-only
tests instead: three capture the pointer handed to Vulkan, perform the real move,
and assert it survives — each verified FAILING first, with genuinely differing
addresses, not a tautology. Two more pin the create-array ownership; those fail
before the fix as compile errors rather than assertions, because the pre-fix bug
there is a dangling pointer and asserting on it is UB.

Also: caps.rs claimed the borrow checker pins a profile chain between wire() and
its last use. False at exactly one site — decoder.rs took a raw *const, ending the
borrow, leaving nothing but inspection to stop a future editor moving the chain
before create_query_pool. Correct today, guarded by prose, which is how the first
bug shipped. It is now compiler-enforced: the pointer write and the create call
live inside one helper that takes the profile by reference, so the borrow is held
across both by the signature. An audit cleared the chains otherwise — no entry
point we pass one to retains it.

Gates: fmt clean; clippy -D warnings over pf-vkdecode AND pf-client-core in the
Linux container (its only real consumer, which cannot build on macOS at all —
wol.rs uses deps its manifest gates to linux/windows, so workspace clippy has
never passed there and does not now); 187 lib tests green on Linux, up from 182.
2026-08-07 09:57:39 +02:00
enricobuehler 185332a866 fix(vkdecode): H.264 and H.265 session parameters own what the driver keeps
The same use-after-free the AV1 rung was just fixed for, closed in the two
rungs that ship. session.rs and session_h265.rs handed their Std parameter
sets to vkCreateVideoSessionParametersKHR and dropped the backings when the
call returned; NVIDIA 610.57.04 was measured retaining such a pointer to
decode-record time, which is what made AV1 diverge on 250 of 250 frames.

Nothing was known to be broken here — both rungs are bit-exact on four
drivers — but that was luck rather than correctness: the freed blocks happen
to still hold the right bytes in that window. The native Vulkan rung sits in
the auto ladder above FFmpeg-Vulkan on shipping clients, so this was live
code, and its failure mode is silent wrong pixels rather than a crash.

StoredParams and StoredParamsH265 hold the parameters object together with
every wrapper it points at, so an object whose backing is gone cannot be
built. create_parameters_object takes the wrappers by value; the Add arm
adopts them only after a successful update, so a failed update drops what it
never stored; the Recreate arm replaces, destroys the old object, then drops
its backings, written explicitly so the ordering survives later edits. The
Add-vs-Recreate decision table and the VPS ledger are untouched — only
ownership moved.

params.rs still carried the refuted claim as a type-level contract, that
Vulkan "copies all parameter data before returning" and keeping the wrapper
alive across the call "is the whole obligation". Corrected to the measured
truth.

The tests are what stop this returning, and each was verified by sabotage:
inlining the H.264 PPS box fails at pps pScalingLists, inlining the H.265 SPS
DPB box fails at sps pDecPicBufMgr, and making either adopt drop instead of
store fails both session tests. Two lessons are recorded in them. Pointer
equality cannot be the assertion, because the Std struct carries pointers by
value and a stale one compares equal — the read-back is the discriminator, so
the tests clobber the dead stack first to make a dangling read deterministic
rather than lucky. And the first H.265 draft read six of eight pointers and
let the sabotage through, so it now reads every one with a labelled assert.

⚠ One site of this class remains, deliberately: the VkVideoProfileInfoKHR
chains, where wire()'s borrow dies with its enclosing block while the object
created from it lives on — three session creates, an image, a buffer, and a
query pool built from a raw pointer into a stack chain. It spans six modules
and all three codecs, and a profile is enums a driver resolves at create time
with no per-frame deref, so the risk is materially lower. It wants its own
pass with its own hardware verification.

Gates: macOS fmt/clippy/196 tests, container clippy -D warnings, pf-vkdecode
182/182 and pf-client-core 140/140. On the RTX 5070 Ti, all 8 gpu_parity legs
re-verified green after the change — H.264, H.265, Main 10 and AV1 all still
bit-identical to libavcodec.
2026-08-07 01:04:17 +02:00
enricobuehler cdd1f3efce fix(vkdecode): AV1 is bit-exact — the bug was a use-after-free, not the driver
250/250 frames bit-identical to libavcodec on NVIDIA 610.57.04, and all four
other parity legs (H.264, H.265, Main 10, both four-byte-prefix twins) still
green.

session_av1 built the sequence header, handed pStdSequenceHeader to
vkCreateVideoSessionParametersKHR, and dropped the backing the instant the
call returned — on the documented assumption that Vulkan copies parameter
data before returning. NVIDIA does not. It keeps the pointer and dereferences
pColorConfig when a decode is RECORDED. The freed block became our own next
allocation, whose bytes read back as mono_chrome = 1, and a monochrome frame
skips exactly loop_filter_level[2..3] (AV1 7.14).

That is the whole fingerprint two earlier rounds chased: luma bit-exact,
chroma off by small amounts, and rewriting the chroma levels in the bitstream
changing nothing — the driver read them correctly and then discarded them,
because it believed the stream had no chroma. StoredParamsAv1 now holds the
parameters object and its Std backing in one value, so an object whose
backing is gone is unrepresentable.

The road there is worth recording, because two well-evidenced conclusions
were wrong before this one was right. A software oracle reproduced the
divergence exactly by disabling chroma deblocking, and a GPU probe showed
chroma levels [8,12] and [63,63] producing byte-identical output — which
looked conclusive and was not. libavcodec's own Vulkan AV1 hwaccel is
bit-exact on this same driver, which proved the hardware fine and the defect
ours. ffmpeg never hits it: with VK_KHR_video_maintenance2 it uses inline
session parameters and never creates a parameters object at all.

The proof is direct rather than inferred: a throwaway Vulkan capture layer
dumped both submissions and every byte of our AV1 picture info already
matched libavcodec's, including the loop filter block; only the session
parameters layer differed. Watching the block's address showed correct bytes
at create and our next allocation at decode.

Ruled out on hardware, so nobody re-tests them: filmGrainSupport,
maxCodedExtent, maxDpbSlots/maxActiveReferences, VkVideoDecodeUsageInfoKHR,
the tile-start sentinel, the setup slot's SavedOrderHints, a NULL
pTimingInfo, and heap luck.

Two earlier fixes are confirmed against libavcodec's captured wire bytes and
kept: CDEF secondary strengths carry the coded value rather than the spec's
in-place fixup, and LoopRestorationSize is log2-based. The refuted
driver-ignores-chroma-levels claim is corrected everywhere it was written
down, and that probe test now passes and points at the lifetime of everything
a submission points at before blaming a vendor.

⚠ Adjacent and NOT fixed: session.rs and session_h265.rs drop their Std
backings the same way, and those sets carry embedded pointers too. Both are
measured bit-exact on four drivers, so nothing is known to be wrong — but the
contract now rests on a driver behaviour measured FALSE for AV1 on a shipping
driver. The SAFETY comments asserting it have been corrected; the structure
is deliberately untouched pending its own pass.

Gates: macOS fmt/clippy/336 tests, container clippy -D warnings, all green;
8/8 gpu_parity and 3/3 gpu_smoke legs verified on the RTX 5070 Ti.
2026-08-07 00:39:34 +02:00
enricobuehler 6d8f3b45b5 feat(pf-vkdecode): the GPU half of HEVC decode — session, pools, recording
M3 WP-2 complete. caps_h265.rs builds the profile the stream actually
needs (profile idc + chroma + bit depths, all three stated on every
Vulkan object) and resolves its picture format — Main to NV12, Main 10
to P010, RExt 4:4:4 to the two-plane 4:4:4 formats — validating it
against the format list of every role the chosen arrangement creates
images in. A Main 10 stream on an 8-bit-only device is refused BEFORE a
session exists, never narrowed: decoding 10-bit into an 8-bit surface is
the silent-wrongness class this crate exists to refuse. session_h265.rs
adds the three-array parameters ledger; decoder_h265.rs adds
VkH265Decoder, mirroring VkH264Decoder method-for-method so the client
wiring is a two-arm dispatch away.

H.264 and H.265 now SHARE the machinery instead of duplicating it:
derive_arrangement (one coincide/distinct/layered decision table),
ring::rebased_offsets (the slices-only rebase — non-VCL NALUs in the
decode range hang VCN firmware), session::bind_session_memory, and a
parameterised build_frame. A DecodeProfile enum replaces the bare
profile idc that images.rs and ring.rs used to take: both codecs' idc
types are c_uint, so handing an H.265 idc to the H.264 path COMPILED
SILENTLY and built a mismatched profile chain. That is now
unrepresentable.

The VPS leg is the ledger's real work. The vendored parser attaches a
VPS to an SPS only when it saw the NALU, and clients join live streams,
so VpsSource is Parsed-or-FromSps and is stored BY VALUE: re-activating
a VPS-less SPS is Current (no churn), but the real VPS arriving under
the same id is a content change and RECREATES onto it, because Vulkan
cannot replace a stored parameter set.

Review round 10 (adversarial) confirmed the hardware-proven H.264 path
is NOT regressed — derive_arrangement's check order and error identity
are byte-for-byte the original, build_frame's call sites still pass the
granularity-aligned extent (the 1088-row scar stays shut), and
rebased_offsets reproduces the deleted inline loop for every input while
moving the sum to u64 so overflow errors instead of wrapping. Also
verified: the refs-order contract on every path, the RESULT_STATUS caps
gate (each of reset/begin/end individually gated, no pool created when
unsupported — recording one on RADV hangs its VCN), pNext lifetimes, and
that no panic is reachable on stream input.

Its 10 findings are fixed. The two that mattered:

- A failed decode stranded a DPB slot. Once plan_to_vk_h265 had mutated
  the slot map, five later failure paths returned without restoring it,
  so planner and slot map both believed a picture was resident while no
  image held it — and every later AU referencing it failed, where H.264
  soft-degrades and keeps delivering. Fail-closed is kept (substituting
  a reference silently is the corruption-hiding this program exists to
  end) but made RECOVERABLE: a latch flushes the planner to AwaitingIdr
  and resets the bindings on the next decode, which composes with the
  client already requesting a keyframe on every decode error. The fix
  deliberately covers pre-mutation failures too — those strand the
  picture the other way round and wedge identically.
- DecodedVkFrame carried no picture format, so a Main 10 frame would
  decode correctly and be rendered with 8-bit transfer/range math. It
  now carries one, stamped from the pool so it is truthful for both
  decoders by construction. The presenter comment says depth 8 is
  because only H.264 is WIRED, not a decoder limit.

Plus: bind_session_memory freed allocations before the session that may
hold them was destroyed (an ordering regression from the extraction,
with a SAFETY comment asserting the opposite) — the bind-stage exit now
hands them back so Drop destroys first; max_level_idc is codec-tagged
rather than an H.264 type carrying H.265 code points; and the decode
family's videoCodecOperations is now checked, turning 'create an H.265
session on a device without the extension' from UB into a clean ladder
demote.

Deferred by design: no HEVC gpu_smoke/gpu_parity yet (its goldens are
already in tests/data/test-25fps-h265.nv12.sha256), and no codec
dispatch in the client — both later legs.

Gates: fmt clean; mac clippy zero warnings, pf-vkdecode 106 +
pf-bitstream 69 green; container clippy -D warnings zero for
pf-client-core + pf-presenter + pf-vkdecode, tests 69/121/106 green.

HARDWARE (.173, after the refactor — review saying the proven path is
safe is not the GPU saying it): gpu_parity '250 frames bit-identical to
libavcodec software decode' on BOTH the NVIDIA 4090 (610.88, coincide
mode) and the AMD iGPU (Adrenalin 25.10.30.02, distinct mode), gpu_smoke
green on both. Two independent drivers, both DPB modes, still bit-exact.
The smoke trace also shows the new videoCodecOperations capture reading
DECODE_H264 | DECODE_H265 | DECODE_AV1 off the real decode family.
2026-08-06 01:59:39 +02:00