mirror of
https://github.com/webadderallorg/Recordly.git
synced 2026-09-24 14:55:37 +00:00
fix(editor): keep source spans in source coordinates across clip reorders
Addresses review feedback on the source in-point split. `buildTimelineItems` still derived `sourceSpan.end` from the timeline `startMs`, so a split clip's source-audio waveform rendered the wrong range while `sourceSpan.start` was already correct. `clipsToTrims` walked clips in timeline order while treating the source in-point as monotonic. That held before, when the two were the same field, but a move now keeps its footage, so the cursor could run backwards: source spans [20,30] then [0,10] emitted overlapping gaps that trimmed away a range a clip was still using. It now walks the claimed source spans in source order and merges overlaps. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ZyJLpqdwGjGvR4yy44xWU
This commit is contained in:
co-authored by
Claude Opus 5
parent
8e5e5e2de5
commit
e3a2046830
@@ -74,6 +74,24 @@ describe("timeline model", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps a split clip's sourceSpan in source coordinates", () => {
|
||||
const items = buildTimelineItems({
|
||||
zoomRegions: [],
|
||||
// The right half of a 2x clip split at timeline 10s: it still sits at
|
||||
// 10s but reads from 20s.
|
||||
clipRegions: [
|
||||
{ id: "c1", startMs: 10_000, endMs: 60_000, sourceStartMs: 20_000, speed: 2 },
|
||||
],
|
||||
annotationRegions: [],
|
||||
audioRegions: [],
|
||||
});
|
||||
|
||||
expect(items[0]).toMatchObject({
|
||||
span: { start: 10_000, end: 60_000 },
|
||||
sourceSpan: { start: 20_000, end: 120_000 },
|
||||
});
|
||||
});
|
||||
|
||||
it("builds all variant labels for annotation and audio", () => {
|
||||
expect(getAnnotationLabel({ ...BASE_ANNOTATION, type: "text", content: " " })).toBe(
|
||||
"Empty text",
|
||||
|
||||
@@ -6,7 +6,7 @@ import type {
|
||||
ClipRegion,
|
||||
ZoomRegion,
|
||||
} from "../../types";
|
||||
import { getClipSourceStartMs } from "../../types";
|
||||
import { getClipSourceEndMs, getClipSourceStartMs } from "../../types";
|
||||
import { CAPTION_ROW_ID, CLIP_ROW_ID, ZOOM_ROW_ID } from "../core/constants";
|
||||
import {
|
||||
getAnnotationTrackIndex,
|
||||
@@ -62,9 +62,8 @@ export function buildTimelineItems(params: {
|
||||
}));
|
||||
|
||||
const clips: TimelineRenderItem[] = clipRegions.map((region, index) => {
|
||||
const displayDurationMs = Math.max(0, region.endMs - region.startMs);
|
||||
const speed = Number.isFinite(region.speed) && region.speed > 0 ? region.speed : 1;
|
||||
const sourceEndMs = region.startMs + displayDurationMs * speed;
|
||||
const sourceEndMs = getClipSourceEndMs(region);
|
||||
const speedLabel = formatClipSpeedLabel(speed);
|
||||
|
||||
return {
|
||||
|
||||
@@ -2,6 +2,8 @@ import { describe, expect, it } from "vitest";
|
||||
import { deriveNextId } from "./projectPersistence";
|
||||
|
||||
import {
|
||||
type ClipRegion,
|
||||
clipsToTrims,
|
||||
extendAutoFullTrackClip,
|
||||
findClipAtTimelineTime,
|
||||
getTimelineDurationMs,
|
||||
@@ -180,3 +182,43 @@ describe("getTimelineDurationMs", () => {
|
||||
).toBe(10_000);
|
||||
});
|
||||
});
|
||||
|
||||
describe("clipsToTrims", () => {
|
||||
it("trims the source ranges no clip covers", () => {
|
||||
const clips: ClipRegion[] = [
|
||||
{ id: "clip-1", startMs: 0, endMs: 10_000, speed: 1 },
|
||||
{ id: "clip-2", startMs: 10_000, endMs: 20_000, sourceStartMs: 30_000, speed: 1 },
|
||||
];
|
||||
|
||||
expect(clipsToTrims(clips, 60_000)).toEqual([
|
||||
{ id: "trim-gap-1", startMs: 10_000, endMs: 30_000 },
|
||||
{ id: "trim-gap-2", startMs: 40_000, endMs: 60_000 },
|
||||
]);
|
||||
});
|
||||
|
||||
it("covers source ranges that sit out of order on the timeline", () => {
|
||||
// A moved clip keeps its source in-point, so the clip that comes first on
|
||||
// the timeline can read from later in the recording.
|
||||
const clips: ClipRegion[] = [
|
||||
{ id: "clip-1", startMs: 0, endMs: 10_000, sourceStartMs: 20_000, speed: 1 },
|
||||
{ id: "clip-2", startMs: 10_000, endMs: 20_000, sourceStartMs: 0, speed: 1 },
|
||||
];
|
||||
|
||||
// Source [0,10] and [20,30] are both in use; only the gaps go.
|
||||
expect(clipsToTrims(clips, 40_000)).toEqual([
|
||||
{ id: "trim-gap-1", startMs: 10_000, endMs: 20_000 },
|
||||
{ id: "trim-gap-2", startMs: 30_000, endMs: 40_000 },
|
||||
]);
|
||||
});
|
||||
|
||||
it("merges overlapping source spans instead of trimming between them", () => {
|
||||
const clips: ClipRegion[] = [
|
||||
{ id: "clip-1", startMs: 0, endMs: 20_000, sourceStartMs: 0, speed: 1 },
|
||||
{ id: "clip-2", startMs: 20_000, endMs: 30_000, sourceStartMs: 10_000, speed: 1 },
|
||||
];
|
||||
|
||||
expect(clipsToTrims(clips, 40_000)).toEqual([
|
||||
{ id: "trim-gap-1", startMs: 20_000, endMs: 40_000 },
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -380,16 +380,24 @@ export function extendAutoFullTrackClip(
|
||||
/** Convert clip regions (kept segments) to trim regions (gaps to remove). */
|
||||
export function clipsToTrims(clips: ClipRegion[], totalDurationMs: number): TrimRegion[] {
|
||||
if (clips.length === 0) return [];
|
||||
const sorted = [...clips].sort((a, b) => a.startMs - b.startMs);
|
||||
// Clips are ordered on the timeline, but a moved clip keeps its source
|
||||
// in-point, so timeline order says nothing about source order. Walk the
|
||||
// source ranges the clips claim and trim whatever is left uncovered.
|
||||
const coveredSpans = clips
|
||||
.map((clip) => ({
|
||||
startMs: getClipSourceStartMs(clip),
|
||||
endMs: getClipSourceEndMs(clip),
|
||||
}))
|
||||
.filter((span) => span.endMs > span.startMs)
|
||||
.sort((left, right) => left.startMs - right.startMs);
|
||||
const trims: TrimRegion[] = [];
|
||||
let cursor = 0;
|
||||
let trimId = 1;
|
||||
for (const clip of sorted) {
|
||||
const sourceStartMs = getClipSourceStartMs(clip);
|
||||
if (sourceStartMs > cursor) {
|
||||
trims.push({ id: `trim-gap-${trimId++}`, startMs: cursor, endMs: sourceStartMs });
|
||||
for (const span of coveredSpans) {
|
||||
if (span.startMs > cursor) {
|
||||
trims.push({ id: `trim-gap-${trimId++}`, startMs: cursor, endMs: span.startMs });
|
||||
}
|
||||
cursor = getClipSourceEndMs(clip);
|
||||
cursor = Math.max(cursor, span.endMs);
|
||||
}
|
||||
if (cursor < totalDurationMs) {
|
||||
trims.push({ id: `trim-gap-${trimId++}`, startMs: cursor, endMs: totalDurationMs });
|
||||
|
||||
Reference in New Issue
Block a user