diff --git a/crates/engine/src/cmd/relink.rs b/crates/engine/src/cmd/relink.rs index d899decbe..d8ba05fca 100644 --- a/crates/engine/src/cmd/relink.rs +++ b/crates/engine/src/cmd/relink.rs @@ -1,6 +1,6 @@ //! Recover moved media without replacing interpretation, sequence ranges or layer timing. -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashMap}; use std::path::Path; use effectcraft_project::{Footage, FootageKind, ItemId, ItemKind}; @@ -11,6 +11,8 @@ use crate::{Result, Session, cmd}; const COMMAND: &str = "file.relinkFootage"; const ENTRY_LIMIT: usize = 100_000; +/// Candidates listed for one missing item (more than one already makes it ambiguous). +const MAX_CANDIDATES: usize = 32; fn enabled(_: &Session) -> std::result::Result<(), String> { if cfg!(target_arch = "wasm32") { Err("folder relinking requires the desktop filesystem".into()) } else { Ok(()) } @@ -95,6 +97,8 @@ fn run(s: &mut Session, p: &Value) -> Result { let wait = p.get("wait").and_then(Value::as_bool).unwrap_or(false); s.spawn_task("relink", "Relinking missing footage", wait, move |ctl| { let index = files(&folder, ctl)?; + // Each candidate file is probed once, however many missing items share its name. + let mut probes: HashMap> = HashMap::new(); let mut rows = vec![]; let mut updates = vec![]; for (n, (id, old)) in missing.iter().enumerate() { @@ -107,15 +111,17 @@ fn run(s: &mut Session, p: &Value) -> Result { let paths = index.get(name).cloned().unwrap_or_default(); let mut candidates = vec![]; let mut accepted = vec![]; - for path in paths { + // Past a few candidates the item is ambiguous anyway: list no more of them. + for path in paths.into_iter().take(MAX_CANDIDATES) { if ctl.cancelled() { return Err(crate::render_queue::CANCELLED.into()); } let mut reason = "source type requires manual replacement".to_string(); let mut confidence = "low"; if supported { - match importer.probe(&path) { - Ok(f) if compatible(old, &f) => { + let probed = probes.entry(path.clone()).or_insert_with(|| importer.probe(&path)); + match probed { + Ok(f) if compatible(old, f) => { if old.kind != FootageKind::Sequence || (!old.sequence.is_empty() && sequence_at(old, &path).is_some()) { confidence = "high"; reason = "exact filename and compatible source metadata".into(); @@ -284,6 +290,19 @@ mod tests { assert_eq!(get(&s, id), expected); } + #[test] + fn many_same_named_candidates_list_at_most_the_cap() { + let dir = Fixture::new(); + let (mut s, id, old) = setup(&dir, "plate.png"); + for i in 0..MAX_CANDIDATES + 8 { + dir.file(&format!("new/{i}/plate.png"), b"valid"); + } + let r = search(&mut s, &dir, false); + assert_eq!(r["items"][0]["status"], "ambiguous"); + assert_eq!(r["items"][0]["candidates"].as_array().unwrap().len(), MAX_CANDIDATES); + assert_eq!(get(&s, id), old); + } + #[test] fn duplicates_and_invalid_media_are_reported_without_edits() { let dir = Fixture::new(); diff --git a/crates/engine/src/footage/sequence.rs b/crates/engine/src/footage/sequence.rs index fc2dab2b0..b30a4c4d9 100644 --- a/crates/engine/src/footage/sequence.rs +++ b/crates/engine/src/footage/sequence.rs @@ -11,6 +11,8 @@ //! The browser has no folders: its file table lists the files picked or dropped so far, so //! picking every frame of a sequence imports it there too. +use std::collections::HashMap; + use effectcraft_project::sequence::{frame_number, missing_frame_ranges, sequence_name, split_frame_number}; use effectcraft_project::{Footage, FootageKind}; use effectcraft_time::FrameRate; @@ -126,10 +128,22 @@ fn sort_run(files: &mut Vec, alphabetical: bool) { files.dedup(); } -/// The files of the run `path` belongs to, from its folder. -fn run_files(services: &dyn Services, path: &str, alphabetical: bool) -> Vec { +type RunKey = (String, String, String); + +/// The files of the run `path` belongs to, from its folder. `folders` keeps each folder's files +/// by run, listed once for all the files picked from it. +fn run_files(services: &dyn Services, path: &str, alphabetical: bool, folders: &mut HashMap>>) -> Vec { let Some(key) = run_key(path, alphabetical) else { return vec![] }; - let mut files: Vec = services.list_dir(&key.0).into_iter().filter(|p| run_key(p, alphabetical).as_ref() == Some(&key)).collect(); + let runs = folders.entry(key.0.clone()).or_insert_with(|| { + let mut runs: HashMap> = HashMap::new(); + for p in services.list_dir(&key.0) { + if let Some(k) = run_key(&p, alphabetical) { + runs.entry(k).or_default().push(p); + } + } + runs + }); + let mut files = runs.get(&key).cloned().unwrap_or_default(); sort_run(&mut files, alphabetical); files } @@ -150,15 +164,17 @@ pub fn group(services: &dyn Services, paths: &[String], opts: SequenceOptions) - files.push((p.clone(), opts.sequence)); } } - // Runs in the order their first file was picked. - let mut runs: Vec<((String, String, String), Vec)> = vec![]; + // Runs in the order their first file was picked (found by key, not by a scan of the runs). + let mut runs: Vec<(RunKey, Vec)> = vec![]; + let mut run_of: HashMap = HashMap::new(); let mut out: Vec> = vec![]; let mut slot: Vec = vec![]; for (f, joins) in files { match run_key(&f, opts.alphabetical).filter(|_| joins) { - Some(key) => match runs.iter_mut().find(|(k, _)| *k == key) { - Some((_, run)) => run.push(f), + Some(key) => match run_of.get(&key) { + Some(&i) => runs[i].1.push(f), None => { + run_of.insert(key.clone(), runs.len()); runs.push((key, vec![f])); slot.push(out.len()); out.push(None); @@ -167,13 +183,14 @@ pub fn group(services: &dyn Services, paths: &[String], opts: SequenceOptions) - None => out.push(Some(Source::File(f))), } } + let mut folders = HashMap::new(); for ((_, mut picked), at) in runs.into_iter().zip(slot) { sort_run(&mut picked, opts.alphabetical); // One picked file: its whole run from the folder. if let [one] = picked.as_slice() && opts.sequence { - let all = run_files(services, one, opts.alphabetical); + let all = run_files(services, one, opts.alphabetical, &mut folders); if all.len() >= 2 { picked = all; } @@ -315,6 +332,34 @@ mod tests { assert_eq!(sequences(&t, &strs(&["/s/a_0001.png", "/s/c.png"]), false).len(), 1); } + /// Thousands of files picked one by one from one folder: the folder is listed once, not + /// once per file, and each finds its run by key. + #[test] + fn picked_files_of_one_folder_list_it_once() { + struct Counting(Table, std::sync::atomic::AtomicUsize); + impl Services for Counting { + fn read_file(&self, path: &str) -> std::io::Result> { + self.0.read_file(path) + } + fn write_file(&self, p: &str, b: &[u8]) -> std::io::Result<()> { + self.0.write_file(p, b) + } + fn list_dir(&self, dir: &str) -> Vec { + self.1.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + self.0.list_dir(dir) + } + fn is_dir(&self, path: &str) -> bool { + self.0.is_dir(path) + } + } + let names: Vec<&'static str> = (0..2000).map(|i| &*Box::leak(format!("/s/x{i}_1.png").into_boxed_str())).collect(); + let t = Counting(Table(names.clone()), Default::default()); + let picked: Vec = names.iter().map(|s| s.to_string()).collect(); + let g = group(&t, &picked, SequenceOptions::default()); + assert_eq!(g.len(), 2000); + assert_eq!(t.1.load(std::sync::atomic::Ordering::Relaxed), 1); + } + /// #431: `file.import` brings picked or dropped numbered stills in as stills; only a folder or /// an explicit `sequence: true` makes a sequence, as After Effects' Sequence checkbox does. #[test]