diff --git a/packages/studio/src/player/hooks/usePlaybackKeyboard.ts b/packages/studio/src/player/hooks/usePlaybackKeyboard.ts index 3059d1ee46..17394ecb6d 100644 --- a/packages/studio/src/player/hooks/usePlaybackKeyboard.ts +++ b/packages/studio/src/player/hooks/usePlaybackKeyboard.ts @@ -60,6 +60,7 @@ export function usePlaybackKeyboard({ const shuttle = useCallback( (direction: "forward" | "backward") => { + if (usePlayerStore.getState().playLocked) return; if (shuttleDirectionRef.current === direction) { shuttleSpeedIndexRef.current = Math.min( shuttleSpeedIndexRef.current + 1, diff --git a/packages/studio/src/player/hooks/useTimelinePlayer.playLock.test.ts b/packages/studio/src/player/hooks/useTimelinePlayer.playLock.test.ts new file mode 100644 index 0000000000..2b552488aa --- /dev/null +++ b/packages/studio/src/player/hooks/useTimelinePlayer.playLock.test.ts @@ -0,0 +1,137 @@ +// @vitest-environment happy-dom + +import { act } from "react"; +import { afterEach, expect, it, vi } from "vitest"; +import { + makeAdapterWindow, + makeFakeIframe, + renderTimelinePlayerHarness, + resetPlayerStore, +} from "./timelinePlayerTestHarness"; +import { usePlayerStore } from "../store/playerStore"; +import { scrubMusicAtSeek } from "../lib/playbackScrub"; + +vi.mock("../lib/playbackScrub", async (importOriginal) => ({ + ...(await importOriginal()), + scrubMusicAtSeek: vi.fn(), +})); + +globalThis.IS_REACT_ACT_ENVIRONMENT = true; + +afterEach(() => { + document.body.innerHTML = ""; + resetPlayerStore(); + usePlayerStore.setState({ playLocked: false, loopEnabled: false, playbackRate: 1 }); + vi.mocked(scrubMusicAtSeek).mockClear(); +}); + +function readyPlayer() { + const { api, root } = renderTimelinePlayerHarness(); + const { adapter, win } = makeAdapterWindow(); + act(() => { + api.iframeRef.current = makeFakeIframe(win); + api.onIframeLoad(); + usePlayerStore.setState({ timelineReady: true }); + }); + return { api, root, adapter }; +} + +const press = (key: string) => + act(() => { + window.dispatchEvent(new KeyboardEvent("keydown", { key, code: `Key${key.toUpperCase()}` })); + }); + +it("starts nothing while the host holds playback: Play, a playback request or play backward", () => { + const { api, root, adapter } = readyPlayer(); + act(() => usePlayerStore.getState().setPlayLocked(true)); + act(() => api.play()); + act(() => usePlayerStore.getState().requestPlayback(true)); + press("j"); + expect(adapter.play).not.toHaveBeenCalled(); + expect(usePlayerStore.getState().isPlaying).toBe(false); + act(() => root.unmount()); +}); + +it("J and L keep the chosen speed while the host holds playback", () => { + const { root } = readyPlayer(); + act(() => usePlayerStore.getState().setPlaybackRate(0.5)); + act(() => usePlayerStore.getState().setPlayLocked(true)); + press("l"); + press("j"); + expect(usePlayerStore.getState().playbackRate).toBe(0.5); + act(() => root.unmount()); +}); + +it("a looping film reaching its end while held stops instead of starting again", () => { + const frames: FrameRequestCallback[] = []; + const raf = vi.spyOn(window, "requestAnimationFrame").mockImplementation((tick) => { + frames.push(tick); + return frames.length; + }); + const { api, root, adapter } = readyPlayer(); + act(() => usePlayerStore.setState({ loopEnabled: true })); + act(() => api.play()); + // Held with the store already reading stopped, so nothing pauses the frame loop first. + act(() => usePlayerStore.setState({ playLocked: true, isPlaying: false })); + adapter.seek(30); + act(() => frames.at(-1)!(0)); + expect(adapter.play).toHaveBeenCalledTimes(1); + expect(usePlayerStore.getState().isPlaying).toBe(false); + raf.mockRestore(); + act(() => root.unmount()); +}); + +it("turning the hold on before the preview has a player still clears playing", () => { + const { root } = renderTimelinePlayerHarness(); + act(() => usePlayerStore.setState({ isPlaying: true })); + act(() => usePlayerStore.getState().setPlayLocked(true)); + expect(usePlayerStore.getState().isPlaying).toBe(false); + act(() => root.unmount()); +}); + +it("a seek that would keep playing does not resume while the host holds playback", () => { + const { api, root, adapter } = readyPlayer(); + act(() => usePlayerStore.getState().setPlayLocked(true)); + // The store still reads playing (a host that set the hold before the player caught up): only the hold stops it. + act(() => usePlayerStore.setState({ isPlaying: true })); + act(() => { + api.seek(5, { keepPlaying: true }); + }); + expect(adapter.play).not.toHaveBeenCalled(); + act(() => root.unmount()); +}); + +it("a seek while the host holds playback moves the picture without scrub audio", () => { + const { api, root, adapter } = readyPlayer(); + act(() => { + api.seek(2); + }); + expect(scrubMusicAtSeek).toHaveBeenCalledTimes(1); + act(() => usePlayerStore.getState().setPlayLocked(true)); + act(() => { + api.seek(4); + }); + expect(adapter.getTime()).toBe(4); + expect(scrubMusicAtSeek).toHaveBeenCalledTimes(1); + act(() => root.unmount()); +}); + +it("a timeline reset keeps the hold: the host owns it", () => { + act(() => usePlayerStore.getState().setPlayLocked(true)); + act(() => usePlayerStore.getState().reset()); + expect(usePlayerStore.getState().playLocked).toBe(true); +}); + +it("turning the hold on pauses a playing film; turning it off lets Play start again", () => { + const { api, root, adapter } = readyPlayer(); + act(() => api.play()); + expect(usePlayerStore.getState().isPlaying).toBe(true); + act(() => usePlayerStore.getState().setPlayLocked(true)); + expect(adapter.pause).toHaveBeenCalled(); + expect(usePlayerStore.getState().isPlaying).toBe(false); + act(() => usePlayerStore.getState().setPlayLocked(false)); + act(() => api.play()); + expect(adapter.play).toHaveBeenCalledTimes(2); + expect(usePlayerStore.getState().isPlaying).toBe(true); + act(() => root.unmount()); +}); diff --git a/packages/studio/src/player/hooks/useTimelinePlayer.ts b/packages/studio/src/player/hooks/useTimelinePlayer.ts index bdb2fb62b8..5076efefbe 100644 --- a/packages/studio/src/player/hooks/useTimelinePlayer.ts +++ b/packages/studio/src/player/hooks/useTimelinePlayer.ts @@ -200,7 +200,8 @@ export function useTimelinePlayer({ applyPreviewAudioFlags(iframeRef.current, audioMuted, audioVolume); }, []); const play = useCallback(() => { - if (!usePlayerStore.getState().timelineReady) return; + const { timelineReady, playLocked } = usePlayerStore.getState(); + if (!timelineReady || playLocked) return; stopRAFLoop(); stopReverseLoop(); stopScrubPreviewAudio(); @@ -286,9 +287,8 @@ export function useTimelinePlayer({ const pause = useCallback(() => { stopReverseLoop(); const adapter = getAdapter(); - if (!adapter) return; - adapter.pause(); - setCurrentTime(adapter.getTime()); // sync store so Split/Delete have accurate time + adapter?.pause(); + if (adapter) setCurrentTime(adapter.getTime()); // sync store so Split/Delete have accurate time setIsPlaying(false); shuttleDirectionRef.current = null; shuttleSpeedIndexRef.current = 0; @@ -306,17 +306,19 @@ export function useTimelinePlayer({ const duration = Math.max(0, adapter.getDuration()); const nextTime = clampToDuration(time, duration); const keepPlaying = options?.keepPlaying === true; + const { playLocked, isPlaying: storeWasPlaying } = usePlayerStore.getState(); const shouldResumeAfterSeek = shouldResumeForwardPlaybackAfterSeek({ keepPlaying, wasReverseShuttle, - storeWasPlaying: usePlayerStore.getState().isPlaying, + storeWasPlaying, duration, nextTime, + playLocked, }); adapter.seek(nextTime, options); publishSeek(nextTime, options); // Direct DOM updates (playhead, timecode, progress) — no re-render setCurrentTime(nextTime); // sync store so Split/Delete have accurate time - if (!shouldResumeAfterSeek && !keepPlaying) scrubMusicAtSeek(iframeRef.current, nextTime); + if (!keepPlaying && !playLocked) scrubMusicAtSeek(iframeRef.current, nextTime); if (shouldResumeAfterSeek) { stopRAFLoop(); applyPlaybackRate(usePlayerStore.getState().playbackRate); @@ -349,6 +351,13 @@ export function useTimelinePlayer({ ], ); + useEffect( + () => + usePlayerStore.subscribe((state, prev) => { + if (state.playLocked && !prev.playLocked && state.isPlaying) pause(); + }), + [pause], + ); useEffect(() => { return usePlayerStore.subscribe((state, prev) => { if (state.requestedSeekTime !== null && state.requestedSeekTime !== prev.requestedSeekTime) { diff --git a/packages/studio/src/player/hooks/useTimelinePlayerLoop.ts b/packages/studio/src/player/hooks/useTimelinePlayerLoop.ts index 306b326d38..cd1c3d853d 100644 --- a/packages/studio/src/player/hooks/useTimelinePlayerLoop.ts +++ b/packages/studio/src/player/hooks/useTimelinePlayerLoop.ts @@ -60,7 +60,7 @@ export function useTimelinePlayerLoop({ const looping = usePlayerStore.getState().loopEnabled && dur > 0; setLoopStart(looping ? loopStart : null); if (time >= loopEnd) { - if (looping) { + if (looping && !usePlayerStore.getState().playLocked) { // keepPlaying skips the adapter's implicit pause; play() below is then a no-op. adapter.seek(loopStart, { keepPlaying: true }); liveTime.notify(loopStart); diff --git a/packages/studio/src/player/lib/playbackSeek.ts b/packages/studio/src/player/lib/playbackSeek.ts index b41c870b31..f0cc313359 100644 --- a/packages/studio/src/player/lib/playbackSeek.ts +++ b/packages/studio/src/player/lib/playbackSeek.ts @@ -4,8 +4,10 @@ export function shouldResumeForwardPlaybackAfterSeek(input: { storeWasPlaying: boolean; duration: number; nextTime: number; + playLocked: boolean; }): boolean { return ( + !input.playLocked && input.keepPlaying && !input.wasReverseShuttle && input.storeWasPlaying && diff --git a/packages/studio/src/player/store/readinessSlice.ts b/packages/studio/src/player/store/readinessSlice.ts index 7582e89f1e..e769c1144b 100644 --- a/packages/studio/src/player/store/readinessSlice.ts +++ b/packages/studio/src/player/store/readinessSlice.ts @@ -13,6 +13,8 @@ export interface PlaybackReadinessSlice { previewBooted: boolean; setTimelineReady: (ready: boolean) => void; markPreviewBooted: () => void; + playLocked: boolean; + setPlayLocked: (locked: boolean) => void; /** Sets timelineReady once doc's readiness inputs settle, or immediately * if doc is null. A wait a later call supersedes never wins the race. * Waits for doc's load step first: it pauses and rewinds the preview, so a @@ -47,6 +49,8 @@ export function createPlaybackReadinessSlice( timelineReady: false, previewBooted: false, markPreviewBooted: () => set({ previewBooted: true }), + playLocked: false, + setPlayLocked: (locked) => set({ playLocked: locked }), setTimelineReady: (ready) => { timelineReadyGeneration++; set({ timelineReady: ready });