From 09aa2db37cbc03e9258077e27878522410d0cba5 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sat, 25 Jul 2026 00:32:21 +0200 Subject: [PATCH] fix(encode): probe Vulkan encode before committing capture to producer-native NV12 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `linux_native_nv12_ok` is the verdict the host threads into capture negotiation (session_plan -> OutputFormat::nv12_native -> ZeroCopyPolicy::native_nv12_session -> pf-capture's prefer_native_nv12). Its doc claimed "the backend must be eligible to open", but it asked only three static questions — codec is H265/AV1, an env default, and a pref denylist — and never whether this GPU has an encode queue at all. It could not: vulkan_video.rs exposed no probe. That verdict is uniquely load-bearing. Once the producer has been asked for two-plane NV12 there is NO fallback: open_video deliberately makes a failed Vulkan open FATAL for an NV12 capture rather than degrading to libav VAAPI, because VAAPI would import that buffer as packed RGB and stream silent garbage. So on a gamescope host whose Mesa lacks Vulkan HEVC encode the session died at its first frame, while the same host with PUNKTFUNK_PIPEWIRE_NV12=0 streamed fine — and the comment promising such a device "degrades gracefully to the old backend rather than breaking the stream" was stale on every gamescope host. Two changes: 1. The gate. The third conjunct was a denylist of the EXPLICIT prefs that skip Vulkan Video ("nvenc"|"nvidia"|"cuda"|"pyrowave"), which silently missed the one that matters: the DEFAULT encoder_pref is "", and "" resolves to auto, which on an NVIDIA box opens NVENC. A stock NVIDIA host passed this gate. It now consults `linux_zero_copy_is_vaapi`, which layers the pref on top of the same auto decision open_video makes — what the note on `linux_auto_is_vaapi` says a capability probe must use, and what the downstream consumer of this very verdict already uses. This alone was worth fixing; a probe added behind the old gate would have opened a Vulkan instance on every NVIDIA handshake. 2. The probe. `vulkan_video::probe_encode_support` runs the FIRST check open_inner performs and hard-fails on — the physical-device + encode-queue-family scan for this codec's op — and nothing more, so it is provably no stricter than the open and can never talk a working host out of the fast path. The scan is now a shared `find_encode_device` used by BOTH, because a probe that mirrors a dispatch goes stale the first time the dispatch grows a case (the failure open_video's backend-label note already records). Cost: one instance plus physical-device queries — no logical device, no video session, no VRAM — cached per (selected GPU, codec) in the can_encode_10bit idiom, with the probe run outside the lock. Per codec, not once: codec_op_for selects a different queue-family bit for AV1, and HEVC-encode-without-AV1-encode is the common VCN/ANV configuration. Later stages (create_device, create_video_session, the capability query) can still fail for reasons the probe does not model. Those are harmless: the capture format is only committed once the probe says yes, and everything after keeps the ordinary packed-RGB negotiation. Verified `-D warnings --all-targets` + tests on Linux (default; shipped nvenc+vulkan-encode+pyrowave). The behaviour needs an on-glass check on the bazzite RADV box — including the negative case, which `RADV_DEBUG=novideo` reproduces. Co-Authored-By: Claude Opus 5 (1M context) --- .../pf-encode/src/enc/linux/vulkan_video.rs | 121 ++++++++++++++---- crates/pf-encode/src/lib.rs | 61 ++++++++- 2 files changed, 148 insertions(+), 34 deletions(-) diff --git a/crates/pf-encode/src/enc/linux/vulkan_video.rs b/crates/pf-encode/src/enc/linux/vulkan_video.rs index 61aadfd7..06e779b0 100644 --- a/crates/pf-encode/src/enc/linux/vulkan_video.rs +++ b/crates/pf-encode/src/enc/linux/vulkan_video.rs @@ -221,6 +221,92 @@ impl NativeProfileStack { /// The Vulkan codec-operation bit for our codec selection (shared by open and the per-import /// profile rebuilds — the two must agree, profile identity is by value). +/// The physical device + encode queue family a session runs on: the FIRST device exposing a +/// `VIDEO_ENCODE` queue family that advertises `codec_op` (llvmpipe advertises none, so it drops +/// out implicitly). Shared by [`VulkanVideoEncoder::open_inner`] and [`probe_encode_support`]. +/// +/// # Safety +/// `instance` must be a live `ash::Instance` and `devices` handles enumerated from it. +unsafe fn find_encode_device( + instance: &ash::Instance, + devices: &[vk::PhysicalDevice], + codec_op: vk::VideoCodecOperationFlagsKHR, +) -> Option<(vk::PhysicalDevice, u32)> { + for &pd in devices { + let qf_len = instance.get_physical_device_queue_family_properties2_len(pd); + let mut video = vec![vk::QueueFamilyVideoPropertiesKHR::default(); qf_len]; + let mut qf = vec![vk::QueueFamilyProperties2::default(); qf_len]; + for i in 0..qf_len { + qf[i].p_next = &mut video[i] as *mut _ as *mut c_void; + } + instance.get_physical_device_queue_family_properties2(pd, &mut qf); + for i in 0..qf_len { + if qf[i] + .queue_family_properties + .queue_flags + .contains(vk::QueueFlags::VIDEO_ENCODE_KHR) + && video[i].video_codec_operations.contains(codec_op) + { + return Some((pd, i as u32)); + } + } + } + None +} + +/// Can this GPU + driver open a Vulkan Video **encode** session for `codec` at all? +/// +/// This is the verdict the native-NV12 capture negotiation needs BEFORE it commits +/// ([`crate::linux_native_nv12_ok`]): once the producer hands over two-plane NV12 there is no VAAPI +/// fallback — libav would import it as packed RGB — so [`crate::open_video`] deliberately makes +/// that open-failure fatal, and a Mesa built without `VK_KHR_video_encode_h265` therefore kills the +/// session at its first frame instead of streaming on VAAPI. +/// +/// Deliberately the FIRST check `open_inner` performs and hard-fails on, and nothing more: the same +/// [`find_encode_device`] scan, run against the same codec op. That makes it provably no stricter +/// than the open, so a failure here can only ever name a session that would have died anyway — it +/// can never talk a working host out of the fast path. It is also cheap: one instance plus +/// physical-device queries, no logical device, no video session, no VRAM. The later stages +/// (`create_device`, `create_video_session`, the capability query) can still fail for reasons this +/// does not model; those keep the session on the packed-RGB negotiation the ordinary way, because +/// the capture format is only committed once this said yes. +/// +/// Probed PER CODEC, not once: `codec_op_for` selects a different queue-family bit for AV1, and +/// HEVC-encode-without-AV1-encode is the common VCN/ANV configuration. +pub(crate) fn probe_encode_support(codec: Codec) -> Result<(), &'static str> { + if !matches!(codec, Codec::H265 | Codec::Av1) { + return Err("the Vulkan Video backend encodes HEVC + AV1 only"); + } + let codec_op = codec_op_for(codec == Codec::Av1); + // SAFETY: creates one Vulkan instance and issues only physical-device queries against it, then + // destroys it on EVERY path below before returning — no handle derived from it escapes, and + // nothing outside this call observes it. `Entry::load` only dlopens the loader (a missing + // libvulkan returns `Err`), touching no process state the rest of the crate relies on. + // `find_encode_device` gets a live instance and handles enumerated from it, as it requires. + unsafe { + let Ok(entry) = ash::Entry::load() else { + return Err("no Vulkan loader"); + }; + let app = vk::ApplicationInfo::default().api_version(vk::API_VERSION_1_3); + let Ok(instance) = entry.create_instance( + &vk::InstanceCreateInfo::default().application_info(&app), + None, + ) else { + return Err("vkCreateInstance failed"); + }; + let found = match instance.enumerate_physical_devices() { + Ok(devices) => find_encode_device(&instance, &devices, codec_op).is_some(), + Err(_) => false, + }; + instance.destroy_instance(None); + if found { + Ok(()) + } else { + Err("no VK_KHR_video_encode queue for this codec on any device") + } + } +} + fn codec_op_for(av1: bool) -> vk::VideoCodecOperationFlagsKHR { if av1 { vk::VideoCodecOperationFlagsKHR::from_raw( @@ -604,34 +690,13 @@ impl VulkanVideoEncoder { let vq_inst = ash::khr::video_queue::Instance::new(&entry, &instance); - // pick the physical device + encode queue family (skip llvmpipe) - let (pd, encode_family) = { - let mut found = None; - for pd in instance.enumerate_physical_devices()? { - let qf_len = instance.get_physical_device_queue_family_properties2_len(pd); - let mut video = vec![vk::QueueFamilyVideoPropertiesKHR::default(); qf_len]; - let mut qf = vec![vk::QueueFamilyProperties2::default(); qf_len]; - for i in 0..qf_len { - qf[i].p_next = &mut video[i] as *mut _ as *mut c_void; - } - instance.get_physical_device_queue_family_properties2(pd, &mut qf); - for i in 0..qf_len { - if qf[i] - .queue_family_properties - .queue_flags - .contains(vk::QueueFlags::VIDEO_ENCODE_KHR) - && video[i].video_codec_operations.contains(codec_op) - { - found = Some((pd, i as u32)); - break; - } - } - if found.is_some() { - break; - } - } - found.context("no VK_KHR_video_encode queue for the requested codec on any device")? - }; + // pick the physical device + encode queue family (skip llvmpipe). The scan itself lives in + // [`find_encode_device`] so the negotiation-time probe asks the SAME question this open + // asks — a probe that MIRRORS a dispatch goes stale the first time the dispatch grows a + // case, which is the failure `open_video`'s backend-label note already records. + let (pd, encode_family) = + find_encode_device(&instance, &instance.enumerate_physical_devices()?, codec_op) + .context("no VK_KHR_video_encode queue for the requested codec on any device")?; let mem_props = instance.get_physical_device_memory_properties(pd); // a compute queue family for the CSC (usually family 0) + its timestamp support (the diff --git a/crates/pf-encode/src/lib.rs b/crates/pf-encode/src/lib.rs index c40ac25b..19ebccfc 100644 --- a/crates/pf-encode/src/lib.rs +++ b/crates/pf-encode/src/lib.rs @@ -855,18 +855,31 @@ fn vulkan_encode_enabled() -> bool { /// negotiation (`OutputFormat::nv12_native` → `ZeroCopyPolicy::native_nv12_session`), which /// then PREFERS gamescope's producer-side NV12 pod (default-on; `PUNKTFUNK_PIPEWIRE_NV12=0` /// escapes at the capture gate). +/// +/// This verdict is load-bearing in a way the other capability helpers are not: once the producer +/// has been asked for two-plane NV12 there is **no fallback**. [`open_video`] deliberately makes a +/// failed Vulkan open FATAL for an NV12 capture rather than degrading to libav VAAPI, because VAAPI +/// would import that buffer as packed RGB and stream silent garbage. So a wrong `true` here does +/// not cost quality — it kills the session at its first frame. Hence the real device probe below, +/// and hence the conjuncts are ordered cheapest-first so it only runs when everything else already +/// said yes. #[cfg(target_os = "linux")] pub fn linux_native_nv12_ok(codec: Codec) -> bool { #[cfg(feature = "vulkan-encode")] { matches!(codec, Codec::H265 | Codec::Av1) && vulkan_encode_enabled() - // NVENC/PyroWave prefs never open the Vulkan Video backend; every other pref - // (auto/vaapi/amd/intel/vulkan) tries it first on AMD/Intel — see [`open_video`]. - && !matches!( - pf_host_config::config().encoder_pref.as_str(), - "nvenc" | "nvidia" | "cuda" | "pyrowave" - ) + // Which backend this host actually resolves to. This used to be a denylist of the + // EXPLICIT prefs that skip Vulkan Video ("nvenc"|"nvidia"|"cuda"|"pyrowave"), which + // silently missed the one that matters: the DEFAULT `encoder_pref` is `""`, and `""` + // resolves to `auto`, which on an NVIDIA box opens NVENC. So a stock NVIDIA host passed + // this gate. `linux_zero_copy_is_vaapi` layers the pref on top of the same auto decision + // `open_video` makes, which is exactly what the note on `linux_auto_is_vaapi` says a + // capability probe must consult — and it is what the downstream consumer of this very + // verdict already uses to pick its zero-copy path. + && linux_zero_copy_is_vaapi() + // …and only then ask the GPU. Ordered last on purpose: it opens a Vulkan instance. + && vulkan_encode_available(codec) } #[cfg(not(feature = "vulkan-encode"))] { @@ -875,6 +888,42 @@ pub fn linux_native_nv12_ok(codec: Codec) -> bool { } } +/// Can this GPU + driver actually open a Vulkan Video **encode** session for `codec`? Cached per +/// (selected GPU, codec) — the [`can_encode_10bit`] idiom, with the probe run outside the lock. +/// +/// Only [`linux_native_nv12_ok`] consults this, and only for the no-fallback decision described +/// there. It is deliberately NOT wired into [`open_video`]'s dispatch: that path already degrades +/// to VAAPI on a failed open for every non-NV12 capture, so making it pay for a probe would buy +/// nothing. Prediction and truth stay separate — the probe answers "would it open", the open itself +/// reports what actually did. +#[cfg(all(target_os = "linux", feature = "vulkan-encode"))] +fn vulkan_encode_available(codec: Codec) -> bool { + use std::collections::HashMap; + use std::sync::{Mutex, OnceLock}; + static CACHE: OnceLock>> = OnceLock::new(); + let key = (pf_gpu::selection_key(), codec.label()); + let cache = CACHE.get_or_init(|| Mutex::new(HashMap::new())); + if let Some(v) = cache.lock().unwrap().get(&key) { + return *v; + } + let verdict = vulkan_video::probe_encode_support(codec); + let ok = verdict.is_ok(); + match &verdict { + Ok(()) => tracing::info!( + ?codec, + "Vulkan Video encode probed OK — producer-native NV12 capture is eligible" + ), + Err(why) => tracing::info!( + ?codec, + why = *why, + "Vulkan Video encode unavailable — keeping the packed-RGB capture negotiation \ + (the native-NV12 path has no VAAPI fallback)" + ), + } + cache.lock().unwrap().insert(key, ok); + ok +} + /// Cheap, side-effect-free NVIDIA-presence probe for the `auto` backend selector: the NVIDIA /// kernel driver exposes these device nodes, AMD/Intel boxes have neither. Deliberately does NOT /// create a CUDA context (that would allocate GPU state on every host that merely *might* be