diff --git a/src/components/video-editor/audio/useAudioPreviewSync.test.ts b/src/components/video-editor/audio/useAudioPreviewSync.test.ts index 46e8df341..a1a2dcff2 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,6 +116,8 @@ describe("source preview playback ownership", () => { time, delay, plays, + seeking, + wasPlaying, }) => { const audio = { src: "", @@ -113,10 +126,16 @@ 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(); + }), }; vi.stubGlobal("Audio", function () { return audio; @@ -132,7 +151,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 +165,28 @@ 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(); + if (wasPlaying) audio.paused = false; 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; + harness.effects.at(-1)?.(); + await Promise.resolve(); + expect(audio.play).toHaveBeenCalledOnce(); + } + 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..aacbafda5 100644 --- a/src/components/video-editor/audio/useAudioPreviewSync.ts +++ b/src/components/video-editor/audio/useAudioPreviewSync.ts @@ -437,6 +437,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 @@ -453,7 +454,8 @@ export function useAudioPreviewSync({ const atEnd = audioDuration !== null && targetTime >= audioDuration; if (isPlaying && !isCurrentClipMuted && !beforeAudioStart && !atEnd) { void ensureSourceAudioRunning().then(() => { - if (!cancelled) audio.play().catch(() => undefined); + if (!cancelled && audio.paused && !audio.seeking) + audio.play().catch(() => undefined); }); } else if (!audio.paused) { audio.pause(); 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..8a258984e --- /dev/null +++ b/tests/ui/clip-cut-delete.spec.ts @@ -0,0 +1,36 @@ +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 }) => { + 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.25, 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.5, canvas.y + 6); + await page.getByRole("button", { name: /Split Clip/ }).click(); + await expect(clips).toHaveCount(3); + + await page.keyboard.press("Delete"); + await expect(clips).toHaveCount(2); + const firstEnd = Number(await clips.nth(0).getAttribute("data-end-ms")); + const lastStart = Number(await clips.nth(1).getAttribute("data-start-ms")); + expect(lastStart).toBe(firstEnd); + 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); + await expect + .poll(() => + page.locator('video[src*="filmstrip.mp4"]').evaluate((video: HTMLVideoElement) => video.currentTime), + ) + .toBeGreaterThan((firstEnd + 500) / 1_000); +});