From 700261ac81b930e683242ba1880740bc4e40f079 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Mon, 2 Nov 2026 21:54:29 -0500 Subject: [PATCH] interchange: end export walks on folder and precomp cycles, and index each clip's transitions on import Export followed a folder's parent chain recursively before caching it, so a project whose folders loop back to themselves overflowed the stack. The walk is now iterative and a repeated folder ends it (the cycle's top becomes a top-level bin, with a warning). comp_has_audio recursed into nested comps with no visited set: a precomp cycle never ended and a chain of comps that each use the next twice took 2^depth steps. It now walks with a stack and a visited set. Import scanned every transition of a track for every clip on it, quadratic in a long flat edit. Each track now indexes its transitions by clip once, and the per-transition warnings are said once per track. Warning de-duplication uses a set instead of a linear search. --- crates/interchange/src/export.rs | 71 ++++++++++++++++++++---- crates/interchange/src/import.rs | 82 ++++++++++++++++----------- crates/interchange/src/tests.rs | 95 ++++++++++++++++++++++++++++++++ 3 files changed, 204 insertions(+), 44 deletions(-) diff --git a/crates/interchange/src/export.rs b/crates/interchange/src/export.rs index 375c1bf1b..d75940d9f 100644 --- a/crates/interchange/src/export.rs +++ b/crates/interchange/src/export.rs @@ -303,16 +303,41 @@ impl Exporter<'_> { } /// The bin mirroring a Project panel folder. + /// + /// Walks up the folder's parents without recursion: a damaged or hostile project can hold a + /// parent cycle (or a very long chain), which recursed until the stack ran out. A folder seen + /// twice in one walk ends it, and the cycle's topmost folder becomes a top-level bin. fn bin(&mut self, folder: Option) -> Option { let f = folder?; - if let Some(b) = self.bins.get(&f) { - return Some(*b); + // The uncached folders from `f` up, and the bin they hang from. + let mut chain: Vec = vec![]; + let mut in_chain = std::collections::HashSet::new(); + let mut parent = None; + let mut at = Some(f); + while let Some(id) = at { + if let Some(b) = self.bins.get(&id) { + parent = Some(*b); + break; + } + if !in_chain.insert(id) { + self.warn("a folder that contains itself was exported as a top-level bin"); + break; + } + let Some(it) = self.p.item(id) else { break }; + chain.push(id); + at = it.parent; } - let it = self.p.item(f)?; - let parent = self.bin(it.parent); - let b = self.fp.add_bin(&it.name, parent); - self.bins.insert(f, b); - Some(b) + if chain.first() != Some(&f) { + // `f` itself is not an item. + return None; + } + for id in chain.into_iter().rev() { + let name = self.p.item(id).map(|it| it.name.clone()).unwrap_or_default(); + let b = self.fp.add_bin(&name, parent); + self.bins.insert(id, b); + parent = Some(b); + } + parent } fn footage_item(&mut self, item: ItemId, f: &Footage) -> fp::ItemId { @@ -615,12 +640,34 @@ impl Exporter<'_> { } } +/// Whether `c` or a comp nested in it (at any depth) has a layer of footage with audio. +/// +/// A walk with a stack and a visited set, as in `Project::comp_contains`: recursion never ended +/// on a precomp cycle, and re-walked a comp shared by many layers once per path (exponential +/// for a chain of comps that each use the next one twice). fn comp_has_audio(p: &Project, c: &Comp) -> bool { - c.layers.iter().any(|l| match &l.source { - LayerSource::Footage { item } => matches!(p.item(*item).map(|i| &i.kind), Some(ItemKind::Footage(f)) if f.has_audio), - LayerSource::Comp { item } => p.comp(*item).is_some_and(|c| comp_has_audio(p, c)), - _ => false, - }) + let mut seen = std::collections::HashSet::new(); + let mut stack = vec![c]; + while let Some(c) = stack.pop() { + for l in &c.layers { + match &l.source { + LayerSource::Footage { item } => { + if matches!(p.item(*item).map(|i| &i.kind), Some(ItemKind::Footage(f)) if f.has_audio) { + return true; + } + } + LayerSource::Comp { item } => { + if seen.insert(*item) + && let Some(nested) = p.comp(*item) + { + stack.push(nested); + } + } + _ => {} + } + } + } + false } fn differs(v: &Value, def: [f64; 2]) -> bool { diff --git a/crates/interchange/src/import.rs b/crates/interchange/src/import.rs index 28f638a90..adeaf9748 100644 --- a/crates/interchange/src/import.rs +++ b/crates/interchange/src/import.rs @@ -1,7 +1,7 @@ //! Timeline documents → compositions (File ▸ Import ▸ Adobe Premiere Pro Project / Timeline //! Interchange…). -use std::collections::{BTreeMap, HashMap}; +use std::collections::{BTreeMap, HashMap, HashSet}; use effectcraft_color::{BlendMode, Label}; use effectcraft_keyframe::{Keyframe, Value}; @@ -163,7 +163,9 @@ pub fn import( } let folder_name = if opts.name.is_empty() { "Imported Timeline".to_string() } else { opts.name.clone() }; res.folder = project.add_item(&folder_name, Label::None, None, ItemKind::Folder); - let mut im = Importer { src: &imported.project, project, res, items: HashMap::new(), solids: HashMap::new(), solids_folder: None, bins: HashMap::new() }; + let warned = res.warnings.iter().cloned().collect(); + let mut im = + Importer { src: &imported.project, project, res, warned, items: HashMap::new(), solids: HashMap::new(), solids_folder: None, bins: HashMap::new() }; im.run(&imported.sequences, probe); Ok(im.res) } @@ -172,6 +174,8 @@ struct Importer<'a> { src: &'a fp::Project, project: &'a mut Project, res: ImportResult, + /// The warnings in `res`, to say each once without searching them all every time. + warned: HashSet, /// Source item → created item (footage, comp). items: HashMap, /// Generator / adjustment solids by (source item, width, height). @@ -184,7 +188,7 @@ struct Importer<'a> { impl Importer<'_> { fn warn(&mut self, m: impl Into) { let m = m.into(); - if !self.res.warnings.contains(&m) { + if self.warned.insert(m.clone()) { self.res.warnings.push(m); } } @@ -321,23 +325,8 @@ impl Importer<'_> { comp.markers = seq.markers.iter().map(marker_in).collect(); // Video: the top track's clips first (layer #1); later clips of a track above earlier ones // so incoming clips of dissolves cover outgoing ones. - for track in seq.video_tracks.iter().rev() { - let mut items: Vec<&fp::TrackItem> = track.items.iter().collect(); - items.sort_by_key(|i| std::cmp::Reverse(i.start)); - for ti in items { - if let Some(l) = self.clip_layer(&comp, seq, track, ti) { - comp.layers.push(l); - } - } - } - for track in &seq.audio_tracks { - let mut items: Vec<&fp::TrackItem> = track.items.iter().collect(); - items.sort_by_key(|i| std::cmp::Reverse(i.start)); - for ti in items { - if let Some(l) = self.clip_layer(&comp, seq, track, ti) { - comp.layers.push(l); - } - } + for track in seq.video_tracks.iter().rev().chain(&seq.audio_tracks) { + self.track_layers(&mut comp, seq, track); } if !seq.caption_tracks.is_empty() { self.warn("caption tracks are not imported"); @@ -400,7 +389,45 @@ impl Importer<'_> { } } - fn clip_layer(&mut self, comp: &Comp, seq: &fp::Sequence, track: &fp::Track, ti: &fp::TrackItem) -> Option { + /// A track's clips as layers of `comp`, the latest first. + fn track_layers(&mut self, comp: &mut Comp, seq: &fp::Sequence, track: &fp::Track) { + let mut items: Vec<&fp::TrackItem> = track.items.iter().collect(); + items.sort_by_key(|i| std::cmp::Reverse(i.start)); + // Each clip's transitions, found once per track: scanning every transition of the track + // for every clip made a long flat edit (many clips and many transitions) quadratic. + let mut transitions: HashMap> = HashMap::new(); + for tr in &track.transitions { + for clip in [tr.from, tr.to].into_iter().flatten() { + let list = transitions.entry(clip).or_default(); + // A transition from and to the same clip still counts once. + if !list.last().is_some_and(|last| std::ptr::eq(*last, tr)) { + list.push(tr); + } + } + } + let mut any = false; + for ti in items { + let own = transitions.get(&ti.id).map(Vec::as_slice).unwrap_or_default(); + if let Some(l) = self.clip_layer(comp, seq, track, ti, own) { + comp.layers.push(l); + any = true; + } + } + // What the track's transitions become, said once a clip of the track was imported. + if any && track.kind != fp::TrackKind::Audio { + for tr in &track.transitions { + let id = tr.effect.effect.as_str(); + if !matches!(id, "cross_dissolve" | "dip_to_black" | "dip_to_white" | "film_dissolve" | "additive_dissolve" | "non_additive_dissolve") { + self.warn(format!("transition \"{}\" was imported as a cross dissolve", tr.effect.def().map(|d| d.name).unwrap_or(id))); + } + } + } + if any && track.transitions.iter().any(|tr| tr.effect.effect == "dip_to_white") { + self.warn("Dip to White was imported as a dip to the background"); + } + } + + fn clip_layer(&mut self, comp: &Comp, seq: &fp::Sequence, track: &fp::Track, ti: &fp::TrackItem, transitions: &[&fp::Transition]) -> Option { let audio_track = track.kind == fp::TrackKind::Audio; let (source, size, has_audio) = self.source_of(ti, comp.width, comp.height)?; let is_adjustment = matches!(self.src.item(ti.item).map(|i| &i.kind), Some(fp::ItemKind::AdjustmentLayer { .. })); @@ -433,18 +460,9 @@ impl Importer<'_> { } // Transitions extend the layer over their handles. let mut fades: Vec<(Tick, Tick, f64, f64)> = vec![]; - for tr in &track.transitions { + for tr in transitions { let (ts, te) = (tick_in(tr.start), tick_in(tr.end())); - let id = tr.effect.effect.as_str(); - let dip = id.starts_with("dip_to"); - if !audio_track - && !matches!(id, "cross_dissolve" | "dip_to_black" | "dip_to_white" | "film_dissolve" | "additive_dissolve" | "non_additive_dissolve") - { - self.warn(format!("transition \"{}\" was imported as a cross dissolve", tr.effect.def().map(|d| d.name).unwrap_or(id))); - } - if dip && id == "dip_to_white" { - self.warn("Dip to White was imported as a dip to the background"); - } + let dip = tr.effect.effect.starts_with("dip_to"); let mid = ts + Tick((te - ts).0 / 2); if tr.to == Some(ti.id) { l.in_point = l.in_point.min(ts); diff --git a/crates/interchange/src/tests.rs b/crates/interchange/src/tests.rs index 97efd76cd..278467d2a 100644 --- a/crates/interchange/src/tests.rs +++ b/crates/interchange/src/tests.rs @@ -639,3 +639,98 @@ fn deeply_nested_documents_error_instead_of_overflowing_the_stack() { assert_eq!(super::import::xml_depth(b"]]>"), 2); assert_eq!(super::import::json_depth(br#"{"a": "[[[{{", "b": [[1], {"c": "\\\"["}]}"#), 3); } + +/// A hostile project: footage in a folder whose parents loop back to it. Export walked the +/// folder's parents recursively and never reached the cache insert, so the stack ran out. +#[test] +fn export_survives_a_folder_parent_cycle() { + let (mut p, cid) = ae_project(); + let a = p.add_item("A", effectcraft_color::Label::None, None, ItemKind::Folder); + let b = p.add_item("B", effectcraft_color::Label::None, Some(a), ItemKind::Folder); + p.items.get_mut(&a).unwrap().parent = Some(b); + let footage: Vec = p.items.values().filter(|i| matches!(i.kind, ItemKind::Footage(_))).map(|i| i.id).collect(); + for f in footage { + p.items.get_mut(&f).unwrap().parent = Some(a); + } + let out = export(&p, cid, &ExportOptions::default(), &HashMap::new()).unwrap(); + assert!(out.warnings.iter().any(|w| w.contains("contains itself")), "{:?}", out.warnings); +} + +/// Precomps that use each other, and a chain of comps that each use the next one twice: the +/// audio check recursed forever on the first and 2^depth times on the second. +#[test] +fn export_audio_check_ends_on_precomp_cycles_and_shared_chains() { + let mut p = Project::default(); + let a = footage_item(&mut p, "/media/a.mov"); + let comp = || Comp::new(W, H, FrameRate::new(25, 1), t(2.0)); + // A chain of 60 comps, each using the next twice; the last one is silent footage. + let mut next = { + let mut c = comp(); + let l = effectcraft_project::build::layer(&mut p, &c, "a", LayerSource::Footage { item: a }, (W, H), None); + c.layers.push(l); + comp_item(&mut p, "Leaf", c) + }; + for i in 0..60 { + let mut c = comp(); + for n in 0..2 { + let l = effectcraft_project::build::layer(&mut p, &c, &format!("use {n}"), LayerSource::Comp { item: next }, (W, H), None); + c.layers.push(l); + } + next = comp_item(&mut p, &format!("Chain {i}"), c); + } + let started = std::time::Instant::now(); + export(&p, next, &ExportOptions::default(), &HashMap::new()).unwrap(); + assert!(started.elapsed() < std::time::Duration::from_secs(30), "{:?}", started.elapsed()); + // Two comps nesting each other. + let x = comp_item(&mut p, "X", comp()); + let mut y = comp(); + let l = effectcraft_project::build::layer(&mut p, &y, "x", LayerSource::Comp { item: x }, (W, H), None); + y.layers.push(l); + let y = comp_item(&mut p, "Y", y); + let mut xc = comp(); + let l = effectcraft_project::build::layer(&mut p, &xc, "y", LayerSource::Comp { item: y }, (W, H), None); + xc.layers.push(l); + if let Some(ItemKind::Comp(c)) = p.items.get_mut(&x).map(|i| &mut i.kind) { + *c = Arc::new(xc); + } + export(&p, x, &ExportOptions::default(), &HashMap::new()).unwrap(); +} + +/// A flat edit with many clips and many transitions: each clip finds its own transitions (it +/// scanned every transition of the track per clip), and they still extend and fade it. +#[test] +fn import_finds_each_clips_transitions_without_scanning_the_track() { + let mut p = fp::Project::new("Flat"); + let a = media(&mut p, "a.png", "/media/a.mov", filmcraft_media::MediaKind::Movie, true, false, None); + let settings = fp::SequenceSettings { width: W, height: H, frame_rate: rate(), ..Default::default() }; + let main = p.new_sequence("Main", settings, 1, 0, None); + const N: usize = 500; + let mut items = Vec::with_capacity(N); + let mut transitions = Vec::with_capacity(N); + for i in 0..N { + let ti = item(&mut p, a, fp::TrackKind::Video, i as f64 * 0.4, 0.0, 0.4); + if let Some(prev) = items.last().map(|x: &fp::TrackItem| x.id) { + transitions.push(fp::Transition { + id: fp::TransitionId(p.alloc_id()), + effect: fp::find_effect("cross_dissolve").unwrap().instance(), + start: secs(i as f64 * 0.4 - 0.08), + duration: secs(0.16), + from: Some(prev), + to: Some(ti.id), + align: fp::TransitionAlign::CenterAtCut, + reverse: false, + }); + } + items.push(ti); + } + let seq = p.sequence_mut(main).unwrap(); + seq.video_tracks[0].items = items; + seq.video_tracks[0].transitions = transitions; + let (bytes, _) = fc::export(&p, main, fc::Format::Otio, &Default::default()).unwrap(); + let (q, r) = import_doc(&bytes, "Flat.otio", &mut probe); + let m = q.comp(r.comps[0]).unwrap(); + assert_eq!(m.layers.len(), N); + // A middle clip reaches over both of its transitions' handles. + let mid = m.layers.iter().find(|l| near(l.in_point, t(100.0 - 0.08))).expect("clip 250 extended into its incoming transition"); + assert!(near(mid.out_point, t(100.4 + 0.08)), "{}", mid.out_point.seconds()); +}