From 6af067da2df6cec55835148598e51c2bd1ff49c1 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 31 Jul 2026 23:40:24 +0200 Subject: [PATCH] =?UTF-8?q?fix(client):=20the=20bottom=20rows=20stop=20sme?= =?UTF-8?q?aring=20=E2=80=94=20the=20decode=20pool=20is=20taller=20than=20?= =?UTF-8?q?the=20picture?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A user's 1080p stream repeated its last row of pixels over the final few rows, so the image looked stretched at the bottom. The Vulkan-Video CSC pass sampled the decoded planes with the fullscreen triangle's normalized 0..1 UVs, but its render target is built at the CROPPED frame size. Those are not the same rectangle: FFmpeg sizes the decode pool from `avctx->coded_*`, and H.264 codes `16 * mb_height` — so a 1080-row picture decodes into a 1088-row pool. Destination row 1079 sampled source row ~1087.5, dragging the 8 alignment rows into view and squashing the picture 0.7%. Encoders fill that padding by replicating the last picture line, which is why it reads as a smeared bottom row rather than garbage. Confirmed on glass (.173, RTX, H.264 1080p, vulkan-video): Vulkan Video first frame width=1920 height=1080 pool_w=1920 pool_h=1088 `VkVideoFrame` now carries the pool extent and `record_csc` takes a `uv_scale`, written to the shader's `params.zw` — which the CSC shader already reserved for a use like this. The chroma cositing offset is unchanged and stays correct: `textureSize` reports the pool width, which is the space the scaled UV is already in. Only the Vulkan-Video path passes a scale below 1.0. D3D11VA already clamps this in its VideoProcessor blit (the same bug, seen as a green bar there because DXVA padding is uninitialized rather than replicated); dmabuf imports its planes at the crop over the real stride; PyroWave allocates its ring at exact stream dims. Apple and Android crop at the OS layer. Every other call site passes [1.0, 1.0], so the change is inert there. Also adds a one-time first-frame layout log mirroring the D3D11VA one, so the frame-vs-pool gap is visible in the field instead of having to be re-derived from FFmpeg internals. Co-Authored-By: Claude Opus 5 (1M context) --- crates/pf-client-core/src/video.rs | 15 +++++++ crates/pf-client-core/src/video_vulkan.rs | 38 ++++++++++++++++++ crates/pf-presenter/shaders/nv12_csc.frag | 19 +++++++-- crates/pf-presenter/shaders/nv12_csc.frag.spv | Bin 3780 -> 3864 bytes crates/pf-presenter/src/vk/present.rs | 21 +++++++++- 5 files changed, 89 insertions(+), 4 deletions(-) diff --git a/crates/pf-client-core/src/video.rs b/crates/pf-client-core/src/video.rs index 678f2a8c..96ee8c78 100644 --- a/crates/pf-client-core/src/video.rs +++ b/crates/pf-client-core/src/video.rs @@ -110,6 +110,21 @@ pub struct VkVideoFrame { pub decode_done_value: u64, pub width: u32, pub height: u32, + /// The decode POOL's allocated extent (`AVHWFramesContext.width`/`.height`) — the + /// CODED picture size (rounded up to the codec's macroblock alignment, then to the + /// driver's Vulkan picture-access granularity), so it is `>=` `width`/`height`. At + /// 1080p the pool is 1088 rows tall: 1080 is not a multiple of 16. + /// + /// The presenter samples this image with NORMALIZED coordinates, so it needs both + /// numbers — `width`/`height` is what to display, `coded_*` is what the texture + /// actually spans. Sampling `0..1` without the ratio stretches the alignment padding + /// into view; because encoders fill those rows by replicating the picture's last + /// line, that reads as the bottom row smeared over the final few rows of the image + /// (field report 2026-07-31). Same class as the D3D11VA source-rect clamp in + /// `crate::video_d3d11`, which shows as a green bar there only because DXVA padding + /// is left uninitialized rather than replicated. + pub coded_width: u32, + pub coded_height: u32, pub color: ColorDesc, /// Intra keyframe (IDR/I): the stream's re-anchor point. The pump resumes display on /// one after suppressing the concealed frames a reference loss leaves in its wake (on diff --git a/crates/pf-client-core/src/video_vulkan.rs b/crates/pf-client-core/src/video_vulkan.rs index e76bbdaa..6650bafc 100644 --- a/crates/pf-client-core/src/video_vulkan.rs +++ b/crates/pf-client-core/src/video_vulkan.rs @@ -382,6 +382,13 @@ impl VulkanDecoder { // sem_value was last written by the decode submission on THIS thread. let timeline_sem = (*vkf).sem[0] as u64; let decode_done_value = (*vkf).sem_value[0]; + log_layout_once( + (*self.frame).width, + (*self.frame).height, + (*fc).width, + (*fc).height, + sw, + ); Ok(VkVideoFrame { vkframe: vkf as usize, frames_ctx: fc as usize, @@ -392,6 +399,13 @@ impl VulkanDecoder { decode_done_value, width: (*self.frame).width as u32, height: (*self.frame).height as u32, + // The pool extent, not the frame's: `avcodec_get_hw_frames_parameters` + // sizes it from `coded_width`/`coded_height` and FFmpeg's Vulkan layer + // rounds that up again to the driver's picture-access granularity. The + // `max` is defensive — a pool SMALLER than the frame would mean sampling + // past the surface, so degrade to "no crop" rather than trust it. + coded_width: ((*fc).width.max((*self.frame).width)) as u32, + coded_height: ((*fc).height.max((*self.frame).height)) as u32, color: ColorDesc::from_raw(self.frame), keyframe: frame_is_keyframe(self.frame), guard: DrmFrameGuard(clone), @@ -400,6 +414,30 @@ impl VulkanDecoder { } } +/// One-time dump of the first decoded frame's layout — the forensics for a new GPU/driver. +/// `pool_*` is the allocated decode surface (`>=` the frame); the gap is the alignment +/// padding the presenter's UV scale excludes. The D3D11VA path logs the same pair. +fn log_layout_once( + width: i32, + height: i32, + pool_w: i32, + pool_h: i32, + sw: ffmpeg::ffi::AVPixelFormat, +) { + use std::sync::atomic::{AtomicBool, Ordering}; + static ONCE: AtomicBool = AtomicBool::new(true); + if ONCE.swap(false, Ordering::Relaxed) { + tracing::info!( + width, + height, + pool_w, + pool_h, + ?sw, + "Vulkan Video first frame" + ); + } +} + impl Drop for VulkanDecoder { fn drop(&mut self) { use ffmpeg::ffi; diff --git a/crates/pf-presenter/shaders/nv12_csc.frag b/crates/pf-presenter/shaders/nv12_csc.frag index 8444aaaa..96512eb3 100644 --- a/crates/pf-presenter/shaders/nv12_csc.frag +++ b/crates/pf-presenter/shaders/nv12_csc.frag @@ -15,6 +15,14 @@ // reference white, BT.2020→709 primaries, a soft maxRGB rolloff for highlights // (BT.2390-flavored simplicity, not libplacebo), then sRGB encode. // params.y = tonemap peak (display-relative, ~= peak_nits / 203). +// params.zw = the crop→surface UV scale (frame size / decode-pool size). A Vulkan-Video +// pool image is the CODED surface, taller than the picture whenever the height is +// not a multiple of the driver's alignment (1080 → 1088); sampling the full 0..1 +// would drag those padding rows into view — and since encoders fill them by +// replicating the last picture line, that reads as the bottom row smeared over the +// final few rows. 1.0/1.0 for every path whose image is already crop-sized (dmabuf +// imports the planes at the crop over the real stride; D3D11VA clamps in its +// VideoProcessor blit). // // Regenerate: shaders/build.sh (committed .spv, no build-time toolchain). #version 450 @@ -29,7 +37,7 @@ layout(push_constant) uniform Csc { vec4 r0; vec4 r1; vec4 r2; - vec4 params; // x: mode, y: tonemap peak, z/w: reserved + vec4 params; // x: mode, y: tonemap peak, zw: crop/pool UV scale } pc; // SMPTE ST.2084 (PQ) EOTF: code value → display-referred linear, normalized to 1.0 = @@ -62,17 +70,22 @@ vec3 srgb_oetf(vec3 c) { } void main() { + // Crop to the visible picture: the triangle spans the whole render target, so its 0..1 + // maps onto the pool surface only after this scale (see params.zw above). + vec2 uv = v_uv * pc.params.zw; // 4:2:0 chroma is left-cosited (H.273 type 0 — the default inference when unsignaled, and // what the hosts produce), but sampling the half-res plane at the luma UV assumes CENTER // siting — a ~0.5-luma-px rightward chroma shift on hard colored edges. Offset +0.25 chroma // texels to re-align (the same correction the Apple/Windows clients apply). Self-disables // when the plane widths match (a full-size 4:4:4 chroma plane needs no correction). - vec2 cuv = v_uv; + // textureSize is the POOL's chroma width, which is the space `uv` is already in — so the + // offset stays a true quarter-texel whatever the crop. + vec2 cuv = uv; int cw = textureSize(u_c, 0).x; if (cw < textureSize(u_y, 0).x) { cuv.x += 0.25 / float(cw); } - vec3 yuv = vec3(texture(u_y, v_uv).r, texture(u_c, cuv).rg); + vec3 yuv = vec3(texture(u_y, uv).r, texture(u_c, cuv).rg); vec3 rgb = vec3( dot(pc.r0.xyz, yuv) + pc.r0.w, dot(pc.r1.xyz, yuv) + pc.r1.w, diff --git a/crates/pf-presenter/shaders/nv12_csc.frag.spv b/crates/pf-presenter/shaders/nv12_csc.frag.spv index 4579428a4dbeda78131d050b3453abc18be8ea34..8760f7b6a83125bacd56d5471d8da7b97401e639 100644 GIT binary patch literal 3864 zcmZXWS!`8R6oyY*X_bONlv)(=7EC}3RiI2ks1-~hsT46t!VAGNRi+1Vo>~!sU<9JY z7&U4H1tlmB5wHln7&VcgV2p_pH1WYHF-B1l#qT>k>w1Dacd`HfU&CH!?|n`wD;-ji zBqd2@(mNSflBAQb7BQD%(~=%3Rv+R&jL_^yt#3{%KKpmOE{b`kY_M!Ei4xVS&&7!K24g{g%_ zTs7Lxk6W&ZxB+mru!tLomLxYf1TRRBY-tRhK2LjRkq^ba^u!0(M%`g>wXkaX91gd) zeBUGBQ#Twdu2Tn}UUz1HtaBB-wz{(`>bmFV7?=98=N}3Gby?Mc$n#DeFWp?MI~qQ( z;_&{cQx7-CIDL^fMqJ%lTz@Ql@9B!J@Q;IU8o99{>Wqh*W1K#9pK}6cudZWE2luXdCkyafm`gDAi5YLrG~*Y7jrYAA1X{$bmN)Ax zMmrDFZyouPOzRJC0~avYvS0bFnEPe^QfBk5I{>tb*>#Q2kKmV(c{>i{%)cWGQgeN| z@3bSPKs;Pd|lc5V01=a>6UQj7agPqN>H zu+Yb#7tq5(AB*lC3meOK*M7YV4r~8`(t~*ts&{As)`~TvdZx=T?}2A+jQuUg)LZzH zn6o0&)U9vbHRS7CiCNPa^F80YFwb@Y-1EOD(-xECx!#v)p7)JtZOjj5I#aREL*Nc9 z>Z}HMKKF$b^b z)QPDXAN_2}bf#k7lVHyy@}2^#xmM&o4Oe%B_6%5!uw*9BQNjEy<{HN9k9+hScnRiN z#65Z*tlpBGo94a%R?{5T^*%kp^SGBUV#c^eBg!`Bm$0b26>M!|U(S57=iA}tt4B|- zfX(&2j-Flxt4B{e!D^c0?4EX^8I$knHF(s`_w+itzUXN;*nIWq=}oY?ep{lax4`Pr z)7xM*&2e^5@1Plz?`aP_>gIdei>@zv+6OjYJbVIXYdI)&fo|*&frt9K5LXSAH$-C zzN1*Y$De~;!x7pS;8-`bFEh<|GU|Q>j=JU;6Tag)pKBN!cj5$Ce*@;7_=efxojAe# zHKyjB5PNn$$G2GIh4vkq&u6Xp7Jm;mR$tuhAHc@wi?|=b#_5Z=pTNfHi@2Y`>aHE< z_Y2rP(&zU&`gQhu?VT9K<`{DjGu~&3`+X@se(C)-F79_1&pH+TGBjhRr#apAYT*3R z`=qZ09Q_Q2>u*MjyM8%b-TwVA*9zsXmk@k{@&Z!L3; z^?47Wdp;+bf57@3p`8J%DTgLeD@B{h(=22z Vftzo|f=FU4Z}q literal 3780 zcmZ9OYlv1=6o${tsFRMRQI0by@=XQ`qobCZVmeJ@jKbuYVZ!_o8>7ryX?Bk$46F>( za4e&X-6+!}D@#pDupd$>MOP{+=myb`eyFJx^*pD&J`Zl@b+7kbm%a8r`<&0J%Hh>n zR*}_YXJ_LovV5GIRib2>^To_HS)ZJ@wO`ZTwDQ)4P2(qwHSnCQ$|?5^$f~nm*kb8| zMa$H?z;Q5;ICU1v+OX7z0f&4sa|3YA;1^;$1#hLuhRsd+4S%JSZ55pv94!d)b-5FF)sI|{f~nmT~hmA^7<3ud;YH87ye7(>&HFX9Caqa%`r}&y8E0A?A3LQiTq~l zEbDGQpC{Xj<@21*tX({p8StS^S51j^X2R71QLi0sYQHt*ea?c~adMITRN`@Ho(ll-m~`+6SRa|yl; z+kFM!gbnjSTBHvB>_beT*cLJq1^Hfyt&_d9a_#);dz~{XXzjtC;(rzHjXMJd3o+_e|Zs)m8)d@3=o{4`8czgE>TbMk6>^^@u&mTCt|{F}|?I zH4#)^aEArOE)g7U2z*ZwH zo6SD_4R{Q=hVlAa=*9Qs3E;cqv#?+L+YHn@va|BsCzGb`u&#G&V>-8|u#Itz7S{9q zdKN@o-#u%4?puJqIP>SQ&EEo|rx&oz^_`5KUc^?9o?gOM6OJ=`dKt}_Vo$HYqi(UM zt?2rqr&qDfSC5{yW1G7TL{G0_t4B{eu+@a)%${CHGp5+nPI%NU_Vfn2zUb*qZ1dIQ znZAYX`Pr}jIQzG;t+@-V0CD#3VC&n-L>9ll@s_JTiv@0WJ}bRQ02 zySGMQ54E5Z^yjvC*L3P12@U}L;j2i#3(yRzXIf1^=JnDC{PsnBZ#8_@(r>MGDZW}C zXh4e^ec^s1_4)o=w;#53^hMnoZ2mI)&~N>y`#!e*gUlOJjREk@X0sG;vLi1+vtY}asv_9=F(8`>91^PP;kUt&jHbBqb! zR|TJI7#nxuYi#|^z&r6Rv%@=~?;D`zozV8&-N*MJ@=tw_RrYH z=!>{tu#M9falc|4r!V4u!&Z0gcz(ZQdq(>FUPr&qey_a~qc|L6_5$PGOWg0l_&huB zw{bq#POy)?PG_!1GiD}s+=U@<{_^{z&;JKTKSSaA+tK2l4uh-vyB%>2*yhe7ZY!s1 zpTps<=lb>$Jzj*yU;h7cYnf}T`}IBc`S|We{84P<&wOW#@#e+(j3(GMJTr5x75D7~ pwth!wC$ZI(Lpzl;@8xWE<8RAfz(), 64); self.device.cmd_push_constants( self.cmd_buf,