Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions apps/desktop/src/routes/editor/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
36 changes: 36 additions & 0 deletions apps/desktop/src/routes/editor/timeline-holds.test.ts
Original file line number Diff line number Diff line change
@@ -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]]);
});
});
18 changes: 17 additions & 1 deletion apps/desktop/src/routes/editor/timeline-holds.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@ export type HoldWindow = [number, number];
export type HoldSourceSegment = Pick<TextSegment, "start" | "end"> &
Partial<Pick<TextSegment, "enabled" | "layout">>;

// 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
Expand All @@ -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);
}
Expand Down
43 changes: 42 additions & 1 deletion crates/project/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
});
Comment on lines +1997 to +2005

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Editor shows extra time

Rust now drops past-end holds, but the Tauri editor's holdWindows() still counts them. With a 3.2-second clip and a one-second card at 5.4 seconds, totalDuration() still returns 4.2 seconds while Rust returns 3.2 seconds. The player displays the longer duration and lets users seek beyond the playable content.

Update the frontend hold calculation and its callers to use the same cutoff.

Knowledge Base Used: Export and editor rendering

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/project/src/configuration.rs
Line: 1997-2005

Comment:
**Editor shows extra time**

Rust now drops past-end holds, but the Tauri editor's `holdWindows()` still counts them. With a 3.2-second clip and a one-second card at 5.4 seconds, `totalDuration()` still returns 4.2 seconds while Rust returns 3.2 seconds. The player displays the longer duration and lets users seek beyond the playable content.

Update the frontend hold calculation and its callers to use the same cutoff.

**Knowledge Base Used:** [Export and editor rendering](https://app.greptile.com/cap/-/custom-context/knowledge-base/capsoftware/cap/-/docs/export-and-editor-rendering.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

windows
}

Expand Down Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion crates/rendering/src/zoom_spring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
);
Expand Down
14 changes: 0 additions & 14 deletions crates/verify/expectations.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
16 changes: 10 additions & 6 deletions crates/verify/tests/editing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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";
Expand Down
18 changes: 16 additions & 2 deletions crates/verify/tests/edits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]

Expand Down Expand Up @@ -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.
Expand Down
Loading