diff --git a/crates/export/src/mov.rs b/crates/export/src/mov.rs index a71d1da8a8..c26d17dc30 100644 --- a/crates/export/src/mov.rs +++ b/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) } } diff --git a/crates/export/src/mp4.rs b/crates/export/src/mp4.rs index 8c4fb4cae1..be25ee2e62 100644 --- a/crates/export/src/mp4.rs +++ b/crates/export/src/mp4.rs @@ -1260,25 +1260,35 @@ 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, @@ -1286,6 +1296,20 @@ impl TemporaryExportOutput { } 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( diff --git a/crates/export/tests/mov_cancellation.rs b/crates/export/tests/mov_cancellation.rs new file mode 100644 index 0000000000..e6f8f47c4a --- /dev/null +++ b/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 { + 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); +}