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

fix(client): M5's DXVA bytes now answer to libavcodec's own

Browse Source 
The native D3D11VA rung was committed with zero hardware evidence and, more
to the point, zero evidence of any kind: every claim it made about the DXVA
structures rested on reading the specification and reading libavcodec, and
reading is exactly the method that produced the four defects the last review
found. Three of those lived where a smoke test cannot look — in the buffer
descriptors — so a passing session would have proved nothing about them.

So this compares our submission against libavcodec's, byte for byte, on the
same access units of the same two vendored vectors. The reference side comes
from an FFmpeg n8.1 cross-built for Windows with the DXVA paths instrumented
(the recipe is in the harness docs); the comparison covers picture parameters,
quantisation matrices AND the buffer descriptors, 250 AUs per codec:

    H.264 / HEVC picture parameters    250 AUs, no undocumented divergence
    H.264 / HEVC quantisation matrices 250 AUs, no undocumented divergence
    H.264 / HEVC buffer descriptors    250 AUs, no undocumented divergence

It found a real defect immediately. The DXVA short slice record is TEN bytes
— dxva.h packs these bitstream-layout structs to a byte — and this crate
declared it `repr(C)`, which pads {u32,u32,u16} to twelve. libavcodec's own
descriptors say so twice over: 20 bytes of slice control for a two-slice
H.264 picture, 10 for a one-slice HEVC one. Record 0 survives either way
(its fields sit at 0/4/8 regardless), so the mistake is invisible on a
single-slice stream and displaces every later record by two bytes on a
multi-slice one — which punktfunk hosts emit. Both records are now
`repr(C, packed)`, and the HEVC slice-control test grew a second record
because one record is the shape that hid this.

The audit that followed matters more than the fix. Per-field offset asserts
cannot see TAIL padding, which is what this was, so all six hand-declared
structs now also assert that their size equals the last field's offset plus
that field's own size. Under that rule the slice records were the only place
packed and natural alignment disagree — 1040, 232, 224 and 1000 were right
all along, and now provably rather than luckily. The module docs claimed
`repr(C)` "reproduces MSVC's default packing exactly for that shape"; that
was a guess wearing a proof's clothes, and it is gone.

Two differences are documented rather than fixed, each with the argument for
why it is inert. libavcodec seeds prev_poc_msb = 1 << 16 at every IDR, so its
POCs are the specification's plus 65536 uniformly; every use a driver makes
of those fields is a difference, and references match on FrameNumList, so the
harness compares POCs relative to that constant and requires it to hold on
every AU rather than importing a magic number into a derivation the Vulkan
rung shares. And HEVC's loop_filter_across_tiles_enabled_flag is inferred 1
by 7.4.3.3.1 when the PPS codes no tiles while libav leaves it 0, with tiles
disabled either way. Both ride a channel that always prints, and both are
guarded by tests that synthesise the differences an allowance must NOT
absorb — a documented divergence that swallows a real defect would be worse
than no harness at all.

Everything checkable without a capture is now a non-ignored test: the buffer
set and order per codec, NumMBsInBuffer's codec asymmetry (mb_width*mb_height
on H.264's bitstream and slice-control buffers, zero everywhere for HEVC),
the three 7.4.5 scaling-list cases, contiguous slice records tiling DataSize,
the 128-byte padding charged to the last record and no other. That is the
part which would have caught the last round's defects with no hardware at all.
This commit is contained in:
enricobuehler
2026-08-06 09:46:53 +02:00
parent 31087697a9
commit 9c10b97e31
6 changed files with 3677 additions and 193 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/pf-dxvadec/src/descriptors.rs
+444
View File
@@ -0,0 +1,444 @@
//! The buffer DESCRIPTORS one `ID3D11VideoContext::SubmitDecoderBuffers` call
//! carries: which buffers are in the set at all, and the four
//! `D3D11_VIDEO_DECODER_BUFFER_DESC` fields whose values are a DECISION rather
//! than a pointer the driver handed back.
//!
//! # Why this is a module of its own
//!
//! Review 13 found four defects in this backend. **Two of the three structural
//! ones lived here rather than in the picture parameters**: an HEVC
//! quantization-matrix buffer submitted unconditionally (so a driver was handed a
//! matrix of zeros on every stream that disables scaling lists), and a
//! `NumMBsInBuffer` left at 0 where libavcodec's H.264 path writes
//! `mb_width * mb_height` — on the exact call (`SubmitDecoderBuffers`) that this
//! codebase has already seen an Intel driver reject a hand-built variant on.
//!
//! Neither is visible in the picture parameters, neither is visible in a smoke
//! test, and — before this module — neither was visible to any gate this program
//! runs, because the descriptors were built inside `cfg(windows)` code that no CI
//! leg compiles. That is the whole reason the values live here: a descriptor set
//! is a pure function of the conversion's output plus the packer's output, so it
//! can be asserted on any host, on every leg, over every AU of the vendored
//! vectors.
//!
//! # ⚠ The Windows layer still builds its own — rewire it
//!
//! `pf-client-core`'s `video_d3d11_native.rs` (`fill_and_submit` + its private
//! `buffer_desc`) constructs the same four descriptors itself. This module was
//! written to be the single source of truth for them, and that file should be
//! rewired to call [`descriptors_h264`] / [`descriptors_h265`] and translate the
//! result into `D3D11_VIDEO_DECODER_BUFFER_DESC` field for field. Until it is,
//! the two must be read together: this module is the SPEC and the tests are its
//! proof, and a divergence between them is a defect in the Windows file. The
//! ordering, the values and the presence rule below are exactly what that file
//! does today, transcribed — not a new invention.
//!
//! # The values, and where each comes from
//!
//! `CompressedBufferType` (D3D11's `BufferType`) code points, from windows-rs at
//! the workspace's pinned rev (`acb5a1a`,
//! `crates/libs/windows/src/Windows/Win32/d3d11/mod.rs`) — the same numbers
//! DXVA2's `DXVA2_*BufferType` enumeration uses:
//!
//! | buffer | code point |
//! |---|---|
//! | picture parameters | 0 |
//! | inverse quantization matrix | 4 |
//! | slice control | 5 |
//! | bitstream | 6 |
//!
//! **Order**: picture parameters, quantization matrices, bitstream, slice
//! control. libavcodec's `ff_dxva2_common_end_frame` fills its four-entry
//! descriptor array in exactly that order and submits the array as filled; a
//! driver is entitled to care, and matching the path every Windows player
//! exercises costs nothing.
//!
//! **`DataOffset`** is 0 on every buffer, for both sides: each buffer is written
//! from its own mapping's byte 0. (libavcodec `memset`s the descriptor and never
//! writes the field.)
//!
//! **`DataSize`** is the number of bytes actually written: the whole
//! hand-declared struct for the picture parameters and the quantization matrices,
//! the packer's PADDED size for the bitstream ([`crate::pack::Packed::data_size`],
//! a multiple of [`crate::dxva::BITSTREAM_ALIGN`]), and `slices *
//! size_of::<DXVA_Slice_*_Short>()` for the slice control — **ten** bytes per
//! record, not twelve. That number is a measured fact rather than a derivation
//! (`dxva.rs`'s alignment section carries the measurement), and the slice-control
//! `DataSize` is where it is observable from outside: 20 bytes for a two-slice
//! H.264 picture, 10 for a one-segment HEVC one.
//!
//! **`NumMBsInBuffer` is codec-ASYMMETRIC, and that is not an accident to be
//! tidied up:**
//!
//! * H.264 — `mb_width * mb_height` on the BITSTREAM and SLICE_CONTROL
//! descriptors ([`crate::pic::DecodePlanDxva::mb_count`]);
//! * HEVC — 0 on the same two. HEVC has no macroblocks and the field has no CTB
//! spelling;
//! * picture parameters and quantization matrices — 0 in both codecs.
//!
//! That asymmetry is libavcodec's, read out of an **FFmpeg n8.1** tree:
//! `dxva2_h264.c:307` computes `const unsigned mb_count = h->mb_width *
//! h->mb_height` and writes it on the bitstream descriptor (`:412` D3D11, `:425`
//! DXVA2) and passes it for the slice-control commit (`:440-442`);
//! `dxva2_hevc.c` writes a literal 0 in the same three places (`:338`, `:349`,
//! `:359-361`); and `dxva2.c` passes a literal 0 for the two parameter buffers.
//! Setting a CTB count on the HEVC path would be a fresh divergence in the other
//! direction, which is why it is spelled out here rather than left to symmetry.
//!
//! # Presence: the quantization matrix is codec-asymmetric too
//!
//! * **H.264: always submitted.** `dxva2_h264.c:513-516` passes `&ctx_pic->qm`
//! with `sizeof(qm)` unconditionally, and the PPS's lists are always meaningful
//! (the vendored parser has already applied Table 7-2's fallback rules, so a PPS
//! that codes no matrix carries the SPS's or the flat default).
//! * **HEVC: submitted only when the sequence enables scaling lists.**
//! `dxva2_hevc.c:417` takes `int scale = ctx_pic->pp.dwCodingParamToolFlags & 1`
//! — bit 0 is `scaling_list_enabled_flag` — and `:423-426` passes `NULL`/0 when
//! it is clear; the generic layer then submits an IQ-matrix buffer only `if
//! (qm_size > 0)` (`dxva2.c` ~962), with `NumMBsInBuffer` 0.
//! [`crate::pic_h265::DecodePlanDxvaH265::qmatrix`] is `None` in exactly that
//! case, so presence here is `qmatrix.is_some()` and nothing else. Handing a
//! driver a matrix the picture parameters just told it to ignore is a bet on the
//! driver ignoring it too — and with the vendored parser leaving an uncoded list
//! all-zero, the losing side of that bet is every residual dequantizing to
//! nothing.
//!
//! ⚠ The flag test is NECESSARY but not SUFFICIENT. HEVC 7.4.5 says that with
//! `scaling_list_enabled_flag` set and NO scaling-list data in either parameter
//! set, the Table 7-5/7-6 DEFAULT lists apply. FFmpeg's parser seeds those
//! defaults; the vendored cros-codecs parser leaves an uncoded SPS's lists ALL
//! ZERO. So "submit iff the flag" is only half the rule, and the other half lives
//! in [`crate::pic_h265`]'s `quantization_matrices`, which reads the PPS's copy
//! (which that parser DOES default-fill) unless the SPS is the only side that
//! coded any. All three cases are named CPU tests — two in `pic_h265.rs` for the
//! contents, three in `tests/libav_picparams_parity.rs` for the submission fact.
//!
//! # Provenance
//!
//! The libavcodec file:line references above were read out of an FFmpeg n8.1 tree
//! by this work package's coordinator, not out of this repository — there is no
//! FFmpeg source in the worktree, so nothing here can verify them, and a capture is
//! the authority. The buffer ORDER is the one claim with no line reference: it is
//! what `video_d3d11_native.rs` already submits and what
//! `ff_dxva2_common_end_frame` fills its array in, and the harness's descriptor
//! comparison is what will confirm it.
use std::mem::size_of;
use crate::dxva::PicParamsH264;
use crate::dxva::PicParamsHevc;
use crate::dxva::QmatrixH264;
use crate::dxva::QmatrixHevc;
use crate::dxva::SliceH264Short;
use crate::dxva::SliceHevcShort;
use crate::pack::Packed;
use crate::pic::DecodePlanDxva;
use crate::pic_h265::DecodePlanDxvaH265;
/// `D3D11_VIDEO_DECODER_BUFFER_PICTURE_PARAMETERS`.
pub const BUFFER_PICTURE_PARAMETERS: u32 = 0;
/// `D3D11_VIDEO_DECODER_BUFFER_INVERSE_QUANTIZATION_MATRIX`.
pub const BUFFER_INVERSE_QUANTIZATION_MATRIX: u32 = 4;
/// `D3D11_VIDEO_DECODER_BUFFER_SLICE_CONTROL`.
pub const BUFFER_SLICE_CONTROL: u32 = 5;
/// `D3D11_VIDEO_DECODER_BUFFER_BITSTREAM`.
pub const BUFFER_BITSTREAM: u32 = 6;
/// One buffer of a submission, reduced to the fields a caller DECIDES.
///
/// Deliberately not a `D3D11_VIDEO_DECODER_BUFFER_DESC`: that structure has
/// fourteen members, of which ten are either for a mode this backend does not use
/// (`BufferIndex`, `FirstMBaddress`, `Width`/`Height`/`Stride` — motion-compensation
/// buffers), or for protected content (`pIV`, `IVSize`, `PartialEncryption`,
/// `EncryptedBlockInfo`), or reserved. All ten are zero on every buffer this
/// backend submits, which the Windows layer expresses as `..Default::default()`;
/// the four here are the ones that carry a decision, and therefore the ones a
/// comparison against libavcodec is about.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct BufferDescriptor {
/// `BufferType` — one of this module's `BUFFER_*` code points. (The DXVA
/// specs and libavcodec's DXVA2 path call the same field
/// `CompressedBufferType`.)
pub buffer_type: u32,
/// `DataOffset` — 0 on every buffer of every submission (module docs).
pub data_offset: u32,
/// `DataSize` — bytes written into the driver's mapping.
pub data_size: u32,
/// `NumMBsInBuffer` — codec-asymmetric; see the module docs.
pub num_mbs_in_buffer: u32,
}
impl BufferDescriptor {
/// A descriptor with `DataOffset` 0, which is the only value this backend
/// ever submits.
const fn new(buffer_type: u32, data_size: u32, num_mbs_in_buffer: u32) -> BufferDescriptor {
BufferDescriptor {
buffer_type,
data_offset: 0,
data_size,
num_mbs_in_buffer,
}
}
}
/// The slice-control buffer's `DataSize`: `n` short-format records back to back,
/// exactly as [`crate::dxva::slice_bytes`] lays them out.
///
/// Saturating rather than panicking on the (unreachable) overflow: a `u32` holds
/// 429 million ten-byte records, and an AU that produced more has already been
/// refused by the packer.
fn slice_control_size(record_size: usize, records: usize) -> u32 {
u32::try_from(record_size.saturating_mul(records)).unwrap_or(u32::MAX)
}
/// The descriptor set of one H.264 submission, in libavcodec's order.
///
/// Four buffers, always: the quantization matrices travel on every H.264 picture
/// (module docs).
pub fn descriptors_h264(plan: &DecodePlanDxva, packed: &Packed) -> Vec<BufferDescriptor> {
let mb_count = plan.mb_count;
vec![
BufferDescriptor::new(
BUFFER_PICTURE_PARAMETERS,
size_of::<PicParamsH264>() as u32,
0,
),
BufferDescriptor::new(
BUFFER_INVERSE_QUANTIZATION_MATRIX,
size_of::<QmatrixH264>() as u32,
0,
),
BufferDescriptor::new(BUFFER_BITSTREAM, packed.data_size, mb_count),
BufferDescriptor::new(
BUFFER_SLICE_CONTROL,
slice_control_size(size_of::<SliceH264Short>(), packed.records.len()),
mb_count,
),
]
}
/// The descriptor set of one HEVC submission, in libavcodec's order.
///
/// THREE buffers when the sequence disables scaling lists (which is every
/// punktfunk HEVC stream and the vendored vector with it), four when it enables
/// them — and `NumMBsInBuffer` is 0 on all of them (module docs).
pub fn descriptors_h265(plan: &DecodePlanDxvaH265, packed: &Packed) -> Vec<BufferDescriptor> {
let mut out = Vec::with_capacity(4);
out.push(BufferDescriptor::new(
BUFFER_PICTURE_PARAMETERS,
size_of::<PicParamsHevc>() as u32,
0,
));
if plan.qmatrix.is_some() {
out.push(BufferDescriptor::new(
BUFFER_INVERSE_QUANTIZATION_MATRIX,
size_of::<QmatrixHevc>() as u32,
0,
));
}
out.push(BufferDescriptor::new(BUFFER_BITSTREAM, packed.data_size, 0));
out.push(BufferDescriptor::new(
BUFFER_SLICE_CONTROL,
slice_control_size(size_of::<SliceHevcShort>(), packed.records.len()),
0,
));
out
}
#[cfg(test)]
mod tests {
use super::*;
use crate::dxva::PicEntry;
use crate::pack::SliceRecord;
use crate::pic::DxvaRef;
/// A conversion result with nothing in it but the two fields the descriptors
/// read. Built by hand rather than planned from a vector: this module's job is
/// the descriptor SET, and the whole-stream evidence (250 H.264 + 250 HEVC AUs
/// through the real planners) is in `tests/libav_picparams_parity.rs`.
fn h264_plan(mb_count: u32) -> DecodePlanDxva {
DecodePlanDxva {
pic_params: PicParamsH264::zeroed(),
qmatrix: QmatrixH264::zeroed(),
slice_ranges: Vec::new(),
setup_slot: 0,
setup_id: 1,
setup_is_reference: true,
refs: Vec::<DxvaRef>::new(),
mb_count,
}
}
fn h265_plan(qmatrix: Option<QmatrixHevc>) -> DecodePlanDxvaH265 {
DecodePlanDxvaH265 {
pic_params: PicParamsHevc::zeroed(),
qmatrix,
slice_ranges: Vec::new(),
setup_slot: 0,
setup_id: 1,
setup_is_reference: true,
refs: Vec::new(),
}
}
/// `n` slices packed into `data_size` bytes; the record contents do not matter
/// here, only how many there are.
fn packed(slices: usize, data_size: u32) -> Packed {
Packed {
records: (0..slices)
.map(|i| SliceRecord {
location: i as u32 * 64,
bytes: 64,
})
.collect(),
data_size,
}
}
#[test]
fn the_buffer_type_code_points_are_the_ones_windows_rs_declares() {
// From the workspace's pinned windows-rs rev (`acb5a1a`),
// `crates/libs/windows/src/Windows/Win32/d3d11/mod.rs`:
// D3D11_VIDEO_DECODER_BUFFER_PICTURE_PARAMETERS = 0,
// …_INVERSE_QUANTIZATION_MATRIX = 4, …_SLICE_CONTROL = 5, …_BITSTREAM = 6.
// Nothing else in this crate can catch a transposed pair, and a
// transposition would hand the driver a bitstream where it expects slice
// control.
assert_eq!(BUFFER_PICTURE_PARAMETERS, 0);
assert_eq!(BUFFER_INVERSE_QUANTIZATION_MATRIX, 4);
assert_eq!(BUFFER_SLICE_CONTROL, 5);
assert_eq!(BUFFER_BITSTREAM, 6);
}
#[test]
fn an_h264_submission_carries_four_buffers_in_libavcodecs_order() {
let descs = descriptors_h264(&h264_plan(300), &packed(2, 512));
assert_eq!(
descs.iter().map(|d| d.buffer_type).collect::<Vec<_>>(),
vec![
BUFFER_PICTURE_PARAMETERS,
BUFFER_INVERSE_QUANTIZATION_MATRIX,
BUFFER_BITSTREAM,
BUFFER_SLICE_CONTROL,
]
);
assert_eq!(descs[0].data_size, 1040);
assert_eq!(descs[1].data_size, 224);
assert_eq!(descs[2].data_size, 512);
assert_eq!(descs[3].data_size, 2 * 10, "two ten-byte short records");
}
#[test]
fn only_the_h264_bitstream_and_slice_control_buffers_carry_a_macroblock_count() {
// Review 13's defect, in the smallest form that can express it: the field
// is 0 on the two parameter buffers and mb_width*mb_height on the two the
// hardware parses.
let descs = descriptors_h264(&h264_plan(300), &packed(1, 256));
assert_eq!(descs[0].num_mbs_in_buffer, 0, "picture parameters");
assert_eq!(descs[1].num_mbs_in_buffer, 0, "quantization matrices");
assert_eq!(descs[2].num_mbs_in_buffer, 300, "bitstream");
assert_eq!(descs[3].num_mbs_in_buffer, 300, "slice control");
}
#[test]
fn an_hevc_submission_omits_the_quantization_matrix_buffer_when_there_is_none() {
let descs = descriptors_h265(&h265_plan(None), &packed(1, 384));
assert_eq!(
descs.iter().map(|d| d.buffer_type).collect::<Vec<_>>(),
vec![
BUFFER_PICTURE_PARAMETERS,
BUFFER_BITSTREAM,
BUFFER_SLICE_CONTROL,
],
"a submission with no matrix must not carry an empty matrix buffer"
);
assert!(descs
.iter()
.all(|d| d.buffer_type != BUFFER_INVERSE_QUANTIZATION_MATRIX));
}
#[test]
fn an_hevc_submission_carries_the_quantization_matrix_buffer_when_there_is_one() {
let descs = descriptors_h265(&h265_plan(Some(QmatrixHevc::zeroed())), &packed(3, 640));
assert_eq!(
descs.iter().map(|d| d.buffer_type).collect::<Vec<_>>(),
vec![
BUFFER_PICTURE_PARAMETERS,
BUFFER_INVERSE_QUANTIZATION_MATRIX,
BUFFER_BITSTREAM,
BUFFER_SLICE_CONTROL,
]
);
assert_eq!(descs[0].data_size, 232);
assert_eq!(descs[1].data_size, 1000);
assert_eq!(descs[2].data_size, 640);
assert_eq!(descs[3].data_size, 3 * 10, "three ten-byte short records");
}
#[test]
fn the_hevc_descriptors_carry_no_macroblock_count_at_all() {
// The asymmetry, asserted rather than assumed: libavcodec's HEVC path
// writes 0 where its H.264 path writes mb_width*mb_height, and a CTB count
// here would be a divergence in the other direction.
for descs in [
descriptors_h265(&h265_plan(None), &packed(1, 256)),
descriptors_h265(&h265_plan(Some(QmatrixHevc::zeroed())), &packed(4, 1024)),
] {
for desc in descs {
assert_eq!(
desc.num_mbs_in_buffer, 0,
"buffer type {} carries a macroblock count",
desc.buffer_type
);
}
}
}
#[test]
fn every_descriptor_starts_at_byte_zero_of_its_own_buffer() {
let h264 = descriptors_h264(&h264_plan(1), &packed(2, 256));
let h265 = descriptors_h265(&h265_plan(Some(QmatrixHevc::zeroed())), &packed(2, 256));
for desc in h264.into_iter().chain(h265) {
assert_eq!(desc.data_offset, 0);
}
}
#[test]
fn the_slice_control_size_is_one_short_format_record_per_slice() {
// TEN bytes per record is the SHORT format, packed — measured against
// libavcodec on hardware, not derived from the field types (a `#[repr(C)]`
// `{u32, u32, u16}` would be twelve). The long format's record is an order of
// magnitude larger, so this size is also the check that the records match the
// `ConfigBitstreamRaw` this backend asks for.
assert_eq!(size_of::<SliceH264Short>(), 10);
assert_eq!(size_of::<SliceHevcShort>(), 10);
for slices in [1usize, 2, 5, 68] {
let h264 = descriptors_h264(&h264_plan(1), &packed(slices, 4096));
assert_eq!(h264[3].data_size, 10 * slices as u32);
let h265 = descriptors_h265(&h265_plan(None), &packed(slices, 4096));
assert_eq!(h265[2].data_size, 10 * slices as u32);
}
}
#[test]
fn a_reference_entry_in_the_plan_does_not_reach_the_descriptors() {
// A guard on the shape of this module rather than on a value: descriptors
// are a function of SIZES and the macroblock count, so nothing about the
// reference list may leak into them. (Also keeps `DxvaRef` in the test's
// vocabulary, so the plan built above stays a realistic one.)
let mut plan = h264_plan(300);
plan.refs.push(DxvaRef {
slot: 2,
id: 7,
is_long_term: true,
top_field_order_cnt: 4,
bottom_field_order_cnt: 4,
frame_num_or_lt_idx: 1,
});
plan.pic_params.RefFrameList[0] = PicEntry::new(2, true);
assert_eq!(
descriptors_h264(&plan, &packed(1, 256)),
descriptors_h264(&h264_plan(300), &packed(1, 256))
);
}
}
crates/pf-dxvadec/src/dxva.rs
+92 -16
View File
@@ -41,12 +41,37 @@
//! The one unsafe in the crate is [`as_bytes`], and it is fenced behind a
//! sealed trait that only these `#[repr(C)]` PODs implement.
//!
//! # Alignment
//! # Alignment — and the one place natural alignment is WRONG
//!
//! Every member is 1/2/4 bytes, so the natural alignment of all of these is 4 and
//! no member ever needs the x86-64 8-byte rules. `#[repr(C)]` reproduces MSVC's
//! default (`/Zp8`) packing exactly for that shape; the offset assertions are what
//! proves it rather than assumes it.
//! `dxva.h` declares these as wire-format structures under **1-byte packing**, not
//! under MSVC's default. For five of the six that is indistinguishable from natural
//! alignment, because every member happens to sit at a naturally-aligned offset and
//! every total is already a multiple of 4: `DXVA_PicParams_H264` is 1040,
//! `DXVA_PicParams_HEVC` 232, the two quantization matrices 224 and 1000 — all
//! confirmed against libavcodec's runtime `sizeof` in the n8.1 capture described in
//! `tests/libav_picparams_parity.rs`.
//!
//! The slice-control records are the exception and the reason this section exists.
//! `{UINT, UINT, USHORT}` is **10 bytes packed and 12 under natural alignment**, and
//! an earlier revision of this file declared them plain `#[repr(C)]` — asserting 12
//! with "2 bytes tail padding" in the comment, which was a guess dressed as a proof.
//! Measured on hardware (RTX 4090, patched FFmpeg n8.1, both vendored vectors, 250
//! AUs each): the H.264 slice-control buffer's `DataSize` is 20 on a stream with two
//! slices per picture, and the HEVC one's is 10 on a stream with one slice segment
//! per picture. Two codecs, two slice counts, one answer — 10.
//!
//! What the mistake costs, so it is never re-introduced: record 0's fields land at
//! 0/4/8 either way, so a SINGLE-slice stream decodes correctly and the two extra
//! bytes are trailing slop nobody reads. From the second record on, every field is
//! displaced by two bytes per preceding record, so the driver reads a slice offset
//! built from half of one field and half of the next. punktfunk hosts do emit
//! multi-slice streams.
//!
//! Hence: **the packed structs carry `#[repr(C, packed)]`** and the proofs below
//! pin `align_of` as well as `size_of`, plus — for every struct — that its size is
//! exactly its last member's offset plus that member's size, which is the assertion
//! that would have caught this one. Interior padding was already impossible (the
//! per-field offset asserts see it); it was TAIL padding that got in.
//!
//! Sources: the DXVA specifications "DirectX Video Acceleration Specification for
//! H.264/AVC Decoding" (§4.2 `DXVA_PicParams_H264`, §4.4 `DXVA_Qmatrix_H264`, §4.6
@@ -62,6 +87,7 @@
// every generated Win32 struct.
#![allow(non_snake_case)]
use std::mem::align_of;
use std::mem::offset_of;
use std::mem::size_of;
@@ -388,16 +414,21 @@ impl QmatrixH264 {
/// UINT BSNALunitDataLocation; /* 0 */
/// UINT SliceBytesInBuffer; /* 4 */
/// USHORT wBadSliceChopping; /* 8 */
/// } DXVA_Slice_H264_Short; /* 12 bytes (2 bytes tail padding) */
/// } DXVA_Slice_H264_Short; /* 10 bytes — PACKED, no tail padding */
/// ```
///
/// **Ten bytes, not twelve** — `#[repr(C, packed)]`, and the single most important
/// number in this file after the picture-parameters offsets. See the module docs'
/// alignment section for the hardware measurement it comes from and for what
/// getting it wrong does to every record after the first.
///
/// Short format only. The long format (`DXVA_Slice_H264_Long`) additionally
/// carries the derived reference lists and the prediction weight tables, which
/// this backend does not build — a device offering only long-format configs is
/// refused at decoder creation and the ladder answers with the FFmpeg rung, which
/// does implement both.
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)]
#[repr(C)]
#[repr(C, packed)]
pub struct SliceH264Short {
/// Byte offset of the slice's **start code** within the bitstream buffer.
pub BSNALunitDataLocation: u32,
@@ -787,10 +818,14 @@ impl QmatrixHevc {
/// UINT BSNALunitDataLocation; /* 0 */
/// UINT SliceBytesInBuffer; /* 4 */
/// USHORT wBadSliceChopping; /* 8 */
/// } DXVA_Slice_HEVC_Short; /* 12 bytes */
/// } DXVA_Slice_HEVC_Short; /* 10 bytes — PACKED, no tail padding */
/// ```
///
/// Ten bytes for the same reason as [`SliceH264Short`], and measured independently:
/// the HEVC capture's slice-control `DataSize` is 10 on a vector with exactly one
/// slice segment per picture.
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)]
#[repr(C)]
#[repr(C, packed)]
pub struct SliceHevcShort {
pub BSNALunitDataLocation: u32,
pub SliceBytesInBuffer: u32,
@@ -805,6 +840,18 @@ pub struct SliceHevcShort {
// reproduced above. This is the whole defence against a silently mis-declared
// buffer: nothing else in the pipeline can tell a wrong offset from a right one,
// because the driver accepts either and only the picture differs.
//
// Three kinds of assertion, and the third is new because the first two missed a
// real defect (the slice records' 12-vs-10; see the module docs):
//
// 1. `size_of` per struct, against the C total.
// 2. `offset_of` per FIELD, which is what makes interior padding or a reordering
// impossible.
// 3. **size == last field's offset + last field's own size**, per struct — the
// assertion that catches TAIL padding, which is exactly what (1) and (2) cannot
// see: a struct whose declared total is itself wrong satisfies both. Every one of
// these buffers is a wire format with no padding anywhere, and this is where that
// is stated as a proof rather than a comment.
const _: () = {
assert!(size_of::<PicEntry>() == 1);
@@ -852,7 +899,8 @@ const _: () = {
assert!(offset_of!(QmatrixH264, bScalingLists4x4) == 0);
assert!(offset_of!(QmatrixH264, bScalingLists8x8) == 96);
assert!(size_of::<SliceH264Short>() == 12);
assert!(size_of::<SliceH264Short>() == 10);
assert!(align_of::<SliceH264Short>() == 1);
assert!(offset_of!(SliceH264Short, BSNALunitDataLocation) == 0);
assert!(offset_of!(SliceH264Short, SliceBytesInBuffer) == 4);
assert!(offset_of!(SliceH264Short, wBadSliceChopping) == 8);
@@ -908,10 +956,25 @@ const _: () = {
assert!(offset_of!(QmatrixHevc, ucScalingListDCCoefSizeID2) == 992);
assert!(offset_of!(QmatrixHevc, ucScalingListDCCoefSizeID3) == 998);
assert!(size_of::<SliceHevcShort>() == 12);
assert!(size_of::<SliceHevcShort>() == 10);
assert!(align_of::<SliceHevcShort>() == 1);
assert!(offset_of!(SliceHevcShort, BSNALunitDataLocation) == 0);
assert!(offset_of!(SliceHevcShort, SliceBytesInBuffer) == 4);
assert!(offset_of!(SliceHevcShort, wBadSliceChopping) == 8);
// NO TAIL PADDING, per struct: the size is the last member's offset plus the
// last member's own size, nothing more. The right-hand sizes are the C
// declarations' (`UCHAR SliceGroupMap[810]`, `UINT StatusReportFeedbackNumber`,
// …), so each line is an independent statement of the total rather than a
// restatement of `size_of`.
assert!(size_of::<PicParamsH264>() == offset_of!(PicParamsH264, SliceGroupMap) + 810);
assert!(size_of::<QmatrixH264>() == offset_of!(QmatrixH264, bScalingLists8x8) + 2 * 64);
assert!(size_of::<SliceH264Short>() == offset_of!(SliceH264Short, wBadSliceChopping) + 2);
assert!(
size_of::<PicParamsHevc>() == offset_of!(PicParamsHevc, StatusReportFeedbackNumber) + 4
);
assert!(size_of::<QmatrixHevc>() == offset_of!(QmatrixHevc, ucScalingListDCCoefSizeID3) + 2);
assert!(size_of::<SliceHevcShort>() == offset_of!(SliceHevcShort, wBadSliceChopping) + 2);
};
// ---------------------------------------------------------------------------
@@ -1164,11 +1227,18 @@ mod tests {
// reader who distrusts a `const _` finds the same claim executable.
assert_eq!(size_of::<PicParamsH264>(), 1040);
assert_eq!(size_of::<QmatrixH264>(), 224);
assert_eq!(size_of::<SliceH264Short>(), 12);
assert_eq!(size_of::<PicParamsHevc>(), 232);
assert_eq!(size_of::<QmatrixHevc>(), 1000);
assert_eq!(size_of::<SliceHevcShort>(), 12);
assert_eq!(size_of::<PicEntry>(), 1);
// TEN, not twelve. Measured against libavcodec on hardware: the H.264
// slice-control buffer is 20 bytes for a two-slice picture and the HEVC one
// 10 bytes for a one-slice-segment picture (module docs). A `#[repr(C)]`
// `{u32, u32, u16}` is 12, and every record after the first would then be
// displaced by two bytes per preceding record.
assert_eq!(size_of::<SliceH264Short>(), 10);
assert_eq!(size_of::<SliceHevcShort>(), 10);
assert_eq!(align_of::<SliceH264Short>(), 1);
assert_eq!(align_of::<SliceHevcShort>(), 1);
}
#[test]
@@ -1207,10 +1277,16 @@ mod tests {
},
];
let bytes = slice_bytes(&records);
assert_eq!(bytes.len(), 24);
// TEN bytes per record, so the second record starts at byte 10 — this is the
// test that fails if the packing is ever relaxed back to natural alignment,
// and it fails on the SECOND record, which is exactly where the driver
// would have started misreading.
assert_eq!(bytes.len(), 20);
assert_eq!(&bytes[0..4], &0u32.to_le_bytes());
assert_eq!(&bytes[4..8], &100u32.to_le_bytes());
assert_eq!(&bytes[12..16], &100u32.to_le_bytes());
assert_eq!(&bytes[16..20], &250u32.to_le_bytes());
assert_eq!(&bytes[8..10], &0u16.to_le_bytes());
assert_eq!(&bytes[10..14], &100u32.to_le_bytes());
assert_eq!(&bytes[14..18], &250u32.to_le_bytes());
assert_eq!(&bytes[18..20], &0u16.to_le_bytes());
}
}
crates/pf-dxvadec/src/lib.rs
+12
View File
@@ -22,6 +22,10 @@
//! - [`pic`] / [`pic_h265`]: one [`pf_bitstream`] `AuPlan` into
//! `DXVA_PicParams_*`, `DXVA_Qmatrix_*` and the slice-control records, with the
//! reference lists resolved through a DPB slot map.
//! - [`descriptors`]: which buffers one `SubmitDecoderBuffers` call carries and
//! the four `D3D11_VIDEO_DECODER_BUFFER_DESC` fields that are a decision —
//! where two of review 13's three structural defects lived, and the reason
//! they are now a CPU test rather than a Windows-only code path.
//!
//! # Why the slot map comes from pf-vkdecode
//!
@@ -46,6 +50,7 @@
#![deny(clippy::undocumented_unsafe_blocks)]
pub mod config;
pub mod descriptors;
pub mod dxva;
pub mod pack;
pub mod pic;
@@ -95,6 +100,13 @@ pub use config::DXGI_FORMAT_P010;
pub use config::H264_VLD_NOFGT;
pub use config::HEVC_VLD_MAIN;
pub use config::HEVC_VLD_MAIN10;
pub use descriptors::descriptors_h264;
pub use descriptors::descriptors_h265;
pub use descriptors::BufferDescriptor;
pub use descriptors::BUFFER_BITSTREAM;
pub use descriptors::BUFFER_INVERSE_QUANTIZATION_MATRIX;
pub use descriptors::BUFFER_PICTURE_PARAMETERS;
pub use descriptors::BUFFER_SLICE_CONTROL;
pub use dxva::as_bytes;
pub use dxva::slice_bytes;
pub use dxva::PicParamsH264;
crates/pf-dxvadec/src/pic.rs
+15 -5
View File
@@ -1084,11 +1084,21 @@ mod tests {
];
let control = slice_control(&records);
assert_eq!(control.len(), 2);
assert_eq!(control[0].BSNALunitDataLocation, 0);
assert_eq!(control[0].SliceBytesInBuffer, 40);
assert_eq!(control[1].BSNALunitDataLocation, 40);
assert_eq!(control[1].SliceBytesInBuffer, 216);
assert!(control.iter().all(|c| c.wBadSliceChopping == 0));
// Read by VALUE, in braces: `DXVA_Slice_H264_Short` is `#[repr(C, packed)]`
// (ten bytes, see `dxva.rs`'s alignment section), so a reference to one of
// its `u32` members would be unaligned and is a compile error — `assert_eq!`
// takes references to its operands.
assert_eq!({ control[0].BSNALunitDataLocation }, 0);
assert_eq!({ control[0].SliceBytesInBuffer }, 40);
assert_eq!({ control[1].BSNALunitDataLocation }, 40);
assert_eq!({ control[1].SliceBytesInBuffer }, 216);
assert!(control.iter().all(|c| { c.wBadSliceChopping } == 0));
// …and the records reach the driver ten bytes apart, which is the fact the
// whole submission depends on: the second record's location is at byte 10,
// not 12.
let bytes = crate::dxva::slice_bytes(&control);
assert_eq!(bytes.len(), 20);
assert_eq!(&bytes[10..14], &40u32.to_le_bytes());
}
#[test]
crates/pf-dxvadec/src/pic_h265.rs
+24 -7
View File
@@ -1298,14 +1298,31 @@ mod tests {
#[test]
fn slice_control_records_carry_the_packers_locations_verbatim() {
let records = [crate::pack::SliceRecord {
location: 0,
bytes: 128,
}];
let records = [
crate::pack::SliceRecord {
location: 0,
bytes: 128,
},
crate::pack::SliceRecord {
location: 128,
bytes: 256,
},
];
let control = slice_control_h265(&records);
assert_eq!(control[0].BSNALunitDataLocation, 0);
assert_eq!(control[0].SliceBytesInBuffer, 128);
assert_eq!(control[0].wBadSliceChopping, 0);
// Read by VALUE, in braces: the record is `#[repr(C, packed)]` (ten bytes),
// so a reference to a `u32` member would be unaligned — and `assert_eq!`
// takes references. See `dxva.rs`'s alignment section.
assert_eq!({ control[0].BSNALunitDataLocation }, 0);
assert_eq!({ control[0].SliceBytesInBuffer }, 128);
assert_eq!({ control[0].wBadSliceChopping }, 0);
// A SECOND record, because the ten-vs-twelve byte defect is invisible on a
// single-record buffer — the vendored HEVC vector is one slice segment per
// picture, which is precisely the shape that hid it.
assert_eq!({ control[1].BSNALunitDataLocation }, 128);
let bytes = crate::dxva::slice_bytes(&control);
assert_eq!(bytes.len(), 20);
assert_eq!(&bytes[10..14], &128u32.to_le_bytes());
assert_eq!(&bytes[14..18], &256u32.to_le_bytes());
}
#[test]
crates/pf-dxvadec/tests/libav_picparams_parity.rs
+3090 -165
View File
File diff suppressed because it is too large Load Diff