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))) }