diff --git a/crates/compositor-view-napi/src/lib.rs b/crates/compositor-view-napi/src/lib.rs index 905a6a5e5..78d351706 100644 --- a/crates/compositor-view-napi/src/lib.rs +++ b/crates/compositor-view-napi/src/lib.rs @@ -11,9 +11,9 @@ use napi::{Env, JsFunction, Task}; use napi_derive::napi; use openscreen_compositor::compositor::{live_params_from_scene, Compositor}; use openscreen_compositor::d3d::{Backend, Gpu}; +use openscreen_compositor::export_control::{ExportCancelled, ExportControl}; use openscreen_compositor::frame_geometry::FootageQuad; use openscreen_compositor::gif_export::{GifExportParams, GifStats}; -use openscreen_compositor::gif_export_control::{GifExportCancelled, GifExportControl}; use openscreen_compositor::live::{LiveView, PausedPreviews}; use openscreen_compositor::scene::Scene; use openscreen_compositor::{config, pipeline}; @@ -597,6 +597,7 @@ pub struct ExportMultiTask { clips: Vec, scene_json: Option, params: Option, + control: ExportControl, on_progress: Option>, } @@ -605,12 +606,14 @@ impl Task for ExportMultiTask { type JsValue = ExportStats; fn compute(&mut self) -> Result { + self.control.check().map_err(mp4_task_error)?; // Previews paused for the whole render (GPU 3D engine freed) and restored // exactly as found when this guard drops, including on the error paths. let _previews = PreviewPause::begin(); // Même sélection que la preview : l'export d'un hôte sans GPU passe par // libopenh264 au lieu d'AMF, plutôt que d'échouer. let gpu = Gpu::create_auto(false).map_err(|e| Error::from_reason(format!("{e:#}")))?; + self.control.check().map_err(mp4_task_error)?; let mut cfg = config::all().pop().expect("au moins une config"); // C8 cfg.zoom = false; cfg.layout_anim = false; @@ -670,7 +673,7 @@ impl Task for ExportMultiTask { comp.set_scene(scene); let mut progress = throttled_progress(self.on_progress.take()); - let s = pipeline::run_composited_multi( + let s = pipeline::run_composited_multi_cancellable( &self.clips, &self.out_path, &gpu, @@ -678,8 +681,9 @@ impl Task for ExportMultiTask { &cfg, &export_params, &mut progress, + &self.control, ) - .map_err(|e| Error::from_reason(format!("{e:#}")))?; + .map_err(mp4_task_error)?; Ok((s.frames as u32, s.wall_s, s.fps, s.video_duration_s)) } @@ -688,11 +692,34 @@ impl Task for ExportMultiTask { } } +/// Le pendant MP4 de `create_gif_export_control` : même contrôle, passé à `export_multi`. Sa +/// présence dit aussi au TS que cet addon sait annuler un MP4 — un `.node` plus ancien ignore le +/// contrôle, et son export va au bout. +#[napi] +pub fn create_mp4_export_control() -> External { + External::new(ExportControl::default()) +} + +#[napi] +pub fn cancel_mp4_export(control: External) -> bool { + control.cancel() +} + +fn mp4_task_error(error: anyhow::Error) -> Error { + if error.is::() { + Error::from_reason("MP4_EXPORT_CANCELLED") + } else { + Error::from_reason(format!("{error:#}")) + } +} + /// Lance un export multiclip natif (vraie timeline → MP4) et résout `Promise`. /// `scene_json` : même `SceneDescription` que la preview (fond/layout/webcam/effets/curseur). /// `params` : taille/cadence/codec de sortie voulus (absent → 1920x1080/fps du 1er clip/h264). /// `on_progress(framesEncodées)` optionnel — rappelé côté JS à ~10 Hz max pendant le rendu ; /// le JS calcule lui-même le pourcentage (il connaît déjà le total attendu, durée×fps des clips). +/// `control` optionnel (`create_mp4_export_control`) : l'annuler arrête le rendu entre deux +/// frames, rejette avec `MP4_EXPORT_CANCELLED` et ne laisse aucun fichier. #[napi] pub fn export_multi( clips: Vec, @@ -700,6 +727,7 @@ pub fn export_multi( scene_json: Option, params: Option, on_progress: Option, + control: Option>, ) -> Result> { let clips = clips .into_iter() @@ -717,6 +745,7 @@ pub fn export_multi( clips, scene_json, params, + control: control.map(|c| (*c).clone()).unwrap_or_default(), on_progress: make_progress_tsfn(on_progress)?, })) } @@ -752,17 +781,17 @@ pub struct GifParamsInput { /// `Compositor` (équivalent de `cfg.cursor = false` dans /// `run_composited_multi`). #[napi] -pub fn create_gif_export_control() -> External { - External::new(GifExportControl::default()) +pub fn create_gif_export_control() -> External { + External::new(ExportControl::default()) } #[napi] -pub fn cancel_gif_export(control: External) -> bool { +pub fn cancel_gif_export(control: External) -> bool { control.cancel() } fn gif_task_error(error: anyhow::Error) -> Error { - if error.is::() { + if error.is::() { Error::from_reason("GIF_EXPORT_CANCELLED") } else { Error::from_reason(format!("{error:#}")) @@ -779,7 +808,7 @@ pub struct ExportGifTask { scene_json: Option, out_path: PathBuf, params: GifExportParams, - control: GifExportControl, + control: ExportControl, on_progress: Option>, } @@ -877,7 +906,7 @@ pub fn export_gif( scene_json: Option, params: Option, on_progress: Option, - control: Option>, + control: Option>, ) -> Result> { // Deliberately the same argument shape as `export_multi`: the caller builds // one clip list and one scene, and picks the container. Cursor comes from diff --git a/crates/compositor/src/gif_export_control.rs b/crates/compositor/src/export_control.rs similarity index 70% rename from crates/compositor/src/gif_export_control.rs rename to crates/compositor/src/export_control.rs index bf4ad1243..8542c55ea 100644 --- a/crates/compositor/src/gif_export_control.rs +++ b/crates/compositor/src/export_control.rs @@ -1,4 +1,4 @@ -//! Cooperative GIF cancellation and publication of a completed output only. +//! Cooperative export cancellation (MP4 and GIF) and publication of a completed output only. use anyhow::{bail, Context, Result}; use std::fs::{self, File, OpenOptions}; @@ -11,20 +11,20 @@ const CANCELLED: u8 = 1; const COMMITTING: u8 = 2; #[derive(Clone, Default)] -pub struct GifExportControl(Arc); +pub struct ExportControl(Arc); #[derive(Debug)] -pub struct GifExportCancelled; +pub struct ExportCancelled; -impl std::fmt::Display for GifExportCancelled { +impl std::fmt::Display for ExportCancelled { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str("GIF export cancelled") + f.write_str("export cancelled") } } -impl std::error::Error for GifExportCancelled {} +impl std::error::Error for ExportCancelled {} -impl GifExportControl { +impl ExportControl { /// False means publication has already won the race. Repeated cancellation /// of the same pending job is harmless. pub fn cancel(&self) -> bool { @@ -36,7 +36,7 @@ impl GifExportControl { pub fn check(&self) -> Result<()> { if self.0.load(Ordering::Acquire) == CANCELLED { - return Err(GifExportCancelled.into()); + return Err(ExportCancelled.into()); } Ok(()) } @@ -44,20 +44,20 @@ impl GifExportControl { fn begin_commit(&self) -> Result<()> { match self.0.compare_exchange(RUNNING, COMMITTING, Ordering::AcqRel, Ordering::Acquire) { Ok(_) => Ok(()), - Err(CANCELLED) => Err(GifExportCancelled.into()), - Err(_) => bail!("GIF export control has already been used"), + Err(CANCELLED) => Err(ExportCancelled.into()), + Err(_) => bail!("export control has already been used"), } } } static NEXT_OUTPUT: AtomicU64 = AtomicU64::new(0); -struct StagedGif { +struct StagedOutput { path: PathBuf, published: bool, } -impl Drop for StagedGif { +impl Drop for StagedOutput { fn drop(&mut self) { if !self.published { let _ = fs::remove_file(&self.path); @@ -66,27 +66,31 @@ impl Drop for StagedGif { } /// Keep the destination intact until rendering and flushing have succeeded. -/// `render` owns the file so it is closed before rename/cleanup on Windows. -pub(crate) fn with_gif_output( +/// `render` owns the file so it is closed before rename/cleanup on Windows; a +/// renderer that reopens the path (ffmpeg) must close it before returning too. +/// The staged name ends with the target's extension, which is what ffmpeg picks +/// the container from. +pub(crate) fn with_staged_output( target: &Path, - control: &GifExportControl, + control: &ExportControl, render: impl FnOnce(File, &Path) -> Result, ) -> Result { control.check()?; let parent = target.parent().filter(|p| !p.as_os_str().is_empty()).unwrap_or(Path::new(".")); fs::create_dir_all(parent)?; + let ext = target.extension().map(|e| e.to_string_lossy()).unwrap_or_default(); let (mut staged, file) = loop { let nonce = NEXT_OUTPUT.fetch_add(1, Ordering::Relaxed); - let path = parent.join(format!(".openscreen-gif-{}-{nonce}.partial", std::process::id())); + let path = parent.join(format!(".openscreen-{ext}-{}-{nonce}.partial.{ext}", std::process::id())); match OpenOptions::new().write(true).create_new(true).open(&path) { - Ok(file) => break (StagedGif { path, published: false }, file), + Ok(file) => break (StagedOutput { path, published: false }, file), Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => continue, - Err(e) => return Err(e).context("creating temporary GIF output"), + Err(e) => return Err(e).context("creating temporary export output"), } }; let result = render(file, &staged.path).and_then(|stats| { control.begin_commit()?; - fs::rename(&staged.path, target).context("publishing completed GIF output")?; + fs::rename(&staged.path, target).context("publishing completed export output")?; staged.published = true; Ok(stats) }); @@ -95,7 +99,7 @@ pub(crate) fn with_gif_output( if cleanup.kind() != std::io::ErrorKind::NotFound { // A failed cleanup is an error, even if cancellation caused it. // Do not report a clean cancellation while leaving an output. - bail!("could not remove partial GIF {}: {cleanup}", staged.path.display()); + bail!("could not remove partial export {}: {cleanup}", staged.path.display()); } } } @@ -128,28 +132,41 @@ mod tests { #[test] fn cancellation_before_rendering_never_opens_output() { let dir = TestDir::new(); - let control = GifExportControl::default(); + let control = ExportControl::default(); assert!(control.cancel()); - let result = with_gif_output(&dir.0.join("out.gif"), &control, |_, _| -> Result<()> { + let result = with_staged_output(&dir.0.join("out.gif"), &control, |_, _| -> Result<()> { panic!("cancelled job must not render"); }); - assert!(result.unwrap_err().is::()); + assert!(result.unwrap_err().is::()); assert_eq!(fs::read_dir(&dir.0).unwrap().count(), 0); } + #[test] + fn staged_output_keeps_the_target_extension() { + // ffmpeg picks the MP4 muxer from the file name; a bare `.partial` cannot be opened. + let dir = TestDir::new(); + let control = ExportControl::default(); + with_staged_output(&dir.0.join("out.mp4"), &control, |_, staged| { + assert_eq!(staged.extension().unwrap(), "mp4"); + Ok(()) + }) + .unwrap(); + dir.assert_only("out.mp4"); + } + #[test] fn cancellation_discards_partial_output_and_preserves_destination() { let dir = TestDir::new(); let target = dir.0.join("out.gif"); fs::write(&target, b"original GIF").unwrap(); - let control = GifExportControl::default(); - let result = with_gif_output(&target, &control, |mut file, _| { + let control = ExportControl::default(); + let result = with_staged_output(&target, &control, |mut file, _| { file.write_all(b"partial replacement")?; assert!(control.cancel()); // Even a renderer that has just completed cannot publish now. Ok(42) }); - assert!(result.unwrap_err().is::()); + assert!(result.unwrap_err().is::()); assert_eq!(fs::read(&target).unwrap(), b"original GIF"); dir.assert_only("out.gif"); } @@ -159,8 +176,8 @@ mod tests { let dir = TestDir::new(); let target = dir.0.join("out.gif"); fs::write(&target, b"original").unwrap(); - let control = GifExportControl::default(); - assert_eq!(with_gif_output(&target, &control, |mut file, _| { + let control = ExportControl::default(); + assert_eq!(with_staged_output(&target, &control, |mut file, _| { file.write_all(b"finished GIF")?; Ok(7) }).unwrap(), 7); @@ -173,8 +190,8 @@ mod tests { fn render_failure_is_not_misreported_as_cancellation() { let dir = TestDir::new(); let target = dir.0.join("out.gif"); - let control = GifExportControl::default(); - let result = with_gif_output(&target, &control, |mut file, _| -> Result<()> { + let control = ExportControl::default(); + let result = with_staged_output(&target, &control, |mut file, _| -> Result<()> { file.write_all(b"partial")?; control.cancel(); bail!("encoder failed") @@ -188,8 +205,8 @@ mod tests { let dir = TestDir::new(); let target = dir.0.join("destination-directory"); fs::create_dir(&target).unwrap(); - let control = GifExportControl::default(); - assert!(with_gif_output(&target, &control, |mut file, _| { + let control = ExportControl::default(); + assert!(with_staged_output(&target, &control, |mut file, _| { file.write_all(b"finished GIF")?; Ok(()) }).is_err()); @@ -200,7 +217,7 @@ mod tests { #[test] fn cancellation_and_commit_have_exactly_one_winner() { for _ in 0..64 { - let control = GifExportControl::default(); + let control = ExportControl::default(); let cancel_control = control.clone(); let barrier = Arc::new(Barrier::new(2)); let cancel_barrier = barrier.clone(); diff --git a/crates/compositor/src/gif_export.rs b/crates/compositor/src/gif_export.rs index 8215704c2..6f4a205cc 100644 --- a/crates/compositor/src/gif_export.rs +++ b/crates/compositor/src/gif_export.rs @@ -74,7 +74,7 @@ use crate::compositor::Compositor; use crate::config::Cfg; use crate::d3d::Gpu; -use crate::gif_export_control::{with_gif_output, GifExportControl}; +use crate::export_control::{with_staged_output, ExportControl}; use crate::pipeline::{ClipSource, Decoder}; use crate::timeline_walk::walk_composited_timeline; use anyhow::{anyhow, bail, Context, Result}; @@ -173,7 +173,7 @@ pub fn export_gif( params: &GifExportParams, progress: &mut dyn FnMut(u64), ) -> Result { - export_gif_cancellable(clips, out_path, gpu, comp, cfg, params, progress, &GifExportControl::default()) + export_gif_cancellable(clips, out_path, gpu, comp, cfg, params, progress, &ExportControl::default()) } pub fn export_gif_cancellable( @@ -184,9 +184,9 @@ pub fn export_gif_cancellable( cfg: &Cfg, params: &GifExportParams, progress: &mut dyn FnMut(u64), - control: &GifExportControl, + control: &ExportControl, ) -> Result { - with_gif_output(out_path, control, |file, staging_path| { + with_staged_output(out_path, control, |file, staging_path| { export_gif_inner(clips, staging_path, file, gpu, comp, cfg, params, progress, control) }) } @@ -200,7 +200,7 @@ fn export_gif_inner( cfg: &Cfg, params: &GifExportParams, progress: &mut dyn FnMut(u64), - control: &GifExportControl, + control: &ExportControl, ) -> Result { control.check()?; if clips.is_empty() { diff --git a/crates/compositor/src/lib.rs b/crates/compositor/src/lib.rs index 767abac8a..b33679705 100644 --- a/crates/compositor/src/lib.rs +++ b/crates/compositor/src/lib.rs @@ -33,11 +33,11 @@ pub mod camera; pub mod config; pub mod cursor; pub mod cursor_sdf; +pub mod export_control; pub mod export_probe; pub mod ffi; pub mod frame_geometry; pub mod gif_export; -pub mod gif_export_control; pub mod regions; // Multiplateforme à dessein : n'utilise que libavformat (liée sur les trois // cibles) et le shim C. Seul Linux l'appelle aujourd'hui, parce que c'est la diff --git a/crates/compositor/src/pipeline_linux.rs b/crates/compositor/src/pipeline_linux.rs index 1c47e0b0f..5dcfde1da 100644 --- a/crates/compositor/src/pipeline_linux.rs +++ b/crates/compositor/src/pipeline_linux.rs @@ -24,6 +24,7 @@ use anyhow::{bail, Result}; use std::collections::HashMap; use std::ffi::CString; +use std::path::Path; use std::ptr; use crate::audio::{ @@ -33,6 +34,7 @@ use crate::audio::{ use crate::audio_jobs::{decode_and_stretch_clip_audio, ClipAudioJobs}; use crate::config::Cfg; use crate::d3d::Gpu; +use crate::export_control::{with_staged_output, ExportControl}; use crate::ffi::AVFrame; use crate::linux_decode::SwDecoder; use crate::timeline_walk::NextFrameTime; @@ -850,6 +852,39 @@ pub fn run_composited_multi( cfg: &Cfg, params: &ExportParams, progress: &mut dyn FnMut(u64), +) -> Result { + run_composited_multi_cancellable(clips, out, gpu, comp, cfg, params, progress, &ExportControl::default()) +} + +/// `run_composited_multi`, arretable entre deux frames par `control` (erreur `ExportCancelled`). +/// Symetrique de `pipeline_windows::run_composited_multi_cancellable` : le MP4 s'ecrit a cote de +/// `out` et n'est renomme par-dessus qu'une fois complet. Le `Drop` de `Muxer` ferme le fichier +/// sur toutes les sorties, annulation comprise. +pub fn run_composited_multi_cancellable( + clips: &[ClipSource], + out: &str, + gpu: &Gpu, + comp: &crate::compositor::Compositor, + cfg: &Cfg, + params: &ExportParams, + progress: &mut dyn FnMut(u64), + control: &ExportControl, +) -> Result { + with_staged_output(Path::new(out), control, |file, staged| { + drop(file); // ffmpeg le rouvre par son nom + run_multi_inner(clips, &staged.to_string_lossy(), gpu, comp, cfg, params, progress, control) + }) +} + +fn run_multi_inner( + clips: &[ClipSource], + out: &str, + gpu: &Gpu, + comp: &crate::compositor::Compositor, + cfg: &Cfg, + params: &ExportParams, + progress: &mut dyn FnMut(u64), + control: &ExportControl, ) -> Result { if clips.is_empty() { bail!("run_composited_multi: aucun clip a exporter"); @@ -1039,6 +1074,7 @@ pub fn run_composited_multi( &mut screen_decs, &mut webcam_decs, &mut |n| { + control.check()?; // Soumet la copie de la frame n SANS l'attendre et recolte la // precedente : c'est tout le pipelining GPU. L'encodage, lui, // n'est plus ici du tout — il tourne sur `worker` pendant que diff --git a/crates/compositor/src/pipeline_macos.rs b/crates/compositor/src/pipeline_macos.rs index ad43084a0..48c775220 100644 --- a/crates/compositor/src/pipeline_macos.rs +++ b/crates/compositor/src/pipeline_macos.rs @@ -36,9 +36,11 @@ use crate::audio::{ use crate::audio_jobs::{decode_and_stretch_clip_audio, ClipAudioJobs}; use crate::compositor::Compositor; use crate::d3d::Gpu; +use crate::export_control::{with_staged_output, ExportControl}; use crate::timeline_walk::NextFrameTime; use anyhow::{anyhow, bail, Result}; use std::ffi::{c_void, CString}; +use std::path::Path; use std::ptr; /// Identique à `pipeline_windows::Stats`. Voir la doc là-bas pour la sémantique. @@ -59,6 +61,33 @@ impl Drop for FrameGuard { } } +/// Garde RAII sur le conteneur de sortie : ferme son fichier puis le libère au Drop, `?` compris. +/// Une annulation est une sortie ordinaire de l'export : sans elle, chaque export annulé +/// laisserait derrière lui un descripteur ouvert et le contexte du muxer. +struct OutputGuard(*mut crate::ffi::AVFormatContext); + +impl Drop for OutputGuard { + fn drop(&mut self) { + unsafe { + let mut pb = crate::ffi::sn_fmt_get_pb(self.0); + if !pb.is_null() { + crate::ffi::avio_closep(&mut pb); + crate::ffi::sn_fmt_set_pb(self.0, ptr::null_mut()); + } + crate::ffi::avformat_free_context(self.0); + } + } +} + +/// Garde RAII sur un AVPacket (le libère au Drop). Identique à `pipeline_windows::PacketGuard`. +struct PacketGuard(*mut crate::ffi::AVPacket); + +impl Drop for PacketGuard { + fn drop(&mut self) { + unsafe { crate::ffi::av_packet_free(&mut self.0) }; + } +} + /// Au-delà de cette distance vers l'avant, `Decoder::seek_to` repart d'une image clé /// plutôt que de dérouler. Identique à `pipeline_windows::SEEK_FORWARD_MAX_SEC` — le /// seuil dépend du GOP des captures, pas du backend de décodage. @@ -1138,6 +1167,38 @@ pub fn run_composited_multi( cfg: &crate::config::Cfg, params: &ExportParams, progress: &mut dyn FnMut(u64), +) -> Result { + run_composited_multi_cancellable(clips, out, gpu, comp, cfg, params, progress, &ExportControl::default()) +} + +/// `run_composited_multi`, arrêtable entre deux frames par `control` (erreur `ExportCancelled`). +/// Symétrique de `pipeline_windows::run_composited_multi_cancellable` : le MP4 s'écrit à côté de +/// `out` et n'est renommé par-dessus qu'une fois complet. +pub fn run_composited_multi_cancellable( + clips: &[ClipSource], + out: &str, + gpu: &Gpu, + comp: &crate::compositor::Compositor, + cfg: &crate::config::Cfg, + params: &ExportParams, + progress: &mut dyn FnMut(u64), + control: &ExportControl, +) -> Result { + with_staged_output(Path::new(out), control, |file, staged| { + drop(file); // ffmpeg le rouvre par son nom + run_multi_inner(clips, &staged.to_string_lossy(), gpu, comp, cfg, params, progress, control) + }) +} + +fn run_multi_inner( + clips: &[ClipSource], + out: &str, + gpu: &Gpu, + comp: &crate::compositor::Compositor, + cfg: &crate::config::Cfg, + params: &ExportParams, + progress: &mut dyn FnMut(u64), + control: &ExportControl, ) -> Result { if clips.is_empty() { bail!("run_composited_multi: aucun clip à exporter"); @@ -1180,6 +1241,7 @@ pub fn run_composited_multi( "alloc_output_context2", )?; } + let _output = OutputGuard(octx); let ostream = unsafe { crate::ffi::avformat_new_stream(octx, ptr::null()) }; if ostream.is_null() { bail!("avformat_new_stream"); @@ -1216,7 +1278,8 @@ pub fn run_composited_multi( let mut audio_jobs: ClipAudioJobs> = ClipAudioJobs::new(clips.len()); let mut clip_frame_counts: Vec = vec![0; clips.len()]; - let mut opkt = unsafe { crate::ffi::av_packet_alloc() }; + let opkt = unsafe { crate::ffi::av_packet_alloc() }; + let _opkt = PacketGuard(opkt); // La marche de timeline est PARTAGÉE (`timeline_walk`) : c'est elle qui décide quelle // frame source appartient à quelle frame de sortie, en tenant compte des régions de @@ -1242,6 +1305,7 @@ pub fn run_composited_multi( &mut screen_decs, &mut webcam_decs, &mut |n| { + control.check()?; enc.send_composited(comp, out_w, out_h, n as i64)?; { let _p = crate::export_probe::scope(crate::export_probe::Stage::DrainMux); @@ -1317,9 +1381,8 @@ pub fn run_composited_multi( crate::ffi::av_write_trailer(octx), "write_trailer", )?; - crate::ffi::avio_closep(&mut pb); - crate::ffi::avformat_free_context(octx); - crate::ffi::av_packet_free(&mut opkt); + // Fichier fermé et contexte libéré par `_output` en fin de portée, avant que + // `with_staged_output` ne renomme le MP4. } let wall_s = t0.elapsed().as_secs_f64(); diff --git a/crates/compositor/src/pipeline_windows.rs b/crates/compositor/src/pipeline_windows.rs index 62755b8f0..0d151ad25 100644 --- a/crates/compositor/src/pipeline_windows.rs +++ b/crates/compositor/src/pipeline_windows.rs @@ -12,6 +12,7 @@ use crate::config::Cfg; use crate::cpu_frames::CpuFrames; use crate::cursor::CursorTrack; use crate::d3d::{Backend, Gpu}; +use crate::export_control::{with_staged_output, ExportControl}; use crate::ffi::*; use crate::regions::{speed_segments_for_window, SpeedSegment}; use crate::scene::Scene; @@ -22,6 +23,7 @@ use crate::timeline_walk::{walk_composited_timeline, NextFrameTime}; use anyhow::{anyhow, bail, Result}; use std::collections::HashMap; use std::ffi::{c_void, CString}; +use std::path::Path; use std::ptr; use std::time::Instant; use windows::core::Interface; @@ -213,6 +215,31 @@ impl Drop for PacketGuard { } } +/// Garde RAII sur le conteneur de sortie : ferme son fichier puis le libère au Drop, `?` compris. +/// Une annulation est une sortie ordinaire, et Windows refuse de supprimer le MP4 partiel tant +/// que ffmpeg le tient ouvert. +struct OutputGuard(*mut AVFormatContext); +impl Drop for OutputGuard { + fn drop(&mut self) { + unsafe { + let mut pb = sn_fmt_get_pb(self.0); + if !pb.is_null() { + avio_closep(&mut pb); + sn_fmt_set_pb(self.0, ptr::null_mut()); + } + avformat_free_context(self.0); + } + } +} + +/// Garde RAII sur un `AVBufferRef` (le désréférence au Drop ; nul accepté). +struct BufferGuard(*mut AVBufferRef); +impl Drop for BufferGuard { + fn drop(&mut self) { + unsafe { av_buffer_unref(&mut self.0) }; + } +} + /// Décode la n-ième frame d'une source sur NOTRE device (textures échantillonnables). /// Sert le harnais de composition (S3+), hors mesure. Retourne une frame indépendante. pub fn decode_frame_n(path: &str, gpu: &Gpu, n: u32) -> Result { @@ -1299,8 +1326,27 @@ pub fn run_composited_multi( params: &ExportParams, progress: &mut dyn FnMut(u64), ) -> Result { - discard_partial_output(out, unsafe { - run_multi_inner(clips, out, gpu, comp, cfg, params, progress) + run_composited_multi_cancellable(clips, out, gpu, comp, cfg, params, progress, &ExportControl::default()) +} + +/// `run_composited_multi`, arrêtable entre deux frames par `control` (erreur `ExportCancelled`). +/// Le MP4 s'écrit à côté de `out` et n'est renommé par-dessus qu'une fois complet : un export +/// annulé ou raté ne laisse aucun partiel, et un `out` existant reste intact. +pub fn run_composited_multi_cancellable( + clips: &[ClipSource], + out: &str, + gpu: &Gpu, + comp: &Compositor, + cfg: &Cfg, + params: &ExportParams, + progress: &mut dyn FnMut(u64), + control: &ExportControl, +) -> Result { + with_staged_output(Path::new(out), control, |file, staged| { + drop(file); // ffmpeg le rouvre par son nom + unsafe { + run_multi_inner(clips, &staged.to_string_lossy(), gpu, comp, cfg, params, progress, control) + } }) } @@ -1641,6 +1687,7 @@ unsafe fn run_multi_inner( cfg: &Cfg, params: &ExportParams, progress: &mut dyn FnMut(u64), + control: &ExportControl, ) -> Result { if clips.is_empty() { bail!("aucun clip à exporter"); @@ -1675,11 +1722,14 @@ unsafe fn run_multi_inner( // écarte alors les candidats zéro-copie et le compositeur alimente l'encodeur en // mémoire système via `send_composited`. let software_frames = gpu.backend == Backend::Cpu; - let (mut enc_hwdev, mut enc_frames) = if software_frames { + let (enc_hwdev, enc_frames) = if software_frames { (ptr::null_mut(), ptr::null_mut()) } else { make_enc_frames(gpu, out_w as i32, out_h as i32)? }; + // Libérés en fin de portée sur toutes les sorties : une annulation qui les laisserait fuir + // coûterait 32 surfaces NV12 à chaque fois. `enc` garde sa propre référence sur le pool. + let _enc_pool = (BufferGuard(enc_frames), BufferGuard(enc_hwdev)); // Le débit vient de l'app, qui le calcule d'après la taille ET la cadence. Le repli // (8Mbps @ 1920x1080 quelle que soit la cadence, plancher 2Mbps) ne sert plus qu'au banc et // aux tests : c'est lui qui affamait un export 1080p60, deux fois plus d'images au même débit. @@ -1702,6 +1752,7 @@ unsafe fn run_multi_inner( avformat_alloc_output_context2(&mut octx, ptr::null(), ptr::null(), outc.as_ptr()), "alloc_output_context2", )?; + let _output = OutputGuard(octx); let ostream = avformat_new_stream(octx, ptr::null()); if ostream.is_null() { bail!("video avformat_new_stream"); @@ -1717,6 +1768,7 @@ unsafe fn run_multi_inner( averr(avformat_write_header(octx, ptr::null_mut()), "write_header")?; let opkt = av_packet_alloc(); + let _opkt = PacketGuard(opkt); let mut clip_frame_counts = vec![0u64; clips.len()]; let mut audio_jobs: ClipAudioJobs> = ClipAudioJobs::new(clips.len()); let t0 = Instant::now(); @@ -1731,6 +1783,7 @@ unsafe fn run_multi_inner( &mut screen_decs, &mut webcam_decs, &mut |frame_index| { + control.check()?; // Backend CPU (WARP) : la frame composée descend en mémoire système via // `send_composited` (le compositeur relit son NV12 interne vers un AVFrame // YUV420P / NV12 et l'encodeur le consomme directement). Pas de hw_frames_ctx, @@ -1812,19 +1865,8 @@ unsafe fn run_multi_inner( averr(av_write_trailer(octx), "write_trailer")?; let wall_s = t0.elapsed().as_secs_f64(); - // teardown (les décodeurs du cache sont droppés en fin de scope). - av_packet_free(&mut (opkt as *mut _)); - let mut pb2 = sn_fmt_get_pb(octx); - if !pb2.is_null() { - avio_closep(&mut pb2); - sn_fmt_set_pb(octx, ptr::null_mut()); - } - avformat_free_context(octx); - // `enc` (donc le contexte encodeur) est libéré par son Drop en fin de portée — après - // ces unref, ce qui est l'ordre voulu : il garde sa propre référence sur le pool. - av_buffer_unref(&mut enc_frames); - av_buffer_unref(&mut enc_hwdev); - + // teardown : les gardes et les Drop (décodeurs, encodeurs) en fin de portée — fichier fermé + // avant que `with_staged_output` ne le renomme. let fps = frames as f64 / wall_s; Ok(Stats { frames, wall_s, fps, video_duration_s: frames as f64 / out_fps as f64 }) } diff --git a/crates/compositor/tests/export_timing.rs b/crates/compositor/tests/export_timing.rs index 34d5e8de4..bc74bee68 100644 --- a/crates/compositor/tests/export_timing.rs +++ b/crates/compositor/tests/export_timing.rs @@ -34,6 +34,7 @@ use openscreen_compositor::compositor::Compositor; use openscreen_compositor::config::Cfg; use openscreen_compositor::d3d::Gpu; +use openscreen_compositor::export_control::{ExportCancelled, ExportControl}; use openscreen_compositor::gif_export::{self, GifExportParams}; use openscreen_compositor::pipeline::{self, ClipSource, ExportCodec, ExportParams}; use std::path::PathBuf; @@ -160,6 +161,62 @@ fn mp4_export_frame_count_follows_output_fps() { assert_eq!(probed, 120, "muxed file disagrees with the reported count"); } +/// Cancelling an MP4 mid-render stops the walk at the next frame, reports `ExportCancelled`, +/// and publishes nothing: the destination keeps what it held and no partial is left beside +/// it. On Windows that last check also proves the muxer closed its file, since an open +/// handle makes the partial undeletable. +#[test] +fn mp4_export_cancelled_mid_render_publishes_nothing() { + let Some(dir) = media_dir() else { + eprintln!("skipped: set OPENSCREEN_TEST_MEDIA"); + return; + }; + let out_dir = dir.join("cancelled_export"); + let _ = std::fs::remove_dir_all(&out_dir); + std::fs::create_dir(&out_dir).expect("output dir"); + let out = out_dir.join("out.mp4"); + std::fs::write(&out, b"previous export").expect("seed the destination"); + + let gpu = Gpu::create(false).expect("gpu"); + let params = ExportParams { + width: 640, + height: 360, + fps: Some(30), + codec: ExportCodec::H264, + bit_rate: None, + }; + let comp = Compositor::new_sized(&gpu, params.width, params.height).expect("compositor"); + let control = ExportControl::default(); + let mut reported = 0; + let error = pipeline::run_composited_multi_cancellable( + &[whole_clip(&dir)], + &out.to_string_lossy(), + &gpu, + &comp, + &Cfg::c8(), + ¶ms, + &mut |frames| { + reported = frames; + if frames == 10 { + control.cancel(); + } + }, + &control, + ) + .err() + .expect("a cancelled export must not succeed"); + + assert!(error.is::(), "expected a cancellation, got: {error:#}"); + assert_eq!(reported, 10, "the walk went on to frame {reported} after the cancel"); + assert_eq!(std::fs::read(&out).expect("destination"), b"previous export"); + let left: Vec<_> = std::fs::read_dir(&out_dir) + .expect("output dir") + .map(|entry| entry.expect("entry").file_name()) + .collect(); + assert_eq!(left, vec![std::ffi::OsString::from("out.mp4")], "a partial export was left behind"); + let _ = std::fs::remove_dir_all(&out_dir); +} + /// The GIF must cover the WHOLE timeline, not just its first /// `out_fps / source_fps` slice. /// diff --git a/electron/ipc/gifExportJobs.test.ts b/electron/ipc/exportJobs.test.ts similarity index 88% rename from electron/ipc/gifExportJobs.test.ts rename to electron/ipc/exportJobs.test.ts index b54550da5..1ae944c28 100644 --- a/electron/ipc/gifExportJobs.test.ts +++ b/electron/ipc/exportJobs.test.ts @@ -1,7 +1,7 @@ import { EventEmitter } from "node:events"; import type { WebContents } from "electron"; import { describe, expect, it, vi } from "vitest"; -import { GifExportJobs, isGifExportId } from "./gifExportJobs"; +import { ExportJobs, isExportId } from "./exportJobs"; function owner(id = 1) { return Object.assign(new EventEmitter(), { @@ -20,9 +20,9 @@ function pending() { return { result, resolve, reject, cancel: vi.fn(() => true) }; } -describe("GIF export jobs", () => { +describe("export jobs", () => { it("binds cancel to the sender and ID, including before native compute starts", async () => { - const jobs = new GifExportJobs(); + const jobs = new ExportJobs(); const sender = owner(); const job = pending(); const run = jobs.run(sender, "job_1", () => job, vi.fn()); @@ -38,7 +38,7 @@ describe("GIF export jobs", () => { }); it("rejects a duplicate job without starting it and allows a retry after failure", async () => { - const jobs = new GifExportJobs(); + const jobs = new ExportJobs(); const sender = owner(); const job = pending(); const run = jobs.run(sender, "first", () => job, vi.fn()); @@ -55,7 +55,7 @@ describe("GIF export jobs", () => { }); it("cancels on sender destruction and suppresses late progress after a retry", async () => { - const jobs = new GifExportJobs(); + const jobs = new ExportJobs(); const sender = owner(); const job = pending(); const progress = vi.fn(); @@ -84,7 +84,7 @@ describe("GIF export jobs", () => { }); it("does not reinterpret a completion that wins the cancellation race", async () => { - const jobs = new GifExportJobs(); + const jobs = new ExportJobs(); const sender = owner(); const job = pending(); job.cancel.mockReturnValue(false); @@ -103,13 +103,13 @@ describe("GIF export jobs", () => { "a/b", "a".repeat(129), ])("rejects malformed IDs: %j", (id) => { - expect(isGifExportId(id)).toBe(false); + expect(isExportId(id)).toBe(false); }); it("rejects an invalid start before invoking native code", async () => { const start = vi.fn(() => pending()); - await expect(new GifExportJobs().run(owner(), "../bad", start, vi.fn())).rejects.toThrow( - "Invalid GIF export ID", + await expect(new ExportJobs().run(owner(), "../bad", start, vi.fn())).rejects.toThrow( + "Invalid export ID", ); expect(start).not.toHaveBeenCalled(); }); diff --git a/electron/ipc/gifExportJobs.ts b/electron/ipc/exportJobs.ts similarity index 72% rename from electron/ipc/gifExportJobs.ts rename to electron/ipc/exportJobs.ts index fe318b35e..727fdff51 100644 --- a/electron/ipc/gifExportJobs.ts +++ b/electron/ipc/exportJobs.ts @@ -2,17 +2,17 @@ import type { WebContents } from "electron"; type ExportOwner = Pick; -export interface GifExportJob { +export interface ExportJob { result: Promise; cancel: () => boolean; } -export function isGifExportId(value: unknown): value is string { +export function isExportId(value: unknown): value is string { return typeof value === "string" && /^[A-Za-z0-9_-]{1,128}$/.test(value); } /** Controls never leave main. Unknown and foreign IDs have the same result. */ -export class GifExportJobs { +export class ExportJobs { private readonly jobs = new Map boolean }>(); cancel(owner: ExportOwner, exportId: string): boolean { @@ -23,12 +23,12 @@ export class GifExportJobs { async run( owner: ExportOwner, exportId: string, - start: (progress: (frames: number) => void) => GifExportJob, + start: (progress: (frames: number) => void) => ExportJob, onProgress: (frames: number) => void, ): Promise { - if (!isGifExportId(exportId)) throw new Error("Invalid GIF export ID."); - if (owner.isDestroyed()) throw new Error("GIF export window is closed."); - if (this.jobs.has(owner.id)) throw new Error("A GIF export is already running in this window."); + if (!isExportId(exportId)) throw new Error("Invalid export ID."); + if (owner.isDestroyed()) throw new Error("Export window is closed."); + if (this.jobs.has(owner.id)) throw new Error("An export is already running in this window."); let active = true; const job = start((frames) => { if (active && !owner.isDestroyed()) onProgress(frames); diff --git a/electron/ipc/nativeBridge.export.test.ts b/electron/ipc/nativeBridge.export.test.ts new file mode 100644 index 000000000..9fbd008eb --- /dev/null +++ b/electron/ipc/nativeBridge.export.test.ts @@ -0,0 +1,88 @@ +// An MP4 export started with an ID runs as a job of its window, so a cancel naming that ID +// reaches the native control, and the native cancellation comes back as CANCELLED. + +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { NativeBridgeResponse } from "../../src/native/contracts"; +import { type NativeBridgeContext, registerNativeBridgeHandlers } from "./nativeBridge"; + +const electron = vi.hoisted(() => ({ handle: vi.fn() })); +vi.mock("electron", () => ({ + app: { getAppPath: () => "", isPackaged: false }, + ipcMain: { handle: electron.handle, removeHandler: vi.fn() }, + shell: {}, +})); +const compositor = vi.hoisted(() => ({ startExportMulti: vi.fn(), exportMulti: vi.fn() })); +vi.mock("../native-bridge/services/compositorViewService", () => ({ + CompositorViewService: class { + startExportMulti = compositor.startExportMulti; + exportMulti = compositor.exportMulti; + }, +})); +vi.mock("../native-bridge/services/aiEditionService", () => ({ + AiEditionService: class {}, +})); + +const sender = { + id: 1, + isDestroyed: () => false, + send: vi.fn(), + once: vi.fn(), + removeListener: vi.fn(), +}; +let invoke: (action: string, payload: unknown) => Promise; + +beforeEach(() => { + electron.handle.mockClear(); + sender.send.mockClear(); + for (const fn of Object.values(compositor)) fn.mockReset(); + registerNativeBridgeHandlers({ + getPlatform: () => "linux", + getAiEditionDocuments: () => ({}), + getAiEditionLlmConfig: () => ({}), + } as unknown as NativeBridgeContext); + const handler = electron.handle.mock.calls[0]?.[1] as ( + event: unknown, + request: unknown, + ) => Promise; + invoke = (action, payload) => handler({ sender }, { domain: "compositor", action, payload }); +}); + +describe("native bridge MP4 export", () => { + it("cancels a running export by its ID and reports CANCELLED", async () => { + let fail!: (error: Error) => void; + const cancel = vi.fn(() => true); + compositor.startExportMulti.mockImplementation((...args: unknown[]) => { + (args[4] as (frames: number) => void)(3); + const result = new Promise((_, reject) => { + fail = reject; + }); + return { result, cancel }; + }); + const running = invoke("exportMulti", { clips: [], outPath: "/tmp/a.mp4", exportId: "mp4-1" }); + await vi.waitFor(() => expect(compositor.startExportMulti).toHaveBeenCalledOnce()); + expect(sender.send).toHaveBeenCalledWith("export:native-progress", 3, "mp4-1"); + + expect(await invoke("cancelExport", { exportId: "other" })).toMatchObject({ + ok: true, + data: { accepted: false }, + }); + expect(await invoke("cancelExport", { exportId: "mp4-1" })).toMatchObject({ + ok: true, + data: { accepted: true }, + }); + expect(cancel).toHaveBeenCalledOnce(); + + fail(new Error("MP4_EXPORT_CANCELLED")); + expect(await running).toMatchObject({ ok: false, error: { code: "CANCELLED" } }); + }); + + it("runs an export without an ID outside the job registry, as the CLI does", async () => { + const stats = { frames: 1, wallS: 1, fps: 1, videoDurationS: 1 }; + compositor.exportMulti.mockResolvedValue(stats); + expect(await invoke("exportMulti", { clips: [], outPath: "/tmp/a.mp4" })).toMatchObject({ + ok: true, + data: stats, + }); + expect(compositor.startExportMulti).not.toHaveBeenCalled(); + }); +}); diff --git a/electron/ipc/nativeBridge.ts b/electron/ipc/nativeBridge.ts index 61ed8ef86..954cc834f 100644 --- a/electron/ipc/nativeBridge.ts +++ b/electron/ipc/nativeBridge.ts @@ -24,7 +24,7 @@ import { CursorService } from "../native-bridge/services/cursorService"; import { ProjectService } from "../native-bridge/services/projectService"; import { SystemService } from "../native-bridge/services/systemService"; import { createNativeBridgeState } from "../native-bridge/store"; -import { GifExportJobs, isGifExportId } from "./gifExportJobs"; +import { ExportJobs, isExportId } from "./exportJobs"; export interface NativeBridgeContext { getPlatform: () => NodeJS.Platform; @@ -250,7 +250,7 @@ export function registerNativeBridgeHandlers(context: NativeBridgeContext) { deleteSession: context.deleteAiEditionChatSession, }); - const gifExportJobs = new GifExportJobs(); + const exportJobs = new ExportJobs(); ipcMain.handle(NATIVE_BRIDGE_CHANNEL, async (event, request: unknown) => { if (!isBridgeRequest(request)) { return createErrorResponse(undefined, "INVALID_REQUEST", "Invalid native bridge request."); @@ -432,17 +432,34 @@ export function registerNativeBridgeHandlers(context: NativeBridgeContext) { return createSuccessResponse(requestId, { ok: true }); case "exportMulti": { const sender = event.sender; - const stats = await compositorViewService.exportMulti( - request.payload.clips, - request.payload.outPath, - request.payload.sceneJson, - request.payload.params, - (frames) => { - if (!sender.isDestroyed()) { - sender.send("export:native-progress", frames); - } - }, - ); + const exportId = request.payload?.exportId; + if (exportId !== undefined && !isExportId(exportId)) { + return createErrorResponse(requestId, "INVALID_REQUEST", "Invalid export ID."); + } + const onProgress = (frames: number) => { + if (!sender.isDestroyed()) sender.send("export:native-progress", frames, exportId); + }; + const stats = exportId + ? await exportJobs.run( + sender, + exportId, + (progress) => + compositorViewService.startExportMulti( + request.payload.clips, + request.payload.outPath, + request.payload.sceneJson, + request.payload.params, + progress, + ), + onProgress, + ) + : await compositorViewService.exportMulti( + request.payload.clips, + request.payload.outPath, + request.payload.sceneJson, + request.payload.params, + onProgress, + ); if (!stats) { return createErrorResponse( requestId, @@ -455,14 +472,14 @@ export function registerNativeBridgeHandlers(context: NativeBridgeContext) { case "exportGif": { const sender = event.sender; const exportId = request.payload?.exportId; - if (exportId !== undefined && !isGifExportId(exportId)) { - return createErrorResponse(requestId, "INVALID_REQUEST", "Invalid GIF export ID."); + if (exportId !== undefined && !isExportId(exportId)) { + return createErrorResponse(requestId, "INVALID_REQUEST", "Invalid export ID."); } const onProgress = (frames: number) => { if (!sender.isDestroyed()) sender.send("export:native-progress", frames, exportId); }; const stats = exportId - ? await gifExportJobs.run( + ? await exportJobs.run( sender, exportId, (progress) => @@ -491,13 +508,13 @@ export function registerNativeBridgeHandlers(context: NativeBridgeContext) { } return createSuccessResponse(requestId, stats); } - case "cancelGifExport": { + case "cancelExport": { const exportId = request.payload?.exportId; - if (!isGifExportId(exportId)) { - return createErrorResponse(requestId, "INVALID_REQUEST", "Invalid GIF export ID."); + if (!isExportId(exportId)) { + return createErrorResponse(requestId, "INVALID_REQUEST", "Invalid export ID."); } return createSuccessResponse(requestId, { - accepted: gifExportJobs.cancel(event.sender, exportId), + accepted: exportJobs.cancel(event.sender, exportId), }); } default: @@ -797,11 +814,11 @@ export function registerNativeBridgeHandlers(context: NativeBridgeContext) { } catch (error) { if ( request.domain === "compositor" && - request.action === "exportGif" && error instanceof Error && - error.message === "GIF_EXPORT_CANCELLED" + ((request.action === "exportGif" && error.message === "GIF_EXPORT_CANCELLED") || + (request.action === "exportMulti" && error.message === "MP4_EXPORT_CANCELLED")) ) { - return createErrorResponse(requestId, "CANCELLED", "GIF export cancelled."); + return createErrorResponse(requestId, "CANCELLED", "Export cancelled."); } // Not retryable by default: most failures here are permanent (a missing // file, a bad payload, an unavailable addon), and a blanket `true` tells diff --git a/electron/native-bridge/services/compositorViewService.test.ts b/electron/native-bridge/services/compositorViewService.test.ts index 4a099a7f9..9de1ac91b 100644 --- a/electron/native-bridge/services/compositorViewService.test.ts +++ b/electron/native-bridge/services/compositorViewService.test.ts @@ -59,6 +59,47 @@ describe("native GIF cancellation capability", () => { }); }); +describe("native MP4 cancellation capability", () => { + const stats = { frames: 1, wallS: 1, fps: 1, videoDurationS: 1 }; + + it("passes one opaque control to the native export and cancels through it", async () => { + const control = {}; + const exportMulti = vi.fn(async () => stats); + const cancelMp4Export = vi.fn(() => true); + const service = new CompositorViewService({ + addon: { + createMp4ExportControl: () => control, + cancelMp4Export, + exportMulti, + } as unknown as CompositorViewAddon, + }); + const progress = vi.fn(); + const job = service.startExportMulti([], "/tmp/test.mp4", undefined, { fps: 30 }, progress); + expect(exportMulti).toHaveBeenCalledWith( + [], + "/tmp/test.mp4", + undefined, + { fps: 30 }, + progress, + control, + ); + expect(job.cancel()).toBe(true); + expect(cancelMp4Export).toHaveBeenCalledWith(control); + await expect(job.result).resolves.toEqual(stats); + }); + + it("still exports through an addon that cannot cancel an MP4, and refuses the cancel", async () => { + const exportMulti = vi.fn(async () => stats); + const service = new CompositorViewService({ + addon: { exportMulti } as unknown as CompositorViewAddon, + }); + const job = service.startExportMulti([], "/tmp/test.mp4"); + expect(job.cancel()).toBe(false); + await expect(job.result).resolves.toEqual(stats); + expect(exportMulti).toHaveBeenCalledOnce(); + }); +}); + describe("CompositorViewService frames handed over as shared GPU textures", () => { const target = { frameTreeNodeId: 1 } as unknown as WebFrameMain; const sharedFrame = { diff --git a/electron/native-bridge/services/compositorViewService.ts b/electron/native-bridge/services/compositorViewService.ts index 6dbfb1461..6d16a2a09 100644 --- a/electron/native-bridge/services/compositorViewService.ts +++ b/electron/native-bridge/services/compositorViewService.ts @@ -12,7 +12,7 @@ import type { CompositorSharedFrameMeta, CompositorSharedFrameReceipt, } from "../../../src/native/contracts"; -import type { GifExportJob } from "../../ipc/gifExportJobs"; +import type { ExportJob } from "../../ipc/exportJobs"; import type { ClipInput, CompositorBackend, @@ -805,6 +805,7 @@ export class CompositorViewService { sceneJson?: string, params?: ExportParamsInput, onProgress?: (frames: number) => void, + control?: object, ): Promise { const addon = this.ensureAddon(); if (!addon) { @@ -817,9 +818,35 @@ export class CompositorViewService { sceneJson ? resolveSceneAssetPaths(sceneJson) : undefined, params, onProgress, + control, ); } + /** `startGifExport` for MP4, except that an addon built before MP4 cancellation still + * exports: the main export must not fail over a missing Cancel. That job runs to the end + * and refuses to cancel. */ + startExportMulti( + clips: ClipInput[], + outPath?: string, + sceneJson?: string, + params?: ExportParamsInput, + onProgress?: (frames: number) => void, + ): ExportJob { + const addon = this.ensureAddon(); + if (!addon?.createMp4ExportControl || !addon.cancelMp4Export) { + return { + result: this.exportMulti(clips, outPath, sceneJson, params, onProgress), + cancel: () => false, + }; + } + const control = addon.createMp4ExportControl(); + const cancel = addon.cancelMp4Export.bind(addon); + return { + result: this.exportMulti(clips, outPath, sceneJson, params, onProgress, control), + cancel: () => cancel(control), + }; + } + /** Native GIF export. Same inputs as `exportMulti` — one clip list, one scene — * because it is the same render: both drive `walk_composited_timeline` in the * compositor crate and differ only in the encoder. The scene carries background, @@ -856,7 +883,7 @@ export class CompositorViewService { sceneJson?: string, params?: GifParamsInput, onProgress?: (frames: number) => void, - ): GifExportJob { + ): ExportJob { const addon = this.ensureAddon(); if (!addon?.createGifExportControl || !addon.cancelGifExport) { throw new Error( diff --git a/electron/native/compositor-view/addon.d.ts b/electron/native/compositor-view/addon.d.ts index e87b830b5..ade2769a7 100644 --- a/electron/native/compositor-view/addon.d.ts +++ b/electron/native/compositor-view/addon.d.ts @@ -216,14 +216,21 @@ export interface CompositorViewAddon { * `sceneJson` — same `SceneDescription` as the live preview (background/layout/webcam/cursor/ * effects); omitted or invalid → nothing configured is applied (not a masking fallback). * `params` — output size/fps/codec; omitted → 1920x1080/first clip's fps/h264. - * `onProgress` (frames encoded so far) is optional, throttled to ~10/s. */ + * `onProgress` (frames encoded so far) is optional, throttled to ~10/s. + * `control` (`createMp4ExportControl`) — cancelling it stops the render between two frames + * and rejects with `MP4_EXPORT_CANCELLED`, leaving no file. */ exportMulti( clips: ClipInput[], outPath: string, sceneJson?: string, params?: ExportParamsInput, onProgress?: (frames: number) => void, + control?: object, ): Promise; + /** Opaque napi External, like the GIF one. Absent from a `.node` whose `exportMulti` ignores + * a control: that addon's MP4 export cannot be cancelled. */ + createMp4ExportControl?(): object; + cancelMp4Export?(control: object): boolean; /** Native GIF export. Identical inputs to `exportMulti` — same clips, same * scene — because it is the same render: both drive `walk_composited_timeline` * in the compositor crate and differ only in the encoder. Cursor, background, diff --git a/src/components/ai-edition/ExportDialog.cancel.test.tsx b/src/components/ai-edition/ExportDialog.cancel.test.tsx index 8d2403b9c..308a76eee 100644 --- a/src/components/ai-edition/ExportDialog.cancel.test.tsx +++ b/src/components/ai-edition/ExportDialog.cancel.test.tsx @@ -7,7 +7,7 @@ vi.mock("sonner", () => ({ toast: { success: vi.fn(), error: vi.fn() } })); vi.mock("@/native", () => ({ exportMultiNative: vi.fn(), exportGifNative: vi.fn(), - cancelGifExportNative: vi.fn(async () => ({ accepted: true })), + cancelExportNative: vi.fn(async () => ({ accepted: true })), useIsCpuCompositor: () => false, })); vi.mock("@/native/sceneDescription", () => ({ @@ -18,9 +18,9 @@ vi.mock("@/native/sceneDescription", () => ({ import { toast } from "sonner"; import { I18nProvider } from "@/contexts/I18nContext"; import { type AxcutDocument, axcutSchemaVersion } from "@/lib/ai-edition/schema"; -import { cancelGifExportNative, exportGifNative } from "@/native"; +import { cancelExportNative, exportGifNative, exportMultiNative } from "@/native"; import { NativeBridgeRequestError } from "@/native/client"; -import type { CompositorExportGifResult } from "@/native/contracts"; +import type { CompositorExportGifResult, CompositorExportResult } from "@/native/contracts"; import { ExportDialog } from "./ExportDialog"; const DOC: AxcutDocument = { @@ -106,10 +106,34 @@ async function start() { return { ...view, id }; } -describe("GIF export cancellation", () => { +function pendingMp4Export() { + let resolve!: (stats: CompositorExportResult) => void; + let reject!: (error: Error) => void; + const result = new Promise((yes, no) => { + resolve = yes; + reject = no; + }); + vi.mocked(exportMultiNative).mockReturnValueOnce(result); + return { resolve, reject }; +} + +async function startMp4() { + render( + + + , + ); + fireEvent.click(screen.getByRole("button", { name: "Export MP4" })); + await waitFor(() => expect(exportMultiNative).toHaveBeenCalledOnce()); + const id = vi.mocked(exportMultiNative).mock.calls[0][4]; + expect(id).toMatch(/^[A-Za-z0-9_-]+$/); + return id as string; +} + +describe("export cancellation", () => { beforeEach(() => { vi.clearAllMocks(); - vi.mocked(cancelGifExportNative).mockResolvedValue({ accepted: true }); + vi.mocked(cancelExportNative).mockResolvedValue({ accepted: true }); onClose = vi.fn(); unsubscribe = vi.fn(); window.electronAPI = { @@ -128,10 +152,10 @@ describe("GIF export cancellation", () => { const cancel = screen.getByRole("button", { name: "Cancel" }); expect(cancel).toBeEnabled(); fireEvent.click(cancel); - await waitFor(() => expect(cancelGifExportNative).toHaveBeenCalledWith(id)); + await waitFor(() => expect(cancelExportNative).toHaveBeenCalledWith(id)); expect(cancel).toBeDisabled(); fireEvent.click(cancel); - expect(cancelGifExportNative).toHaveBeenCalledOnce(); + expect(cancelExportNative).toHaveBeenCalledOnce(); expect(screen.queryByRole("button", { name: "Export GIF" })).not.toBeInTheDocument(); await act(async () => job.reject( @@ -186,7 +210,7 @@ describe("GIF export cancellation", () => { }); it("surfaces a rejected cancel request instead of keeping the progress view", async () => { - vi.mocked(cancelGifExportNative).mockRejectedValueOnce(new Error("cancel ipc failed")); + vi.mocked(cancelExportNative).mockRejectedValueOnce(new Error("cancel ipc failed")); pendingExport(); await start(); fireEvent.click(screen.getByRole("button", { name: "Cancel" })); @@ -196,8 +220,48 @@ describe("GIF export cancellation", () => { expect(screen.queryByText(/Exporting your video/i)).not.toBeInTheDocument(); }); + it("cancels an MP4 mid-render and returns to the same options", async () => { + const job = pendingMp4Export(); + const id = await startMp4(); + const cancel = screen.getByRole("button", { name: "Cancel" }); + expect(cancel).toBeEnabled(); + fireEvent.click(cancel); + await waitFor(() => expect(cancelExportNative).toHaveBeenCalledWith(id)); + expect(cancel).toBeDisabled(); + await act(async () => + job.reject( + new NativeBridgeRequestError({ + code: "CANCELLED", + message: "cancelled", + retryable: false, + }), + ), + ); + expect(screen.getByRole("button", { name: "Export MP4" })).toBeEnabled(); + expect(onClose).not.toHaveBeenCalled(); + expect(toast.success).not.toHaveBeenCalled(); + expect(toast.error).not.toHaveBeenCalled(); + }); + + it("goes back to the progress when native refuses the cancel", async () => { + // An addon built before MP4 cancellation refuses every cancel; the export runs on. + vi.mocked(cancelExportNative).mockResolvedValue({ accepted: false }); + const job = pendingMp4Export(); + const id = await startMp4(); + const cancel = screen.getByRole("button", { name: "Cancel" }); + fireEvent.click(cancel); + await waitFor(() => expect(cancelExportNative).toHaveBeenCalledWith(id)); + await waitFor(() => expect(cancel).toBeEnabled()); + // 10 s at the default 60 fps: 60 frames is 10%. + act(() => progress(60, id)); + expect(screen.getByText("10%")).toBeVisible(); + await act(async () => job.resolve(STATS)); + expect(screen.getByText("/tmp/result.gif")).toBeVisible(); + expect(toast.success).toHaveBeenCalledOnce(); + }); + it("reports success when native publication wins the race", async () => { - vi.mocked(cancelGifExportNative).mockResolvedValue({ accepted: false }); + vi.mocked(cancelExportNative).mockResolvedValue({ accepted: false }); const job = pendingExport(); await start(); fireEvent.click(screen.getByRole("button", { name: "Cancel" })); @@ -221,7 +285,7 @@ describe("GIF export cancellation", () => { const job = pendingExport(); const { id, unmount } = await start(); unmount(); - expect(cancelGifExportNative).toHaveBeenCalledWith(id); + expect(cancelExportNative).toHaveBeenCalledWith(id); await act(async () => job.resolve(STATS)); expect(toast.success).not.toHaveBeenCalled(); expect(toast.error).not.toHaveBeenCalled(); diff --git a/src/components/ai-edition/ExportDialog.tsx b/src/components/ai-edition/ExportDialog.tsx index 47b08b08a..195cb22bf 100644 --- a/src/components/ai-edition/ExportDialog.tsx +++ b/src/components/ai-edition/ExportDialog.tsx @@ -1,7 +1,7 @@ // Export dialog for the new editor. Wires together: // 1. pickExportSavePath (native save dialog) // 2. the native D3D exporter (exportMultiNative / exportGifNative) -// 3. per-job GIF cancellation, with native cleanup before returning to options +// 3. per-job cancellation (MP4 and GIF), with native cleanup before returning to options // // Format/quality/GIF options live in the dialog's local state. The // dialog uses the new shell's modal style. @@ -33,7 +33,7 @@ import { import { calculateMp4ExportSettings, wouldUpscale } from "@/lib/exporter/mp4ExportSettings"; import { outputFrameCount } from "@/lib/exporter/outputFrameCount"; import { - cancelGifExportNative, + cancelExportNative, exportGifNative, exportMultiNative, useIsCpuCompositor, @@ -48,7 +48,7 @@ import { Toggle } from "./RightPanes"; type Phase = "idle" | "configuring" | "rendering" | "writing" | "done" | "error"; interface ActiveExport { - id?: string; + id: string; cancelRequested: boolean; unsubscribe?: () => void; } @@ -56,8 +56,8 @@ interface ActiveExport { function disposeExport(exportJob: ActiveExport | null) { exportJob?.unsubscribe?.(); if (exportJob?.id) { - void cancelGifExportNative(exportJob.id).catch((error) => { - console.warn("[export] failed to cancel detached GIF export", error); + void cancelExportNative(exportJob.id).catch((error) => { + console.warn("[export] failed to cancel detached export", error); }); } } @@ -307,8 +307,14 @@ export function ExportDialog({ open, onClose, document }: ExportDialogProps) { job.cancelRequested = true; setCancelPending(true); try { - await cancelGifExportNative(job.id); - // Native settlement decides the winner and confirms file cleanup. + const { accepted } = await cancelExportNative(job.id); + // Native settlement decides the winner and confirms file cleanup. A refusal means the + // export finishes on its own (already publishing, or an addon too old to cancel an + // MP4), so the dialog goes back to showing its progress. + if (!accepted && activeExport.current === job) { + job.cancelRequested = false; + setCancelPending(false); + } } catch (err) { if (activeExport.current !== job) return; job.cancelRequested = false; @@ -367,10 +373,7 @@ export function ExportDialog({ open, onClose, document }: ExportDialogProps) { // as the live preview, so an export can no longer disagree with what the // user previewed. { - const job: ActiveExport = { - id: format === "gif" ? crypto.randomUUID() : undefined, - cancelRequested: false, - }; + const job: ActiveExport = { id: crypto.randomUUID(), cancelRequested: false }; activeExport.current = job; setPhase("rendering"); // Render the real timeline when there are clips; else fall back to the fixture. @@ -428,16 +431,22 @@ export function ExportDialog({ open, onClose, document }: ExportDialogProps) { }, job.id, ) - : await exportMultiNative(exportClips, pickedPath, sceneJson, { - width: outDims?.width, - height: outDims?.height, - fps, - // H.264 only. The native pipeline still encodes H.265, but nothing - // offers it: it is software-only on Linux, slower than software on the - // measured Macs, and the files half the players cannot open. - codec: "h264", - bitrate: outDims?.bitrate, - }); + : await exportMultiNative( + exportClips, + pickedPath, + sceneJson, + { + width: outDims?.width, + height: outDims?.height, + fps, + // H.264 only. The native pipeline still encodes H.265, but nothing + // offers it: it is software-only on Linux, slower than software on the + // measured Macs, and the files half the players cannot open. + codec: "h264", + bitrate: outDims?.bitrate, + }, + job.id, + ); if (activeExport.current !== job) return; setSavedPath(pickedPath); setPhase("done"); @@ -698,7 +707,7 @@ export function ExportDialog({ open, onClose, document }: ExportDialogProps) { type="button" className={`${styles.btn} ${styles.btnSecondary}`} onClick={handleCancel} - disabled={isBusy && !(format === "gif" && phase === "rendering" && !cancelPending)} + disabled={isBusy && !(phase === "rendering" && !cancelPending)} aria-busy={cancelPending} > {cancelPending && } diff --git a/src/native/compositorViewClient.test.ts b/src/native/compositorViewClient.test.ts index 15f3617f6..76cf5234b 100644 --- a/src/native/compositorViewClient.test.ts +++ b/src/native/compositorViewClient.test.ts @@ -1,11 +1,11 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { NativeBridgeRequestError } from "./client"; -import { cancelGifExportNative, exportGifNative } from "./compositorViewClient"; +import { cancelExportNative, exportGifNative, exportMultiNative } from "./compositorViewClient"; import type { NativeBridgeRequest } from "./contracts"; afterEach(() => vi.unstubAllGlobals()); -describe("GIF export bridge client", () => { +describe("export bridge client", () => { it("keeps the export ID independent from RPC IDs", async () => { const invoke = vi.fn(async (_request: NativeBridgeRequest) => ({ ok: true, @@ -13,7 +13,7 @@ describe("GIF export bridge client", () => { })); vi.stubGlobal("window", { electronAPI: { invokeNativeBridge: invoke } }); await exportGifNative([], "/tmp/test.gif", undefined, { fps: 15 }, "gif-1"); - await cancelGifExportNative("gif-1"); + await cancelExportNative("gif-1"); expect(invoke.mock.calls).toHaveLength(2); const first = invoke.mock.calls[0]?.[0]; expect(first?.requestId).not.toBe(invoke.mock.calls[1]?.[0].requestId); @@ -24,7 +24,19 @@ describe("GIF export bridge client", () => { }), ); expect(invoke.mock.calls[1]?.[0]).toEqual( - expect.objectContaining({ action: "cancelGifExport", payload: { exportId: "gif-1" } }), + expect.objectContaining({ action: "cancelExport", payload: { exportId: "gif-1" } }), + ); + }); + + it("sends the MP4 export ID that a cancel names", async () => { + const invoke = vi.fn(async (_request: NativeBridgeRequest) => ({ ok: true, data: {} })); + vi.stubGlobal("window", { electronAPI: { invokeNativeBridge: invoke } }); + await exportMultiNative([], "/tmp/test.mp4", undefined, { fps: 30 }, "mp4-1"); + expect(invoke.mock.calls[0]?.[0]).toEqual( + expect.objectContaining({ + action: "exportMulti", + payload: expect.objectContaining({ exportId: "mp4-1", params: { fps: 30 } }), + }), ); }); @@ -33,14 +45,17 @@ describe("GIF export bridge client", () => { electronAPI: { invokeNativeBridge: vi.fn(async () => ({ ok: false, - error: { code: "CANCELLED", message: "GIF export cancelled.", retryable: false }, + error: { code: "CANCELLED", message: "Export cancelled.", retryable: false }, })), }, }); - const error = await exportGifNative([], "/tmp/test.gif", undefined, undefined, "gif-2").catch( - (e: unknown) => e, - ); - expect(error).toBeInstanceOf(NativeBridgeRequestError); - expect(error).toMatchObject({ code: "CANCELLED", message: "GIF export cancelled." }); + for (const run of [ + () => exportGifNative([], "/tmp/test.gif", undefined, undefined, "gif-2"), + () => exportMultiNative([], "/tmp/test.mp4", undefined, undefined, "mp4-2"), + ]) { + const error = await run().catch((e: unknown) => e); + expect(error).toBeInstanceOf(NativeBridgeRequestError); + expect(error).toMatchObject({ code: "CANCELLED", message: "Export cancelled." }); + } }); }); diff --git a/src/native/compositorViewClient.ts b/src/native/compositorViewClient.ts index 66f63ce56..131d8909c 100644 --- a/src/native/compositorViewClient.ts +++ b/src/native/compositorViewClient.ts @@ -196,11 +196,12 @@ export function exportMultiNative( outPath?: string, sceneJson?: string, params?: CompositorExportParams, + exportId?: string, ): Promise { return requireNativeBridgeData({ domain: "compositor", action: "exportMulti", - payload: { clips, outPath, sceneJson, params }, + payload: { clips, outPath, sceneJson, params, exportId }, }); } @@ -220,10 +221,12 @@ export function exportGifNative( }); } -export function cancelGifExportNative(exportId: string): Promise<{ accepted: boolean }> { +/** Cancels the MP4 or GIF export started with `exportId`. `accepted: false` means it finishes on + * its own: it was already being published, or the addon cannot cancel it. */ +export function cancelExportNative(exportId: string): Promise<{ accepted: boolean }> { return requireNativeBridgeData<{ accepted: boolean }>({ domain: "compositor", - action: "cancelGifExport", + action: "cancelExport", payload: { exportId }, }); } diff --git a/src/native/contracts.ts b/src/native/contracts.ts index 5421b994a..cc76bb59b 100644 --- a/src/native/contracts.ts +++ b/src/native/contracts.ts @@ -875,6 +875,7 @@ export type NativeBridgeRequest = domain: "compositor"; action: "exportMulti"; payload: { + exportId?: string; clips: CompositorClipInput[]; outPath?: string; sceneJson?: string; @@ -927,7 +928,8 @@ export type NativeBridgeRequest = } | { domain: "compositor"; - action: "cancelGifExport"; + /** Either format: the export ID names the job, MP4 or GIF. */ + action: "cancelExport"; payload: { exportId: string }; requestId?: string; } diff --git a/technical-documentation/architecture/export-pipeline.md b/technical-documentation/architecture/export-pipeline.md index c94a9c1c7..ad8bc0bb2 100644 --- a/technical-documentation/architecture/export-pipeline.md +++ b/technical-documentation/architecture/export-pipeline.md @@ -110,6 +110,19 @@ and **one** encoder + muxer pair: table, asserted by `outputFrameCount.test.ts` and by `speed_segments_match_the_exporter_frame_totals`. +- **Cancel stops between frames, and only a finished file is published.** + The dialog's Cancel reaches the same `ExportControl` the GIF export uses + ([`export_control.rs`](../../crates/compositor/src/export_control.rs)), + which `run_composited_multi_cancellable` checks before every frame. The + MP4 is muxed into a staged file beside the destination, named with the + destination's extension because ffmpeg picks the container from it, and + renamed over the destination only once the trailer is written. A + cancelled or failed run deletes the staged file and leaves an existing + destination untouched. Guards close the muxer on every exit, which + Windows requires: it refuses to delete a file ffmpeg still holds open. + Audio jobs already in flight are joined, so a cancel settles once they + finish. + - **Imported audio tracks** (voiceover / BGM / SFX, issue #350) are mixed on top of the assembled programme by `audio.rs::mix_external_tracks`, between `assemble_concatenated_pcm` and `finish_audio`. Each track's diff --git a/technical-documentation/testing/manual-e2e-checklist.md b/technical-documentation/testing/manual-e2e-checklist.md index 60095b9e2..de6dfdeaf 100644 --- a/technical-documentation/testing/manual-e2e-checklist.md +++ b/technical-documentation/testing/manual-e2e-checklist.md @@ -551,6 +551,7 @@ The dialog is one settings panel: *Format* (MP4 / GIF), *Quality* (720p, 1080p o - [ ] Select GIF and confirm GIF frame-rate (15, 20, 25, 30 FPS), size (Small, Medium, Large, Original), and *Loop GIF* controls appear. - [ ] Change GIF frame rate and size, toggle looping, and confirm the summary reflects the choices. - [ ] Start an MP4 export with *Export MP4* and confirm the native rendering progress reports advancing frames or percentage. +- [ ] During MP4 rendering, press Cancel and confirm it waits for native cleanup, returns to the same export options, leaves no partial MP4 (no `.openscreen-mp4-*.partial.mp4` beside the destination), and preserves an existing destination; retry and confirm a complete MP4 is saved. - [ ] Confirm the export dialog reports *Saved to* with the output path after MP4 completes, and that *Show in folder* opens it. - [ ] **v2.0.0** — Confirm the one-time star prompt under *Saved to*, when it appears, goes away on either answer and does not come back on the next export. - [ ] **v2.0.0** — Export a take with speech at the Audio facet's default output level and measure it (`ffmpeg -i -af ebur128=peak=sample -f null -`): integrated loudness about -16 LUFS (a very quiet voice gets at most +12 dB), true peak at most -1 dBTP (`ebur128=peak=true`). The limiter caps samples at -1.5 dBFS before the AAC encoder, which overshoots a little, so a file reading -1.4 dBFS is in spec.