Merge pull request #585 from webadderallorg/fix/timeline-speed-layout

fix(editor): guard timeline speed overlap

src/components/video-editor/VideoEditor.tsx

@@ -86,6 +86,7 @@ import {
 	getAspectRatioLabel,
 	getAspectRatioValue,
 } from "@/utils/aspectRatioUtils";
+import { planClipSpeedChange } from "./clipSpeedChange";
 import { ExtensionIcon } from "./ExtensionIcon";
 import { calculateMp4ExportDimensions, calculateMp4SourceDimensions } from "./exportDimensions";
 import { resolveSavingExportProgress } from "./exportProgressState";
@@ -3530,31 +3531,33 @@ export default function VideoEditor() {
 			if (!Number.isFinite(speed) || speed <= 0) {
 				return;
 			}
-			const clip = clipRegions.find((c) => c.id === selectedClipId);
-			if (!clip) return;
-			const oldSpeed = Number.isFinite(clip.speed) && clip.speed > 0 ? clip.speed : 1;
-			const sourceDurationMs = (clip.endMs - clip.startMs) * oldSpeed;
-			const newEndMs = Math.round(clip.startMs + sourceDurationMs / speed);
-			const scaleFactor = oldSpeed / speed;
+			const plan = planClipSpeedChange({
+				clipRegions,
+				zoomRegions,
+				selectedClipId,
+				speed,
+			});
+			if (!plan) return;
 
-			setClipRegions((prev) =>
-				prev.map((c) => (c.id === selectedClipId ? { ...c, speed, endMs: newEndMs } : c)),
-			);
-			// Scale zoom regions that lie within this clip proportionally
-			setZoomRegions((prev) =>
-				prev.map((zoom) => {
-					if (zoom.startMs < clip.startMs || zoom.startMs >= clip.endMs) return zoom;
-					return {
-						...zoom,
-						startMs: Math.round(
-							clip.startMs + (zoom.startMs - clip.startMs) * scaleFactor,
-						),
-						endMs: Math.round(clip.startMs + (zoom.endMs - clip.startMs) * scaleFactor),
-					};
-				}),
-			);
+			if ("blockedReason" in plan) {
+				toast.warning(
+					plan.blockedReason === "clip-overlap"
+						? t(
+								"editor.timeline.speedClipOverlap",
+								"Speed change would overlap the next clip. Move or split clips before slowing this section.",
+							)
+						: t(
+								"editor.timeline.speedZoomOverlap",
+								"Speed change would overlap another zoom. Move or delete the overlapping zoom first.",
+							),
+				);
+				return;
+			}
+
+			setClipRegions(plan.clipRegions);
+			setZoomRegions(plan.zoomRegions);
 		},
-		[selectedClipId, clipRegions],
+		[selectedClipId, clipRegions, zoomRegions, t],
 	);
 
 	const handleClipMutedChange = useCallback(

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

@@ -0,0 +1,153 @@
+import { describe, expect, it } from "vitest";
+
+import {
+	type BlockedClipSpeedChange,
+	formatClipSpeedLabel,
+	planClipSpeedChange,
+} from "./clipSpeedChange";
+
+describe("formatClipSpeedLabel", () => {
+	it("returns labels only for non-default positive speeds", () => {
+		expect(formatClipSpeedLabel(1)).toBeNull();
+		expect(formatClipSpeedLabel(0)).toBeNull();
+		expect(formatClipSpeedLabel(-1)).toBeNull();
+		expect(formatClipSpeedLabel(Number.POSITIVE_INFINITY)).toBeNull();
+		expect(formatClipSpeedLabel(Number.NaN)).toBeNull();
+		expect(formatClipSpeedLabel(0.5)).toBe("0.5x");
+		expect(formatClipSpeedLabel(2)).toBe("2x");
+	});
+});
+
+describe("planClipSpeedChange", () => {
+	it("returns null for missing clips and invalid speeds", () => {
+		const clipRegions = [{ id: "clip-1", startMs: 0, endMs: 5_000, speed: 1 }];
+
+		expect(
+			planClipSpeedChange({
+				clipRegions,
+				zoomRegions: [],
+				selectedClipId: "missing",
+				speed: 0.5,
+			}),
+		).toBeNull();
+
+		for (const speed of [0, -1, Number.NaN, Number.POSITIVE_INFINITY]) {
+			expect(
+				planClipSpeedChange({
+					clipRegions,
+					zoomRegions: [],
+					selectedClipId: "clip-1",
+					speed,
+				}),
+			).toBeNull();
+		}
+	});
+
+	it("extends an isolated clip when slowing it down", () => {
+		const result = planClipSpeedChange({
+			clipRegions: [{ id: "clip-1", startMs: 0, endMs: 5_000, speed: 1 }],
+			zoomRegions: [],
+			selectedClipId: "clip-1",
+			speed: 0.5,
+		});
+
+		expect(result).toEqual({
+			clipRegions: [{ id: "clip-1", startMs: 0, endMs: 10_000, speed: 0.5 }],
+			zoomRegions: [],
+		});
+	});
+
+	it("shortens an isolated clip when speeding it up", () => {
+		const result = planClipSpeedChange({
+			clipRegions: [{ id: "clip-1", startMs: 0, endMs: 6_000, speed: 1 }],
+			zoomRegions: [],
+			selectedClipId: "clip-1",
+			speed: 2,
+		});
+
+		expect(result).toEqual({
+			clipRegions: [{ id: "clip-1", startMs: 0, endMs: 3_000, speed: 2 }],
+			zoomRegions: [],
+		});
+	});
+
+	it("treats invalid stored clip speed as 1x", () => {
+		const result = planClipSpeedChange({
+			clipRegions: [{ id: "clip-1", startMs: 0, endMs: 4_000, speed: Number.NaN }],
+			zoomRegions: [],
+			selectedClipId: "clip-1",
+			speed: 0.5,
+		});
+
+		expect(result).toEqual({
+			clipRegions: [{ id: "clip-1", startMs: 0, endMs: 8_000, speed: 0.5 }],
+			zoomRegions: [],
+		});
+	});
+
+	it("blocks slow speed changes that would overlap the next clip", () => {
+		const result = planClipSpeedChange({
+			clipRegions: [
+				{ id: "clip-1", startMs: 0, endMs: 5_000, speed: 1 },
+				{ id: "clip-2", startMs: 5_000, endMs: 10_000, speed: 1 },
+			],
+			zoomRegions: [],
+			selectedClipId: "clip-1",
+			speed: 0.5,
+		}) as BlockedClipSpeedChange;
+
+		expect(result.blockedReason).toBe("clip-overlap");
+	});
+
+	it("scales zoom regions inside the changed clip", () => {
+		const result = planClipSpeedChange({
+			clipRegions: [{ id: "clip-1", startMs: 1_000, endMs: 5_000, speed: 1 }],
+			zoomRegions: [
+				{ id: "zoom-1", startMs: 2_000, endMs: 3_000, depth: 2, focus: { cx: 0.5, cy: 0.5 } },
+			],
+			selectedClipId: "clip-1",
+			speed: 0.5,
+		});
+
+		expect(result).toEqual({
+			clipRegions: [{ id: "clip-1", startMs: 1_000, endMs: 9_000, speed: 0.5 }],
+			zoomRegions: [
+				{ id: "zoom-1", startMs: 3_000, endMs: 5_000, depth: 2, focus: { cx: 0.5, cy: 0.5 } },
+			],
+		});
+	});
+
+	it("does not scale zoom regions that start outside the changed clip", () => {
+		const result = planClipSpeedChange({
+			clipRegions: [{ id: "clip-1", startMs: 2_000, endMs: 6_000, speed: 1 }],
+			zoomRegions: [
+				{ id: "zoom-before", startMs: 1_000, endMs: 1_500, depth: 2, focus: { cx: 0.5, cy: 0.5 } },
+				{ id: "zoom-after", startMs: 6_000, endMs: 6_500, depth: 2, focus: { cx: 0.5, cy: 0.5 } },
+			],
+			selectedClipId: "clip-1",
+			speed: 0.5,
+		});
+
+		expect(result).toEqual({
+			clipRegions: [{ id: "clip-1", startMs: 2_000, endMs: 10_000, speed: 0.5 }],
+			zoomRegions: [
+				{ id: "zoom-before", startMs: 1_000, endMs: 1_500, depth: 2, focus: { cx: 0.5, cy: 0.5 } },
+				{ id: "zoom-after", startMs: 6_000, endMs: 6_500, depth: 2, focus: { cx: 0.5, cy: 0.5 } },
+			],
+		});
+	});
+
+	it("blocks speed changes that would make scaled zooms overlap unchanged zooms", () => {
+		const result = planClipSpeedChange({
+			clipRegions: [{ id: "clip-1", startMs: 0, endMs: 5_000, speed: 1 }],
+			zoomRegions: [
+				{ id: "zoom-1", startMs: 2_000, endMs: 3_000, depth: 2, focus: { cx: 0.5, cy: 0.5 } },
+				{ id: "zoom-2", startMs: 5_500, endMs: 6_500, depth: 3, focus: { cx: 0.5, cy: 0.5 } },
+			],
+			selectedClipId: "clip-1",
+			speed: 0.5,
+		}) as BlockedClipSpeedChange;
+
+		expect(result.blockedReason).toBe("zoom-overlap");
+	});
+});

src/components/video-editor/clipSpeedChange.ts

@@ -0,0 +1,99 @@
+import type { ClipRegion, ZoomRegion } from "./types";
+
+export type ClipSpeedChangeBlockReason = "clip-overlap" | "zoom-overlap";
+
+export interface ClipSpeedChangePlan {
+	clipRegions: ClipRegion[];
+	zoomRegions: ZoomRegion[];
+}
+
+export interface BlockedClipSpeedChange {
+	blockedReason: ClipSpeedChangeBlockReason;
+}
+
+export function formatClipSpeedLabel(speed: number): string | null {
+	if (!Number.isFinite(speed) || speed <= 0 || speed === 1) {
+		return null;
+	}
+
+	return `${Number.isInteger(speed) ? speed.toFixed(0) : speed.toString()}x`;
+}
+
+function spansOverlap(
+	left: { startMs: number; endMs: number },
+	right: { startMs: number; endMs: number },
+): boolean {
+	return left.startMs < right.endMs && left.endMs > right.startMs;
+}
+
+export function planClipSpeedChange(params: {
+	clipRegions: ClipRegion[];
+	zoomRegions: ZoomRegion[];
+	selectedClipId: string;
+	speed: number;
+}): ClipSpeedChangePlan | BlockedClipSpeedChange | null {
+	const { clipRegions, zoomRegions, selectedClipId, speed } = params;
+	if (!selectedClipId || !Number.isFinite(speed) || speed <= 0) {
+		return null;
+	}
+
+	const clip = clipRegions.find((candidate) => candidate.id === selectedClipId);
+	if (!clip) {
+		return null;
+	}
+
+	const oldSpeed = Number.isFinite(clip.speed) && clip.speed > 0 ? clip.speed : 1;
+	const sourceDurationMs = Math.max(0, clip.endMs - clip.startMs) * oldSpeed;
+	const newEndMs = Math.round(clip.startMs + sourceDurationMs / speed);
+	const nextClip = clipRegions
+		.filter((candidate) => candidate.id !== selectedClipId && candidate.startMs >= clip.endMs)
+		.sort((left, right) => left.startMs - right.startMs)[0];
+
+	if (nextClip && newEndMs > nextClip.startMs) {
+		return { blockedReason: "clip-overlap" };
+	}
+
+	const scaleFactor = oldSpeed / speed;
+	const nextZoomRegions = zoomRegions.map((zoom) => {
+		if (zoom.startMs < clip.startMs || zoom.startMs >= clip.endMs) {
+			return zoom;
+		}
+
+		return {
+			...zoom,
+			startMs: Math.round(clip.startMs + (zoom.startMs - clip.startMs) * scaleFactor),
+			endMs: Math.round(clip.startMs + (zoom.endMs - clip.startMs) * scaleFactor),
+		};
+	});
+
+	const changedZoomIds = new Set(
+		nextZoomRegions
+			.filter((zoom, index) => {
+				const previous = zoomRegions[index];
+				return previous.startMs !== zoom.startMs || previous.endMs !== zoom.endMs;
+			})
+			.map((zoom) => zoom.id),
+	);
+
+	const hasZoomOverlap = nextZoomRegions.some((zoom, index) =>
+		nextZoomRegions.some(
+			(other, otherIndex) =>
+				index !== otherIndex &&
+				(changedZoomIds.has(zoom.id) || changedZoomIds.has(other.id)) &&
+				spansOverlap(zoom, other),
+		),
+	);
+
+	if (hasZoomOverlap) {
+		return { blockedReason: "zoom-overlap" };
+	}
+
+	return {
+		clipRegions: clipRegions.map((candidate) =>
+			candidate.id === selectedClipId
+				? { ...candidate, speed, endMs: newEndMs }
+				: candidate,
+		),
+		zoomRegions: nextZoomRegions,
+	};
+}

src/components/video-editor/timeline/Item.tsx

@@ -11,8 +11,9 @@ import {
 import type { Span } from "dnd-timeline";
 import { useItem } from "dnd-timeline";
 import { useMemo } from "react";
-import { cn } from "@/lib/utils";
 import { Skeleton } from "@/components/ui/skeleton";
+import { cn } from "@/lib/utils";
+import { formatClipSpeedLabel } from "../clipSpeedChange";
 import AudioWaveform from "./components/waveform/AudioWaveform";
 import type { AudioPeaksData } from "./core/timelineTypes";
 import glassStyles from "./ItemGlass.module.css";
@@ -125,6 +126,7 @@ export default function Item({
 	const isSpeed = variant === "speed";
 	const isAudio = variant === "audio";
 	const showAudioWaveform = isAudio && Boolean(waveformPeaks);
+	const clipSpeedLabel = isClip ? formatClipSpeedLabel(speedValue ?? 1) : null;
 
 	const glassClass = isZoom
 		? glassStyles.glassPurple
@@ -234,6 +236,11 @@ export default function Item({
 									<span className="text-[11px] font-semibold tracking-tight whitespace-nowrap">
 										Clip
 									</span>
+									{clipSpeedLabel && (
+										<span className="rounded-[4px] bg-black/10 px-1 text-[9px] font-bold tabular-nums text-black/65 dark:bg-white/15 dark:text-white/80">
+											{clipSpeedLabel}
+										</span>
+									)}
 								</>
 							) : isSpeed ? (
 								<>

src/components/video-editor/timeline/components/viewport/TimelineCanvas.tsx

@@ -1,31 +1,21 @@
 import { Plus } from "@phosphor-icons/react";
 import { useTimelineContext } from "dnd-timeline";
 import {
+	type MouseEvent,
+	type MouseEventHandler,
 	memo,
 	useCallback,
 	useEffect,
 	useMemo,
 	useRef,
 	useState,
-	type MouseEvent,
-	type MouseEventHandler,
 } from "react";
-import { cn } from "@/lib/utils";
 import type {
 	SourceAudioTrackSettings,
 	SourceAudioTrackWithPeaks,
 } from "@/components/video-editor/audio/audioTypes";
-import {
-	getTimelineContentMinHeightPx,
-	getTimelineRowsMinHeightPx,
-	getTimelineViewportStretchFactor,
-	TIMELINE_AXIS_HEIGHT_PX,
-} from "../../timelineLayout";
-import glassStyles from "../../ItemGlass.module.css";
-import Item from "../../Item";
-import Row from "../../Row";
+import { cn } from "@/lib/utils";
 import { CLIP_ROW_ID, SOURCE_AUDIO_ROW_ID, ZOOM_ROW_ID } from "../../core/constants";
-import type { TimelineRenderItem } from "../../core/timelineTypes";
 import {
 	getAnnotationTrackIndex,
 	getAnnotationTrackRowId,
@@ -34,10 +24,20 @@ import {
 	isAnnotationTrackRowId,
 	isAudioTrackRowId,
 } from "../../core/rows";
+import type { TimelineRenderItem } from "../../core/timelineTypes";
+import { useTimelineAudioPeaks } from "../../hooks/useTimelineAudioPeaks";
+import Item from "../../Item";
+import glassStyles from "../../ItemGlass.module.css";
+import Row from "../../Row";
+import {
+	getTimelineContentMinHeightPx,
+	getTimelineRowsMinHeightPx,
+	getTimelineViewportStretchFactor,
+	TIMELINE_AXIS_HEIGHT_PX,
+} from "../../timelineLayout";
 import TimelineAxis from "../axis/TimelineAxis";
 import ClipMarkerOverlay from "../overlays/ClipMarkerOverlay";
 import PlaybackCursor from "../playhead/PlaybackCursor";
-import { useTimelineAudioPeaks } from "../../hooks/useTimelineAudioPeaks";
 
 const HINT_CLIP = "Press C to split clip";
 const HINT_ANNOTATION = "Press A to add annotation";
@@ -386,6 +386,7 @@ const TimelineCanvasRows = memo(function TimelineCanvasRows({
 						isSelected={selectAllBlocksActive || item.id === selectedClipId}
 						onSelectId={onSelectClip}
 						variant="clip"
+						speedValue={item.speedValue}
 						isLoading={isLoading}
 						loadingLabel="Analyzing..."
 					>

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

@@ -47,6 +47,21 @@ describe("timeline model", () => {
 		expect(items.find((i) => i.id === "au1")?.label).toBe("foo");
 	});
 
+	it("exposes clip speed for non-default speed labels", () => {
+		const items = buildTimelineItems({
+			zoomRegions: [],
+			clipRegions: [{ id: "c1", startMs: 0, endMs: 8000, speed: 0.5 }],
+			annotationRegions: [],
+			audioRegions: [],
+		});
+
+		expect(items[0]).toMatchObject({
+			id: "c1",
+			label: "Clip 1 0.5x",
+			speedValue: 0.5,
+		});
+	});
+
 	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

@@ -1,10 +1,10 @@
+import { formatClipSpeedLabel } from "../../clipSpeedChange";
 import type {
 	AnnotationRegion,
 	AudioRegion,
 	ClipRegion,
 	ZoomRegion,
 } from "../../types";
-import type { TimelineRegionSpan, TimelineRenderItem } from "../core/timelineTypes";
 import { CLIP_ROW_ID, ZOOM_ROW_ID } from "../core/constants";
 import {
 	getAnnotationTrackIndex,
@@ -14,6 +14,7 @@ import {
 	isAnnotationTrackRowId,
 	isAudioTrackRowId,
 } from "../core/rows";
+import type { TimelineRegionSpan, TimelineRenderItem } from "../core/timelineTypes";
 
 export function getAnnotationLabel(region: AnnotationRegion): string {
 	if (region.type === "text") {
@@ -51,13 +52,15 @@ export function buildTimelineItems(params: {
 		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 speedLabel = formatClipSpeedLabel(speed);
 
 		return {
 			id: region.id,
 			rowId: CLIP_ROW_ID,
 			span: { start: region.startMs, end: region.endMs },
 			sourceSpan: { start: region.startMs, end: sourceEndMs },
-			label: `Clip ${index + 1}`,
+			label: speedLabel ? `Clip ${index + 1} ${speedLabel}` : `Clip ${index + 1}`,
+			speedValue: speedLabel ? speed : undefined,
 			showSourceAudio: region.showSourceAudio,
 			muted: Boolean(region.muted),
 			variant: "clip",