diff --git a/crates/engine/src/output_guard.rs b/crates/engine/src/output_guard.rs index 86c9fa5e6..fc625c309 100644 --- a/crates/engine/src/output_guard.rs +++ b/crates/engine/src/output_guard.rs @@ -312,6 +312,7 @@ struct Clash<'a> { #[derive(Default)] struct Writes { all: Vec, + path_bytes: usize, /// The first output to write a file, by name. files: HashMap, /// ... and by what the file is on its volume. @@ -374,7 +375,12 @@ impl Writes { } } - fn push(&mut self, write: Write) { + fn push(&mut self, write: Write) -> Result<(), String> { + let path_bytes = self.path_bytes.saturating_add(write.path.len()); + if self.all.len() >= 65_536 || path_bytes > 16 * 1024 * 1024 { + return Err("render output plan exceeds 65536 paths or 16 MiB of path names; split the queue".into()); + } + self.path_bytes = path_bytes; let at = self.all.len(); match &write.target { Target::File { key, identity } => { @@ -386,6 +392,7 @@ impl Writes { Target::Frames { .. } => {} } self.all.push(write); + Ok(()) } } @@ -459,8 +466,11 @@ impl Session { if seen.contains(&item.id) { continue; } - let reason = match self.check_output(&protected, item, path) { - Ok(reason) => reason, + let reason = match self.write_paths(item, path) { + Ok(paths) => paths.iter().find_map(|path| match self.check_output(&protected, item, path) { + Ok(reason) => reason, + Err(reason) => Some(reason), + }), Err(reason) => Some(reason), }; if let Some(reason) = reason { @@ -472,18 +482,55 @@ impl Session { } /// What the job writes more than once. An item in `refused` writes nothing, so it takes no part. - pub(crate) fn duplicate_outputs(&self, items: &[Vec<(RenderQueueItem, String)>], refused: &[Refused]) -> Vec { + pub(crate) fn duplicate_outputs(&self, items: &[Vec<(RenderQueueItem, String)>], refused: &[Refused]) -> Result, String> { let skip: HashSet = refused.iter().map(|r| r.item).collect(); let mut writes = Writes::default(); let mut duplicates = Vec::new(); for (m, path) in items.iter().flatten().filter(|(m, _)| !skip.contains(&m.id)) { - let write = self.write_of(m, path, true); - if let Some(clash) = writes.clash(&write) { - duplicates.push(self.duplicate_of(&write, &clash)); + let paths = self.write_paths(m, path)?; + // Alternative destinations of one output cannot collide with themselves. + for path in &paths { + let write = self.write_of(m, path, true); + if let Some(clash) = writes.clash(&write) { + duplicates.push(self.duplicate_of(&write, &clash)); + break; + } + } + for path in paths { + writes.push(self.write_of(m, &path, true))?; } - writes.push(write); } - duplicates + Ok(duplicates) + } + + /// Every possible destination, since quota decisions can change during rendering. + fn write_paths<'a>(&self, item: &RenderQueueItem, path: &'a str) -> Result>, String> { + let overflow = &self.project.render_prefs.overflow_folders; + const PATH_BYTES: usize = 1024 * 1024; + if path.len() > PATH_BYTES { + return Err("render output destinations exceed 1 MiB of path names".into()); + } + let mut paths = vec![std::borrow::Cow::Borrowed(path)]; + let mut bytes = path.len(); + if item.settings.storage_overflow && self.storage_quota.is_some() { + if overflow.len() > 256 { + return Err("render output planning supports at most 256 storage overflow folders".into()); + } + if let Some(name) = Path::new(path).file_name() { + for folder in overflow { + if bytes.saturating_add(folder.len()).saturating_add(name.len()).saturating_add(1) > PATH_BYTES { + return Err("render output destinations exceed 1 MiB of path names".into()); + } + let destination = Path::new(folder).join(name).to_string_lossy().into_owned(); + bytes = bytes.saturating_add(destination.len()); + if bytes > PATH_BYTES { + return Err("render output destinations exceed 1 MiB of path names".into()); + } + paths.push(std::borrow::Cow::Owned(destination)); + } + } + } + Ok(paths) } /// What `path`, an output of `m`, writes. `by_identity`: see [`Target::file`]. @@ -494,7 +541,7 @@ impl Session { } else { Target::file(Path::new(path), by_identity) }; - let skips = m.settings.skip_existing && self.exporter.as_ref().is_some_and(|exporter| exporter.supports_skip_existing()); + let skips = m.settings.skip_existing && self.exporter.as_ref().is_some_and(|exporter| exporter.supports_skip_existing(&m.settings)); Write { item: m.id, path: path.to_string(), target, skips } } @@ -548,7 +595,7 @@ impl Session { // Native source protection stays with its filesystem guard; virtual web outputs still // need distinct queue names because the browser stores completed files by path. let protected = if cfg!(target_arch = "wasm32") { Protected::default() } else { self.protected_files() }; - let queue = self.queue_writes(); + let Ok(queue) = self.queue_writes() else { return }; if protected.is_empty() && queue.is_empty() { return; } @@ -566,7 +613,11 @@ impl Session { } Ok(None) => {} } - in_the_way |= queue.clash(&self.write_of(module, path, false)).is_some(); + let Ok(paths) = self.write_paths(module, path) else { + item.output.output = template; + return; + }; + in_the_way |= paths.iter().any(|path| queue.clash(&self.write_of(module, path, false)).is_some()); } if !in_the_way { return; @@ -577,14 +628,16 @@ impl Session { /// What the items of the queue write (by name: a new item is not compared with what is on /// disk). - fn queue_writes(&self) -> Writes { + fn queue_writes(&self) -> Result { let mut writes = Writes::default(); for outputs in self.project.render_queue.iter().filter_map(|it| self.outputs_of(it, None).ok()) { for (m, path) in &outputs { - writes.push(self.write_of(m, path, false)); + for path in self.write_paths(m, path)? { + writes.push(self.write_of(m, &path, false))?; + } } } - writes + Ok(writes) } } @@ -602,11 +655,11 @@ mod tests { s.execute("renderQueue.add", json!({"format": "h264", "output": "/virtual/shared.mp4"})).unwrap(); } let (outputs, _) = s.queued_outputs(&effectcraft_project::render_queue::OutputFormat::ALL); - let duplicates = s.duplicate_outputs(&outputs, &[]); + let duplicates = s.duplicate_outputs(&outputs, &[]).unwrap(); assert_eq!(duplicates.len(), 1); assert!(duplicates[0].path.ends_with("shared.mp4")); let refused = [Refused { item: duplicates[0].item, reason: "already refused".into() }]; - assert!(s.duplicate_outputs(&outputs, &refused).is_empty()); + assert!(s.duplicate_outputs(&outputs, &refused).unwrap().is_empty()); } /// An output of queue item `item` that is a file, known by its name alone. @@ -622,11 +675,22 @@ mod tests { fn job_of(writes: impl IntoIterator) -> Writes { let mut job = Writes::default(); for write in writes { - job.push(write); + job.push(write).unwrap(); } job } + #[test] + fn output_plan_path_budget_returns_an_error() { + let mut job = Writes::default(); + let path = "x".repeat(1024 * 1024); + for _ in 0..16 { + job.push(movie(1, &path)).unwrap(); + } + assert!(job.push(movie(2, "one-too-many")).is_err()); + assert_eq!(job.all.len(), 16); + } + /// The item an output replaces a file of, how many files, and the first. fn clash(job: &Writes, later: &Write) -> Option<(u64, u64, i64)> { job.clash(later).map(|c| (c.earlier.item, c.files, c.first)) diff --git a/crates/engine/src/render_queue.rs b/crates/engine/src/render_queue.rs index 045b9f782..41474ceef 100644 --- a/crates/engine/src/render_queue.rs +++ b/crates/engine/src/render_queue.rs @@ -59,7 +59,8 @@ pub trait Exporter: Send + Sync { fn formats(&self) -> Vec; /// Whether Skip Existing Files preserves sequence frames written by earlier items. Exporters /// without an existence check (such as memory sinks) must keep the conservative default. - fn supports_skip_existing(&self) -> bool { + /// The check must cover the final destination for these settings, including any overflow. + fn supports_skip_existing(&self, _settings: &effectcraft_project::render_queue::RenderSettings) -> bool { false } /// Render and write one item (blocking). `progress(done, total)` returns `false` to cancel; @@ -500,7 +501,7 @@ impl Session { let refused = self.check_outputs(&items); // Two outputs of this job on one file: the render would replace its own output, so it is // refused before anything is written (the items refused above take no part). - if let Some(refusal) = crate::output_guard::duplicate_refusal(&self.duplicate_outputs(&items, &refused)) { + if let Some(refusal) = crate::output_guard::duplicate_refusal(&self.duplicate_outputs(&items, &refused)?) { return Err(refusal); } items.retain(|modules| modules.first().is_none_or(|(it, _)| !refused.iter().any(|r| r.item == it.id))); diff --git a/crates/engine/src/tests/render_overwrite.rs b/crates/engine/src/tests/render_overwrite.rs index 8572a56f8..d1ca6e65a 100644 --- a/crates/engine/src/tests/render_overwrite.rs +++ b/crates/engine/src/tests/render_overwrite.rs @@ -28,7 +28,7 @@ impl Exporter for Writer { fn formats(&self) -> Vec { OutputFormat::ALL.to_vec() } - fn supports_skip_existing(&self) -> bool { + fn supports_skip_existing(&self, _: &effectcraft_project::render_queue::RenderSettings) -> bool { true } fn export(&self, job: &ExportJob, _: &mut dyn FnMut(u64, u64) -> bool) -> Result { @@ -633,6 +633,7 @@ impl Fixture { let duplicates: Vec = self .s .duplicate_outputs(&items, &refused) + .unwrap() .iter() .map(|d| { json!({ @@ -658,6 +659,42 @@ fn span(pattern: &str, start: f64, end: f64) -> Value { json!({"format": "png", "output": pattern, "timeSpan": "custom", "start": start, "end": end}) } +fn with_overflow(f: &mut Fixture) { + struct Room; + impl effectcraft_project::render_queue::StorageQuota for Room { + fn has_room(&self, _: &str, _: u64) -> bool { + true + } + } + f.s.storage_quota = Some(Arc::new(Room)); + std::fs::create_dir_all(f.path("overflow")).unwrap(); + Arc::make_mut(&mut f.s.project).render_prefs.overflow_folders = vec![f.text("overflow")]; +} + +#[test] +fn default_names_avoid_possible_storage_overflow_collisions() { + let mut f = Fixture::new("overflow-default"); + f.comp("Main"); + with_overflow(&mut f); + f.add(json!({"output": f.text("other/Main.mp4"), "storageOverflow": true})); + f.add(json!({"storageOverflow": true})); + assert_eq!(f.output(1), f.path("Main_1.mp4")); + assert!(f.clashes().is_empty()); +} + +#[test] +fn possible_storage_overflow_destinations_cannot_replace_footage() { + let mut f = Fixture::new("overflow-footage"); + f.comp("Main"); + with_overflow(&mut f); + f.footage("overflow/Main.mp4"); + f.add(json!({"output": f.text("other/Main.mp4"), "storageOverflow": true})); + let error = f.render(json!({})).unwrap_err().to_string(); + assert!(error.contains("footage") && error.contains("Main.mp4"), "{error}"); + assert!(f.exported().is_empty()); + assert_eq!(f.read("overflow/Main.mp4"), SOURCE); +} + /// After Effects numbers the second item of one composition (`Comp 1_1.mov`). A default output /// name that another item of the queue already has is numbered the same way. #[test] diff --git a/crates/host/src/lib.rs b/crates/host/src/lib.rs index 1137a8bce..9a6556fdc 100644 --- a/crates/host/src/lib.rs +++ b/crates/host/src/lib.rs @@ -33,8 +33,9 @@ impl Exporter for FileExporter { fn formats(&self) -> Vec { effectcraft_export::available_formats() } - fn supports_skip_existing(&self) -> bool { - self.sink.is_none() && !cfg!(target_arch = "wasm32") + fn supports_skip_existing(&self, settings: &effectcraft_project::render_queue::RenderSettings) -> bool { + // The exporter's existence check precedes storage-overflow redirection. + self.sink.is_none() && !cfg!(target_arch = "wasm32") && !settings.storage_overflow } fn export(&self, job: &ExportJob, progress: &mut dyn FnMut(u64, u64) -> bool) -> Result { effectcraft_engine::render::passes::block_on(self.export_async(job, progress)) diff --git a/crates/host/src/tests.rs b/crates/host/src/tests.rs index 447411dc7..f628ce4e4 100644 --- a/crates/host/src/tests.rs +++ b/crates/host/src/tests.rs @@ -1,5 +1,49 @@ use serde_json::json; +#[test] +#[cfg(not(target_arch = "wasm32"))] +fn storage_overflow_cannot_bypass_duplicate_output_protection() { + for same_folder in [true, false] { + refuse_overflow_collision(same_folder); + } +} + +#[cfg(not(target_arch = "wasm32"))] +fn refuse_overflow_collision(same_folder: bool) { + use std::{path::PathBuf, sync::Arc}; + struct OverflowOnly(PathBuf); + impl effectcraft_project::render_queue::StorageQuota for OverflowOnly { + fn has_room(&self, path: &str, _: u64) -> bool { + std::path::Path::new(path).starts_with(&self.0) + } + } + let dir = std::env::temp_dir().join(format!("ec-duplicate-overflow-{}-{same_folder}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let overflow = dir.join("overflow"); + std::fs::create_dir_all(&overflow).unwrap(); + let mut s = effectcraft_engine::Session { + exporter: Some(Arc::new(super::FileExporter::default())), + storage_quota: Some(Arc::new(OverflowOnly(overflow.clone()))), + ..Default::default() + }; + Arc::make_mut(&mut s.project).render_prefs.overflow_folders = vec![overflow.to_string_lossy().into_owned(); 2]; + s.execute("comp.new", json!({"width": 16, "height": 16, "frameRate": 10, "duration": 0.2})).unwrap(); + for index in 0..2 { + let folder = if same_folder { dir.clone() } else { dir.join(index.to_string()) }; + s.execute("renderQueue.add", json!({"format": "png", "output": folder.join("frames_[#####].png"), "skipExisting": true, "storageOverflow": true})) + .unwrap(); + } + let result = s.execute("renderQueue.render", json!({})); + let written = std::fs::read_dir(&overflow).unwrap().count(); + assert!(result.is_err(), "overflow destinations cannot skip previous frames: {result:?}"); + assert_eq!(written, 0, "refused before any overflow file is written"); + s.execute("renderQueue.remove", json!({"index": 2})).unwrap(); + let result = s.execute("renderQueue.render", json!({})).unwrap(); + assert_eq!(result["items"][0]["status"], "Done", "one output's alternatives are not separate outputs: {result}"); + assert_eq!(std::fs::read_dir(&overflow).unwrap().count(), 2); + std::fs::remove_dir_all(&dir).unwrap(); +} + #[test] fn memory_sink_cannot_overwrite_a_previous_output_even_with_skip_existing() { use std::sync::{ diff --git a/docs/agents.md b/docs/agents.md index 3d1f4dd83..8d411add4 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -323,7 +323,8 @@ build real projects through these interfaces; they are worked examples of everyt `renderQueue.duplicate` copy, output modules, segments) name the same file, the whole render is refused before anything is written, naming both items. Sequences clash only on frames both write. A later item with Skip Existing Files leaves the earlier one's frames alone only when - the exporter can check existing files; browser and other memory sinks still report collisions. Default + the exporter can check the final destination. Browser/memory sinks and storage-overflow renders still report collisions. + Preflight compares every configured overflow destination, since available space can change during a render. Default names are numbered like footage collisions when another queue item writes them (`renderQueue.add`, `render.preRender`, `comp.saveFrameAs {queue}`, `file.createProxy`: a second `Main` renders to `Main_1.mp4`); a name you give is kept, and `renderQueue.duplicate` copies the diff --git a/docs/mcp.md b/docs/mcp.md index 116158706..fb02e79b6 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -59,8 +59,9 @@ Two outputs of one job that write the same file (two items with one Output To, a output modules or segments that meet) refuse the whole `renderQueue.render` before anything is written: the reply is `isError: true`, naming both items, and the queue is left as it was. Sequences clash only on the frames both write. Skip Existing Files leaves earlier frames alone -only when the exporter can check existing files; browser and other memory sinks still report -collisions. A new item's default output name is numbered (`Main.mp4`, +only when the exporter can check the final destination; browser/memory sinks and storage-overflow +renders still report collisions. Preflight checks every configured overflow destination, since free +space can change during a render. A new item's default output name is numbered (`Main.mp4`, `Main_1.mp4`) when another item of the queue writes it. ## Progress and cancellation