Merge pull request #923 from webadderallorg/fix/clip-split-source-in-point

Fix clip splitting and reordering so timeline positions remain independent from source in-points.

src/components/video-editor/audio/clipAudio.ts

@@ -1,5 +1,5 @@
-import { getClipSourceEndMs, sortClipRegions } from "../types";
 import type { ClipRegion } from "../types";
+import { getClipSourceEndMs, getClipSourceStartMs, sortClipRegions } from "../types";
 
 export function getActiveClipIdAtSourceTime(
 	sourceTimeSeconds: number,
@@ -7,7 +7,7 @@ export function getActiveClipIdAtSourceTime(
 ): string | null {
 	const sourceMs = Math.round(sourceTimeSeconds * 1000);
 	const activeClip = sortClipRegions(clipRegions).find(
-		(clip) => sourceMs >= clip.startMs && sourceMs < getClipSourceEndMs(clip),
+		(clip) => sourceMs >= getClipSourceStartMs(clip) && sourceMs < getClipSourceEndMs(clip),
 	);
 	return activeClip?.id ?? null;
 }

src/components/video-editor/clipSplit.test.ts

@@ -0,0 +1,162 @@
+import { describe, expect, it } from "vitest";
+import { planClipSplit } from "./clipSplit";
+import {
+	type ClipRegion,
+	clipsToTrims,
+	getClipSourceEndMs,
+	getClipSourceStartMs,
+	mapTimelineTimeToSourceTime,
+} from "./types";
+
+function createIdFactory() {
+	let next = 1;
+	return () => `clip-${next++}`;
+}
+
+function splitAndDeleteMiddle(clip: ClipRegion, firstSplitMs: number, secondOffsetMs: number) {
+	const createId = createIdFactory();
+	const first = planClipSplit({ clipRegions: [clip], splitMs: firstSplitMs, createId });
+	if (!first) throw new Error("first split failed");
+	const second = planClipSplit({
+		clipRegions: [first.right],
+		splitMs: first.right.startMs + secondOffsetMs,
+		createId,
+	});
+	if (!second) throw new Error("second split failed");
+	return { kept: [first.left, second.right], deleted: second.left };
+}
+
+describe("planClipSplit", () => {
+	it("returns null when no clip contains the split position", () => {
+		const clips: ClipRegion[] = [{ id: "clip-1", startMs: 0, endMs: 1000, speed: 1 }];
+		expect(
+			planClipSplit({ clipRegions: clips, splitMs: 2000, createId: createIdFactory() }),
+		).toBeNull();
+		expect(
+			planClipSplit({ clipRegions: clips, splitMs: 0, createId: createIdFactory() }),
+		).toBeNull();
+		expect(
+			planClipSplit({ clipRegions: clips, splitMs: 1000, createId: createIdFactory() }),
+		).toBeNull();
+	});
+
+	it("splits a 1x clip at the playhead", () => {
+		const clip: ClipRegion = { id: "clip-1", startMs: 0, endMs: 120_000, speed: 1 };
+		const plan = planClipSplit({
+			clipRegions: [clip],
+			splitMs: 30_000,
+			createId: createIdFactory(),
+		});
+
+		expect(plan?.left).toMatchObject({ startMs: 0, endMs: 30_000 });
+		expect(plan?.right).toMatchObject({ startMs: 30_000, endMs: 120_000 });
+	});
+
+	it("anchors the right half to the source time the split maps to at non-1x speed", () => {
+		const clip: ClipRegion = { id: "clip-1", startMs: 0, endMs: 60_000, speed: 2 };
+		const plan = planClipSplit({
+			clipRegions: [clip],
+			splitMs: 10_000,
+			createId: createIdFactory(),
+		});
+		if (!plan) throw new Error("split failed");
+
+		// 10s of playback at 2x consumes 20s of source.
+		expect(getClipSourceEndMs(plan.left)).toBe(20_000);
+		expect(getClipSourceStartMs(plan.right)).toBe(20_000);
+		// The halves still cover exactly the source the original clip covered.
+		expect(getClipSourceEndMs(plan.right)).toBe(getClipSourceEndMs(clip));
+	});
+
+	it("leaves both halves where the clip already sat on the timeline", () => {
+		const clip: ClipRegion = { id: "clip-1", startMs: 0, endMs: 60_000, speed: 2 };
+		const plan = planClipSplit({
+			clipRegions: [clip],
+			splitMs: 10_000,
+			createId: createIdFactory(),
+		});
+
+		// No visual gap: the halves abut at the playhead and still end where the clip did.
+		expect(plan?.left).toMatchObject({ startMs: 0, endMs: 10_000 });
+		expect(plan?.right).toMatchObject({ startMs: 10_000, endMs: 60_000 });
+	});
+
+	it("keeps the timeline-to-source mapping continuous across the split", () => {
+		const clip: ClipRegion = { id: "clip-1", startMs: 0, endMs: 60_000, speed: 2 };
+		const plan = planClipSplit({
+			clipRegions: [clip],
+			splitMs: 10_000,
+			createId: createIdFactory(),
+		});
+		if (!plan) throw new Error("split failed");
+
+		const halves = [plan.left, plan.right];
+		for (const timelineMs of [0, 5_000, 10_000, 30_000, 60_000]) {
+			expect(mapTimelineTimeToSourceTime(timelineMs, halves)).toBe(
+				mapTimelineTimeToSourceTime(timelineMs, [clip]),
+			);
+		}
+	});
+
+	it("keeps split halves contiguous in source time so no gap is trimmed", () => {
+		const clip: ClipRegion = { id: "clip-1", startMs: 0, endMs: 48_000, speed: 2.5 };
+		const plan = planClipSplit({
+			clipRegions: [clip],
+			splitMs: 17_333,
+			createId: createIdFactory(),
+		});
+		if (!plan) throw new Error("split failed");
+
+		expect(clipsToTrims([plan.left, plan.right], getClipSourceEndMs(clip))).toEqual([]);
+	});
+
+	it("removes the source range the user cut out when the clip is sped up", () => {
+		const sourceDurationMs = 120_000;
+		const clip: ClipRegion = { id: "clip-1", startMs: 0, endMs: 60_000, speed: 2 };
+
+		// Cut the 10s-20s playback window out of a 2x clip => source 20s-40s.
+		const { kept, deleted } = splitAndDeleteMiddle(clip, 10_000, 10_000);
+
+		expect([getClipSourceStartMs(deleted), getClipSourceEndMs(deleted)]).toEqual([
+			20_000, 40_000,
+		]);
+		expect(clipsToTrims(kept, sourceDurationMs)).toEqual([
+			{ id: "trim-gap-1", startMs: 20_000, endMs: 40_000 },
+		]);
+		// Nothing else is lost: the tail of the recording is still covered.
+		expect(getClipSourceEndMs(kept[1])).toBe(sourceDurationMs);
+	});
+
+	it("removes the source range the user cut out at 1x", () => {
+		const sourceDurationMs = 120_000;
+		const clip: ClipRegion = { id: "clip-1", startMs: 0, endMs: sourceDurationMs, speed: 1 };
+
+		const { kept } = splitAndDeleteMiddle(clip, 10_000, 10_000);
+
+		expect(clipsToTrims(kept, sourceDurationMs)).toEqual([
+			{ id: "trim-gap-1", startMs: 10_000, endMs: 20_000 },
+		]);
+		expect(getClipSourceEndMs(kept[1])).toBe(sourceDurationMs);
+	});
+
+	it("carries clip settings into both halves", () => {
+		const clip: ClipRegion = {
+			id: "clip-1",
+			startMs: 0,
+			endMs: 60_000,
+			speed: 2,
+			muted: true,
+			showSourceAudio: true,
+		};
+		const plan = planClipSplit({
+			clipRegions: [clip],
+			splitMs: 10_000,
+			createId: createIdFactory(),
+		});
+
+		for (const half of [plan?.left, plan?.right]) {
+			expect(half).toMatchObject({ speed: 2, muted: true, showSourceAudio: true });
+		}
+		expect(plan?.left.id).not.toBe(plan?.right.id);
+	});
+});

src/components/video-editor/clipSplit.ts

@@ -0,0 +1,48 @@
+import { type ClipRegion, getClipSourceEndMs } from "./types";
+
+export interface ClipSplitPlan {
+	targetId: string;
+	left: ClipRegion;
+	right: ClipRegion;
+}
+
+/**
+ * Split the clip under the playhead into two clips.
+ *
+ * `splitMs` is a timeline position, so the halves stay put on the timeline and
+ * only their source in-points differ. At non-1x speed the split consumes more
+ * (or less) source than timeline, so the right half reads from
+ * `getClipSourceEndMs(left)` — otherwise it re-reads footage the left half
+ * already covers and the tail of the recording falls outside every clip.
+ */
+export function planClipSplit(params: {
+	clipRegions: ClipRegion[];
+	splitMs: number;
+	createId: () => string;
+}): ClipSplitPlan | null {
+	const { clipRegions, splitMs, createId } = params;
+	if (!Number.isFinite(splitMs)) {
+		return null;
+	}
+
+	const splitAtMs = Math.round(splitMs);
+	const target = clipRegions.find((clip) => splitAtMs > clip.startMs && splitAtMs < clip.endMs);
+	if (!target) {
+		return null;
+	}
+
+	const left: ClipRegion = {
+		...target,
+		id: createId(),
+		endMs: splitAtMs,
+	};
+	const right: ClipRegion = {
+		...target,
+		id: createId(),
+		startMs: splitAtMs,
+		endMs: target.endMs,
+		sourceStartMs: getClipSourceEndMs(left),
+	};
+
+	return { targetId: target.id, left, right };
+}

src/components/video-editor/hooks/useClipRegionCommands.ts

@@ -2,6 +2,7 @@ import type { Span } from "dnd-timeline";
 import { type Dispatch, type MutableRefObject, type SetStateAction, useCallback } from "react";
 import { toast } from "sonner";
 import { planClipSpeedChange } from "../clipSpeedChange";
+import { planClipSplit } from "../clipSplit";
 import type {
 	AnnotationRegion,
 	AudioRegion,
@@ -10,6 +11,7 @@ import type {
 	SpeedRegion,
 	ZoomRegion,
 } from "../types";
+import { getClipSourceStartMs } from "../types";
 
 type Translator = (
 	key: string,
@@ -79,19 +81,18 @@ export function useClipRegionCommands({
 
 	const handleClipSplit = useCallback(
 		(splitMs: number) => {
-			const target = clipRegions.find(
-				(clip) => splitMs > clip.startMs && splitMs < clip.endMs,
-			);
-			if (!target) return;
-			const leftId = `clip-${nextClipIdRef.current++}`;
-			const rightId = `clip-${nextClipIdRef.current++}`;
-			const splitAt = Math.round(splitMs);
-			const left: ClipRegion = { ...target, id: leftId, endMs: splitAt };
-			const right: ClipRegion = { ...target, id: rightId, startMs: splitAt };
+			const plan = planClipSplit({
+				clipRegions,
+				splitMs,
+				createId: () => `clip-${nextClipIdRef.current++}`,
+			});
+			if (!plan) return;
 			setClipRegions((current) =>
-				current.flatMap((clip) => (clip.id === target.id ? [left, right] : [clip])),
+				current.flatMap((clip) =>
+					clip.id === plan.targetId ? [plan.left, plan.right] : [clip],
+				),
 			);
-			if (selectedClipId === target.id) setSelectedClipId(leftId);
+			if (selectedClipId === plan.targetId) setSelectedClipId(plan.left.id);
 		},
 		[clipRegions, nextClipIdRef, selectedClipId, setClipRegions, setSelectedClipId],
 	);
@@ -149,9 +150,19 @@ export function useClipRegionCommands({
 			}
 
 			setClipRegions((current) =>
-				current.map((clip) =>
-					clip.id === id ? { ...clip, startMs: newStart, endMs: newEnd } : clip,
-				),
+				current.map((clip) => {
+					if (clip.id !== id) return clip;
+					const speed = Number.isFinite(clip.speed) && clip.speed > 0 ? clip.speed : 1;
+					const startDelta = newStart - clip.startMs;
+					const endDelta = newEnd - clip.endMs;
+					// A move carries its footage along; trimming the left edge skips
+					// into the source by however much source that edge covered.
+					const isMove = Math.abs(startDelta - endDelta) < 1;
+					const sourceStartMs = isMove
+						? getClipSourceStartMs(clip)
+						: Math.max(0, Math.round(getClipSourceStartMs(clip) + startDelta * speed));
+					return { ...clip, startMs: newStart, endMs: newEnd, sourceStartMs };
+				}),
 			);
 		},
 		[

src/components/video-editor/hooks/useTimelineProjection.ts

@@ -7,6 +7,7 @@ import {
 	clipsToTrims,
 	extendAutoFullTrackClip,
 	getClipSourceEndMs,
+	getClipSourceStartMs,
 	getTimelineDurationMs,
 	mapSourceTimeToTimelineTime,
 	mapTimelineTimeToSourceTime,
@@ -119,7 +120,7 @@ export function useTimelineProjection({
 			.filter(({ speed }) => speed !== 1)
 			.map((clip) => ({
 				id: `clip-speed-${clip.id}`,
-				startMs: clip.startMs,
+				startMs: getClipSourceStartMs(clip),
 				endMs: getClipSourceEndMs(clip),
 				speed: clip.speed as SpeedRegion["speed"],
 			}));

src/components/video-editor/project/useProjectLibraryController.ts

@@ -5,7 +5,7 @@ import { toFileUrl } from "../projectPersistence";
 import type { useAppearanceState } from "../state/useAppearanceState";
 import type { useProjectState } from "../state/useProjectState";
 import type { useTimelineState } from "../state/useTimelineState";
-import { getClipSourceEndMs, type SpeedRegion } from "../types";
+import { getClipSourceEndMs, getClipSourceStartMs, type SpeedRegion } from "../types";
 import type { VideoPlaybackRef } from "../VideoPlayback";
 
 type Input = {
@@ -180,7 +180,7 @@ export function useProjectLibraryController({
 							.filter((clip) => clip.speed !== 1)
 							.map((clip) => ({
 								id: `clip-speed-${clip.id}`,
-								startMs: clip.startMs,
+								startMs: getClipSourceStartMs(clip),
 								endMs: getClipSourceEndMs(clip),
 								speed: clip.speed as SpeedRegion["speed"],
 							}));

src/components/video-editor/projectPersistence.ts

@@ -537,6 +537,9 @@ export function normalizeProjectEditor(editor: Partial<ProjectEditorState>): Pro
 						id: region.id,
 						startMs,
 						endMs,
+						...(isFiniteNumber(region.sourceStartMs)
+							? { sourceStartMs: Math.max(0, Math.round(region.sourceStartMs)) }
+							: {}),
 						speed: isFiniteNumber(region.speed) ? region.speed : 1,
 						muted: typeof region.muted === "boolean" ? region.muted : false,
 						showSourceAudio:

src/components/video-editor/timeline/model/timelineModel.test.ts

@@ -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",

src/components/video-editor/timeline/model/timelineModel.ts

@@ -6,6 +6,7 @@ import type {
 	ClipRegion,
 	ZoomRegion,
 } from "../../types";
+import { getClipSourceEndMs, getClipSourceStartMs } from "../../types";
 import { CAPTION_ROW_ID, CLIP_ROW_ID, ZOOM_ROW_ID } from "../core/constants";
 import {
 	getAnnotationTrackIndex,
@@ -61,16 +62,15 @@ 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 {
 			id: region.id,
 			rowId: CLIP_ROW_ID,
 			span: { start: region.startMs, end: region.endMs },
-			sourceSpan: { start: region.startMs, end: sourceEndMs },
+			sourceSpan: { start: getClipSourceStartMs(region), end: sourceEndMs },
 			label: speedLabel ? `Clip ${index + 1} ${speedLabel}` : `Clip ${index + 1}`,
 			speedValue: speedLabel ? speed : undefined,
 			showSourceAudio: region.showSourceAudio,

src/components/video-editor/types.test.ts

@@ -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 },
+		]);
+	});
+});

src/components/video-editor/types.ts

@@ -226,17 +226,29 @@ export interface TrimRegion {
 
 export interface ClipRegion {
 	id: string;
+	/** Where the clip sits on the timeline. */
 	startMs: number;
+	/** Where the clip ends on the timeline (`startMs` + source duration / speed). */
 	endMs: number;
+	/**
+	 * Where the clip reads from in the recording. Defaults to `startMs`, which is
+	 * only the same thing while everything before it plays at 1x — splitting or
+	 * left-trimming a sped-up clip moves the source in without moving the clip.
+	 */
+	sourceStartMs?: number;
 	speed: number;
 	muted?: boolean;
 	showSourceAudio?: boolean;
 }
 
+export function getClipSourceStartMs(clip: ClipRegion): number {
+	return Number.isFinite(clip.sourceStartMs) ? (clip.sourceStartMs as number) : clip.startMs;
+}
+
 export function getClipSourceEndMs(clip: ClipRegion): number {
 	const displayDurationMs = Math.max(0, clip.endMs - clip.startMs);
 	const speed = Number.isFinite(clip.speed) && clip.speed > 0 ? clip.speed : 1;
-	return Math.round(clip.startMs + displayDurationMs * speed);
+	return Math.round(getClipSourceStartMs(clip) + displayDurationMs * speed);
 }
 
 export function getTimelineDurationMs(clips: ClipRegion[], sourceDurationMs: number): number {
@@ -271,7 +283,7 @@ function clampToNearestClipBoundary(
 		const boundaries =
 			kind === "timeline"
 				? [clip.startMs, clip.endMs]
-				: [clip.startMs, getClipSourceEndMs(clip)];
+				: [getClipSourceStartMs(clip), getClipSourceEndMs(clip)];
 
 		for (const boundary of boundaries) {
 			const distance = Math.abs(timeMs - boundary);
@@ -294,7 +306,9 @@ export function mapTimelineTimeToSourceTime(timeMs: number, clips: ClipRegion[])
 			continue;
 		}
 
-		return Math.round(clip.startMs + (roundedTimeMs - clip.startMs) * getSafeClipSpeed(clip));
+		return Math.round(
+			getClipSourceStartMs(clip) + (roundedTimeMs - clip.startMs) * getSafeClipSpeed(clip),
+		);
 	}
 
 	if (sortedClips.length === 0) {
@@ -309,12 +323,13 @@ export function mapSourceTimeToTimelineTime(timeMs: number, clips: ClipRegion[])
 	const sortedClips = sortClipRegions(clips);
 
 	for (const clip of sortedClips) {
+		const sourceStartMs = getClipSourceStartMs(clip);
 		const sourceEndMs = getClipSourceEndMs(clip);
-		if (roundedTimeMs < clip.startMs || roundedTimeMs > sourceEndMs) {
+		if (roundedTimeMs < sourceStartMs || roundedTimeMs > sourceEndMs) {
 			continue;
 		}
 
-		return Math.round(clip.startMs + (roundedTimeMs - clip.startMs) / getSafeClipSpeed(clip));
+		return Math.round(clip.startMs + (roundedTimeMs - sourceStartMs) / getSafeClipSpeed(clip));
 	}
 
 	if (sortedClips.length === 0) {
@@ -365,15 +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) {
-		if (clip.startMs > cursor) {
-			trims.push({ id: `trim-gap-${trimId++}`, startMs: cursor, endMs: clip.startMs });
+	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 });