fix(client): the native rung now follows the stream's colour and reports true decode latency

The round-4 residuals, closed after the WP-D hardware verdict:

- VUI colour plumbing (the one silent-wrong): the picture's ACTIVE SPS's
  colour signalling (H.273 code points + range, with E.2.1's 'unspecified'
  inference where the VUI is silent — the vendored parser's defaults ARE the
  inferred values, verified) rides PicturePlan -> DecodedVkFrame ->
  NativeVkFrame per frame, never latched: the Windows host switches an HDR
  desktop to PQ/BT.2020 IN-BAND while the Welcome still says SDR. Before
  this, the native path would have painted PQ washed out, silently.
- Native decode-latency stat: the deliberately-deferred NativeVk arm of the
  pump's sampled once-per-stats-window decode measurement now feeds - the
  frame's (semaphore, semaphore_value) is the decode-done signal, resolved
  through the shipped ledger before a bounded, pure-measurement
  vkWaitSemaphores (VkH264Decoder::wait_decoded).
- The renegotiation-teardown window is settled as NO HOLE: rebuild_state now
  documents the full safety argument (graveyarded pools stay intact under
  presenter holds, tokens route strictly by generation, session objects die
  only post-drain with the generation gate INSIDE read_status), and the two
  backend comments that wrongly claimed stale pools were 'gone' are fixed.
- VK_KHR_unified_image_layouts stays deferred (fleet drivers lack it).

Adversarial review round 6: 3 minor findings (2 doc fixes applied; the
SPS-replaced-without-PPS-resend divergence stays a documented envelope
assumption - hosts re-send both at every keyframe, and a hardening
PlanWarning could cost real frames on a false positive).

Gates: fmt clean; clippy -D warnings zero (mac + pf-lxcheck2 container,
incl. pf-client-core/pf-presenter); tests 45+30+53 mac, 30+121+53 container.
This commit is contained in:
2026-08-05 22:22:03 +02:00
parent e6d6498a49
commit dc0766b2f3
6 changed files with 259 additions and 25 deletions
+143
View File
@@ -101,6 +101,12 @@ pub struct PicturePlan {
pub coded_height: u32,
/// Conformance-window crop (7.4.2.1.1), in luma samples of the coded picture.
pub display_crop: DisplayCrop,
/// Colour signalling from the ACTIVE SPS's VUI (E.2.1 inference where absent).
/// Per picture, like [`Self::display_crop`], never latched at session start:
/// the Windows host switches an HDR desktop to PQ/BT.2020 IN-BAND with a new
/// SPS mid-stream, so a backend that captured the first AU's colour would
/// paint HDR frames washed out.
pub colour: ColourDescription,
pub profile_idc: u8,
pub level_idc: Level,
pub bit_depth_luma_minus8: u8,
@@ -120,6 +126,22 @@ pub struct DisplayCrop {
pub height: u32,
}
/// One picture's colour signalling: raw H.273 code points off the active SPS's
/// VUI. When the VUI (or its `video_signal_type`/`colour_description` blocks) is
/// absent these hold E.2.1's INFERRED values — 2/2/2 ("unspecified") with limited
/// range — never a raw struct-zero (0 is a reserved code point no real stream
/// means). That matches the CICP libavcodec reports for such streams, so backends
/// forward these untouched and the consumer's CSC resolves "unspecified" to its
/// SDR default.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct ColourDescription {
pub colour_primaries: u8,
pub transfer_characteristics: u8,
pub matrix_coefficients: u8,
/// `video_full_range_flag` (E.2.1 infers limited range when absent).
pub video_full_range: bool,
}
/// One slice NALU of the picture, with its reference lists fully derived.
#[derive(Debug, Clone)]
pub struct SlicePlan {
@@ -1587,6 +1609,17 @@ impl H264Planner {
width: rect.max.x,
height: rect.max.y,
},
// Read unconditionally: the vendored parser builds every SPS from
// `Default`, whose `VuiParams` already holds E.2.1's inferred values
// (2/2/2, limited range), and parsing only overwrites them under the
// present flags — so this IS the spec inference whether or not the
// stream carried a VUI.
colour: ColourDescription {
colour_primaries: sps.vui_parameters.colour_primaries,
transfer_characteristics: sps.vui_parameters.transfer_characteristics,
matrix_coefficients: sps.vui_parameters.matrix_coefficients,
video_full_range: sps.vui_parameters.video_full_range_flag,
},
profile_idc: sps.profile_idc,
level_idc: sps.level_idc,
bit_depth_luma_minus8: sps.bit_depth_luma_minus8,
@@ -2274,6 +2307,116 @@ mod tests {
);
}
/// A 64x64 SPS with the VUI colour fields set as given. SpsBuilder has no
/// colour setters, so the built Sps is unwrapped and mutated directly (the
/// separate_colour_plane test's idiom); the synthesizer writes the whole
/// `video_signal_type` block from the struct.
fn sps_with_vui_colour(
signal_type: bool,
full_range: bool,
description: Option<(u8, u8, u8)>,
) -> Rc<Sps> {
let mut sps = Rc::try_unwrap(base_sps().resolution(64, 64).build()).expect("freshly built");
sps.vui_parameters_present_flag = true;
sps.vui_parameters.video_signal_type_present_flag = signal_type;
sps.vui_parameters.video_full_range_flag = full_range;
if let Some((primaries, transfer, matrix)) = description {
sps.vui_parameters.colour_description_present_flag = true;
sps.vui_parameters.colour_primaries = primaries;
sps.vui_parameters.transfer_characteristics = transfer;
sps.vui_parameters.matrix_coefficients = matrix;
}
Rc::new(sps)
}
fn plan_one_idr(sps: &Rc<Sps>) -> AuPlan {
let pps = PpsBuilder::new(Rc::clone(sps))
.pic_parameter_set_id(0)
.pic_init_qp(26)
.build();
let mut au = param_set_au(sps, &pps);
au.extend(write_idr_slice());
H264Planner::new().plan_au(&au).unwrap()
}
#[test]
fn an_sps_without_vui_plans_the_e211_unspecified_colour() {
let (sps, pps) = authored_sps_pps();
assert!(
!sps.vui_parameters_present_flag,
"the base SPS carries no VUI"
);
let mut au = param_set_au(&sps, &pps);
au.extend(write_idr_slice());
let plan = H264Planner::new().plan_au(&au).unwrap();
assert_eq!(
plan.picture.colour,
ColourDescription {
colour_primaries: 2,
transfer_characteristics: 2,
matrix_coefficients: 2,
video_full_range: false,
},
"E.2.1 inference: 'unspecified' code points + limited range, never a raw 0"
);
}
#[test]
fn an_explicit_colour_description_rides_the_plan_and_follows_a_new_sps() {
// BT.2020/PQ HDR signalling — the in-band switch the Windows host emits.
let hdr = sps_with_vui_colour(true, false, Some((9, 16, 9)));
let plan = plan_one_idr(&hdr);
assert_eq!(
plan.picture.colour,
ColourDescription {
colour_primaries: 9,
transfer_characteristics: 16,
matrix_coefficients: 9,
video_full_range: false,
}
);
// The colour must track the SPS active for EACH picture, not the
// session's first: an SDR stream renegotiated to HDR mid-stream (same
// SPS id, new content, SPS+PPS in-band at the IDR — the parser's Pps
// snapshots its SPS at PPS-parse time, and hosts re-send both exactly
// so the new content activates) flips at the very next planned picture.
let (sdr_sps, sdr_pps) = authored_sps_pps();
let mut au0 = param_set_au(&sdr_sps, &sdr_pps);
au0.extend(write_idr_slice());
let mut planner = H264Planner::new();
let plan0 = planner.plan_au(&au0).unwrap();
assert_eq!(plan0.picture.colour.matrix_coefficients, 2);
let hdr_pps = PpsBuilder::new(Rc::clone(&hdr))
.pic_parameter_set_id(0)
.pic_init_qp(26)
.build();
let mut au1 = param_set_au(&hdr, &hdr_pps);
au1.extend(write_idr_slice());
let plan1 = planner.plan_au(&au1).unwrap();
assert_eq!(
plan1.picture.colour.matrix_coefficients, 9,
"the replacing SPS's colour lands on its own picture, not latched"
);
}
#[test]
fn a_vui_without_colour_description_keeps_unspecified_but_honours_the_range_flag() {
// video_signal_type present, full-range set, but NO colour description:
// the code points stay E.2.1's "unspecified" while the range flag rides.
let plan = plan_one_idr(&sps_with_vui_colour(true, true, None));
assert_eq!(
plan.picture.colour,
ColourDescription {
colour_primaries: 2,
transfer_characteristics: 2,
matrix_coefficients: 2,
video_full_range: true,
}
);
}
#[test]
fn a_malformed_nalu_mid_au_truncates_with_a_warning_keeping_prior_slices() {
let (sps, pps) = authored_sps_pps();
+8 -6
View File
@@ -822,12 +822,14 @@ fn pump(
// every frame (its decode really is done by now).
let hw_fence = match &image {
DecodedImage::VkFrame(v) => Some((v.timeline_sem, v.decode_done_value)),
// DecodedImage::NativeVk carries the same (semaphore,
// value) pair and COULD feed this sampled decode-time
// stat identically — deliberately deferred to WP-D:
// the native rung's field A/B should measure the same
// stats surface the FFmpeg rung had at parity time,
// and grow new ones after the verdict, not during it.
// The native rung's frame carries the same pair: the
// decode signals `semaphore_value` when the pixels are
// ready (the presenter's write-back is the `+ 1`), so
// waiting it measures received→decode-complete exactly
// like the AVVkFrame arm. Fed since the WP-D hardware
// verdict landed (bit-exact parity, both DPB modes) —
// one stats surface across both Vulkan rungs.
DecodedImage::NativeVk(f) => Some((f.semaphore, f.semaphore_value)),
_ => None,
};
if present {
+9 -4
View File
@@ -256,9 +256,11 @@ pub struct NativeVkFrame {
/// wrong window — carried so that assumption is checkable, not silent.
pub crop_x: u32,
pub crop_y: u32,
/// Colour signalling. The native H.264 path serves the SDR envelope; the backend
/// fills H.273 "unspecified" code points, which every consumer already resolves to
/// the BT.709-limited SDR default (`csc_rows`' documented fallback).
/// Colour signalling, read from the SPS active for THIS picture (H.264 VUI →
/// H.273 code points, with E.2.1's "unspecified" inference where the VUI is
/// silent) — per frame, like the FFmpeg rungs' AVFrame CICP, because the host
/// switches HDR in-band; "unspecified" resolves to the BT.709-limited SDR
/// default (`csc_rows`' documented fallback).
pub color: ColorDesc,
/// IDR — the stream's re-anchor point (the pump's post-loss resume signal).
pub keyframe: bool,
@@ -877,10 +879,13 @@ impl Decoder {
}
/// Wait for a Vulkan-Video frame's GPU decode to complete (timeline semaphore) —
/// the pump's decode-stat measurement. `false` = not the Vulkan backend, or timeout.
/// the pump's decode-stat measurement. `false` = not a Vulkan backend, timeout, or
/// (native rung) a pair no longer in the shipped ledger / a stale session
/// generation — every false just declines the sample.
pub fn wait_hw_decoded(&self, timeline_sem: u64, value: u64, timeout_ns: u64) -> bool {
match &self.backend {
Backend::Vulkan(v) => v.wait_timeline(timeline_sem, value, timeout_ns),
Backend::NativeVulkan(d) => d.wait_timeline(timeline_sem, value, timeout_ns),
_ => false,
}
}
+45 -12
View File
@@ -286,13 +286,17 @@ impl NativeVulkanDecoder {
coded_height: frame.coded_height,
crop_x: frame.crop.x,
crop_y: frame.crop.y,
// H.273 "unspecified" — every consumer resolves it to the BT.709-limited
// SDR default, the native H.264 envelope's colour contract (`csc_rows`).
// H.273 code points straight off the picture's ACTIVE SPS — per frame,
// never latched, because the Windows host switches an HDR desktop to
// PQ/BT.2020 IN-BAND (the Welcome still says SDR). pf-bitstream applies
// E.2.1's "unspecified" inference (2/2/2, limited) where the VUI is
// silent, and `csc_rows` resolves "unspecified" to its BT.709-limited
// SDR default — same verdicts libavcodec's CICP passthrough produced.
color: ColorDesc {
primaries: 2,
transfer: 2,
matrix: 2,
full_range: false,
primaries: frame.colour.colour_primaries,
transfer: frame.colour.transfer_characteristics,
matrix: frame.colour.matrix_coefficients,
full_range: frame.colour.video_full_range,
},
keyframe: frame.is_idr,
poc: frame.poc,
@@ -315,6 +319,20 @@ impl NativeVulkanDecoder {
native
}
/// Bounded wait for a shipped frame's decode-complete signal — the pump's
/// sampled decode-latency stat (`Decoder::wait_hw_decoded`), one frame per
/// stats window. The raw pair names a frame still in the shipped ledger (the
/// pump waits on the same thread that just shipped it, before any settle
/// could retire it); the ledger lookup is the liveness proof — an unreleased
/// frame pins its pool, so a pair matching nothing (already settled, or a
/// stray) just declines the sample instead of touching unknown handles.
pub(crate) fn wait_timeline(&self, sem: u64, value: u64, timeout_ns: u64) -> bool {
self.outstanding
.iter()
.find(|s| s.frame.semaphore.as_raw() == sem && s.frame.value == value)
.is_some_and(|s| self.dec.wait_decoded(&s.frame, timeout_ns))
}
/// Drain the release channel, marking returned frames (release itself waits for
/// the status read — see [`Self::settle_statuses`]).
fn drain_releases(&mut self) {
@@ -346,9 +364,12 @@ impl NativeVulkanDecoder {
continue;
}
// A session rebuild (stream renegotiation) already made this frame stale:
// its pools are gone and a status poll would read the conservative Failed
// — which is NOT driver corruption. Resolve it quietly; the rebuild rode
// an IDR, so the stream has its re-anchor already.
// its SESSION objects (query pool included) are gone — the picture pool
// lives on in the decoder's graveyard while we hold the image, but the
// query verdict is unknowable and poll_status would report the
// conservative Failed — which is NOT driver corruption. Resolve it
// quietly; the rebuild rode an IDR, so the stream has its re-anchor
// already.
if s.frame.generation != dec.generation() {
tracing::debug!(
poc = s.frame.poc,
@@ -392,8 +413,11 @@ impl NativeVulkanDecoder {
}
match dec.release_frame(&s.frame, s.presented) {
Ok(()) => {}
// A session rebuild (stream renegotiation) already dropped the pools
// this frame indexed — nothing left to release.
// Not a best-effort no-op: stale-generation frames release into the
// decoder's graveyard (a rebuild retires a still-held pool INTACT,
// and this very call is what lets it die on its last token). An Err
// is therefore a bookkeeping ghost — a double release — never a
// held image left dangling.
Err(e) => tracing::debug!(error = %e, "release_frame: {e}"),
}
false
@@ -425,7 +449,10 @@ impl Drop for NativeVulkanDecoder {
// the decoder's Drop destroys the pool images: a returned token proves the
// sampling submission's fence was waited, i.e. no GPU work of the
// presenter's still reads the pools (the decoder's own drain covers only
// decode work; graveyarded pools ride the same token contract).
// decode work). Graveyarded pools ride the same token contract — a
// mid-stream renegotiation retires a still-held pool INTACT, and the
// release calls below route stale-generation frames into the graveyard,
// so those pools too die only once their last presenter fence was waited.
let deadline = Instant::now() + TEARDOWN_BUDGET;
loop {
self.drain_releases();
@@ -499,6 +526,12 @@ mod tests {
coded_width: 1920,
coded_height: 1088,
crop: pf_bitstream_crop(1920, 1080),
colour: pf_vkdecode::ColourDescription {
colour_primaries: 2,
transfer_characteristics: 2,
matrix_coefficients: 2,
video_full_range: false,
},
semaphore: vk::Semaphore::null(),
value: 0,
poc: 0,
+48
View File
@@ -37,6 +37,7 @@ use std::collections::VecDeque;
use ash::vk;
use ash::vk::native as hh;
use pf_bitstream::h264::AuPlan;
use pf_bitstream::h264::ColourDescription;
use pf_bitstream::h264::DisplayCrop;
use pf_bitstream::h264::DpbUpdate;
use pf_bitstream::h264::H264Planner;
@@ -125,6 +126,10 @@ pub struct DecodedVkFrame {
pub coded_height: u32,
/// Conformance-window crop: the region to display.
pub crop: DisplayCrop,
/// Colour signalling from the picture's ACTIVE SPS (pf-bitstream applies
/// E.2.1's "unspecified" inference where the VUI is silent). Per frame, like
/// [`Self::crop`]: the host switches HDR in-band with a new SPS mid-stream.
pub colour: ColourDescription,
/// Timeline pair: pixels ready at `semaphore >= value`; the sampling
/// consumer signals `value + 1` (see the type docs).
pub semaphore: vk::Semaphore,
@@ -409,6 +414,7 @@ struct PendingPic {
/// The image's timeline value the decode signalled (frame readiness).
timeline_value: u64,
crop: DisplayCrop,
colour: ColourDescription,
poc: i32,
is_idr: bool,
}
@@ -733,6 +739,7 @@ impl VkH264Decoder {
query_slot: query_index,
timeline_value: signal_value,
crop: plan.picture.display_crop,
colour: plan.picture.colour,
poc: plan.picture.pic_order_cnt,
is_idr: plan.picture.is_idr,
},
@@ -981,6 +988,30 @@ impl VkH264Decoder {
}
}
/// Wait — bounded by `timeout_ns` — for a delivered frame's decode-complete
/// signal ([`DecodedVkFrame::semaphore`] reaching [`DecodedVkFrame::value`]).
/// Pure measurement (the integration layer's sampled decode-latency stat):
/// touches no decoder state, so a timeout or error only degrades the stat —
/// the consumer's own GPU wait is what gates sampling, never this. `frame`
/// must be unreleased (`release_frame` still owed), which pins its pool — and
/// with it the semaphore — alive, graveyarded generations included; a
/// stale-generation frame declines rather than block on a verdict the
/// rebuild's drain already implied.
pub fn wait_decoded(&self, frame: &DecodedVkFrame, timeout_ns: u64) -> bool {
if frame.generation != self.generation {
return false;
}
let semaphores = [frame.semaphore];
let values = [frame.value];
let info = vk::SemaphoreWaitInfo::default()
.semaphores(&semaphores)
.values(&values);
// SAFETY: live device (constructor contract); the semaphore is a pool
// semaphore the unreleased frame keeps alive (fn docs); the info arrays
// are locals outliving the call.
unsafe { self.dev.ash().wait_semaphores(&info, timeout_ns) }.is_ok()
}
/// Drain the planner (teardown / stream discontinuity): every buffered
/// picture becomes display-ready via [`Self::take_ready`] (zero-copy — the
/// images already hold the content), all DPB slots free, and any picture
@@ -1050,6 +1081,22 @@ impl VkH264Decoder {
/// retiring its picture pool to the graveyard when the consumer still holds
/// images) and build a fresh one shaped by `plan`, bumping
/// [`Self::generation`] so frames of the old one route to the graveyard.
///
/// Why a mid-stream rebuild is safe against presenter-held frames (the
/// renegotiation-teardown question, settled):
/// - **Images**: a pool with consumer holds retires to the graveyard INTACT —
/// images, views and semaphores stay live until `release_frame` takes its
/// last token, and a token is sent only after the presenter's sampling
/// submission's fence was waited (its `value+1` write-back included), so no
/// pool image is ever destroyed under in-flight GPU reads.
/// - **Tokens**: every frame and its token carry the generation they were
/// born under, and `release_frame` routes strictly by it (current pool vs
/// graveyard entry), so releases cannot alias across generations.
/// - **Session objects**: the session/ring/ops (query pool included) DO die
/// right here — but only after [`Self::drain_gpu`], and no consumer-facing
/// handle points at them: [`DecodedVkFrame`] borrows pool resources only,
/// and `poll_status` generation-gates before it would touch the NEW
/// generation's query pool with an old frame's slot.
fn rebuild_state(&mut self, plan: &AuPlan) -> Result<(), VkDecodeError> {
self.drain_gpu()?;
if let Some(state) = self.state.take() {
@@ -1251,6 +1298,7 @@ fn build_frame(state: &mut SessionState, entry: &PendingPic, generation: u64) ->
coded_width: state.image_extent.width,
coded_height: state.image_extent.height,
crop: entry.crop,
colour: entry.colour,
semaphore: picture.semaphore,
value: entry.timeline_value,
poc: entry.poc,
+6 -3
View File
@@ -62,10 +62,13 @@ pub mod slots;
/// flattens them to raw `u64`s through `ash::vk::Handle` — via THIS instance of ash, so
/// the versions can never skew.
pub use ash;
/// Re-exported so [`VkH264Decoder::take_warnings`] consumers name the warning type —
/// and [`DecodedVkFrame::crop`]'s type — without growing a pf-bitstream dependency of
/// their own.
// The pf-bitstream types a [`DecodedVkFrame`] consumer names, re-exported so it
// doesn't grow a pf-bitstream dependency of its own:
/// [`DecodedVkFrame::colour`]'s type.
pub use pf_bitstream::h264::ColourDescription;
/// [`DecodedVkFrame::crop`]'s type.
pub use pf_bitstream::h264::DisplayCrop;
/// [`VkH264Decoder::take_warnings`]'s warning type.
pub use pf_bitstream::h264::PlanWarning;
pub use caps::derive_caps;