From 63c82526ed1ac34af18fbf1134c217aff197c76a Mon Sep 17 00:00:00 2001 From: Etienne Lescot Date: Thu, 13 Aug 2026 08:42:06 +0200 Subject: [PATCH 1/2] fix(compositor): say when a declared camera is unreadable Both places that open the preview's webcam decoder fell back to opening the screen recording a second time, silently, on any error: let wdec = match Decoder::open(webcam_path, gpu) { Ok(d) => d, Err(_) => Decoder::open(screen_path, gpu)?, }; That collapses two cases which deserve opposite treatment. A clip with no camera arrives with an EMPTY path -- assetCameraSource returns "" -- and the stand-in is harmless there: nothing draws it, because compositing only places a webcam rect when the document declares a camera. Verified end to end today: recording with the camera off and reopening the project shows no stray picture-in-picture. A NON-EMPTY path that fails to open is the other case entirely. The project declares a camera whose file has moved or been deleted, the webcam rect IS drawn, and what appears inside it is the screen recording. That is #265's symptom, still reachable, and nothing in the logs distinguished it from the harmless case. The two sites now share one named function, and the bad case logs the path and the underlying error. Behaviour is otherwise unchanged. ponytail: keeps the stand-in rather than making wdec an Option, which would touch 22 sites including the decoder pool and the unsafe compositing loop -- a refactor of the preview engine, not a cleanup, for a cost nobody has measured. The log is what would tell us whether the wasted decoder is worth that work, and how often the reachable case actually fires. Compile-verified in CI only: the compositor needs thirdparty/ffmpeg, which is not present in a fresh checkout. --- crates/compositor/src/live.rs | 46 +++++++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/crates/compositor/src/live.rs b/crates/compositor/src/live.rs index 015d01d78..047093a89 100644 --- a/crates/compositor/src/live.rs +++ b/crates/compositor/src/live.rs @@ -76,6 +76,42 @@ struct PrefetchedClip { /// Ouvre + positionne la paire de décodeurs d'un clip (même travail que /// `Player::set_active_clip`, mais autonome — sans instance `Player` existante, pour pouvoir /// tourner sur un thread dédié pendant que le `Player` réel joue encore le clip actif). +/// Ouvre le décodeur webcam, ou un remplaçant si le clip n'en a pas. +/// +/// Un clip sans caméra arrive ici avec un chemin VIDE (`assetCameraSource` côté TS renvoie +/// `""`), et le reste du moteur veut une paire de décodeurs toujours valide plutôt qu'un +/// `Option` à dérouler sur tout le chemin chaud. Le remplaçant est donc l'écran lui-même : +/// il n'est jamais dessiné, puisque la composition ne pose une vignette que si le document +/// déclare une caméra. +/// +/// Le cas qui compte est l'autre : un chemin NON VIDE qui ne s'ouvre pas. Cela veut dire +/// que le projet déclare une caméra dont le fichier a été déplacé ou supprimé — et là, la +/// vignette EST dessinée, avec l'enregistrement d'écran dedans. C'est le symptôme de #265, +/// et il était silencieux : aucune trace ne distinguait « pas de caméra » de « caméra +/// introuvable ». +/// +/// ponytail: on garde le remplaçant plutôt que de passer `wdec` en `Option`, ce qui +/// toucherait 22 sites dont le pool et la boucle de composition `unsafe`. À faire si quelqu'un +/// mesure que le décodeur inutile coûte (VRAM des pools D3D11VA, ouverture par clip) — le +/// journal ci-dessous dit enfin combien de fois le mauvais cas se produit. +unsafe fn open_webcam_or_stand_in( + screen_path: &str, + webcam_path: &str, + gpu: &Gpu, +) -> Result { + match Decoder::open(webcam_path, gpu) { + Ok(d) => Ok(d), + Err(e) => { + if !webcam_path.is_empty() { + eprintln!( + "WARNING: caméra déclarée mais illisible ({webcam_path}) : {e}. L'enregistrement d'écran sert de remplaçant, donc la vignette caméra affichera l'écran. Média à relier." + ); + } + Decoder::open(screen_path, gpu) + } + } +} + unsafe fn open_and_seek_clip( screen_path: &str, webcam_path: &str, @@ -85,10 +121,7 @@ unsafe fn open_and_seek_clip( ) -> Result { let source_time_sec = source_time_sec.max(0.0); let mut sdec = Decoder::open(screen_path, gpu)?; - let mut wdec = match Decoder::open(webcam_path, gpu) { - Ok(d) => d, - Err(_) => Decoder::open(screen_path, gpu)?, - }; + let mut wdec = open_webcam_or_stand_in(screen_path, webcam_path, gpu)?; let sf = sdec.seek_to(source_time_sec)?; let mut wf = wdec.seek_to(webcam_seek_time(source_time_sec, webcam_offset_sec))?; if wf.is_null() { @@ -225,10 +258,7 @@ pub struct Player { impl Player { pub unsafe fn open(screen: &str, webcam: &str, gpu: &Gpu) -> Result { - let wdec = match Decoder::open(webcam, gpu) { - Ok(d) => d, - Err(_) => Decoder::open(screen, gpu)?, - }; + let wdec = open_webcam_or_stand_in(screen, webcam, gpu)?; Ok(Player { sdec: Decoder::open(screen, gpu)?, wdec, From e491add8902a010845ffc6c576822489b8b90ae6 Mon Sep 17 00:00:00 2001 From: Etienne Lescot Date: Thu, 13 Aug 2026 19:01:07 +0200 Subject: [PATCH 2/2] fix(compositor): use the shared predicate for "is this a real camera" The previous commit tested `webcam_path.is_empty()`. This crate already answers that question, in `webcam_is_real`, and it covers two cases the shorter test misses: a whitespace-only path, and a caller that passes the screen's own path as the webcam path -- which `ExportDialog.tsx` does and older scenes still contain. So the warning would have fired on a project with no camera whenever the caller used the screen-path spelling, telling the reader their camera file is missing when there is no camera at all. Adding a third convention for "no camera" is also precisely what made this bug class hard to see in the first place. `webcam_is_real`'s own doc comment says it: missing the empty-string case is what put the screen recording inside the picture-in-picture box. Checking it first also skips a decoder open that was always going to fail on the no-camera path, and lets the warning drop its condition. Also unwraps the message from the backslash continuations, which collapsed into one line with a run of spaces in the middle. --- crates/compositor/src/live.rs | 41 +++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/crates/compositor/src/live.rs b/crates/compositor/src/live.rs index 047093a89..c486da8e1 100644 --- a/crates/compositor/src/live.rs +++ b/crates/compositor/src/live.rs @@ -76,37 +76,40 @@ struct PrefetchedClip { /// Ouvre + positionne la paire de décodeurs d'un clip (même travail que /// `Player::set_active_clip`, mais autonome — sans instance `Player` existante, pour pouvoir /// tourner sur un thread dédié pendant que le `Player` réel joue encore le clip actif). -/// Ouvre le décodeur webcam, ou un remplaçant si le clip n'en a pas. +/// Ouvre le décodeur webcam, ou un remplaçant quand le clip n'a pas de caméra. /// -/// Un clip sans caméra arrive ici avec un chemin VIDE (`assetCameraSource` côté TS renvoie -/// `""`), et le reste du moteur veut une paire de décodeurs toujours valide plutôt qu'un -/// `Option` à dérouler sur tout le chemin chaud. Le remplaçant est donc l'écran lui-même : -/// il n'est jamais dessiné, puisque la composition ne pose une vignette que si le document -/// déclare une caméra. +/// La question « ce chemin désigne-t-il une vraie caméra ? » a déjà une réponse dans ce +/// crate : `webcam_is_real`. Elle couvre le chemin vide, le chemin composé d'espaces, et le +/// cas où l'appelant renvoie le chemin de l'écran lui-même — ce que fait `ExportDialog.tsx` +/// et ce que contiennent les scènes plus anciennes. Un simple `is_empty()` en raterait deux +/// sur trois, et le commentaire de `webcam_is_real` rappelle que c'est exactement cet oubli +/// qui avait mis l'enregistrement d'écran dans la vignette caméra (#265). /// -/// Le cas qui compte est l'autre : un chemin NON VIDE qui ne s'ouvre pas. Cela veut dire -/// que le projet déclare une caméra dont le fichier a été déplacé ou supprimé — et là, la -/// vignette EST dessinée, avec l'enregistrement d'écran dedans. C'est le symptôme de #265, -/// et il était silencieux : aucune trace ne distinguait « pas de caméra » de « caméra -/// introuvable ». +/// Sans caméra, le remplaçant est l'écran : le moteur veut une paire de décodeurs toujours +/// valide plutôt qu'un `Option` à dérouler sur tout le chemin chaud, et rien ne le dessine +/// puisque la composition ne pose une vignette que si le document déclare une caméra. +/// +/// Avec une caméra déclarée dont le fichier ne s'ouvre pas, c'est l'inverse : la vignette +/// EST dessinée et affiche l'écran. Ce cas-là méritait une trace, et n'en avait aucune. /// /// ponytail: on garde le remplaçant plutôt que de passer `wdec` en `Option`, ce qui -/// toucherait 22 sites dont le pool et la boucle de composition `unsafe`. À faire si quelqu'un -/// mesure que le décodeur inutile coûte (VRAM des pools D3D11VA, ouverture par clip) — le -/// journal ci-dessous dit enfin combien de fois le mauvais cas se produit. +/// toucherait 22 sites dont le pool de décodeurs et la boucle de composition `unsafe`. À faire +/// si quelqu'un mesure que le décodeur inutile coûte (VRAM des pools D3D11VA, une ouverture +/// par clip) — l'avertissement ci-dessous dit enfin à quelle fréquence le cas visible arrive. unsafe fn open_webcam_or_stand_in( screen_path: &str, webcam_path: &str, gpu: &Gpu, ) -> Result { + if !webcam_is_real(webcam_path, screen_path) { + return Decoder::open(screen_path, gpu); + } match Decoder::open(webcam_path, gpu) { Ok(d) => Ok(d), Err(e) => { - if !webcam_path.is_empty() { - eprintln!( - "WARNING: caméra déclarée mais illisible ({webcam_path}) : {e}. L'enregistrement d'écran sert de remplaçant, donc la vignette caméra affichera l'écran. Média à relier." - ); - } + eprintln!( + "WARNING: caméra déclarée mais illisible ({webcam_path}) : {e}. La vignette caméra affichera l'enregistrement d'écran ; le média est à relier." + ); Decoder::open(screen_path, gpu) } }