collective
Browse / github.com/storytold/filmcraft / patch
interchange: infer OTIO canvas from linked source video
interchange: infer OTIO canvas from linked source video OTIO imports without explicit sequence settings now derive their canvas from the first successfully linked video in video-track and timeline order. This fixes a portrait source opening in the default landscape canvas. Explicit settings (including 1920×1080) remain authoritative, and unavailable media keep the existing fallback. Nested sequences preserve explicit canvases; inferred canvases are resolved before automatic Motion positions/anchors. The affected import, linking and sizing edits share one undo step. Frame rate remains unchanged. Refs #3256. Validation: - Real host-probed relative-
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- jnohclee-rgb (imported from pull request #3263, open)
- Base
a504f3c5adc4cd60d9c1120916cddb50c9b24b86- Size
- +249 −12 in 5 files
- Review
- pending
- Shared
- 2026-10-28
- Id
c2136cf12ee92d020e9bfae5
How it went
No other operator has reported using it yet.
Diff
raw · crates/engine/src/interchange.rs crates/engine/src/interchange_auto_points_tests.rs crates/interchange/src/common.rs crates/interchange/src/lib.rs crates/interchange/src/otio.rs
@@ -80,16 +80,37 @@ pub fn import(s: &mut Session, path: &str, bytes: &[u8], format: Format) -> Resu sequence.settings.validate().map_err(|e| EngineError::Other(format!("{path}: invalid imported sequence settings: {e}")))?; } }- let before: std::collections::HashSet<ItemId> = s.project.items.keys().copied().collect(); let name = opts.name.clone().unwrap_or_else(|| format.name().to_string());- let seqs = s.edit(&format!("Import {name}"), |proj, _| {- let seqs = filmcraft_interchange::merge_into(proj, fragment, None);+ let infer_size = !fragment.source_size_sequences.is_empty();+ let apply = |session: &mut Session| import_fragment(session, path, fragment, report, format, &name);+ if infer_size {+ // Sizing and its auto-points must undo with the import, never expose a half-sized cut.+ s.edit_group("Import OTIO", apply)+ } else {+ apply(s)+ }+}++fn import_fragment(+ s: &mut Session,+ path: &str,+ fragment: filmcraft_interchange::Imported,+ report: filmcraft_interchange::Report,+ format: Format,+ name: &str,+) -> Result<Value> {+ let before: std::collections::HashSet<ItemId> = s.project.items.keys().copied().collect();+ let (seqs, source_size_sequences) = s.edit(&format!("Import {name}"), |proj, _| {+ let (seqs, source_size_sequences) = filmcraft_interchange::merge_with_provenance(proj, fragment, None); // A document that sets only some Motion parameters leaves the others at their "auto" // (NaN) defaults. Resolve them as placing a clip does, for every format. A media file's // picture size is only what the document says so far (an EDL says nothing: the importer // assumes the sequence size), so its clips' anchors wait for Link Media below.- proj.resolve_placed_auto_points(|seq| !before.contains(&seq), |source| !is_file_media(source));- Ok(seqs)+ proj.resolve_placed_auto_points(+ |seq| !before.contains(&seq) && !source_size_sequences.contains(&seq),+ |source| !is_file_media(source) && !source_size_sequences.contains(&source.id),+ );+ Ok((seqs, source_size_sequences)) })?; // Link media: probe each new file-backed item that exists, through the host's reader as a // direct import does (only the container index is read now, not the whole file: #461).@@ -109,6 +130,7 @@ pub fn import(s: &mut Session, path: &str, bytes: &[u8], format: Format) -> Resu // The document's media paths are opened as stored paths of the document. s.media.add_stored_paths(new_media.iter().map(|(_, p)| p.clone()), Some(path)); let mut linked = 0;+ let mut linked_items = std::collections::HashSet::new(); let mut offline = Vec::new(); for (id, mpath) in new_media { let file = match s.media.follow(&mpath) {@@ -137,15 +159,37 @@ pub fn import(s: &mut Session, path: &str, bytes: &[u8], format: Format) -> Resu } } // the picture size is now the file's own: centre the anchors of its clips in it- proj.resolve_placed_auto_points(|seq| !before.contains(&seq), |source| source.id == id);+ proj.resolve_placed_auto_points(|seq| !before.contains(&seq) && !source_size_sequences.contains(&seq), |source| source.id == id); Ok(()) })?; s.media.insert_file(id, &mpath, src); linked += 1;+ linked_items.insert(id); } None => offline.push(mpath), } }+ if !source_size_sequences.is_empty() {+ s.edit("Size imported sequences", |proj, _| {+ let sizes: Vec<_> = source_size_sequences+ .iter()+ .filter_map(|&id| first_linked_picture(proj, id, &source_size_sequences, &linked_items).map(|size| (id, size)))+ .collect();+ for (id, (width, height)) in sizes {+ if let Some(sequence) = proj.sequence_mut(id) {+ let mut settings = sequence.settings.clone();+ settings.width = width;+ settings.height = height;+ settings.preset = format!("Custom {width}x{height} {}", settings.frame_rate.label());+ if settings.validate().is_ok() {+ sequence.settings = settings;+ }+ }+ }+ proj.resolve_placed_auto_points(|seq| !before.contains(&seq), |_| true);+ Ok(())+ })?;+ } if let Some(&first) = seqs.first() { s.state.active_sequence = Some(first); if !s.state.open_sequences.contains(&first) {@@ -161,6 +205,46 @@ pub fn import(s: &mut Session, path: &str, bytes: &[u8], format: Format) -> Resu })) } +// Choose the earliest picture in each video track, visiting tracks in document order.+// Only successfully linked files provide dimensions; parser placeholders are not evidence.+fn first_linked_picture(+ project: &filmcraft_project::Project,+ root: ItemId,+ source_size_sequences: &[ItemId],+ linked: &std::collections::HashSet<ItemId>,+) -> Option<(u32, u32)> {+ let mut visited = std::collections::HashSet::new();+ let mut pending = vec![root];+ while let Some(id) = pending.pop() {+ if !visited.insert(id) {+ continue;+ }+ let Some(item) = project.item(id) else { continue };+ match &item.kind {+ ItemKind::Media(media) if linked.contains(&id) => {+ if let Some(video) = &media.info.video+ && filmcraft_project::validate_frame_size(video.width, video.height).is_ok()+ {+ return Some((video.width, video.height));+ }+ }+ ItemKind::Subclip { parent, .. } => pending.push(*parent),+ ItemKind::Sequence(sequence) => {+ if !source_size_sequences.contains(&id) {+ return Some((sequence.settings.width, sequence.settings.height));+ }+ for track in sequence.video_tracks.iter().rev() {+ let mut clips: Vec<_> = track.items.iter().collect();+ clips.sort_by_key(|clip| clip.start);+ pending.extend(clips.into_iter().rev().map(|clip| clip.item));+ }+ }+ _ => {}+ }+ }+ None+}+ /// `file.exportInterchange {format: "edl"|"xml"|"fcpxml"|"otio"|"aaf"|"omf" = "xml", path, sequence?}` pub fn export(s: &mut Session, p: &Value) -> Result<Value> { // FCP7 XML only when no format is asked for: a format we don't know is an error, not XML.@@ -256,3 +256,145 @@ fn an_interchange_import_links_media_without_reading_the_files_whole() { assert_eq!(s.project.source_size(clip.item), Some((320, 180)), "the file's real size, from its index"); let _ = std::fs::remove_dir_all(&d); }++#[test]+fn otio_without_sequence_settings_uses_the_linked_picture_size() {+ let d = tmp_dir("otio-source-size");+ make_movie(&d.join("portrait.mov"), DemoScene::OceanSunset, 48, 80, 24);+ let range = json!({"OTIO_SCHEMA":"TimeRange.1","start_time":{"OTIO_SCHEMA":"RationalTime.1","value":0,"rate":24},"duration":{"OTIO_SCHEMA":"RationalTime.1","value":24,"rate":24}});+ let doc = json!({"OTIO_SCHEMA":"Timeline.1","name":"Portrait cut","tracks":{"OTIO_SCHEMA":"Stack.1","children":[{"OTIO_SCHEMA":"Track.1","kind":"Video","children":[{"OTIO_SCHEMA":"Clip.2","name":"Portrait","source_range":range,"media_reference":{"OTIO_SCHEMA":"ExternalReference.1","target_url":"portrait.mov","available_range":range}}]}]}});+ let path = d.join("cut.otio");+ std::fs::write(&path, serde_json::to_vec(&doc).unwrap()).unwrap();+ let mut s = Session::default();+ let (result, seq) = import(&mut s, &path);+ assert_eq!(result["linkedMedia"], 1, "the host actually probed the local source");+ let settings = &s.project.sequence(seq).unwrap().settings;+ assert_eq!((settings.width, settings.height), (48, 80));+ let clip = first_clip(&s, seq);+ assert_eq!(motion_point(&clip, "position"), Vec2::new(24.0, 40.0), "resolve placement against the inferred canvas");+ assert_eq!(motion_point(&clip, "anchor"), Vec2::new(24.0, 40.0));+}++fn portrait_otio(path: &str) -> serde_json::Value {+ let range = json!({"OTIO_SCHEMA":"TimeRange.1","start_time":{"OTIO_SCHEMA":"RationalTime.1","value":0,"rate":24},"duration":{"OTIO_SCHEMA":"RationalTime.1","value":24,"rate":24}});+ json!({"OTIO_SCHEMA":"Timeline.1","name":"Cut","tracks":{"OTIO_SCHEMA":"Stack.1","children":[{"OTIO_SCHEMA":"Track.1","kind":"Video","children":[{"OTIO_SCHEMA":"Clip.2","source_range":range,"media_reference":{"OTIO_SCHEMA":"ExternalReference.1","target_url":path,"available_range":range}}]}]}})+}++#[test]+fn otio_explicit_hd_canvas_overrides_a_linked_portrait() {+ let d = tmp_dir("otio-explicit-size");+ make_movie(&d.join("portrait.mov"), DemoScene::OceanSunset, 48, 80, 24);+ let mut doc = portrait_otio("portrait.mov");+ doc["metadata"] = json!({"filmcraft":{"settings":filmcraft_project::SequenceSettings::default()}});+ let path = d.join("cut.otio");+ std::fs::write(&path, serde_json::to_vec(&doc).unwrap()).unwrap();+ let mut session = Session::default();+ let (_, id) = import(&mut session, &path);+ let settings = &session.project.sequence(id).unwrap().settings;+ assert_eq!((settings.width, settings.height), (1920, 1080));+ assert_eq!(motion_point(&first_clip(&session, id), "position"), Vec2::new(960., 540.));+ std::fs::remove_dir_all(d).unwrap();+}++#[test]+fn otio_offline_picture_keeps_the_default_canvas() {+ let d = tmp_dir("otio-offline-size");+ let path = d.join("cut.otio");+ std::fs::write(&path, serde_json::to_vec(&portrait_otio("absent.mov")).unwrap()).unwrap();+ let mut session = Session::default();+ let (result, id) = import(&mut session, &path);+ assert_eq!(result["linkedMedia"], 0);+ let settings = &session.project.sequence(id).unwrap().settings;+ assert_eq!((settings.width, settings.height), (1920, 1080));+ assert_eq!(motion_point(&first_clip(&session, id), "position"), Vec2::new(960., 540.));+ std::fs::remove_dir_all(d).unwrap();+}++#[test]+fn otio_nested_canvas_uses_explicit_settings_or_the_linked_picture() {+ let d = tmp_dir("otio-nested-size");+ make_movie(&d.join("portrait.mov"), DemoScene::OceanSunset, 48, 80, 24);+ for explicit in [false, true] {+ let original = portrait_otio("portrait.mov");+ let mut nested = original["tracks"].clone();+ nested["source_range"] = original["tracks"]["children"][0]["children"][0]["source_range"].clone();+ if explicit {+ nested["metadata"] = json!({"filmcraft":{"settings":filmcraft_project::SequenceSettings::default()}});+ }+ let mut doc = original;+ doc["tracks"]["children"][0]["children"] = json!([nested]);+ let path = d.join("cut.otio");+ std::fs::write(&path, serde_json::to_vec(&doc).unwrap()).unwrap();+ let mut session = Session::default();+ let (_, id) = import(&mut session, &path);+ let settings = &session.project.sequence(id).unwrap().settings;+ let expected = if explicit { (1920, 1080) } else { (48, 80) };+ assert_eq!((settings.width, settings.height), expected);+ assert_eq!(motion_point(&first_clip(&session, id), "position"), Vec2::new(expected.0 as f64 / 2., expected.1 as f64 / 2.));+ assert!(auto_points(&session.project).is_empty());+ }+ std::fs::remove_dir_all(d).unwrap();+}++#[test]+fn otio_source_sizing_and_auto_points_undo_and_redo_together() {+ let d = tmp_dir("otio-size-history");+ make_movie(&d.join("portrait.mov"), DemoScene::OceanSunset, 48, 80, 24);+ let path = d.join("cut.otio");+ std::fs::write(&path, serde_json::to_vec(&portrait_otio("portrait.mov")).unwrap()).unwrap();+ let mut session = Session::default();+ let before = session.project.clone();+ let (_, _) = import(&mut session, &path);+ let imported = session.project.clone();+ session.execute("edit.undo", json!({})).unwrap();+ assert_eq!(*session.project, *before);+ session.execute("edit.redo", json!({})).unwrap();+ assert_eq!(*session.project, *imported);+ std::fs::remove_dir_all(d).unwrap();+}++#[test]+fn otio_picture_choice_follows_video_timeline_not_media_allocation() {+ let d = tmp_dir("otio-picture-order");+ make_movie(&d.join("portrait.mov"), DemoScene::OceanSunset, 48, 80, 24);+ make_movie(&d.join("landscape.mov"), DemoScene::OceanSunset, 80, 48, 24);+ let mut doc = portrait_otio("portrait.mov");+ let landscape = portrait_otio("landscape.mov");+ let mut audio = landscape["tracks"]["children"][0].clone();+ audio["kind"] = json!("Audio");+ let mut video = doc["tracks"]["children"][0].clone();+ video["children"].as_array_mut().unwrap().push(landscape["tracks"]["children"][0]["children"][0].clone());+ // Audio creates landscape's media item first; it must not choose the video canvas.+ doc["tracks"]["children"] = json!([audio, video]);+ let path = d.join("cut.otio");+ std::fs::write(&path, serde_json::to_vec(&doc).unwrap()).unwrap();+ let mut session = Session::default();+ let (result, id) = import(&mut session, &path);+ assert_eq!(result["linkedMedia"], 2);+ let settings = &session.project.sequence(id).unwrap().settings;+ assert_eq!((settings.width, settings.height), (48, 80));+ assert_eq!(settings.frame_rate, filmcraft_time::FrameRate::new(24, 1));+ assert_eq!(settings.preset, "Custom 48x80 24");+ assert!(auto_points(&session.project).is_empty());+ std::fs::remove_dir_all(d).unwrap();+}++#[test]+fn otio_explicit_parent_waits_for_an_inferred_nested_anchor() {+ let d = tmp_dir("otio-nested-anchor");+ make_movie(&d.join("portrait.mov"), DemoScene::OceanSunset, 48, 80, 24);+ let original = portrait_otio("portrait.mov");+ let mut nested = original["tracks"].clone();+ nested["source_range"] = original["tracks"]["children"][0]["children"][0]["source_range"].clone();+ let mut doc = original;+ doc["metadata"] = json!({"filmcraft":{"settings":filmcraft_project::SequenceSettings::default()}});+ doc["tracks"]["children"][0]["children"] = json!([nested]);+ let path = d.join("cut.otio");+ std::fs::write(&path, serde_json::to_vec(&doc).unwrap()).unwrap();+ let mut session = Session::default();+ let (_, id) = import(&mut session, &path);+ let clip = first_clip(&session, id);+ assert_eq!(motion_point(&clip, "position"), Vec2::new(960., 540.));+ assert_eq!(motion_point(&clip, "anchor"), Vec2::new(24., 40.));+ std::fs::remove_dir_all(d).unwrap();+}@@ -348,12 +348,13 @@ pub(crate) struct Builder { pub p: Project, media: HashMap<String, ItemId>, pub top: Vec<ItemId>,+ pub source_size_sequences: Vec<ItemId>, next_link: u64, } impl Builder { pub fn new(name: &str) -> Self {- Self { p: Project::new(name), media: HashMap::new(), top: Vec::new(), next_link: 1 }+ Self { p: Project::new(name), media: HashMap::new(), top: Vec::new(), source_size_sequences: Vec::new(), next_link: 1 } } pub fn bin(&mut self, name: &str, parent: Option<BinId>) -> BinId {@@ -512,7 +513,7 @@ impl Builder { m.info.duration = end; } }- Imported { project: self.p, sequences: self.top }+ Imported { project: self.p, sequences: self.top, source_size_sequences: self.source_size_sequences } } } @@ -236,6 +236,8 @@ pub struct Imported { pub project: Project, /// Top-level sequences, in document order (nested sequences are items too but not listed here). pub sequences: Vec<ItemId>,+ /// OTIO sequences without explicit settings, eligible for host-probed picture sizing.+ pub source_size_sequences: Vec<ItemId>, } /// Import options beyond the defaults of [`import`].@@ -424,7 +426,12 @@ pub fn export_edl_per_track(project: &Project, sequence: ItemId, opts: &ExportOp /// fragment's root children go into `bin` (or the root). Returns the new ids of the fragment's /// top-level sequences. pub fn merge_into(target: &mut Project, fragment: Imported, bin: Option<BinId>) -> Vec<ItemId> {- let Imported { project: frag, sequences } = fragment;+ merge_with_provenance(target, fragment, bin).0+}++/// Merge a fragment, also returning remapped sequences eligible for source picture sizing.+pub fn merge_with_provenance(target: &mut Project, fragment: Imported, bin: Option<BinId>) -> (Vec<ItemId>, Vec<ItemId>) {+ let Imported { project: frag, sequences, source_size_sequences } = fragment; let mut items: HashMap<ItemId, ItemId> = HashMap::new(); for id in frag.items.keys() { items.insert(*id, ItemId(target.alloc_id()));@@ -498,7 +505,7 @@ pub fn merge_into(target: &mut Project, fragment: Imported, bin: Option<BinId>) None => &mut target.root, }; dest.children.extend(root.children);- sequences.iter().filter_map(|s| items.get(s).copied()).collect()+ (sequences.iter().filter_map(|s| items.get(s).copied()).collect(), source_size_sequences.iter().filter_map(|s| items.get(s).copied()).collect()) } /// After probing a media file imported from an EDL (whose source times are absolute timecode),@@ -434,9 +434,12 @@ impl Imp<'_, '_> { } fn sequence(&mut self, stack: &Value, name: &str, meta: Option<&Value>, rate: FrameRate) -> ItemId {- let settings: SequenceSettings =- meta.and_then(|m| m.get("settings")).and_then(|s| serde_json::from_value(s.clone()).ok()).unwrap_or_else(|| settings_for(rate, 1920, 1080, false));+ let explicit: Option<SequenceSettings> = meta.and_then(|m| m.get("settings")).and_then(|s| serde_json::from_value(s.clone()).ok());+ let settings = explicit.clone().unwrap_or_else(|| settings_for(rate, 1920, 1080, false)); let id = self.b.reserve_sequence(name, settings.clone(), None);+ if explicit.is_none() {+ self.b.source_size_sequences.push(id);+ } if let Some(l) = meta.and_then(|m| m.get("label")).and_then(|l| serde_json::from_value::<Label>(l.clone()).ok()) && let Some(it) = self.b.p.item_mut(id) {