From 9ac3172f5bc412b590b4a34e19e3fc6b24e434b7 Mon Sep 17 00:00:00 2001 From: Christian Schmittel <90287914+christian-wr@users.noreply.github.com> Date: Tue, 29 Sep 2026 19:04:54 +0200 Subject: [PATCH 1/2] fix(compositor): copy the decoder surface before sampling it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `nv12_srvs` built its Y/UV views straight on the ffmpeg D3D11VA output — a texture array slice, addressed through `FirstArraySlice`. Two documented rules say that is not a supported way to read decoded video. `D3D11_BIND_FLAG` is explicit about the first: "you cannot use texture arrays that are created with this flag in calls to `ID3D11Device::CreateShaderResourceView`". Our pool is exactly such an array — `get_hw_format` asks for `initial_pool_size = 32` with `D3D11_BIND_DECODER | D3D11_BIND_SHADER_RESOURCE`. Drivers are free to let the view creation succeed anyway, and most do, which is why this held up everywhere else. The second rule is the one that actually bit. The decoded surface stays the decoder's reference frame, and between the video engine and the 3D pipeline "there is no automatic hazard tracking" — so the shader may sample a surface the decoder is concurrently rewriting. ffmpeg's `ID3D11VideoContext` is the very same object as our immediate context (it comes out of a `QueryInterface` on it), and `SetMultithreadProtected(TRUE)` only makes an individual call atomic, never a sequence. Nothing ordered the decode against our draws. On a Snapdragon X Elite (Adreno X1-85) the editor preview drew opaque black for most of playback. Measured on 1.13, same build, same recording, the two paths selected by an environment variable: 768 black frames out of 792 sampling the decoder surface, 2 out of 545 through the copy — and those two are the transparent frames before the first compose, not black ones. The black was opaque and total, so not "one frame late" but no frame at all. What hid the cause for so long: exporting the same project never produced a single black frame (28 342 verified), because the export drains the GPU every frame through the encoder, which lets the decode finish before the read. Everything that slowed the live loop — one more readback, a lock, a sleep — cut the black proportionally without ever removing it. Four mitigations along those lines were tried and discarded: a `Flush` and then a `D3D11_QUERY_EVENT` wait around the readback, holding `ID3D11Multithread::Enter` across compose and readback (90 % black down to 57 %), and capping the loop period (69 % at 0 ms, 38 % at 33 ms). They were all treating the symptom. The fix copies the slice into a private texture — `ArraySize = 1`, `BIND_SHADER_RESOURCE` only — with `CopySubresourceRegion` on the immediate context, which IS ordered against the draws that follow, and samples that copy. One allocation per decoder texture, then one GPU→GPU copy per frame and per source; nothing goes back to system memory. `clear_srv_cache` keeps its meaning and now also drops the copies, so a new decoder texture landing on a recycled address cannot inherit one sized for the old one. Export output is unchanged: the same project re-exported frame for frame identical, 326 frames, mean luminance 168.0, min 84.3. This also supersedes the diagnosis in `feat/compositor-force-cpu-backend`, which read the same symptom as a broken D3D11 hardware path on that adapter and added `OPENSCREEN_FORCE_CPU_BACKEND` to work around it. The compose path was never at fault — the export proves it on the same GPU — so that override is no longer the answer here. Not covered: no regression test. The failure only appears where the video engine and the 3D pipeline actually race, so it does not reproduce on the CI adapters, and a test that passes everywhere would prove nothing. The change is exercised by the existing compose and export tests; the evidence above is the A/B on the affected hardware. Refs: - https://learn.microsoft.com/windows/win32/api/d3d11/ne-d3d11-d3d11_bind_flag - https://learn.microsoft.com/windows/win32/direct3d11/overviews-direct3d-11-devices-intro#threading-considerations - https://learn.microsoft.com/windows/win32/api/d3d11_4/nn-d3d11_4-id3d11multithread - https://learn.microsoft.com/windows/win32/api/dxgiformat/ne-dxgiformat-dxgi_format --- crates/compositor/src/compositor_windows.rs | 112 ++++++++++++++------ 1 file changed, 82 insertions(+), 30 deletions(-) diff --git a/crates/compositor/src/compositor_windows.rs b/crates/compositor/src/compositor_windows.rs index 59431e383..ec31b943d 100644 --- a/crates/compositor/src/compositor_windows.rs +++ b/crates/compositor/src/compositor_windows.rs @@ -25,7 +25,7 @@ use std::collections::HashMap; use std::ffi::c_void; use windows::core::Interface; use windows::Win32::Graphics::Direct3D::{ - D3D11_SRV_DIMENSION_TEXTURE2DARRAY, D3D_PRIMITIVE_TOPOLOGY_TRIANGLESTRIP, + D3D11_SRV_DIMENSION_TEXTURE2D, D3D_PRIMITIVE_TOPOLOGY_TRIANGLESTRIP, }; use windows::Win32::Graphics::Direct3D11::*; use windows::Win32::Graphics::Dxgi::Common::*; @@ -149,9 +149,11 @@ pub struct Compositor { timeline_t_override: RefCell>, /// Temps programme (secondes de sortie) — cf. `FrameGeometryInput::programme_time`. programme_time: RefCell>, - // cache des SRV décodeur par (texture array, slice) : le pool réutilise ~32 textures, - // donc après warmup plus aucune création de SRV par frame (overhead CPU supprimé). - srv_cache: RefCell>, + /// A private copy of a decoder surface, with its two plane views (Y, UV). + /// Keyed by the decoder texture's pointer. `clear_srv_cache` drops these when a + /// decoder is replaced — otherwise a new texture landing on the SAME address would + /// inherit a copy sized for the old one. + srv_cache: RefCell>, live_params: RefCell, /// Scène pilotée par l'app (contrat) : quand présente, remplace le layout fixture de /// `timeline()`. Voir `scene.rs` / `SceneDescription` (TS). @@ -805,41 +807,91 @@ impl Compositor { *self.scene.borrow_mut() = s; } - /// Crée les SRV Y (R8) et UV (R8G8) sur la tranche d'array de la frame décodeur. + /// The Y (R8) and UV (R8G8) views of the decoder frame — over a private COPY, never + /// over the decoder surface itself. + /// + /// Two documented D3D11 rules rule out the direct path. The first is explicit, on + /// `D3D11_BIND_DECODER`: "you cannot use texture arrays that are created with this + /// flag in calls to ID3D11Device::CreateShaderResourceView". The ffmpeg pool IS such + /// an array (`initial_pool_size` = 32, see `get_hw_format`). The second explains what + /// we were seeing: between the video engine and the 3D pipeline "there is no automatic + /// hazard tracking" — the surface stays the decoder's reference frame, so it may be + /// rewritten WHILE the shader samples it. ffmpeg's `ID3D11VideoContext` is in fact the + /// SAME object as our immediate context (it comes out of a QueryInterface on it), and + /// `SetMultithreadProtected(TRUE)` only makes an individual call atomic, never a + /// sequence. + /// + /// Measured on a Snapdragon X Elite (Adreno X1-85), same build, same recording, the + /// path picked by an environment variable: 768 black frames out of 792 when sampling + /// the decoder surface, 2 out of 545 through the copy. The black was OPAQUE and total, + /// so not "one frame late" but no frame at all. + /// + /// `CopySubresourceRegion` is issued on the immediate context, so it IS ordered against + /// the draws that follow; the destination texture is `ArraySize = 1` and carries only + /// `BIND_SHADER_RESOURCE`, which also takes it out of the restriction above. The cost + /// is one GPU→GPU copy per frame and per source, with nothing going back to system + /// memory. + /// + /// What hid the cause for so long: exporting the SAME scene never produced a black + /// frame (28 342 verified), because the export drains the GPU every frame through the + /// encoder. Everything that slowed the live loop — one more readback, a lock, a pause — + /// cut the black proportionally without ever removing it. pub unsafe fn nv12_srvs( &self, frame: *const AVFrame, ) -> Result<(ID3D11ShaderResourceView, ID3D11ShaderResourceView)> { let tex_ptr = (*frame).data[0] as *mut c_void; let slice = (*frame).data[1] as u32; - // cache hit : le pool réutilise les mêmes textures -> zéro création après warmup - let key = (tex_ptr as usize, slice); - if let Some((y, uv)) = self.srv_cache.borrow().get(&key) { - return Ok((y.clone(), uv.clone())); - } - let tex = ID3D11Texture2D::from_raw_borrowed(&tex_ptr) - .ok_or_else(|| anyhow::anyhow!("frame sans texture D3D11"))? + let src = ID3D11Texture2D::from_raw_borrowed(&tex_ptr) + .ok_or_else(|| anyhow::anyhow!("frame has no D3D11 texture"))? .clone(); - let mk = |fmt: DXGI_FORMAT| -> Result { - let mut d = D3D11_SHADER_RESOURCE_VIEW_DESC { - Format: fmt, - ViewDimension: D3D11_SRV_DIMENSION_TEXTURE2DARRAY, - ..Default::default() - }; - d.Anonymous.Texture2DArray = D3D11_TEX2D_ARRAY_SRV { - MostDetailedMip: 0, - MipLevels: 1, - FirstArraySlice: slice, - ArraySize: 1, - }; - let mut srv: Option = None; - self.dev.CreateShaderResourceView(&tex, Some(&d), Some(&mut srv))?; - Ok(srv.unwrap()) + // Cache hit: the pool reuses the same textures, so this allocates once per decoder + // after warmup. The COPY itself happens on EVERY frame — that is what freezes the + // slice's contents before the decoder takes it back. + let key = tex_ptr as usize; + let cached = self.srv_cache.borrow().get(&key).cloned(); + let (dst, y, uv) = match cached { + Some(v) => v, + None => { + let mut sd = D3D11_TEXTURE2D_DESC::default(); + src.GetDesc(&mut sd); + let dd = D3D11_TEXTURE2D_DESC { + Width: sd.Width, + Height: sd.Height, + MipLevels: 1, + ArraySize: 1, + Format: sd.Format, + SampleDesc: DXGI_SAMPLE_DESC { Count: 1, Quality: 0 }, + Usage: D3D11_USAGE_DEFAULT, + BindFlags: D3D11_BIND_SHADER_RESOURCE.0 as u32, + CPUAccessFlags: 0, + MiscFlags: 0, + }; + let mut dst: Option = None; + self.dev.CreateTexture2D(&dd, None, Some(&mut dst))?; + let dst = dst.unwrap(); + let mk = |fmt: DXGI_FORMAT| -> Result { + let mut d = D3D11_SHADER_RESOURCE_VIEW_DESC { + Format: fmt, + ViewDimension: D3D11_SRV_DIMENSION_TEXTURE2D, + ..Default::default() + }; + d.Anonymous.Texture2D = D3D11_TEX2D_SRV { MostDetailedMip: 0, MipLevels: 1 }; + let mut srv: Option = None; + self.dev.CreateShaderResourceView(&dst, Some(&d), Some(&mut srv))?; + Ok(srv.unwrap()) + }; + let y = mk(DXGI_FORMAT_R8_UNORM)?; + let uv = mk(DXGI_FORMAT_R8G8_UNORM)?; + self.srv_cache + .borrow_mut() + .insert(key, (dst.clone(), y.clone(), uv.clone())); + (dst, y, uv) + } }; - let y = mk(DXGI_FORMAT_R8_UNORM)?; - let uv = mk(DXGI_FORMAT_R8G8_UNORM)?; - self.srv_cache.borrow_mut().insert(key, (y.clone(), uv.clone())); + + self.ctx.CopySubresourceRegion(&dst, 0, 0, 0, 0, &src, slice, None); Ok((y, uv)) } From e11e9ab18fd97d3e8897bec693c8134cce8fefca Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Wed, 30 Sep 2026 09:55:39 +0200 Subject: [PATCH 2/2] fix(compositor): do not reuse a decoder copy across a resized source The copy cache is keyed by the source texture's address and no longer holds a reference to it. A decoder replaced without clear_srv_cache (mid-stream resolution change, CpuFrames::ensure_tex) can hand a new texture the address of an old one; a larger source then met a smaller destination and CopySubresourceRegion dropped the copy silently, freezing the picture. Check the destination's size and format against the source on every hit and reallocate on mismatch. Adds a regression test that re-keys a small entry under a larger source's address. --- crates/compositor/src/compositor_windows.rs | 78 +++++++++++++++++++-- crates/compositor/src/cpu_frames_windows.rs | 5 +- 2 files changed, 75 insertions(+), 8 deletions(-) diff --git a/crates/compositor/src/compositor_windows.rs b/crates/compositor/src/compositor_windows.rs index ec31b943d..b926f732b 100644 --- a/crates/compositor/src/compositor_windows.rs +++ b/crates/compositor/src/compositor_windows.rs @@ -150,9 +150,9 @@ pub struct Compositor { /// Temps programme (secondes de sortie) — cf. `FrameGeometryInput::programme_time`. programme_time: RefCell>, /// A private copy of a decoder surface, with its two plane views (Y, UV). - /// Keyed by the decoder texture's pointer. `clear_srv_cache` drops these when a - /// decoder is replaced — otherwise a new texture landing on the SAME address would - /// inherit a copy sized for the old one. + /// Keyed by the decoder texture's pointer, which pins nothing: `nv12_srvs` re-checks + /// the copy's size and format on every hit, so a new texture landing on an old address + /// is never handed a copy sized for the old one. `clear_srv_cache` frees stale entries. srv_cache: RefCell>, live_params: RefCell, /// Scène pilotée par l'app (contrat) : quand présente, remplace le layout fixture de @@ -849,13 +849,23 @@ impl Compositor { // Cache hit: the pool reuses the same textures, so this allocates once per decoder // after warmup. The COPY itself happens on EVERY frame — that is what freezes the // slice's contents before the decoder takes it back. + // + // The key is a bare address and pins nothing. A decoder replaced without + // `clear_srv_cache` (a mid-stream resolution change, `CpuFrames::ensure_tex`) can + // hand a NEW texture the address of an old one, and a copy into a smaller destination + // is dropped without an error: the picture freezes. So a hit only counts while the + // destination still has the source's size and format. + let mut sd = D3D11_TEXTURE2D_DESC::default(); + src.GetDesc(&mut sd); let key = tex_ptr as usize; - let cached = self.srv_cache.borrow().get(&key).cloned(); + let cached = self.srv_cache.borrow().get(&key).cloned().filter(|(dst, ..)| { + let mut dd = D3D11_TEXTURE2D_DESC::default(); + dst.GetDesc(&mut dd); + (dd.Width, dd.Height, dd.Format) == (sd.Width, sd.Height, sd.Format) + }); let (dst, y, uv) = match cached { Some(v) => v, None => { - let mut sd = D3D11_TEXTURE2D_DESC::default(); - src.GetDesc(&mut sd); let dd = D3D11_TEXTURE2D_DESC { Width: sd.Width, Height: sd.Height, @@ -3301,6 +3311,62 @@ mod tests { ); } + /// Un NV12 sans données, à la taille voulue : tout ce que `nv12_srvs` lit d'une frame. + fn nv12_frame(gpu: &crate::d3d::Gpu, w: u32, h: u32) -> (Box, ID3D11Texture2D) { + let desc = D3D11_TEXTURE2D_DESC { + Width: w, + Height: h, + MipLevels: 1, + ArraySize: 1, + Format: DXGI_FORMAT_NV12, + SampleDesc: DXGI_SAMPLE_DESC { Count: 1, Quality: 0 }, + Usage: D3D11_USAGE_DEFAULT, + BindFlags: D3D11_BIND_SHADER_RESOURCE.0 as u32, + CPUAccessFlags: 0, + MiscFlags: 0, + }; + unsafe { + let mut tex: Option = None; + gpu.device.CreateTexture2D(&desc, None, Some(&mut tex)).expect("texture NV12"); + let tex = tex.expect("texture NV12"); + let mut frame: Box = Box::new(std::mem::zeroed()); + frame.data[0] = tex.as_raw() as *mut u8; + frame.data[1] = std::ptr::null_mut(); + (frame, tex) + } + } + + /// Un décodeur remplacé sans `clear_srv_cache` (changement de résolution en cours de flux, + /// `CpuFrames::ensure_tex`) peut donner à une texture NEUVE l'adresse d'une ancienne. La + /// copie privée gardée pour l'ancienne est alors trop petite, `CopySubresourceRegion` la + /// saute sans erreur et l'image gèle. On ne peut pas choisir l'adresse d'une texture ; + /// l'entrée de la petite est donc replacée sous la clé de la grande, l'état exact où le + /// cache se retrouve après un tel recyclage. + #[test] + fn a_larger_source_at_a_cached_address_gets_a_copy_of_its_own_size() { + let Ok(gpu) = crate::d3d::Gpu::create_auto(false) else { + eprintln!("pas de device D3D11 — test sauté"); + return; + }; + let comp = Compositor::new_sized(&gpu, 320, 180).expect("compositeur"); + let (small, _small_tex) = nv12_frame(&gpu, 64, 64); + let (large, _large_tex) = nv12_frame(&gpu, 128, 96); + let (small_key, large_key) = (small.data[0] as usize, large.data[0] as usize); + + unsafe { comp.nv12_srvs(&*small) }.expect("source de 64x64"); + { + let mut cache = comp.srv_cache.borrow_mut(); + let stale = cache.remove(&small_key).expect("entrée de la petite source"); + cache.insert(large_key, stale); + } + unsafe { comp.nv12_srvs(&*large) }.expect("source de 128x96"); + + let (dst, ..) = comp.srv_cache.borrow().get(&large_key).cloned().expect("entrée remplacée"); + let mut dd = D3D11_TEXTURE2D_DESC::default(); + unsafe { dst.GetDesc(&mut dd) }; + assert_eq!((dd.Width, dd.Height), (128, 96), "la copie a gardé la taille de l'ancienne texture"); + } + /// Un cran de padding en format Auto change la taille de rendu de la preview. `resized` /// réalloue les cibles et garde le reste : le masque webcam, et la boîte aux lettres où le /// worker de segmentation dépose les suivants. diff --git a/crates/compositor/src/cpu_frames_windows.rs b/crates/compositor/src/cpu_frames_windows.rs index 790f23a7f..ea0e921fb 100644 --- a/crates/compositor/src/cpu_frames_windows.rs +++ b/crates/compositor/src/cpu_frames_windows.rs @@ -41,8 +41,9 @@ pub(crate) struct CpuFrames { /// NV12 en mémoire système : la cible de swscale, la source de l'upload. nv12: *mut AVFrame, /// La texture NV12 échantillonnée par les shaders. UNE seule, réécrite à chaque - /// frame — le `srv_cache` du compositeur (clé `(ptr, slice)`) n'a donc qu'une entrée - /// et ne recrée jamais de SRV, contrairement au pool tournant de D3D11VA. + /// frame — le `srv_cache` du compositeur (clé : le pointeur de texture) n'a donc qu'une + /// entrée et ne recrée jamais de SRV, contrairement au pool tournant de D3D11VA. Quand + /// `ensure_tex` la réalloue à une autre taille, `nv12_srvs` voit l'écart et refait la copie. // ponytail: une seule texture = le CPU peut attendre que le GPU ait fini de lire la // frame précédente. Sur WARP tout est CPU et le pilote sérialise déjà ; si un backend // GPU réutilise ce chemin un jour et que le Map bloque, double-bufferiser ici.