From bbbb7f5723793936b057888960f1ae3ad189160e Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sun, 12 Jul 2026 19:24:12 +0200 Subject: [PATCH] fix+perf(encode): clamp Vulkan CSC to source edge + cache dmabuf imports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two refinements after the initial on-glass validation on RADV (780M): - Green padding bar at non-16-aligned heights (e.g. 1080 → coded 1088): the CSC compute shader read past the edge of the shorter source dmabuf for the 8 alignment-padding rows, producing undefined/green garbage that showed on a client rendering the coded frame. Clamp every source fetch to `textureSize-1` so padding rows duplicate the last real row (invisible, and the SPS conformance window still crops it for a compliant decoder). BT.709 conversion is byte-identical for in-bounds pixels. 5120x1440 (exactly aligned) was never affected. - Per-frame dmabuf import churn: the backend created + imported + destroyed a VkImage every frame (allocation jitter → stutter). PipeWire cycles a small fixed pool, so import each underlying buffer ONCE (keyed by st_dev/st_ino — each frame's fd is a fresh dup of the same buffer) and reuse it, matching the CUDA-path VkBridge. First import acquires from the foreign producer; cached re-reads keep queue ownership and use a plain visibility barrier. On-glass: ~3-6 imports per session then silent (was ~one per frame at 240 Hz), stutter gone at resolutions with headroom. Also adds a PF_SMOKE_W/H override to the headless smoke test to exercise the conformance-window crop path (ffprobe confirms coded 1088 → displayed 1080). Co-Authored-By: Claude Opus 4.8 --- .../src/encode/linux/rgb2yuv.comp | 13 ++- .../src/encode/linux/rgb2yuv.spv | Bin 4376 -> 4688 bytes .../src/encode/linux/vulkan_video.rs | 88 +++++++++++++++--- 3 files changed, 82 insertions(+), 19 deletions(-) diff --git a/crates/punktfunk-host/src/encode/linux/rgb2yuv.comp b/crates/punktfunk-host/src/encode/linux/rgb2yuv.comp index 9fe5d072..1286fd12 100644 --- a/crates/punktfunk-host/src/encode/linux/rgb2yuv.comp +++ b/crates/punktfunk-host/src/encode/linux/rgb2yuv.comp @@ -7,15 +7,20 @@ layout(binding = 2, rg8) uniform writeonly image2D uvImg; // half-res UV (inte float lumaY(vec3 c) { return 16.0/255.0 + 0.1826*c.r + 0.6142*c.g + 0.0620*c.b; } +// Source may be SMALLER than the coded (16-aligned) Y plane — e.g. 1080 source vs 1088 coded. Clamp +// every fetch to the source edge so the alignment-padding rows duplicate the last real row instead +// of reading out of bounds (undefined → green garbage that shows if a client ignores the SPS +// conformance-window crop). `textureSize` gives the bound source's real extent. void main() { ivec2 sz = imageSize(yImg); + ivec2 rmax = textureSize(rgb, 0) - 1; ivec2 uvc = ivec2(gl_GlobalInvocationID.xy); ivec2 p = uvc * 2; if (p.x >= sz.x || p.y >= sz.y) return; - vec3 c00 = texelFetch(rgb, p, 0).rgb; - vec3 c10 = texelFetch(rgb, p + ivec2(1, 0), 0).rgb; - vec3 c01 = texelFetch(rgb, p + ivec2(0, 1), 0).rgb; - vec3 c11 = texelFetch(rgb, p + ivec2(1, 1), 0).rgb; + vec3 c00 = texelFetch(rgb, min(p, rmax), 0).rgb; + vec3 c10 = texelFetch(rgb, min(p + ivec2(1, 0), rmax), 0).rgb; + vec3 c01 = texelFetch(rgb, min(p + ivec2(0, 1), rmax), 0).rgb; + vec3 c11 = texelFetch(rgb, min(p + ivec2(1, 1), rmax), 0).rgb; imageStore(yImg, p, vec4(lumaY(c00), 0, 0, 1)); imageStore(yImg, p + ivec2(1, 0), vec4(lumaY(c10), 0, 0, 1)); imageStore(yImg, p + ivec2(0, 1), vec4(lumaY(c01), 0, 0, 1)); diff --git a/crates/punktfunk-host/src/encode/linux/rgb2yuv.spv b/crates/punktfunk-host/src/encode/linux/rgb2yuv.spv index 321fdef9bb24823c493c462424ba997203f4f0ad..1ebfc6a27ed81af23f0f578ec127b89d271bc6b1 100644 GIT binary patch literal 4688 zcmZve`Inbt7{=e`J5!?}B}#}1DWt^`QITy#l>J2HI};;jnzRgArzrcrhU`@KU6x4J z?2g|Z|AXV$4#(&7exG~1-sA0D=f1A%e%|}JpXK{bmAXCpWm%uBDQnEW&uY)0tPUs3 zj1yAapV*Z3&Ha`cr_LC=c*(r62Oc!uHwI)CL!R4~F%9r~&{>_oKz}kAh_5#YG$uGl zg@1L#&4>m@6yIyWX(HFLw6i*UboT`Z9Ojrx)*r78$VU+uUtW86ANaEAo$Z9Y)_%lA zoz=@~x{|MDQG09t-XzSW-EHv3yubF2xh)-Ct<{d{3%a}7s!Qf~Etr0EUZ;{xBQ8vQ zHnDB|_`F7|d_s+Pi_6Uwgz1Vu5~T(^u#w1 z&&YXwwuQK~ySDHa#<>r}KnwH9M*z0BSMLt@G4J`B_X%Chdv>0|E_uF^&lRFsfkh1*vP(YAhz!qW_%pLhtz3&_De6Q?K5LesuCzy?Qfx&7hG} zJhRW>&gmW%ANR=n?0$Tn5{`9W)mxHhkM8NWpb?Crn)~nXO;31lC$ewWbPs(zv)^zU zh}PVTI10G$q5WnS^`w%XR?yBpyQEu7+V5i2b9{G6uPAB1kHztRAB%c*N&9__yuatQ zCGB_8{1nzZnz%3U_j4h7Bymjgrm{Pul6LGkxPSBfZOlXa`#CUq?dd)qlsxa_tmGXG zkGr=bd57RR)|~UW$9m4=xlINBW@gf7NAjyr1&uXI9Q@_c`V|8L#Ho`Z)z{Ki>Hmb1Gav`Z*2m zd-}gKN6cmXg?+a_|F+s+18}^3*z1`@AA5Do89>io<$rNPay{oCM?4F7chsKE*}%OH zJqPYR*qXV`p94H+*Wp<6aqrHBTW>C7{Vtvd^n9ldc>iYdCFn=2g7Xpxc}>T7*8Wz_ zM~`jrVvqCC_Slo|$^~$}^Fj384lnk7Av*eYy!q&RKHNE??~CAizEke|Vm$N0$C}Z1 zE8Kka-3c%Dy#QV8dm&n{3q;?Sz>9q^LPy_@Hy?d3hC4^}y#%i3JLSHY;+Ypd#lAbx z=A-XR;l;i$Ll^tL9IdwuMBi7yi+x{-j=mjlKKi~2?i|tg)o?xEDffL1o_XO@?Avd& zd3*4i*#q`T+Pk_O*tdD_+_k{Fl^m7|rzub>=Ti<;2b344)&mHJu zKX;<_;_lhc-N3tGKX)Y^{oI4LAN}a(Ubud_ALq8d`RM09c(I@R(ZzlqK=Ti<-#g-77th0u?}J&Wbs2j_kaI7UClJPvn%V$2ip z$AEr}c@l2VG3F__l8ls+mO9m021$qO4b-ka@0sWPXQ9lpX1O020 z_X2z!Fc-C6f?I1Sh*~eh^`q7+@E3u8)Or-`w6AGJP#ZwC5N>r=S(%(>nTL_KSKM*IS7NqQvlOE4*E_vttr z!>tkb{2RDq^jCsa!0(cEt!3W7^@jleZ*9#c;IkDtn4TJm{ek}<*qRt?-Y)sYyEhn5 zKi0fGd;rjov)BQ?4KNqAc7!`e@$L;n>qo7f;6s3Z)Y=(tJ#$fOSGcu`XR#YvKWa6@ UcLDlQYXsbS&ERj=s&-%g0rJ~=Q~&?~ literal 4376 zcmZve`IlF77{{+>riQdBC5j1Cq0(s6Uc?wmp%5`AI)Soaa36_xpM7^LakYz27P8azVEwIVI_yR3<+prO_wp zf|4Zqn#^B@@1Ar`_1f7t%^tOC_2N-u$Bp)ho=KT5jrAg@0$h%@G%RaXH>o$eS|6-3 zGv_GduM7Sdq=Jn6a}_Avi8ZfjX;?VC{hl$C%_&PxLu@0#~LzENQNaF=ssAA8q`U!P6su0Q?jz;_lO@zimqAv)*9LwK(5}_$thN z*HevGGwynZ;+;=@AgdXUcMay}hxw)6%ae<;fVq>YA@@uj&M^vaZtU5mVC$)yFZYb2 zry8*N&f%KuZ$ifVp-uwZE3a1Ct8sh19)Gf4qxW;@U@kf4`TRV*L(M+q-X%xOJHJHJ zb$FMpU!IFE0$YCsx8FUx15@+4(C*H-dT73DaxKnnE!Yu!UCKJrb9UYrN_<+xUxIJx zNIK(t;Pu^p>bz(B;l~#p?#w>`|M{0+JNXg#z9~O;@{i$r`&3VLFTVhvY%e_%@3Z^& zCEj7K`zQDPa@eD1_&w&kFoJ5H_pwY<_uh_Y->m7LI=mx)qEz6mITSw(^G*gIS>S#L zBko*t3VdOKHx~Hv0{45EujltL&$ku0-@|--SC0Gs?k?~{8K27fd}l7fd|#HrtMMZ; zZ5q2VEaT?7fBvTVuFPdd-;*(!rta`AZp`Y81sk`HdnUJzbBxE-=4JQxa`Sy5VodjB5_>gE^j`?}hQ0_cm~T-rM2M+mW6BonSS;Y0*nP*yo~`yHcDa z<`#QtKr^m6^1b*@8joI@!1-Pl!}Gn|16R8{^`(7T0`|G+Whq$A++r`w(2Q%2d@qe~ zl=^SPlCM*!Jh(q7F)Ov-j$~@bJQc}8L;~kInRR4Q;(eI!1f$D&x6fV z-Cy6?b? z>wOnZJ?b6?TUR~mz6W-_#-i5y;Np5eKvR!eAA*ly>apIBz}7PswLSqC*ZV1&der(1 z{4u5;wLS-1&zS2ygjaLVzQBKl9nE+({%dSx#@(-PF!w0-_*<|w;+}s8Hb;Fewhr^V zWL;~0hxyxiIp+V2*7RL*oQh4Lr%L>3n7^;5<73TdWP1MY^+i*UHTMJe#MI*~&II?s zj76=pz|N7sdjsL>QR{4Qe@s1UoddR>v8Z(}*jo9!cOG0lYE^*;Vd_z92-td6*uSh* H?!Np7yHQwy diff --git a/crates/punktfunk-host/src/encode/linux/vulkan_video.rs b/crates/punktfunk-host/src/encode/linux/vulkan_video.rs index bb80e9f0..b41e6cb0 100644 --- a/crates/punktfunk-host/src/encode/linux/vulkan_video.rs +++ b/crates/punktfunk-host/src/encode/linux/vulkan_video.rs @@ -13,9 +13,12 @@ use anyhow::{bail, Context, Result}; use ash::vk; use std::collections::VecDeque; use std::ffi::c_void; -use std::os::fd::IntoRawFd; +use std::os::fd::{AsRawFd, IntoRawFd}; const NV12: vk::Format = vk::Format::G8_B8R8_2PLANE_420_UNORM; +/// Max resident dmabuf imports (comfortably above any PipeWire pool depth; imports alias existing +/// buffers so this holds handles, not new allocations). +const IMPORT_CACHE_CAP: usize = 16; // Prebuilt SPIR-V for the RGB→NV12 BT.709 compute CSC. Source is `rgb2yuv.comp` beside this file; // regenerate with `glslangValidator -V rgb2yuv.comp -o rgb2yuv.spv` after editing the shader. const CSC_SPV: &[u8] = include_bytes!("rgb2yuv.spv"); @@ -81,6 +84,9 @@ pub struct VulkanVideoEncoder { // CPU-input staging (lazily sized) cpu_img: Option<(vk::Image, vk::DeviceMemory, vk::ImageView, vk::Format)>, cpu_stage: Option<(vk::Buffer, vk::DeviceMemory, u64)>, + // Per-buffer dmabuf-import cache, keyed by (st_dev, st_ino) — PipeWire cycles a small fixed pool, + // so each underlying buffer is imported ONCE and reused (no per-frame VkImage create/import/destroy). + import_cache: Vec<(u64, u64, vk::Image, vk::DeviceMemory, vk::ImageView)>, // --- bitstream + submit --- bs_buf: vk::Buffer, @@ -557,6 +563,7 @@ impl VulkanVideoEncoder { nv12_view, cpu_img: None, cpu_stage: None, + import_cache: Vec::new(), bs_buf, bs_mem, bs_size, @@ -683,6 +690,43 @@ impl VulkanVideoEncoder { Ok((img, mem, view)) } + /// Import a dmabuf, reusing a cached per-buffer import when the same underlying buffer recurs + /// (PipeWire cycles a small fixed pool). Keyed by `(st_dev, st_ino)` because each `DmabufFrame` + /// owns a fresh *dup* — a new fd number, same inode. Returns `(image, view, fresh)`; `fresh` is + /// true only on a first import (caller uses UNDEFINED old-layout to preserve modifier-tiled data). + unsafe fn import_cached( + &mut self, + d: &crate::capture::DmabufFrame, + cw: u32, + ch: u32, + ) -> Result<(vk::Image, vk::ImageView, bool)> { + let mut st: libc::stat = std::mem::zeroed(); + let key = if libc::fstat(d.fd.as_raw_fd(), &mut st) == 0 { + (st.st_dev as u64, st.st_ino as u64) + } else { + // fstat failed → uncacheable; a per-frame-unique sentinel key never matches, so this + // frame imports fresh (as before) but is still owned by the cache and freed on evict/Drop. + (u64::MAX, self.enc_count) + }; + if let Some(&(_, _, img, _, view)) = self.import_cache.iter().find(|e| (e.0, e.1) == key) { + return Ok((img, view, false)); + } + let (img, mem, view) = self.import_dmabuf(d, cw, ch)?; + // Bound the cache; evict oldest (FIFO). A stable PipeWire pool never trips this in steady state + // (all imports resident); it only cycles across a pool change (which also rebuilds the session). + while self.import_cache.len() >= IMPORT_CACHE_CAP { + let (_, _, oi, om, ov) = self.import_cache.remove(0); + self.device.destroy_image_view(ov, None); + self.device.destroy_image(oi, None); + self.device.free_memory(om, None); + } + self.import_cache.push((key.0, key.1, img, mem, view)); + // Fires once per distinct pool buffer then goes quiet in steady state — the signal the cache + // is hitting (a per-frame log here would mean inode keying failed and we're re-importing). + tracing::debug!(resident = self.import_cache.len(), "vulkan-encode: imported a new dmabuf buffer"); + Ok((img, view, true)) + } + /// Reusable RGB image + staging buffer for software (CPU) capture; (re)created on format change. unsafe fn ensure_cpu_rgb(&mut self, fmt: vk::Format, bytes: &[u8]) -> Result { let need = (self.width * self.height * 4) as u64; @@ -779,7 +823,6 @@ impl VulkanVideoEncoder { // ---- 2. RGB source -> compute_cmd: prep barriers + CSC + copy into nv12_src ---- let cw = frame.width.min(w); let ch = frame.height.min(h_px); - let mut temp_import: Option<(vk::Image, vk::DeviceMemory, vk::ImageView)> = None; let dev = self.device.clone(); // cheap handle clone -> lets us also call &mut self helpers dev.begin_command_buffer( self.compute_cmd, @@ -789,18 +832,33 @@ impl VulkanVideoEncoder { let rgb_view = match &frame.payload { FramePayload::Dmabuf(d) => { - let (img, mem, view) = self.import_dmabuf(d, frame.width, frame.height)?; - temp_import = Some((img, mem, view)); - // acquire from the foreign (capture) domain; UNDEFINED preserves modifier-tiled data + // Reuse the per-buffer import (PipeWire cycles a small pool) — no per-frame VkImage + // create/import/destroy. The producer wrote new content out-of-band, so still acquire + // from FOREIGN each frame; a fresh import starts UNDEFINED (preserves modifier-tiled + // data), a cached one is already SHADER_READ_ONLY_OPTIMAL. + let (img, view, fresh) = self.import_cached(d, frame.width, frame.height)?; + // First import: acquire from the foreign producer (UNDEFINED preserves the modifier-tiled + // bytes). Cached re-read: we still own it, so no queue-family transfer — just a visibility + // barrier so the shader read sees the content the producer wrote out-of-band this frame + // (single-GPU coherent; the capture layer guarantees the buffer is ready at hand-off). + let (old, src_qf, dst_qf) = if fresh { + (vk::ImageLayout::UNDEFINED, vk::QUEUE_FAMILY_FOREIGN_EXT, self.compute_family) + } else { + ( + vk::ImageLayout::SHADER_READ_ONLY_OPTIMAL, + vk::QUEUE_FAMILY_IGNORED, + vk::QUEUE_FAMILY_IGNORED, + ) + }; let acq = vk::ImageMemoryBarrier2::default() .src_stage_mask(vk::PipelineStageFlags2::NONE) .src_access_mask(vk::AccessFlags2::NONE) .dst_stage_mask(vk::PipelineStageFlags2::COMPUTE_SHADER) .dst_access_mask(vk::AccessFlags2::SHADER_READ) - .old_layout(vk::ImageLayout::UNDEFINED) + .old_layout(old) .new_layout(vk::ImageLayout::SHADER_READ_ONLY_OPTIMAL) - .src_queue_family_index(vk::QUEUE_FAMILY_FOREIGN_EXT) - .dst_queue_family_index(self.compute_family) + .src_queue_family_index(src_qf) + .dst_queue_family_index(dst_qf) .image(img) .subresource_range(color_range(0)); dev.cmd_pipeline_barrier2( @@ -1244,12 +1302,6 @@ impl VulkanVideoEncoder { self.enc_count += 1; self.first_frame = false; self.force_kf = false; - - if let Some((i, m, v)) = temp_import.take() { - dev.destroy_image_view(v, None); - dev.destroy_image(i, None); - dev.free_memory(m, None); - } Ok(()) } } @@ -1322,6 +1374,11 @@ impl Drop for VulkanVideoEncoder { // memory, session params before session, session memory last). unsafe { let _ = self.device.device_wait_idle(); + for (_, _, img, mem, view) in std::mem::take(&mut self.import_cache) { + self.device.destroy_image_view(view, None); + self.device.destroy_image(img, None); + self.device.free_memory(mem, None); + } if let Some((i, m, v, _)) = self.cpu_img.take() { self.device.destroy_image_view(v, None); self.device.destroy_image(i, None); @@ -1682,7 +1739,8 @@ mod tests { #[test] #[ignore = "needs a real VK_KHR_video_encode_h265 device (run on the RADV host, not the build box)"] fn vulkan_smoke() { - let (w, h) = (256u32, 256u32); + let env_dim = |k: &str, d: u32| std::env::var(k).ok().and_then(|v| v.parse().ok()).unwrap_or(d); + let (w, h) = (env_dim("PF_SMOKE_W", 256), env_dim("PF_SMOKE_H", 256)); let mut enc = VulkanVideoEncoder::open(Codec::H265, w, h, 60, 10_000_000).expect("open"); assert!(enc.caps().supports_rfi, "must advertise RFI");