From 7a3d00319493c05b215a91bc96a46528bb97fc69 Mon Sep 17 00:00:00 2001 From: Ikki Nishihara Date: Mon, 12 Oct 2026 01:55:24 +0900 Subject: [PATCH] =?UTF-8?q?fix:=20=E3=82=B5=E3=83=96=E3=82=AF=E3=83=AA?= =?UTF-8?q?=E3=83=83=E3=83=97=E9=85=8D=E7=BD=AE=E3=81=A7=E8=A6=AA=E3=81=AE?= =?UTF-8?q?=E9=9F=B3=E5=A3=B0=E3=82=92=E4=BF=9D=E6=8C=81=20#913?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/engine/src/commands.rs | 16 +-- crates/engine/src/sequence_tools.rs | 7 +- crates/engine/src/source_placement_tests.rs | 103 +++++++++++++++++++- crates/project/src/lib.rs | 23 ++++- 4 files changed, 135 insertions(+), 14 deletions(-) diff --git a/crates/engine/src/commands.rs b/crates/engine/src/commands.rs index 6609afdd..33b2b104 100644 --- a/crates/engine/src/commands.rs +++ b/crates/engine/src/commands.rs @@ -681,8 +681,9 @@ pub(crate) fn place_item( if s.state.sequences_as_clips && pi.as_sequence().is_some_and(|q| q.multicam.is_none()) { return crate::sequence_tools::place_sequence_clips(s, item, range, at, vdest, adest, insert, label); } - let has_v = pi.has_video() && vdest.is_some(); - let has_a = pi.has_audio() && adest.is_some(); + let (video, audio) = s.project.source_streams(item); + let has_v = video && vdest.is_some(); + let has_a = audio && adest.is_some(); if !has_v && !has_a { return Err(EngineError::Other("no destination track for this media (check source patching)".into())); } @@ -724,7 +725,7 @@ pub(crate) fn place_item( let mut a = p.make_track_item(item, TrackKind::Audio, aat, arange, rate).ok_or_else(|| bad(label, "bad item"))?; // Modify ▸ Audio Channels with several audio clips: one per clip, on the tracks below let clips: Vec> = - p.item(item).and_then(|i| i.as_media()).and_then(|m| m.interpret.audio_channels.as_ref()).map(|m| m.clips.clone()).unwrap_or_default(); + p.resolve_media(item).and_then(|(_, m, _)| m.interpret.audio_channels.as_ref()).map(|m| m.clips.clone()).unwrap_or_default(); let streams = p.resolve_media(item).map_or(1, |(_, m, _)| m.info.audio_streams.len()); let specs = audio_placement_specs(&clips, streams); let first = p.sequence(seq_id).and_then(|q| q.audio_tracks.iter().position(|t| t.id == adest)).unwrap_or(0); @@ -2203,6 +2204,7 @@ fn build() -> Vec { let at = time_p(s, p, "").unwrap_or(s.playhead()); let tg = s.targeting(); let pi = s.project.item(item).ok_or_else(|| bad("timeline.place", "no such item"))?; + let (has_video, has_audio) = s.project.source_streams(item); let is_audio = |t: TrackId| s.active_sequence().is_some_and(|q| q.audio_tracks.iter().any(|x| x.id == t)); // `track` is where the picture goes and `audioTrack` where the sound goes. A caller that // names an audio track as `track` asks for the sound alone, on that track. @@ -2211,21 +2213,21 @@ fn build() -> Vec { return Err(bad("timeline.place", "`track` names an audio track and `audioTrack` is given as well: name the sound's track once")); } (_, Some(t)) if !is_audio(t) => return Err(bad("timeline.place", "`audioTrack` names a video track")), - (Some(t), None) if is_audio(t) && !pi.has_audio() => { + (Some(t), None) if is_audio(t) && !has_audio => { return Err(bad("timeline.place", "`track` names an audio track and this item has no sound")); } (Some(t), None) if is_audio(t) => (None, Some(t)), - (Some(_), None) if !pi.has_video() => { + (Some(_), None) if !has_video => { return Err(bad("timeline.place", "`track` names a video track and this item has no picture: name an audio track")); } (v, a) => (v.or(tg.video_dest), a.or(tg.audio_dest)), }; let v = v.filter(|_| video); let a = a.filter(|_| audio); - if p.get("video").is_some() && video && pi.has_video() && v.is_none() { + if p.get("video").is_some() && video && has_video && v.is_none() { return Err(bad("timeline.place", "no video destination: add or enable a video track")); } - if p.get("audio").is_some() && audio && pi.has_audio() && a.is_none() { + if p.get("audio").is_some() && audio && has_audio && a.is_none() { return Err(bad("timeline.place", "no audio destination: add or enable an audio track")); } let full = full_duration(s, pi); diff --git a/crates/engine/src/sequence_tools.rs b/crates/engine/src/sequence_tools.rs index dc6c5795..a6e508dd 100644 --- a/crates/engine/src/sequence_tools.rs +++ b/crates/engine/src/sequence_tools.rs @@ -735,11 +735,12 @@ pub(crate) fn wants_new_track(p: &Value) -> bool { pub(crate) fn place_on_new_tracks(s: &mut Session, p: &Value) -> Result { let cmd = "timeline.place"; let item = crate::commands::item_p(p, "item").ok_or_else(|| bad(cmd, "need `item`"))?; - let pi = s.project.item(item).ok_or_else(|| bad(cmd, "no such item"))?; + s.project.item(item).ok_or_else(|| bad(cmd, "no such item"))?; + let (video, audio) = s.project.source_streams(item); let is_new = |k: &str| p.get(k).and_then(Value::as_str).is_some_and(|v| v.eq_ignore_ascii_case("new")); let enabled = |k: &str| p.get(k).and_then(Value::as_bool).unwrap_or(true); - let new_v = is_new("track") && pi.has_video() && enabled("video"); - let new_a = is_new("audioTrack") && pi.has_audio() && enabled("audio"); + let new_v = is_new("track") && video && enabled("video"); + let new_a = is_new("audioTrack") && audio && enabled("audio"); if !new_v && !new_a { return Err(bad(cmd, "no new track to add: the item has no picture or sound for it")); } diff --git a/crates/engine/src/source_placement_tests.rs b/crates/engine/src/source_placement_tests.rs index 0fb56556..8e85f7f2 100644 --- a/crates/engine/src/source_placement_tests.rs +++ b/crates/engine/src/source_placement_tests.rs @@ -1,5 +1,5 @@ use super::*; -use filmcraft_project::ItemId; +use filmcraft_project::{ItemId, ItemKind}; use filmcraft_time::{Tick, TimeRange}; use serde_json::json; @@ -83,6 +83,107 @@ fn audio_only_source_drops_only_sound() { assert_eq!(s.active_sequence().unwrap().audio_tracks[1].items.len(), 1); } +fn subclip_session(video: bool, audio: bool) -> (Session, ItemId) { + let mut s = Session::default(); + s.execute("file.newSequence", json!({"name":"Subclip test","fps":24})).unwrap(); + let parent = + s.execute("file.newOfflineFile", json!({"name":"Parent","video":video,"audio":audio,"fps":24,"seconds":10})).unwrap()["item"].as_u64().unwrap(); + let sub = s.execute("clip.makeSubclip", json!({"item":parent,"startFrame":10,"endFrame":60})).unwrap()["item"].as_u64().unwrap(); + (s, ItemId(sub)) +} + +fn assert_subclip_placement(s: &Session, item: ItemId, expected: (usize, usize)) { + assert_eq!(counts(s), expected); + let q = s.active_sequence().unwrap(); + let ItemKind::Subclip { range, .. } = &s.project.item(item).unwrap().kind else { panic!("expected a subclip") }; + for clip in q.all_tracks().flat_map(|t| t.items.iter()) { + assert_eq!(clip.item, item); + assert_eq!(clip.source_in, range.start); + assert_eq!(clip.duration, q.settings.frame_rate.snap_nearest(range.duration)); + } + if expected == (1, 1) { + let video = &q.video_tracks[0].items[0]; + let audio = &q.audio_tracks[0].items[0]; + assert!(video.link.is_some()); + assert_eq!(video.link, audio.link); + } +} + +#[test] +fn subclip_placement_keeps_parent_streams_range_and_history() { + for (video, audio, expected) in [(true, true, (1, 1)), (false, true, (0, 1)), (true, false, (1, 0))] { + let (mut s, sub) = subclip_session(video, audio); + let before = s.project.to_json(); + s.execute("timeline.place", json!({"item":sub.0,"track":"V1","audioTrack":"A1","time":0,"video":true,"audio":true})).unwrap(); + assert_subclip_placement(&s, sub, expected); + let placed = s.project.to_json(); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), before); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), placed); + s.project = std::sync::Arc::new(Project::from_json(&placed).unwrap()); + assert_subclip_placement(&s, sub, expected); + } +} + +#[test] +fn subclip_audio_can_be_placed_on_an_audio_track() { + for video in [false, true] { + let (mut s, sub) = subclip_session(video, true); + s.execute("timeline.place", json!({"item":sub.0,"track":"A1","time":0,"video":false,"audio":true})).unwrap(); + assert_subclip_placement(&s, sub, (0, 1)); + } + let (mut s, sub) = subclip_session(true, false); + let before = s.project.to_json(); + assert!(s.execute("timeline.place", json!({"item":sub.0,"track":"A1","time":0,"video":false,"audio":true})).is_err()); + assert_eq!(s.project.to_json(), before); +} + +#[test] +fn source_insert_and_overwrite_keep_subclip_sound() { + for command in ["source.insert", "source.overwrite"] { + for (video, audio, expected) in [(true, true, (1, 1)), (false, true, (0, 1)), (true, false, (1, 0))] { + let (mut s, sub) = subclip_session(video, audio); + s.execute("source.open", json!({"item":sub.0})).unwrap(); + s.execute(command, json!({})).unwrap(); + assert_subclip_placement(&s, sub, expected); + } + } +} + +#[test] +fn subclip_placement_can_add_tracks_for_parent_streams() { + for (video, audio, expected) in [(true, true, (1, 1)), (false, true, (0, 1)), (true, false, (1, 0))] { + let (mut s, sub) = subclip_session(video, audio); + let before = s.project.to_json(); + s.execute("timeline.place", json!({"item":sub.0,"track":"new","audioTrack":"new","time":0})).unwrap(); + assert_eq!(counts(&s), expected); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.project.to_json(), before); + } +} + +#[test] +fn subclip_placement_keeps_parent_channel_map_and_all_audio_streams() { + let (mut s, sub) = subclip_session(true, true); + let parent = s.project.resolve_media(sub).unwrap().0; + let m = std::sync::Arc::make_mut(&mut s.project).item_mut(parent).unwrap().as_media_mut().unwrap(); + m.info.audio_streams.push(m.info.audio_streams[0].clone()); + m.interpret.audio_channels = Some(filmcraft_project::AudioChannelMap { format: filmcraft_project::AudioChannels::Mono, clips: vec![vec![1], vec![0]] }); + s.execute("timeline.place", json!({"item":sub.0,"track":"V1","audioTrack":"A1","time":0})).unwrap(); + assert_eq!(counts(&s), (1, 3)); + let q = s.active_sequence().unwrap(); + let link = q.video_tracks[0].items[0].link; + assert!(link.is_some()); + for (index, (stream, channels)) in [(0, vec![1]), (0, vec![0]), (1, vec![])].into_iter().enumerate() { + let clip = &q.audio_tracks[index].items[0]; + assert_eq!(clip.item, sub); + assert_eq!(clip.audio_stream, stream); + assert_eq!(clip.source_channels, channels); + assert_eq!(clip.link, link); + } +} + #[test] fn malformed_source_spans_are_bounded() { let s = setup(); diff --git a/crates/project/src/lib.rs b/crates/project/src/lib.rs index 1291ca05..fb55786d 100644 --- a/crates/project/src/lib.rs +++ b/crates/project/src/lib.rs @@ -1595,9 +1595,10 @@ impl Project { // a multi-camera source sequence edits in as a multi-camera clip showing its first angle let multicam = it.as_sequence().and_then(|q| q.multicam.as_ref()).map(|m| MulticamSel { enabled: true, angle: m.first_video_angle().unwrap_or(0) as u32 }); - let source_channels = match (kind, &it.kind) { - (TrackKind::Audio, ItemKind::Media(m)) => m.interpret.audio_channels.as_ref().and_then(|a| a.clips.first().cloned()).unwrap_or_default(), - _ => Vec::new(), + let source_channels = if kind == TrackKind::Audio { + self.resolve_media(item).and_then(|(_, m, _)| m.interpret.audio_channels.as_ref()).and_then(|a| a.clips.first().cloned()).unwrap_or_default() + } else { + Vec::new() }; let id = ClipId(self.alloc_id()); let dur = seq_rate.snap_nearest(source.duration).max(seq_rate.frame_duration()); @@ -1671,6 +1672,16 @@ fn resolve_auto_points_sized(e: &mut EffectInstance, frame: (u32, u32), source: } impl Project { + /// Available picture and sound, resolving a subclip to its parent media. Missing parents, + /// cycles and overly deep subclip chains carry neither stream. + pub fn source_streams(&self, item: ItemId) -> (bool, bool) { + let Some(pi) = self.item(item) else { return (false, false) }; + match &pi.kind { + ItemKind::Subclip { .. } => self.resolve_media(item).map_or((false, false), |(_, m, _)| (m.info.has_video(), m.info.has_audio())), + _ => (pi.has_video(), pi.has_audio()), + } + } + /// Resolve media through at most sixteen items, preserving the outermost subclip range. /// Missing parents, cycles and longer chains are unavailable media. pub fn resolve_media(&self, item: ItemId) -> Option<(ItemId, &MediaClip, Option)> { @@ -1895,6 +1906,7 @@ mod tests { fn media_resolution_bounds_depth_and_preserves_the_outer_range() { let (mut p, media, _) = demo_project(); assert_eq!(p.resolve_media(media).map(|(id, _, range)| (id, range)), Some((media, None))); + let streams = p.source_streams(media); let mut parent = media; let mut last_range = TimeRange::default(); for n in 0..15 { @@ -1902,9 +1914,14 @@ mod tests { parent = p.add_item("Sub", Label::Iris, ItemKind::Subclip { parent, range: last_range, restrict_trims: false }, None); } assert_eq!(p.resolve_media(parent).map(|(id, _, range)| (id, range)), Some((media, Some(last_range)))); + assert_eq!(p.source_streams(parent), streams); let too_deep = p.add_item("Too deep", Label::Iris, ItemKind::Subclip { parent, range: last_range, restrict_trims: false }, None); assert!(p.resolve_media(too_deep).is_none()); assert!(p.resolve_media(ItemId(u64::MAX)).is_none()); + assert_eq!(p.source_streams(too_deep), (false, false)); + assert_eq!(p.source_streams(ItemId(u64::MAX)), (false, false)); + p.item_mut(parent).unwrap().kind = ItemKind::Subclip { parent, range: last_range, restrict_trims: false }; + assert_eq!(p.source_streams(parent), (false, false)); } #[test]