diff --git a/src/components/video-editor/audio/useAudioPreviewSync.test.ts b/src/components/video-editor/audio/useAudioPreviewSync.test.ts index 46e8df341..0d483889c 100644 --- a/src/components/video-editor/audio/useAudioPreviewSync.test.ts +++ b/src/components/video-editor/audio/useAudioPreviewSync.test.ts @@ -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, @@ -105,7 +116,10 @@ describe("source preview playback ownership", () => { time, delay, plays, + seeking, + wasPlaying, }) => { + const seekedListeners = new Set<() => void>(); const audio = { src: "", dataset: {}, @@ -113,10 +127,22 @@ describe("source preview playback ownership", () => { 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; @@ -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, @@ -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(); + } }); }); diff --git a/src/components/video-editor/audio/useAudioPreviewSync.ts b/src/components/video-editor/audio/useAudioPreviewSync.ts index 2b3743c54..d3febb6e3 100644 --- a/src/components/video-editor/audio/useAudioPreviewSync.ts +++ b/src/components/video-editor/audio/useAudioPreviewSync.ts @@ -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; @@ -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 @@ -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(); } @@ -463,6 +471,9 @@ export function useAudioPreviewSync({ lastSourceAudioSyncTimeRef.current = currentTime; return () => { cancelled = true; + for (const { audio, resume } of seekedListeners) { + audio.removeEventListener("seeked", resume); + } }; }, [ sourceLoadVersion, diff --git a/src/components/video-editor/hooks/useClipRegionCommands.test.ts b/src/components/video-editor/hooks/useClipRegionCommands.test.ts new file mode 100644 index 000000000..9017e8e14 --- /dev/null +++ b/src/components/video-editor/hooks/useClipRegionCommands.test.ts @@ -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()), + 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 }, + ]); + }); +}); diff --git a/src/components/video-editor/hooks/useClipRegionCommands.ts b/src/components/video-editor/hooks/useClipRegionCommands.ts index b754bb630..e0c52c550 100644 --- a/src/components/video-editor/hooks/useClipRegionCommands.ts +++ b/src/components/video-editor/hooks/useClipRegionCommands.ts @@ -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( diff --git a/src/components/video-editor/videoPlayback/clipPlayback.test.ts b/src/components/video-editor/videoPlayback/clipPlayback.test.ts index 7c3eeb19c..40323799f 100644 --- a/src/components/video-editor/videoPlayback/clipPlayback.test.ts +++ b/src/components/video-editor/videoPlayback/clipPlayback.test.ts @@ -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(); @@ -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(); diff --git a/src/components/video-editor/videoPlayback/clipPlayback.ts b/src/components/video-editor/videoPlayback/clipPlayback.ts index 755b53b8c..ee05f391f 100644 --- a/src/components/video-editor/videoPlayback/clipPlayback.ts +++ b/src/components/video-editor/videoPlayback/clipPlayback.ts @@ -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(); @@ -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() { diff --git a/tests/ui/clip-cut-delete.spec.ts b/tests/ui/clip-cut-delete.spec.ts new file mode 100644 index 000000000..751a0d479 --- /dev/null +++ b/tests/ui/clip-cut-delete.spec.ts @@ -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); +});