From 87d7eceabf4ed3cac4630ee136e2af2d6e19ae6a Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Tue, 21 Jul 2026 10:18:41 +0200 Subject: [PATCH] fix(encode/qsv): write the colour VUI unconditionally + Intel-lane colour diagnostics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The native QSV encoder only attached mfxExtVideoSignalInfo on HDR sessions — an SDR stream carried no colour description at all (NVENC writes 709-limited unconditionally; qsv.rs now mirrors it). Diagnostics for the field-reported blue/magenta Intel-host colours: - hdr-p010-selftest takes an optional WxH + GPU vendor (intel|nvidia|amd) and prints which adapter it tested — 64x64 on the default GPU proved nothing on dual-GPU boxes, and the field heights (1080/1400) are not 16-aligned. - pf-capture: ignored live test running the converter at 1920x1080 pinned to the Intel adapter. - pf-encode: qsv_live_p010_1080_colorbars_dump — known P010 bars through the UNALIGNED-height ingest copy (1080 src -> align16 1088 pool, the seam no 640x480 test exercises) to Main10 HEVC, dumped for off-box decode checks. - dxgi.rs: drop the stale 'falls back to the R10 path' claim (no such path). Co-Authored-By: Claude Fable 5 --- crates/pf-capture/src/windows/dxgi.rs | 92 +++++++++-- crates/pf-encode/src/enc/windows/qsv.rs | 193 +++++++++++++++++++++++- crates/punktfunk-host/src/main.rs | 28 +++- 3 files changed, 292 insertions(+), 21 deletions(-) diff --git a/crates/pf-capture/src/windows/dxgi.rs b/crates/pf-capture/src/windows/dxgi.rs index 0e101624..4586a28a 100644 --- a/crates/pf-capture/src/windows/dxgi.rs +++ b/crates/pf-capture/src/windows/dxgi.rs @@ -308,8 +308,9 @@ float2 main(float4 pos : SV_POSITION, float2 uv : TEXCOORD0) : SV_TARGET { /// Plane writes use per-plane render-target views of the single P010 texture: an `R16_UNORM` RTV /// selects plane 0 (luma, full WxH), an `R16G16_UNORM` RTV selects plane 1 (chroma, W/2 x H/2). This /// planar-RTV mechanism needs a D3D11.3+ runtime + driver support; [`HdrP010Converter::convert`] -/// surfaces a clear error if `CreateRenderTargetView` rejects the plane format so the caller can fall -/// back to the existing R10 path. +/// surfaces a clear error if `CreateRenderTargetView` rejects the plane format. (There is no runtime +/// fallback — the error propagates through `try_consume` and ends the session; the "R10 path" the +/// original design referenced was never kept.) pub(crate) struct HdrP010Converter { vs: ID3D11VertexShader, ps_y: ID3D11PixelShader, @@ -737,14 +738,28 @@ fn p010_reference(r: f64, g: f64, b: f64) -> (f64, f64, f64) { /// Y ≤ 4 codes, U/V ≤ 5 codes (rounding + chroma averaging). Prints a per-colour table + PASS/FAIL. #[cfg(target_os = "windows")] pub fn hdr_p010_selftest() -> Result<()> { - use windows::Win32::Graphics::Direct3D::D3D_DRIVER_TYPE_HARDWARE; - use windows::Win32::Graphics::Dxgi::IDXGIAdapter; + hdr_p010_selftest_at(64, 64, None) +} - // 64x64, even dims. A 4x4 grid of 16x16 flat scRGB blocks (each 2x2 chroma footprint uniform → - // exact chroma comparison) covering pure R/G/B/white/black/gray at plausible HDR nit levels, plus - // a couple of bright (>1.0 scRGB) colours, then the rest is a gradient (compared on Y only). - const W: u32 = 64; - const H: u32 = 64; +/// [`hdr_p010_selftest`] at an arbitrary even size and (optionally) on a specific GPU vendor +/// (PCI vendor id, e.g. `0x8086` Intel / `0x10de` NVIDIA / `0x1002` AMD). The size matters on +/// top of the 64×64 default because the field sessions run at capture resolutions whose height +/// is NOT 16-aligned (1080 → the encoder's align16 pool seam) and a driver may treat the planar +/// RTVs differently at real sizes; the vendor pin matters on dual-GPU boxes where the default +/// adapter is not the one the session encodes on. +#[cfg(target_os = "windows")] +pub fn hdr_p010_selftest_at(w: u32, h: u32, vendor: Option) -> Result<()> { + use windows::Win32::Graphics::Direct3D::{D3D_DRIVER_TYPE_HARDWARE, D3D_DRIVER_TYPE_UNKNOWN}; + use windows::Win32::Graphics::Dxgi::{CreateDXGIFactory1, IDXGIAdapter, IDXGIFactory1}; + + if w == 0 || h == 0 || w % 2 != 0 || h % 2 != 0 { + bail!("hdr-p010-selftest needs even non-zero dimensions, got {w}x{h}"); + } + // A grid of 16x16 flat scRGB blocks (each 2x2 chroma footprint uniform → exact chroma + // comparison) covering pure R/G/B/white/black/gray at plausible HDR nit levels, plus a couple + // of bright (>1.0 scRGB) colours, then the rest is a gradient (compared on Y only). + #[allow(non_snake_case)] + let (W, H) = (w, h); const BLK: u32 = 16; // (name, r, g, b) scRGB linear (1.0 = 80 nits). Mix of SDR-ish and HDR (>1.0) values. let named: [(&str, f32, f32, f32); 8] = [ @@ -797,12 +812,36 @@ pub fn hdr_p010_selftest() -> Result<()> { // `fp16` outlives the synchronous `CreateTexture2D` that reads it. The mapped-pointer reads are // proven individually at the `read_u16` closure below. unsafe { - // Hardware D3D11 device (no adapter pin — the default GPU is fine for the self-test). + // Device on the requested vendor's adapter (dual-GPU boxes encode on a specific one), else + // the default hardware GPU. Always says which adapter ran — a PASS is only meaningful for + // the GPU it actually tested. + let adapter: Option = match vendor { + None => None, + Some(want) => { + let factory: IDXGIFactory1 = CreateDXGIFactory1().context("dxgi factory")?; + let mut found = None; + for i in 0.. { + let Ok(a) = factory.EnumAdapters(i) else { + break; + }; + let desc = a.GetDesc().context("adapter desc")?; + if desc.VendorId == want { + found = Some(a); + break; + } + } + Some(found.with_context(|| format!("no adapter with vendor id {want:#x}"))?) + } + }; let mut device: Option = None; let mut context: Option = None; D3D11CreateDevice( - None::<&IDXGIAdapter>, - D3D_DRIVER_TYPE_HARDWARE, + adapter.as_ref(), + if adapter.is_some() { + D3D_DRIVER_TYPE_UNKNOWN + } else { + D3D_DRIVER_TYPE_HARDWARE + }, HMODULE::default(), D3D11_CREATE_DEVICE_BGRA_SUPPORT, Some(&[D3D_FEATURE_LEVEL_11_0]), @@ -814,6 +853,22 @@ pub fn hdr_p010_selftest() -> Result<()> { .context("D3D11CreateDevice(hardware) for hdr-p010-selftest")?; let device = device.context("null device")?; let context = context.context("null context")?; + { + let dxgi: windows::Win32::Graphics::Dxgi::IDXGIDevice = + device.cast().context("device -> IDXGIDevice")?; + let desc = dxgi.GetAdapter().context("GetAdapter")?.GetDesc()?; + let name = String::from_utf16_lossy( + &desc.Description[..desc + .Description + .iter() + .position(|&c| c == 0) + .unwrap_or(desc.Description.len())], + ); + println!( + "adapter: {name} (vendor {:#06x}, luid {:08x}:{:08x})", + desc.VendorId, desc.AdapterLuid.HighPart, desc.AdapterLuid.LowPart + ); + } // Source FP16 texture (initialized) + SRV. let src_desc = D3D11_TEXTURE2D_DESC { @@ -1175,3 +1230,16 @@ impl VideoConverter { blt.context("VideoProcessorBlt") } } + +#[cfg(test)] +mod hdr_selftests { + /// LIVE (needs the GPU): [`super::hdr_p010_selftest_at`] at the field capture size — 1080 is + /// NOT 16-aligned, and the planar-RTV write path is driver-specific per vendor. Pinned to the + /// Intel adapter (`0x8086`), so it runs on the Intel validation boxes and errors out cleanly + /// ("no adapter") elsewhere. `cargo test -p pf-capture -- --ignored hdr_p010 --nocapture`. + #[test] + #[ignore] + fn hdr_p010_selftest_intel_1080_live() { + super::hdr_p010_selftest_at(1920, 1080, Some(0x8086)).expect("hdr p010 selftest @1080"); + } +} diff --git a/crates/pf-encode/src/enc/windows/qsv.rs b/crates/pf-encode/src/enc/windows/qsv.rs index 6fe26bb7..121b73d6 100644 --- a/crates/pf-encode/src/enc/windows/qsv.rs +++ b/crates/pf-encode/src/enc/windows/qsv.rs @@ -478,10 +478,14 @@ fn build_params(cfg: &EncodeConfig) -> ParamSet { b }); - // HDR signalling (10-bit sessions are the HDR path on Windows — same coupling as NVENC): - // BT.2020/PQ colour description + the source's mastering/CLL grade at every IDR. + // Colour signalling, written UNCONDITIONALLY (mirrors nvenc_core.rs): the input is already + // CSC'd to a specific matrix — BT.709 limited for SDR (the capture-side VideoConverter), + // BT.2020 PQ for HDR (HdrP010Converter) — so the stream must say so. An SDR stream without a + // colour description leaves the choice to the decoder's "unspecified" default, and + // Moonlight/third-party/Android-vendor decoders default to 601 at sub-HD → mis-rendered + // colours. (10-bit sessions are the HDR path on Windows — same coupling as NVENC.) let hdr = cfg.ten_bit && cfg.codec != Codec::H264; - let vsi = hdr.then(|| { + let vsi = { // SAFETY: all-zero is valid; header stamped below. let mut b: Box = Box::new(unsafe { std::mem::zeroed() }); b.Header.BufferId = vpl::MFX_EXTBUFF_VIDEO_SIGNAL_INFO as u32; @@ -489,11 +493,17 @@ fn build_params(cfg: &EncodeConfig) -> ParamSet { b.VideoFormat = 5; // unspecified b.VideoFullRange = 0; b.ColourDescriptionPresent = 1; - b.ColourPrimaries = 9; // BT.2020 - b.TransferCharacteristics = 16; // SMPTE ST 2084 (PQ) - b.MatrixCoefficients = 9; // BT.2020 non-constant - b - }); + if hdr { + b.ColourPrimaries = 9; // BT.2020 + b.TransferCharacteristics = 16; // SMPTE ST 2084 (PQ) + b.MatrixCoefficients = 9; // BT.2020 non-constant + } else { + b.ColourPrimaries = 1; // BT.709 + b.TransferCharacteristics = 1; // BT.709 + b.MatrixCoefficients = 1; // BT.709 + } + Some(b) + }; let mastering = cfg.hdr_meta.filter(|_| hdr).map(|m| { // SAFETY: all-zero is valid; header stamped below. let mut b: Box = @@ -1994,4 +2004,171 @@ mod tests { "the bitrate retarget emitted a keyframe (StartNewSequence leak)" ); } + + /// FULL-CHAIN colour check at the field capture size: a known P010 colour-bar source at + /// 1920x1080 — whose height is NOT 16-aligned, so the ingest `CopySubresourceRegion` copies + /// into a 1920x1088 runtime pool surface whose chroma plane sits at a DIFFERENT row offset + /// than the source's (the seam no 640x480 test exercises) — encoded to Main10 HEVC and + /// dumped to `%TEMP%\pf_qsv_1080_bars.h265` for off-box decode verification against the + /// same codes. On-box this asserts stream shape; the pixel verdict needs a decoder. + #[test] + fn qsv_live_p010_1080_colorbars_dump() { + use windows::Win32::Graphics::Direct3D::D3D_DRIVER_TYPE_UNKNOWN; + use windows::Win32::Graphics::Direct3D11::{ + D3D11CreateDevice, D3D11_BIND_RENDER_TARGET, D3D11_BIND_SHADER_RESOURCE, + D3D11_SDK_VERSION, D3D11_SUBRESOURCE_DATA, D3D11_USAGE_DEFAULT, + }; + use windows::Win32::Graphics::Dxgi::Common::{DXGI_FORMAT_P010, DXGI_SAMPLE_DESC}; + use windows::Win32::Graphics::Dxgi::{CreateDXGIFactory1, IDXGIAdapter1, IDXGIFactory4}; + + // (Y, Cb, Cr) 10-bit limited codes for the 8 sRGB bars white/yellow/cyan/green/magenta/ + // red/blue/black at 80-nit SDR white under PQ/BT.2020 — the same math as pf-capture's + // `p010_reference` (and the bars_pq2020 client fixture). Stored MSB-aligned (`<<6`). + const BARS: [(u16, u16, u16); 8] = [ + (490, 512, 512), + (478, 423, 518), + (464, 525, 473), + (450, 432, 476), + (350, 584, 585), + (325, 448, 598), + (226, 650, 535), + (64, 512, 512), + ]; + const W: u32 = 1920; + const H: u32 = 1080; + + init_tracing(); + let Ok((_l, impls)) = intel_loader() else { + eprintln!("skipping: no VPL loader"); + return; + }; + let Some(imp) = impls.iter().find(|i| i.luid_valid) else { + eprintln!("skipping: no Intel VPL implementation on this box"); + return; + }; + if !probe_can_encode_10bit(Codec::H265) { + eprintln!("skipping: this GPU declines 10-bit HEVC"); + return; + } + + // P010 initial data: plane 0 = H rows of W u16 luma; plane 1 = H/2 rows of W u16 + // (interleaved Cb,Cr pairs), same pitch. Bars are vertical: bar index = x / (W/8). + let bar_w = (W / 8) as usize; + let mut init = vec![0u16; (W as usize) * (H as usize + H as usize / 2)]; + for y in 0..H as usize { + for x in 0..W as usize { + init[y * W as usize + x] = BARS[(x / bar_w).min(7)].0 << 6; + } + } + let chroma_base = (W as usize) * (H as usize); + for cy in 0..(H as usize / 2) { + for cx in 0..(W as usize / 2) { + let (_, cb, cr) = BARS[((cx * 2) / bar_w).min(7)]; + init[chroma_base + cy * W as usize + cx * 2] = cb << 6; + init[chroma_base + cy * W as usize + cx * 2 + 1] = cr << 6; + } + } + + // SAFETY: self-contained harness on one thread/device (same contract as `drive_live`); + // the initial-data pointer outlives the synchronous CreateTexture2D that reads it. + let (device, tex) = unsafe { + let luid = windows::Win32::Foundation::LUID { + LowPart: u32::from_le_bytes(imp.luid[..4].try_into().unwrap()), + HighPart: i32::from_le_bytes(imp.luid[4..].try_into().unwrap()), + }; + let factory: IDXGIFactory4 = CreateDXGIFactory1().expect("dxgi factory"); + let adapter: IDXGIAdapter1 = factory.EnumAdapterByLuid(luid).expect("intel adapter"); + let mut device = None; + D3D11CreateDevice( + &adapter, + D3D_DRIVER_TYPE_UNKNOWN, + windows::Win32::Foundation::HMODULE::default(), + Default::default(), + None, + D3D11_SDK_VERSION, + Some(&mut device), + None, + None, + ) + .expect("d3d11 device on intel adapter"); + let device: ID3D11Device = device.expect("device"); + let desc = D3D11_TEXTURE2D_DESC { + Width: W, + Height: H, + MipLevels: 1, + ArraySize: 1, + Format: DXGI_FORMAT_P010, + SampleDesc: DXGI_SAMPLE_DESC { + Count: 1, + Quality: 0, + }, + Usage: D3D11_USAGE_DEFAULT, + BindFlags: (D3D11_BIND_RENDER_TARGET.0 | D3D11_BIND_SHADER_RESOURCE.0) as u32, + CPUAccessFlags: 0, + MiscFlags: 0, + }; + let data = D3D11_SUBRESOURCE_DATA { + pSysMem: init.as_ptr() as *const std::ffi::c_void, + SysMemPitch: W * 2, + SysMemSlicePitch: 0, + }; + let mut t: Option = None; + device + .CreateTexture2D(&desc, Some(&data), Some(&mut t)) + .expect("bar texture"); + (device.clone(), t.expect("texture")) + }; + + let mut enc = QsvEncoder::open( + Codec::H265, + PixelFormat::P010, + W, + H, + 30, + 10_000_000, + 10, + ChromaFormat::Yuv420, + ) + .expect("open"); + enc.set_hdr_meta(Some(test_hdr_meta())); + let mut stream = Vec::new(); + let mut aus = 0usize; + let mut keyframes = 0usize; + for i in 0..12u32 { + let frame = CapturedFrame { + width: W, + height: H, + pts_ns: i as u64 * 33_333_333, + format: PixelFormat::P010, + payload: FramePayload::D3d11(pf_frame::dxgi::D3d11Frame { + texture: tex.clone(), + device: device.clone(), + pyro: None, + }), + cursor: None, + }; + enc.submit_indexed(&frame, i).expect("submit"); + if let Some(au) = enc.poll().expect("poll") { + aus += 1; + keyframes += au.keyframe as usize; + stream.extend_from_slice(&au.data); + } + } + enc.flush().expect("flush"); + while let Some(au) = enc.poll().expect("drain") { + aus += 1; + keyframes += au.keyframe as usize; + stream.extend_from_slice(&au.data); + } + assert!(aus >= 10, "expected ≥10 AUs, got {aus}"); + assert!(keyframes >= 1, "expected an IDR in the dump"); + let path = std::env::temp_dir().join("pf_qsv_1080_bars.h265"); + std::fs::write(&path, &stream).expect("write dump"); + println!( + "wrote {} AUs ({} bytes, {keyframes} keyframes) to {}", + aus, + stream.len(), + path.display() + ); + } } diff --git a/crates/punktfunk-host/src/main.rs b/crates/punktfunk-host/src/main.rs index f2159707..76e8906f 100644 --- a/crates/punktfunk-host/src/main.rs +++ b/crates/punktfunk-host/src/main.rs @@ -322,7 +322,33 @@ fn real_main() -> Result<()> { // `PUNKTFUNK_HDR_SHADER_P010` colour math without green-screening a live HDR stream. Prints // PASS/FAIL + max Y/Cb/Cr error. #[cfg(target_os = "windows")] - Some("hdr-p010-selftest") => crate::capture::dxgi::hdr_p010_selftest(), + Some("hdr-p010-selftest") => { + // Optional args: a `WxH` size (default 64x64 — pass the real capture size: heights + // like 1080 are NOT 16-aligned and exercise a different driver path) and a GPU + // vendor (`intel`|`nvidia`|`amd` — dual-GPU boxes otherwise test the default + // adapter, which may not be the one that encodes). + let mut size = (64u32, 64u32); + let mut vendor = None; + for a in args.iter().skip(2) { + match a.as_str() { + "intel" => vendor = Some(0x8086), + "nvidia" => vendor = Some(0x10de), + "amd" => vendor = Some(0x1002), + s => { + let parsed = s + .split_once('x') + .and_then(|(w, h)| Some((w.parse().ok()?, h.parse().ok()?))); + match parsed { + Some(wh) => size = wh, + None => anyhow::bail!( + "hdr-p010-selftest: unrecognized arg {s:?} (want WxH or intel|nvidia|amd)" + ), + } + } + } + } + crate::capture::dxgi::hdr_p010_selftest_at(size.0, size.1, vendor) + } // Linux HDR readiness probe (GNOME 50+ portal path): prints whether a monitor is currently // in BT.2100 (HDR) colour mode, whether the NVENC/VAAPI backend probes Main10 for // HEVC/AV1, and the GameStream HDR capability the two combine into — the "why isn't my