Skip to content
Open
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
54 changes: 48 additions & 6 deletions src/components/video-editor/audio/useAudioPreviewSync.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,17 @@ describe("source preview playback ownership", () => {
delay: 0,
plays: true,
},
{
name: "source seek pending",
muted: false,
playing: true,
rate: 1,
time: 1,
delay: 0,
seeking: true,
wasPlaying: true,
plays: false,
},
{
name: "gap or muted clip",
muted: true,
Expand Down Expand Up @@ -105,18 +116,33 @@ describe("source preview playback ownership", () => {
time,
delay,
plays,
seeking,
wasPlaying,
}) => {
const seekedListeners = new Set<() => void>();
const audio = {
src: "",
dataset: {},
duration: 10,
currentTime: 0,
playbackRate: 1,
paused: true,
seeking: seeking ?? false,
volume: 1,
load: vi.fn(),
pause: vi.fn(),
play: vi.fn().mockResolvedValue(undefined),
pause: vi.fn(() => {
audio.paused = true;
}),
play: vi.fn().mockImplementation(() => {
audio.paused = false;
return Promise.resolve();
}),
addEventListener: vi.fn((event: string, listener: () => void) => {
if (event === "seeked") seekedListeners.add(listener);
}),
removeEventListener: vi.fn((event: string, listener: () => void) => {
if (event === "seeked") seekedListeners.delete(listener);
}),
};
vi.stubGlobal("Audio", function () {
return audio;
Expand All @@ -132,7 +158,7 @@ describe("source preview playback ownership", () => {
},
);
// Execute mocked effects explicitly so the asynchronous load can finish between syncs.
useAudioPreviewSync({
const params = {
audioRegions: [],
previewVolume: 1,
isPlaying: playing,
Expand All @@ -146,14 +172,30 @@ describe("source preview playback ownership", () => {
isCurrentClipMuted: muted,
getSourceTrackPreviewGain: () => 1,
onSourceFallbackLoadError: vi.fn(),
});
};
useAudioPreviewSync(params);
for (const effect of harness.effects) effect();
await Promise.resolve();
expect(harness.loaded).toHaveBeenCalledOnce();
expect(audio.play).not.toHaveBeenCalled();
harness.effects.at(-1)?.();
if (wasPlaying) audio.paused = false;
const cleanup = harness.effects.at(-1)?.();
await Promise.resolve();
expect(audio.play).toHaveBeenCalledTimes(plays ? 1 : 0);
if (plays) expect(audio.currentTime).toBeCloseTo(time - delay / 1000);
if (wasPlaying) expect(audio.pause).toHaveBeenCalled();
if (seeking) {
audio.seeking = false;
for (const listener of seekedListeners) listener();
await Promise.resolve();
expect(audio.play).toHaveBeenCalledOnce();
if (typeof cleanup === "function") cleanup();
expect(seekedListeners.size).toBe(0);
}
if (plays) {
expect(audio.currentTime).toBeCloseTo(time - delay / 1000);
harness.effects.at(-1)?.();
await Promise.resolve();
expect(audio.play).toHaveBeenCalledOnce();
}
});
});
17 changes: 14 additions & 3 deletions src/components/video-editor/audio/useAudioPreviewSync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ export function useAudioPreviewSync({
return;
}
let cancelled = false;
const seekedListeners: Array<{ audio: HTMLAudioElement; resume: () => void }> = [];
// A newly resolved source must pass the same playback checks as timeline updates.
void sourceLoadVersion;

Expand Down Expand Up @@ -437,6 +438,7 @@ export function useAudioPreviewSync({
(isPlaying && Math.abs(audio.currentTime - targetTime) > 0.9);
if (shouldSeek) {
try {
if (!audio.paused) audio.pause();
audio.currentTime = targetTime;
} catch {
// no-op
Expand All @@ -452,9 +454,15 @@ export function useAudioPreviewSync({

const atEnd = audioDuration !== null && targetTime >= audioDuration;
if (isPlaying && !isCurrentClipMuted && !beforeAudioStart && !atEnd) {
void ensureSourceAudioRunning().then(() => {
if (!cancelled) audio.play().catch(() => undefined);
});
const resume = () => {
void ensureSourceAudioRunning().then(() => {
if (!cancelled && audio.paused && !audio.seeking)
audio.play().catch(() => undefined);
});
};
audio.addEventListener("seeked", resume);
seekedListeners.push({ audio, resume });
resume();
} else if (!audio.paused) {
audio.pause();
}
Expand All @@ -463,6 +471,9 @@ export function useAudioPreviewSync({
lastSourceAudioSyncTimeRef.current = currentTime;
return () => {
cancelled = true;
for (const { audio, resume } of seekedListeners) {
audio.removeEventListener("seeked", resume);
}
};
}, [
sourceLoadVersion,
Expand Down
51 changes: 51 additions & 0 deletions src/components/video-editor/hooks/useClipRegionCommands.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { describe, expect, it, vi } from "vitest";
import type { ClipRegion } from "../types";
import { useClipRegionCommands } from "./useClipRegionCommands";

vi.mock("react", async (importOriginal) => ({
...(await importOriginal<typeof import("react")>()),
useCallback: (callback: unknown) => callback,
}));

describe("clip split selection", () => {
it("selects the middle section after two cuts so Delete removes it", () => {
let clips: ClipRegion[] = [{ id: "original", startMs: 0, endMs: 3_000, speed: 1 }];
let selectedClipId: string | null = null;
const nextClipIdRef = { current: 1 };
const setClipRegions = (value: ClipRegion[] | ((current: ClipRegion[]) => ClipRegion[])) => {
clips = typeof value === "function" ? value(clips) : value;
};
const setSelectedClipId = (value: string | null) => {
selectedClipId = value;
};
const commands = () =>
useClipRegionCommands({
clipRegions: clips,
selectedClipId,
sourceDurationMs: 3_000,
nextClipIdRef,
setClipRegions,
setSelectedClipId,
setZoomRegions: () => {},
setAnnotationRegions: () => {},
setAudioRegions: () => {},
setSelectedZoomId: () => {},
setSelectedAnnotationId: () => {},
setSelectedAudioId: () => {},
setSelectedCaptionId: () => {},
setActiveEffectSection: () => {},
t: (key) => key,
});

commands().handleClipSplit(1_000);
expect(selectedClipId).toBe(clips[0].id);
selectedClipId = null;
commands().handleClipSplit(2_000);
expect(selectedClipId).toBe(clips[1].id);
commands().handleClipDelete(selectedClipId!);
expect(clips).toMatchObject([
{ startMs: 0, endMs: 1_000 },
{ startMs: 1_000, endMs: 2_000, sourceStartMs: 2_000 },
]);
});
});
4 changes: 2 additions & 2 deletions src/components/video-editor/hooks/useClipRegionCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,9 @@ export function useClipRegionCommands({
clip.id === plan.targetId ? [plan.left, plan.right] : [clip],
),
);
if (selectedClipId === plan.targetId) setSelectedClipId(plan.left.id);
handleSelectClip(plan.left.id);
},
[clipRegions, nextClipIdRef, selectedClipId, setClipRegions, setSelectedClipId],
[clipRegions, nextClipIdRef, setClipRegions, handleSelectClip],
);

const handleClipSpanChange = useCallback(
Expand Down
33 changes: 31 additions & 2 deletions src/components/video-editor/videoPlayback/clipPlayback.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,14 @@ describe("clip timeline playback", () => {
duration: 12,
currentTime: 0,
seeking: false,
paused: true,
playbackRate: 1,
play: vi.fn(async () => {}),
pause: vi.fn(),
play: vi.fn(async () => {
video.paused = false;
}),
pause: vi.fn(() => {
video.paused = true;
}),
} as unknown as HTMLVideoElement;
const onTime = vi.fn();
const onPlaying = vi.fn();
Expand Down Expand Up @@ -75,6 +80,30 @@ describe("clip timeline playback", () => {
advance(1);
expect(onTime).toHaveBeenLastCalledWith(2.801, 3.501);
});
it("pauses source playback until a cut seek finishes", async () => {
const { video, playback } = setup([
{ id: "a", startMs: 0, endMs: 1_000, sourceStartMs: 0, speed: 1 },
{ id: "b", startMs: 1_000, endMs: 2_000, sourceStartMs: 5_000, speed: 1 },
]);
await playback.play();
const playCount = vi.mocked(video.play).mock.calls.length;
let sourceTime = 0;
Object.defineProperty(video, "currentTime", {
get: () => sourceTime,
set: (value: number) => {
sourceTime = value;
if (value === 5) Object.assign(video, { seeking: true });
},
});
video.currentTime = 1;
advance(1);
expect(video.pause).toHaveBeenCalled();
expect(video.currentTime).toBe(5);
expect(video.play).toHaveBeenCalledTimes(playCount);
Object.assign(video, { seeking: false });
advance(1);
expect(video.play).toHaveBeenCalledTimes(playCount + 1);
});
it("cannot simulate playback after the final clip is deleted", async () => {
const { video, playback, onTime } = setup([]);
await playback.play();
Expand Down
14 changes: 11 additions & 3 deletions src/components/video-editor/videoPlayback/clipPlayback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,14 @@ export function createClipPlayback({
// asynchronous seek in Chromium (especially disruptive at zero).
if (Math.abs(video.currentTime - target) > 1e-8) {
onSourceSeek?.(playing && !seek ? "cut" : "seek");
if (playing) {
playRequest++;
video.pause();
}
video.currentTime = target;
}
}
if (playing && (seek || clip !== activeClip)) playSource();
if (playing && (seek || clip !== activeClip) && !video.seeking) playSource();
} else {
playRequest++;
video.pause();
Expand Down Expand Up @@ -134,8 +138,12 @@ export function createClipPlayback({
lastTick = now;
sync();
if (!playing) return;
if (timeMs >= duration()) pause();
else request = requestAnimationFrame(tick);
if (timeMs >= duration()) {
pause();
} else {
if (activeClip && !video.seeking && video.paused) playSource();
request = requestAnimationFrame(tick);
}
};
return {
get isPlaying() {
Expand Down
53 changes: 53 additions & 0 deletions tests/ui/clip-cut-delete.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { expect, test } from "@playwright/test";
import { installDesktopBridge } from "./bridge";

test.use({ channel: process.env.RECORDLY_TEST_BROWSER_CHANNEL === "chrome" ? "chrome" : undefined });

test("two cuts select the middle clip for deletion", async ({ page }) => {
test.setTimeout(45_000);
await installDesktopBridge(page, "filmstrip.mp4");
await page.goto("/?windowType=editor");
const clips = page.locator('[data-variant="clip"]');
await expect(clips).toHaveCount(1, { timeout: 20_000 });
const firstClip = await clips.first().boundingBox();
const timeline = page.locator(".select-none.bg-editor-bg.relative.cursor-pointer.group.flex.flex-col").last();
const canvas = await timeline.boundingBox();
if (!firstClip || !canvas) throw new Error("Timeline is not visible");

await page.mouse.click(firstClip.x + firstClip.width * 0.2, canvas.y + 6);
await page.getByRole("button", { name: /Split Clip/ }).click();
await expect(clips).toHaveCount(2);
await page.mouse.click(firstClip.x + firstClip.width * 0.65, canvas.y + 6);
await page.getByRole("button", { name: /Split Clip/ }).click();
await expect(clips).toHaveCount(3);
const beforeDelete = await clips.evaluateAll((elements) =>
elements.map((element) => ({
start: Number(element.getAttribute("data-start-ms")),
end: Number(element.getAttribute("data-end-ms")),
})),
);
const [firstBefore, middleBefore, lastBefore] = beforeDelete;

await page.keyboard.press("Delete");
await expect(clips).toHaveCount(2);
await expect(clips.first()).toHaveAttribute("data-start-ms", String(firstBefore.start));
await expect(clips.first()).toHaveAttribute("data-end-ms", String(firstBefore.end));
await expect(clips.last()).toHaveAttribute("data-start-ms", String(firstBefore.end));
await expect(clips.last()).toHaveAttribute(
"data-end-ms",
String(firstBefore.end + lastBefore.end - lastBefore.start),
);
expect(middleBefore.end - middleBefore.start).toBeGreaterThan(0);
const lastClip = await clips.nth(1).boundingBox();
if (!lastClip) throw new Error("Retained clip is not visible");
await page.mouse.click(lastClip.x + Math.min(8, lastClip.width / 4), canvas.y + 6);
const video = page.locator('video[src*="filmstrip.mp4"]');
await expect
.poll(() => video.evaluate((element: HTMLVideoElement) => element.currentTime))
.toBeGreaterThan(middleBefore.end / 1000);
const timeBeforePlay = await video.evaluate((element: HTMLVideoElement) => element.currentTime);
await page.getByRole("button", { name: "Play", exact: true }).click();
await expect
.poll(() => video.evaluate((element: HTMLVideoElement) => element.currentTime))
.toBeGreaterThan(timeBeforePlay + 0.1);
});