fix(export): preserve MOV destinations on failed exports

crates/export/src/mov.rs

@@ -5,9 +5,9 @@ use cap_rendering::{ProjectUniforms, RenderSegment, RenderedFrame};
 use futures::FutureExt;
 use serde::{Deserialize, Serialize};
 use specta::Type;
-use std::{path::PathBuf, time::Duration};
+use std::{path::PathBuf, sync::Arc, time::Duration};
 
-use crate::{ExportError, ExporterBase};
+use crate::{ExportError, ExporterBase, mp4::temporary_export_output};
 
 #[derive(Serialize, Deserialize, Clone, Copy, Debug, Type)]
 pub struct MovExportSettings {
@@ -79,13 +79,17 @@ impl MovExportSettings {
         if let Some(parent) = mov_output_path.parent() {
             std::fs::create_dir_all(parent).map_err(|e| e.to_string())?;
         }
+        let temporary_output = Arc::new(temporary_export_output(&mov_output_path)?);
+        let encoder_output_path = temporary_output.to_path_buf();
+        let encoder_temporary_output = temporary_output.clone();
 
         let video_info =
             VideoInfo::from_raw(RawVideoFormat::Rgba, output_size.0, output_size.1, fps);
 
         let sample_timing = base.sample_timing.clone();
         let encoder_thread = tokio::task::spawn_blocking(move || {
-            let mut mov_encoder = MOVFile::init(mov_output_path.clone(), |output| {
+            let _temporary_output_guard = encoder_temporary_output;
+            let mut mov_encoder = MOVFile::init(encoder_output_path, |output| {
                 ProResEncoder::builder(video_info).build(output)
             })
             .map_err(|e| ExportError::Other(format!("Failed to create MOV encoder: {e}")))?;
@@ -131,7 +135,7 @@ impl MovExportSettings {
                 .finish()
                 .map_err(|e| ExportError::Other(format!("Failed to finish MOV: {e}")))?;
 
-            Ok(mov_output_path)
+            Ok(())
         })
         .then(|f| async {
             f.map_err(|e| e.to_string())
@@ -160,10 +164,15 @@ impl MovExportSettings {
         )
         .then(|f| async { f.map_err(|v| v.to_string()) });
 
-        let (output_path, _) =
-            tokio::try_join!(encoder_thread, render_video_task).map_err(|e| e.to_string())?;
+        let (encoder_result, render_result) = tokio::join!(encoder_thread, render_video_task);
+        encoder_result?;
+        render_result?;
+        Arc::try_unwrap(temporary_output)
+            .map_err(|_| "Export output is still in use".to_string())?
+            .persist_replace(&mov_output_path)
+            .map_err(|error| error.to_string())?;
 
-        Ok(output_path)
+        Ok(mov_output_path)
     }
 }
 

crates/export/src/mp4.rs

@@ -1260,32 +1260,56 @@ impl TemporaryExportOutput {
         self.path.to_path_buf()
     }
 
+    #[cfg(unix)]
+    fn restore_permissions(&self, output: Option<&std::path::Path>) -> std::io::Result<()> {
+        use std::os::unix::fs::MetadataExt;
+        let path_metadata = std::fs::symlink_metadata(&self.path)?;
+        let file_metadata = self.file.metadata()?;
+        if !path_metadata.is_file()
+            || path_metadata.dev() != file_metadata.dev()
+            || path_metadata.ino() != file_metadata.ino()
+        {
+            return Err(std::io::Error::other("Export temporary file was replaced"));
+        }
+        let existing_permissions = match output {
+            Some(path) => match std::fs::symlink_metadata(path) {
+                Ok(metadata) if metadata.is_file() => Some(metadata.permissions()),
+                Err(error) if error.kind() != std::io::ErrorKind::NotFound => return Err(error),
+                _ => None,
+            },
+            None => None,
+        };
+        self.file
+            .set_permissions(existing_permissions.unwrap_or_else(|| self.final_permissions.clone()))
+    }
+
     pub(crate) fn persist_noclobber(
         self,
         output: &std::path::Path,
     ) -> Result<(), tempfile::PathPersistError> {
         #[cfg(unix)]
-        let prepare_permissions = || {
-            use std::os::unix::fs::MetadataExt;
-            let path_metadata = std::fs::symlink_metadata(&self.path)?;
-            let file_metadata = self.file.metadata()?;
-            if !path_metadata.is_file()
-                || path_metadata.dev() != file_metadata.dev()
-                || path_metadata.ino() != file_metadata.ino()
-            {
-                return Err(std::io::Error::other("Export temporary file was replaced"));
-            }
-            self.file.set_permissions(self.final_permissions.clone())
-        };
-        #[cfg(unix)]
-        if let Err(error) = prepare_permissions() {
+        if let Err(error) = self.restore_permissions(None) {
             return Err(tempfile::PathPersistError {
                 error,
                 path: self.path,
             });
         }
         self.path.persist_noclobber(output)
     }
+
+    pub(crate) fn persist_replace(
+        self,
+        output: &std::path::Path,
+    ) -> Result<(), tempfile::PathPersistError> {
+        #[cfg(unix)]
+        if let Err(error) = self.restore_permissions(Some(output)) {
+            return Err(tempfile::PathPersistError {
+                error,
+                path: self.path,
+            });
+        }
+        self.path.persist(output)
+    }
 }
 
 pub(crate) fn temporary_export_output(

crates/export/tests/mov_cancellation.rs

@@ -0,0 +1,205 @@
+use cap_export::{ExporterBase, mov::MovExportSettings};
+use cap_project::{ProjectConfiguration, XY};
+use std::{fs, path::Path, process::Command};
+
+fn create_project(path: &Path) {
+    fs::create_dir_all(path).unwrap();
+    let video = Command::new("ffmpeg")
+        .args([
+            "-v",
+            "error",
+            "-f",
+            "lavfi",
+            "-i",
+            "testsrc2=size=64x64:rate=10:duration=1",
+            "-c:v",
+            "libx264",
+            "-preset",
+            "ultrafast",
+            "-threads",
+            "1",
+        ])
+        .arg(path.join("display.mp4"))
+        .output()
+        .unwrap();
+    assert!(
+        video.status.success(),
+        "{}",
+        String::from_utf8_lossy(&video.stderr)
+    );
+    fs::write(
+        path.join("recording-meta.json"),
+        serde_json::to_vec(&serde_json::json!({
+            "pretty_name": "MOV cancellation",
+            "display": { "path": "display.mp4", "fps": 10 }
+        }))
+        .unwrap(),
+    )
+    .unwrap();
+}
+
+fn assert_playable_mov(path: &Path) {
+    let decoded = Command::new("ffmpeg")
+        .args(["-v", "error", "-i"])
+        .arg(path)
+        .args(["-f", "null", "-"])
+        .output()
+        .unwrap();
+    assert!(
+        decoded.status.success(),
+        "{}",
+        String::from_utf8_lossy(&decoded.stderr)
+    );
+}
+
+async fn export(
+    project: &Path,
+    output: &Path,
+    on_progress: impl FnMut(u32) -> bool + Send + 'static,
+) -> Result<std::path::PathBuf, String> {
+    let base = Box::pin(
+        ExporterBase::builder(project.to_path_buf())
+            .with_config(ProjectConfiguration::default())
+            .with_output_path(output.to_path_buf())
+            .build_video_only(),
+    )
+    .await
+    .unwrap();
+    Box::pin(
+        MovExportSettings {
+            fps: 10,
+            resolution_base: XY::new(64, 64),
+            cursor_only: true,
+        }
+        .export(base, on_progress),
+    )
+    .await
+}
+
+#[tokio::test]
+#[ignore = "requires ffmpeg and a wgpu adapter"]
+async fn cancelled_mov_preserves_existing_destination() {
+    let directory = tempfile::tempdir().unwrap();
+    let project = directory.path().join("project.cap");
+    create_project(&project);
+    let output = directory.path().join("export.mov");
+    fs::write(&output, b"existing export").unwrap();
+
+    assert!(export(&project, &output, |frame| frame < 2).await.is_err());
+
+    assert_eq!(fs::read(&output).unwrap(), b"existing export");
+    assert_eq!(fs::read_dir(directory.path()).unwrap().count(), 2);
+}
+
+#[tokio::test]
+#[ignore = "requires ffmpeg and a wgpu adapter"]
+async fn dropped_export_keeps_temporary_output_owned_by_encoder() {
+    let directory = tempfile::tempdir().unwrap();
+    let project = directory.path().join("project.cap");
+    create_project(&project);
+    let output = directory.path().join("export.mov");
+    fs::write(&output, b"existing export").unwrap();
+
+    let (started_tx, started_rx) = tokio::sync::oneshot::channel();
+    let (release_tx, release_rx) = std::sync::mpsc::channel();
+    let task = tokio::spawn(async move {
+        let mut started_tx = Some(started_tx);
+        export(&project, &output, move |_| {
+            if let Some(started_tx) = started_tx.take() {
+                let _ = started_tx.send(());
+            }
+            release_rx.recv().unwrap();
+            false
+        })
+        .await
+    });
+
+    started_rx.await.unwrap();
+    task.abort();
+    assert!(task.await.unwrap_err().is_cancelled());
+    let temporary_exists = fs::read_dir(directory.path()).unwrap().any(|entry| {
+        entry
+            .unwrap()
+            .file_name()
+            .to_string_lossy()
+            .starts_with(".cap-export-")
+    });
+    release_tx.send(()).unwrap();
+
+    assert!(temporary_exists);
+    assert_eq!(
+        fs::read(directory.path().join("export.mov")).unwrap(),
+        b"existing export"
+    );
+}
+
+#[tokio::test]
+#[ignore = "requires ffmpeg and a wgpu adapter"]
+async fn finished_mov_replaces_existing_destination() {
+    let directory = tempfile::tempdir().unwrap();
+    let project = directory.path().join("project.cap");
+    create_project(&project);
+    let output = directory.path().join("export.mov");
+    fs::write(&output, b"existing export").unwrap();
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt;
+        fs::set_permissions(&output, fs::Permissions::from_mode(0o600)).unwrap();
+    }
+    let destination = output.clone();
+
+    let result = export(&project, &output, move |_| {
+        assert_eq!(fs::read(&destination).unwrap(), b"existing export");
+        true
+    })
+    .await
+    .unwrap();
+
+    assert_eq!(result, output);
+    assert_playable_mov(&output);
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt;
+        assert_eq!(
+            fs::metadata(&output).unwrap().permissions().mode() & 0o777,
+            0o600
+        );
+    }
+    assert_eq!(fs::read_dir(directory.path()).unwrap().count(), 2);
+}
+
+#[tokio::test]
+#[ignore = "requires ffmpeg and a wgpu adapter"]
+async fn mov_reports_destination_replacement_failure() {
+    let directory = tempfile::tempdir().unwrap();
+    let project = directory.path().join("project.cap");
+    create_project(&project);
+    let output = directory.path().join("export.mov");
+    fs::create_dir(&output).unwrap();
+
+    assert!(export(&project, &output, |_| true).await.is_err());
+
+    assert!(output.is_dir());
+    assert_eq!(fs::read_dir(directory.path()).unwrap().count(), 2);
+}
+
+#[tokio::test]
+#[ignore = "requires ffmpeg and a wgpu adapter"]
+async fn finished_mov_creates_new_destination() {
+    let directory = tempfile::tempdir().unwrap();
+    let project = directory.path().join("project.cap");
+    create_project(&project);
+    let output = directory.path().join("export.mov");
+    let destination = output.clone();
+
+    let result = export(&project, &output, move |_| {
+        assert!(!destination.exists());
+        true
+    })
+    .await
+    .unwrap();
+
+    assert_eq!(result, output);
+    assert_playable_mov(&output);
+    assert_eq!(fs::read_dir(directory.path()).unwrap().count(), 2);
+}