From 43e713ecca7ad05656bd9303e542ba99e448a9b0 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 7 Aug 2026 13:46:28 +0200 Subject: [PATCH 1/8] fix(client): the probe now accounts for every bit it prints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First hardware run of --probe-decode, on the RTX 5070 Ti: driver decode ops: H.264, H.265, AV1 (0xF) Three names, four bits. 0xF is H.264|H.265|AV1|VP9 — bit 3 is VK_VIDEO_CODEC_OPERATION_DECODE_VP9_BIT_KHR, a real decode operation this client has no rung for, so the name table stopped short of it and the line looked complete while silently dropping a codec the driver had advertised. That is the exact failure this flag exists to prevent. The whole point of --probe-decode is that a reader can trust the words to cover the number; a mask with an unexplained bit asks them to trust it instead. VP9 is now named (marked as having no punktfunk rung, because advertising it as decodable would be its own lie), and any bit beyond the four we know prints as "unrecognised bits 0x…" rather than vanishing — so the next codec Khronos adds shows up as an unknown rather than as nothing at all. Gates: fmt clean; clippy -D warnings on punktfunk-client-session. --- clients/session/src/main.rs | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/clients/session/src/main.rs b/clients/session/src/main.rs index b1750633..ee11a1dc 100644 --- a/clients/session/src/main.rs +++ b/clients/session/src/main.rs @@ -561,11 +561,29 @@ mod session_main { " vulkan video decode: {}", if a.usable { "YES" } else { "no" } ); - let codecs: Vec<&str> = [(0x1u32, "H.264"), (0x2, "H.265"), (0x4, "AV1")] + // Name every bit, and ACCOUNT for the ones we cannot name. The + // 5070 Ti reports 0xF — four bits — while punktfunk decodes three + // codecs, so the first version of this line printed three names + // beside a four-bit mask and looked complete. VP9 (bit 3) is a + // real decode operation this client has no rung for; a codec the + // tool cannot name must not silently vanish from a mask it prints, + // or the reader is left to trust that the words cover the number. + const OPS: [(u32, &str); 4] = [ + (0x1, "H.264"), + (0x2, "H.265"), + (0x4, "AV1"), + (0x8, "VP9 (no punktfunk rung)"), + ]; + let mut codecs: Vec = OPS .iter() .filter(|(bit, _)| a.codec_ops & bit != 0) - .map(|(_, n)| *n) + .map(|(_, n)| (*n).to_string()) .collect(); + let named: u32 = OPS.iter().map(|(b, _)| b).sum(); + let unknown = a.codec_ops & !named; + if unknown != 0 { + codecs.push(format!("unrecognised bits 0x{unknown:X}")); + } println!( " driver decode ops: {}", if codecs.is_empty() { From c0f8f051c31ad9fb2d11f5cd5ff43ab1e26e8b62 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 7 Aug 2026 13:52:23 +0200 Subject: [PATCH 2/8] fix(client): the probe printed a device index the env var does not take MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --probe-decode printed its DISPLAY position and called it the PUNKTFUNK_VK_DEVICE value. It is not. pick_device resolves that variable against the RAW vkEnumeratePhysicalDevices order (setup.rs, `devices.get(i)`) BEFORE any ranking runs, while the probe sorts discrete-first for readability. Those two orders disagree precisely on the hardware this flag exists to diagnose. pick_device's own comment records why the ranking is there: "enumeration order puts the iGPU FIRST on some hybrids (observed: Ryzen iGPU ahead of an RTX dGPU)". So on a hybrid laptop the number the probe printed for the iGPU could well be the number for the dGPU — a diagnostic handing out an actionable value that selects the other GPU, which is worse than printing none. Measured on the Arc + RTX 3500 Ada laptop, which is also where the first output went out with the wrong claim in it: three adapters, and the same Arc iGPU enumerated TWICE. So AdapterDecode now carries the raw enumeration index, captured before the sort, and the printer uses it; the "default presenter" marker stays on the first LISTED entry, because sorted-first is what pick_device lands on when nothing overrides. The duplicate is why the trailing hint names PUNKTFUNK_VK_ADAPTER as the safer knob and admits its limit: two adapters sharing a marketing name cannot be told apart by it, and a name match resolves to whichever enumerates first. The hint also states the thing this whole output invites a reader to get wrong — that a capable GPU in the list does not mean the decoder will use it, because Vulkan Video decodes on the presenter's device and PUNKTFUNK_DECODER does not move the presenter. Gates: fmt clean; clippy -D warnings on punktfunk-client-session and pf-presenter. --- clients/session/src/main.rs | 33 +++++++++++++++++++++++++++-- crates/pf-presenter/src/vk/setup.rs | 19 +++++++++++++++-- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/clients/session/src/main.rs b/clients/session/src/main.rs index ee11a1dc..88a8d46a 100644 --- a/clients/session/src/main.rs +++ b/clients/session/src/main.rs @@ -548,12 +548,21 @@ mod session_main { println!("no Vulkan physical devices"); } for (i, a) in adapters.iter().enumerate() { - // The index IS the PUNKTFUNK_VK_DEVICE value, and entry 0 is what + // The bracketed number is the PUNKTFUNK_VK_DEVICE value, and the + // FIRST listed entry is what // a default run presents on — the decoder shares that device, so // on a hybrid box this line is usually the answer. let kind = if a.discrete { "discrete" } else { "integrated" }; + // `a.index`, NOT the loop position. This list is sorted + // discrete-first for reading, but PUNKTFUNK_VK_DEVICE indexes the + // raw enumeration, which puts the iGPU first on some hybrids — + // printing the loop position would name the other GPU on exactly + // the machines this flag is for. The `i == 0` marker is still the + // loop position, because sorted-first IS what pick_device lands on + // when nothing overrides it. println!( - "[{i}] {} ({kind}){}", + "[{}] {} ({kind}){}", + a.index, a.name, if i == 0 { " <- default presenter" } else { "" } ); @@ -620,6 +629,26 @@ mod session_main { println!(" extensions: {}", a.codec_exts.join(", ")); } } + if adapters.len() > 1 { + // The single most common misreading of this output: seeing a + // capable GPU listed and concluding the decoder will use it. + // Vulkan Video decodes on the PRESENTER's device, and the decoder + // preference does not move the presenter. + println!(); + println!( + "Vulkan Video decodes on the presenter's device. PUNKTFUNK_DECODER \ + picks the rung," + ); + println!( + "not the GPU — move the presenter with PUNKTFUNK_VK_DEVICE= or" + ); + println!( + "PUNKTFUNK_VK_ADAPTER=, which is the safer knob \ + where two" + ); + println!("adapters share a name."); + } 0 } Err(e) => { diff --git a/crates/pf-presenter/src/vk/setup.rs b/crates/pf-presenter/src/vk/setup.rs index 33ad5ce6..64f59d7a 100644 --- a/crates/pf-presenter/src/vk/setup.rs +++ b/crates/pf-presenter/src/vk/setup.rs @@ -55,7 +55,19 @@ pub(crate) fn video_decode_gate( /// hardware decode starts here: WHICH adapter, and does it advertise the codec. #[derive(Debug, Clone)] pub struct AdapterDecode { - /// Marketing name — also the `PUNKTFUNK_VK_ADAPTER` match key. + /// The device's position in the RAW `vkEnumeratePhysicalDevices` order — and + /// therefore the value `PUNKTFUNK_VK_DEVICE` takes, because `pick_device` indexes the + /// unsorted list (`devices.get(i)`) before any ranking runs. + /// + /// ⚠ NOT the display position. This list is sorted discrete-first for readability, + /// while enumeration order puts the iGPU first on some hybrids — so the two disagree + /// on exactly the machines this probe exists to diagnose. Printing the display + /// position as if it were the env value would hand a hybrid-laptop reporter the + /// number for the other GPU. + pub index: usize, + /// Marketing name — also the `PUNKTFUNK_VK_ADAPTER` match key. Not necessarily + /// unique: a hybrid can expose the same iGPU twice, and a name match then resolves to + /// whichever enumerates first. pub name: String, /// Discrete GPUs sort first, exactly as `pick_device` ranks them, so index 0 here is /// the device a default run will pick. @@ -743,7 +755,9 @@ pub fn probe_decode() -> Result> { // filling locals returned by value. let devices = unsafe { instance.enumerate_physical_devices() }?; let mut out: Vec<(u8, AdapterDecode)> = Vec::with_capacity(devices.len()); - for pdev in devices { + // `enumerate()` BEFORE any filtering or sorting: this index is what + // `PUNKTFUNK_VK_DEVICE` selects, so it has to survive both. + for (raw_index, pdev) in devices.into_iter().enumerate() { // SAFETY: per the Vulkan contract above - a read-only query on the live // instance/device, filling locals returned by value. let props = unsafe { instance.get_physical_device_properties(pdev) }; @@ -834,6 +848,7 @@ pub fn probe_decode() -> Result> { out.push(( rank, AdapterDecode { + index: raw_index, name, discrete: rank == 0, api_1_3, From c34e1412fb58ad0d54739d798e1059672ba42168 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 7 Aug 2026 14:34:59 +0200 Subject: [PATCH 3/8] fix(client): a decoder pin with a stray space was silently ignored MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found on glass, and it cost a whole session to find. PUNKTFUNK_DECODER was read untrimmed, so "native-vulkan " — ONE trailing space — matched no arm of native_vulkan_gate, fell through to `auto`, and on an Intel box `auto` takes d3d11va first. The operator's pin never ran and NOTHING said so. Read against a log, that is indistinguishable from the rung being refused for a hardware reason, which is precisely the ambiguity the rest of this module's logging was just rewritten to remove. The space is not exotic. A Windows .cmd produces it for free: `echo x>> file` keeps the space before the redirect, so every line written that way carries one. PUNKTFUNK_VK_ADAPTER already trimmed; this did not, and the inconsistency is what made it invisible — the GPU override obeyed while the decoder override did not. The rule now lives in one pure function, resolve_decoder_pref, called by BOTH readers. decode_pinned_to_software had the identical untrimmed expression, and its own doc comment says a second reading of the same two inputs is a second place for them to drift — fixing one and not the other would have proved it right. Whitespace-only counts as ABSENT rather than as a pin to "", because an exported-but-empty variable means "no override" and "" is a value the gate happens to accept. Tested as a pure rule (no process environment), including the end-to-end leg that matters: the trimmed pin reaches native_vulkan_gate and is admitted. Like the create-array tests in dee97e89 its before-state is a compile error rather than a failing assertion, because the function is new — what it guards going forward is real, and an editor who drops the trim fails it. Gates: fmt clean; clippy -D warnings over pf-client-core, punktfunk-client-session and pf-presenter in the Linux container; 164 pf-client-core tests. --- crates/pf-client-core/src/video.rs | 75 ++++++++++++++++++++++++++---- 1 file changed, 67 insertions(+), 8 deletions(-) diff --git a/crates/pf-client-core/src/video.rs b/crates/pf-client-core/src/video.rs index 86bd73b3..e20237df 100644 --- a/crates/pf-client-core/src/video.rs +++ b/crates/pf-client-core/src/video.rs @@ -1340,11 +1340,30 @@ pub fn migrate_decoder_pref(pref: &str) -> String { /// Same precedence as [`Decoder::new`] resolves (env first, then the setting), because a /// second reading of the same two inputs is a second place for them to drift. pub fn decode_pinned_to_software(pref: &str) -> bool { - std::env::var("PUNKTFUNK_DECODER") - .ok() + resolve_decoder_pref(std::env::var("PUNKTFUNK_DECODER").ok().as_deref(), pref) == "software" +} + +/// Resolve the decoder preference: the `PUNKTFUNK_DECODER` override if it carries a +/// value, else the stored setting. Pure, so the rule is testable without touching the +/// process environment — and shared, because [`Decoder::new`] and +/// [`decode_pinned_to_software`] read the same two inputs and a second reading is a +/// second place for them to drift. +/// +/// **Trimmed**, which is the part that had to be fixed rather than merely factored out. +/// `PUNKTFUNK_VK_ADAPTER` already trimmed; this did not, so `"native-vulkan "` — one +/// trailing space, which a Windows `.cmd` produces for free because `echo x>> file` +/// keeps the space before the redirect — matched no arm of [`native_vulkan_gate`] and +/// fell through to `auto` SILENTLY. An operator's pin was ignored and nothing said so, +/// which is the exact failure the rest of this module's logging exists to prevent. It +/// cost a full on-glass session to find. +/// +/// Whitespace-only is treated as absent, not as a pin to `""`: someone who exported the +/// variable empty means "no override", and `""` is a value `native_vulkan_gate` happens +/// to accept. +pub(crate) fn resolve_decoder_pref(env: Option<&str>, pref: &str) -> String { + env.map(str::trim) .filter(|v| !v.is_empty()) - .unwrap_or_else(|| pref.to_string()) - == "software" + .map_or_else(|| pref.to_string(), str::to_string) } /// The `quic` codec bitfield this client can decode — the union of the codecs the RUNGS @@ -1609,10 +1628,7 @@ impl Decoder { vk: Option<&VulkanDecodeDevice>, stream: StreamFormat, ) -> Result { - let stored = std::env::var("PUNKTFUNK_DECODER") - .ok() - .filter(|v| !v.is_empty()) - .unwrap_or_else(|| pref.to_string()); + let stored = resolve_decoder_pref(std::env::var("PUNKTFUNK_DECODER").ok().as_deref(), pref); let choice = migrate_decoder_pref(&stored); if choice != stored { // Said once per session, at `warn`, because a developer who set the env var to @@ -3006,6 +3022,49 @@ mod tests { /// device leg: admitting HEVC on an H.264-only decode family would create a video /// session for an operation the family cannot run, which is undefined behaviour /// rather than an error. + /// A pin with stray whitespace is still a pin, and the gate must accept it. + /// + /// This is a regression test with a field cost: `"native-vulkan "` (one trailing + /// space, which a Windows `.cmd` adds for free) matched no arm of + /// `native_vulkan_gate`, so the rung fell through to `auto` with nothing logged — + /// on a box where `auto` picks a different rung, that reads exactly like the pin + /// being refused for a hardware reason. The second half is what makes it a *shared* + /// rule: `decode_pinned_to_software` reads the same variable, and its own docs say + /// a second reading is a second place to drift. + #[test] + fn a_decoder_pin_survives_the_whitespace_a_shell_script_adds() { + assert_eq!( + resolve_decoder_pref(Some("native-vulkan "), "auto"), + "native-vulkan", + "a trailing space must not turn a pin into an unrecognised value" + ); + assert_eq!( + resolve_decoder_pref(Some(" software\t"), "auto"), + "software" + ); + // Trimmed to nothing means ABSENT — fall back to the stored setting rather than + // pinning to "", which the gate would otherwise accept as the auto family. + assert_eq!( + resolve_decoder_pref(Some(" "), "native-vaapi"), + "native-vaapi" + ); + assert_eq!( + resolve_decoder_pref(Some(""), "native-vaapi"), + "native-vaapi" + ); + assert_eq!(resolve_decoder_pref(None, "native-vaapi"), "native-vaapi"); + // …and the trimmed value is what the gate actually admits. + assert!( + native_vulkan_gate( + &resolve_decoder_pref(Some("native-vulkan "), "auto"), + punktfunk_core::quic::CODEC_HEVC, + true, + VIDEO_CODEC_OP_DECODE_H265, + ), + "the whole point: the trimmed pin reaches the gate and is admitted" + ); + } + #[test] fn native_vulkan_gate_admits_pin_and_auto_family_per_codec_on_a_capable_family() { // Pin the raw spec values, not the implementation constants — a typo'd bit From fb1a0a61e92d304760414eabae214883ed6311b2 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 7 Aug 2026 14:50:08 +0200 Subject: [PATCH 4/8] diag(vkdecode): log the driver's video capabilities verbatim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing in the caps module logged anything, so when a device refused with "advertises neither DPB_AND_OUTPUT_COINCIDE nor DISTINCT" there was no way to separate two very different situations that present identically as a zero: the driver filling the chain and genuinely declaring no DPB mode, versus our own pNext chain never reaching VkVideoDecodeCapabilitiesKHR at all. Printing the BASE VkVideoCapabilitiesKHR beside the decode flags is the discriminator. A populated max_dpb_slots next to decode_flags: 0 means the driver traversed the chain and answered; zeros across both mean the query never landed and the refusal is ours, not the driver's. Raised by the Intel Arc result on .221, where I concluded "driver bug" on the strength of our own code's report — which is precisely the circular reasoning this line exists to break. --- crates/pf-vkdecode/src/caps_h265.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/crates/pf-vkdecode/src/caps_h265.rs b/crates/pf-vkdecode/src/caps_h265.rs index 901f6414..bed9cce1 100644 --- a/crates/pf-vkdecode/src/caps_h265.rs +++ b/crates/pf-vkdecode/src/caps_h265.rs @@ -308,6 +308,26 @@ pub(crate) unsafe fn query_h265_caps( let decode_flags = decode_caps.flags; let max_level_idc = h265_caps.max_level_idc; + // What the driver ACTUALLY said, before any of our interpretation. Nothing in this + // module logged, so a refusal downstream ("advertises neither COINCIDE nor DISTINCT") + // was indistinguishable from our own chain never reaching the struct: both present as + // a zero. Printing the BASE capabilities beside the decode ones is the discriminator — + // a populated `max_dpb_slots` next to `decode_flags: 0` means the driver filled the + // chain and genuinely declared no DPB mode; zeros across both mean the query never + // landed. Debug rather than info: one line per profile per session, wanted only when + // someone is asking this exact question. + tracing::debug!( + codec = "H.265", + ?capability_flags, + ?decode_flags, + max_dpb_slots, + max_active_reference_pictures, + ?min_coded_extent, + ?max_coded_extent, + ?picture_access_granularity, + "driver video capabilities, verbatim" + ); + // The three queries carry the REAL creation usages (SAMPLED included for the // presenter-facing roles) so the answers validate the images the pools build. let decode_profile = DecodeProfile::H265(key); From a183cac8aad9a3163bec256a23b7f8d4501cf193 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 7 Aug 2026 14:55:27 +0200 Subject: [PATCH 5/8] diag(vkdecode): log maxLevelIdc beside the decode flags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Arc returned decode_flags=0b1100 = 12 with a fully populated base struct (15 DPB slots, 8192x8192 max extent). Neither COINCIDE (0x1) nor DISTINCT (0x2) is set, and 0x4|0x8 are not defined for that field at all — but 12 IS STD_VIDEO_H265_LEVEL_IDC_6_2, and VkVideoDecodeCapabilitiesKHR and VkVideoDecodeH265CapabilitiesKHR have identical layouts (sType, pNext, one u32). So the suspicion is that we are reading H.265's maxLevelIdc where the decode flags belong. Logging both settles it: if max_level_idc comes back as 1 or 2 the two structs are crossed, and the refusal is ours rather than the driver's. --- crates/pf-vkdecode/src/caps_h265.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/pf-vkdecode/src/caps_h265.rs b/crates/pf-vkdecode/src/caps_h265.rs index bed9cce1..d19852e7 100644 --- a/crates/pf-vkdecode/src/caps_h265.rs +++ b/crates/pf-vkdecode/src/caps_h265.rs @@ -320,6 +320,8 @@ pub(crate) unsafe fn query_h265_caps( codec = "H.265", ?capability_flags, ?decode_flags, + decode_flags_raw = decode_flags.as_raw(), + max_level_idc, max_dpb_slots, max_active_reference_pictures, ?min_coded_extent, From ca667cb79ae8e2f03b36279f9d86f16973397e46 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 7 Aug 2026 15:07:48 +0200 Subject: [PATCH 6/8] fix(vkdecode): the pNext order decided which struct got the decode caps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Intel Arc never used Vulkan Video decode on Windows. The rung refused every session with "driver advertises neither DPB_AND_OUTPUT_COINCIDE nor DISTINCT" and fell back to D3D11VA — and that refusal was ours. vkGetPhysicalDeviceVideoCapabilitiesKHR was called with the codec capability struct chained BEFORE VkVideoDecodeCapabilitiesKHR (push_next prepends, so the chain was caps -> h265_caps -> decode_caps). On Arc/Windows 101.8724 the driver fills those two by POSITION, not by sType, and returned them SWAPPED. Measured, on glass, both ways: before: decode_flags_raw=12 max_level_idc=1 after: decode_flags_raw=1 max_level_idc=12 12 is STD_VIDEO_H265_LEVEL_IDC_6_2 and 1 is DPB_AND_OUTPUT_COINCIDE. We were reading an H.265 level as a decode-capability bitmask; 12 contains neither 0x1 nor 0x2, so the check concluded the device had no DPB mode. It had one all along. The base struct was fully populated throughout — 15 DPB slots, 8192x8192 max extent — which is what gave the lie away: a driver that answers in that much detail is not declining. NVIDIA and RADV dispatch by sType and do not care about the order, which is exactly why the fleet stayed green and this reached the field. Both orders are spec-legal for us to write; only one survives a driver that assumes the conventional one, and the conventional one — decode caps first, as every Vulkan sample writes it — is now what all three codecs use. ⚠ This does NOT yet give the Arc Vulkan Video. It moves the refusal one step down the same function: the device advertises only COINCIDE (no DISTINCT), and its NV12 coincide entry does not advertise SAMPLED usage, which the zero-copy presenter path needs. Whether that is a second bug of ours or a real Intel constraint is not yet established, and this commit does not claim it either way. Found because the user disbelieved my "Intel driver bug" conclusion. He was right: I had reasoned from our own error message, which is the same circularity the caps logging added in fb1a0a61/a183cac8 now exists to break. Gates: fmt clean; clippy -D warnings; 187 pf-vkdecode tests. The GPU parity legs that cover this code cannot run here (no GPU on the build host) — the evidence is the on-glass A/B above. --- crates/pf-vkdecode/src/caps.rs | 7 +++++-- crates/pf-vkdecode/src/caps_av1.rs | 7 +++++-- crates/pf-vkdecode/src/caps_h265.rs | 17 +++++++++++++++-- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/crates/pf-vkdecode/src/caps.rs b/crates/pf-vkdecode/src/caps.rs index e42c89ac..722b1772 100644 --- a/crates/pf-vkdecode/src/caps.rs +++ b/crates/pf-vkdecode/src/caps.rs @@ -605,9 +605,12 @@ pub(crate) unsafe fn query_h264_caps( let mut h264_caps = vk::VideoDecodeH264CapabilitiesKHR::default(); let mut decode_caps = vk::VideoDecodeCapabilitiesKHR::default(); + // ⚠ ORDER IS LOAD-BEARING — see the measured Intel Arc swap in + // [`crate::caps_h265::query_h265_caps`]. `push_next` prepends, so pushing the codec + // struct FIRST leaves VkVideoDecodeCapabilitiesKHR directly after the base struct. let mut caps = vk::VideoCapabilitiesKHR::default() - .push_next(&mut decode_caps) - .push_next(&mut h264_caps); + .push_next(&mut h264_caps) + .push_next(&mut decode_caps); // SAFETY: physical device is live (DeviceHandles contract); `profile` roots a // fully wired, immovable chain; `caps` chains driver-fillable structs that all // outlive the call. diff --git a/crates/pf-vkdecode/src/caps_av1.rs b/crates/pf-vkdecode/src/caps_av1.rs index 40ae6d7a..509ae1b9 100644 --- a/crates/pf-vkdecode/src/caps_av1.rs +++ b/crates/pf-vkdecode/src/caps_av1.rs @@ -278,9 +278,12 @@ pub(crate) unsafe fn query_av1_caps( let mut av1_caps = vk::VideoDecodeAV1CapabilitiesKHR::default(); let mut decode_caps = vk::VideoDecodeCapabilitiesKHR::default(); + // ⚠ ORDER IS LOAD-BEARING — see the measured Intel Arc swap in + // [`crate::caps_h265::query_h265_caps`]. `push_next` prepends, so pushing the codec + // struct FIRST leaves VkVideoDecodeCapabilitiesKHR directly after the base struct. let mut caps = vk::VideoCapabilitiesKHR::default() - .push_next(&mut decode_caps) - .push_next(&mut av1_caps); + .push_next(&mut av1_caps) + .push_next(&mut decode_caps); // SAFETY: physical device is live (DeviceHandles contract); `profile` roots a // fully wired, immovable chain; `caps` chains driver-fillable structs that all // outlive the call. diff --git a/crates/pf-vkdecode/src/caps_h265.rs b/crates/pf-vkdecode/src/caps_h265.rs index d19852e7..a25c92ff 100644 --- a/crates/pf-vkdecode/src/caps_h265.rs +++ b/crates/pf-vkdecode/src/caps_h265.rs @@ -279,9 +279,22 @@ pub(crate) unsafe fn query_h265_caps( let mut h265_caps = vk::VideoDecodeH265CapabilitiesKHR::default(); let mut decode_caps = vk::VideoDecodeCapabilitiesKHR::default(); + // ⚠ ORDER IS LOAD-BEARING on at least one shipping driver. `push_next` PREPENDS, so + // the chain is the reverse of the call order: pushing the codec struct last puts + // VkVideoDecodeCapabilitiesKHR FIRST after the base struct, which is the order every + // Vulkan sample writes it in. + // + // Measured on Intel Arc (Windows 101.8724) with the previous order — codec struct + // first — the driver filled the two by POSITION rather than by sType and returned + // them SWAPPED: `decode_caps.flags` came back 12 (= STD_VIDEO_H265_LEVEL_IDC_6_2) + // and `h265_caps.maxLevelIdc` came back 1 (= DPB_AND_OUTPUT_COINCIDE). Reading a + // level as a flag bitmask means neither COINCIDE nor DISTINCT appeared set, so the + // rung refused a device that in fact supports it, and every Arc fell back to D3D11VA. + // NVIDIA and RADV dispatch by sType and are indifferent to the order, which is why + // the fleet was green and this survived to the field. let mut caps = vk::VideoCapabilitiesKHR::default() - .push_next(&mut decode_caps) - .push_next(&mut h265_caps); + .push_next(&mut h265_caps) + .push_next(&mut decode_caps); // SAFETY: physical device is live (DeviceHandles contract); `profile` roots a // fully wired, immovable chain; `caps` chains driver-fillable structs that all // outlive the call. From c06ee55b61350c02394f6cc9cbf3dd0203898d76 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 7 Aug 2026 15:34:12 +0200 Subject: [PATCH 7/8] diag(vkdecode): --probe-decode reports what the driver says about video images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Intel Arc refusal moved one step down the caps query and stopped again: the coincide NV12 entry does not advertise SAMPLED. That sentence is punktfunk's, not the driver's, and the last two times a conclusion was drawn from a sentence of ours the conclusion was wrong. So --probe-decode now prints the driver's own answers instead. For every profile the client can negotiate (H.264 High, H.265 Main and Main 10, AV1 Main 8- and 10-bit) it asks vkGetPhysicalDeviceVideoFormatPropertiesKHR in six usage combinations — the three the image pools really create with, plus DPB|DST without sampling, SAMPLED alone and DST alone, which are what localise a refusal to a half. Each answer is printed as the driver gave it: format, usage and create flags named AND in hex with unrecognised bits called out, image type, tiling. A failed query prints its VkResult rather than vanishing into an empty list. It goes through pf-vkdecode's own query rather than a copy of it, which meant splitting query_formats into a physical-device form — the call never needed the VkDevice the old signature demanded. VideoFormat gains imageType and imageTiling to carry the whole record; VUID-VkImageCreateInfo-pNext-06811 compares both for equality, so they were being assumed rather than read. And because a driver that under-reports usage would be indistinguishable from one that genuinely lacks it, the probe asks a second, independent question — vkGetPhysicalDeviceImageFormatProperties2 over the same profile list — and prints it only where the two disagree. A disagreement is the finding. No behaviour change to any decode path: derivation reads the same fields it did. --- Cargo.lock | 1 + clients/session/src/main.rs | 56 ++++ crates/pf-presenter/Cargo.toml | 6 + crates/pf-presenter/src/vk/mod.rs | 5 + crates/pf-presenter/src/vk/setup.rs | 21 ++ crates/pf-vkdecode/src/caps.rs | 82 +++++- crates/pf-vkdecode/src/caps_av1.rs | 3 + crates/pf-vkdecode/src/caps_h265.rs | 3 + crates/pf-vkdecode/src/images.rs | 1 + crates/pf-vkdecode/src/lib.rs | 1 + crates/pf-vkdecode/src/probe.rs | 418 ++++++++++++++++++++++++++++ 11 files changed, 585 insertions(+), 12 deletions(-) create mode 100644 crates/pf-vkdecode/src/probe.rs diff --git a/Cargo.lock b/Cargo.lock index 458d5a25..8401cb07 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3196,6 +3196,7 @@ dependencies = [ "ash", "async-channel", "pf-client-core", + "pf-vkdecode", "punktfunk-core", "sdl3", "tracing", diff --git a/clients/session/src/main.rs b/clients/session/src/main.rs index 88a8d46a..d84f031a 100644 --- a/clients/session/src/main.rs +++ b/clients/session/src/main.rs @@ -510,6 +510,61 @@ mod session_main { ); } + /// The driver's own answers about video images, printed with nothing in front of + /// them (`--probe-decode`). + /// + /// Passing the five conjuncts above only says Vulkan Video EXISTS on a device; this + /// says whether the zero-copy pipeline can be BUILT on it — a different question + /// with, on at least one shipping driver, a different answer. Verbatim on purpose: + /// the Intel Arc refusal was twice diagnosed from punktfunk's own error text and + /// twice the diagnosis was wrong, and what broke it open both times was reading what + /// the driver actually said. + fn print_video_formats(a: &pf_presenter::vk::AdapterDecode) { + use pf_presenter::vk::probe::{describe_create_flags, describe_usage}; + for p in &a.formats { + println!(" {} (wants {:?}):", p.profile, p.wanted); + for u in &p.usages { + let answer = match &u.formats { + Err(e) => format!("query failed: {e:?}"), + Ok(entries) if entries.is_empty() => "no formats offered".to_string(), + Ok(entries) => entries + .iter() + .map(|f| { + format!( + "{:?} usage={} create={} {:?} {:?}", + f.format, + describe_usage(f.image_usage), + describe_create_flags(f.image_create_flags), + f.image_type, + f.image_tiling, + ) + }) + .collect::>() + .join("; "), + }; + println!(" {:<24} {answer}", u.label); + // The independent second opinion, printed only where it DISAGREES with + // the format query. Agreement is the normal case and would be noise; a + // disagreement means one of the driver's two paths is wrong, which is + // the whole reason a second question gets asked at all. + let listed = u + .wanted_entry(p.wanted) + .is_some_and(|f| f.image_usage.contains(u.usage)); + if listed != u.image_format_support.is_ok() { + let second = match &u.image_format_support { + Ok(()) => "says creatable".to_string(), + Err(e) => format!("says {e:?}"), + }; + println!( + " {:<24} ^ DISAGREES: \ + vkGetPhysicalDeviceImageFormatProperties2 {second}", + "" + ); + } + } + } + } + pub fn run() -> u8 { // Logs to STDERR — stdout is the machine interface (ready/stats/error lines). tracing_subscriber::fmt() @@ -628,6 +683,7 @@ mod session_main { } else { println!(" extensions: {}", a.codec_exts.join(", ")); } + print_video_formats(a); } if adapters.len() > 1 { // The single most common misreading of this output: seeing a diff --git a/crates/pf-presenter/Cargo.toml b/crates/pf-presenter/Cargo.toml index 5c4fa827..40c789e3 100644 --- a/crates/pf-presenter/Cargo.toml +++ b/crates/pf-presenter/Cargo.toml @@ -17,6 +17,12 @@ repository.workspace = true # C++ in — fatal on Windows ARM64, where Granite has no SIMD path. pf-client-core = { path = "../pf-client-core", default-features = false } punktfunk-core = { path = "../punktfunk-core", features = ["quic"] } +# `--probe-decode` reports the driver's own video-format answers through pf-vkdecode's +# query rather than a second copy of it — a probe that keeps its own copy is a probe +# that eventually disagrees with the code it exists to explain (VIDEO_BASE's doc says +# the same thing about the extension list). Already in the tree via pf-client-core; +# named here because setup.rs calls it directly. +pf-vkdecode = { path = "../pf-vkdecode" } # `loaded` dlopens libvulkan at runtime (no link-time dependency — GPU-less boxes still # start and fail into a clean error; on Windows vulkan-1.dll is a GPU-driver component). diff --git a/crates/pf-presenter/src/vk/mod.rs b/crates/pf-presenter/src/vk/mod.rs index 998d506a..814cb780 100644 --- a/crates/pf-presenter/src/vk/mod.rs +++ b/crates/pf-presenter/src/vk/mod.rs @@ -43,6 +43,11 @@ mod setup; pub use setup::{list_adapters, probe_decode, AdapterDecode, PresentPref}; +/// The video-format probe behind [`AdapterDecode::formats`], re-exported so a caller +/// that prints the report does not need its own `pf-vkdecode` dependency (and cannot +/// end up printing a DIFFERENT crate version's idea of the flag names). +pub use pf_vkdecode::probe; + /// One presenter iteration's video input. pub enum FrameInput<'a> { /// No new frame — re-composite the retained video image (expose/resize). diff --git a/crates/pf-presenter/src/vk/setup.rs b/crates/pf-presenter/src/vk/setup.rs index 64f59d7a..9930ceaf 100644 --- a/crates/pf-presenter/src/vk/setup.rs +++ b/crates/pf-presenter/src/vk/setup.rs @@ -85,6 +85,16 @@ pub struct AdapterDecode { pub codec_exts: Vec, /// [`video_decode_gate`] over the fields above. pub usable: bool, + /// What the driver answers about video image formats, verbatim — one row per + /// (profile, usage) question ([`pf_vkdecode::probe`]). + /// + /// This is the half of the report that says why a device which passes every gate + /// above still cannot host the decoder. The five conjuncts answer "is Vulkan Video + /// here at all"; this answers "can the pipeline actually use it", which on at least + /// one shipping driver (Intel Arc, Windows) is a different question with a different + /// answer. Empty when the gate already failed — there is nothing to ask a device + /// with no video queue. + pub formats: Vec, } /// `VK_EXT_present_mode_fifo_latest_ready`, hand-declared: it postdates the Vulkan headers @@ -845,6 +855,16 @@ pub fn probe_decode() -> Result> { base_missing.is_empty(), !codec_exts.is_empty(), ); + // Only where the gate passed: the format queries need `VK_KHR_video_queue`'s + // entry points, and asking a device that does not expose them produces a null + // dispatch, not an answer. + let formats = if usable { + // SAFETY: `instance` is the live instance created above and `pdev` one of + // the physical devices it enumerated; the probe only reads. + unsafe { pf_vkdecode::probe::probe_video_formats(&entry, &instance, pdev) } + } else { + Vec::new() + }; out.push(( rank, AdapterDecode { @@ -858,6 +878,7 @@ pub fn probe_decode() -> Result> { base_missing, codec_exts, usable, + formats, }, )); } diff --git a/crates/pf-vkdecode/src/caps.rs b/crates/pf-vkdecode/src/caps.rs index 722b1772..e37963c0 100644 --- a/crates/pf-vkdecode/src/caps.rs +++ b/crates/pf-vkdecode/src/caps.rs @@ -86,7 +86,7 @@ pub const COINCIDE_USAGE: vk::ImageUsageFlags = vk::ImageUsageFlags::from_raw( /// plus the driver's advertised usage/create-flag envelope for it — creation must /// stay INSIDE that envelope (finding of the adversarial round: the flags used to /// be assumed, not honoured). -#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct VideoFormat { pub format: vk::Format, /// `imageUsageFlags` the driver supports for this format under the queried @@ -94,7 +94,38 @@ pub struct VideoFormat { pub image_usage: vk::ImageUsageFlags, /// `imageCreateFlags` the driver allows — per-plane views require /// `MUTABLE_FORMAT` to appear here. + /// + /// ⚠ This field is also the ONLY gate on `VK_IMAGE_CREATE_EXTENDED_USAGE_BIT`, + /// which is the spec's one escape hatch from `image_usage`: `supportedVideoFormat` + /// (VUID-VkImageCreateInfo-pNext-06811) admits a usage bit outside `image_usage` + /// only when `VkImageCreateInfo::flags` includes `EXTENDED_USAGE`, and admits that + /// flag only when it is "also set in `VkVideoFormatPropertiesKHR::imageCreateFlags`" + /// (or is `VIDEO_PROFILE_INDEPENDENT`, which needs `VK_KHR_video_maintenance1`). + /// So an EMPTY value here closes the escape hatch as well as the door — measured on + /// Intel Arc, where it is empty for every profile ([`crate::probe`] docs). pub image_create_flags: vk::ImageCreateFlags, + /// `imageType` — the image type this format may be created with. Part of the + /// `supportedVideoFormat` match (VUID-06811 compares it for EQUALITY), so it is + /// recorded rather than assumed; every fleet driver reports `TYPE_2D`, which is + /// what [`crate::images`] creates. + pub image_type: vk::ImageType, + /// `imageTiling` — likewise compared for equality by VUID-06811; every fleet + /// driver reports `OPTIMAL`. + pub image_tiling: vk::ImageTiling, +} + +impl Default for VideoFormat { + /// The shape the pools create with (`TYPE_2D` + `OPTIMAL`), so a fixture that + /// names only the interesting fields still describes a creatable image. + fn default() -> Self { + Self { + format: vk::Format::UNDEFINED, + image_usage: vk::ImageUsageFlags::empty(), + image_create_flags: vk::ImageCreateFlags::empty(), + image_type: vk::ImageType::TYPE_2D, + image_tiling: vk::ImageTiling::OPTIMAL, + } + } } /// Everything the thin query copies out of the driver, hand-buildable for tests. @@ -677,6 +708,36 @@ pub(crate) unsafe fn query_formats( dev: &DecodeDevice, decode_profile: DecodeProfile, usage: vk::ImageUsageFlags, +) -> Result, vk::Result> { + // SAFETY: the caller's DeviceHandles contract makes these two live, which is + // exactly what the physical-device form needs. + unsafe { + query_formats_on( + dev.video_queue_instance(), + dev.physical_device(), + decode_profile, + usage, + ) + } +} + +/// [`query_formats`] against a bare physical device — no `VkDevice` in sight. +/// +/// Split out so [`crate::probe`] enumerates through the SAME code the session's caps +/// query runs, rather than a second copy that would drift (the probe's whole value is +/// that its answer is the one derivation will see). `vkGetPhysicalDeviceVideoFormat- +/// PropertiesKHR` is an instance-level command over a physical device, so nothing here +/// ever needed the logical device the old signature demanded. +/// +/// # Safety +/// +/// `video_queue_instance` must be loaded against a live `VkInstance`, and +/// `physical_device` must be one of that instance's physical devices. +pub(crate) unsafe fn query_formats_on( + video_queue_instance: &ash::khr::video_queue::Instance, + physical_device: vk::PhysicalDevice, + decode_profile: DecodeProfile, + usage: vk::ImageUsageFlags, ) -> Result, vk::Result> { let mut chain = decode_profile.chain(); let profile = chain.wire(); @@ -686,21 +747,13 @@ pub(crate) unsafe fn query_formats( .image_usage(usage) .push_next(&mut profile_list); - let fp = dev - .video_queue_instance() + let fp = video_queue_instance .fp() .get_physical_device_video_format_properties_khr; let mut count = 0u32; // SAFETY: live physical device; `info` roots a wired chain outliving the call; // null properties pointer is the spec's count-query form. - let r = unsafe { - fp( - dev.physical_device(), - &info, - &mut count, - std::ptr::null_mut(), - ) - }; + let r = unsafe { fp(physical_device, &info, &mut count, std::ptr::null_mut()) }; match r { vk::Result::SUCCESS => {} // "This usage/profile combination has no formats" — an arrangement gap, @@ -711,7 +764,7 @@ pub(crate) unsafe fn query_formats( } let mut props = vec![vk::VideoFormatPropertiesKHR::default(); count as usize]; // SAFETY: as above, with a properties array of exactly the driver-reported count. - let r = unsafe { fp(dev.physical_device(), &info, &mut count, props.as_mut_ptr()) }; + let r = unsafe { fp(physical_device, &info, &mut count, props.as_mut_ptr()) }; if r != vk::Result::SUCCESS && r != vk::Result::INCOMPLETE { return Err(r); } @@ -722,6 +775,8 @@ pub(crate) unsafe fn query_formats( format: p.format, image_usage: p.image_usage_flags, image_create_flags: p.image_create_flags, + image_type: p.image_type, + image_tiling: p.image_tiling, }) .collect()) } @@ -738,6 +793,7 @@ mod tests { image_create_flags: vk::ImageCreateFlags::MUTABLE_FORMAT | vk::ImageCreateFlags::ALIAS | vk::ImageCreateFlags::EXTENDED_USAGE, + ..Default::default() } } @@ -783,6 +839,7 @@ mod tests { format: NV12, image_usage: DPB_USAGE, image_create_flags: vk::ImageCreateFlags::empty(), + ..Default::default() }], output_formats: vec![entry(NV12, OUTPUT_USAGE)], coincide_formats: vec![], @@ -927,6 +984,7 @@ mod tests { format: NV12, image_usage: COINCIDE_USAGE, image_create_flags: vk::ImageCreateFlags::empty(), + ..Default::default() }]; assert_eq!( derive_caps(&raw).unwrap_err(), diff --git a/crates/pf-vkdecode/src/caps_av1.rs b/crates/pf-vkdecode/src/caps_av1.rs index 509ae1b9..26073201 100644 --- a/crates/pf-vkdecode/src/caps_av1.rs +++ b/crates/pf-vkdecode/src/caps_av1.rs @@ -354,6 +354,7 @@ mod tests { format, image_usage: usage, image_create_flags: vk::ImageCreateFlags::MUTABLE_FORMAT, + ..Default::default() } } @@ -638,6 +639,7 @@ mod tests { format: P010, image_usage: DPB_USAGE, image_create_flags: vk::ImageCreateFlags::empty(), + ..Default::default() }], output_formats: vec![entry(NV12, OUTPUT_USAGE)], ..coincide_device(vec![]) @@ -683,6 +685,7 @@ mod tests { format: NV12, image_usage: COINCIDE_USAGE, image_create_flags: vk::ImageCreateFlags::empty(), + ..Default::default() }]); assert_eq!( derive_caps_av1(&raw, NV12).unwrap_err(), diff --git a/crates/pf-vkdecode/src/caps_h265.rs b/crates/pf-vkdecode/src/caps_h265.rs index a25c92ff..24c8fcf0 100644 --- a/crates/pf-vkdecode/src/caps_h265.rs +++ b/crates/pf-vkdecode/src/caps_h265.rs @@ -383,6 +383,7 @@ mod tests { format, image_usage: usage, image_create_flags: vk::ImageCreateFlags::MUTABLE_FORMAT, + ..Default::default() } } @@ -722,6 +723,7 @@ mod tests { format: P010, image_usage: DPB_USAGE, image_create_flags: vk::ImageCreateFlags::empty(), + ..Default::default() }], output_formats: vec![entry(NV12, OUTPUT_USAGE)], ..coincide_device(vec![]) @@ -767,6 +769,7 @@ mod tests { format: P010, image_usage: COINCIDE_USAGE, image_create_flags: vk::ImageCreateFlags::empty(), + ..Default::default() }]); assert_eq!( derive_caps_h265(&raw, P010).unwrap_err(), diff --git a/crates/pf-vkdecode/src/images.rs b/crates/pf-vkdecode/src/images.rs index 900af398..139759b8 100644 --- a/crates/pf-vkdecode/src/images.rs +++ b/crates/pf-vkdecode/src/images.rs @@ -497,6 +497,7 @@ mod tests { format: NV12, image_usage: usage, image_create_flags: vk::ImageCreateFlags::MUTABLE_FORMAT, + ..Default::default() }; let raw = RawH264Caps { capability_flags: if layered { diff --git a/crates/pf-vkdecode/src/lib.rs b/crates/pf-vkdecode/src/lib.rs index ccc1fd95..d46407ae 100644 --- a/crates/pf-vkdecode/src/lib.rs +++ b/crates/pf-vkdecode/src/lib.rs @@ -133,6 +133,7 @@ pub mod params_h265; pub mod pic; pub mod pic_av1; pub mod pic_h265; +pub mod probe; pub mod recovery; pub mod ring; pub mod session; diff --git a/crates/pf-vkdecode/src/probe.rs b/crates/pf-vkdecode/src/probe.rs new file mode 100644 index 00000000..aa7f2455 --- /dev/null +++ b/crates/pf-vkdecode/src/probe.rs @@ -0,0 +1,418 @@ +//! What the driver ACTUALLY answers about video image formats, verbatim — the +//! physical-device-only probe behind `punktfunk-session --probe-decode`. +//! +//! # Why this exists +//! +//! Twice in a row an Intel Arc refusal was diagnosed from punktfunk's OWN error text +//! and twice the conclusion ("Intel driver bug") was wrong — the bug was ours, in the +//! `pNext` order of the capability query. What broke it open both times was logging +//! the driver's answer with no interpretation in front of it. This module makes that +//! the DEFAULT rather than a debugging afterthought: for every decode profile the +//! client can negotiate, it asks the driver the same question the session's caps query +//! asks, in every usage combination the image pools would create with, and records +//! what came back — including the failures, spelled as the `VkResult` the driver +//! returned. +//! +//! It shares [`crate::caps::query_formats_on`] with the real caps path on purpose. A +//! probe with its own copy of the query is a probe that eventually disagrees with the +//! code it is meant to explain, which is worse than no probe at all. +//! +//! # What it measured +//! +//! Intel Arc (Windows driver 101.8861, 2026-08): every one of the 26 decode profiles +//! reports the SAME envelope for its NV12/P010 picture format — +//! `TRANSFER_SRC | VIDEO_DECODE_DST | VIDEO_DECODE_DPB`, and `imageCreateFlags` EMPTY — +//! with `DPB_AND_OUTPUT_COINCIDE` as the only decode mode. No `SAMPLED`, so a shader +//! cannot read the decoded picture; and no `MUTABLE_FORMAT`/`EXTENDED_USAGE`, which +//! closes the spec's only escape hatch (see [`crate::caps::VideoFormat:: +//! image_create_flags`]). `TRANSFER_SRC` is the sole way out of the image — i.e. a +//! copy. NVIDIA (596.41) answers the same queries with +//! `TRANSFER_SRC|TRANSFER_DST|SAMPLED|DECODE_DST|DECODE_DPB|ENCODE_SRC|ENCODE_DPB` and +//! `MUTABLE_FORMAT|EXTENDED_USAGE`, which is what makes the zero-copy path work there. +//! +//! The [`UsageProbe::image_format_support`] cross-check exists because of the ONE +//! inconsistency in the above: asked for a usage it does not support, the Intel driver +//! returns `VK_SUCCESS` with an entry that lacks the requested bit, where the spec says +//! the returned `imageUsageFlags` "will contain at least the same set of image usage +//! flags" (and `VK_ERROR_IMAGE_USAGE_NOT_SUPPORTED_KHR` is the documented refusal). So +//! the probe asks a SECOND, independent question — `vkGetPhysicalDeviceImageFormat- +//! Properties2` with the same profile list chained — and prints both answers. Where the +//! two disagree, the disagreement itself is the finding. + +use ash::vk; +use ash::vk::native as hh; + +use crate::caps::query_formats_on; +use crate::caps::DecodeProfile; +use crate::caps::VideoFormat; +use crate::caps::COINCIDE_USAGE; +use crate::caps::DPB_USAGE; +use crate::caps::NV12; +use crate::caps::OUTPUT_USAGE; +use crate::caps::P010; +use crate::caps_av1::Av1ProfileKey; +use crate::caps_h265::H265ProfileKey; + +/// The usage combinations the probe asks about, widest question first. +/// +/// The first three are the REAL ones — exactly what [`crate::images`] creates with, so +/// their answers are the ones derivation acts on. The rest exist to localise a refusal: +/// when `DPB|DST|SAMPLED` fails, `DPB|DST` says whether the decode roles alone are fine +/// (i.e. the gap is sampling) and `SAMPLED` alone says whether the format is sampleable +/// under this profile at all. Without them, a single failed query leaves "which half is +/// missing?" to inference — which is how this device got misdiagnosed twice. +const USAGE_MATRIX: [(&str, vk::ImageUsageFlags); 6] = [ + ("coincide DPB|DST|SAMPLED", COINCIDE_USAGE), + ("distinct DPB", DPB_USAGE), + ("distinct DST|SAMPLED", OUTPUT_USAGE), + ("DPB|DST (no SAMPLED)", DECODE_ONLY_USAGE), + ("SAMPLED alone", vk::ImageUsageFlags::SAMPLED), + ("DST alone", vk::ImageUsageFlags::VIDEO_DECODE_DST_KHR), +]; + +/// Decode roles with sampling deliberately withheld — the discriminator between "this +/// device cannot decode this profile" and "it can decode it but not let anyone read it". +const DECODE_ONLY_USAGE: vk::ImageUsageFlags = vk::ImageUsageFlags::from_raw( + vk::ImageUsageFlags::VIDEO_DECODE_DPB_KHR.as_raw() + | vk::ImageUsageFlags::VIDEO_DECODE_DST_KHR.as_raw(), +); + +/// One decode profile's worth of driver answers. +#[derive(Debug, Clone)] +pub struct ProfileProbe { + /// The profile as a human would name it ("H.265 Main 4:2:0 8-bit"). + pub profile: &'static str, + /// The picture format this profile decodes to — the entry the probe looks for. + pub wanted: vk::Format, + /// One row per [`USAGE_MATRIX`] entry, in that order. + pub usages: Vec, +} + +/// The driver's answer for ONE (profile, usage) question. +#[derive(Debug, Clone)] +pub struct UsageProbe { + pub label: &'static str, + pub usage: vk::ImageUsageFlags, + /// `vkGetPhysicalDeviceVideoFormatPropertiesKHR`: every entry it returned, or the + /// `VkResult` it failed with. An EMPTY vector is itself an answer — the two + /// "no formats for this combination" results + /// (`ERROR_FORMAT_NOT_SUPPORTED`/`ERROR_IMAGE_USAGE_NOT_SUPPORTED_KHR`) are mapped + /// to it by the shared query, exactly as derivation sees them. + pub formats: Result, vk::Result>, + /// `vkGetPhysicalDeviceImageFormatProperties2` for the SAME profile list, format + /// and usage — the independent second opinion (module docs). `Ok(())` means the + /// driver says an image of that shape is creatable. + pub image_format_support: Result<(), vk::Result>, +} + +impl UsageProbe { + /// The entry for the profile's picture format, if the driver returned one. + pub fn wanted_entry(&self, wanted: vk::Format) -> Option { + self.formats + .as_ref() + .ok()? + .iter() + .copied() + .find(|f| f.format == wanted) + } +} + +/// The profiles worth probing: one per codec rung the client can negotiate, plus the +/// 10-bit legs, because an HDR stream picks a DIFFERENT Vulkan profile from an SDR one +/// and a device may well host one and not the other. +/// +/// Deliberately NOT every profile the driver supports — this answers "can punktfunk +/// decode here", and a list padded with profiles no rung ever requests makes the row +/// that matters harder to find. `vulkaninfo --show-video-props` is the tool for the +/// exhaustive sweep. +fn probed_profiles() -> Vec<(&'static str, DecodeProfile, vk::Format)> { + // Every key is built through the SAME constructor the decoders negotiate with + // (`from_negotiated`), so a profile the client could never request cannot appear + // here — and a combination those constructors refuse simply drops out of the list + // instead of being hand-rolled into existence for the probe's benefit. + // 4:2:0 is chroma_format_idc 1 in both codecs' vocabulary; H.265 states depth as + // `bit_depth_luma_minus8`, AV1 in whole bits. + let mut out: Vec<(&'static str, DecodeProfile, vk::Format)> = vec![( + "H.264 High 4:2:0 8-bit", + DecodeProfile::H264(hh::StdVideoH264ProfileIdc_STD_VIDEO_H264_PROFILE_IDC_HIGH), + NV12, + )]; + if let Ok(key) = H265ProfileKey::from_negotiated(1, 0) { + out.push(("H.265 Main 4:2:0 8-bit", DecodeProfile::H265(key), NV12)); + } + if let Ok(key) = H265ProfileKey::from_negotiated(1, 2) { + out.push(("H.265 Main 10 4:2:0 10-bit", DecodeProfile::H265(key), P010)); + } + // AV1 without film grain: `filmGrainSupport` is part of the Vulkan PROFILE, and + // grain-less is what a punktfunk host encodes. + if let Ok(key) = Av1ProfileKey::from_negotiated(1, 8, false) { + out.push(("AV1 Main 4:2:0 8-bit", DecodeProfile::Av1(key), NV12)); + } + if let Ok(key) = Av1ProfileKey::from_negotiated(1, 10, false) { + out.push(("AV1 Main 4:2:0 10-bit", DecodeProfile::Av1(key), P010)); + } + out +} + +/// Ask one physical device every question in the matrix, for every probed profile. +/// +/// Never fails as a whole: a driver that refuses a profile outright is a ROW in the +/// output, not an error return — the point is to come back with the full picture even +/// when most of it is refusals. +/// +/// # Safety +/// +/// `instance` must be a live `VkInstance` created through `entry`, and +/// `physical_device` one of its physical devices. Nothing here creates or destroys +/// anything; every call is a physical-device query. +pub unsafe fn probe_video_formats( + entry: &ash::Entry, + instance: &ash::Instance, + physical_device: vk::PhysicalDevice, +) -> Vec { + let video_queue_instance = ash::khr::video_queue::Instance::new(entry, instance); + probed_profiles() + .into_iter() + .map(|(profile, decode_profile, wanted)| { + let usages = USAGE_MATRIX + .iter() + .map(|(label, usage)| { + // SAFETY: caller contract — live instance the video_queue table was + // loaded from, and one of its physical devices. + let formats = unsafe { + query_formats_on( + &video_queue_instance, + physical_device, + decode_profile, + *usage, + ) + }; + // SAFETY: as above. + let image_format_support = unsafe { + image_format_supported( + instance, + physical_device, + decode_profile, + wanted, + *usage, + ) + }; + UsageProbe { + label, + usage: *usage, + formats, + image_format_support, + } + }) + .collect(); + ProfileProbe { + profile, + wanted, + usages, + } + }) + .collect() +} + +/// The second opinion: can an image of (`format`, `usage`) exist for this video profile, +/// according to `vkGetPhysicalDeviceImageFormatProperties2`? +/// +/// This is the same question `vkCreateImage` will be validated against +/// (VUID-VkImageCreateInfo-pNext-06811 routes through the video format properties, but +/// the general image-format query is what reports whether the combination is creatable +/// at all), asked through a DIFFERENT entry point. Where it disagrees with the video +/// format properties, one of the two driver paths is wrong — and knowing which is the +/// difference between a bug report to Intel and a fix in this repository. +/// +/// # Safety +/// +/// As [`probe_video_formats`]. +unsafe fn image_format_supported( + instance: &ash::Instance, + physical_device: vk::PhysicalDevice, + decode_profile: DecodeProfile, + format: vk::Format, + usage: vk::ImageUsageFlags, +) -> Result<(), vk::Result> { + let mut chain = decode_profile.chain(); + let profile = chain.wire(); + let mut profile_list = + vk::VideoProfileListInfoKHR::default().profiles(std::slice::from_ref(profile)); + let info = vk::PhysicalDeviceImageFormatInfo2::default() + .format(format) + .ty(vk::ImageType::TYPE_2D) + .tiling(vk::ImageTiling::OPTIMAL) + .usage(usage) + .push_next(&mut profile_list); + let mut props = vk::ImageFormatProperties2::default(); + // SAFETY: caller contract (live instance + one of its physical devices); `info` + // roots a wired chain of locals that outlive the call, and `props` is a local the + // driver fills. + unsafe { + instance.get_physical_device_image_format_properties2(physical_device, &info, &mut props) + } +} + +/// `usage` as `NAME|NAME (0xHEX)`, with any bit this build cannot name kept VISIBLE. +/// +/// The raw value is always printed beside the words for the same reason the codec-op +/// line prints its mask: a reader must be able to check the names against the number, +/// and a bit the tool has no word for must not silently vanish from a mask it reports. +pub fn describe_usage(usage: vk::ImageUsageFlags) -> String { + const BITS: [(vk::ImageUsageFlags, &str); 9] = [ + (vk::ImageUsageFlags::TRANSFER_SRC, "TRANSFER_SRC"), + (vk::ImageUsageFlags::TRANSFER_DST, "TRANSFER_DST"), + (vk::ImageUsageFlags::SAMPLED, "SAMPLED"), + (vk::ImageUsageFlags::STORAGE, "STORAGE"), + (vk::ImageUsageFlags::COLOR_ATTACHMENT, "COLOR_ATTACHMENT"), + (vk::ImageUsageFlags::VIDEO_DECODE_DST_KHR, "DECODE_DST"), + (vk::ImageUsageFlags::VIDEO_DECODE_SRC_KHR, "DECODE_SRC"), + (vk::ImageUsageFlags::VIDEO_DECODE_DPB_KHR, "DECODE_DPB"), + (vk::ImageUsageFlags::INPUT_ATTACHMENT, "INPUT_ATTACHMENT"), + ]; + describe_mask(usage.as_raw(), &BITS.map(|(f, n)| (f.as_raw(), n))) +} + +/// `imageCreateFlags` as `NAME|NAME (0xHEX)`, same accounting rule as +/// [`describe_usage`]. +pub fn describe_create_flags(flags: vk::ImageCreateFlags) -> String { + const BITS: [(vk::ImageCreateFlags, &str); 5] = [ + (vk::ImageCreateFlags::MUTABLE_FORMAT, "MUTABLE_FORMAT"), + (vk::ImageCreateFlags::EXTENDED_USAGE, "EXTENDED_USAGE"), + (vk::ImageCreateFlags::ALIAS, "ALIAS"), + (vk::ImageCreateFlags::DISJOINT, "DISJOINT"), + (vk::ImageCreateFlags::PROTECTED, "PROTECTED"), + ]; + describe_mask(flags.as_raw(), &BITS.map(|(f, n)| (f.as_raw(), n))) +} + +/// The shared naming rule: named bits joined by `|`, then the raw value, then any +/// leftover bits called out as unrecognised rather than dropped. +fn describe_mask(raw: u32, bits: &[(u32, &str)]) -> String { + if raw == 0 { + return "(none) (0x0)".to_string(); + } + let mut names: Vec<&str> = bits + .iter() + .filter(|(bit, _)| raw & bit != 0) + .map(|(_, name)| *name) + .collect(); + let named: u32 = bits.iter().map(|(bit, _)| bit).sum(); + let leftover = raw & !named; + let extra; + if leftover != 0 { + extra = format!("unrecognised 0x{leftover:X}"); + names.push(&extra); + } + format!("{} (0x{raw:X})", names.join("|")) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The matrix must contain the three combinations the pools REALLY create with — + /// a probe that answers about usages nobody creates explains nothing. + #[test] + fn the_matrix_covers_every_usage_the_pools_create_with() { + for real in [COINCIDE_USAGE, DPB_USAGE, OUTPUT_USAGE] { + assert!( + USAGE_MATRIX.iter().any(|(_, u)| *u == real), + "{real:?} is created by the image pools but never probed" + ); + } + // And the two discriminators that localise a refusal. + assert!(USAGE_MATRIX.iter().any(|(_, u)| *u == DECODE_ONLY_USAGE)); + assert!(USAGE_MATRIX + .iter() + .any(|(_, u)| *u == vk::ImageUsageFlags::SAMPLED)); + } + + /// Every profile the client can negotiate gets a row, each with the picture format + /// its caps derivation will look for — a probe that reported a 10-bit profile + /// against NV12 would "find" nothing and read as a device gap. + #[test] + fn every_probed_profile_names_the_format_derivation_wants() { + let profiles = probed_profiles(); + assert!( + profiles.len() >= 5, + "expected H.264 + H.265 8/10-bit + AV1 8/10-bit, got {}", + profiles.len() + ); + for (name, _, wanted) in &profiles { + assert!( + crate::caps::OUTPUT_FORMATS.contains(wanted), + "{name} wants {wanted:?}, which is outside this crate's output vocabulary" + ); + } + assert!(profiles.iter().any(|(_, _, w)| *w == P010), "no 10-bit leg"); + } + + /// The mask printers must never drop a bit: names AND the raw value AND anything + /// unnamed. This is the accounting rule the codec-op line already follows, and the + /// reason it exists is that a silently-dropped bit reads as a capability the device + /// does not have (or worse, hides one it does). + #[test] + fn mask_descriptions_account_for_every_bit_they_print() { + assert_eq!( + describe_usage(COINCIDE_USAGE), + "SAMPLED|DECODE_DST|DECODE_DPB (0x1404)" + ); + assert_eq!(describe_usage(vk::ImageUsageFlags::empty()), "(none) (0x0)"); + // The Intel Arc envelope, verbatim — the string a field report will contain. + let intel = vk::ImageUsageFlags::TRANSFER_SRC + | vk::ImageUsageFlags::VIDEO_DECODE_DST_KHR + | vk::ImageUsageFlags::VIDEO_DECODE_DPB_KHR; + assert_eq!( + describe_usage(intel), + "TRANSFER_SRC|DECODE_DST|DECODE_DPB (0x1401)" + ); + // A bit with no name still shows up. + let unknown = vk::ImageUsageFlags::from_raw(0x8000_0000); + assert_eq!( + describe_usage(unknown), + "unrecognised 0x80000000 (0x80000000)" + ); + assert_eq!( + describe_create_flags( + vk::ImageCreateFlags::MUTABLE_FORMAT | vk::ImageCreateFlags::EXTENDED_USAGE + ), + "MUTABLE_FORMAT|EXTENDED_USAGE (0x108)" + ); + assert_eq!( + describe_create_flags(vk::ImageCreateFlags::empty()), + "(none) (0x0)" + ); + } + + /// `wanted_entry` picks by FORMAT, so a driver that returns several entries cannot + /// hide the one derivation will act on behind a different format. + #[test] + fn the_wanted_entry_is_found_by_format_among_others() { + let probe = UsageProbe { + label: "x", + usage: COINCIDE_USAGE, + formats: Ok(vec![ + VideoFormat { + format: P010, + image_usage: COINCIDE_USAGE, + ..Default::default() + }, + VideoFormat { + format: NV12, + image_usage: DPB_USAGE, + ..Default::default() + }, + ]), + image_format_support: Ok(()), + }; + assert_eq!(probe.wanted_entry(NV12).unwrap().image_usage, DPB_USAGE); + assert!(probe.wanted_entry(crate::caps::YUV444_8).is_none()); + // A failed query has no entry at all — distinct from "returned nothing". + let failed = UsageProbe { + formats: Err(vk::Result::ERROR_IMAGE_USAGE_NOT_SUPPORTED_KHR), + ..probe + }; + assert!(failed.wanted_entry(NV12).is_none()); + } +} From b166c53cc2548db788f1dedb33e560177fe7ac21 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 7 Aug 2026 15:44:07 +0200 Subject: [PATCH 8/8] fix(vkdecode): a refused device says what the refusal costs, and about which format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three things the Intel Arc measurement showed were wrong or unhelpful in the refusal path. The message named NV12 whatever the stream was. A Main 10 session refused over P010 was told about NV12, which sends the reader to look up the wrong format's support. Both variants now carry the format the driver's own entry reported. A missing SAMPLED now says what it costs. "does not advertise usage SAMPLED" is accurate and tells a field reporter nothing: the consequence is that no shader can read this device's decoded pictures, so the zero-copy path cannot exist on it at all — which is a different conversation from a device that is merely slower. The line points at --probe-decode for the driver's own words. And the probe's second opinion no longer claims to be one. Measured on both vendors, vkGetPhysicalDeviceImageFormatProperties2 answers "creatable" for combinations the video-format query rejects — on NVIDIA too, for SAMPLED alone, which is not a legal video image usage at all. So it does not honour the chained profile list and must not be read as permission; it is still printed, because otherwise everyone who reads a refusal asks the question again, but it is labelled as not authority. Also names the three video ENCODE usage bits, which NVIDIA advertises on decode pictures and the probe was printing as "unrecognised 0xC000". --- clients/session/src/main.rs | 20 +++--- crates/pf-vkdecode/src/caps.rs | 96 ++++++++++++++++++++++++++--- crates/pf-vkdecode/src/caps_av1.rs | 4 +- crates/pf-vkdecode/src/caps_h265.rs | 4 +- crates/pf-vkdecode/src/probe.rs | 48 +++++++++++---- 5 files changed, 141 insertions(+), 31 deletions(-) diff --git a/clients/session/src/main.rs b/clients/session/src/main.rs index d84f031a..96272edc 100644 --- a/clients/session/src/main.rs +++ b/clients/session/src/main.rs @@ -543,21 +543,25 @@ mod session_main { .join("; "), }; println!(" {:<24} {answer}", u.label); - // The independent second opinion, printed only where it DISAGREES with - // the format query. Agreement is the normal case and would be noise; a - // disagreement means one of the driver's two paths is wrong, which is - // the whole reason a second question gets asked at all. + // The second opinion, printed only where it differs from the video + // format query. Worded as "also asked" rather than "disagrees" on + // purpose: measured on both vendors this call answers "creatable" for + // combinations the video query rejects (NVIDIA included, for SAMPLED + // alone), so it does not honour the profile list and a difference here + // is NOT the driver contradicting itself. Printed anyway because the + // question gets re-asked by everyone who reads a refusal. let listed = u .wanted_entry(p.wanted) .is_some_and(|f| f.image_usage.contains(u.usage)); if listed != u.image_format_support.is_ok() { let second = match &u.image_format_support { - Ok(()) => "says creatable".to_string(), - Err(e) => format!("says {e:?}"), + Ok(()) => "creatable".to_string(), + Err(e) => format!("{e:?}"), }; println!( - " {:<24} ^ DISAGREES: \ - vkGetPhysicalDeviceImageFormatProperties2 {second}", + " {:<24} (also asked: \ + vkGetPhysicalDeviceImageFormatProperties2 says {second} — that \ + call does not honour the profile list; not authority)", "" ); } diff --git a/crates/pf-vkdecode/src/caps.rs b/crates/pf-vkdecode/src/caps.rs index e37963c0..90b4639c 100644 --- a/crates/pf-vkdecode/src/caps.rs +++ b/crates/pf-vkdecode/src/caps.rs @@ -288,12 +288,18 @@ pub enum CapsError { /// anyway would be a silent VUID violation. UsageUnsupported { mode: &'static str, + /// The picture format whose entry fell short — NOT always NV12 (a Main 10 + /// stream is refused about P010), which is what this used to say regardless. + format: vk::Format, missing: vk::ImageUsageFlags, }, /// The presenter-facing entry for `mode` does not allow `MUTABLE_FORMAT`, so /// the per-plane views the presenter samples through ([`plane_formats`]) /// cannot exist on this device. - NoMutableFormat { mode: &'static str }, + NoMutableFormat { + mode: &'static str, + format: vk::Format, + }, /// The driver forces COINCIDE mode AND a layered DPB (one image array, no /// `SEPARATE_REFERENCE_IMAGES`): the picture-pool model — a re-activated slot /// binding a fresh free image, so delivered pictures are never decode targets @@ -322,16 +328,36 @@ impl std::fmt::Display for CapsError { CapsError::NoPlaneMapping { format } => { write!(f, "no per-plane view mapping for {format:?}") } - CapsError::UsageUnsupported { mode, missing } => { + CapsError::UsageUnsupported { + mode, + format, + missing, + } => { write!( f, - "the {mode} NV12 entry does not advertise usage {missing:?}" - ) + "the {mode} {format:?} entry does not advertise usage {missing:?}" + )?; + // Name the CONSEQUENCE for the one missing bit that is not a passing + // driver quirk. Without `SAMPLED` nothing in a shader can read the + // decoded picture, so the zero-copy path this rung exists for cannot be + // built here at all — a fact worth stating in the log line rather than + // leaving a field reporter to work out from a flag name. Measured on + // Intel Arc (Windows 101.8861), where the whole advertised envelope is + // TRANSFER_SRC|DECODE_DST|DECODE_DPB with no image create flags: + // `punktfunk-session --probe-decode` prints the driver's own answer. + if missing.contains(vk::ImageUsageFlags::SAMPLED) { + write!( + f, + " — no shader can read this device's decoded pictures, so the \ + zero-copy path cannot exist on it (see --probe-decode)" + )?; + } + Ok(()) } - CapsError::NoMutableFormat { mode } => { + CapsError::NoMutableFormat { mode, format } => { write!( f, - "the {mode} NV12 entry does not allow MUTABLE_FORMAT (per-plane views)" + "the {mode} {format:?} entry does not allow MUTABLE_FORMAT (per-plane views)" ) } CapsError::CoincideLayeredDpb => { @@ -503,7 +529,14 @@ fn require_usage( if missing.is_empty() { Ok(()) } else { - Err(CapsError::UsageUnsupported { mode, missing }) + // The entry's OWN format, not the caller's `wanted`: they are equal here (the + // entry was picked by format), and taking it from the driver's record keeps the + // message describing what the driver actually said. + Err(CapsError::UsageUnsupported { + mode, + format: entry.format, + missing, + }) } } @@ -514,7 +547,10 @@ fn require_mutable(entry: &VideoFormat, mode: &'static str) -> Result<(), CapsEr { Ok(()) } else { - Err(CapsError::NoMutableFormat { mode }) + Err(CapsError::NoMutableFormat { + mode, + format: entry.format, + }) } } @@ -961,9 +997,21 @@ mod tests { derive_caps(&raw).unwrap_err(), CapsError::UsageUnsupported { mode: "coincide (DPB|DST|SAMPLED)", + format: NV12, missing: vk::ImageUsageFlags::SAMPLED } ); + // The missing bit is SAMPLED, so the message says what that COSTS — a field + // report carrying this line should not need a second round trip to learn that + // the device cannot host the rung at all. + assert!( + derive_caps(&raw) + .unwrap_err() + .to_string() + .contains("zero-copy path cannot exist"), + "a missing SAMPLED must name its consequence: {}", + derive_caps(&raw).unwrap_err() + ); // Same on the distinct output half. let mut raw = nvidia_like(); @@ -972,9 +1020,38 @@ mod tests { derive_caps(&raw).unwrap_err(), CapsError::UsageUnsupported { mode: "output (DST|SAMPLED)", + format: NV12, missing: vk::ImageUsageFlags::SAMPLED } ); + + // A 10-bit stream is refused about P010, not about NV12 — the message used to + // say "NV12" whatever the stream was, which sends a reader looking at the wrong + // format's support. + let mut raw = radv_like(); + raw.coincide_formats = vec![entry( + P010, + vk::ImageUsageFlags::VIDEO_DECODE_DPB_KHR | vk::ImageUsageFlags::VIDEO_DECODE_DST_KHR, + )]; + let err = crate::caps_h265::derive_caps_h265( + &crate::caps_h265::RawH265Caps { + capability_flags: raw.capability_flags, + decode_flags: raw.decode_flags, + coincide_formats: raw.coincide_formats.clone(), + ..Default::default() + }, + P010, + ) + .unwrap_err(); + assert_eq!( + err, + CapsError::UsageUnsupported { + mode: "coincide (DPB|DST|SAMPLED)", + format: P010, + missing: vk::ImageUsageFlags::SAMPLED + } + ); + assert!(err.to_string().contains("G10X6"), "{err}"); } #[test] @@ -989,7 +1066,8 @@ mod tests { assert_eq!( derive_caps(&raw).unwrap_err(), CapsError::NoMutableFormat { - mode: "coincide (DPB|DST|SAMPLED)" + mode: "coincide (DPB|DST|SAMPLED)", + format: NV12, } ); diff --git a/crates/pf-vkdecode/src/caps_av1.rs b/crates/pf-vkdecode/src/caps_av1.rs index 26073201..c3b88b9a 100644 --- a/crates/pf-vkdecode/src/caps_av1.rs +++ b/crates/pf-vkdecode/src/caps_av1.rs @@ -676,6 +676,7 @@ mod tests { derive_caps_av1(&raw, NV12).unwrap_err(), CapsError::UsageUnsupported { mode: "coincide (DPB|DST|SAMPLED)", + format: NV12, missing: vk::ImageUsageFlags::SAMPLED } ); @@ -690,7 +691,8 @@ mod tests { assert_eq!( derive_caps_av1(&raw, NV12).unwrap_err(), CapsError::NoMutableFormat { - mode: "coincide (DPB|DST|SAMPLED)" + mode: "coincide (DPB|DST|SAMPLED)", + format: NV12, } ); } diff --git a/crates/pf-vkdecode/src/caps_h265.rs b/crates/pf-vkdecode/src/caps_h265.rs index 24c8fcf0..7231d17b 100644 --- a/crates/pf-vkdecode/src/caps_h265.rs +++ b/crates/pf-vkdecode/src/caps_h265.rs @@ -760,6 +760,7 @@ mod tests { derive_caps_h265(&raw, P010).unwrap_err(), CapsError::UsageUnsupported { mode: "coincide (DPB|DST|SAMPLED)", + format: P010, missing: vk::ImageUsageFlags::SAMPLED } ); @@ -774,7 +775,8 @@ mod tests { assert_eq!( derive_caps_h265(&raw, P010).unwrap_err(), CapsError::NoMutableFormat { - mode: "coincide (DPB|DST|SAMPLED)" + mode: "coincide (DPB|DST|SAMPLED)", + format: P010, } ); } diff --git a/crates/pf-vkdecode/src/probe.rs b/crates/pf-vkdecode/src/probe.rs index aa7f2455..cd5b116d 100644 --- a/crates/pf-vkdecode/src/probe.rs +++ b/crates/pf-vkdecode/src/probe.rs @@ -30,14 +30,30 @@ //! `TRANSFER_SRC|TRANSFER_DST|SAMPLED|DECODE_DST|DECODE_DPB|ENCODE_SRC|ENCODE_DPB` and //! `MUTABLE_FORMAT|EXTENDED_USAGE`, which is what makes the zero-copy path work there. //! -//! The [`UsageProbe::image_format_support`] cross-check exists because of the ONE -//! inconsistency in the above: asked for a usage it does not support, the Intel driver -//! returns `VK_SUCCESS` with an entry that lacks the requested bit, where the spec says -//! the returned `imageUsageFlags` "will contain at least the same set of image usage -//! flags" (and `VK_ERROR_IMAGE_USAGE_NOT_SUPPORTED_KHR` is the documented refusal). So -//! the probe asks a SECOND, independent question — `vkGetPhysicalDeviceImageFormat- -//! Properties2` with the same profile list chained — and prints both answers. Where the -//! two disagree, the disagreement itself is the finding. +//! The Intel answers carry one genuine conformance bug, which is what made the refusal +//! read oddly: the driver ignores the REQUESTED `imageUsage` completely. Asked for +//! `SAMPLED` alone it still returns that same decode envelope, where the spec says the +//! returned `imageUsageFlags` "will contain at least the same set of image usage flags" +//! and `VK_ERROR_IMAGE_USAGE_NOT_SUPPORTED_KHR` is the documented refusal. That is why +//! derivation reported "the NV12 entry does not advertise SAMPLED" (an entry was found) +//! rather than "no NV12 in the coincide list" (nothing returned). It changes the error +//! text, not the answer. +//! +//! # What the cross-check is, and is NOT +//! +//! [`UsageProbe::image_format_support`] asks `vkGetPhysicalDeviceImageFormatProperties2` +//! the same question with the same profile list chained. It is deliberately kept, and +//! deliberately NOT treated as authority, because measuring it settled what it is worth: +//! on BOTH vendors it answers "creatable" for combinations the video-format query +//! rejects — NVIDIA included, for `SAMPLED` alone, which is not a legal video image +//! usage at all. So that entry point does not fully honour the video profile list on any +//! driver measured here, and a "creatable" from it is NOT evidence that a decode picture +//! can be sampled. It is reported because the question is otherwise re-asked by every +//! person who reads the refusal; the answer is on the record instead. +//! +//! `vkGetPhysicalDeviceVideoFormatPropertiesKHR` remains the authority, and two +//! independent implementations of it — this probe and `vulkaninfo --show-video-props` — +//! agree on every value above. use ash::vk; use ash::vk::native as hh; @@ -100,8 +116,10 @@ pub struct UsageProbe { /// to it by the shared query, exactly as derivation sees them. pub formats: Result, vk::Result>, /// `vkGetPhysicalDeviceImageFormatProperties2` for the SAME profile list, format - /// and usage — the independent second opinion (module docs). `Ok(())` means the - /// driver says an image of that shape is creatable. + /// and usage. `Ok(())` means that call says an image of the shape is creatable — + /// which, as the module docs record, is a WEAKER claim than it looks: measured on + /// both vendors, this entry point does not fully honour the chained video profile + /// list, so it must not be read as permission to create a video image. pub image_format_support: Result<(), vk::Result>, } @@ -259,16 +277,22 @@ unsafe fn image_format_supported( /// line prints its mask: a reader must be able to check the names against the number, /// and a bit the tool has no word for must not silently vanish from a mask it reports. pub fn describe_usage(usage: vk::ImageUsageFlags) -> String { - const BITS: [(vk::ImageUsageFlags, &str); 9] = [ + // The encode trio is here because NVIDIA advertises it on DECODE pictures (measured: + // 0xC000 beside the decode bits), and a mask printed as "unrecognised" invites the + // reader to wonder whether the tool is out of date rather than reading the answer. + const BITS: [(vk::ImageUsageFlags, &str); 12] = [ (vk::ImageUsageFlags::TRANSFER_SRC, "TRANSFER_SRC"), (vk::ImageUsageFlags::TRANSFER_DST, "TRANSFER_DST"), (vk::ImageUsageFlags::SAMPLED, "SAMPLED"), (vk::ImageUsageFlags::STORAGE, "STORAGE"), (vk::ImageUsageFlags::COLOR_ATTACHMENT, "COLOR_ATTACHMENT"), + (vk::ImageUsageFlags::INPUT_ATTACHMENT, "INPUT_ATTACHMENT"), (vk::ImageUsageFlags::VIDEO_DECODE_DST_KHR, "DECODE_DST"), (vk::ImageUsageFlags::VIDEO_DECODE_SRC_KHR, "DECODE_SRC"), (vk::ImageUsageFlags::VIDEO_DECODE_DPB_KHR, "DECODE_DPB"), - (vk::ImageUsageFlags::INPUT_ATTACHMENT, "INPUT_ATTACHMENT"), + (vk::ImageUsageFlags::VIDEO_ENCODE_DST_KHR, "ENCODE_DST"), + (vk::ImageUsageFlags::VIDEO_ENCODE_SRC_KHR, "ENCODE_SRC"), + (vk::ImageUsageFlags::VIDEO_ENCODE_DPB_KHR, "ENCODE_DPB"), ]; describe_mask(usage.as_raw(), &BITS.map(|(f, n)| (f.as_raw(), n))) }