diff --git a/apps/effectcraft-cli/tests/it/cli.rs b/apps/effectcraft-cli/tests/it/cli.rs index 7a770fa87..5f78ec2c1 100644 --- a/apps/effectcraft-cli/tests/it/cli.rs +++ b/apps/effectcraft-cli/tests/it/cli.rs @@ -419,6 +419,38 @@ fn render_out_is_relative_to_the_working_directory() { let _ = std::fs::remove_dir_all(&root); } +/// Two queue items on one file are refused by the render before anything is written, with the +/// file as it was. +#[test] +fn render_refuses_two_items_on_one_file() { + let root = tmp("render-clash"); + let _ = std::fs::remove_dir_all(&root); + std::fs::create_dir_all(&root).unwrap(); + let out = root.join("clash.mp4"); + std::fs::write(&out, b"EARLIER").unwrap(); + let project = root.join("p.ecproj"); + let p = project.to_str().unwrap(); + let add = json!({"output": out.to_str().unwrap()}).to_string(); + ok_json(&[ + "run", + "comp.new", + r#"{"name":"T","width":64,"height":36,"duration":1}"#, + "renderQueue.add", + &add, + "renderQueue.add", + &add, + "--empty", + "--save-as", + p, + ]); + let (code, v) = run_json(&["render", "--queue", "--project", p]); + assert_eq!(code, 1, "{v}"); + let error = v["error"].as_str().unwrap(); + assert!(error.contains("#1") && error.contains("#2") && error.contains("clash.mp4"), "{v}"); + assert_eq!(std::fs::read(&out).unwrap(), b"EARLIER"); + let _ = std::fs::remove_dir_all(&root); +} + /// A real stdio client: each edit's reply is read before disconnecting or killing the server. struct McpChild { child: std::process::Child, diff --git a/crates/engine/src/output_guard.rs b/crates/engine/src/output_guard.rs index 5d4dc289e..ba6947694 100644 --- a/crates/engine/src/output_guard.rs +++ b/crates/engine/src/output_guard.rs @@ -5,12 +5,22 @@ //! exporter. Existing aliases are compared by native file identity, including hard links. //! Missing sources remain protected by their normalized path. Checks happen before dispatch; //! filesystem changes between the check and the write require exporter-side revalidation. +//! +//! Nor does a render write one file twice. When two outputs of one job name the same file (two +//! items with one Output To, a duplicated item, output modules or segments that meet), the one +//! rendered later replaces what the earlier one just wrote, and nothing is on disk to say so when +//! the job starts. [`Session::duplicate_outputs`] lists them and the whole render is refused before +//! anything is written: no answer makes a render replacing its own output right. A numbered sequence is compared by the frames +//! it writes: two items of one pattern clash only where their frames do, and an item that skips +//! existing frames leaves the earlier one's alone. A new item's default output name steps aside +//! the same way it does for footage ([`Session::avoid_sources`]). +//! use std::collections::{HashMap, HashSet}; use std::path::{Component, Path, PathBuf}; use std::sync::Arc; -use effectcraft_project::render_queue::{RenderQueueItem, log_path, numbered_template, sequence_frame, sequence_path}; +use effectcraft_project::render_queue::{RenderQueueItem, log_path, numbered_template, sequence_frame, sequence_parts, sequence_path}; use effectcraft_project::{Footage, ItemKind}; use crate::Session; @@ -26,6 +36,10 @@ const PROBE_LIMIT: u64 = 100_000; /// render time. const MAX_NUMBER: u32 = 1_000; +/// Stands for the frame number in the name a numbered sequence is compared by: a character no +/// file is named with. +const FRAME: char = '\u{e000}'; + /// What reads a file the project depends on. #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) enum Source { @@ -148,6 +162,24 @@ fn identity_hash(handle: &same_file::Handle) -> u64 { hash.finish() } +/// What an existing regular file is known by on its volume (links and hard links to one file give one), +/// `None` when it does not exist, is not a regular file, or the volume has no such identity. +fn identity(path: &Path) -> Option { + #[cfg(not(target_arch = "wasm32"))] + { + // Opening a FIFO can wait for a writer: only regular files are looked at. + if !std::fs::metadata(path).ok()?.is_file() { + return None; + } + same_file::Handle::from_path(path).ok().map(|handle| identity_hash(&handle).to_string()) + } + #[cfg(target_arch = "wasm32")] + { + let _ = path; + None + } +} + /// `path` made absolute, with `.` and `..` resolved by name alone (no file is looked at). fn lexical(path: &Path) -> PathBuf { let absolute = std::path::absolute(path).unwrap_or_else(|_| path.to_path_buf()); @@ -228,6 +260,105 @@ fn frames_on_disk(path: &str, first: i64, count: u64) -> Result, St .collect()) } +/// What an output writes, as far as a clash with another output goes. +#[derive(Clone, Debug, PartialEq)] +enum Target { + /// One file: known by its name and, when it exists, by what it is on its volume. + File { key: String, identity: Option }, + /// The numbered files `first..end` of a sequence: `key` is its pattern, with the width the + /// frame numbers are padded to. + Frames { key: String, first: i64, end: i64 }, +} + +impl Target { + /// The file at `path`. `by_identity` also looks up what it is on its volume (a link to it + /// is it); that asks the file system, so it is left out where only names matter. + fn file(path: &Path, by_identity: bool) -> Target { + let identity = if by_identity { identity(path) } else { None }; + Target::File { key: key(path), identity } + } + + /// The `count` files of the sequence `path` (its frame-number pattern) from frame `first`. + fn frames(path: &str, first: i64, count: u64) -> Target { + let (before, width, after) = sequence_parts(path); + let key = format!("{width}:{}", key(Path::new(&format!("{before}{FRAME}{after}")))); + Target::Frames { key, first, end: first.saturating_add(i64::try_from(count).unwrap_or(i64::MAX)) } + } +} + +/// One output of a job. +#[derive(Clone, Debug)] +struct Write { + /// The queue item (`0`: one that is not in the queue yet). + item: u64, + /// The output path (a sequence's has its frame-number pattern). + path: String, + target: Target, + /// Skip Existing Files: a frame that is there already is left alone. + skips: bool, +} + +/// What an output rendered later writes over. +struct Clash<'a> { + /// The earlier output that writes it. + earlier: &'a Write, + /// How many files, and the first of them (its frame number, for a sequence). + files: u64, + first: i64, +} + +/// The outputs of a job in the order they render, to find the one that writes what an earlier +/// one does. +#[derive(Default)] +struct Writes { + all: Vec, + /// The first output to write a file, by name. + files: HashMap, + /// ... and by what the file is on its volume. + identities: HashMap, + /// Every output that writes the frames of a sequence pattern. + frames: HashMap>, +} + +impl Writes { + fn is_empty(&self) -> bool { + self.all.is_empty() + } + + /// The earliest output `later`, rendered after all of these, writes over. + fn clash(&self, later: &Write) -> Option> { + match &later.target { + Target::File { key, identity } => { + let at = self.files.get(key).or_else(|| identity.as_ref().and_then(|id| self.identities.get(id)))?; + Some(Clash { earlier: self.all.get(*at)?, files: 1, first: 0 }) + } + // A frame that is there already is left alone, so a later output that skips existing + // frames replaces none of an earlier one's. + Target::Frames { .. } if later.skips => None, + Target::Frames { key, first, end } => self.frames.get(key)?.iter().filter_map(|at| self.all.get(*at)).find_map(|earlier| { + let Target::Frames { first: from, end: to, .. } = &earlier.target else { return None }; + let start = (*first).max(*from); + let files = u64::try_from((*end).min(*to).saturating_sub(start)).ok().filter(|n| *n > 0)?; + Some(Clash { earlier, files, first: start }) + }), + } + } + + fn push(&mut self, write: Write) { + let at = self.all.len(); + match &write.target { + Target::File { key, identity } => { + self.files.entry(key.clone()).or_insert(at); + if let Some(identity) = identity { + self.identities.entry(identity.clone()).or_insert(at); + } + } + Target::Frames { key, .. } => self.frames.entry(key.clone()).or_default().push(at), + } + self.all.push(write); + } +} + /// A queue item refused before any of its output modules are dispatched. #[derive(Clone, Debug, PartialEq)] pub(crate) struct Refused { @@ -235,6 +366,50 @@ pub(crate) struct Refused { pub reason: String, } +/// An output that would write a file an earlier output of the same job writes. +#[derive(Clone, Debug, PartialEq)] +pub(crate) struct DuplicateOutput { + /// The queue item that would write over it (the later one), its place in the queue (1-based) + /// and its composition. + pub item: u64, + pub index: usize, + pub comp: String, + /// The file: a sequence's pattern, or the frame when it is one. + pub path: String, + /// How many files the two outputs have in common. + pub files: u64, + pub sequence: bool, + /// The queue item whose output is written first, and its place and composition. + pub with: u64, + pub with_index: usize, + pub with_comp: String, +} + +impl DuplicateOutput { + /// Why the render is refused, naming the two items. + pub(crate) fn reason(&self) -> String { + let label = |index: usize, comp: &str| format!("#{index} “{comp}”"); + let what = if self.sequence && self.files != 1 { format!("{} frames of {}", self.files, self.path) } else { self.path.clone() }; + let rule = "and a render never writes one file twice; choose another Output To for one of them"; + if self.item == self.with { + format!("{} would write {what} twice (two of its output modules), {rule}", label(self.index, &self.comp)) + } else { + format!("{} and {} would both write {what}, {rule}", label(self.with_index, &self.with_comp), label(self.index, &self.comp)) + } + } +} + +/// Why a render is refused when outputs of the job write one file twice: it would replace its own output. +pub(crate) fn duplicate_refusal(duplicates: &[DuplicateOutput]) -> Option { + let first = duplicates.first()?; + let more = match duplicates.len().saturating_sub(1) { + 0 => String::new(), + 1 => "; 1 more output would write a file twice too".into(), + n => format!("; {n} more outputs would write a file twice too"), + }; + Some(format!("{}. Nothing was rendered{more}", first.reason())) +} + impl Session { /// The files the project reads. pub(crate) fn protected_files(&self) -> Protected { @@ -266,6 +441,51 @@ impl Session { refused } + /// 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 { + if cfg!(target_arch = "wasm32") { + return Vec::new(); + } + 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)); + } + writes.push(write); + } + duplicates + } + + /// What `path`, an output of `m`, writes. `by_identity`: see [`Target::file`]. + fn write_of(&self, m: &RenderQueueItem, path: &str, by_identity: bool) -> Write { + let target = if m.output.format.is_sequence() { + let (first, count) = self.project.comp(m.comp).map_or((0, 0), |c| (m.settings.first_frame(c), m.settings.frame_count(c))); + Target::frames(path, first, count) + } else { + Target::file(Path::new(path), by_identity) + }; + Write { item: m.id, path: path.to_string(), target, skips: m.settings.skip_existing } + } + + /// The report of `later` writing over `clash`. + fn duplicate_of(&self, later: &Write, clash: &Clash) -> DuplicateOutput { + // Where a queue item stands, and what it renders. + let place = |id: u64| { + let index = self.project.render_queue.iter().position(|i| i.id == id); + let comp = index.and_then(|k| self.project.render_queue.get(k)).and_then(|i| self.project.item(i.comp)).map(|i| i.name.clone()); + (index.map_or(0, |k| k + 1), comp.unwrap_or_default()) + }; + let sequence = matches!(later.target, Target::Frames { .. }); + // One shared frame is named by its file, more by the pattern. + let path = if sequence && clash.files == 1 { sequence_path(&later.path, clash.first) } else { later.path.clone() }; + let (index, comp) = place(later.item); + let (with_index, with_comp) = place(clash.earlier.item); + DuplicateOutput { item: later.item, index, comp, path, files: clash.files, sequence, with: clash.earlier.item, with_index, with_comp } + } + fn check_output(&self, protected: &Protected, item: &RenderQueueItem, path: &str) -> Result, String> { if protected.is_empty() { return Ok(None); @@ -290,43 +510,54 @@ impl Session { .find_map(|path| protected.find(path).map(|source| source.refusal(&path.to_string_lossy())))) } - /// A new item's default output must not be a file the project reads: `[compName]_1`, - /// `[compName]_2`… take its place (and avoid the queue's other outputs), as After Effects - /// numbers a second item of the same composition. An output nothing is in the way of keeps - /// the composition's own name, and one the caller chose is not touched: rendering it is - /// refused instead. + /// A new item's default output must not be a file the project reads, nor one another item of + /// the queue writes (any of its output modules, whether or not it is ticked): `[compName]_1`, + /// `[compName]_2`… take its place, as After Effects numbers a second item of the same + /// composition. An output nothing is in the way of keeps the composition's own name; the name + /// is stored in the item, so it stays as it is when the item it avoided goes away. One the + /// caller chose is not touched: rendering it is refused instead ([`Session::duplicate_outputs`]). pub(crate) fn avoid_sources(&self, item: &mut RenderQueueItem) { if cfg!(target_arch = "wasm32") { return; } let protected = self.protected_files(); - if protected.is_empty() { + let queue = self.queue_writes(); + if protected.is_empty() && queue.is_empty() { return; } let template = item.output.output.clone(); - let mut taken: Option> = None; - 'candidate: for n in 0..=MAX_NUMBER { + for n in 0..=MAX_NUMBER { item.output.output = if n == 0 { template.clone() } else { numbered_template(&template, n) }; let Ok(outputs) = self.outputs_of(item, None) else { break }; + let mut in_the_way = false; for (module, path) in &outputs { match self.check_output(&protected, module, path) { - Ok(Some(_)) => continue 'candidate, - Err(_) => break 'candidate, + Ok(Some(_)) => in_the_way = true, + Err(_) => { + item.output.output = template; + return; + } Ok(None) => {} } + in_the_way |= queue.clash(&self.write_of(module, path, false)).is_some(); } - // Only a name that had to move is kept clear of the other items. - let others = if n == 0 { None } else { Some(taken.get_or_insert_with(|| self.queue_output_keys())) }; - if others.is_none_or(|others| !outputs.iter().any(|(_, path)| others.contains(&key(Path::new(path))))) { + if !in_the_way { return; } } item.output.output = template; } - /// [`key`] of every output of the queue's items. - fn queue_output_keys(&self) -> HashSet { - self.project.render_queue.iter().filter_map(|it| self.outputs_of(it, None).ok()).flatten().map(|(_, path)| key(Path::new(&path))).collect() + /// 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 { + 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)); + } + } + writes } } @@ -334,6 +565,148 @@ impl Session { mod tests { use super::*; + /// An output of queue item `item` that is a file, known by its name alone. + fn movie(item: u64, path: &str) -> Write { + Write { item, path: path.into(), target: Target::file(Path::new(path), false), skips: false } + } + + /// An output of queue item `item` that is `count` frames of a sequence from frame `first`. + fn frames(item: u64, pattern: &str, first: i64, count: u64) -> Write { + Write { item, path: pattern.into(), target: Target::frames(pattern, first, count), skips: false } + } + + fn job_of(writes: impl IntoIterator) -> Writes { + let mut job = Writes::default(); + for write in writes { + job.push(write); + } + job + } + + /// 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)) + } + + #[test] + fn a_file_is_replaced_by_the_output_that_has_its_name() { + let job = job_of([movie(1, "out/a.mp4"), movie(2, "out/b.mp4"), movie(3, "out/b.mp4")]); + assert_eq!(clash(&job, &movie(9, "out/a.mp4")), Some((1, 1, 0))); + assert_eq!(clash(&job, &movie(9, "out/b.mp4")), Some((2, 1, 0)), "the first to write it is named"); + assert_eq!(clash(&job, &movie(9, "out/x/../a.mp4")), Some((1, 1, 0)), "dot segments are resolved"); + assert_eq!(clash(&job, &movie(9, "out/c.mp4")), None); + assert_eq!(clash(&job, &movie(9, "other/a.mp4")), None); + assert_eq!(clash(&job, &frames(9, "out/a.mp4", 0, 5)), None, "a sequence is not a movie"); + assert!(Writes::default().is_empty() && !job.is_empty()); + } + + #[test] + fn two_names_of_one_file_on_disk_are_one_file() { + let on_disk = |item: u64, name: &str, id: Option<&str>| Write { + item, + path: name.into(), + target: Target::File { key: name.into(), identity: id.map(String::from) }, + skips: false, + }; + let job = job_of([on_disk(1, "/a/link.mp4", Some("7:1")), on_disk(2, "/a/nothing-yet.mp4", None)]); + assert_eq!(clash(&job, &on_disk(9, "/b/real.mp4", Some("7:1"))), Some((1, 1, 0))); + assert_eq!(clash(&job, &on_disk(9, "/b/real.mp4", Some("7:2"))), None); + assert_eq!(clash(&job, &on_disk(9, "/b/real.mp4", None)), None, "a file that is not there has no identity"); + assert_eq!(clash(&job, &on_disk(9, "/a/nothing-yet.mp4", None)), Some((2, 1, 0)), "its name is all there is"); + } + + #[test] + fn sequences_clash_where_their_frames_do() { + let job = job_of([frames(1, "a_[#####].png", 0, 5)]); // frames 0-4 + assert_eq!(clash(&job, &frames(2, "a_[#####].png", 5, 5)), None, "the five after it"); + assert_eq!(clash(&job, &frames(2, "a_[#####].png", -3, 3)), None, "the three before it"); + assert_eq!(clash(&job, &frames(2, "a_[#####].png", 4, 5)), Some((1, 1, 4)), "frame 4 only"); + assert_eq!(clash(&job, &frames(2, "a_[#####].png", 2, 1)), Some((1, 1, 2))); + assert_eq!(clash(&job, &frames(2, "a_[#####].png", 1, 2)), Some((1, 2, 1))); + assert_eq!(clash(&job, &frames(2, "a_[#####].png", -3, 100)), Some((1, 5, 0)), "all five"); + // The first of several that overlap is the one named. + let job = job_of([frames(1, "a_[#####].png", 0, 3), frames(2, "a_[#####].png", 3, 3)]); + assert_eq!(clash(&job, &frames(9, "a_[#####].png", 2, 3)), Some((1, 1, 2))); + assert_eq!(clash(&job, &frames(9, "a_[#####].png", 3, 3)), Some((2, 3, 3))); + } + + #[test] + fn sequences_of_other_names_do_not_clash() { + let job = job_of([frames(1, "out/a_[#####].png", 0, 5)]); + for other in ["out/b_[#####].png", "out/a_[####].png", "out/a_[#####].jpg", "elsewhere/a_[#####].png", "out/a[#####].png", "out/a_[#####]/x.png"] { + assert_eq!(clash(&job, &frames(2, other, 0, 5)), None, "{other}"); + } + // One name, however it is spelled: the run with or without brackets, none at all, dot segments. + for same in ["out/a_[#####].png", "out/a_#####.png", "out/a.png", "out/b/../a_[#####].png"] { + assert_eq!(clash(&job, &frames(2, same, 0, 5)), Some((1, 5, 0)), "{same}"); + } + // A frame number that starts a folder's name is not a file name's. + let job = job_of([frames(1, "out/[##].png", 0, 5)]); + assert_eq!(clash(&job, &frames(2, "out[##].png", 0, 5)), None); + assert_eq!(clash(&job, &frames(2, "out/[##].png", 0, 5)), Some((1, 5, 0))); + } + + /// Skip Existing Files leaves a frame that is there alone: it matters for the output that + /// comes second, and for sequences only. + #[test] + fn an_output_that_skips_existing_frames_replaces_none() { + let skipping = |mut write: Write| { + write.skips = true; + write + }; + let job_of_one = job_of([frames(1, "a_[#####].png", 0, 5)]); + assert_eq!(clash(&job_of_one, &skipping(frames(2, "a_[#####].png", 0, 5))), None); + let job_of_one = job_of([skipping(frames(1, "a_[#####].png", 0, 5))]); + assert_eq!(clash(&job_of_one, &frames(2, "a_[#####].png", 3, 5)), Some((1, 2, 3)), "the earlier one's frames are replaced"); + let job_of_one = job_of([movie(1, "a.mp4")]); + assert_eq!(clash(&job_of_one, &skipping(movie(2, "a.mp4"))), Some((1, 1, 0)), "a movie is written whole"); + } + + #[test] + fn frame_numbers_beyond_what_a_project_holds_do_not_panic() { + let top = i64::MAX - 1; + let job = job_of([frames(1, "a_[#####].png", top, u64::MAX), frames(2, "a_[#####].png", i64::MIN, u64::MAX)]); + assert_eq!(clash(&job, &frames(9, "a_[#####].png", top, u64::MAX)), Some((1, 1, top))); + assert_eq!(clash(&job, &frames(9, "a_[#####].png", -5, 3)), Some((2, 3, -5))); + assert_eq!(clash(&job, &frames(9, "a_[#####].png", 0, 5)), None, "between the two"); + assert_eq!(clash(&job, &frames(9, "a_[#####].png", i64::MAX, 0)), None, "nothing written"); + assert_eq!(clash(&job, &frames(9, "a_[#####].png", i64::MIN, 0)), None); + } + + #[test] + fn the_reason_names_both_items_and_what_they_write() { + let both = DuplicateOutput { + item: 7, + index: 3, + comp: "Main".into(), + path: "/o/Main.mp4".into(), + files: 1, + sequence: false, + with: 4, + with_index: 1, + with_comp: "Intro".into(), + }; + assert_eq!( + both.reason(), + "#1 “Intro” and #3 “Main” would both write /o/Main.mp4, and a render never writes one file twice; choose another Output To for one of them" + ); + let sequence = DuplicateOutput { sequence: true, files: 12, path: "/o/Main_[#####].png".into(), ..both.clone() }; + assert!(sequence.reason().contains("would both write 12 frames of /o/Main_[#####].png,"), "{}", sequence.reason()); + let one = DuplicateOutput { files: 1, path: "/o/Main_00004.png".into(), ..sequence }; + assert!(one.reason().contains("would both write /o/Main_00004.png,"), "{}", one.reason()); + let itself = DuplicateOutput { with: 7, with_index: 3, with_comp: "Main".into(), ..both.clone() }; + assert!(itself.reason().starts_with("#3 “Main” would write /o/Main.mp4 twice (two of its output modules), and a render never"), "{}", itself.reason()); + + let mut found = vec![]; + assert_eq!(duplicate_refusal(&found), None); + found.push(both.clone()); + assert_eq!(duplicate_refusal(&found), Some(format!("{}. Nothing was rendered", both.reason()))); + found.push(both.clone()); + assert!(duplicate_refusal(&found).is_some_and(|r| r.ends_with("Nothing was rendered; 1 more output would write a file twice too"))); + found.push(both); + assert!(duplicate_refusal(&found).is_some_and(|r| r.ends_with("Nothing was rendered; 2 more outputs would write a file twice too"))); + } + #[test] #[cfg(not(target_arch = "wasm32"))] fn unrelated_same_size_frames_are_indexed_once() { diff --git a/crates/engine/src/render_queue.rs b/crates/engine/src/render_queue.rs index 1ff91e8e8..0285a64d2 100644 --- a/crates/engine/src/render_queue.rs +++ b/crates/engine/src/render_queue.rs @@ -493,6 +493,11 @@ impl Session { let formats = exporter.formats(); let (mut items, mut problems) = self.queued_outputs(&formats); 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)) { + return Err(refusal); + } items.retain(|modules| modules.first().is_none_or(|(it, _)| !refused.iter().any(|r| r.item == it.id))); problems.extend(refused.iter().map(|r| (r.item, RenderStatus::Failed(r.reason.clone())))); if !problems.is_empty() { diff --git a/crates/engine/src/tests/render_overwrite.rs b/crates/engine/src/tests/render_overwrite.rs index 420763da2..68357acb7 100644 --- a/crates/engine/src/tests/render_overwrite.rs +++ b/crates/engine/src/tests/render_overwrite.rs @@ -1,5 +1,6 @@ //! Rendering never replaces a file the project reads (#1743). Ordinary-file replacement -//! behavior stays with the exporter and its confirmation API. +//! behavior stays with the exporter and its confirmation API. Two items of one job never write +//! the same file either. use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; @@ -604,3 +605,369 @@ fn a_watched_folder_still_renders_a_project_again_over_its_outputs() { assert_eq!(done[0]["state"], "done", "{:?}", done[0]); assert_eq!(f.read("Main.mp4"), RENDERED); } + +// ------------------------------------------------------------ one file, two items of one job +// +// Nothing is on disk when a job starts, so the check for files that exist said nothing about two +// queue items that resolve to one output: the item rendered last replaced the file of the one +// before it, inside the same job. + +impl Fixture { + /// The output path of every queue item, in queue order. + fn outputs(&self) -> Vec { + (0..self.s.project.render_queue.len()).map(|i| self.output(i)).collect() + } + + fn add(&mut self, params: Value) { + self.s.execute("renderQueue.add", params).unwrap(); + } + + /// What the queue would write twice, in the shape the tests read: `{"duplicates": [...]}`. + fn check(&mut self) -> Value { + let formats = self.s.exporter.as_ref().map(|e| e.formats()).unwrap_or_default(); + let (items, _) = self.s.queued_outputs(&formats); + let refused = self.s.check_outputs(&items); + let duplicates: Vec = self + .s + .duplicate_outputs(&items, &refused) + .iter() + .map(|d| { + json!({ + "item": d.item, "index": d.index, "comp": d.comp, "path": d.path, "files": d.files, "sequence": d.sequence, + "with": {"item": d.with, "index": d.with_index, "comp": d.with_comp}, "reason": d.reason(), + }) + }) + .collect(); + json!({"duplicates": duplicates}) + } + + /// What `check` lists under `duplicates`: for each, the place in the queue of the item that + /// would write over a file, the place of the item that wrote it first, and how many files. + fn clashes(&mut self) -> Vec<(u64, u64, u64)> { + let found = self.check(); + let list = found["duplicates"].as_array().unwrap_or_else(|| panic!("checkOutputs lists no duplicates: {found}")); + list.iter().map(|d| (d["index"].as_u64().unwrap_or(0), d["with"]["index"].as_u64().unwrap_or(0), d["files"].as_u64().unwrap_or(0))).collect() + } +} + +/// A PNG sequence of `pattern` for the composition's seconds `start..end`. +fn span(pattern: &str, start: f64, end: f64) -> Value { + json!({"format": "png", "output": pattern, "timeSpan": "custom", "start": start, "end": end}) +} + +/// 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] +fn a_second_default_item_for_one_comp_gets_the_next_free_name() { + let mut f = Fixture::new("second-default"); + f.comp("Main"); + for _ in 0..3 { + f.add(json!({})); + } + assert_eq!(f.outputs(), [f.path("Main.mp4"), f.path("Main_1.mp4"), f.path("Main_2.mp4")]); + // The name is stored in the item: taking the first one out renames nothing, and the name it + // frees is the next default. + f.s.execute("renderQueue.remove", json!({"index": 1})).unwrap(); + assert_eq!(f.outputs(), [f.path("Main_1.mp4"), f.path("Main_2.mp4")]); + f.add(json!({})); + assert_eq!(f.output(2), f.path("Main.mp4")); + assert!(f.clashes().is_empty()); + f.render(json!({})).unwrap(); + for file in ["Main.mp4", "Main_1.mp4", "Main_2.mp4"] { + assert_eq!(f.read(file), RENDERED, "{file}"); + } + assert_eq!(f.exported().len(), 3); +} + +#[test] +fn only_outputs_that_name_one_file_are_numbered() { + let mut f = Fixture::new("same-file-only"); + f.comp("Main"); + f.add(json!({"format": "h264"})); + f.add(json!({"format": "prores"})); + f.add(json!({"format": "hevc"})); // H.264 and HEVC both write .mp4 + f.add(json!({"format": "png"})); + f.add(json!({"format": "png"})); // the same five frames of the same sequence + assert_eq!(f.outputs(), [f.path("Main.mp4"), f.path("Main.mov"), f.path("Main_1.mp4"), f.path("Main_[#####].png"), f.path("Main_1_[#####].png")]); + assert!(f.clashes().is_empty()); +} + +#[test] +fn a_default_name_avoids_every_output_module_of_the_other_items() { + let mut f = Fixture::new("modules"); + f.comp("Main"); + f.add(json!({})); + // The item's second module is named Main_2.mp4, so that it does not clash with its first. + let item = f.s.execute("render.addOutputModule", json!({})).unwrap(); + assert_eq!(Path::new(item["extraOutputs"][0]["outputPath"].as_str().unwrap()), f.path("Main_2.mp4"), "{item}"); + f.add(json!({})); + f.add(json!({})); + assert_eq!(f.output(1), f.path("Main_1.mp4")); + assert_eq!(f.output(2), f.path("Main_3.mp4"), "Main_2.mp4 is the first item's second module"); + assert!(f.clashes().is_empty()); +} + +#[test] +fn a_name_the_caller_gave_is_never_numbered() { + let mut f = Fixture::new("named"); + f.comp("Main"); + let out = f.text("final.mp4"); + f.add(json!({"output": out})); + f.add(json!({"output": out})); + f.add(json!({"path": out})); + assert_eq!(f.outputs(), vec![f.path("final.mp4"); 3]); + assert!(f.s.project.render_queue.iter().all(|i| i.output.output == out), "the names stored in the items are the ones given"); + // Rendering them is what is refused (below): items 2 and 3 each clash with item 1. + assert_eq!(f.clashes(), [(2, 1, 1), (3, 1, 1)]); +} + +#[test] +fn a_pre_render_numbers_its_default_name_too() { + let mut f = Fixture::new("pre-render"); + f.comp("Main"); + f.s.execute("render.preRender", json!({})).unwrap(); + f.s.execute("render.preRender", json!({})).unwrap(); + f.s.execute("render.preRender", json!({"output": f.text("mine.mov")})).unwrap(); + f.s.execute("render.preRender", json!({"output": f.text("mine.mov")})).unwrap(); + assert_eq!(f.outputs(), [f.path("Main_prerender.mov"), f.path("Main_prerender_1.mov"), f.path("mine.mov"), f.path("mine.mov")]); +} + +/// File ▸ Create Proxy queues a render with a default name of its own too. +#[test] +fn create_proxy_numbers_its_default_name_too() { + let mut f = Fixture::new("proxy-names"); + f.comp("Main"); + f.s.execute("file.createProxy", json!({"kind": "movie"})).unwrap(); + f.s.execute("file.createProxy", json!({"kind": "movie"})).unwrap(); + f.s.execute("file.createProxy", json!({"kind": "movie", "output": f.text("mine.mov")})).unwrap(); + f.s.execute("file.createProxy", json!({"kind": "movie", "output": f.text("mine.mov")})).unwrap(); + assert_eq!(f.outputs(), [f.path("Main_proxy.mov"), f.path("Main_proxy_1.mov"), f.path("mine.mov"), f.path("mine.mov")]); +} + +/// Save Frame As ▸ File… with `queue`: every file of a sequence has its frame number in its name, +/// so two frames of one composition share no file, and the same frame twice does. +#[test] +fn queued_frames_number_their_default_names_only_when_they_are_the_same_frame() { + let mut f = Fixture::new("frames"); + f.comp("Main"); + for time in [0.0, 0.2, 0.0] { + f.s.execute("comp.saveFrameAs", json!({"queue": true, "time": time})).unwrap(); + } + assert_eq!(f.outputs(), [f.path("Main_[#####].png"), f.path("Main_[#####].png"), f.path("Main_1_[#####].png")]); + assert!(f.clashes().is_empty()); + f.render(json!({})).unwrap(); + for file in ["Main_00000.png", "Main_00002.png", "Main_1_00000.png"] { + assert_eq!(f.read(file), RENDERED, "{file}"); + } +} + +/// The report: two queue items forced to one path (the same Output To) replaced each other's file +/// inside one job. The render is refused before anything is written, whatever `overwrite` says: +/// that flag is for files that exist, and this is the render replacing its own output. +#[test] +fn two_items_forced_to_one_file_are_reported_and_the_render_is_refused() { + let mut f = Fixture::new("one-file"); + f.comp("Main"); + let out = f.text("render.mp4"); + f.add(json!({"output": out})); + f.add(json!({"output": out})); + let found = f.check(); + let list = found["duplicates"].as_array().unwrap_or_else(|| panic!("checkOutputs lists no duplicates: {found}")); + assert_eq!(list.len(), 1, "{found}"); + let d = &list[0]; + let (first, second) = (f.s.project.render_queue[0].id, f.s.project.render_queue[1].id); + assert_eq!((d["item"].as_u64(), d["index"].as_u64(), d["comp"].as_str()), (Some(second), Some(2), Some("Main")), "{found}"); + assert_eq!((d["with"]["item"].as_u64(), d["with"]["index"].as_u64(), d["with"]["comp"].as_str()), (Some(first), Some(1), Some("Main")), "{found}"); + assert_eq!((d["files"].as_u64(), d["sequence"].as_bool()), (Some(1), Some(false)), "{found}"); + assert_eq!(Path::new(d["path"].as_str().unwrap_or_default()), f.path("render.mp4"), "{found}"); + let why = d["reason"].as_str().unwrap_or_default(); + assert!(why.contains("#1") && why.contains("#2") && why.contains("render.mp4") && why.contains("Output To"), "{found}"); + + let queue = f.s.project.render_queue.clone(); + let e = f.render(json!({})).unwrap_err().to_string(); + assert!(e.contains("#1") && e.contains("#2") && e.contains("render.mp4") && e.contains("Nothing was rendered"), "{e}"); + assert!(!f.path("render.mp4").exists()); + assert!(f.exported().is_empty(), "the exporter was not started"); + assert_eq!(f.s.project.render_queue, queue, "a refused render changes nothing in the queue"); + + // A file that exists as well does not change that: replacing it would not help. + f.write("render.mp4", EARLIER); + let found = f.check(); + assert_eq!(found["duplicates"].as_array().map(Vec::len), Some(1), "{found}"); + let e = f.render(json!({})).unwrap_err().to_string(); + assert!(e.contains("#2") && e.contains("Nothing was rendered"), "{e}"); + assert_eq!(f.read("render.mp4"), EARLIER); + + // Another Output To for one of them, and both render. + f.s.execute("renderQueue.setOutput", json!({"index": 2, "path": f.text("second.mp4")})).unwrap(); + assert!(f.clashes().is_empty()); + f.render(json!({})).unwrap(); + assert_eq!((f.read("render.mp4"), f.read("second.mp4")), (RENDERED.to_vec(), RENDERED.to_vec())); +} + +/// `renderQueue.duplicate` copies the item as it is, output included: it is reported until one of +/// the two gets another Output To (or is turned off). +#[test] +fn a_duplicated_item_is_a_copy_and_clashes_until_one_of_them_changes() { + let mut f = Fixture::new("duplicate"); + f.comp("Main"); + f.add(json!({})); + f.s.execute("renderQueue.duplicate", json!({"index": 1})).unwrap(); + assert_eq!(f.outputs(), [f.path("Main.mp4"), f.path("Main.mp4")], "a duplicate is a copy, its output included"); + assert_eq!(f.clashes(), [(2, 1, 1)]); + let e = f.render(json!({})).unwrap_err().to_string(); + assert!(e.contains("#1") && e.contains("#2") && e.contains("Main.mp4"), "{e}"); + assert!(f.exported().is_empty()); + // Turned off, the copy is not part of the job. + f.s.execute("renderQueue.setRender", json!({"index": 2, "render": false})).unwrap(); + assert!(f.clashes().is_empty()); + f.render(json!({})).unwrap(); + assert_eq!(f.read("Main.mp4"), RENDERED); + assert_eq!(f.exported().len(), 1); +} + +/// Only the items of the job are compared: an earlier job's file is a file that exists, and +/// replacing it is up to `overwrite`. +#[test] +fn an_item_that_is_not_in_the_job_does_not_clash() { + let mut f = Fixture::new("not-in-job"); + f.comp("Main"); + let out = f.text("render.mp4"); + f.add(json!({"output": out})); + f.add(json!({"output": out})); + f.s.execute("renderQueue.setRender", json!({"index": 2, "render": false})).unwrap(); + f.render(json!({})).unwrap(); // item 1 renders alone + f.s.execute("renderQueue.setRender", json!({"index": 2, "render": true})).unwrap(); + assert!(f.clashes().is_empty(), "item 1 is done and is not in this job"); + // item 2 now replaces the file item 1 wrote in the earlier job: an ordinary file, left to the exporter + f.render(json!({})).unwrap(); +} + +#[test] +fn another_spelling_of_one_file_is_one_file_where_the_volume_ignores_case() { + let mut f = Fixture::new("spellings"); + f.comp("Main"); + f.write("probe.txt", b"x"); + if !f.path("PROBE.TXT").exists() { + eprintln!("this volume tells the two names apart: skipped"); + return; + } + f.add(json!({"output": f.text("Render.mp4")})); + f.add(json!({"output": f.text("render.MP4")})); + f.add(json!({"output": f.dir.join("sub").join("..").join("RENDER.mp4").to_string_lossy()})); + f.add(json!({"output": f.text("other.mp4")})); + assert_eq!(f.clashes(), [(2, 1, 1), (3, 1, 1)]); +} + +/// A sequence writes one file per frame: two items of one pattern clash on the frames they share, +/// and not at all when their frames differ. +#[test] +fn sequences_clash_only_on_the_frames_they_both_write() { + let mut f = Fixture::new("sequence-halves"); + f.comp("Main"); + let pattern = f.text("seq_[#####].png"); + f.add(span(&pattern, 0.0, 0.2)); // frames 0-1 + f.add(span(&pattern, 0.2, 0.5)); // frames 2-4 + assert!(f.clashes().is_empty()); + f.render(json!({})).unwrap(); + for n in 0..5 { + assert_eq!(f.read(&format!("seq_{n:05}.png")), RENDERED, "frame {n}"); + } + + let mut f = Fixture::new("sequence-overlap"); + f.comp("Main"); + let pattern = f.text("seq_[#####].png"); + f.add(span(&pattern, 0.0, 0.2)); + f.add(span(&pattern, 0.2, 0.5)); + f.add(span(&pattern, 0.3, 0.5)); // frames 3-4 again + assert_eq!(f.clashes(), [(3, 2, 2)]); + let found = f.check(); + assert_eq!(found["duplicates"][0]["sequence"], true, "{found}"); + let e = f.render(json!({"overwrite": true})).unwrap_err().to_string(); + assert!(e.contains("#2") && e.contains("#3") && e.contains("2 frames") && e.contains("seq_[#####].png"), "{e}"); + assert!(!f.path("seq_00000.png").exists(), "the frames before the clash were not rendered either"); + assert!(f.exported().is_empty()); +} + +/// Skip Existing Files leaves a frame alone when it is there: the item that skips and comes second +/// writes only what the first did not. The item that skips and comes first is written over. +#[test] +fn a_later_item_that_skips_existing_frames_leaves_the_earlier_ones_alone() { + let mut f = Fixture::new("skip-later"); + f.comp("Main"); + let pattern = f.text("seq_[#####].png"); + f.add(json!({"format": "png", "output": pattern})); + f.add(json!({"format": "png", "output": pattern, "skipExisting": true})); + assert!(f.clashes().is_empty()); + f.render(json!({})).unwrap(); + assert_eq!(f.read("seq_00004.png"), RENDERED); + + let mut f = Fixture::new("skip-earlier"); + f.comp("Main"); + let pattern = f.text("seq_[#####].png"); + f.add(json!({"format": "png", "output": pattern, "skipExisting": true})); + f.add(json!({"format": "png", "output": pattern})); + assert_eq!(f.clashes(), [(2, 1, 5)]); +} + +#[test] +fn two_output_modules_of_one_item_cannot_share_a_file() { + let mut f = Fixture::new("two-modules"); + f.comp("Main"); + let out = f.text("render.mp4"); + f.add(json!({"output": out})); + f.s.execute("render.addOutputModule", json!({"output": out})).unwrap(); + assert_eq!(f.clashes(), [(1, 1, 1)]); + let e = f.render(json!({})).unwrap_err().to_string(); + assert!(e.contains("#1") && e.contains("twice"), "{e}"); + assert!(f.exported().is_empty()); +} + +/// Settings ▸ Export ▸ Segment Sequences puts each item's frames in numbered folders: default +/// names keep two items' folders apart, and items given one name clash segment by segment. +#[test] +fn segmented_sequences_clash_segment_by_segment() { + let mut f = Fixture::new("segments"); + f.comp("Main"); + f.s.prefs.export.segment_sequences = true; + f.s.prefs.export.segment_sequence_files = 2; // five frames: segments of 2, 2 and 1 + f.add(json!({"format": "png"})); + f.add(json!({"format": "png"})); + assert_eq!(f.output(1), f.path("Main_1_[#####].png")); + assert!(f.clashes().is_empty()); + f.render(json!({})).unwrap(); + assert_eq!(f.read("Main_001/Main_00000.png"), RENDERED); + assert_eq!(f.read("Main_1_003/Main_1_00004.png"), RENDERED); + + let mut f = Fixture::new("segments-named"); + f.comp("Main"); + f.s.prefs.export.segment_sequences = true; + f.s.prefs.export.segment_sequence_files = 2; + let pattern = f.text("seg_[#####].png"); + f.add(json!({"format": "png", "output": pattern})); + f.add(json!({"format": "png", "output": pattern})); + assert_eq!(f.clashes(), [(2, 1, 2), (2, 1, 2), (2, 1, 1)]); + let e = f.render(json!({})).unwrap_err().to_string(); + assert!(e.contains("2 more"), "the first clash is named, the rest counted: {e}"); + assert!(f.exported().is_empty()); +} + +/// An item that is refused for the file it would replace does not write anything, so it cannot +/// clash with another item. +#[test] +fn items_refused_for_footage_are_not_compared_with_each_other() { + let mut f = Fixture::new("refused-pair"); + f.footage("clip.mov"); + f.comp("Main"); + let clip = f.text("clip.mov"); + f.add(json!({"format": "prores", "output": clip})); + f.add(json!({"format": "prores", "output": clip})); + f.add(json!({"output": f.text("ok.mp4")})); + let found = f.check(); + assert_eq!(found["duplicates"].as_array().map(Vec::len), Some(0), "{found}"); + let r = f.render(json!({})).unwrap(); + let failed = r["items"].as_array().unwrap().iter().filter(|i| i["status"].get("Failed").is_some()).count(); + assert_eq!(failed, 2, "both items on the footage are refused: {r}"); + assert_eq!(f.read("clip.mov"), SOURCE); + assert_eq!(f.read("ok.mp4"), RENDERED); +} diff --git a/docs/agents.md b/docs/agents.md index d801fcec6..882fb13c3 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -319,6 +319,14 @@ build real projects through these interfaces; they are worked examples of everyt proresProfile: 4444, output}` then `renderQueue.render {"wait": true}`. Check a render by importing it (`file.import`, `file.newCompFromSelection`) and `render_frame {transparent: true}` for its alpha (ProRes 4444 and WebM VP9 alpha both read back as straight alpha). + **A render never writes one file twice either:** when two outputs of the job (two items, a + `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, and a later item with Skip Existing Files leaves the earlier one's frames alone. 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 + output as it is, so change one with `renderQueue.setOutput` or turn it off. * **Essential Graphics**: controls can be addressed by name (`essential.set {"layer":"#1", "control":"Title","value":"John Smith"}`); a command that targets an explicit `comp` runs even when the active comp would disable it (`essential.exportTemplate {"comp":"Lower Third", …}`). diff --git a/docs/mcp.md b/docs/mcp.md index 686adcb35..59d41552e 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -53,6 +53,15 @@ receive `resultType: complete` and list/read cache hints; document reads are nev {"jsonrpc":"2.0","id":2,"method":"resources/read","params":{"uri":"effectcraft://document"}} ``` +## Two outputs on one file + +Two outputs of one job that write the same file (two items with one Output To, a duplicated item, +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, and an item with Skip Existing Files leaves the +earlier one's frames alone. 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 Headless `command_run` / `execute_command` calls to `renderQueue.render` (unless `wait: false`)