From 9d33c200d5df7bcd6df808b4ddd12e9234d3521d Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Sat, 24 Oct 2026 23:00:46 +0000 Subject: [PATCH] #1743: protect render sources and refresh proxies safely --- .../workflows/render-source-protection.yml | 34 ++ Cargo.lock | 1 + Cargo.toml | 1 + crates/engine/Cargo.toml | 4 + crates/engine/src/commands/comp_more.rs | 7 +- crates/engine/src/commands/proxy.rs | 2 + crates/engine/src/commands/render_queue.rs | 9 + crates/engine/src/lib.rs | 3 + crates/engine/src/output_guard.rs | 435 +++++++++++++ crates/engine/src/render_queue.rs | 95 +-- crates/engine/src/tests_render_overwrite.rs | 571 ++++++++++++++++++ crates/project/src/render_queue.rs | 80 ++- docs/agents.md | 4 + docs/render-source-protection.md | 26 + 14 files changed, 1230 insertions(+), 42 deletions(-) create mode 100644 .github/workflows/render-source-protection.yml create mode 100644 crates/engine/src/output_guard.rs create mode 100644 crates/engine/src/tests_render_overwrite.rs create mode 100644 docs/render-source-protection.md diff --git a/.github/workflows/render-source-protection.yml b/.github/workflows/render-source-protection.yml new file mode 100644 index 000000000..3c040d831 --- /dev/null +++ b/.github/workflows/render-source-protection.yml @@ -0,0 +1,34 @@ +name: Render source protection + +on: + pull_request: + paths: + - .github/workflows/render-source-protection.yml + - Cargo.toml + - Cargo.lock + - crates/engine/** + - crates/project/src/render_queue.rs + workflow_dispatch: + +permissions: + contents: read + +jobs: + regression: + name: Source guard (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + timeout-minutes: 30 + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest] + env: + CARGO_PROFILE_DEV_DEBUG: 0 + CARGO_PROFILE_TEST_DEBUG: 0 + CARGO_INCREMENTAL: 0 + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@1.95.0 + - run: cargo test --locked -p effectcraft-engine --lib output_guard::tests + - run: cargo test --locked -p effectcraft-engine --lib tests_render_overwrite + - run: cargo test --locked -p effectcraft-project render_queue::tests diff --git a/Cargo.lock b/Cargo.lock index f99e09a06..2f9163471 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1285,6 +1285,7 @@ dependencies = [ "kurbo", "log", "rayon", + "same-file", "serde", "serde_json", "thiserror 2.0.21", diff --git a/Cargo.toml b/Cargo.toml index a3935f047..bf3ac938f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -72,6 +72,7 @@ skrifa = { version = "0.44", default-features = false, features = ["std"] } harfrust = { version = "0.12", default-features = false, features = ["std"] } exr = "1.74" image = { version = "0.25", default-features = false, features = ["png", "jpeg"] } +same-file = "1.0" # media codecs + containers: FilmCraft's pure-Rust crates, pinned to a revision (plan/adr/0001). # Only `crates/media`, `crates/export` and `crates/interchange` may depend on these. diff --git a/crates/engine/Cargo.toml b/crates/engine/Cargo.toml index 7104bc1cc..4bb488569 100644 --- a/crates/engine/Cargo.toml +++ b/crates/engine/Cargo.toml @@ -39,3 +39,7 @@ rayon = { workspace = true } image = { workspace = true } web-time = { workspace = true } exr = { workspace = true } + +# Native only: the volume's own identity for a file, to tell that a hard link is the footage it leads to. +[target.'cfg(not(target_arch = "wasm32"))'.dependencies] +same-file = { workspace = true } diff --git a/crates/engine/src/commands/comp_more.rs b/crates/engine/src/commands/comp_more.rs index d6de320ec..fca36e670 100644 --- a/crates/engine/src/commands/comp_more.rs +++ b/crates/engine/src/commands/comp_more.rs @@ -93,8 +93,11 @@ fn queue_frame(s: &mut Session, p: &Value) -> Result { it.output = s.project.render_templates.default_output_module(TemplateSlot::Still); if let Some(path) = str_p(p, "path") { it.output.output = path.to_string(); - } else if !it.output.format.is_sequence() { - it.output.output = "[compName]_frame.[fileExtension]".into(); + } else { + if !it.output.format.is_sequence() { + it.output.output = "[compName]_frame.[fileExtension]".into(); + } + s.avoid_sources(&mut it); } let id = s.edit("Save Frame As", None, |proj, _| { it.id = proj.render_queue.iter().map(|i| i.id).max().unwrap_or(0) + 1; diff --git a/crates/engine/src/commands/proxy.rs b/crates/engine/src/commands/proxy.rs index b7d644b94..dfa259618 100644 --- a/crates/engine/src/commands/proxy.rs +++ b/crates/engine/src/commands/proxy.rs @@ -190,6 +190,8 @@ fn create_proxy(s: &mut Session, p: &Value) -> Result { it.output.channels = Channels::Rgba; if let Some(o) = str_p(p, "path").or(str_p(p, "output")) { it.output.output = o.to_string(); + } else { + s.avoid_sources(&mut it); } it.post_render = PostRenderAction::SetProxy; let id = s.edit("Create Proxy", None, |proj, _| { diff --git a/crates/engine/src/commands/render_queue.rs b/crates/engine/src/commands/render_queue.rs index 09a7c7d2f..d7e914b1a 100644 --- a/crates/engine/src/commands/render_queue.rs +++ b/crates/engine/src/commands/render_queue.rs @@ -532,6 +532,9 @@ fn add(s: &mut Session, p: &Value) -> Result { object.remove("template"); } apply_output(&mut it.output, &templates, s.state.region_of_interest, &output_params, "renderQueue.add")?; + if str_p(p, "output").or(str_p(p, "path")).is_none() { + s.avoid_sources(&mut it); + } if let Some(l) = enum_p(p, "log", "renderQueue.add", RenderLog::parse, "errorsOnly|plusSettings|plusPerFrameInfo")? { it.log = l; } @@ -770,6 +773,9 @@ fn pre_render(s: &mut Session, p: &Value) -> Result { it.output = templates.default_output_module(TemplateSlot::PreRender); it.output.output = "[compName]_prerender.[fileExtension]".into(); apply_output(&mut it.output, &templates, s.state.region_of_interest, p, "render.preRender")?; + if str_p(p, "output").or(str_p(p, "path")).is_none() { + s.avoid_sources(&mut it); + } it.post_render = PostRenderAction::ImportAndReplace; let id = s.edit("Pre-render", None, |proj, _| { it.id = proj.render_queue.iter().map(|i| i.id).max().unwrap_or(0) + 1; @@ -795,6 +801,9 @@ fn save_current_preview(s: &mut Session, p: &Value) -> Result { let mut item = RenderQueueItem::new(0, cid); item.output = OutputModule::for_format(format); item.output.output = path.clone(); + if let Some(refusal) = s.check_outputs(&[vec![(item.clone(), path.clone())]]).first() { + return Err(EngineError::Other(refusal.reason.clone())); + } let job = crate::ExportJob { project: &s.project, footage: s.footage.as_ref(), diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 382534a44..3f703129b 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -30,6 +30,7 @@ pub mod media_cache; pub mod menus; pub mod models; pub mod offload; +mod output_guard; pub mod perf; pub mod prefs; pub mod preview; @@ -1333,6 +1334,8 @@ mod tests_proxy; #[cfg(test)] mod tests_quick_offset; #[cfg(test)] +mod tests_render_overwrite; +#[cfg(test)] mod tests_replace_source; #[cfg(test)] mod tests_rig3d; diff --git a/crates/engine/src/output_guard.rs b/crates/engine/src/output_guard.rs new file mode 100644 index 000000000..5d4dc289e --- /dev/null +++ b/crates/engine/src/output_guard.rs @@ -0,0 +1,435 @@ +//! Protect files the project reads from render outputs (#1743). +//! +//! This guard is independent of replacement consent and export staging: it never authorizes +//! overwriting ordinary outputs or opens a second confirmation flow. Those belong to the +//! 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. + +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::{Footage, ItemKind}; + +use crate::Session; + +/// Volumes on Windows and macOS ignore case unless they were set up not to. +const IGNORES_CASE: bool = cfg!(any(windows, target_os = "macos")); + +/// How many frames are looked at one by one for a sequence whose folder cannot be read at once +/// (its frame number is in a folder's name). +const PROBE_LIMIT: u64 = 100_000; + +/// How far a default output name is numbered (`_1` … `_1000`) before it is left to the check at +/// render time. +const MAX_NUMBER: u32 = 1_000; + +/// What reads a file the project depends on. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) enum Source { + /// A footage item, by name. + Footage(Arc), + /// An item's proxy, by the item's name. + Proxy(Arc), + /// The project file. + Project, +} + +impl Source { + /// Why `path`, a file this reads, cannot be a render's output. + pub(crate) fn refusal(&self, path: &str) -> String { + match self { + Source::Footage(name) => format!("{path} is the footage “{name}” of this project, and a render never replaces footage; choose another Output To"), + Source::Proxy(name) => format!("{path} is the proxy of “{name}”, and a render never replaces a proxy; choose another Output To"), + Source::Project => format!("{path} is this project's own file, and a render never replaces it; choose another Output To"), + } + } +} + +/// The files the project reads: footage (every frame of a sequence), proxies and the project file. +#[derive(Default)] +pub(crate) struct Protected { + files: Vec<(PathBuf, Source)>, + by_path: HashMap, + /// Hashes of native identities, built once. Handles are closed after indexing so long + /// sequences do not exhaust the process's file descriptors. Hash hits are verified with + /// native handle equality; a hash collision cannot make another file a protected source. + #[cfg(not(target_arch = "wasm32"))] + by_identity: std::sync::OnceLock>>, + #[cfg(all(test, not(target_arch = "wasm32")))] + identity_reads: std::sync::atomic::AtomicUsize, +} + +impl Protected { + pub(crate) fn of(s: &Session) -> Protected { + let mut found = Protected::default(); + for item in s.project.items.values() { + let name: Arc = item.name.as_str().into(); + if let ItemKind::Footage(f) = &item.kind { + found.add_footage(s, f, &Source::Footage(name.clone())); + } + if let Some(proxy) = &item.proxy { + found.add_footage(s, &proxy.footage, &Source::Proxy(name)); + } + } + if let Some(path) = s.path.as_deref().filter(|p| !p.is_empty()) { + found.add(PathBuf::from(path), Source::Project); + } + found + } + + fn add_footage(&mut self, s: &Session, f: &Footage, source: &Source) { + for stored in std::iter::once(&f.path).chain(&f.sequence).filter(|p| !p.is_empty()) { + // The file the media layer opens for the path the project stores. + let path = s.stored_path(stored).unwrap_or_else(|_| PathBuf::from(stored)); + self.add(path, source.clone()); + } + } + + fn add(&mut self, path: PathBuf, source: Source) { + self.by_path.entry(key(&path)).or_insert(self.files.len()); + self.files.push((path, source)); + } + + pub(crate) fn is_empty(&self) -> bool { + self.files.is_empty() + } + + /// The source at this normalized path or native file identity. A new output only needs + /// its path lookup and one failed filesystem probe; existing outputs build the index once. + pub(crate) fn find(&self, path: &Path) -> Option<&Source> { + self.by_path.get(&key(path)).and_then(|i| self.files.get(*i)).map(|(_, source)| source).or_else(|| self.alias_of(path)) + } + + #[cfg(not(target_arch = "wasm32"))] + fn handle(&self, path: &Path) -> Option { + #[cfg(test)] + self.identity_reads.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + // Opening a FIFO can wait for a writer. Only regular files can be protected media; + // preserve the old size-indexed lookup's exclusion of special files and directories. + if !std::fs::metadata(path).ok()?.is_file() { + return None; + } + same_file::Handle::from_path(path).ok() + } + + /// Hashes narrow the search, while native handle equality also checks any hash collisions. + #[cfg(not(target_arch = "wasm32"))] + fn alias_of(&self, path: &Path) -> Option<&Source> { + let output = self.handle(path)?; + let indexed = self.by_identity.get_or_init(|| { + let mut ids: HashMap> = HashMap::new(); + for (i, (known, _)) in self.files.iter().enumerate() { + if let Some(handle) = self.handle(known) { + ids.entry(identity_hash(&handle)).or_default().push(i); + } + } + ids + }); + indexed.get(&identity_hash(&output))?.iter().find_map(|i| { + let (known, source) = self.files.get(*i)?; + (self.handle(known)? == output).then_some(source) + }) + } + + #[cfg(target_arch = "wasm32")] + fn alias_of(&self, _: &Path) -> Option<&Source> { + None + } +} + +#[cfg(not(target_arch = "wasm32"))] +fn identity_hash(handle: &same_file::Handle) -> u64 { + use std::hash::{Hash, Hasher}; + let mut hash = std::collections::hash_map::DefaultHasher::new(); + handle.hash(&mut hash); + hash.finish() +} + +/// `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()); + let mut out = PathBuf::new(); + for part in absolute.components() { + match part { + Component::CurDir => {} + Component::ParentDir => match out.components().next_back() { + Some(Component::Normal(_)) => { + out.pop(); + } + Some(Component::RootDir | Component::Prefix(_)) => {} + _ => out.push(".."), + }, + other => out.push(other.as_os_str()), + } + } + out +} + +/// The name a path is compared by: absolute, resolved by name, without case where volumes ignore it. +fn key(path: &Path) -> String { + let parts: Vec = lexical(path).components().map(|c| c.as_os_str().to_string_lossy().into_owned()).collect(); + fold(&parts.join("/")) +} + +fn fold(name: &str) -> String { + if IGNORES_CASE { name.to_lowercase() } else { name.to_string() } +} + +#[cfg(test)] +fn same_file(a: &Path, b: &Path) -> bool { + if key(a) == key(b) { + return true; + } + #[cfg(not(target_arch = "wasm32"))] + return same_file::is_same_file(a, b).unwrap_or(false); + #[cfg(target_arch = "wasm32")] + false +} + +/// Whether a file (or a link, even one leading nowhere) is at `path`. +fn exists(path: &str) -> bool { + std::fs::symlink_metadata(path).is_ok_and(|m| !m.is_dir()) +} + +/// The files of a numbered sequence (`count` frames from `first`) that are on disk: its folder is +/// read once, not asked about frame by frame. +fn frames_on_disk(path: &str, first: i64, count: u64) -> Result, String> { + let last = first.saturating_add(i64::try_from(count).unwrap_or(i64::MAX)); + let p = Path::new(path); + let dir = p.parent().filter(|d| !d.as_os_str().is_empty()).unwrap_or(Path::new(".")); + let name = p.file_name().map(|n| n.to_string_lossy().into_owned()); + let Some(name) = name.filter(|_| !dir.to_string_lossy().contains('#')) else { + // The frame number is in a folder's name: every frame has a folder of its own. + if count > PROBE_LIMIT { + return Err(format!( + "cannot safely check {count} output frames with a number in a folder name (limit {PROBE_LIMIT}); choose a fixed output folder or a shorter range" + )); + } + return Ok((0..count) + .map(|k| sequence_path(path, first.saturating_add(i64::try_from(k).unwrap_or(i64::MAX)))) + .filter(|frame| exists(frame)) + .map(PathBuf::from) + .collect()); + }; + let pattern = fold(&name); + let entries = match std::fs::read_dir(dir) { + Ok(entries) => entries, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(Vec::new()), + Err(e) => return Err(format!("cannot check render output folder {}: {e}", dir.display())), + }; + Ok(entries + .filter_map(Result::ok) + .filter(|e| sequence_frame(&pattern, &fold(&e.file_name().to_string_lossy())).is_some_and(|frame| (first..last).contains(&frame))) + .filter(|e| !e.file_type().is_ok_and(|t| t.is_dir())) + .map(|e| e.path()) + .collect()) +} + +/// A queue item refused before any of its output modules are dispatched. +#[derive(Clone, Debug, PartialEq)] +pub(crate) struct Refused { + pub item: u64, + pub reason: String, +} + +impl Session { + /// The files the project reads. + pub(crate) fn protected_files(&self) -> Protected { + Protected::of(self) + } + + /// Reject protected outputs and ranges that cannot be checked completely. Ordinary-file + /// replacement consent is left to the exporter, independently of this source guard. + pub(crate) fn check_outputs(&self, items: &[Vec<(RenderQueueItem, String)>]) -> Vec { + if cfg!(target_arch = "wasm32") { + return Vec::new(); + } + let protected = self.protected_files(); + let mut seen = HashSet::new(); + let mut refused = Vec::new(); + for (item, path) in items.iter().flatten() { + if seen.contains(&item.id) { + continue; + } + let reason = match self.check_output(&protected, item, path) { + Ok(reason) => reason, + Err(reason) => Some(reason), + }; + if let Some(reason) = reason { + seen.insert(item.id); + refused.push(Refused { item: item.id, reason }); + } + } + refused + } + + fn check_output(&self, protected: &Protected, item: &RenderQueueItem, path: &str) -> Result, String> { + if protected.is_empty() { + return Ok(None); + } + let sequence = item.output.format.is_sequence(); + // Skip Existing Files leaves existing frames untouched, but not the render log. + let frames = if sequence && !item.settings.skip_existing { + match self.project.comp(item.comp) { + Some(comp) => frames_on_disk(path, item.settings.first_frame(comp), item.settings.frame_count(comp))?, + None => Vec::new(), + } + } else { + Vec::new() + }; + let log = log_path(path); + let movie = (!sequence).then_some(Path::new(path)); + Ok(frames + .iter() + .map(PathBuf::as_path) + .chain(movie) + .chain(Some(Path::new(&log))) + .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. + pub(crate) fn avoid_sources(&self, item: &mut RenderQueueItem) { + if cfg!(target_arch = "wasm32") { + return; + } + let protected = self.protected_files(); + if protected.is_empty() { + return; + } + let template = item.output.output.clone(); + let mut taken: Option> = None; + 'candidate: 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 }; + for (module, path) in &outputs { + match self.check_output(&protected, module, path) { + Ok(Some(_)) => continue 'candidate, + Err(_) => break 'candidate, + Ok(None) => {} + } + } + // 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))))) { + 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() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + #[cfg(not(target_arch = "wasm32"))] + fn unrelated_same_size_frames_are_indexed_once() { + let dir = std::env::temp_dir().join(format!("effectcraft-identity-index-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let mut protected = Protected::default(); + for i in 0..128 { + let source = dir.join(format!("source{i}")).join("plate.png"); + let output = dir.join(format!("output{i}")).join("plate.png"); + for path in [&source, &output] { + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, b"same-size frame").unwrap(); + } + protected.add(source, Source::Footage(format!("plate{i}").into())); + } + for i in 0..128 { + assert!(protected.find(&dir.join(format!("output{i}")).join("plate.png")).is_none()); + } + assert_eq!(protected.identity_reads.load(std::sync::atomic::Ordering::Relaxed), 256, "one source-index pass plus one lookup per output"); + let alias = dir.join("renamed.png"); + std::fs::hard_link(dir.join("source64").join("plate.png"), &alias).unwrap(); + assert_eq!(protected.find(&alias), Some(&Source::Footage("plate64".into()))); + assert_eq!(protected.identity_reads.load(std::sync::atomic::Ordering::Relaxed), 258, "native equality verifies a matching identity"); + std::fs::remove_dir_all(dir).unwrap(); + } + + #[test] + fn folder_numbered_sequences_are_never_partially_checked() { + let pattern = std::env::temp_dir().join("frames[######]").join("plate.png").to_string_lossy().into_owned(); + let error = frames_on_disk(&pattern, 0, PROBE_LIMIT + 1).unwrap_err(); + assert!(error.contains("cannot safely check") && error.contains("100000"), "{error}"); + assert!(frames_on_disk(&pattern, 0, u64::MAX).is_err()); + } + + #[test] + #[cfg(not(target_arch = "wasm32"))] + fn non_file_outputs_do_not_build_the_identity_index() { + let protected = Protected::default(); + assert!(protected.find(&std::env::temp_dir()).is_none()); + assert!(protected.by_identity.get().is_none()); + } + + #[test] + fn paths_compare_by_name_once_resolved() { + let here = std::env::current_dir().unwrap(); + assert_eq!(key(Path::new("a/b/../c.mp4")), key(Path::new("a/c.mp4"))); + assert_eq!(key(Path::new("./a/c.mp4")), key(&here.join("a/c.mp4"))); + assert_ne!(key(Path::new("a/c.mp4")), key(Path::new("b/c.mp4"))); + assert_eq!(key(Path::new(&format!("{}x", "../".repeat(64)))), key(Path::new(&format!("{}x", "../".repeat(80)))), "past the root stays at the root"); + assert_eq!(IGNORES_CASE, key(Path::new("A/C.MP4")) == key(Path::new("a/c.mp4"))); + } + + #[test] + fn a_file_is_the_same_file_under_every_spelling_the_volume_allows() { + let dir = std::env::temp_dir().join(format!("effectcraft-output-guard-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(dir.join("sub")).unwrap(); + std::fs::write(dir.join("clip.mov"), b"x").unwrap(); + std::fs::write(dir.join("other.mov"), b"y").unwrap(); + assert!(same_file(&dir.join("clip.mov"), &dir.join("sub").join("..").join("clip.mov"))); + assert!(!same_file(&dir.join("clip.mov"), &dir.join("other.mov"))); + // Nothing there yet: only the name decides. + assert!(same_file(&dir.join("gone.mov"), &dir.join("sub").join("..").join("gone.mov"))); + assert!(!same_file(&dir.join("gone.mov"), &dir.join("clip.mov"))); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn a_sequences_folder_is_read_once_for_its_frames() { + let dir = std::env::temp_dir().join(format!("effectcraft-output-frames-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(dir.join("a_00003.png")).unwrap(); // a folder is not a frame + for name in ["a_00001.png", "a_00002.png", "a_0004.png", "a_00004.jpg", "b_00002.png", "a_00009.png"] { + std::fs::write(dir.join(name), b"x").unwrap(); + } + let pattern = dir.join("a_[#####].png").to_string_lossy().into_owned(); + let names = |frames: Vec| { + let mut names: Vec = frames.iter().filter_map(|f| f.file_name()).map(|n| n.to_string_lossy().into_owned()).collect(); + names.sort(); + names + }; + assert_eq!( + names(frames_on_disk(&pattern, 0, 5).unwrap()), + ["a_00001.png", "a_00002.png"], + "frames 0-4: not the 4-digit one, the jpg, the folder or frame 9" + ); + assert_eq!(names(frames_on_disk(&pattern, 2, 1).unwrap()), ["a_00002.png"]); + assert_eq!(names(frames_on_disk(&pattern, 9, 100).unwrap()), ["a_00009.png"]); + assert_eq!(names(frames_on_disk(&pattern, 0, u64::MAX).unwrap()), ["a_00001.png", "a_00002.png", "a_00009.png"], "a hostile frame count is not walked"); + // A frame number in a folder's name: each frame has its own folder. + std::fs::create_dir_all(dir.join("f02")).unwrap(); + std::fs::write(dir.join("f02").join("x.png"), b"x").unwrap(); + let in_folder = dir.join("f[##]").join("x.png").to_string_lossy().into_owned(); + assert_eq!(names(frames_on_disk(&in_folder, 0, 4).unwrap()), ["x.png"]); + // A folder that is not there holds nothing. + assert!(frames_on_disk(&dir.join("none").join("a_[#####].png").to_string_lossy(), 0, 5).unwrap().is_empty()); + let _ = std::fs::remove_dir_all(&dir); + } +} diff --git a/crates/engine/src/render_queue.rs b/crates/engine/src/render_queue.rs index 74b333151..f36b500cc 100644 --- a/crates/engine/src/render_queue.rs +++ b/crates/engine/src/render_queue.rs @@ -432,10 +432,46 @@ impl Session { self.render_job.as_ref().map(|j| j.shared.snapshot()) } - /// Start rendering every queued item. `wait`: block until done (otherwise a background thread - /// renders, or the [`Session::offload`] worker; call [`Session::poll_render`] regularly). - /// Returns the ids being rendered. Without threads or an offload (wasm32) the render always - /// runs to completion before this returns. + /// The (item, resolved output path) of each output of `it`: one per output module and, with + /// Segment Sequences / Segment Movie Files, per segment. `formats`: the ones that can be + /// written (`None`: all). Fails with the status the item gets instead. + pub(crate) fn outputs_of(&self, it: &RenderQueueItem, formats: Option<&[OutputFormat]>) -> Result, RenderStatus> { + let mut modules = vec![]; + for om in it.output_modules() { + let mut m = it.clone(); + m.output = om.clone(); + match self.resolve_output(&m) { + Some(p) if formats.is_none_or(|f| f.contains(&om.format)) => { + // Settings ▸ Export: Append Bit Depth to File Name, Segment Sequences / + // Movie Files. + let bits = output_bits(om.format, self.project.settings.bit_depth); + let p = self.prefs.output_name(&p, bits); + modules.extend(self.segments(&m, &p)); + } + Some(_) => return Err(RenderStatus::Failed(format!("{} export is not available", om.format.label()))), + None if self.project.comp(it.comp).is_none() => return Err(RenderStatus::Failed("composition missing".into())), + None => return Err(RenderStatus::NeedsOutput), + } + } + Ok(modules) + } + + /// What the next Render would write: for each queued item its [`Session::outputs_of`], and + /// the status of each queued item that has none. + pub(crate) fn queued_outputs(&self, formats: &[OutputFormat]) -> (Vec>, Vec<(u64, RenderStatus)>) { + let (mut items, mut problems) = (Vec::new(), Vec::new()); + for it in self.project.render_queue.iter().filter(|i| i.is_queued()) { + match self.outputs_of(it, Some(formats)) { + Ok(modules) => items.push(modules), + Err(status) => problems.push((it.id, status)), + } + } + (items, problems) + } + + /// Start rendering every queued item, refusing outputs that replace footage, proxies or + /// the project file. Other existing outputs keep the exporter's usual replacement policy. + /// `wait`: block until done, otherwise render in the background and poll regularly. pub fn start_render(&mut self, wait: bool) -> Result, String> { let offload = self.offload.clone().filter(|_| !wait); let wait = wait || (cfg!(target_arch = "wasm32") && offload.is_none()); @@ -445,46 +481,33 @@ impl Session { self.poll_render(); let exporter = self.exporter.clone().ok_or("export is not available in this build")?; let formats = exporter.formats(); - let mut items = Vec::new(); - let mut needs_output = Vec::new(); - for it in self.project.render_queue.iter().filter(|i| i.is_queued()) { - let mut modules = vec![]; - let mut problem = None; - for om in it.output_modules() { - let mut m = it.clone(); - m.output = om.clone(); - match self.resolve_output(&m) { - Some(p) if formats.contains(&om.format) => { - // Settings ▸ Export: Append Bit Depth to File Name, Segment Sequences / - // Movie Files. - let bits = output_bits(om.format, self.project.settings.bit_depth); - let p = self.prefs.output_name(&p, bits); - modules.extend(self.segments(&m, &p)); - } - Some(_) => problem = Some(RenderStatus::Failed(format!("{} export is not available", om.format.label()))), - None if self.project.comp(it.comp).is_none() => problem = Some(RenderStatus::Failed("composition missing".into())), - None => problem = Some(RenderStatus::NeedsOutput), - } - if problem.is_some() { - break; - } - } - match problem { - Some(st) => needs_output.push((it.id, st)), - None => items.push(modules), - } - } - if !needs_output.is_empty() { + let (mut items, mut problems) = self.queued_outputs(&formats); + let refused = self.check_outputs(&items); + 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() { let p = Arc::make_mut(&mut self.project); - for (id, st) in needs_output { + for (id, st) in problems { if let Some(i) = p.render_queue.iter_mut().find(|i| i.id == id) { i.status = st; } } self.bump(); } + if let Some(first) = refused.first() { + let message = if refused.len() == 1 { + format!("Not rendered: {}", first.reason) + } else { + format!("{} items were not rendered; first: {}", refused.len(), first.reason) + }; + self.events.push(Event::Toast { message, error: true }); + } if items.is_empty() { - return Err("nothing is queued: add a composition (Composition ▸ Add to Render Queue) and tick Render".into()); + return Err(match refused.as_slice() { + [] => "nothing is queued: add a composition (Composition ▸ Add to Render Queue) and tick Render".into(), + [only] => only.reason.clone(), + [first, ..] => format!("{} items would replace files the project reads; first: {}", refused.len(), first.reason), + }); } let ids: Vec = items.iter().filter_map(|m| m.first().map(|(i, _)| i.id)).collect(); if let Some(off) = offload { diff --git a/crates/engine/src/tests_render_overwrite.rs b/crates/engine/src/tests_render_overwrite.rs new file mode 100644 index 000000000..a2473c6e8 --- /dev/null +++ b/crates/engine/src/tests_render_overwrite.rs @@ -0,0 +1,571 @@ +//! Rendering never replaces a file the project reads (#1743). Ordinary-file replacement +//! behavior stays with the exporter and its confirmation API. + +use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex}; + +use effectcraft_color::Label; +use effectcraft_project::render_queue::{OutputFormat, RenderStatus, sequence_path}; +use effectcraft_project::{Footage, FootageKind, ItemId, ItemKind}; +use effectcraft_time::Tick; +use serde_json::{Value, json}; + +use crate::{ExportJob, ExportResult, Exporter, Session}; + +const SOURCE: &[u8] = b"SOURCE FOOTAGE"; +const EARLIER: &[u8] = b"AN EARLIER RENDER"; +const RENDERED: &[u8] = b"RENDERED"; + +/// An exporter that writes like the real one: `RENDERED` at the job's path, or at the path of +/// every frame for a sequence (leaving the frames that exist alone when the item skips them). +#[derive(Default)] +struct Writer { + jobs: Mutex>, +} + +impl Exporter for Writer { + fn formats(&self) -> Vec { + OutputFormat::ALL.to_vec() + } + fn export(&self, job: &ExportJob, _: &mut dyn FnMut(u64, u64) -> bool) -> Result { + self.jobs.lock().unwrap().push(job.path.to_string()); + let comp = job.project.comp(job.item.comp).ok_or("no comp")?; + let paths: Vec = if job.item.output.format.is_sequence() { + let first = job.item.settings.first_frame(comp); + (0..job.item.settings.frame_count(comp)).map(|k| sequence_path(job.path, first + k as i64)).collect() + } else { + vec![job.path.to_string()] + }; + for path in &paths { + if job.item.settings.skip_existing && Path::new(path).exists() { + continue; + } + if let Some(dir) = Path::new(path).parent() { + std::fs::create_dir_all(dir).map_err(|e| e.to_string())?; + } + std::fs::write(path, RENDERED).map_err(|e| e.to_string())?; + } + Ok(ExportResult { path: paths.first().cloned().unwrap_or_default(), ..Default::default() }) + } +} + +/// A session whose project is saved in a fresh folder (the footage goes there too), with an +/// exporter that writes real files. +struct Fixture { + s: Session, + writer: Arc, + dir: PathBuf, +} + +impl Fixture { + fn new(name: &str) -> Fixture { + let dir = std::env::temp_dir().join("effectcraft-render-overwrite").join(format!("{}-{name}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let mut s = Session::default(); + let writer = Arc::new(Writer::default()); + s.exporter = Some(writer.clone()); + s.execute("file.saveAs", json!({"path": dir.join("Project.ecproj").to_string_lossy()})).unwrap(); + Fixture { s, writer, dir } + } + + fn path(&self, file: &str) -> PathBuf { + self.dir.join(file) + } + + fn text(&self, file: &str) -> String { + self.path(file).to_string_lossy().into_owned() + } + + fn read(&self, file: &str) -> Vec { + std::fs::read(self.path(file)).unwrap() + } + + fn write(&self, file: &str, bytes: &[u8]) { + std::fs::write(self.path(file), bytes).unwrap(); + } + + /// A video file in the project's folder, imported. + fn footage(&mut self, file: &str) -> ItemId { + self.write(file, SOURCE); + let f = Footage { + path: self.text(file), + kind: FootageKind::Video, + width: 64, + height: 36, + has_video: true, + duration: Tick::from_seconds_f64(1.0), + ..Default::default() + }; + Arc::make_mut(&mut self.s.project).add_item(file, Label::None, None, ItemKind::Footage(f)) + } + + /// Numbered stills in the project's folder, imported as one image sequence. + fn sequence(&mut self, name: &str, frames: std::ops::Range) -> ItemId { + let files: Vec = frames + .map(|n| { + let file = format!("{name}_{n:05}.png"); + self.write(&file, SOURCE); + self.text(&file) + }) + .collect(); + let f = Footage { + path: files[0].clone(), + kind: FootageKind::Sequence, + sequence: files, + width: 16, + height: 16, + has_video: true, + duration: Tick::from_seconds_f64(0.5), + ..Default::default() + }; + Arc::make_mut(&mut self.s.project).add_item(&format!("{name}_[00000-00004].png"), Label::None, None, ItemKind::Footage(f)) + } + + /// A five-frame composition. + fn comp(&mut self, name: &str) { + self.s.execute("comp.new", json!({"name": name, "width": 16, "height": 16, "frameRate": 10, "duration": 0.5})).unwrap(); + } + + /// The output path of queue item `index` (0-based). + fn output(&self, index: usize) -> PathBuf { + PathBuf::from(self.s.resolve_output(&self.s.project.render_queue[index]).unwrap()) + } + + /// The paths the exporter was started for. + fn exported(&self) -> Vec { + self.writer.jobs.lock().unwrap().clone() + } + + fn render(&mut self, params: Value) -> crate::Result { + self.s.execute("renderQueue.render", params) + } +} + +impl Drop for Fixture { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.dir); + } +} + +fn link(target: &Path, link: &Path) -> std::io::Result<()> { + #[cfg(unix)] + return std::os::unix::fs::symlink(target, link); + #[cfg(windows)] + return std::os::windows::fs::symlink_file(target, link); + #[cfg(not(any(unix, windows)))] + { + let _ = (target, link); + Err(std::io::ErrorKind::Unsupported.into()) + } +} + +/// A folder that leads to `target`: a symbolic link, or on Windows, where that takes a privilege +/// most accounts lack, a junction. +fn link_dir(target: &Path, link: &Path) -> std::io::Result<()> { + #[cfg(unix)] + return std::os::unix::fs::symlink(target, link); + #[cfg(windows)] + return std::os::windows::fs::symlink_dir(target, link).or_else(|_| { + let made = std::process::Command::new("cmd").args(["/C", "mklink", "/J"]).arg(link).arg(target).output()?; + if made.status.success() { Ok(()) } else { Err(std::io::Error::other(String::from_utf8_lossy(&made.stdout).into_owned())) } + }); + #[cfg(not(any(unix, windows)))] + { + let _ = (target, link); + Err(std::io::ErrorKind::Unsupported.into()) + } +} + +/// The report: save a new project beside `footage.mp4`, import it, make a composition from it +/// (it is named after the file), add that to the Render Queue and click Render. The composition's +/// default output is `/footage.mp4`, the footage itself. +#[test] +fn a_comp_made_from_footage_does_not_render_over_it() { + let mut f = Fixture::new("comp-from-footage"); + let footage = f.footage("footage.mp4"); + f.s.state.project_selection = vec![footage]; + f.s.execute("file.newCompFromSelection", json!({"addToRenderQueue": true})).unwrap(); + assert_eq!(f.s.project.render_queue.len(), 1); + f.render(json!({})).unwrap(); + assert_eq!(f.read("footage.mp4"), SOURCE, "the footage is still the footage"); + assert_eq!(f.s.project.render_queue[0].status, RenderStatus::Done); + // The default name stepped aside: the render sits next to it. + assert_eq!(f.output(0), f.path("footage_1.mp4")); + assert_eq!(f.read("footage_1.mp4"), RENDERED); + // Another item for the same composition takes the next free number. + f.s.execute("renderQueue.add", json!({})).unwrap(); + assert_eq!(f.output(1), f.path("footage_2.mp4")); +} + +#[test] +fn an_output_that_is_the_footage_is_refused_with_or_without_overwrite() { + for (n, params) in [json!({}), json!({"overwrite": true})].into_iter().enumerate() { + let mut f = Fixture::new(&format!("explicit-footage-{n}")); + f.footage("clip.mov"); + f.comp("Main"); + let clip = f.text("clip.mov"); + f.s.execute("renderQueue.add", json!({"format": "prores", "output": clip})).unwrap(); + let e = f.render(params).unwrap_err().to_string(); + assert!(e.contains("clip.mov") && e.contains("footage"), "{e}"); + assert_eq!(f.read("clip.mov"), SOURCE); + assert!(f.exported().is_empty(), "the exporter was not started"); + let RenderStatus::Failed(why) = &f.s.project.render_queue[0].status else { panic!("{:?}", f.s.project.render_queue[0].status) }; + assert!(why.contains("clip.mov"), "{why}"); + } +} + +/// An item that would replace the footage fails on its own; the rest of the queue renders. +#[test] +fn the_rest_of_the_queue_renders_when_one_item_would_replace_footage() { + let mut f = Fixture::new("one-refused"); + f.footage("clip.mov"); + f.comp("Main"); + let clip = f.text("clip.mov"); + f.s.execute("renderQueue.add", json!({"format": "prores", "output": clip})).unwrap(); + let ok = f.text("main.mp4"); + f.s.execute("renderQueue.add", json!({"output": ok})).unwrap(); + let r = f.render(json!({})).unwrap(); + assert_eq!(f.read("clip.mov"), SOURCE); + assert_eq!(f.read("main.mp4"), RENDERED); + assert!(matches!(f.s.project.render_queue[0].status, RenderStatus::Failed(_))); + assert_eq!(f.s.project.render_queue[1].status, RenderStatus::Done); + assert!(r["items"].as_array().unwrap().iter().any(|i| i["status"] == "Done"), "{r}"); +} + +#[test] +fn the_project_file_is_never_an_output() { + let mut f = Fixture::new("project-file"); + // A project saved under a name a movie could have. + let name = f.text("final.mp4"); + f.s.execute("file.saveAs", json!({"path": name})).unwrap(); + f.comp("final"); + f.s.execute("renderQueue.add", json!({})).unwrap(); + assert_eq!(f.output(0), f.path("final_1.mp4"), "the default name moves aside"); + f.s.execute("renderQueue.remove", json!({"index": 1})).unwrap(); + f.s.execute("renderQueue.add", json!({"output": name})).unwrap(); + let e = f.render(json!({"overwrite": true})).unwrap_err().to_string(); + assert!(e.contains("final.mp4") && e.contains("project"), "{e}"); + assert!(std::fs::read_to_string(f.path("final.mp4")).unwrap().contains("\"schema\""), "the project file is still a project"); +} + +#[test] +fn a_proxy_is_not_an_output_either() { + let mut f = Fixture::new("proxy"); + f.footage("clip.mov"); + f.write("proxy.mov", SOURCE); + f.comp("Main"); + let proxy = Footage { path: f.text("proxy.mov"), kind: FootageKind::Video, has_video: true, ..Default::default() }; + let comp = f.s.active_comp_id().unwrap(); + Arc::make_mut(&mut f.s.project).item_mut(comp).unwrap().proxy = Some(Box::new(effectcraft_project::Proxy { footage: proxy, enabled: true })); + let target = f.text("proxy.mov"); + f.s.execute("renderQueue.add", json!({"format": "prores", "output": target})).unwrap(); + let e = f.render(json!({"overwrite": true})).unwrap_err().to_string(); + assert!(e.contains("proxy"), "{e}"); + assert_eq!(f.read("proxy.mov"), SOURCE); +} + +/// Recreating a default proxy writes a new file and attaches it, without replacing the proxy +/// the project is currently reading. Explicitly choosing that protected path is still refused. +#[test] +fn creating_a_proxy_again_chooses_a_new_default_and_attaches_it() { + struct Probe; + impl crate::Importer for Probe { + fn probe(&self, path: &str) -> Result { + if !Path::new(path).is_file() { + return Err("proxy file missing".into()); + } + Ok(Footage { + path: path.into(), + kind: if path.ends_with(".png") { FootageKind::Still } else { FootageKind::Video }, + width: 16, + height: 16, + has_video: true, + ..Default::default() + }) + } + } + for kind in ["movie", "still"] { + let mut f = Fixture::new(&format!("recreate-proxy-{kind}")); + f.s.importer = Some(Arc::new(Probe)); + f.comp("Main"); + let comp = f.s.active_comp_id().unwrap(); + f.s.execute("file.createProxy", json!({"kind": kind})).unwrap(); + f.render(json!({})).unwrap(); + let first = f.s.project.item(comp).unwrap().proxy.as_ref().unwrap().footage.path.clone(); + assert_eq!(std::fs::read(&first).unwrap(), RENDERED); + + f.s.execute("file.createProxy", json!({"kind": kind})).unwrap(); + f.render(json!({})).unwrap(); + let second = f.s.project.item(comp).unwrap().proxy.as_ref().unwrap().footage.path.clone(); + assert_ne!(second, first, "the replacement proxy gets its own file"); + assert_eq!(std::fs::read(&second).unwrap(), RENDERED); + assert_eq!(std::fs::read(&first).unwrap(), RENDERED); + assert_eq!(f.exported().len(), 2); + + // For a still proxy, an explicit output is the sequence pattern, not its first + // rendered frame (a bare PNG name would have another frame number appended). + let protected_output = f.output(1); + f.s.execute("file.createProxy", json!({"kind": kind, "path": protected_output})).unwrap(); + let error = f.render(json!({})).unwrap_err().to_string(); + assert!(error.contains("never replaces a proxy"), "{error}"); + assert_eq!(f.exported().len(), 2); + } +} + +/// Exercise the engine's import and project serialization paths, rather than injecting a +/// Footage item directly. The importer reads dimensions from a real, generated PNG. +#[test] +fn imported_footage_stays_protected_after_the_project_is_reopened() { + struct PngProbe; + impl crate::Importer for PngProbe { + fn probe(&self, path: &str) -> Result { + let (width, height) = image::image_dimensions(path).map_err(|e| e.to_string())?; + Ok(Footage { path: path.into(), kind: FootageKind::Still, width, height, has_video: true, ..Default::default() }) + } + } + let mut f = Fixture::new("import-reopen"); + f.s.importer = Some(Arc::new(PngProbe)); + let path = f.path("plate_00002.png"); + image::RgbaImage::from_pixel(4, 4, image::Rgba([40, 80, 120, 255])).save(&path).unwrap(); + let original = std::fs::read(&path).unwrap(); + let result = f.s.execute("file.import", json!({"paths": [path]})).unwrap(); + assert_eq!(result["items"].as_array().unwrap().len(), 1); + f.comp("Main"); + f.s.execute("renderQueue.add", json!({"format": "png", "output": f.text("plate_[#####].png")})).unwrap(); + f.s.execute("file.save", json!({})).unwrap(); + f.s.execute("file.open", json!({"path": f.text("Project.ecproj")})).unwrap(); + let error = f.render(json!({})).unwrap_err().to_string(); + assert!(error.contains("plate_00002.png") && error.contains("footage"), "{error}"); + assert_eq!(std::fs::read(&path).unwrap(), original); + assert!(f.exported().is_empty()); +} + +#[test] +fn a_protected_frame_past_the_probe_limit_is_not_rendered() { + struct LastFrame(PathBuf); + impl Exporter for LastFrame { + fn formats(&self) -> Vec { + vec![OutputFormat::PngSequence] + } + fn export(&self, _: &ExportJob, _: &mut dyn FnMut(u64, u64) -> bool) -> Result { + // A synthetic exporter writes only the vulnerable late frame, so a broken guard + // can be reproduced without creating 100,001 output directories. + std::fs::write(&self.0, RENDERED).map_err(|e| e.to_string())?; + Ok(ExportResult { path: self.0.to_string_lossy().into_owned(), ..Default::default() }) + } + } + let mut f = Fixture::new("late-frame"); + std::fs::create_dir_all(f.path("f100000")).unwrap(); + f.footage("f100000/plate.png"); + f.comp("Main"); + f.s.execute("comp.settings", json!({"duration": 10000.1})).unwrap(); + f.s.exporter = Some(Arc::new(LastFrame(f.path("f100000/plate.png")))); + f.s.execute("renderQueue.add", json!({"format": "png", "output": f.text("f[######]/plate.png")})).unwrap(); + let error = f.render(json!({})).unwrap_err().to_string(); + assert!(error.contains("cannot safely check"), "{error}"); + assert_eq!(f.read("f100000/plate.png"), SOURCE); +} + +/// A numbered sequence is footage frame by frame: one colliding frame is enough, and nothing of +/// the item is rendered. +#[test] +fn a_sequence_never_replaces_one_of_its_footage_frames() { + let mut f = Fixture::new("sequence"); + f.sequence("shot", 0..5); + f.comp("shot"); + // The default name steps aside as a whole: shot_1_00000.png … + f.s.execute("renderQueue.add", json!({"format": "png"})).unwrap(); + assert_eq!(f.output(0), f.path("shot_1_[#####].png")); + f.render(json!({})).unwrap(); + for n in 0..5 { + assert_eq!(f.read(&format!("shot_{n:05}.png")), SOURCE, "frame {n}"); + assert_eq!(f.read(&format!("shot_1_{n:05}.png")), RENDERED, "frame {n}"); + } + + // Named by hand, one frame of five is a footage file. + let mut f = Fixture::new("sequence-one-frame"); + f.write("shot_00002.png", SOURCE); + let still = Footage { path: f.text("shot_00002.png"), kind: FootageKind::Still, width: 16, height: 16, has_video: true, ..Default::default() }; + Arc::make_mut(&mut f.s.project).add_item("shot_00002.png", Label::None, None, ItemKind::Footage(still)); + f.comp("shot"); + let pattern = f.text("shot_[#####].png"); + f.s.execute("renderQueue.add", json!({"format": "png", "output": pattern})).unwrap(); + let e = f.render(json!({"overwrite": true})).unwrap_err().to_string(); + assert!(e.contains("shot_00002.png"), "{e}"); + assert_eq!(f.read("shot_00002.png"), SOURCE); + assert!(!f.path("shot_00000.png").exists(), "the frames before it were not rendered either"); + assert!(f.exported().is_empty()); +} + +#[test] +fn ordinary_existing_outputs_keep_the_exporters_replacement_policy() { + let mut f = Fixture::new("earlier-render"); + f.comp("Main"); + f.s.execute("renderQueue.add", json!({"output": f.text("render.mp4")})).unwrap(); + f.write("render.mp4", EARLIER); + f.render(json!({})).unwrap(); + assert_eq!(f.read("render.mp4"), RENDERED); + assert_eq!(f.s.project.render_queue[0].status, RenderStatus::Done); +} + +#[test] +fn a_render_with_nothing_in_the_way_needs_no_flag() { + let mut f = Fixture::new("free"); + f.comp("Main"); + f.s.execute("renderQueue.add", json!({})).unwrap(); + assert_eq!(f.output(0), f.path("Main.mp4"), "no footage in the way: the comp's own name"); + f.render(json!({})).unwrap(); + assert_eq!(f.read("Main.mp4"), RENDERED); +} + +/// Skip Existing Files leaves the frames that exist alone, so they need no answer. +#[test] +fn a_sequence_that_skips_existing_frames_needs_no_flag() { + let mut f = Fixture::new("skip-existing"); + f.comp("Main"); + let pattern = f.text("render_[#####].png"); + f.s.execute("renderQueue.add", json!({"format": "png", "output": pattern, "skipExisting": true})).unwrap(); + for n in 0..3 { + f.write(&format!("render_{n:05}.png"), EARLIER); + } + f.render(json!({})).unwrap(); + assert_eq!(f.read("render_00002.png"), EARLIER); + assert_eq!(f.read("render_00003.png"), RENDERED); + // Ordinary files still follow the exporter's replacement policy when skipping is off. + f.s.execute("renderQueue.setRenderSettings", json!({"index": 1, "skipExisting": false})).unwrap(); + f.render(json!({})).unwrap(); + assert_eq!(f.read("render_00002.png"), RENDERED); +} + +/// A link is another name for the file it points to. +#[test] +fn a_link_to_the_footage_is_the_footage() { + let mut f = Fixture::new("link"); + f.footage("clip.mov"); + f.comp("Main"); + let alias = f.path("render.mov"); + if link(&f.path("clip.mov"), &alias).is_err() { + eprintln!("this account cannot make symbolic links: skipped"); + return; + } + f.s.execute("renderQueue.add", json!({"format": "prores", "output": f.text("render.mov")})).unwrap(); + let e = f.render(json!({"overwrite": true})).unwrap_err().to_string(); + assert!(e.contains("render.mov") && e.contains("footage"), "{e}"); + assert_eq!(f.read("clip.mov"), SOURCE); + assert!(std::fs::symlink_metadata(&alias).unwrap().file_type().is_symlink(), "the link was not replaced either"); +} + +/// A hard link to the footage under another name is the same file on disk: replacing it would truncate the footage. +#[test] +fn a_hard_link_to_the_footage_is_the_footage() { + let mut f = Fixture::new("hardlink"); + f.footage("clip.mov"); + f.comp("Main"); + if std::fs::hard_link(f.path("clip.mov"), f.path("render.mov")).is_err() { + eprintln!("this volume cannot make hard links: skipped"); + return; + } + f.s.execute("renderQueue.add", json!({"format": "prores", "output": f.text("render.mov")})).unwrap(); + // not even with overwrite + let e = f.render(json!({"overwrite": true})).unwrap_err().to_string(); + assert!(e.contains("render.mov") && e.contains("footage"), "{e}"); + assert_eq!(f.read("clip.mov"), SOURCE); + assert_eq!(f.read("render.mov"), SOURCE, "the other name still leads to the footage"); + assert!(f.exported().is_empty(), "nothing was rendered"); +} + +/// A copy of the footage has its size and bytes, but it is another file: an earlier render may replace it. +#[test] +fn a_copy_of_the_footage_is_not_the_footage() { + let mut f = Fixture::new("copy"); + f.footage("clip.mov"); + f.comp("Main"); + std::fs::copy(f.path("clip.mov"), f.path("copy.mov")).unwrap(); + f.s.execute("renderQueue.add", json!({"format": "prores", "output": f.text("copy.mov")})).unwrap(); + f.render(json!({})).unwrap(); + assert_eq!(f.read("copy.mov"), RENDERED); + assert_eq!(f.read("clip.mov"), SOURCE, "the footage is as it was"); +} + +/// Footage imported through a linked folder is the same file as the one in the real folder. +#[test] +fn footage_reached_through_a_linked_folder_is_still_footage() { + let mut f = Fixture::new("linked-folder"); + std::fs::create_dir_all(f.path("real")).unwrap(); + std::fs::write(f.path("real").join("clip.mov"), SOURCE).unwrap(); + if link_dir(&f.path("real"), &f.path("linked")).is_err() { + eprintln!("this account cannot link folders: skipped"); + return; + } + let via_link = + Footage { path: f.path("linked").join("clip.mov").to_string_lossy().into_owned(), kind: FootageKind::Video, has_video: true, ..Default::default() }; + Arc::make_mut(&mut f.s.project).add_item("clip.mov", Label::None, None, ItemKind::Footage(via_link)); + f.comp("Main"); + f.s.execute("renderQueue.add", json!({"format": "prores", "output": f.path("real").join("clip.mov").to_string_lossy()})).unwrap(); + let e = f.render(json!({"overwrite": true})).unwrap_err().to_string(); + assert!(e.contains("footage"), "{e}"); + assert_eq!(std::fs::read(f.path("real").join("clip.mov")).unwrap(), SOURCE); +} + +/// Where the volume ignores case, `CLIP.MOV` is `clip.mov`. +#[test] +fn another_spelling_of_the_footage_name_is_the_footage_where_the_volume_ignores_case() { + let mut f = Fixture::new("case"); + f.footage("clip.mov"); + f.comp("Main"); + if !f.path("CLIP.MOV").exists() { + eprintln!("this volume tells the two names apart: skipped"); + return; + } + let shouting = f.text("CLIP.MOV"); + f.s.execute("renderQueue.add", json!({"format": "prores", "output": shouting})).unwrap(); + let e = f.render(json!({"overwrite": true})).unwrap_err().to_string(); + assert!(e.contains("footage"), "{e}"); + assert_eq!(f.read("clip.mov"), SOURCE); +} + +#[test] +fn save_current_preview_never_replaces_footage() { + let mut f = Fixture::new("preview"); + f.footage("clip.mov"); + f.comp("Main"); + let clip = f.text("clip.mov"); + let e = f.s.execute("render.saveCurrentPreview", json!({"path": clip})).unwrap_err().to_string(); + assert!(e.contains("clip.mov") && e.contains("footage"), "{e}"); + assert_eq!(f.read("clip.mov"), SOURCE); + assert!(f.exported().is_empty()); + // Anywhere else it renders. + let elsewhere = f.text("preview.mp4"); + f.s.execute("render.saveCurrentPreview", json!({"path": elsewhere})).unwrap(); + assert_eq!(f.read("preview.mp4"), RENDERED); +} + +#[test] +fn save_current_preview_checks_sequence_frames_as_well_as_the_pattern() { + let mut f = Fixture::new("preview-sequence"); + f.sequence("plate", 0..5); + f.comp("Main"); + let error = f.s.execute("render.saveCurrentPreview", json!({"path": f.text("plate_[#####].png")})).unwrap_err().to_string(); + assert!(error.contains("plate_000") && error.contains("footage"), "{error}"); + for frame in 0..5 { + assert_eq!(f.read(&format!("plate_{frame:05}.png")), SOURCE); + } + assert!(f.exported().is_empty()); +} + +/// A watched folder renders unattended; delete the status file to render a project again. +#[test] +fn a_watched_folder_still_renders_a_project_again_over_its_outputs() { + let mut f = Fixture::new("watch"); + f.comp("Main"); + let out = f.text("Main.mp4"); + f.s.execute("renderQueue.add", json!({"output": out})).unwrap(); + f.s.execute("file.save", json!({})).unwrap(); + f.write("Main.mp4", EARLIER); + let folder = f.dir.to_string_lossy().into_owned(); + let done = crate::commands::watch_folder::poll(&mut f.s, &folder).unwrap(); + assert_eq!(done.len(), 1, "{done:?}"); + assert_eq!(done[0]["state"], "done", "{:?}", done[0]); + assert_eq!(f.read("Main.mp4"), RENDERED); +} diff --git a/crates/project/src/render_queue.rs b/crates/project/src/render_queue.rs index 2d3ab741b..c1cda7087 100644 --- a/crates/project/src/render_queue.rs +++ b/crates/project/src/render_queue.rs @@ -1455,22 +1455,56 @@ pub fn expand_template(template: &str, format: OutputFormat, v: &TemplateVars) - /// Replace the first run of `#` (optionally in brackets, `[####]`) with the zero-padded frame /// number. Paths without a run get `_NNNNN` before the extension. pub fn sequence_path(path: &str, frame: i64) -> String { + let (before, width, after) = sequence_parts(path); + format!("{before}{frame:0width$}{after}") +} + +/// A sequence output path around its frame number: what comes before it, how many digits it is +/// padded to and what comes after (`out/a_[#####].png` → `out/a_`, 5, `.png`). +pub fn sequence_parts(path: &str) -> (String, usize, String) { if let Some(start) = path.find('#') { let len = path[start..].chars().take_while(|c| *c == '#').count(); let (mut a, mut b) = (start, start + len); - if a > 0 && path.as_bytes()[a - 1] == b'[' && path.as_bytes().get(b) == Some(&b']') { + if a > 0 && path.as_bytes().get(a - 1) == Some(&b'[') && path.as_bytes().get(b) == Some(&b']') { a -= 1; b += 1; } - return format!("{}{:0width$}{}", &path[..a], frame, &path[b..], width = len); + return (path[..a].to_string(), len, path[b..].to_string()); } let p = std::path::Path::new(path); match p.extension() { - Some(ext) => format!("{}_{frame:05}.{}", p.with_extension("").to_string_lossy(), ext.to_string_lossy()), - None => format!("{path}_{frame:05}"), + Some(ext) => (format!("{}_", p.with_extension("").to_string_lossy()), 5, format!(".{}", ext.to_string_lossy())), + None => (format!("{path}_"), 5, String::new()), } } +/// The frame number `candidate` is the file of, when `path` (a sequence output path) writes it: +/// the inverse of [`sequence_path`]. +pub fn sequence_frame(path: &str, candidate: &str) -> Option { + let (before, width, after) = sequence_parts(path); + let digits = candidate.strip_prefix(before.as_str())?.strip_suffix(after.as_str())?; + let frame: i64 = digits.parse().ok()?; + (format!("{frame:0width$}") == digits).then_some(frame) +} + +/// `template` with `_n` added to the name: before the frame-number run of a sequence, else before +/// the extension (`[compName].[fileExtension]` → `[compName]_1.[fileExtension]`, +/// `[compName]_[#####].[fileExtension]` → `[compName]_1_[#####].[fileExtension]`). +pub fn numbered_template(template: &str, n: u32) -> String { + let at = match template.find('#') { + Some(hash) => { + // The run's `[`, then the separator that sets it off from the name. + let run = if template[..hash].ends_with('[') { hash - 1 } else { hash }; + if template[..run].ends_with(['_', '-', '.', ' ']) { run - 1 } else { run } + } + None => { + let name = template.rfind(['/', '\\']).map_or(0, |i| i + 1); + template[name..].rfind('.').map_or(template.len(), |i| name + i) + } + }; + format!("{}_{n}{}", &template[..at], &template[at..]) +} + /// Make a name safe as a file name component. pub fn sanitize(name: &str) -> String { name.chars().map(|c| if matches!(c, '/' | '\\' | ':' | '*' | '?' | '"' | '<' | '>' | '|') { '_' } else { c }).collect() @@ -1490,6 +1524,44 @@ mod tests { assert_eq!(sequence_path("a.png", 3), "a_00003.png"); } + /// `sequence_frame` reads back what `sequence_path` writes, and nothing else. + #[test] + fn sequence_frames_read_back() { + for path in ["out/a_[#####].png", "a###.tif", "a.png", "/x/[compName]/shot_[##].exr", "noext", "dir.v2/clip"] { + for frame in [0, 7, 42, 99_999, 123_456, -3] { + assert_eq!(sequence_frame(path, &sequence_path(path, frame)), Some(frame), "{path} {frame}"); + } + } + let (before, width, after) = sequence_parts("out/a_[#####].png"); + assert_eq!((before.as_str(), width, after.as_str()), ("out/a_", 5, ".png")); + let (before, width, after) = sequence_parts("a.png"); + assert_eq!((before.as_str(), width, after.as_str()), ("a_", 5, ".png")); + // Other numbers, other widths, other names are not frames of the sequence. + assert_eq!(sequence_frame("a_[#####].png", "a_0042.png"), None, "padded to four"); + assert_eq!(sequence_frame("a_[####].png", "a_+042.png"), None, "a sign is not padding"); + assert_eq!(sequence_frame("a_[#####].png", "a_00042.jpg"), None); + assert_eq!(sequence_frame("a_[#####].png", "b_00042.png"), None); + assert_eq!(sequence_frame("a_[#####].png", "a_00x42.png"), None); + assert_eq!(sequence_frame("a_[#####].png", "a_.png"), None); + assert_eq!(sequence_frame("a_[#####].png", "a_99999999999999999999.png"), None, "too big for a frame number"); + } + + #[test] + fn numbered_templates() { + assert_eq!(numbered_template(DEFAULT_TEMPLATE, 1), "[compName]_1.[fileExtension]"); + assert_eq!(numbered_template(DEFAULT_SEQUENCE_TEMPLATE, 2), "[compName]_2_[#####].[fileExtension]"); + assert_eq!(numbered_template("[compName]_frame.[fileExtension]", 3), "[compName]_frame_3.[fileExtension]"); + assert_eq!(numbered_template("out/[compName]###.png", 1), "out/[compName]_1###.png"); + assert_eq!(numbered_template("my.dir/clip", 4), "my.dir/clip_4", "a dot in a folder is not an extension"); + assert_eq!(numbered_template("clip.v2.mov", 1), "clip.v2_1.mov"); + assert_eq!(numbered_template("", 1), "_1"); + assert_eq!(numbered_template("#", 1), "_1#"); + assert_eq!(numbered_template("[#]", 1), "_1[#]"); + // A numbered default is still a default: it expands to the numbered name. + let v = TemplateVars { comp_name: "footage", project_name: "P", width: 64, height: 36, frame_rate: 30.0, start_frame: 0, end_frame: 29 }; + assert_eq!(expand_template(&numbered_template(DEFAULT_TEMPLATE, 1), OutputFormat::H264, &v), "footage_1.mp4"); + } + #[test] fn set_format_fixes_names() { let mut m = OutputModule::default(); diff --git a/docs/agents.md b/docs/agents.md index 5c58d24b7..183315366 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -4,6 +4,10 @@ Everything in EffectCraft is an engine command with a stable id and JSON params. panel gestures, the CLI, the control channel and MCP all go through the same commands, so anything a user can do, an agent can do too. There are three ways in: +Render-queue outputs cannot replace footage, proxies or the project file. See +[render source protection](render-source-protection.md) for aliases, default names and sequence +limits. This guard does not change ordinary-file overwrite defaults or add another confirmation API. + | Interface | Best for | Needs a window | |---|---|---| | **MCP** (`effectcraft-cli mcp`) | Claude Code and other MCP clients | no (headless), or yes with `--bridge` | diff --git a/docs/render-source-protection.md b/docs/render-source-protection.md new file mode 100644 index 000000000..238684ed4 --- /dev/null +++ b/docs/render-source-protection.md @@ -0,0 +1,26 @@ +# Render source protection + +The render queue refuses an output that would replace footage, an attached proxy, or the +project file. It checks every output module and segment before dispatching the item. A refused +item is marked Failed with its reason; other valid queued items can still render. Render logs +are checked too. No render parameter overrides this protection. + +Checks compare normalized paths and native file identities, so an existing symbolic link, +hard link, junction or Windows short name cannot disguise a source as another output file. +Source identities are indexed once per check and matching identities are verified. The index +does not keep file handles open for the lifetime of a long sequence. + +Default output names move aside when they would replace a protected file. This also applies to +Create Proxy: recreating a proxy chooses a new default name and attaches the new proxy, leaving +the previous proxy file intact. An explicit output path is kept as entered and refused if it +would replace a protected file. + +Image-sequence checks inspect existing frames in the requested range. Skip Existing Files leaves +existing frames alone. If a frame number appears in a folder name, the checker probes each +frame separately and refuses ranges over 100,000 frames instead of silently checking a prefix. +Choose a fixed output folder or a shorter range for those exports. + +This guard does not introduce an overwrite flag, confirmation dialog, or changed replacement +defaults for the CLI, scripts or MCP. Replacement consent and staging ordinary output files +belong to the exporter. The source check runs before export; filesystem changes between that +check and a write still require exporter-side revalidation.