fix(export): let an MP4 export be cancelled mid-render

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<pipeline::ClipSource>,
     scene_json: Option<String>,
     params: Option<ExportParamsInput>,
+    control: ExportControl,
     on_progress: Option<ThreadsafeFunction<u32, ErrorStrategy::Fatal>>,
 }
 
@@ -605,12 +606,14 @@ impl Task for ExportMultiTask {
     type JsValue = ExportStats;
 
     fn compute(&mut self) -> Result<Self::Output> {
+        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,16 +673,17 @@ 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,
             &comp,
             &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,18 +692,42 @@ 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<ExportControl> {
+    External::new(ExportControl::default())
+}
+
+#[napi]
+pub fn cancel_mp4_export(control: External<ExportControl>) -> bool {
+    control.cancel()
+}
+
+fn mp4_task_error(error: anyhow::Error) -> Error {
+    if error.is::<ExportCancelled>() {
+        Error::from_reason("MP4_EXPORT_CANCELLED")
+    } else {
+        Error::from_reason(format!("{error:#}"))
+    }
+}
+
 /// Lance un export multiclip natif (vraie timeline → MP4) et résout `Promise<ExportStats>`.
 /// `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<ClipInput>,
     out_path: String,
     scene_json: Option<String>,
     params: Option<ExportParamsInput>,
     on_progress: Option<JsFunction>,
+    control: Option<External<ExportControl>>,
 ) -> Result<AsyncTask<ExportMultiTask>> {
     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<GifExportControl> {
-    External::new(GifExportControl::default())
+pub fn create_gif_export_control() -> External<ExportControl> {
+    External::new(ExportControl::default())
 }
 
 #[napi]
-pub fn cancel_gif_export(control: External<GifExportControl>) -> bool {
+pub fn cancel_gif_export(control: External<ExportControl>) -> bool {
     control.cancel()
 }
 
 fn gif_task_error(error: anyhow::Error) -> Error {
-    if error.is::<GifExportCancelled>() {
+    if error.is::<ExportCancelled>() {
         Error::from_reason("GIF_EXPORT_CANCELLED")
     } else {
         Error::from_reason(format!("{error:#}"))
@@ -779,7 +808,7 @@ pub struct ExportGifTask {
     scene_json: Option<String>,
     out_path: PathBuf,
     params: GifExportParams,
-    control: GifExportControl,
+    control: ExportControl,
     on_progress: Option<ThreadsafeFunction<u32, ErrorStrategy::Fatal>>,
 }
 
@@ -877,7 +906,7 @@ pub fn export_gif(
     scene_json: Option<String>,
     params: Option<GifParamsInput>,
     on_progress: Option<JsFunction>,
-    control: Option<External<GifExportControl>>,
+    control: Option<External<ExportControl>>,
 ) -> Result<AsyncTask<ExportGifTask>> {
     // Deliberately the same argument shape as `export_multi`: the caller builds
     // one clip list and one scene, and picks the container. Cursor comes from

crates/compositor/src/gif_export_control.rs → 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<AtomicU8>);
+pub struct ExportControl(Arc<AtomicU8>);
 
 #[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,28 +36,28 @@ impl GifExportControl {
 
     pub fn check(&self) -> Result<()> {
         if self.0.load(Ordering::Acquire) == CANCELLED {
-            return Err(GifExportCancelled.into());
+            return Err(ExportCancelled.into());
         }
         Ok(())
     }
 
     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<T>(
+/// `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<T>(
     target: &Path,
-    control: &GifExportControl,
+    control: &ExportControl,
     render: impl FnOnce(File, &Path) -> Result<T>,
 ) -> Result<T> {
     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<T>(
             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::<GifExportCancelled>());
+        assert!(result.unwrap_err().is::<ExportCancelled>());
         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::<GifExportCancelled>());
+        assert!(result.unwrap_err().is::<ExportCancelled>());
         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();

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<GifStats> {
-    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<GifStats> {
-    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<GifStats> {
 	control.check()?;
 	if clips.is_empty() {