diff --git a/apps/desktop/src/routes/editor/context.ts b/apps/desktop/src/routes/editor/context.ts index 645385066df..d3c3cf33fc6 100644 --- a/apps/desktop/src/routes/editor/context.ts +++ b/apps/desktop/src/routes/editor/context.ts @@ -1799,13 +1799,19 @@ export const [EditorContextProvider, useBaseEditorContext] = : undefined, ); - const totalDuration = () => - project.timeline - ? clipTimelineDuration( - project.timeline.segments, - project.timeline.transitions ?? [], - ) + totalHeldDuration(holdWindows(project.timeline.textSegments)) - : props.editorInstance.recordingDuration; + const totalDuration = () => { + if (!project.timeline) return props.editorInstance.recordingDuration; + const clipsEnd = clipTimelineDuration( + project.timeline.segments, + project.timeline.transitions ?? [], + ); + return ( + clipsEnd + + totalHeldDuration( + holdWindows(project.timeline.textSegments, clipsEnd), + ) + ); + }; type State = { zoom: number; diff --git a/apps/desktop/src/routes/editor/timeline-holds.test.ts b/apps/desktop/src/routes/editor/timeline-holds.test.ts new file mode 100644 index 00000000000..b637b6c8518 --- /dev/null +++ b/apps/desktop/src/routes/editor/timeline-holds.test.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from "vitest"; +import { holdWindows, totalHeldDuration } from "./timeline-holds"; + +const fullscreen = (start: number, end: number) => ({ + start, + end, + layout: "fullscreen" as const, +}); + +describe("holdWindows", () => { + const texts = [ + fullscreen(1, 2), + fullscreen(4.2, 5), + fullscreen(6.4, 7.4), + fullscreen(9, 9.5), + ]; + + it("counts every hold without the clips' end", () => { + expect(holdWindows(texts)).toEqual([ + [1, 2], + [4.2, 5], + [6.4, 7.4], + [9, 9.5], + ]); + }); + + it("leaves out holds that start after the clips have ended", () => { + const holds = holdWindows(texts, 3.2); + expect(holds).toEqual([ + [1, 2], + [4.2, 5], + ]); + expect(3.2 + totalHeldDuration(holds)).toBeCloseTo(5); + expect(holdWindows(texts, 2)).toEqual([[1, 2]]); + }); +}); diff --git a/apps/desktop/src/routes/editor/timeline-holds.ts b/apps/desktop/src/routes/editor/timeline-holds.ts index c3b1de010f4..80e993cd5d0 100644 --- a/apps/desktop/src/routes/editor/timeline-holds.ts +++ b/apps/desktop/src/routes/editor/timeline-holds.ts @@ -13,8 +13,12 @@ export type HoldWindow = [number, number]; export type HoldSourceSegment = Pick & Partial>; +// Pass `clipsEnd` (the clips' gapless duration) to leave out holds that start +// after the clips have ended: they hold no frame and are never reached, so +// they add nothing to the timeline, as in Rust. export function holdWindows( textSegments: readonly HoldSourceSegment[] | null | undefined, + clipsEnd?: number, ): HoldWindow[] { if (!textSegments?.length) return []; const windows = textSegments @@ -35,9 +39,21 @@ export function holdWindows( merged.push([window[0], window[1]]); } } - return merged; + if (clipsEnd === undefined) return merged; + const reachable: HoldWindow[] = []; + let held = 0; + for (const window of merged) { + if (window[0] - held > clipsEnd + CLOSING_HOLD_TOLERANCE) break; + held += window[1] - window[0]; + reachable.push(window); + } + return reachable; } +// CLOSING_HOLD_TOLERANCE in configuration.rs: a hold starting this close to +// the clips' end is a closing title. +const CLOSING_HOLD_TOLERANCE = 1e-9; + export function totalHeldDuration(holds: HoldWindow[]): number { return holds.reduce((sum, [start, end]) => sum + (end - start), 0); } diff --git a/crates/project/src/configuration.rs b/crates/project/src/configuration.rs index edda76a60be..328cc8da059 100644 --- a/crates/project/src/configuration.rs +++ b/crates/project/src/configuration.rs @@ -1970,7 +1970,11 @@ impl TimelineConfiguration { /// Output-time windows where a fullscreen text segment pauses the /// recording clock, sorted and merged. Empty for every project without /// fullscreen text — the mapping below then short-circuits to the exact - /// pre-hold arithmetic. + /// pre-hold arithmetic. A hold that starts after the clips have ended + /// (a text card left there by a clip trim or delete, which do not ripple + /// text) has no frame to hold and is never reached, so it is left out + /// and adds nothing to `duration()`; one starting exactly at the end is + /// a closing title and stays. pub fn hold_windows(&self) -> Vec<(f64, f64)> { let mut windows: Vec<(f64, f64)> = self .text_segments @@ -1990,6 +1994,15 @@ impl TimelineConfiguration { false } }); + let clips_end = self.clips_duration(); + let mut held = 0.0; + windows.retain(|(start, end)| { + if start - held > clips_end + CLOSING_HOLD_TOLERANCE { + return false; + } + held += end - start; + true + }); windows } @@ -3933,6 +3946,34 @@ mod tests { } } + #[test] + fn holds_left_past_the_end_of_the_clips_are_not_counted() { + let mut timeline = timeline_with_transitions(Vec::new()); + timeline.segments.truncate(1); + timeline.segments[0].end = 3.2; + timeline.text_segments = vec![ + fullscreen_text(1.0, 2.0), + fullscreen_text(4.2, 5.0), + fullscreen_text(6.4, 7.4), + fullscreen_text(9.0, 9.5), + ]; + + assert_eq!(timeline.hold_windows(), vec![(1.0, 2.0), (4.2, 5.0)]); + assert_eq!(timeline.duration(), 5.0); + for frame in 0..150 { + let time = f64::from(frame) / 30.0; + assert!( + timeline.get_frame_mapping(time).is_some(), + "no source at output time {time}" + ); + } + assert!(timeline.get_frame_mapping(5.0).is_none()); + + timeline.segments[0].end = 2.0; + assert_eq!(timeline.hold_windows(), vec![(1.0, 2.0)]); + assert_eq!(timeline.duration(), 3.0); + } + #[test] fn fullscreen_title_does_not_create_a_source_for_an_empty_timeline() { let mut timeline = timeline_with_transitions(Vec::new()); diff --git a/crates/rendering/src/zoom_spring.rs b/crates/rendering/src/zoom_spring.rs index 5f66c11806a..9656a8f6971 100644 --- a/crates/rendering/src/zoom_spring.rs +++ b/crates/rendering/src/zoom_spring.rs @@ -1561,7 +1561,7 @@ mod tests { |project| { project.timeline.as_mut().unwrap().text_segments.push( serde_json::from_value(serde_json::json!({ - "start": 50.0, "end": 52.0, "layout": "fullscreen" + "start": 12.0, "end": 14.0, "layout": "fullscreen" })) .unwrap(), ); diff --git a/crates/verify/expectations.toml b/crates/verify/expectations.toml index 089cead2918..eae67780f60 100644 --- a/crates/verify/expectations.toml +++ b/crates/verify/expectations.toml @@ -129,20 +129,6 @@ bound = { screen-frames = 1, delta = 1, frame = [149, 149] } reason = "During a cross-fade both clips read the same AVAssetReader decoder at two positions; a reset for one position drops the other position's last vended frame from the cache, so at 60 fps the incoming side of output frame 149 (2.483 s) shows source frame 115 instead of 116. Not the IOSurface source lifetime bug: it stays with that fix (sol-avf proof run 38762654200). Same frame in export and in offline playback. Fires in almost every run but depends on the decoder's cache state, which its 250 ms idle timeout (IDLE_READER_TIMEOUT) clears: a 300 ms pause in the request stream at 2.0, 2.42 or 2.48 s makes offline playback correct (measured on Apple silicon), and the export passed once on macos-15 (push run 38763177849). Bound: that one screen frame, one source frame stale" link = "https://github.com/CapSoftware/Cap/pull/2638" -[[xfail]] -case = "editing/mapping/duration-covered" -class = "coverage" -bound = { count = [1, 4], failed-per-mille = [95, 98] } -reason = "NEW: a fullscreen text segment (a hold) left past the end of the clips (place a text card near the end, then trim or delete clips; clip edits do not ripple text segments) still adds its length to TimelineConfiguration::duration(), but get_frame_mapping returns None from the content end on, so the hold and the time before it have no source frame. Deterministic seeds: 97 of 1000 fast-tier and 1925 of 20000 full-tier sequences fail, the ones that leave a hold past the end (failed-per-mille 97.0 and 96.3). Minimal repro: Hold at 0.92 of a 7 s clip, then trim the clip end" -link = "" - -[[xfail]] -case = "editing/export/hold-past-end" -class = "frame-count" -bound = { count = 1, missing = 30 } -reason = "NEW: same as editing/mapping/duration-covered, end to end: a 1 s fullscreen text at 5.4 s and the clip trimmed to 3.2 s give a 4.2 s timeline, but the export stops at the first unmapped frame: 96 frames instead of 126, and the text card is never rendered" -link = "" - [[xfail]] case = "playback/basic/cuts-speeds-reorder/realtime30-from25/avf" class = "stale" diff --git a/crates/verify/tests/editing.rs b/crates/verify/tests/editing.rs index c5515825789..002cfeb7f10 100644 --- a/crates/verify/tests/editing.rs +++ b/crates/verify/tests/editing.rs @@ -110,7 +110,9 @@ fn check_config( model.duration() )); } - let core = if model.holds.is_empty() { + // The core reference model leaves projects with text unchecked, holds + // past the end of the clips included. + let core = if !timeline.text_segments.iter().any(|t| t.enabled) { let (meta, truth) = edits::synthetic_meta(durations); Some(ReferenceModel::new(&meta, config, truth).expect("core model")) } else { @@ -295,9 +297,10 @@ fn mapping_all() { ); } -/// Fullscreen text holds left past the end of the clips (place a text card, -/// then trim or delete clips) still count towards `duration()`, but those -/// output times have no source frame, so renders stop early. +/// Every output time inside `duration()` maps to a source frame, including +/// after edits that leave a fullscreen text hold past the end of the clips +/// (place a text card, then trim or delete clips): such a hold is never +/// reached, so it must not count towards the duration. #[test] fn mapping_duration_covered() { mapping_case( @@ -442,8 +445,9 @@ fn same_clip_transition_export() { } /// Deterministic repro for `editing/mapping/duration-covered`: a 1 s -/// fullscreen text card at 5.4 s, then the clip trimmed to 3.2 s. The -/// timeline reports 4.2 s; the export must have as many frames. +/// fullscreen text card at 5.4 s, then the clip trimmed to 3.2 s. The card +/// is past the end, so the timeline is 3.2 s and the export must have as +/// many frames. #[test] fn hold_past_end_export() { let id = "editing/export/hold-past-end"; diff --git a/crates/verify/tests/edits/mod.rs b/crates/verify/tests/edits/mod.rs index 5f4f6448c95..a7011c4bea4 100644 --- a/crates/verify/tests/edits/mod.rs +++ b/crates/verify/tests/edits/mod.rs @@ -19,7 +19,8 @@ //! pauses for their output-time window (overlapping or touching windows //! merge), showing the frame at the hold start; the timeline grows by the //! held time. A hold starting at the clips' end (a closing title) shows -//! the last clip's final frame; a hold past it shows nothing. +//! the last clip's final frame; a hold starting past it is never reached +//! and is left out, so the timeline does not grow by it. #![allow(dead_code)] @@ -495,7 +496,20 @@ impl TimeModel { _ => holds.push(w), } } - Self { segments, holds } + let mut model = Self { + segments, + holds: Vec::new(), + }; + let clips_end = model.base_duration(); + let mut held = 0.0; + for (a, b) in holds { + if a - held > clips_end + 1e-9 { + break; + } + held += b - a; + model.holds.push((a, b)); + } + model } /// Output duration without holds.