From b7e848a629763907a7c0e10dfbc9b1dec0802b2b Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 15:54:01 -0500 Subject: [PATCH] Import, Relink: index sequence runs and folder listings; probe relink candidates once (#3000) * fix(engine): index sequence runs and folder listings on import; probe relink candidates once * address review: cap relink candidates before cloning paths; reuse a picked folder's listing for its run lookups --- crates/engine/src/cmd/relink.rs | 31 ++++++-- crates/engine/src/footage/sequence.rs | 101 +++++++++++++++++++++++--- 2 files changed, 116 insertions(+), 16 deletions(-) diff --git a/crates/engine/src/cmd/relink.rs b/crates/engine/src/cmd/relink.rs index d899decbe..e74e4bf97 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() { @@ -104,19 +108,21 @@ fn run(s: &mut Session, p: &Value) -> Result { let supported = matches!(old.kind, FootageKind::Still | FootageKind::Sequence | FootageKind::Video | FootageKind::Audio) && old.layer.is_none() && old.page == 0; let name = Path::new(&old.path).file_name().and_then(std::ffi::OsStr::to_str).unwrap_or(""); - let paths = index.get(name).cloned().unwrap_or_default(); + let paths = index.get(name).map_or(&[][..], Vec::as_slice); 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.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) => { - if old.kind != FootageKind::Sequence || (!old.sequence.is_empty() && sequence_at(old, &path).is_some()) { + 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(); accepted.push(path.clone()); @@ -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..891ebdbe1 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 } @@ -138,27 +152,41 @@ fn run_files(services: &dyn Services, path: &str, alphabetical: bool) -> Vec Vec { + // Each folder's runs, listed once for everything that needs them (a picked folder's listing + // is kept for the run lookup of its lone files). + let mut folders: HashMap>> = HashMap::new(); // Each file, and whether it may join a sequence. let mut files: Vec<(String, bool)> = vec![]; for p in paths { if services.is_dir(p) { - let mut inside: Vec = - services.list_dir(p).into_iter().filter(|f| matches!(crate::media_browser::kind_of(f), Some(k) if k != "project")).collect(); + let listing = services.list_dir(p); + let mut runs: HashMap> = HashMap::new(); + for f in &listing { + if let Some(k) = run_key(f, opts.alphabetical) { + runs.entry(k).or_default().push(f.clone()); + } + } + if let Some(dir) = runs.keys().next().map(|k| k.0.clone()) { + folders.entry(dir).or_insert(runs); + } + let mut inside: Vec = listing.into_iter().filter(|f| matches!(crate::media_browser::kind_of(f), Some(k) if k != "project")).collect(); inside.sort_by_key(|f| file_name(f).to_lowercase()); files.extend(inside.into_iter().map(|f| (f, opts.folders))); } else { 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); @@ -173,7 +201,7 @@ pub fn group(services: &dyn Services, paths: &[String], opts: SequenceOptions) - 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 +343,59 @@ 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); + } + + /// A picked folder's listing serves the run lookup of its lone files: one listing in all. + #[test] + fn a_picked_folder_is_listed_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 t = Counting(Table(vec!["/s/a_0001.png", "/s/b_0001.png", "/s/b_0002.png"]), Default::default()); + let g = group(&t, &strs(&["/s"]), SequenceOptions { sequence: true, folders: true, alphabetical: false }); + assert_eq!(g.len(), 2, "{g:?}"); + 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]