diff --git a/crates/vedit-cli/src/diff/render.rs b/crates/vedit-cli/src/diff/render.rs index 67f78b6..496173b 100644 --- a/crates/vedit-cli/src/diff/render.rs +++ b/crates/vedit-cli/src/diff/render.rs @@ -144,6 +144,36 @@ fn mirrors(a: &Change, b: &Change) -> bool { .. }, ) => ta != tb && neighbor_names_match(ba1, bb1) && neighbor_names_match(ba2, bb2), + ( + TransitionChanged { + between_before: ba1, + between_after: ba2, + before_name: bna, + after_name: ana, + before_duration: bda, + after_duration: ada, + track: ta, + .. + }, + TransitionChanged { + between_before: bb1, + between_after: bb2, + before_name: bnb, + after_name: anb, + before_duration: bdb, + after_duration: adb, + track: tb, + .. + }, + ) => { + ta != tb + && neighbor_names_match(ba1, bb1) + && neighbor_names_match(ba2, bb2) + && bna == bnb + && ana == anb + && bda == bdb + && ada == adb + } _ => false, } } @@ -213,8 +243,8 @@ fn render_one(change: &Change, synced: bool) -> String { } => format!( " Effects on \"{}\" changed ({} → {}){}", clip_label(clip), - before, - after, + effect_summary(before), + effect_summary(after), suffix ), Change::Replaced { @@ -248,9 +278,49 @@ fn render_one(change: &Change, synced: bool) -> String { render_transition_removed(between_before, between_after, name), suffix ), + Change::TransitionChanged { + between_before, + between_after, + before_name, + after_name, + before_duration, + after_duration, + .. + } => format!( + "{}{}", + render_transition_changed( + between_before, + between_after, + before_name, + after_name, + before_duration, + after_duration + ), + suffix + ), } } +fn effect_summary(effects: &[vedit_core::model::Effect]) -> String { + if effects.is_empty() { + return "none".to_string(); + } + effects + .iter() + .map(|effect| { + if effect.name.is_empty() { + effect + .effect_name + .clone() + .unwrap_or_else(|| "unnamed".to_string()) + } else { + effect.name.clone() + } + }) + .collect::>() + .join(", ") +} + fn render_replaced( clip: &ClipRef, before_media: &Option, @@ -367,6 +437,37 @@ fn render_transition_removed( format!(" Removed {} {}", label, endpoints) } +fn render_transition_changed( + before: &Option, + after: &Option, + before_name: &str, + after_name: &str, + before_duration: &Option, + after_duration: &Option, +) -> String { + let left = before.as_ref().map(clip_label).unwrap_or("start"); + let right = after.as_ref().map(clip_label).unwrap_or("end"); + format!( + " Changed transition between \"{}\" and \"{}\" ({} {} → {} {})", + left, + right, + transition_name(before_name), + fmt_duration(before_duration), + transition_name(after_name), + fmt_duration(after_duration) + ) +} + +fn transition_name(name: &str) -> &str { + if name.is_empty() { "transition" } else { name } +} + +fn fmt_duration(duration: &Option) -> String { + duration + .map(|d| format!("({:.0} frames)", d.frames())) + .unwrap_or_else(|| "(unknown duration)".to_string()) +} + fn endpoints_phrase(a: &Option, b: &Option) -> String { match (a, b) { (Some(x), Some(y)) => format!("between \"{}\" and \"{}\"", clip_label(x), clip_label(y)), diff --git a/crates/vedit-core/src/diff.rs b/crates/vedit-core/src/diff.rs index 55d59d7..ea8e461 100644 --- a/crates/vedit-core/src/diff.rs +++ b/crates/vedit-core/src/diff.rs @@ -7,7 +7,7 @@ //! ID. That choice is what makes vedit work on OTIO from any source, //! including editors that strip third-party metadata. -use crate::model::{Clip, RationalTime, TimeRange, Timeline, Track, TrackChild, TrackKind}; +use crate::model::{Clip, Effect, RationalTime, TimeRange, Timeline, Track, TrackChild, TrackKind}; use serde::{Deserialize, Serialize}; /// One unit of change between two timelines. The shape is designed to be @@ -54,12 +54,12 @@ pub enum Change { track: String, index: usize, }, - /// Effect count on a matched clip changed. + /// Effects on a matched clip changed. EffectsChanged { clip: ClipRef, track: String, - before: usize, - after: usize, + before: Vec, + after: Vec, }, /// A clip kept its name and position but its media reference changed. /// This is the "I dropped a different take onto the same clip slot" @@ -85,6 +85,17 @@ pub enum Change { between_after: Option, name: String, }, + /// Transition remained between the same adjacent clips, but its + /// identity or duration changed. + TransitionChanged { + track: String, + between_before: Option, + between_after: Option, + before_name: String, + after_name: String, + before_duration: Option, + after_duration: Option, + }, } /// Reference to a clip suitable for human display: name + media url. @@ -281,13 +292,13 @@ fn diff_track(before: &Track, after: &Track, out: &mut Vec) { }); } - // Effect count delta. - if b_clip.effect_count != a_clip.effect_count { + // Effect identity / metadata delta. + if b_clip.effects != a_clip.effects { out.push(Change::EffectsChanged { clip: a_clip.into(), track: track_name.clone(), - before: b_clip.effect_count, - after: a_clip.effect_count, + before: b_clip.effects.clone(), + after: a_clip.effects.clone(), }); } @@ -358,8 +369,34 @@ fn diff_transitions( let before_neighbor: Option = before_clips.get(*b_pos + 1).map(|(_, c)| (*c).into()); let after_neighbor: Option = after_clips.get(*a_pos + 1).map(|(_, c)| (*c).into()); - let this_clip: ClipRef = before_clips[*b_pos].1.into(); - let _ = this_clip; // not used currently; kept for symmetry + let same_right_neighbor = same_transition_right_neighbor( + matches, + *b_pos, + *a_pos, + before_clips.len(), + after_clips.len(), + ); + + if !same_right_neighbor { + if let Some(t) = b_t { + out.push(Change::TransitionRemoved { + track: track_name.to_string(), + between_before: Some(before_clips[*b_pos].1.into()), + between_after: before_neighbor.clone(), + name: t.name.clone(), + }); + } + if let Some(t) = a_t { + out.push(Change::TransitionAdded { + track: track_name.to_string(), + between_before: Some(after_clips[*a_pos].1.into()), + between_after: after_neighbor.clone(), + name: t.name.clone(), + duration: t.duration, + }); + } + continue; + } match (b_t, a_t) { (None, Some(t)) => out.push(Change::TransitionAdded { @@ -375,11 +412,40 @@ fn diff_transitions( between_after: before_neighbor.clone(), name: t.name.clone(), }), + (Some(before_t), Some(after_t)) if before_t != after_t => { + out.push(Change::TransitionChanged { + track: track_name.to_string(), + between_before: Some(after_clips[*a_pos].1.into()), + between_after: after_neighbor.clone(), + before_name: before_t.name.clone(), + after_name: after_t.name.clone(), + before_duration: before_t.duration, + after_duration: after_t.duration, + }); + } _ => {} } } } +fn same_transition_right_neighbor( + matches: &[(usize, usize)], + b_pos: usize, + a_pos: usize, + before_clip_count: usize, + after_clip_count: usize, +) -> bool { + let before_next = b_pos + 1 < before_clip_count; + let after_next = a_pos + 1 < after_clip_count; + match (before_next, after_next) { + (false, false) => true, + (true, true) => matches + .iter() + .any(|(b, a)| *b == b_pos + 1 && *a == a_pos + 1), + _ => false, + } +} + /// For each clip in `clip_list` (in list order), return the transition that /// immediately follows it in the track's children, if any. fn transitions_after_each_clip( @@ -581,7 +647,12 @@ fn verb_phrase(change: &Change) -> String { after, .. } => { - format!("effects on \"{}\" {}→{}", clip.name, before, after) + format!( + "effects on \"{}\" {}→{}", + clip.name, + before.len(), + after.len() + ) } Change::Replaced { clip, .. } => format!("replaced media on \"{}\"", clip.name), Change::TransitionAdded { name, .. } => { @@ -598,6 +669,17 @@ fn verb_phrase(change: &Change) -> String { format!("removed {name}") } } + Change::TransitionChanged { + before_name, + after_name, + .. + } => { + if before_name == after_name { + format!("changed {after_name}") + } else { + format!("changed {before_name} to {after_name}") + } + } } } @@ -610,6 +692,7 @@ fn summary_phrase(changes: &[Change]) -> String { let mut effects = 0u32; let mut transitions_added = 0u32; let mut transitions_removed = 0u32; + let mut transitions_changed = 0u32; let mut tracks_added = 0u32; let mut tracks_removed = 0u32; @@ -623,6 +706,7 @@ fn summary_phrase(changes: &[Change]) -> String { Change::EffectsChanged { .. } => effects += 1, Change::TransitionAdded { .. } => transitions_added += 1, Change::TransitionRemoved { .. } => transitions_removed += 1, + Change::TransitionChanged { .. } => transitions_changed += 1, Change::TrackAdded { .. } => tracks_added += 1, Change::TrackRemoved { .. } => tracks_removed += 1, } @@ -653,6 +737,12 @@ fn summary_phrase(changes: &[Change]) -> String { "transition removed", "transitions removed", ); + push( + &mut parts, + transitions_changed, + "transition changed", + "transitions changed", + ); push(&mut parts, tracks_added, "track added", "tracks added"); push( &mut parts, diff --git a/crates/vedit-core/src/merge.rs b/crates/vedit-core/src/merge.rs index e213a4f..2ed672a 100644 --- a/crates/vedit-core/src/merge.rs +++ b/crates/vedit-core/src/merge.rs @@ -260,7 +260,7 @@ mod tests { start_time: rt(0.0), duration: rt(24.0), }), - effect_count: 0, + effects: Vec::new(), }) } diff --git a/crates/vedit-core/src/model.rs b/crates/vedit-core/src/model.rs index 3936877..87d9020 100644 --- a/crates/vedit-core/src/model.rs +++ b/crates/vedit-core/src/model.rs @@ -6,6 +6,7 @@ //! JSON on the parent object so we can write it back unchanged later. use serde::{Deserialize, Serialize}; +use serde_json::Value; #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct Timeline { @@ -43,7 +44,15 @@ pub struct Clip { pub name: String, pub media_reference: Option, pub source_range: Option, - pub effect_count: usize, + pub effects: Vec, +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct Effect { + pub name: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub effect_name: Option, + pub metadata: Value, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] diff --git a/crates/vedit-core/src/otio.rs b/crates/vedit-core/src/otio.rs index d25c70d..972aaff 100644 --- a/crates/vedit-core/src/otio.rs +++ b/crates/vedit-core/src/otio.rs @@ -4,7 +4,9 @@ //! fields are tolerated. The parser fails only when the document is //! structurally not an OTIO timeline. -use crate::model::{Clip, Gap, RationalTime, TimeRange, Timeline, Track, TrackChild, TrackKind}; +use crate::model::{ + Clip, Effect, Gap, RationalTime, TimeRange, Timeline, Track, TrackChild, TrackKind, +}; use anyhow::{Context, Result, anyhow}; use serde_json::Value; use std::path::Path; @@ -126,16 +128,38 @@ fn parse_clip(value: &Value) -> Clip { .to_string(); let source_range = map.get("source_range").and_then(parse_time_range); let media_reference = map.get("media_reference").and_then(parse_media_reference); - let effect_count = map + let effects = map .get("effects") .and_then(|e| e.as_array()) - .map(|a| a.len()) - .unwrap_or(0); + .map(|effects| effects.iter().map(parse_effect).collect()) + .unwrap_or_default(); Clip { name, media_reference, source_range, - effect_count, + effects, + } +} + +fn parse_effect(value: &Value) -> Effect { + let map = value.as_object().cloned().unwrap_or_default(); + let name = map + .get("name") + .and_then(|s| s.as_str()) + .unwrap_or("") + .to_string(); + let effect_name = map + .get("effect_name") + .and_then(|s| s.as_str()) + .map(|s| s.to_string()); + let metadata = map + .get("metadata") + .cloned() + .unwrap_or_else(|| Value::Object(Default::default())); + Effect { + name, + effect_name, + metadata, } } diff --git a/crates/vedit-core/tests/corpus.rs b/crates/vedit-core/tests/corpus.rs index 9045da5..2184687 100644 --- a/crates/vedit-core/tests/corpus.rs +++ b/crates/vedit-core/tests/corpus.rs @@ -46,16 +46,30 @@ fn clip(name: &str, media: &str, src_start: f64, src_dur: f64) -> Value { }) } +fn effect(name: &str, metadata: Value) -> Value { + json!({ + "OTIO_SCHEMA": "Effect.1", + "name": name, + "metadata": metadata, + }) +} + +fn effect_with_effect_name(effect_name: &str) -> Value { + json!({ + "OTIO_SCHEMA": "Effect.1", + "name": "", + "effect_name": effect_name, + "metadata": {}, + }) +} + fn clip_with_effects( name: &str, media: &str, src_start: f64, src_dur: f64, - effect_count: usize, + effects: Vec, ) -> Value { - let effects: Vec = (0..effect_count) - .map(|i| json!({"OTIO_SCHEMA": "Effect.1", "name": format!("e{i}"), "metadata": {}})) - .collect(); json!({ "OTIO_SCHEMA": "Clip.2", "name": name, @@ -378,7 +392,55 @@ fn case_09_effects_changed() { vec![track( "V1", "Video", - vec![clip_with_effects("a", "media://a.mov", 0.0, 24.0, 0)], + vec![clip_with_effects("a", "media://a.mov", 0.0, 24.0, vec![])], + )], + ); + let after = timeline( + "doc", + vec![track( + "V1", + "Video", + vec![clip_with_effects( + "a", + "media://a.mov", + 0.0, + 24.0, + vec![ + effect("blur", json!({"radius": 4})), + effect("color", json!({"saturation": 1.2})), + ], + )], + )], + ); + let changes = run_diff(before, after); + assert_eq!(changes.len(), 1, "{:#?}", changes); + match &changes[0] { + Change::EffectsChanged { before, after, .. } => { + assert!(before.is_empty()); + assert_eq!(after.len(), 2); + assert_eq!(after[0].name, "blur"); + assert_eq!(after[0].metadata["radius"], json!(4)); + assert_eq!(after[1].name, "color"); + assert_eq!(after[1].metadata["saturation"], json!(1.2)); + } + other => panic!("expected EffectsChanged, got {:?}", other), + } +} + +#[test] +fn case_10_effect_parameters_changed_without_count_change() { + let before = timeline( + "doc", + vec![track( + "V1", + "Video", + vec![clip_with_effects( + "a", + "media://a.mov", + 0.0, + 24.0, + vec![effect("blur", json!({"radius": 4}))], + )], )], ); let after = timeline( @@ -386,22 +448,166 @@ fn case_09_effects_changed() { vec![track( "V1", "Video", - vec![clip_with_effects("a", "media://a.mov", 0.0, 24.0, 2)], + vec![clip_with_effects( + "a", + "media://a.mov", + 0.0, + 24.0, + vec![effect("blur", json!({"radius": 12}))], + )], )], ); let changes = run_diff(before, after); assert_eq!(changes.len(), 1, "{:#?}", changes); match &changes[0] { Change::EffectsChanged { before, after, .. } => { - assert_eq!(*before, 0); - assert_eq!(*after, 2); + assert_eq!(before.len(), 1); + assert_eq!(after.len(), 1); + assert_eq!(before[0].metadata["radius"], json!(4)); + assert_eq!(after[0].metadata["radius"], json!(12)); } other => panic!("expected EffectsChanged, got {:?}", other), } } #[test] -fn case_10_track_added_and_multitrack() { +fn case_11_transition_changed() { + let before = timeline( + "doc", + vec![track( + "V1", + "Video", + vec![ + clip("a", "media://a.mov", 0.0, 24.0), + transition("crossfade", 6.0, 6.0), + clip("b", "media://b.mov", 0.0, 24.0), + ], + )], + ); + let after = timeline( + "doc", + vec![track( + "V1", + "Video", + vec![ + clip("a", "media://a.mov", 0.0, 24.0), + transition("dip to black", 12.0, 12.0), + clip("b", "media://b.mov", 0.0, 24.0), + ], + )], + ); + let changes = run_diff(before, after); + assert_eq!(changes.len(), 1, "{:#?}", changes); + match &changes[0] { + Change::TransitionChanged { + before_name, + after_name, + before_duration, + after_duration, + .. + } => { + assert_eq!(before_name, "crossfade"); + assert_eq!(after_name, "dip to black"); + assert_eq!(before_duration.unwrap().value, 12.0); + assert_eq!(after_duration.unwrap().value, 24.0); + } + other => panic!("expected TransitionChanged, got {:?}", other), + } +} + +#[test] +fn case_12_effect_name_changed_without_metadata_change() { + let before = timeline( + "doc", + vec![track( + "V1", + "Video", + vec![clip_with_effects( + "a", + "media://a.mov", + 0.0, + 24.0, + vec![effect_with_effect_name("LinearTimeWarp")], + )], + )], + ); + let after = timeline( + "doc", + vec![track( + "V1", + "Video", + vec![clip_with_effects( + "a", + "media://a.mov", + 0.0, + 24.0, + vec![effect_with_effect_name("FreezeFrame")], + )], + )], + ); + let changes = run_diff(before, after); + assert_eq!(changes.len(), 1, "{:#?}", changes); + assert!( + matches!(changes[0], Change::EffectsChanged { .. }), + "expected EffectsChanged, got {:?}", + changes[0] + ); +} + +#[test] +fn case_13_transition_retarget_reports_remove_and_add_not_changed() { + let before = timeline( + "doc", + vec![track( + "V1", + "Video", + vec![ + clip("a", "media://a.mov", 0.0, 24.0), + transition("crossfade", 6.0, 6.0), + clip("b", "media://b.mov", 0.0, 24.0), + clip("c", "media://c.mov", 0.0, 24.0), + ], + )], + ); + let after = timeline( + "doc", + vec![track( + "V1", + "Video", + vec![ + clip("a", "media://a.mov", 0.0, 24.0), + transition("dip to black", 12.0, 12.0), + clip("c", "media://c.mov", 0.0, 24.0), + clip("b", "media://b.mov", 0.0, 24.0), + ], + )], + ); + let changes = run_diff(before, after); + assert!( + !changes + .iter() + .any(|c| matches!(c, Change::TransitionChanged { .. })), + "retargeted transition should not be TransitionChanged: {:#?}", + changes + ); + assert!( + changes + .iter() + .any(|c| matches!(c, Change::TransitionRemoved { .. })), + "expected TransitionRemoved: {:#?}", + changes + ); + assert!( + changes + .iter() + .any(|c| matches!(c, Change::TransitionAdded { .. })), + "expected TransitionAdded: {:#?}", + changes + ); +} + +#[test] +fn case_14_track_added_and_multitrack() { let before = timeline( "doc", vec![track( @@ -430,7 +636,7 @@ fn case_10_track_added_and_multitrack() { } #[test] -fn case_11_no_changes() { +fn case_15_no_changes() { let same = timeline( "doc", vec![track( @@ -444,7 +650,7 @@ fn case_11_no_changes() { } #[test] -fn case_12_combined_trim_and_add() { +fn case_16_combined_trim_and_add() { let before = timeline( "doc", vec![track( diff --git a/crates/vedit-py/src/lib.rs b/crates/vedit-py/src/lib.rs index a266e86..f796336 100644 --- a/crates/vedit-py/src/lib.rs +++ b/crates/vedit-py/src/lib.rs @@ -207,7 +207,8 @@ impl From for PyChange { impl PyChange { /// The change's discriminator: "trimmed", "moved", "added", "removed", /// "replaced", "effects_changed", "transition_added", - /// "transition_removed", "track_added", "track_removed". + /// "transition_removed", "transition_changed", "track_added", + /// "track_removed". #[getter] fn op(&self) -> &'static str { match self.inner { @@ -221,6 +222,7 @@ impl PyChange { core_diff::Change::Replaced { .. } => "replaced", core_diff::Change::TransitionAdded { .. } => "transition_added", core_diff::Change::TransitionRemoved { .. } => "transition_removed", + core_diff::Change::TransitionChanged { .. } => "transition_changed", } } diff --git a/crates/vedit-py/tests/test_workflow.py b/crates/vedit-py/tests/test_workflow.py index ce3aff7..d40aa8a 100644 --- a/crates/vedit-py/tests/test_workflow.py +++ b/crates/vedit-py/tests/test_workflow.py @@ -210,6 +210,62 @@ def test_change_objects_are_iterable_and_have_op_and_dict(): assert d["op"] == c.op +def test_python_exposes_richer_effect_and_transition_changes(): + before = make_timeline("doc", 2) + after = make_timeline("doc", 2) + + before_children = before["tracks"]["children"][0]["children"] + after_children = after["tracks"]["children"][0]["children"] + + before_children[0]["effects"] = [ + { + "OTIO_SCHEMA": "Effect.1", + "name": "blur", + "metadata": {"radius": 4}, + } + ] + after_children[0]["effects"] = [ + { + "OTIO_SCHEMA": "Effect.1", + "name": "blur", + "metadata": {"radius": 12}, + } + ] + + before_children.insert( + 1, + { + "OTIO_SCHEMA": "Transition.1", + "name": "crossfade", + "in_offset": {"OTIO_SCHEMA": "RationalTime.1", "value": 6.0, "rate": 24.0}, + "out_offset": {"OTIO_SCHEMA": "RationalTime.1", "value": 6.0, "rate": 24.0}, + "metadata": {}, + }, + ) + after_children.insert( + 1, + { + "OTIO_SCHEMA": "Transition.1", + "name": "dip to black", + "in_offset": {"OTIO_SCHEMA": "RationalTime.1", "value": 12.0, "rate": 24.0}, + "out_offset": {"OTIO_SCHEMA": "RationalTime.1", "value": 12.0, "rate": 24.0}, + "metadata": {}, + }, + ) + + changes = {change.op: change.to_dict() for change in vedit.diff(before, after)} + + effect = changes["effects_changed"] + assert effect["before"][0]["metadata"]["radius"] == 4 + assert effect["after"][0]["metadata"]["radius"] == 12 + + transition = changes["transition_changed"] + assert transition["before_name"] == "crossfade" + assert transition["after_name"] == "dip to black" + assert transition["before_duration"]["value"] == 12.0 + assert transition["after_duration"]["value"] == 24.0 + + def test_python_reads_commit_made_by_cli(vedit_bin): with tempfile.TemporaryDirectory() as tmp: workdir = pathlib.Path(tmp) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index a0be5e2..5912a48 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -82,8 +82,8 @@ Output shape is the same for humans and agents — a list of structured changes: - `Added { clip, track, index }` - `Removed { clip, track, index }` - `Replaced { clip, before_media, after_media }` -- `EffectsChanged { clip, before_count, after_count }` -- `TransitionAdded` / `TransitionRemoved` +- `EffectsChanged { clip, before, after }` +- `TransitionAdded` / `TransitionRemoved` / `TransitionChanged` - `TrackAdded` / `TrackRemoved` The CLI renders these as prose; `--json` and the Python bindings hand them back as structured objects. Same engine, two surfaces. @@ -92,7 +92,7 @@ The renderer collapses video/audio mirror pairs (most edits in Resolve happen on ## Round-trip fidelity -vedit re-emits canonical OTIO when you `checkout` a commit. The recovered file is **semantically identical** to the original — every clip, range, media reference, transition, effect count, and Resolve metadata block is preserved. It is **not byte-identical**: whitespace, key ordering, and floating-point string representations get normalized. In one round-trip on a real Resolve project the file shrank ~35%, and the recovered timeline opened cleanly when re-imported. +vedit re-emits canonical OTIO when you `checkout` a commit. The recovered file is **semantically identical** to the original — every clip, range, media reference, transition, effect, and Resolve metadata block is preserved. It is **not byte-identical**: whitespace, key ordering, and floating-point string representations get normalized. In one round-trip on a real Resolve project the file shrank ~35%, and the recovered timeline opened cleanly when re-imported. Editor-internal data that doesn't appear in OTIO (color grades, render-cache hints, Fusion compositions in some cases) is never seen by vedit and isn't preserved. Treat vedit as a snapshot tool for your timeline, not a replacement for your `.drp` / `.prproj` project file.