diff --git a/package.json b/package.json index e93f1a6d3..186de375b 100644 --- a/package.json +++ b/package.json @@ -33,6 +33,7 @@ "routes": "tsr generate", "test": "vp test", "test:run": "vp test run", + "test:editor-hardening": "vp test run src/features/timeline/components/timeline-content.test.tsx src/features/timeline/components/timeline-item/use-timeline-item-pointer-handlers.test.tsx src/features/timeline/hooks/shortcuts/use-clipboard-shortcuts.test.tsx src/features/timeline/hooks/shortcuts/use-playback-shortcuts.test.tsx src/features/timeline/stores/export-snapshot.test.ts src/features/export/components/export-dialog.test.tsx src/features/export/hooks/client-render-source.test.ts src/features/export/hooks/use-client-render.test.tsx src/features/preview/workers/consume-video-samples.test.ts src/features/preview/utils/media-resolver.test.ts src/features/preview/hooks/use-preview-media-resolution.test.tsx src/features/preview/components/source-composition.generation.test.tsx src/features/preview/components/video-preview.sync.test.tsx src/infrastructure/browser/blob-url-manager.test.ts", "test:preview-sync": "vp test run src/features/preview/components/video-preview.sync.test.tsx", "test:preview-sync:stress": "node scripts/preview-sync-stress.mjs --runs 20", "test:coverage": "vp test run --coverage", diff --git a/src/features/export/components/export-dialog.test.tsx b/src/features/export/components/export-dialog.test.tsx index 4b712d11d..406d51a1b 100644 --- a/src/features/export/components/export-dialog.test.tsx +++ b/src/features/export/components/export-dialog.test.tsx @@ -9,6 +9,69 @@ const mockDownloadVideo = vi.fn() const mockResetState = vi.fn() const mockGetSupportedCodecs = vi.fn<(...args: unknown[]) => Promise>() +const { mainSequence, selectedSequence, mockGetExportableSequence } = vi.hoisted(() => { + const sequence = (id: string | null, name: string, itemId: string) => { + const trackId = `track-${itemId}` + const item = { + id: itemId, + trackId, + type: 'text' as const, + from: 0, + durationInFrames: 30, + label: name, + text: name, + color: '#ffffff', + } + return { + id, + name, + tracks: [ + { + id: trackId, + name: 'V1', + kind: 'video' as const, + height: 60, + locked: false, + visible: true, + muted: false, + solo: false, + order: 0, + items: [item], + }, + ], + items: [item], + transitions: [], + keyframes: [], + fps: 30, + width: 1920, + height: 1080, + backgroundColor: '#000000', + masterBusDb: 0, + durationFrames: 30, + inPoint: null, + outPoint: null, + markers: [], + } + } + + const main = sequence(null, 'Main Timeline', 'main-title') + const selected = sequence('agent-cut', 'Agent Cut', 'agent-title') + return { + mainSequence: main, + selectedSequence: selected, + mockGetExportableSequence: vi.fn((id: string | null) => (id === selected.id ? selected : main)), + } +}) + +vi.mock('@/features/export/deps/timeline-compositions', () => ({ + getActiveExportSequenceId: () => null, + getExportableSequence: mockGetExportableSequence, + listExportableSequences: () => [ + { id: null, name: mainSequence.name }, + { id: selectedSequence.id, name: selectedSequence.name }, + ], +})) + vi.mock('../hooks/use-client-render', () => ({ useClientRender: () => ({ isExporting: false, @@ -115,4 +178,22 @@ describe('ExportDialog', () => { const h265Option = await screen.findByRole('option', { name: /H\.265/i }) expect(h265Option).toHaveAttribute('data-disabled') }) + + it('passes the selected sequence snapshot to direct export', async () => { + mockGetSupportedCodecs.mockResolvedValue(['avc']) + mockStartExport.mockResolvedValue(undefined) + + render( {}} />) + + fireEvent.keyDown(screen.getByLabelText('Sequence'), { key: 'ArrowDown' }) + fireEvent.click(await screen.findByRole('option', { name: selectedSequence.name })) + + const exportButton = screen.getByRole('button', { name: 'Export Video' }) + await waitFor(() => expect(exportButton).not.toBeDisabled()) + fireEvent.click(exportButton) + + await waitFor(() => { + expect(mockStartExport).toHaveBeenCalledWith(expect.any(Object), selectedSequence) + }) + }) }) diff --git a/src/features/export/components/export-dialog.tsx b/src/features/export/components/export-dialog.tsx index 1b0332335..e64ce7b7b 100644 --- a/src/features/export/components/export-dialog.tsx +++ b/src/features/export/components/export-dialog.tsx @@ -394,8 +394,7 @@ export function ExportDialog({ open, onClose, onOpenRenderQueue }: ExportDialogP const reversedClipIds = new Set( items .filter( - (item) => - (item.type === 'video' || item.type === 'audio') && item.isReversed === true, + (item) => (item.type === 'video' || item.type === 'audio') && item.isReversed === true, ) .map((item) => item.id), ) @@ -643,8 +642,9 @@ export function ExportDialog({ open, onClose, onOpenRenderQueue }: ExportDialogP // Start export const handleStartExport = async () => { + const seq = captureSelection() setView('progress') - await startExport(buildExtendedSettings()) + await startExport(buildExtendedSettings(), seq) } // The active render range for a sequence (whole timeline unless in/out set). @@ -1574,8 +1574,7 @@ export function ExportDialog({ open, onClose, onOpenRenderQueue }: ExportDialogP
- {status === 'preparing' && - (progressMessage ?? t('export.progress.preparing'))} + {status === 'preparing' && (progressMessage ?? t('export.progress.preparing'))} {status === 'rendering' && t('export.progress.rendering')} {status === 'encoding' && t('export.progress.encoding')} {status === 'finalizing' && t('export.progress.finalizing')} diff --git a/src/features/export/hooks/client-render-source.test.ts b/src/features/export/hooks/client-render-source.test.ts new file mode 100644 index 000000000..0105c64b6 --- /dev/null +++ b/src/features/export/hooks/client-render-source.test.ts @@ -0,0 +1,51 @@ +// @vitest-environment node + +import { describe, expect, it } from 'vite-plus/test' +import type { ExportableSequence } from '@/features/export/deps/timeline-compositions' +import { resolveClientRenderSource } from './client-render-source' + +function makeSequence(overrides: Partial = {}): ExportableSequence { + return { + id: 'selected', + name: 'Selected', + tracks: [], + items: [], + transitions: [], + keyframes: [], + fps: 24, + width: 1280, + height: 720, + masterBusDb: -3, + durationFrames: 0, + inPoint: null, + outPoint: null, + markers: [], + ...overrides, + } +} + +describe('resolveClientRenderSource', () => { + it('preserves an explicitly unset selected-sequence range and EQ', () => { + const sequence = makeSequence({ busAudioEq: undefined, backgroundColor: undefined }) + const result = resolveClientRenderSource( + sequence, + makeSequence({ id: null, inPoint: 30, outPoint: 90 }), + { + busAudioEq: { enabled: true, lowGainDb: 4, midGainDb: 2, highGainDb: 3 }, + masterBusDb: 6, + }, + { width: 1920, height: 1080, backgroundColor: '#ff0000' }, + ) + + expect(result).toMatchObject({ + fps: 24, + inPoint: null, + outPoint: null, + busAudioEq: undefined, + masterBusDb: -3, + backgroundColor: undefined, + width: 1280, + height: 720, + }) + }) +}) diff --git a/src/features/export/hooks/client-render-source.ts b/src/features/export/hooks/client-render-source.ts new file mode 100644 index 000000000..23a9bde74 --- /dev/null +++ b/src/features/export/hooks/client-render-source.ts @@ -0,0 +1,43 @@ +import type { ExportableSequence } from '@/features/export/deps/timeline-compositions' +import { DEFAULT_PROJECT_HEIGHT, DEFAULT_PROJECT_WIDTH } from '@/shared/projects/defaults' + +type TimelineRenderSource = Pick< + ExportableSequence, + 'tracks' | 'items' | 'transitions' | 'fps' | 'inPoint' | 'outPoint' | 'keyframes' +> + +type PlaybackRenderSource = Pick + +interface ProjectRenderMetadata { + width?: number + height?: number + backgroundColor?: string +} + +/** + * Select one complete render source. Once a sequence snapshot is supplied, + * its nullable/optional values are authoritative too: an unset range or EQ + * must not inherit state from whichever timeline happens to be active. + */ +export function resolveClientRenderSource( + sequence: ExportableSequence | undefined, + timeline: TimelineRenderSource, + playback: PlaybackRenderSource, + projectMetadata: ProjectRenderMetadata | undefined, +) { + const source = sequence ?? timeline + return { + tracks: source.tracks, + items: source.items, + transitions: source.transitions, + fps: source.fps, + inPoint: source.inPoint, + outPoint: source.outPoint, + keyframes: source.keyframes, + busAudioEq: sequence ? sequence.busAudioEq : playback.busAudioEq, + masterBusDb: sequence ? sequence.masterBusDb : playback.masterBusDb, + backgroundColor: sequence ? sequence.backgroundColor : projectMetadata?.backgroundColor, + width: sequence?.width ?? projectMetadata?.width ?? DEFAULT_PROJECT_WIDTH, + height: sequence?.height ?? projectMetadata?.height ?? DEFAULT_PROJECT_HEIGHT, + } +} diff --git a/src/features/export/hooks/use-client-render.test.tsx b/src/features/export/hooks/use-client-render.test.tsx new file mode 100644 index 000000000..df351144d --- /dev/null +++ b/src/features/export/hooks/use-client-render.test.tsx @@ -0,0 +1,333 @@ +import { act, renderHook } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vite-plus/test' +import { StrictMode, type PropsWithChildren } from 'react' + +const mocks = vi.hoisted(() => ({ + resolveMediaUrls: vi.fn(), + runRender: vi.fn(), + resolveClientSettings: vi.fn(), + mapRequestedClientSettings: vi.fn(), + trySmartCopyExport: vi.fn(), + convertTimelineToComposition: vi.fn(), + buildTranscriptSubtitleCues: vi.fn(), + releaseTemporaryExportOutput: vi.fn(), + setResult: vi.fn(), +})) + +vi.mock('@/features/export/deps/media-library', () => ({ + resolveMediaUrls: mocks.resolveMediaUrls, +})) +vi.mock('../utils/smart-copy', () => ({ trySmartCopyExport: mocks.trySmartCopyExport })) +vi.mock('../utils/render-pipeline', () => ({ + isExtendedSettings: (settings: unknown) => + typeof settings === 'object' && settings !== null && 'mode' in settings, + mapRequestedClientSettings: mocks.mapRequestedClientSettings, + resolveClientSettings: mocks.resolveClientSettings, + runRender: mocks.runRender, +})) +vi.mock('../utils/timeline-to-composition', () => ({ + convertTimelineToComposition: mocks.convertTimelineToComposition, +})) +vi.mock('../utils/embedded-subtitle-export', () => ({ + buildTranscriptSubtitleCues: mocks.buildTranscriptSubtitleCues, +})) +vi.mock('@/shared/utils/subtitles', () => ({ serializeSrt: vi.fn(() => '') })) +vi.mock('../utils/export-output-target', () => ({ + releaseTemporaryExportOutput: mocks.releaseTemporaryExportOutput, +})) +vi.mock('../utils/client-renderer', () => ({ + formatBytes: (bytes: number) => `${bytes} bytes`, + estimateFileSize: vi.fn(() => 1), + getSupportedCodecs: vi.fn(async () => []), + getVideoBitrateForQuality: vi.fn(() => 1), + mapToClientSettings: vi.fn(() => ({})), +})) +vi.mock('@/features/export/deps/timeline', () => ({ + useTimelineStore: { + getState: () => ({ + tracks: [], + items: [], + transitions: [], + fps: 30, + inPoint: null, + outPoint: null, + keyframes: [], + busAudioEq: [], + masterBusDb: 0, + backgroundColor: '#000', + width: 1920, + height: 1080, + }), + }, +})) +vi.mock('@/features/export/deps/projects', () => ({ + useProjectStore: { getState: () => ({ currentProject: null }) }, +})) +vi.mock('@/shared/state/playback', () => ({ + usePlaybackStore: { getState: () => ({}) }, +})) +vi.mock('./client-render-source', () => ({ + resolveClientRenderSource: (_sequence: unknown, state: unknown) => state, +})) +vi.mock('@/shared/projects/defaults', () => ({ + DEFAULT_PROJECT_WIDTH: 1920, + DEFAULT_PROJECT_HEIGHT: 1080, +})) +vi.mock('@/shared/logging/logger', () => ({ + createLogger: () => ({ + startEvent: () => ({ set: vi.fn(), merge: vi.fn(), success: vi.fn(), failure: vi.fn() }), + warn: vi.fn(), + event: vi.fn(), + }), + createOperationId: () => 'test-op', +})) + +import { useClientRender } from './use-client-render' + +const settings = { quality: 'medium', resolution: { width: 640, height: 360 } } as Record< + string, + unknown +> +const renderedResult = { + blob: new Blob(['encoded']), + fileSize: 7, + duration: 1, + mimeType: 'video/mp4', +} + +function deferred() { + let resolve!: (value: T) => void + let reject!: (error: unknown) => void + const promise = new Promise((res, rej) => { + resolve = res + reject = rej + }) + return { promise, resolve, reject } +} + +beforeEach(() => { + vi.clearAllMocks() + mocks.trySmartCopyExport.mockResolvedValue({ result: null }) + const clientSettings = { + resolution: { width: 640, height: 360 }, + subtitleMode: 'burn', + codec: 'avc', + container: 'mp4', + } + mocks.mapRequestedClientSettings.mockReturnValue({ + clientSettings, + exportMode: 'video', + renderWholeProject: false, + }) + mocks.resolveClientSettings.mockImplementation(async (settings: { subtitleMode?: string }) => ({ + clientSettings: { ...clientSettings, subtitleMode: settings.subtitleMode ?? 'burn' }, + exportMode: 'video', + renderWholeProject: false, + })) + mocks.convertTimelineToComposition.mockReturnValue({ tracks: [], durationInFrames: 30 }) + mocks.resolveMediaUrls.mockImplementation(async (tracks: unknown) => tracks) + mocks.buildTranscriptSubtitleCues.mockReturnValue([]) + mocks.releaseTemporaryExportOutput.mockResolvedValue(undefined) +}) + +afterEach(() => vi.restoreAllMocks()) + +describe('useClientRender lifecycle ownership', () => { + it('aborts the active render on unmount and propagates its signal through media resolution', async () => { + const render = deferred() + mocks.runRender.mockReturnValue( + render.promise.then((result) => ({ result, renderPath: 'worker' })), + ) + const hook = renderHook(() => useClientRender(), { + wrapper: ({ children }: PropsWithChildren) => {children}, + }) + + let exportPromise!: Promise + await act(async () => { + exportPromise = hook.result.current.startExport(settings as never) + await Promise.resolve() + }) + expect(mocks.resolveMediaUrls).toHaveBeenCalledWith( + [], + expect.objectContaining({ useProxy: false, signal: expect.any(AbortSignal) }), + ) + const signal = mocks.resolveMediaUrls.mock.calls[0]![1].signal as AbortSignal + hook.unmount() + expect(signal.aborted).toBe(true) + render.resolve(renderedResult) + await act(async () => { + await exportPromise + }) + }) + + it('aborts replacement renders and releases a result that becomes stale', async () => { + const first = deferred() + const second = deferred() + mocks.runRender + .mockReturnValueOnce(first.promise.then((result) => ({ result, renderPath: 'worker' }))) + .mockReturnValueOnce(second.promise.then((result) => ({ result, renderPath: 'worker' }))) + const hook = renderHook(() => useClientRender()) + let firstExport!: Promise + await act(async () => { + firstExport = hook.result.current.startExport(settings as never) + await Promise.resolve() + }) + const firstSignal = mocks.runRender.mock.calls[0]![0].signal as AbortSignal + let secondExport!: Promise + await act(async () => { + secondExport = hook.result.current.startExport(settings as never) + await Promise.resolve() + }) + expect(firstSignal.aborted).toBe(true) + first.resolve(renderedResult) + second.resolve({ ...renderedResult, blob: new Blob(['second']) }) + await act(async () => { + await Promise.all([firstExport, secondExport]) + }) + expect(mocks.releaseTemporaryExportOutput).toHaveBeenCalledWith(renderedResult) + }) + + it('ignores progress and result ownership from an aborted run after a restart', async () => { + const first = deferred() + const secondResult = { ...renderedResult, blob: new Blob(['second']) } + const second = deferred() + mocks.runRender + .mockReturnValueOnce(first.promise.then((result) => ({ result, renderPath: 'worker' }))) + .mockReturnValueOnce(second.promise.then((result) => ({ result, renderPath: 'worker' }))) + const hook = renderHook(() => useClientRender()) + + let firstExport!: Promise + await act(async () => { + firstExport = hook.result.current.startExport(settings as never) + await Promise.resolve() + }) + const firstProgress = mocks.runRender.mock.calls[0]![0].onProgress + + let secondExport!: Promise + await act(async () => { + secondExport = hook.result.current.startExport(settings as never) + await Promise.resolve() + }) + const secondProgress = mocks.runRender.mock.calls[1]![0].onProgress + + act(() => { + secondProgress({ + phase: 'rendering', + progress: 25, + message: 'new run', + currentFrame: 5, + totalFrames: 20, + }) + }) + expect(hook.result.current).toMatchObject({ + progress: 25, + progressMessage: 'new run', + status: 'rendering', + }) + + act(() => { + firstProgress({ + phase: 'encoding', + progress: 90, + message: 'stale run', + currentFrame: 18, + totalFrames: 20, + }) + }) + expect(hook.result.current).toMatchObject({ + progress: 25, + progressMessage: 'new run', + status: 'rendering', + }) + + second.resolve(secondResult) + await act(async () => { + await secondExport + }) + expect(hook.result.current).toMatchObject({ + progress: 100, + status: 'completed', + result: secondResult, + }) + + act(() => { + firstProgress({ + phase: 'finalizing', + progress: 99, + message: 'late stale run', + currentFrame: 20, + totalFrames: 20, + }) + }) + expect(hook.result.current).toMatchObject({ + progress: 100, + status: 'completed', + result: secondResult, + }) + + first.resolve(renderedResult) + await act(async () => { + await firstExport + }) + expect(hook.result.current.result).toBe(secondResult) + expect(mocks.releaseTemporaryExportOutput).toHaveBeenCalledWith(renderedResult) + }) + + it('aborts a cancelled run exactly once when unmount races its completion', async () => { + const abortSpy = vi.spyOn(AbortController.prototype, 'abort') + const render = deferred() + mocks.runRender.mockReturnValue( + render.promise.then((result) => ({ result, renderPath: 'worker' })), + ) + const hook = renderHook(() => useClientRender(), { + wrapper: ({ children }: PropsWithChildren) => {children}, + }) + + let exportPromise!: Promise + await act(async () => { + exportPromise = hook.result.current.startExport(settings as never) + await Promise.resolve() + }) + act(() => hook.result.current.cancelExport()) + hook.unmount() + + expect(abortSpy).toHaveBeenCalledTimes(1) + render.resolve(renderedResult) + await act(async () => { + await exportPromise + }) + expect(abortSpy).toHaveBeenCalledTimes(1) + expect(mocks.releaseTemporaryExportOutput).toHaveBeenCalledTimes(1) + expect(mocks.releaseTemporaryExportOutput).toHaveBeenCalledWith(renderedResult) + }) + + it('releases a rendered output when finalization/update work throws before ownership transfer', async () => { + const output = { + ...renderedResult, + temporaryOutput: { directory: 'scratch', fileName: 'out.mp4' }, + } + mocks.runRender.mockResolvedValue({ result: output, renderPath: 'worker' }) + mocks.buildTranscriptSubtitleCues.mockImplementation(() => { + throw new Error('state/update failed') + }) + const hook = renderHook(() => useClientRender()) + await act(async () => { + await hook.result.current.startExport({ ...settings, subtitleMode: 'sidecar' } as never) + }) + expect(mocks.releaseTemporaryExportOutput).toHaveBeenCalledTimes(1) + expect(mocks.releaseTemporaryExportOutput).toHaveBeenCalledWith(output) + }) + + it('releases a successfully owned result once on unmount, with reset/unmount causing no double release', async () => { + mocks.runRender.mockResolvedValue({ result: renderedResult, renderPath: 'worker' }) + const hook = renderHook(() => useClientRender()) + await act(async () => { + await hook.result.current.startExport(settings as never) + }) + expect(hook.result.current.result).toBe(renderedResult) + act(() => hook.result.current.resetState()) + hook.unmount() + expect(mocks.releaseTemporaryExportOutput).toHaveBeenCalledTimes(1) + expect(mocks.releaseTemporaryExportOutput).toHaveBeenCalledWith(renderedResult) + }) +}) diff --git a/src/features/export/hooks/use-client-render.ts b/src/features/export/hooks/use-client-render.ts index aa2009645..70c866b7a 100644 --- a/src/features/export/hooks/use-client-render.ts +++ b/src/features/export/hooks/use-client-render.ts @@ -31,11 +31,13 @@ import { buildTranscriptSubtitleCues } from '../utils/embedded-subtitle-export' import { serializeSrt } from '@/shared/utils/subtitles' import { releaseTemporaryExportOutput } from '../utils/export-output-target' import { useTimelineStore } from '@/features/export/deps/timeline' +import type { ExportableSequence } from '@/features/export/deps/timeline-compositions' import { useProjectStore } from '@/features/export/deps/projects' import { DEFAULT_PROJECT_HEIGHT, DEFAULT_PROJECT_WIDTH } from '@/shared/projects/defaults' import { resolveMediaUrls } from '@/features/export/deps/media-library' import { usePlaybackStore } from '@/shared/state/playback' import { createLogger, createOperationId } from '@/shared/logging/logger' +import { resolveClientRenderSource } from './client-render-source' const log = createLogger('Export') @@ -61,7 +63,10 @@ interface UseClientRenderReturn { result: ClientRenderResult | null // Actions - startExport: (settings: ExportSettings | ExtendedExportSettings) => Promise + startExport: ( + settings: ExportSettings | ExtendedExportSettings, + sequence?: ExportableSequence, + ) => Promise cancelExport: () => void downloadVideo: () => void resetState: () => void @@ -84,15 +89,40 @@ export function useClientRender(): UseClientRenderReturn { const [status, setStatus] = useState('idle') const [error, setError] = useState(null) const [result, setResult] = useState(null) - const resultRef = useRef(null) + const resultOwnerRef = useRef<{ + runToken: number + result: ClientRenderResult + released: boolean + } | null>(null) + + const activeRunRef = useRef<{ + token: number + controller: AbortController + } | null>(null) + const latestRunTokenRef = useRef(0) + + const abortActiveRun = useCallback(() => { + const run = activeRunRef.current + if (!run) return + activeRunRef.current = null + if (!run.controller.signal.aborted) run.controller.abort() + }, []) - // AbortController for cancellation - const abortControllerRef = useRef(null) + const releaseOwnedResult = useCallback( + ( + owner: { runToken: number; result: ClientRenderResult; released: boolean } | null | undefined, + ) => { + if (!owner || owner.released) return + owner.released = true + void releaseTemporaryExportOutput(owner.result) + }, + [], + ) /** * Handle progress updates from the render engine */ - const handleProgress = useCallback((progressData: RenderProgress) => { + const applyProgress = useCallback((progressData: RenderProgress) => { setProgress(progressData.progress) setProgressMessage(progressData.message) setRenderedFrames(progressData.currentFrame) @@ -119,14 +149,38 @@ export function useClientRender(): UseClientRenderReturn { * Start client-side export */ const startExport = useCallback( - async (settings: ExportSettings | ExtendedExportSettings) => { + async (settings: ExportSettings | ExtendedExportSettings, sequence?: ExportableSequence) => { const opId = createOperationId() const event = log.startEvent('render', opId) + const runToken = ++latestRunTokenRef.current + abortActiveRun() + const controller = new AbortController() + const run = { token: runToken, controller } + activeRunRef.current = run + let temporaryResult: ClientRenderResult | null = null + + const releaseTemporaryResult = () => { + const ownedResult = temporaryResult + temporaryResult = null + if (ownedResult) void releaseTemporaryExportOutput(ownedResult) + } + const isActive = () => + activeRunRef.current === run && + latestRunTokenRef.current === runToken && + !controller.signal.aborted + const ensureActive = () => { + if (!isActive()) { + throw new DOMException('Render cancelled', 'AbortError') + } + } + const handleRunProgress = (progressData: RenderProgress) => { + if (isActive()) applyProgress(progressData) + } try { - const previousResult = resultRef.current - resultRef.current = null - void releaseTemporaryExportOutput(previousResult) + const previousResultOwner = resultOwnerRef.current + resultOwnerRef.current = null + releaseOwnedResult(previousResultOwner) setIsExporting(true) setProgress(0) setProgressMessage(undefined) @@ -134,28 +188,31 @@ export function useClientRender(): UseClientRenderReturn { setResult(null) setStatus('preparing') - // Create abort controller for cancellation - abortControllerRef.current = new AbortController() - // Read current state from stores const state = useTimelineStore.getState() - const { tracks, items, transitions, fps, inPoint, outPoint, keyframes } = state - - // Get project metadata (background color and native resolution) const currentProject = useProjectStore.getState().currentProject - const busAudioEq = usePlaybackStore.getState().busAudioEq - const masterBusDb = usePlaybackStore.getState().masterBusDb - const backgroundColor = currentProject?.metadata?.backgroundColor - // Use PROJECT resolution for composition (transform calculations match preview) - const projectWidth = currentProject?.metadata?.width ?? DEFAULT_PROJECT_WIDTH - const projectHeight = currentProject?.metadata?.height ?? DEFAULT_PROJECT_HEIGHT + const playback = usePlaybackStore.getState() + const { + tracks, + items, + transitions, + fps, + inPoint, + outPoint, + keyframes, + busAudioEq, + masterBusDb, + backgroundColor, + width: projectWidth, + height: projectHeight, + } = resolveClientRenderSource(sequence, state, playback, currentProject?.metadata) const requested = mapRequestedClientSettings(settings, fps) // When renderWholeProject is true, ignore in/out points. const { exportMode, renderWholeProject } = requested const effectiveInPoint = renderWholeProject ? null : inPoint const effectiveOutPoint = renderWholeProject ? null : outPoint - const signal = abortControllerRef.current.signal + const signal = controller.signal const smartCopy = await trySmartCopyExport( { @@ -173,12 +230,19 @@ export function useClientRender(): UseClientRenderReturn { masterBusDb, }, signal, - handleProgress, + handleRunProgress, ) + ensureActive() if (smartCopy.result) { - resultRef.current = smartCopy.result - setResult(smartCopy.result) + temporaryResult = smartCopy.result + setResult(temporaryResult) + resultOwnerRef.current = { + runToken, + result: temporaryResult, + released: false, + } + temporaryResult = null setStatus('completed') setProgress(100) event.set('renderPath', 'smart-copy') @@ -192,6 +256,7 @@ export function useClientRender(): UseClientRenderReturn { // Resolve settings + codec fallback only when an encoder is required. const { clientSettings, codecFallback } = await resolveClientSettings(settings, fps) + ensureActive() if (codecFallback) event.set('codecFallback', codecFallback) const extended = isExtendedSettings(settings) @@ -246,7 +311,11 @@ export function useClientRender(): UseClientRenderReturn { // Resolve media URLs (convert mediaIds to blob URLs) // Export always uses full-res source, never proxies - const resolvedTracks = await resolveMediaUrls(composition.tracks, { useProxy: false }) + const resolvedTracks = await resolveMediaUrls(composition.tracks, { + useProxy: false, + signal, + }) + ensureActive() composition.tracks = resolvedTracks // Count resolved items for diagnostics @@ -291,8 +360,10 @@ export function useClientRender(): UseClientRenderReturn { exportMode, composition, signal, - onProgress: handleProgress, + onProgress: handleRunProgress, }) + temporaryResult = renderResult + ensureActive() if (fallbackReason) event.set('workerFallbackReason', fallbackReason) // Sidecar mode: the video is muxed clean; build the .srt from the same @@ -310,8 +381,10 @@ export function useClientRender(): UseClientRenderReturn { } } - resultRef.current = finalResult + if (finalResult !== renderResult) temporaryResult = finalResult setResult(finalResult) + resultOwnerRef.current = { runToken, result: finalResult, released: false } + temporaryResult = null setStatus('completed') setProgress(100) @@ -322,6 +395,8 @@ export function useClientRender(): UseClientRenderReturn { duration: renderResult.duration, }) } catch (err) { + releaseTemporaryResult() + if (runToken !== latestRunTokenRef.current) return if (err instanceof DOMException && err.name === 'AbortError') { event.set('outcome', 'cancelled') event.set('duration_ms', Date.now()) @@ -334,11 +409,13 @@ export function useClientRender(): UseClientRenderReturn { setStatus('failed') } } finally { - setIsExporting(false) - abortControllerRef.current = null + if (activeRunRef.current === run) { + activeRunRef.current = null + setIsExporting(false) + } } }, - [handleProgress], + [abortActiveRun, applyProgress, releaseOwnedResult], ) /** @@ -346,12 +423,12 @@ export function useClientRender(): UseClientRenderReturn { * which posts the cancel to its worker and terminates it. */ const cancelExport = useCallback(() => { - if (abortControllerRef.current) { - abortControllerRef.current.abort() + if (activeRunRef.current) { + abortActiveRun() setStatus('cancelled') setIsExporting(false) } - }, []) + }, [abortActiveRun]) /** * Download the rendered video/audio @@ -402,8 +479,8 @@ export function useClientRender(): UseClientRenderReturn { * Reset state */ const resetState = useCallback(() => { - abortControllerRef.current?.abort() - abortControllerRef.current = null + latestRunTokenRef.current++ + abortActiveRun() setIsExporting(false) setProgress(0) setProgressMessage(undefined) @@ -411,18 +488,21 @@ export function useClientRender(): UseClientRenderReturn { setTotalFrames(undefined) setStatus('idle') setError(null) - const previousResult = resultRef.current - resultRef.current = null - void releaseTemporaryExportOutput(previousResult) + const previousResultOwner = resultOwnerRef.current + resultOwnerRef.current = null + releaseOwnedResult(previousResultOwner) setResult(null) - }, []) + }, [abortActiveRun, releaseOwnedResult]) useEffect( () => () => { - void releaseTemporaryExportOutput(resultRef.current) - resultRef.current = null + latestRunTokenRef.current++ + abortActiveRun() + const ownedResult = resultOwnerRef.current + resultOwnerRef.current = null + releaseOwnedResult(ownedResult) }, - [], + [abortActiveRun, releaseOwnedResult], ) /** diff --git a/src/features/export/hooks/use-render-queue-runner.ts b/src/features/export/hooks/use-render-queue-runner.ts index 1c76aadb8..e280068a7 100644 --- a/src/features/export/hooks/use-render-queue-runner.ts +++ b/src/features/export/hooks/use-render-queue-runner.ts @@ -110,7 +110,10 @@ async function renderQueuedJob(job: RenderJob): Promise { ) // Resolve mediaIds → blob URLs fresh at render time (export never proxies). - composition.tracks = await resolveMediaUrls(composition.tracks, { useProxy: false }) + composition.tracks = await resolveMediaUrls(composition.tracks, { + useProxy: false, + signal: controller.signal, + }) const { result, renderPath, fallbackReason } = await runRender({ clientSettings: job.clientSettings, diff --git a/src/features/export/utils/client-render-engine.test.ts b/src/features/export/utils/client-render-engine.test.ts index 4015a78b4..b6945a4de 100644 --- a/src/features/export/utils/client-render-engine.test.ts +++ b/src/features/export/utils/client-render-engine.test.ts @@ -403,7 +403,7 @@ describe('selectPreviewVideoSource', () => { ).toEqual(['blob:original', null, null]) }) - it('includes cached proxies when proxy media is selected', () => { + it('keeps the authoritative current source ahead of stale item and registered URLs', () => { expect( getPreviewVideoSourceCandidates({ itemSource: 'blob:proxy', @@ -412,7 +412,21 @@ describe('selectPreviewVideoSource', () => { cachedSource: 'blob:source', useProxyMedia: true, }), - ).toEqual(['blob:proxy', 'blob:proxy', 'blob:registered-proxy', 'blob:source']) + ).toEqual(['blob:proxy', 'blob:source', 'blob:registered-proxy', 'blob:proxy']) + }) + + it('selects a relinked current source instead of stale registered and item fallbacks', () => { + expect( + selectPreviewVideoSource({ + candidates: getPreviewVideoSourceCandidates({ + itemSource: 'blob:old-item', + proxySource: null, + registeredSource: 'blob:old-registered', + cachedSource: 'blob:new-current', + useProxyMedia: true, + }), + }), + ).toBe('blob:new-current') }) it('selects the cached proxy when a compound item still carries its original source', () => { diff --git a/src/features/export/utils/client-render-engine.ts b/src/features/export/utils/client-render-engine.ts index 081b8cd0f..eec34a600 100644 --- a/src/features/export/utils/client-render-engine.ts +++ b/src/features/export/utils/client-render-engine.ts @@ -210,7 +210,7 @@ export function getPreviewVideoSourceCandidates({ useProxyMedia: boolean }): Array { if (useProxyMedia) { - return [proxySource, itemSource, registeredSource, cachedSource] + return [proxySource, cachedSource, registeredSource, itemSource] } // blobUrlManager owns the current original-media URL and is also what the @@ -305,12 +305,15 @@ function selectComparisonVideoSource( registeredSource: string | undefined, useProxyMedia: boolean, ): string | null { - return selectFirstMediaSource([ - registeredSource, - useProxyMedia && item.mediaId ? resolveProxyUrl(item.mediaId) : null, - item.src, - item.mediaId ? blobUrlManager.get(item.mediaId) : null, - ]) + return selectFirstMediaSource( + getPreviewVideoSourceCandidates({ + itemSource: item.src, + proxySource: item.mediaId ? resolveProxyUrl(item.mediaId) : null, + registeredSource, + cachedSource: item.mediaId ? blobUrlManager.get(item.mediaId) : null, + useProxyMedia, + }), + ) } function selectExportVideoSource( @@ -385,20 +388,26 @@ function waitForFallbackVideoReady(options: { }) } +function resolveRendererProxySource( + item: VideoItem | ImageItem | LottieItem, + useProxyMedia: boolean, +): string | null { + if (!useProxyMedia || item.type !== 'video' || !item.mediaId) return null + return resolveProxyUrl(item.mediaId) +} + async function resolveRendererMediaSource( item: VideoItem | ImageItem | LottieItem, useProxyMedia: boolean, signal?: AbortSignal, ): Promise { throwIfAborted(signal) - if (useProxyMedia && item.type === 'video' && item.mediaId) { - const proxyUrl = resolveProxyUrl(item.mediaId) - if (proxyUrl) return proxyUrl - } - if (item.src) return item.src - if (!item.mediaId) return null + const proxyUrl = resolveRendererProxySource(item, useProxyMedia) + if (proxyUrl) return proxyUrl + if (!item.mediaId) return item.src ?? null const cachedUrl = blobUrlManager.get(item.mediaId) if (cachedUrl) return cachedUrl + if (item.src) return item.src const resolvedUrl = await resolveMediaUrl(item.mediaId) throwIfAborted(signal) return resolvedUrl || null diff --git a/src/features/media-library/stores/media-delete-actions.test.ts b/src/features/media-library/stores/media-delete-actions.test.ts index cef929c77..5e2465433 100644 --- a/src/features/media-library/stores/media-delete-actions.test.ts +++ b/src/features/media-library/stores/media-delete-actions.test.ts @@ -17,7 +17,7 @@ const proxyServiceMocks = vi.hoisted(() => ({ })) const blobUrlManagerMocks = vi.hoisted(() => ({ - release: vi.fn(), + invalidate: vi.fn(), })) vi.mock('../services/media-library-service', () => ({ @@ -139,7 +139,7 @@ describe('createDeleteActions', () => { ) expect(currentState.mediaItems.map((item) => item.id)).toEqual(['media-2']) expect(currentState.selectedMediaIds).toEqual([]) - expect(blobUrlManagerMocks.release).toHaveBeenCalledWith('media-1') + expect(blobUrlManagerMocks.invalidate).toHaveBeenCalledWith('media-1') expect(proxyServiceMocks.clearProxyKey).toHaveBeenCalledWith('media-1') }) @@ -161,7 +161,7 @@ describe('createDeleteActions', () => { expect(currentState.mediaItems.map((item) => item.id)).toEqual(['media-1', 'media-2']) expect(currentState.selectedMediaIds).toEqual(['media-1']) expect(currentState.error).toBe('Delete failed hard') - expect(blobUrlManagerMocks.release).not.toHaveBeenCalled() + expect(blobUrlManagerMocks.invalidate).not.toHaveBeenCalled() }) it('uses the legacy batch delete path when no project is selected', async () => { @@ -183,7 +183,7 @@ describe('createDeleteActions', () => { expect(mediaLibraryServiceMocks.deleteMediaBatch).toHaveBeenCalledWith(['media-1', 'media-2']) expect(currentState.mediaItems).toEqual([]) expect(currentState.selectedMediaIds).toEqual([]) - expect(blobUrlManagerMocks.release).toHaveBeenCalledTimes(2) + expect(blobUrlManagerMocks.invalidate).toHaveBeenCalledTimes(2) expect(proxyServiceMocks.clearProxyKey).toHaveBeenCalledTimes(2) }) }) diff --git a/src/features/media-library/stores/media-delete-actions.ts b/src/features/media-library/stores/media-delete-actions.ts index 53bffdfa9..caf487875 100644 --- a/src/features/media-library/stores/media-delete-actions.ts +++ b/src/features/media-library/stores/media-delete-actions.ts @@ -31,7 +31,10 @@ function releaseDeletedMediaResources( const previousMediaById = new Map(previousItems.map((item) => [item.id, item])) for (const id of ids) { - blobUrlManager.release(id) + // Deletion is a source retirement, not a consumer release. Advance the + // media epoch even when no URL has settled yet so a pending storage read + // cannot resurrect the deleted source. + blobUrlManager.invalidate(id) proxyService.clearProxyKey(id) // Drop every Scene Browser cache tied to this media — thumbnail blob // URLs (which otherwise pin the JPEG in memory forever), lazy-thumb diff --git a/src/features/media-library/utils/media-resolver.ts b/src/features/media-library/utils/media-resolver.ts index 779e962f2..725dfdc50 100644 --- a/src/features/media-library/utils/media-resolver.ts +++ b/src/features/media-library/utils/media-resolver.ts @@ -12,7 +12,113 @@ const logger = createLogger('MediaResolver') * Pending requests to prevent concurrent OPFS access to the same file * This prevents multiple sync access handle creation for the same OPFS file */ -const pendingRequests = new Map>() +interface PendingMediaRequest { + epoch: string + promise: Promise +} + +const pendingRequests = new Map() + +type MediaLibraryServiceModule = + typeof import('@/features/media-library/services/media-library-service') +type MediaLibraryService = MediaLibraryServiceModule['mediaLibraryService'] +type ResolvedMedia = NonNullable>> + +function isCurrentMediaEpoch(mediaId: string, epoch: string): boolean { + return blobUrlManager.getEpoch(mediaId) === epoch +} + +function acquireResolvedMediaUrl(mediaId: string, blob: Blob, media: ResolvedMedia): string { + const blobUrl = blobUrlManager.acquire(mediaId, blob, { + mediaId, + storageType: media.storageType, + fileHandle: media.storageType === 'handle' ? media.fileHandle : undefined, + opfsPath: media.storageType === 'opfs' ? media.opfsPath : undefined, + fileSize: media.fileSize, + }) + + if (media.keyframeTimestamps && media.keyframeTimestamps.length > 0) { + registerKeyframeIndex(blobUrl, media.keyframeTimestamps) + } + return blobUrl +} + +async function resolveCurrentMediaUrl( + mediaId: string, + requestEpoch: string, + mediaLibraryService: MediaLibraryService, +): Promise { + const media = await mediaLibraryService.getMedia(mediaId) + if (!isCurrentMediaEpoch(mediaId, requestEpoch)) return '' + + if (!media) { + logger.warn(`Media not found: ${mediaId}`) + return '' + } + + // Get the source blob without an extra validation pass; getMediaFile + // surfaces permission/missing-file errors with the same relink UI. + const blob = await mediaLibraryService.getMediaFile(media) + if (!isCurrentMediaEpoch(mediaId, requestEpoch)) return '' + + if (!blob) { + // The media record exists but its bytes can't be resolved (no valid + // storage path — e.g. opened on an origin whose OPFS lacks it and the + // workspace folder has no copy). getMediaFile returns null WITHOUT a + // FileAccessError, so surface it into the broken-media system here so + // the clip shows a relink state and the missing-media dialog lights up. + logger.warn(`Media blob not found: ${mediaId}`) + useMediaLibraryStore.getState().markMediaBroken(mediaId, { + mediaId, + fileName: media.fileName ?? 'Unknown file', + errorType: 'file_missing', + }) + return '' + } + + const blobUrl = acquireResolvedMediaUrl(mediaId, blob, media) + useMediaLibraryStore.getState().markMediaHealthy(mediaId) + return blobUrl +} + +async function markMediaBrokenFromAccessError( + mediaId: string, + requestEpoch: string, + error: unknown, + mediaLibraryService: MediaLibraryService, + FileAccessError: MediaLibraryServiceModule['FileAccessError'], +): Promise { + if (!(error instanceof FileAccessError)) return + + const media = await mediaLibraryService.getMedia(mediaId) + if (!isCurrentMediaEpoch(mediaId, requestEpoch)) return + useMediaLibraryStore.getState().markMediaBroken(mediaId, { + mediaId, + fileName: media?.fileName ?? 'Unknown file', + errorType: error.type === 'permission_denied' ? 'permission_denied' : 'file_missing', + }) +} + +async function loadMediaRequest(mediaId: string, requestEpoch: string): Promise { + const { mediaLibraryService, FileAccessError } = + await import('@/features/media-library/services/media-library-service') + if (!isCurrentMediaEpoch(mediaId, requestEpoch)) return '' + + try { + return await resolveCurrentMediaUrl(mediaId, requestEpoch, mediaLibraryService) + } catch (error) { + if (!isCurrentMediaEpoch(mediaId, requestEpoch)) return '' + logger.error(`Failed to resolve media ${mediaId}:`, error) + await markMediaBrokenFromAccessError( + mediaId, + requestEpoch, + error, + mediaLibraryService, + FileAccessError, + ) + return '' + } +} /** * Resolves a mediaId to a blob URL for use in Composition Player @@ -27,86 +133,28 @@ export async function resolveMediaUrl(mediaId: string): Promise { return cached } - // Check if there's already a pending request for this media - if (pendingRequests.has(mediaId)) { - return pendingRequests.get(mediaId)! + const requestEpoch = blobUrlManager.getEpoch(mediaId) + + // Deduplicate only within the currently valid source generation. Relinking + // can invalidate an ID while an old storage read is still pending; that old + // promise must not block or populate the replacement generation. + const pendingRequest = pendingRequests.get(mediaId) + if (pendingRequest?.epoch === requestEpoch) { + return pendingRequest.promise } // Create the request promise - const requestPromise = (async () => { - const { mediaLibraryService, FileAccessError } = - await import('@/features/media-library/services/media-library-service') - - try { - // Get media metadata from library - const media = await mediaLibraryService.getMedia(mediaId) - - if (!media) { - logger.warn(`Media not found: ${mediaId}`) - return '' // Fallback: empty string (Composition will skip) - } - - // Get the source blob without an extra validation pass; getMediaFile - // surfaces permission/missing-file errors with the same relink UI. - const blob = await mediaLibraryService.getMediaFile(media) - - if (!blob) { - // The media record exists but its bytes can't be resolved (no valid - // storage path — e.g. opened on an origin whose OPFS lacks it and the - // workspace folder has no copy). getMediaFile returns null WITHOUT a - // FileAccessError, so surface it into the broken-media system here so - // the clip shows a relink state and the missing-media dialog lights up. - logger.warn(`Media blob not found: ${mediaId}`) - useMediaLibraryStore.getState().markMediaBroken(mediaId, { - mediaId, - fileName: media.fileName ?? 'Unknown file', - errorType: 'file_missing', - }) - return '' - } - - // Acquire blob URL through centralized manager (handles caching + ref counting) - const blobUrl = blobUrlManager.acquire(mediaId, blob, { - mediaId, - storageType: media.storageType, - fileHandle: media.storageType === 'handle' ? media.fileHandle : undefined, - opfsPath: media.storageType === 'opfs' ? media.opfsPath : undefined, - fileSize: media.fileSize, - }) - - // Register keyframe index for adaptive seek backtracking - if (media.keyframeTimestamps && media.keyframeTimestamps.length > 0) { - registerKeyframeIndex(blobUrl, media.keyframeTimestamps) - } - - // Resolved successfully — clear any stale broken flag (e.g. the repair - // sweep just restored the workspace copy) so the clip stops showing the - // offline state without needing a reload. - useMediaLibraryStore.getState().markMediaHealthy(mediaId) - - return blobUrl - } catch (error) { - logger.error(`Failed to resolve media ${mediaId}:`, error) - - // Mark media as broken if it's a file access error - if (error instanceof FileAccessError) { - const media = await mediaLibraryService.getMedia(mediaId) - useMediaLibraryStore.getState().markMediaBroken(mediaId, { - mediaId, - fileName: media?.fileName ?? 'Unknown file', - errorType: error.type === 'permission_denied' ? 'permission_denied' : 'file_missing', - }) - } - - return '' // Fallback: empty string - } finally { - // Clean up pending request + let requestPromise!: Promise + requestPromise = loadMediaRequest(mediaId, requestEpoch).finally(() => { + // A later source generation may already own this mediaId's slot. Only + // the exact request that installed an entry may remove it. + if (pendingRequests.get(mediaId)?.promise === requestPromise) { pendingRequests.delete(mediaId) } - })() + }) // Store the pending request - pendingRequests.set(mediaId, requestPromise) + pendingRequests.set(mediaId, { epoch: requestEpoch, promise: requestPromise }) return requestPromise } @@ -144,6 +192,11 @@ export async function resolveMediaUrls( const useProxy = options?.useProxy ?? true const signal = options?.signal + const throwIfAborted = () => { + if (signal?.aborted) throw new DOMException('Media resolution aborted', 'AbortError') + } + throwIfAborted() + // Deep clone tracks to avoid mutating original const resolvedTracks: TimelineTrack[] = structuredClone(tracks) @@ -160,7 +213,8 @@ export async function resolveMediaUrls( item.type === 'image' || item.type === 'lottie') ) { - const promise = resolveMediaUrl(item.mediaId).then((blobUrl) => { + const resolution = resolveMediaUrl(item.mediaId).then((blobUrl) => { + throwIfAborted() // For video items in preview mode, prefer proxy URL if available if (useProxy && item.type === 'video') { const proxyUrl = resolveProxyUrl(item.mediaId!) @@ -173,7 +227,30 @@ export async function resolveMediaUrls( } } }) - resolutionPromises.push(promise) + if (signal) { + resolutionPromises.push( + new Promise((resolve, reject) => { + const onAbort = () => { + signal.removeEventListener('abort', onAbort) + reject(new DOMException('Media resolution aborted', 'AbortError')) + } + signal.addEventListener('abort', onAbort, { once: true }) + resolution.then( + () => { + signal.removeEventListener('abort', onAbort) + resolve() + }, + (error) => { + signal.removeEventListener('abort', onAbort) + reject(error) + }, + ) + if (signal.aborted) onAbort() + }), + ) + } else { + resolutionPromises.push(resolution) + } } } } @@ -182,9 +259,7 @@ export async function resolveMediaUrls( await Promise.all(resolutionPromises) // Check if aborted after resolution - if (signal?.aborted) { - throw new DOMException('Media resolution aborted', 'AbortError') - } + throwIfAborted() return resolvedTracks } diff --git a/src/features/preview/components/inline-source-preview.test.tsx b/src/features/preview/components/inline-source-preview.test.tsx new file mode 100644 index 000000000..cacc4cf90 --- /dev/null +++ b/src/features/preview/components/inline-source-preview.test.tsx @@ -0,0 +1,166 @@ +import { useEffect, useSyncExternalStore, type ReactNode } from 'react' +import { act, render, waitFor } from '@testing-library/react' +import { beforeEach, describe, expect, it, vi } from 'vite-plus/test' + +const harness = vi.hoisted(() => ({ + globalVersion: 0, + epochs: new Map(), + resolveMediaUrl: vi.fn<(mediaId: string) => Promise>(), + mounts: 0, + unmounts: 0, + listeners: new Set<() => void>(), + publish: () => { + for (const listener of harness.listeners) listener() + }, +})) + +vi.mock('@/features/preview/deps/player-context', () => ({ + PlayerEmitterProvider: ({ children }: { children: ReactNode }) => <>{children}, + ClockBridgeProvider: ({ children }: { children: ReactNode }) => <>{children}, + VideoConfigProvider: ({ children }: { children: ReactNode }) => <>{children}, + useClock: () => ({ seekToFrame: vi.fn() }), +})) + +vi.mock('@/features/preview/deps/media-library', () => ({ + useMediaLibraryStore: (selector: (state: Record) => unknown) => + selector({ + mediaById: { + 'media-1': { + id: 'media-1', + fileName: 'clip.mp4', + mimeType: 'video/mp4', + duration: 5, + width: 1920, + height: 1080, + fps: 30, + }, + }, + }), + getMediaType: () => 'video', +})) + +vi.mock('@/shared/state/playback', () => ({ + usePlaybackStore: (selector: (state: { zoom: number }) => unknown) => selector({ zoom: -1 }), +})) + +vi.mock('@/infrastructure/browser/blob-url-manager', () => ({ + useBlobUrlVersion: () => + useSyncExternalStore( + (listener) => { + harness.listeners.add(listener) + return () => harness.listeners.delete(listener) + }, + () => harness.globalVersion, + ), + useBlobUrlEpoch: (mediaId: string) => + useSyncExternalStore( + (listener) => { + harness.listeners.add(listener) + return () => harness.listeners.delete(listener) + }, + () => String(harness.epochs.get(mediaId) ?? 0), + ), +})) + +vi.mock('../utils/media-resolver', () => ({ + resolveMediaUrl: harness.resolveMediaUrl, +})) + +vi.mock('./source-composition', () => ({ + SourceComposition: ({ src }: { src: string }) => { + useEffect(() => { + harness.mounts += 1 + return () => { + harness.unmounts += 1 + } + }, []) + return
+ }, +})) + +import { InlineSourcePreview } from './inline-source-preview' + +describe('InlineSourcePreview source binding ownership', () => { + beforeEach(() => { + vi.clearAllMocks() + harness.globalVersion = 0 + harness.epochs.clear() + harness.resolveMediaUrl.mockResolvedValue('blob:media-1') + harness.mounts = 0 + harness.unmounts = 0 + harness.listeners.clear() + }) + + it('preserves the current frame generation across unrelated blob URL activity', async () => { + const rendered = render( + , + ) + + await waitFor(() => { + expect(rendered.getByTestId('inline-source-composition')).toHaveAttribute( + 'data-source', + 'blob:media-1', + ) + }) + expect(harness.resolveMediaUrl).toHaveBeenCalledTimes(1) + + act(() => { + harness.globalVersion += 1 + harness.publish() + }) + await act(async () => { + await Promise.resolve() + }) + + expect(harness.resolveMediaUrl).toHaveBeenCalledTimes(1) + expect(harness.mounts).toBe(1) + expect(harness.unmounts).toBe(0) + }) + + it('retires the relevant frame generation before resolving its replacement once', async () => { + let resolveReplacement!: (url: string) => void + const replacement = new Promise((resolve) => { + resolveReplacement = resolve + }) + harness.resolveMediaUrl.mockResolvedValueOnce('blob:old').mockReturnValueOnce(replacement) + const rendered = render( + , + ) + + await waitFor(() => { + expect(rendered.getByTestId('inline-source-composition')).toHaveAttribute( + 'data-source', + 'blob:old', + ) + }) + + act(() => { + harness.epochs.set('media-1', 1) + harness.globalVersion += 1 + harness.publish() + }) + + expect(rendered.queryByTestId('inline-source-composition')).toBeNull() + await waitFor(() => expect(harness.resolveMediaUrl).toHaveBeenCalledTimes(2)) + + await act(async () => { + resolveReplacement('blob:new') + await replacement + }) + + expect(rendered.getByTestId('inline-source-composition')).toHaveAttribute( + 'data-source', + 'blob:new', + ) + expect(harness.resolveMediaUrl).toHaveBeenCalledTimes(2) + expect(harness.unmounts).toBe(1) + }) +}) diff --git a/src/features/preview/components/inline-source-preview.tsx b/src/features/preview/components/inline-source-preview.tsx index be9e1e115..05b7ca6c3 100644 --- a/src/features/preview/components/inline-source-preview.tsx +++ b/src/features/preview/components/inline-source-preview.tsx @@ -1,4 +1,4 @@ -import { memo, useEffect, useMemo, useState } from 'react' +import { memo, useEffect, useLayoutEffect, useMemo, useState } from 'react' import { PlayerEmitterProvider, ClockBridgeProvider, @@ -11,6 +11,7 @@ import { SourceComposition } from './source-composition' import { usePlaybackStore } from '@/shared/state/playback' import { EDITOR_LAYOUT_CSS_VALUES } from '@/config/editor-layout' import { getPreviewNeedsOverflow, getPreviewPlayerSize } from '../utils/preview-pixel-snap' +import { useBlobUrlEpoch } from '@/infrastructure/browser/blob-url-manager' interface InlineSourcePreviewProps { mediaId: string @@ -53,14 +54,17 @@ const InlineSourcePreviewContent = memo(function InlineSourcePreviewContent({ }: InlineSourcePreviewProps) { const [blobUrl, setBlobUrl] = useState('') const media = useMediaLibraryStore((s) => s.mediaById[mediaId]) + const blobUrlEpoch = useBlobUrlEpoch(mediaId) const zoom = usePlaybackStore((s) => s.zoom) const mediaWidth = media?.width || 640 const mediaHeight = media?.height || 360 - useEffect(() => { - let cancelled = false + useLayoutEffect(() => { setBlobUrl('') + }, [blobUrlEpoch, mediaId]) + useEffect(() => { + let cancelled = false resolveMediaUrl(mediaId) .then((url) => { if (!cancelled) { @@ -74,7 +78,7 @@ const InlineSourcePreviewContent = memo(function InlineSourcePreviewContent({ return () => { cancelled = true } - }, [mediaId]) + }, [blobUrlEpoch, mediaId]) const containerWidth = containerSize.width const containerHeight = containerSize.height diff --git a/src/features/preview/components/source-composition.generation.test.tsx b/src/features/preview/components/source-composition.generation.test.tsx new file mode 100644 index 000000000..80ede9aec --- /dev/null +++ b/src/features/preview/components/source-composition.generation.test.tsx @@ -0,0 +1,342 @@ +import { act, cleanup, render, waitFor } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vite-plus/test' + +type Deferred = { + promise: Promise + resolve: (value: T) => void +} + +function deferred(): Deferred { + let resolve!: (value: T) => void + const promise = new Promise((done) => { + resolve = done + }) + return { promise, resolve } +} + +const decoderHarness = vi.hoisted(() => ({ + extractors: new Map< + string, + { + init: ReturnType + drawFrame: ReturnType + getDimensions: ReturnType + getDuration: ReturnType + getLastFailureKind: ReturnType + } + >(), + waitForInflightPredecodedBitmap: vi.fn(), + createImageBitmap: vi.fn(), +})) + +const clockHarness = vi.hoisted(() => ({ + clock: { + currentFrame: 0, + onFrameChange: vi.fn(() => () => {}), + }, +})) + +vi.mock('@/features/preview/deps/player-core', () => ({ + AbsoluteFill: ({ children }: { children?: React.ReactNode }) =>
{children}
, +})) + +vi.mock('@/features/preview/deps/player-context', () => ({ + useClock: () => clockHarness.clock, + useClockIsPlaying: () => false, + useClockPlaybackRate: () => 1, + usePlayer: () => ({ seek: vi.fn() }), + useVideoConfig: () => ({ fps: 30, durationInFrames: 30 }), +})) + +vi.mock('@/features/preview/deps/player-pool', () => ({ + getGlobalVideoSourcePool: () => ({ + preloadSource: vi.fn(async () => {}), + acquireForClip: vi.fn(() => null), + releaseClip: vi.fn(), + seekClip: vi.fn(), + }), +})) + +vi.mock('@/features/preview/deps/export', () => ({ + SharedVideoExtractorPool: class SharedVideoExtractorPool { + getOrCreateItemExtractor(_itemId: string, src: string) { + const extractor = decoderHarness.extractors.get(src) + if (!extractor) throw new Error(`Missing extractor for ${src}`) + return extractor + } + + releaseItem() {} + }, +})) + +vi.mock('../utils/media-resolver', () => ({ resolveProxyUrl: () => null })) +vi.mock('../utils/decoder-prewarm', () => ({ + backgroundBatchPreseek: vi.fn(async () => {}), + getCachedPredecodedBitmap: vi.fn(() => null), + waitForInflightPredecodedBitmap: decoderHarness.waitForInflightPredecodedBitmap, +})) +vi.mock('../utils/fast-scrub-prewarm', () => ({ getDirectionalPrewarmOffsets: () => [] })) +vi.mock('../utils/source-media-sync', () => ({ shouldSeekPlayingMedia: () => false })) +vi.mock('./source-audio-waveform', () => ({ SourceAudioWaveform: () => null })) +vi.mock('@/infrastructure/lottie/lottie-frame-provider', () => ({ LottieRenderer: class {} })) + +vi.mock('@/shared/state/playback', () => { + const state = { useProxy: false } + const usePlaybackStore = Object.assign( + (selector: (value: typeof state) => unknown) => selector(state), + { getState: () => state }, + ) + return { usePlaybackStore } +}) + +vi.mock('@/shared/state/source-player', () => { + const state = { + currentSourceFrame: 0, + previewSourceFrame: null as number | null, + setCurrentSourceFrame: vi.fn(), + } + const useSourcePlayerStore = Object.assign( + (selector: (value: typeof state) => unknown) => selector(state), + { + getState: () => state, + subscribe: () => () => {}, + }, + ) + return { useSourcePlayerStore } +}) + +vi.mock('@/features/preview/deps/media-library', () => ({ + useMediaLibraryStore: (selector: (state: { proxyStatus: Map }) => unknown) => + selector({ proxyStatus: new Map() }), +})) + +import { SourceComposition } from './source-composition' + +type MockCanvasContext = CanvasRenderingContext2D & { + clearRect: ReturnType + drawImage: ReturnType +} + +const canvasContexts = new WeakMap() + +function getCanvasContext(canvas: HTMLCanvasElement): MockCanvasContext { + const existing = canvasContexts.get(canvas) + if (existing) return existing + const context = { + canvas, + clearRect: vi.fn(), + drawImage: vi.fn(), + } as unknown as MockCanvasContext + canvasContexts.set(canvas, context) + return context +} + +function makeExtractor(init: Promise = Promise.resolve(true)) { + return { + init: vi.fn(() => init), + drawFrame: vi.fn(async () => true), + getDimensions: vi.fn(() => ({ width: 4, height: 4 })), + getDuration: vi.fn(() => 1), + getLastFailureKind: vi.fn(() => null), + } +} + +async function flushDeferredWork(): Promise { + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) +} + +describe('SourceComposition source generations', () => { + beforeEach(() => { + decoderHarness.extractors.clear() + decoderHarness.waitForInflightPredecodedBitmap.mockReset() + decoderHarness.waitForInflightPredecodedBitmap.mockResolvedValue(null) + decoderHarness.createImageBitmap.mockReset() + decoderHarness.createImageBitmap.mockResolvedValue({ close: vi.fn() }) + vi.stubGlobal('createImageBitmap', decoderHarness.createImageBitmap) + const getContextSpy = vi.spyOn(HTMLCanvasElement.prototype, 'getContext') + ;( + getContextSpy as unknown as { + mockImplementation: ( + implementation: ( + this: HTMLCanvasElement, + contextId: string, + ) => CanvasRenderingContext2D | null, + ) => void + } + ).mockImplementation(function (this: HTMLCanvasElement, contextId) { + return contextId === '2d' ? getCanvasContext(this) : null + }) + vi.spyOn(HTMLMediaElement.prototype, 'pause').mockImplementation(() => {}) + vi.spyOn(HTMLMediaElement.prototype, 'play').mockResolvedValue(undefined) + }) + + afterEach(() => { + cleanup() + vi.restoreAllMocks() + vi.unstubAllGlobals() + }) + + it('keeps a deferred old extractor draw off the visible canvas after a same-id src change', async () => { + const oldDraw = deferred() + const oldExtractor = makeExtractor() + oldExtractor.drawFrame.mockReturnValue(oldDraw.promise) + decoderHarness.extractors.set('blob:old', oldExtractor) + decoderHarness.extractors.set('blob:new', makeExtractor(Promise.resolve(false))) + + const rendered = render( + , + ) + await waitFor(() => expect(oldExtractor.drawFrame).toHaveBeenCalledOnce()) + const visibleCanvas = rendered.container.querySelector('canvas')! + const visibleContext = getCanvasContext(visibleCanvas) + visibleContext.clearRect.mockClear() + visibleContext.drawImage.mockClear() + + rendered.rerender() + expect(visibleContext.clearRect).toHaveBeenCalled() + + oldDraw.resolve(true) + await flushDeferredWork() + + expect(visibleContext.drawImage).not.toHaveBeenCalled() + expect(visibleCanvas.style.display).toBe('none') + expect(decoderHarness.createImageBitmap).not.toHaveBeenCalled() + }) + + it('hands the decode pump to the replacement generation after an old draw drains', async () => { + const oldDraw = deferred() + const oldExtractor = makeExtractor() + oldExtractor.drawFrame.mockReturnValue(oldDraw.promise) + const newExtractor = makeExtractor() + decoderHarness.extractors.set('blob:old', oldExtractor) + decoderHarness.extractors.set('blob:new', newExtractor) + + const rendered = render( + , + ) + await waitFor(() => expect(oldExtractor.drawFrame).toHaveBeenCalledOnce()) + const visibleCanvas = rendered.container.querySelector('canvas')! + const visibleContext = getCanvasContext(visibleCanvas) + visibleContext.drawImage.mockClear() + + rendered.rerender() + await waitFor(() => expect(newExtractor.init).toHaveBeenCalledOnce()) + expect(newExtractor.drawFrame).not.toHaveBeenCalled() + + oldDraw.resolve(true) + await flushDeferredWork() + + await waitFor(() => expect(newExtractor.drawFrame).toHaveBeenCalled()) + await waitFor(() => expect(visibleCanvas.style.display).toBe('block')) + expect(visibleContext.drawImage).toHaveBeenCalled() + expect(decoderHarness.createImageBitmap).toHaveBeenCalled() + }) + + it('closes a stale bitmap completion without marking the replacement decoded', async () => { + const staleBitmap = { close: vi.fn() } + const bitmapCompletion = deferred() + const replacementDraw = deferred() + decoderHarness.createImageBitmap.mockReturnValueOnce(bitmapCompletion.promise) + decoderHarness.extractors.set('blob:old', makeExtractor()) + const replacementExtractor = makeExtractor() + replacementExtractor.drawFrame.mockReturnValue(replacementDraw.promise) + decoderHarness.extractors.set('blob:new', replacementExtractor) + + const rendered = render( + , + ) + await waitFor(() => expect(decoderHarness.createImageBitmap).toHaveBeenCalledOnce()) + const visibleCanvas = rendered.container.querySelector('canvas')! + const visibleContext = getCanvasContext(visibleCanvas) + const drawCountBeforeReset = visibleContext.drawImage.mock.calls.length + + rendered.rerender() + bitmapCompletion.resolve(staleBitmap) + await flushDeferredWork() + + expect(staleBitmap.close).toHaveBeenCalledOnce() + expect(visibleContext.drawImage).toHaveBeenCalledTimes(drawCountBeforeReset) + expect(visibleCanvas.style.display).toBe('none') + await waitFor(() => expect(replacementExtractor.drawFrame).toHaveBeenCalledOnce()) + expect(visibleCanvas.style.display).toBe('none') + + replacementDraw.resolve(false) + await flushDeferredWork() + }) + + it('closes a deferred bitmap after unmount without repainting the retired canvas', async () => { + const staleBitmap = { close: vi.fn() } + const bitmapCompletion = deferred() + decoderHarness.createImageBitmap.mockReturnValueOnce(bitmapCompletion.promise) + decoderHarness.extractors.set('blob:old', makeExtractor()) + + const rendered = render( + , + ) + await waitFor(() => expect(decoderHarness.createImageBitmap).toHaveBeenCalledOnce()) + const visibleCanvas = rendered.container.querySelector('canvas')! + const visibleContext = getCanvasContext(visibleCanvas) + const drawCountBeforeUnmount = visibleContext.drawImage.mock.calls.length + const clearCountBeforeUnmount = visibleContext.clearRect.mock.calls.length + + rendered.unmount() + expect(visibleContext.clearRect.mock.calls.length).toBeGreaterThan(clearCountBeforeUnmount) + + bitmapCompletion.resolve(staleBitmap) + await flushDeferredWork() + + expect(staleBitmap.close).toHaveBeenCalledOnce() + expect(visibleContext.drawImage).toHaveBeenCalledTimes(drawCountBeforeUnmount) + }) + + it('ignores an in-flight shared-cache completion from the old source', async () => { + const sharedCompletion = deferred<{ close: ReturnType } | null>() + decoderHarness.waitForInflightPredecodedBitmap.mockReturnValueOnce(sharedCompletion.promise) + const oldExtractor = makeExtractor() + decoderHarness.extractors.set('blob:old', oldExtractor) + decoderHarness.extractors.set('blob:new', makeExtractor(Promise.resolve(false))) + + const rendered = render( + , + ) + await waitFor(() => + expect(decoderHarness.waitForInflightPredecodedBitmap).toHaveBeenCalledOnce(), + ) + const visibleCanvas = rendered.container.querySelector('canvas')! + const visibleContext = getCanvasContext(visibleCanvas) + visibleContext.drawImage.mockClear() + + rendered.rerender() + sharedCompletion.resolve({ close: vi.fn() }) + await flushDeferredWork() + + expect(oldExtractor.drawFrame).not.toHaveBeenCalled() + expect(visibleContext.drawImage).not.toHaveBeenCalled() + expect(visibleCanvas.style.display).toBe('none') + }) + + it('ignores old extractor initialization after unmount and remount', async () => { + const oldInit = deferred() + const oldExtractor = makeExtractor(oldInit.promise) + decoderHarness.extractors.set('blob:old', oldExtractor) + decoderHarness.extractors.set('blob:new', makeExtractor(Promise.resolve(false))) + + const oldRender = render( + , + ) + await waitFor(() => expect(oldExtractor.init).toHaveBeenCalledOnce()) + oldRender.unmount() + + const newRender = render( + , + ) + oldInit.resolve(true) + await flushDeferredWork() + + expect(oldExtractor.drawFrame).not.toHaveBeenCalled() + expect(newRender.container.querySelector('canvas')?.style.display).toBe('none') + }) +}) diff --git a/src/features/preview/components/source-composition.tsx b/src/features/preview/components/source-composition.tsx index 8fcc5cfb7..d1328f21d 100644 --- a/src/features/preview/components/source-composition.tsx +++ b/src/features/preview/components/source-composition.tsx @@ -1,4 +1,4 @@ -import { useRef, useEffect, useState, useMemo, useCallback } from 'react' +import { useRef, useEffect, useLayoutEffect, useState, useMemo, useCallback } from 'react' import { AbsoluteFill } from '@/features/preview/deps/player-core' import { useClock, @@ -101,26 +101,45 @@ export function SourceComposition({ function LottieSource({ src }: { src: string }) { const canvasRef = useRef(null) + const sourceGenerationRef = useRef(0) const clock = useClock() + useLayoutEffect(() => { + const generation = ++sourceGenerationRef.current + const canvas = canvasRef.current + canvas?.getContext('2d')?.clearRect(0, 0, canvas.width, canvas.height) + if (canvas) canvas.style.display = 'none' + return () => { + if (sourceGenerationRef.current !== generation) return + sourceGenerationRef.current += 1 + canvas?.getContext('2d')?.clearRect(0, 0, canvas.width, canvas.height) + if (canvas) canvas.style.display = 'none' + } + }, [src]) + useEffect(() => { const canvas = canvasRef.current if (!canvas || !src) return + const generation = sourceGenerationRef.current + const isCurrent = () => sourceGenerationRef.current === generation const renderer = new LottieRenderer({ canvas, src, autoResize: true }) let raf = 0 let lastFrame = -1 let loaded = false renderer.ready.then(() => { - loaded = renderer.isLoaded + if (isCurrent()) loaded = renderer.isLoaded }) // Drive frames from the source clock imperatively (no per-frame React render). const tick = () => { + if (!isCurrent()) return if (loaded) { const total = renderer.totalFrames const frame = total > 0 ? Math.max(0, Math.min(Math.round(clock.currentFrame), total - 1)) : 0 if (frame !== lastFrame) { renderer.renderFrame(frame) + if (!isCurrent()) return + canvas.style.display = 'block' lastFrame = frame } } @@ -135,7 +154,7 @@ function LottieSource({ src }: { src: string }) { return ( - + ) } @@ -175,8 +194,10 @@ function VideoSource({ const canvasRef = useRef(null) const contextRef = useRef(null) const mountedRef = useRef(true) + const sourceGenerationRef = useRef(0) const decoderReadyRef = useRef(false) - const renderInFlightRef = useRef(false) + const renderInFlightGenerationRef = useRef(null) + const pumpLatestDecodedFrameRef = useRef<() => void>(() => {}) const pendingTimeRef = useRef(null) const latestTargetTimeRef = useRef(0) const consecutiveDecodeFailuresRef = useRef(0) @@ -246,7 +267,23 @@ function VideoSource({ } }, []) - useEffect(() => { + useLayoutEffect(() => { + const generation = ++sourceGenerationRef.current + mountedRef.current = true + decoderReadyRef.current = false + extractorRef.current = null + pendingTimeRef.current = null + contextRef.current = null + prewarmInFlightRef.current = false + queuedPrewarmTimesRef.current = [] + prewarmAnchorFrameRef.current = null + for (const bitmap of frameCacheRef.current.values()) bitmap.close() + frameCacheRef.current.clear() + frameCacheOrderRef.current = [] + const canvas = canvasRef.current + const context = canvas?.getContext('2d') + if (canvas && context) context.clearRect(0, 0, canvas.width, canvas.height) + const resetCanvas = canvas setUseLegacyPausedSeek(false) setHasDecodedFrame(false) setDecodedFrameKey(null) @@ -254,6 +291,16 @@ function VideoSource({ prewarmInFlightRef.current = false queuedPrewarmTimesRef.current = [] prewarmAnchorFrameRef.current = null + return () => { + if (sourceGenerationRef.current === generation) { + sourceGenerationRef.current += 1 + const currentCanvas = resetCanvas + const currentContext = currentCanvas?.getContext('2d') + if (currentCanvas && currentContext) { + currentContext.clearRect(0, 0, currentCanvas.width, currentCanvas.height) + } + } + } }, [activeSrc, mediaId]) const pumpDirectionalPrewarm = useCallback(() => { @@ -365,6 +412,11 @@ function VideoSource({ const extractor = extractorRef.current const canvas = canvasRef.current if (!extractor || !canvas) return false + const generation = sourceGenerationRef.current + const isCurrent = () => + mountedRef.current && + sourceGenerationRef.current === generation && + extractorRef.current === extractor let ctx = contextRef.current if (!ctx) { @@ -382,10 +434,21 @@ function VideoSource({ canvas.height = targetHeight } + // Keep deferred extractor work away from the visible presentation. A + // decoder can paint before its promise settles, so only commit staged + // pixels after validating the source generation. + const stagingCanvas = document.createElement('canvas') + stagingCanvas.width = targetWidth + stagingCanvas.height = targetHeight + const stagingContext = stagingCanvas.getContext('2d') + if (!stagingContext) return false + const cacheKey = quantizeSourceMonitorTime(targetTime) const markDecodedFrame = () => { + if (!isCurrent()) return false setHasDecodedFrame(true) setDecodedFrameKey((prev) => (prev === cacheKey ? prev : cacheKey)) + return true } const cache = frameCacheRef.current const cacheOrder = frameCacheOrderRef.current @@ -398,8 +461,7 @@ function VideoSource({ cacheOrder.splice(cacheIndex, 1) cacheOrder.push(cacheKey) } - markDecodedFrame() - return true + return markDecodedFrame() } const drawSharedBitmap = (bitmap: ImageBitmap): boolean => { @@ -425,24 +487,36 @@ function VideoSource({ SOURCE_MONITOR_CACHE_TIME_QUANTUM, SOURCE_MONITOR_SHARED_CACHE_WAIT_MS, ).catch(() => null) - if (inflightBitmap && drawSharedBitmap(inflightBitmap)) { - markDecodedFrame() - return true + if (inflightBitmap && isCurrent() && drawSharedBitmap(inflightBitmap)) { + return markDecodedFrame() } } + if (!isCurrent()) return false + const didDraw = await extractor.drawFrame( - ctx, + stagingContext, Math.max(0, targetTime), 0, 0, - canvas.width, - canvas.height, + stagingCanvas.width, + stagingCanvas.height, ) + if (!isCurrent()) { + ctx.clearRect(0, 0, canvas.width, canvas.height) + return false + } if (!didDraw) return false + ctx.clearRect(0, 0, canvas.width, canvas.height) + ctx.drawImage(stagingCanvas, 0, 0, canvas.width, canvas.height) + try { - const bitmap = await createImageBitmap(canvas) + const bitmap = await createImageBitmap(stagingCanvas) + if (!isCurrent()) { + bitmap.close() + return false + } cache.set(cacheKey, bitmap) cacheOrder.push(cacheKey) while (cacheOrder.length > SOURCE_MONITOR_FRAME_CACHE_MAX) { @@ -457,19 +531,42 @@ function VideoSource({ // Cache population is best-effort only. } - markDecodedFrame() - return true + return markDecodedFrame() }, [activeSrc], ) + const commitDecodedFrameResult = useCallback( + (didDraw: boolean, targetTime: number): boolean => { + if (didDraw) { + consecutiveDecodeFailuresRef.current = 0 + queueDirectionalPrewarm(targetTime) + return true + } + + if (extractorRef.current?.getLastFailureKind() !== 'decode-error') return true + consecutiveDecodeFailuresRef.current += 1 + if (consecutiveDecodeFailuresRef.current < SOURCE_MONITOR_STRICT_DECODE_FALLBACK_FAILURES) { + return true + } + + decoderReadyRef.current = false + setStrictDecodeReady(false) + setUseLegacyPausedSeek((prev) => (prev ? prev : true)) + return false + }, + [queueDirectionalPrewarm], + ) + const pumpLatestDecodedFrame = useCallback(() => { - if (renderInFlightRef.current) return - renderInFlightRef.current = true + if (renderInFlightGenerationRef.current !== null) return + const generation = sourceGenerationRef.current + renderInFlightGenerationRef.current = generation const run = async () => { try { while ( + sourceGenerationRef.current === generation && decoderReadyRef.current && pendingTimeRef.current !== null && mountedRef.current && @@ -479,28 +576,17 @@ function VideoSource({ pendingTimeRef.current = null const didDraw = await drawDecodedFrame(targetTime).catch(() => false) - if (didDraw) { - consecutiveDecodeFailuresRef.current = 0 - queueDirectionalPrewarm(targetTime) - continue - } - - const failureKind = extractorRef.current?.getLastFailureKind() ?? 'decode-error' - if (failureKind === 'decode-error') { - consecutiveDecodeFailuresRef.current += 1 - if ( - consecutiveDecodeFailuresRef.current >= SOURCE_MONITOR_STRICT_DECODE_FALLBACK_FAILURES - ) { - decoderReadyRef.current = false - setStrictDecodeReady(false) - setUseLegacyPausedSeek((prev) => (prev ? prev : true)) - return - } + if (!mountedRef.current || sourceGenerationRef.current !== generation) { + return } + if (!commitDecodedFrameResult(didDraw, targetTime)) return } } finally { - renderInFlightRef.current = false + if (renderInFlightGenerationRef.current === generation) { + renderInFlightGenerationRef.current = null + } if ( + renderInFlightGenerationRef.current === null && decoderReadyRef.current && pendingTimeRef.current !== null && mountedRef.current && @@ -508,14 +594,15 @@ function VideoSource({ ) { queueMicrotask(() => { if (!mountedRef.current) return - pumpLatestDecodedFrame() + pumpLatestDecodedFrameRef.current() }) } } } void run() - }, [drawDecodedFrame, queueDirectionalPrewarm]) + }, [commitDecodedFrameResult, drawDecodedFrame]) + pumpLatestDecodedFrameRef.current = pumpLatestDecodedFrame // Acquire/release pooled element when source changes. useEffect(() => { @@ -579,12 +666,19 @@ function VideoSource({ const pool = decoderPoolRef.current const extractor = pool.getOrCreateItemExtractor(decoderItemId, activeSrc) extractorRef.current = extractor + const generation = sourceGenerationRef.current let cancelled = false void extractor .init() .then((ready) => { - if (cancelled || !mountedRef.current) return + if ( + cancelled || + !mountedRef.current || + sourceGenerationRef.current !== generation || + extractorRef.current !== extractor + ) + return if (!ready) { setUseLegacyPausedSeek((prev) => (prev ? prev : true)) return @@ -597,7 +691,13 @@ function VideoSource({ } }) .catch(() => { - if (cancelled || !mountedRef.current) return + if ( + cancelled || + !mountedRef.current || + sourceGenerationRef.current !== generation || + extractorRef.current !== extractor + ) + return setUseLegacyPausedSeek((prev) => (prev ? prev : true)) }) @@ -826,6 +926,7 @@ function ImageSource({ src }: { src: string }) { return ( Source preview ({ + globalVersion: 0, + epochs: new Map(), + resolveMediaUrl: vi.fn<(mediaId: string) => Promise>(), + compositionMounts: 0, + compositionUnmounts: 0, + listeners: new Set<() => void>(), + publish: () => { + for (const listener of sourceBindingState.listeners) listener() + }, +})) const editorStoreState = vi.hoisted(() => ({ sourcePreviewMediaId: 'media-1' as string | null, @@ -78,7 +90,15 @@ vi.mock('@/features/preview/deps/player-context', () => ({ })) vi.mock('./source-composition', () => ({ - SourceComposition: () =>
, + SourceComposition: ({ src }: { src: string }) => { + useEffect(() => { + sourceBindingState.compositionMounts += 1 + return () => { + sourceBindingState.compositionUnmounts += 1 + } + }, []) + return
+ }, })) vi.mock('@/components/ui/tooltip', () => ({ @@ -97,7 +117,26 @@ vi.mock('@/components/ui/dropdown-menu', () => ({ })) vi.mock('../utils/media-resolver', () => ({ - resolveMediaUrl: vi.fn().mockResolvedValue('blob:media-1'), + resolveMediaUrl: sourceBindingState.resolveMediaUrl, +})) + +vi.mock('@/infrastructure/browser/blob-url-manager', () => ({ + useBlobUrlVersion: () => + useSyncExternalStore( + (listener) => { + sourceBindingState.listeners.add(listener) + return () => sourceBindingState.listeners.delete(listener) + }, + () => sourceBindingState.globalVersion, + ), + useBlobUrlEpoch: (mediaId: string) => + useSyncExternalStore( + (listener) => { + sourceBindingState.listeners.add(listener) + return () => sourceBindingState.listeners.delete(listener) + }, + () => String(sourceBindingState.epochs.get(mediaId) ?? 0), + ), })) vi.mock('@/features/preview/deps/media-library', () => { @@ -192,6 +231,12 @@ describe('SourceMonitor current media ownership', () => { beforeEach(() => { vi.clearAllMocks() + sourceBindingState.globalVersion = 0 + sourceBindingState.epochs.clear() + sourceBindingState.resolveMediaUrl.mockResolvedValue('blob:media-1') + sourceBindingState.compositionMounts = 0 + sourceBindingState.compositionUnmounts = 0 + sourceBindingState.listeners.clear() editorStoreState.sourcePreviewMediaId = 'media-1' clockState.currentFrame = 0 clockState.isPlaying = false @@ -283,4 +328,66 @@ describe('SourceMonitor current media ownership', () => { expect(playerMethodsState.pause).toHaveBeenCalledTimes(1) }) + + it('keeps the current source generation mounted across unrelated blob URL activity', async () => { + const rendered = render() + + await waitFor(() => { + expect(rendered.getByTestId('source-composition')).toHaveAttribute( + 'data-source', + 'blob:media-1', + ) + }) + expect(sourceBindingState.resolveMediaUrl).toHaveBeenCalledTimes(1) + expect(sourceBindingState.compositionMounts).toBe(1) + + act(() => { + sourceBindingState.globalVersion += 1 + sourceBindingState.publish() + }) + await act(async () => { + await Promise.resolve() + }) + + expect(sourceBindingState.resolveMediaUrl).toHaveBeenCalledTimes(1) + expect(sourceBindingState.compositionMounts).toBe(1) + expect(sourceBindingState.compositionUnmounts).toBe(0) + expect(rendered.getByTestId('source-composition')).toHaveAttribute( + 'data-source', + 'blob:media-1', + ) + }) + + it('retires and resolves a relevant source epoch exactly once', async () => { + let resolveReplacement!: (url: string) => void + const replacement = new Promise((resolve) => { + resolveReplacement = resolve + }) + sourceBindingState.resolveMediaUrl + .mockResolvedValueOnce('blob:old') + .mockReturnValueOnce(replacement) + const rendered = render() + + await waitFor(() => { + expect(rendered.getByTestId('source-composition')).toHaveAttribute('data-source', 'blob:old') + }) + + act(() => { + sourceBindingState.epochs.set('media-1', 1) + sourceBindingState.globalVersion += 1 + sourceBindingState.publish() + }) + + expect(rendered.queryByTestId('source-composition')).toBeNull() + await waitFor(() => expect(sourceBindingState.resolveMediaUrl).toHaveBeenCalledTimes(2)) + expect(sourceBindingState.compositionUnmounts).toBe(1) + + await act(async () => { + resolveReplacement('blob:new') + await replacement + }) + + expect(rendered.getByTestId('source-composition')).toHaveAttribute('data-source', 'blob:new') + expect(sourceBindingState.resolveMediaUrl).toHaveBeenCalledTimes(2) + }) }) diff --git a/src/features/preview/components/source-monitor.tsx b/src/features/preview/components/source-monitor.tsx index 1063dcd78..6791c60c3 100644 --- a/src/features/preview/components/source-monitor.tsx +++ b/src/features/preview/components/source-monitor.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect, useRef, useCallback, useMemo, memo } from 'react' +import { useState, useEffect, useLayoutEffect, useRef, useCallback, useMemo, memo } from 'react' import { X, Play, @@ -70,6 +70,7 @@ import { import { formatTimecodeCompact } from '@/shared/utils/time-utils' import { getPreviewPixelSnapSize } from '../utils/preview-pixel-snap' import type { TimelineTrack } from '@/types/timeline' +import { useBlobUrlEpoch } from '@/infrastructure/browser/blob-url-manager' interface SourceMonitorProps { mediaId: string @@ -205,6 +206,7 @@ const SourceMonitorContent = memo(function SourceMonitorContent({ }: SourceMonitorProps) { const [blobUrl, setBlobUrl] = useState('') const media = useMediaLibraryStore((s) => s.mediaById[mediaId]) + const blobUrlEpoch = useBlobUrlEpoch(mediaId) // Sync current media ID into source player store for I/O points useEffect(() => { @@ -228,8 +230,14 @@ const SourceMonitorContent = memo(function SourceMonitorContent({ } }, [media, onClose]) - // Resolve the original source URL once. SourceComposition can swap to a - // ready proxy for video preview without losing the original fallback URL. + // Blank the retired source in layout so a same-ID relink cannot leave its + // canvas visible while the replacement URL resolves. + useLayoutEffect(() => { + setBlobUrl('') + }, [blobUrlEpoch, mediaId]) + + // SourceComposition can swap to a ready proxy for video preview without + // losing the original fallback URL. Blob invalidation retries the same ID. useEffect(() => { let cancelled = false resolveMediaUrl(mediaId) @@ -242,7 +250,7 @@ const SourceMonitorContent = memo(function SourceMonitorContent({ return () => { cancelled = true } - }, [mediaId]) + }, [blobUrlEpoch, mediaId]) if (!media) return null diff --git a/src/features/preview/components/video-preview.sync.test.tsx b/src/features/preview/components/video-preview.sync.test.tsx index c47e53631..08fa70f36 100644 --- a/src/features/preview/components/video-preview.sync.test.tsx +++ b/src/features/preview/components/video-preview.sync.test.tsx @@ -22,6 +22,7 @@ const playMock = vi.fn() const pauseMock = vi.fn() const mockState = vi.hoisted(() => { const blobUrls = new Map() + const mediaEpochs = new Map() const listeners = new Set<() => void>() const version = { current: 0 } const resolveMediaUrlMock = vi.fn(async (mediaId: string) => blobUrls.get(mediaId) ?? '') @@ -40,6 +41,7 @@ const mockState = vi.hoisted(() => { if (url === null) { blobUrls.delete(mediaId) + mediaEpochs.set(mediaId, (mediaEpochs.get(mediaId) ?? 0) + 1) } else { blobUrls.set(mediaId, url) } @@ -55,6 +57,7 @@ const mockState = vi.hoisted(() => { return { blobUrls, + mediaEpochs, listeners, version, resolveMediaUrlMock, @@ -67,12 +70,26 @@ const mockState = vi.hoisted(() => { const { blobUrls: mockBlobUrls, + mediaEpochs: mockMediaEpochs, listeners: blobUrlListeners, version: mockBlobUrlVersion, resolveMediaUrlMock, resolveProxyUrlMock, setBlobUrl: setMockBlobUrl, } = mockState + +function BlobBindingLayoutProbe({ onLayout }: { onLayout: (version: number) => void }) { + const version = React.useSyncExternalStore( + mockState.subscribeVersion, + () => mockState.version.current, + ) + + React.useLayoutEffect(() => { + onLayout(version) + }, [onLayout, version]) + + return null +} let mockedPlayerFrame = 0 let mockedPlayerIsPlaying = false let deferPlayerSeekCompletion = false @@ -104,36 +121,55 @@ const rendererMockState = vi.hoisted(() => { } const instances: RendererMock[] = [] + const renderedSources: Array<{ frame: number; src: string | null }> = [] const getBestDomVideoElementForItem = vi.fn<(itemId: string) => HTMLVideoElement | null>( () => null, ) - const create = vi.fn(async () => { - const prewarmFrame = vi.fn(async (frame: number) => { - void frame - }) - const renderer: RendererMock = { - preload: vi.fn(async () => {}), - renderFrame: vi.fn(async () => {}), - prewarmFrame, - prewarmFrames: vi.fn(async (frames: number[]) => { - for (const frame of frames) { - await prewarmFrame(frame) - } - }), - invalidateFrameCache: vi.fn(), - setDomVideoElementProvider: vi.fn(), - wasLastRenderAborted: vi.fn(() => false), - getScrubbingCache: () => null, - dispose: vi.fn(), - } - instances.push(renderer) - return renderer - }) + const create = vi.fn( + async (inputProps?: { + tracks?: Array<{ + items?: Array<{ type?: string; from?: number; durationInFrames?: number; src?: string }> + }> + }) => { + const prewarmFrame = vi.fn(async (frame: number) => { + void frame + }) + const renderFrame = vi.fn(async (frame: number) => { + const activeItem = (inputProps?.tracks ?? []) + .flatMap((track) => track.items ?? []) + .find( + (item) => + item.type === 'video' && + frame >= (item.from ?? 0) && + frame < (item.from ?? 0) + (item.durationInFrames ?? 0), + ) + renderedSources.push({ frame, src: activeItem?.src ?? null }) + }) + const renderer: RendererMock = { + preload: vi.fn(async () => {}), + renderFrame, + prewarmFrame, + prewarmFrames: vi.fn(async (frames: number[]) => { + for (const frame of frames) { + await prewarmFrame(frame) + } + }), + invalidateFrameCache: vi.fn(), + setDomVideoElementProvider: vi.fn(), + wasLastRenderAborted: vi.fn(() => false), + getScrubbingCache: () => null, + dispose: vi.fn(), + } + instances.push(renderer) + return renderer + }, + ) return { create, getBestDomVideoElementForItem, instances, + renderedSources, } }) @@ -257,6 +293,7 @@ vi.mock('@/infrastructure/browser/blob-url-manager', async () => { return { blobUrlManager: { get: (mediaId: string) => mockState.blobUrls.get(mediaId) ?? null, + getEpoch: (mediaId: string) => String(mockState.mediaEpochs.get(mediaId) ?? 0), getMediaIdByUrl: (url: string) => [...mockState.blobUrls.entries()].find(([, candidate]) => candidate === url)?.[0] ?? null, has: (mediaId: string) => mockState.blobUrls.has(mediaId), @@ -274,9 +311,9 @@ vi.mock('@/infrastructure/browser/blob-url-manager', async () => { } }, invalidate: (mediaId: string) => { - if (mockState.blobUrls.delete(mediaId)) { - mockState.publishVersion() - } + mockState.blobUrls.delete(mediaId) + mockState.mediaEpochs.set(mediaId, (mockState.mediaEpochs.get(mediaId) ?? 0) + 1) + mockState.publishVersion() }, invalidateAll: () => { if (mockState.blobUrls.size === 0) return @@ -534,6 +571,61 @@ function getCanvasDrawImageCallCount() { ) } +function getCanvasClearRectCallCount(canvas: HTMLCanvasElement) { + const results = canvasGetContextSpy?.mock.results as + | Array<{ type: string; value: unknown }> + | undefined + return ( + results?.reduce((total: number, result) => { + if (result.type !== 'return' || !result.value) return total + const context = result.value as { + canvas?: HTMLCanvasElement + clearRect?: unknown + } + if (context.canvas !== canvas || typeof context.clearRect !== 'function') return total + if (!('mock' in context.clearRect)) return total + return total + (context.clearRect as { mock: { calls: unknown[] } }).mock.calls.length + }, 0) ?? 0 + ) +} + +function getCanvasDrawImageCallCountFor(canvas: HTMLCanvasElement) { + const results = canvasGetContextSpy?.mock.results as + | Array<{ type: string; value: unknown }> + | undefined + return ( + results?.reduce((total: number, result) => { + if (result.type !== 'return' || !result.value) return total + const context = result.value as { + canvas?: HTMLCanvasElement + drawImage?: unknown + } + if (context.canvas !== canvas || typeof context.drawImage !== 'function') return total + if (!('mock' in context.drawImage)) return total + return total + (context.drawImage as { mock: { calls: unknown[] } }).mock.calls.length + }, 0) ?? 0 + ) +} + +function getCanvasDrawImageCallCountFrom(canvas: HTMLCanvasElement, source: CanvasImageSource) { + const results = canvasGetContextSpy?.mock.results as + | Array<{ type: string; value: unknown }> + | undefined + return ( + results?.reduce((total: number, result) => { + if (result.type !== 'return' || !result.value) return total + const context = result.value as { + canvas?: HTMLCanvasElement + drawImage?: unknown + } + if (context.canvas !== canvas || typeof context.drawImage !== 'function') return total + if (!('mock' in context.drawImage)) return total + const calls = (context.drawImage as { mock: { calls: unknown[][] } }).mock.calls + return total + calls.filter(([drawSource]) => drawSource === source).length + }, 0) ?? 0 + ) +} + function resetStores() { usePlaybackStore.setState({ currentFrame: 0, @@ -903,12 +995,14 @@ describe('VideoPreview sync behavior', () => { completeDeferredPlayerSeek = null lastPlayerDimensions = null playerDimensionsHistory = [] + rendererMockState.renderedSources.length = 0 seekToMock.mockReset() playMock.mockReset() pauseMock.mockReset() lastCompositionKeyframes = [] lastCompositionMediaSources = [] mockBlobUrls.clear() + mockMediaEpochs.clear() blobUrlListeners.clear() mockBlobUrlVersion.current = 0 resolveMediaUrlMock.mockClear() @@ -1464,6 +1558,94 @@ describe('VideoPreview sync behavior', () => { }) }) + it('clears and invalidates an in-flight same-item source render before replacement starts', async () => { + canvasPixelReadbackEnabled = true + const makeItem = (src: string) => ({ + id: 'same-source-item', + label: 'Same source item', + src, + effects: [ + { + id: 'effect-source-generation', + enabled: true, + effect: { type: 'gpu-effect', gpuEffectType: 'gpu-sepia', params: { amount: 0.5 } }, + }, + ], + }) + setSingleVideoItemAtFrame(makeItem('blob:old-source')) + const { renderer, scrubCanvas } = await renderReadySingleRendererPreview(24, { + expectedDisplayedFrame: 24, + }) + setMockCanvasBlank(scrubCanvas, false) + + let resolveOldRender: (() => void) | null = null + renderer.renderFrame.mockImplementation(async (frame: number) => { + if (frame !== 25) return + await new Promise((resolve) => { + resolveOldRender = resolve + }) + }) + act(() => { + usePlaybackStore.getState().setPreviewFrame(25) + }) + await waitFor(() => { + expect(renderer.renderFrame).toHaveBeenCalledWith(25) + expect(resolveOldRender).not.toBeNull() + }) + + const clearCountBeforeReplacement = getCanvasClearRectCallCount(scrubCanvas) + const rendererCalls = createCompositionRendererMock.mock.calls as unknown as Array< + [unknown, HTMLCanvasElement] + > + const oldOffscreen = rendererCalls[0]![1] + const replacementRenderer = createRendererDouble() + let resolveReplacementInit: (() => void) | null = null + let oldGenerationRetiredBeforeReplacement = false + createCompositionRendererMock.mockImplementationOnce( + () => + new Promise((resolve) => { + oldGenerationRetiredBeforeReplacement = + renderer.dispose.mock.calls.length > 0 && + getCanvasClearRectCallCount(scrubCanvas) > clearCountBeforeReplacement && + blankCanvasState.has(scrubCanvas) + resolveReplacementInit = () => { + rendererMockState.instances.push(replacementRenderer) + resolve(replacementRenderer) + } + }), + ) + + act(() => { + useItemsStore.getState().setItems([ + { + type: 'video', + trackId: 'track-video', + from: 0, + durationInFrames: 120, + ...makeItem('blob:new-source'), + } as unknown as TimelineItem, + ]) + }) + + await waitFor(() => expect(createCompositionRendererMock).toHaveBeenCalledTimes(2)) + expect(oldGenerationRetiredBeforeReplacement).toBe(true) + + setMockCanvasBlank(oldOffscreen, false) + await act(async () => { + resolveOldRender?.() + await Promise.resolve() + await Promise.resolve() + }) + expect(blankCanvasState.has(scrubCanvas)).toBe(true) + expect(getDisplayedFrame()).not.toBe(25) + + await act(async () => { + resolveReplacementInit?.() + await Promise.resolve() + }) + await waitFor(() => expect(replacementRenderer.renderFrame).toHaveBeenCalledWith(25)) + }) + it('prepares the initial playback lookahead without replacing the visible paused frame', async () => { setSingleVideoItemAtFrame({ id: 'item-initial-lookahead', @@ -1962,6 +2144,399 @@ describe('VideoPreview sync behavior', () => { }) }) + it('retires a deferred split-grade surface before same-item source replacement', async () => { + canvasPixelReadbackEnabled = true + const gradeEffect = { + id: 'effect-grade', + enabled: true, + effect: { + type: 'gpu-effect' as const, + gpuEffectType: 'gpu-color-wheels' as const, + params: { exposure: 0.5 }, + }, + } + setSingleVideoItemAtFrame({ + id: 'item-graded-replacement', + src: 'blob:old-graded-source', + effects: [gradeEffect], + }) + + const { container } = renderDefaultPreview() + await waitFor(() => expect(rendererMockState.instances).toHaveLength(1)) + act(() => { + useGizmoStore.getState().setColorGradeComparisonMode('split') + }) + + const splitRenderer = await waitFor(() => { + expect(rendererMockState.instances).toHaveLength(2) + expect(container.querySelector('[data-grade-comparison-after-layer="true"]')).not.toBeNull() + return rendererMockState.instances[1]! + }) + const rendererCalls = createCompositionRendererMock.mock.calls as unknown as Array< + [unknown, HTMLCanvasElement] + > + const oldSplitOffscreen = rendererCalls[1]![1] + const gpuDisplayCanvas = container.querySelectorAll('canvas')[1] as HTMLCanvasElement + setMockCanvasBlank(oldSplitOffscreen, false) + setMockCanvasBlank(gpuDisplayCanvas, false) + + let resolveOldRender: (() => void) | null = null + splitRenderer.renderFrame.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveOldRender = resolve + }), + ) + act(() => { + useGizmoStore.getState().setEffectsPreviewNew({ + 'item-graded-replacement': [ + { ...gradeEffect, effect: { ...gradeEffect.effect, params: { exposure: 0.8 } } }, + ], + }) + }) + await waitFor(() => expect(resolveOldRender).not.toBeNull()) + + const clearCountBeforeReplacement = getCanvasClearRectCallCount(gpuDisplayCanvas) + const drawCountBeforeReplacement = getCanvasDrawImageCallCountFor(gpuDisplayCanvas) + const defaultRendererFactory = createCompositionRendererMock.getMockImplementation()! + createCompositionRendererMock.mockImplementation(() => new Promise(() => undefined)) + + act(() => { + useItemsStore.getState().setItems([ + { + id: 'item-graded-replacement', + type: 'video', + trackId: 'track-video', + from: 0, + durationInFrames: 120, + src: 'blob:new-graded-source', + effects: [gradeEffect], + } as unknown as TimelineItem, + ]) + }) + + await waitFor(() => expect(splitRenderer.dispose).toHaveBeenCalledOnce()) + expect(getCanvasClearRectCallCount(gpuDisplayCanvas)).toBeGreaterThan( + clearCountBeforeReplacement, + ) + expect(blankCanvasState.has(gpuDisplayCanvas)).toBe(true) + expect(gpuDisplayCanvas.style.visibility).toBe('hidden') + expect(container.querySelector('[data-grade-comparison-after-layer="true"]')).toBeNull() + + await act(async () => { + resolveOldRender?.() + await Promise.resolve() + await Promise.resolve() + }) + expect(getCanvasDrawImageCallCountFor(gpuDisplayCanvas)).toBe(drawCountBeforeReplacement) + expect(blankCanvasState.has(gpuDisplayCanvas)).toBe(true) + expect(gpuDisplayCanvas.style.visibility).toBe('hidden') + createCompositionRendererMock.mockImplementation(defaultRendererFactory) + }) + + it('retires a split-grade source binding in layout before resolver effects can reuse it', async () => { + canvasPixelReadbackEnabled = true + const mediaId = 'same-media-relink' + const oldUrl = 'blob:old-relink-source' + const newUrl = 'blob:new-relink-source' + const oldPresentationUrl = 'blob:old-relink-proxy' + const newPresentationUrl = 'blob:new-relink-proxy' + const gradeEffect = { + id: 'effect-grade', + enabled: true, + effect: { + type: 'gpu-effect' as const, + gpuEffectType: 'gpu-color-wheels' as const, + params: { exposure: 0.5 }, + }, + } + resolveProxyUrlMock.mockImplementation((candidateId) => { + if (candidateId !== mediaId) return null + return mockBlobUrls.get(mediaId) === newUrl ? newPresentationUrl : oldPresentationUrl + }) + setMockBlobUrl(mediaId, oldUrl) + setSingleVideoItemAtFrame({ + id: 'item-same-media-relink', + mediaId, + src: oldUrl, + effects: [gradeEffect], + }) + + let observeRelinkLayout = false + let layoutObservation: + | { + blank: boolean + hidden: boolean + oldRendererDisposed: boolean + resolveCallCount: number + } + | undefined + let gpuDisplayCanvas: HTMLCanvasElement | null = null + let oldSplitRenderer: (typeof rendererMockState.instances)[number] | null = null + const onBindingLayout = () => { + if (!observeRelinkLayout || !gpuDisplayCanvas || !oldSplitRenderer) return + layoutObservation = { + blank: blankCanvasState.has(gpuDisplayCanvas), + hidden: gpuDisplayCanvas.style.visibility === 'hidden', + oldRendererDisposed: oldSplitRenderer.dispose.mock.calls.length === 1, + resolveCallCount: resolveMediaUrlMock.mock.calls.length, + } + } + + const { container } = render( + <> + + + , + ) + await waitFor(() => expect(rendererMockState.instances).toHaveLength(1)) + act(() => { + useGizmoStore.getState().setColorGradeComparisonMode('split') + }) + + oldSplitRenderer = await waitFor(() => { + expect(rendererMockState.instances).toHaveLength(2) + expect(container.querySelector('[data-grade-comparison-after-layer="true"]')).not.toBeNull() + return rendererMockState.instances[1]! + }) + const oldSplitCall = createCompositionRendererMock.mock.calls[1] as unknown as [ + unknown, + HTMLCanvasElement, + ] + gpuDisplayCanvas = container.querySelectorAll('canvas')[1] as HTMLCanvasElement + setMockCanvasBlank(oldSplitCall[1], false) + setMockCanvasBlank(gpuDisplayCanvas, false) + + let resolveOldRender: (() => void) | null = null + oldSplitRenderer.renderFrame.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveOldRender = resolve + }), + ) + act(() => { + useGizmoStore.getState().setEffectsPreviewNew({ + 'item-same-media-relink': [ + { ...gradeEffect, effect: { ...gradeEffect.effect, params: { exposure: 0.8 } } }, + ], + }) + }) + await waitFor(() => expect(resolveOldRender).not.toBeNull()) + + const resolveCallsBeforeInvalidation = resolveMediaUrlMock.mock.calls.length + observeRelinkLayout = true + act(() => { + setMockBlobUrl(mediaId, null) + }) + + expect(layoutObservation).toEqual({ + blank: true, + hidden: true, + oldRendererDisposed: true, + resolveCallCount: resolveCallsBeforeInvalidation, + }) + await waitFor(() => { + expect(resolveMediaUrlMock.mock.calls.length).toBeGreaterThan(resolveCallsBeforeInvalidation) + }) + expect(blankCanvasState.has(gpuDisplayCanvas)).toBe(true) + expect(gpuDisplayCanvas.style.visibility).toBe('hidden') + + const oldSourceDrawCount = getCanvasDrawImageCallCountFrom(gpuDisplayCanvas, oldSplitCall[1]) + await act(async () => { + resolveOldRender?.() + await Promise.resolve() + await Promise.resolve() + }) + expect(getCanvasDrawImageCallCountFrom(gpuDisplayCanvas, oldSplitCall[1])).toBe( + oldSourceDrawCount, + ) + expect(gpuDisplayCanvas.style.visibility).toBe('hidden') + + act(() => { + setMockBlobUrl(mediaId, newUrl) + }) + const sourceBindingRendererCalls = createCompositionRendererMock.mock.calls as unknown as Array< + [ + inputProps: { + tracks: Array<{ items: Array<{ mediaId?: string; src?: string }> }> + }, + ] + > + const replacementCallIndex = await waitFor(() => { + const index = sourceBindingRendererCalls.findIndex(([inputProps]) => + inputProps.tracks.some((track) => + track.items.some((item) => item.mediaId === mediaId && item.src === newPresentationUrl), + ), + ) + expect(index).toBeGreaterThan(1) + return index + }) + const replacementRenderer = rendererMockState.instances[replacementCallIndex]! + await waitFor(() => expect(replacementRenderer.renderFrame).toHaveBeenCalledWith(24)) + await waitFor(() => { + expect(container.querySelector('[data-grade-comparison-after-layer="true"]')).not.toBeNull() + expect(gpuDisplayCanvas.style.visibility).toBe('visible') + }) + + for (const [inputProps] of sourceBindingRendererCalls.slice(2)) { + const sources = inputProps.tracks.flatMap((track) => + track.items.filter((item) => item.mediaId === mediaId).map((item) => item.src), + ) + expect(sources).not.toContain(oldPresentationUrl) + } + }) + + it('retires a reachable nested same-id source binding before the unchanged wrapper can repaint', async () => { + canvasPixelReadbackEnabled = true + const mediaId = 'nested-same-media-relink' + const gradeEffect = { + id: 'effect-grade', + enabled: true, + effect: { + type: 'gpu-effect' as const, + gpuEffectType: 'gpu-color-wheels' as const, + params: { exposure: 0.5 }, + }, + } + setMockBlobUrl(mediaId, 'blob:nested-old') + const nestedTrack = { + id: 'nested-track', + name: 'Nested', + height: 60, + locked: false, + visible: true, + muted: false, + solo: false, + order: 0, + items: [], + } + useCompositionsStore.getState().setCompositions([ + { + id: 'nested-composition', + name: 'Nested composition', + width: 1920, + height: 1080, + fps: 30, + durationInFrames: 120, + tracks: [nestedTrack], + transitions: [], + keyframes: [], + items: [ + { + id: 'nested-video', + label: 'Nested video', + type: 'video', + trackId: nestedTrack.id, + mediaId, + src: 'blob:nested-stale-item', + from: 0, + durationInFrames: 120, + } as unknown as TimelineItem, + ], + }, + ]) + setSingleVideoTrack() + useItemsStore.getState().setItems([ + { + id: 'compound-with-grade', + label: 'Compound', + type: 'composition', + trackId: 'track-video', + compositionId: 'nested-composition', + compositionWidth: 1920, + compositionHeight: 1080, + from: 0, + durationInFrames: 120, + effects: [gradeEffect], + } as unknown as TimelineItem, + ]) + act(() => { + usePlaybackStore.getState().setCurrentFrame(24) + }) + + let observeRelinkLayout = false + let wasBlankInLayout = false + let wasHiddenInLayout = false + let wasDisposedInLayout = false + let gpuDisplayCanvas: HTMLCanvasElement | null = null + let oldSplitRenderer: (typeof rendererMockState.instances)[number] | null = null + const onBindingLayout = () => { + if (!observeRelinkLayout || !gpuDisplayCanvas || !oldSplitRenderer) return + wasBlankInLayout = blankCanvasState.has(gpuDisplayCanvas) + wasHiddenInLayout = gpuDisplayCanvas.style.visibility === 'hidden' + wasDisposedInLayout = oldSplitRenderer.dispose.mock.calls.length === 1 + } + + const { container } = render( + <> + + + , + ) + await waitFor(() => expect(rendererMockState.instances).toHaveLength(1)) + act(() => { + useGizmoStore.getState().setColorGradeComparisonMode('split') + }) + + oldSplitRenderer = await waitFor(() => { + expect(rendererMockState.instances).toHaveLength(2) + return rendererMockState.instances[1]! + }) + const oldSplitCall = createCompositionRendererMock.mock.calls[1] as unknown as [ + unknown, + HTMLCanvasElement, + ] + gpuDisplayCanvas = container.querySelectorAll('canvas')[1] as HTMLCanvasElement + setMockCanvasBlank(oldSplitCall[1], false) + setMockCanvasBlank(gpuDisplayCanvas, false) + + let resolveOldRender: (() => void) | null = null + oldSplitRenderer.renderFrame.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveOldRender = resolve + }), + ) + act(() => { + useGizmoStore.getState().setEffectsPreviewNew({ + 'compound-with-grade': [ + { ...gradeEffect, effect: { ...gradeEffect.effect, params: { exposure: 0.8 } } }, + ], + }) + }) + await waitFor(() => expect(resolveOldRender).not.toBeNull()) + + observeRelinkLayout = true + act(() => { + setMockBlobUrl(mediaId, null) + }) + + expect(wasBlankInLayout).toBe(true) + expect(wasHiddenInLayout).toBe(true) + expect(wasDisposedInLayout).toBe(true) + const drawCountAfterRetirement = getCanvasDrawImageCallCountFrom( + gpuDisplayCanvas, + oldSplitCall[1], + ) + + await act(async () => { + resolveOldRender?.() + await Promise.resolve() + await Promise.resolve() + }) + expect(getCanvasDrawImageCallCountFrom(gpuDisplayCanvas, oldSplitCall[1])).toBe( + drawCountAfterRetirement, + ) + + act(() => { + setMockBlobUrl(mediaId, 'blob:nested-new') + }) + await waitFor(() => expect(rendererMockState.instances.length).toBeGreaterThan(2)) + await waitFor(() => { + expect(rendererMockState.instances.at(-1)?.renderFrame).toHaveBeenCalledWith(24) + }) + }) + it('keeps the split after renderer warm when toggling away from split and back', async () => { setSingleVideoItemAtFrame({ id: 'item-graded', @@ -4281,6 +4856,50 @@ describe('VideoPreview sync behavior', () => { }) }) + it('repaints a visible scrub canvas from the exact source after a paused cross-clip seek', async () => { + setMockBlobUrl('media-red', 'blob:red') + setMockBlobUrl('media-blue', 'blob:blue') + setSingleVideoTrack() + useItemsStore.getState().setItems([ + { + id: 'red', + label: 'Red', + type: 'video', + trackId: 'track-video', + mediaId: 'media-red', + src: 'blob:red', + from: 1, + durationInFrames: 90, + }, + { + id: 'blue', + label: 'Blue', + type: 'video', + trackId: 'track-video', + mediaId: 'media-blue', + src: 'blob:blue', + from: 91, + durationInFrames: 90, + }, + ] as TimelineItem[]) + + const { container } = renderDefaultPreview() + const scrubCanvas = getScrubCanvas(container) + await setScrubFrameAndWaitVisible(scrubCanvas, 45) + + act(() => { + usePlaybackStore.getState().setPreviewFrame(null) + usePlaybackStore.getState().setCurrentFrame(135) + }) + + await waitFor(() => { + expect(usePlaybackStore.getState().currentFrame).toBe(135) + expect(getDisplayedFrame()).toBe(135) + expect(scrubCanvas.style.visibility).toBe('visible') + expect(rendererMockState.renderedSources).toContainEqual({ frame: 135, src: 'blob:blue' }) + }) + }) + it('replays the latest scrub seek on play start when the warm seek has not landed yet', async () => { await renderAfterInitialSeek() diff --git a/src/features/preview/components/video-preview.tsx b/src/features/preview/components/video-preview.tsx index 0405cabb2..771fe19d7 100644 --- a/src/features/preview/components/video-preview.tsx +++ b/src/features/preview/components/video-preview.tsx @@ -72,6 +72,27 @@ interface PreviewItemsSnapshot { itemsByTrackId: Record } +function hasResolvedVisualSourceAtFrame( + tracks: Array<{ visible?: boolean; solo?: boolean; items: TimelineItem[] }>, + frame: number, +): boolean { + const hasSoloTrack = tracks.some((track) => track.solo) + for (const track of tracks) { + if (track.visible === false || (hasSoloTrack && !track.solo)) continue + for (const item of track.items) { + if (frame < item.from || frame >= item.from + item.durationInFrames) continue + if ( + item.mediaId && + (item.type === 'video' || item.type === 'image' || item.type === 'lottie') && + (!('src' in item) || !item.src) + ) { + return false + } + } + } + return true +} + /** * Video Preview Component * @@ -123,13 +144,18 @@ const VideoPreviewBase = memo(function VideoPreviewBase({ const livePreviewEdits = useGizmoStore((s) => s.preview) const [playerDisplayedFrame, setPlayerDisplayedFrame] = useState(null) const latestPlayerDisplayedFrameRef = useRef(null) - const [splitAfterRenderedFrame, setSplitAfterRenderedFrame] = useState(null) + const [splitAfterPresentation, setSplitAfterPresentation] = useState<{ + frame: number + structureKey: string + } | null>(null) const splitAfterRendererRef = useRef(null) const splitAfterInitPromiseRef = useRef | null>(null) const splitAfterInitGenerationRef = useRef(0) const splitAfterCanvasRef = useRef(null) const splitAfterRendererStructureKeyRef = useRef(null) - const splitAfterRenderInFlightRef = useRef(false) + const splitAfterRenderGenerationRef = useRef(0) + const splitAfterRenderOwnerRef = useRef(null) + const splitAfterRenderPumpRef = useRef<() => void>(() => {}) const splitAfterPendingFrameRef = useRef(null) const { playerRef, @@ -291,6 +317,7 @@ const VideoPreviewBase = memo(function VideoPreviewBase({ fastScrubInputProps, fastScrubPreviewItems, fastScrubTracksTopologyFingerprint, + sourceBindingIdentity, getPreviewTransformOverride, getPreviewEffectsOverride, getPreviewCornerPinOverride, @@ -377,6 +404,7 @@ const VideoPreviewBase = memo(function VideoPreviewBase({ renderSize.height, project.backgroundColor ?? '', useProxy ? 'proxy' : 'source', + sourceBindingIdentity, fastScrubTracksTopologyFingerprint, domTextScrubOverlayPlan.enabled ? 'dom-text-overlay' : 'composited-text', playbackTransitionFingerprint, @@ -391,6 +419,7 @@ const VideoPreviewBase = memo(function VideoPreviewBase({ project.width, renderSize.height, renderSize.width, + sourceBindingIdentity, useProxy, ], ) @@ -404,12 +433,12 @@ const VideoPreviewBase = memo(function VideoPreviewBase({ const disposeSplitAfterRenderer = useCallback(() => { splitAfterInitGenerationRef.current += 1 + splitAfterRenderGenerationRef.current += 1 splitAfterInitPromiseRef.current = null splitAfterRendererStructureKeyRef.current = null splitAfterCanvasRef.current = null splitAfterPendingFrameRef.current = null - splitAfterRenderInFlightRef.current = false - setSplitAfterRenderedFrame(null) + setSplitAfterPresentation(null) const renderer = splitAfterRendererRef.current splitAfterRendererRef.current = null @@ -429,6 +458,20 @@ const VideoPreviewBase = memo(function VideoPreviewBase({ if (canvas.height !== backingSize.height) canvas.height = backingSize.height }, [gpuEffectsCanvasRef, playerSize, renderSize]) + // A structure/source replacement can retain the same target frame, so frame + // readiness alone is insufficient. Retire the old split surface during the + // layout cleanup, before the replacement commit can paint, and use the same + // barrier on unmount. + useLayoutEffect(() => { + const canvas = gpuEffectsCanvasRef.current + return () => { + disposeSplitAfterRenderer() + if (!canvas) return + canvas.getContext('2d')?.clearRect(0, 0, canvas.width, canvas.height) + canvas.style.visibility = 'hidden' + } + }, [disposeSplitAfterRenderer, fastScrubRendererStructureKey, gpuEffectsCanvasRef]) + const ensureSplitAfterRenderer = useCallback(async (): Promise => { if (!FAST_SCRUB_RENDERER_ENABLED) return null @@ -452,6 +495,7 @@ const VideoPreviewBase = memo(function VideoPreviewBase({ if (!ctx) return null const { createCompositionRenderer } = await importCompositionRenderer() + if (splitAfterInitGenerationRef.current !== initGeneration) return null const renderer = await createCompositionRenderer(fastScrubInputProps, canvas, ctx, { mode: 'preview', useProxyMedia: useProxy, @@ -508,10 +552,6 @@ const VideoPreviewBase = memo(function VideoPreviewBase({ useProxy, ]) - useEffect(() => { - disposeSplitAfterRenderer() - }, [disposeSplitAfterRenderer, fastScrubRendererStructureKey]) - // Enter the composited path in the same render that activates the editor. // Waiting for the timeline-wide effect scan adds a reactive round trip that // makes the first neutral-EV drag look stuck until another parameter changes. @@ -703,6 +743,41 @@ const VideoPreviewBase = memo(function VideoPreviewBase({ setDisplayedFrame, ...previewRuntimeRefs.rendererControllerRefs, }) + + // The renderer replacement is asynchronous, so retire the old front buffer + // in the layout phase. This covers same-ID source swaps and topology changes + // before a stale scrub frame can remain visible for one paint. + useLayoutEffect(() => { + // Advance the render generation before replacement work can start. The + // controller's async pump checks this generation and cannot publish the + // disposed renderer's result afterward. + disposeFastScrubRenderer() + const canvas = scrubCanvasRef.current + if (canvas) { + const context = canvas.getContext('2d') + context?.clearRect(0, 0, canvas.width, canvas.height) + } + const hadFastScrubOverlay = showFastScrubOverlayRef.current + const hadTransitionOverlay = showPlaybackTransitionOverlayRef.current + hideFastScrubOverlay() + hidePlaybackTransitionOverlay() + // Keep the active routing owner alive so its replacement render can be + // scheduled in the same commit; the cleared canvas is the synchronous + // stale-pixel barrier. + if (hadFastScrubOverlay) showFastScrubOverlayForFrame() + else if (hadTransitionOverlay) showPlaybackTransitionOverlayForFrame() + }, [ + disposeFastScrubRenderer, + fastScrubRendererStructureKey, + hideFastScrubOverlay, + hidePlaybackTransitionOverlay, + scrubCanvasRef, + showFastScrubOverlayForFrame, + showFastScrubOverlayRef, + showPlaybackTransitionOverlayForFrame, + showPlaybackTransitionOverlayRef, + ]) + useEffect(() => { if (!shouldWarmGpuEffectsRenderer || isResolving) return @@ -863,6 +938,14 @@ const VideoPreviewBase = memo(function VideoPreviewBase({ stageColorGradeComparisonMode === 'split' && comparisonDisplayedFrame !== null ? comparisonDisplayedFrame : baseComparisonTargetFrame + const isComparisonSourceBindingReady = hasResolvedVisualSourceAtFrame( + fastScrubScaledTracks as Array<{ + visible?: boolean + solo?: boolean + items: TimelineItem[] + }>, + comparisonTargetFrame, + ) // Leaving split comparison clears the rendered after-frame. Kept as its own // effect keyed only on the mode so the per-frame `comparisonTargetFrame` @@ -870,34 +953,43 @@ const VideoPreviewBase = memo(function VideoPreviewBase({ useEffect(() => { if (stageColorGradeComparisonMode === 'split') return splitAfterPendingFrameRef.current = null - setSplitAfterRenderedFrame((frame) => (frame === null ? frame : null)) + setSplitAfterPresentation((presentation) => (presentation === null ? presentation : null)) }, [stageColorGradeComparisonMode]) useEffect(() => { if (stageColorGradeComparisonMode !== 'split') return + if (!isComparisonSourceBindingReady) { + splitAfterPendingFrameRef.current = null + setSplitAfterPresentation((presentation) => (presentation === null ? presentation : null)) + return + } let cancelled = false + const renderGeneration = ++splitAfterRenderGenerationRef.current splitAfterPendingFrameRef.current = comparisonTargetFrame - // Intentionally NOT resetting `splitAfterRenderedFrame` here: the readiness - // check (`splitAfterRenderedFrame === comparisonTargetFrame`) already gates - // the overlay, so a stale frame stays hidden until the async render catches - // up. The previous synchronous reset fed a render cascade - // (displayedFrame → comparisonTargetFrame → setState → displayedFrame …) - // that tripped React's "maximum update depth". + const isCurrent = () => !cancelled && splitAfterRenderGenerationRef.current === renderGeneration const renderPendingSplitAfter = async () => { - if (splitAfterRenderInFlightRef.current) return - splitAfterRenderInFlightRef.current = true + if (splitAfterRenderOwnerRef.current !== null) return + splitAfterRenderOwnerRef.current = renderGeneration try { - while (!cancelled && splitAfterPendingFrameRef.current !== null) { + while (isCurrent() && splitAfterPendingFrameRef.current !== null) { const targetFrame = splitAfterPendingFrameRef.current splitAfterPendingFrameRef.current = null const renderer = await ensureSplitAfterRenderer() + if (!isCurrent()) return const offscreen = splitAfterCanvasRef.current const displayCanvas = gpuEffectsCanvasRef.current - if (cancelled || !renderer || !offscreen || !displayCanvas) return + if ( + !renderer || + !offscreen || + !displayCanvas || + splitAfterRendererRef.current !== renderer || + splitAfterCanvasRef.current !== offscreen + ) + return try { renderer.invalidateFrameCache({ frames: [targetFrame] }) @@ -905,36 +997,56 @@ const VideoPreviewBase = memo(function VideoPreviewBase({ // Some renderer doubles do not support selective invalidation. } await renderer.renderFrame(targetFrame) - if (cancelled || splitAfterPendingFrameRef.current !== null) continue + if ( + !isCurrent() || + splitAfterRendererRef.current !== renderer || + splitAfterCanvasRef.current !== offscreen || + gpuEffectsCanvasRef.current !== displayCanvas + ) + return + if (splitAfterPendingFrameRef.current !== null) continue const displayCtx = displayCanvas.getContext('2d') if (!displayCtx) return + if (!isCurrent()) return drawSourceToPreviewDisplayCanvas(displayCtx, displayCanvas, offscreen) - setSplitAfterRenderedFrame(targetFrame) + if (!isCurrent()) return + setSplitAfterPresentation({ + frame: targetFrame, + structureKey: fastScrubRendererStructureKey, + }) } } finally { - splitAfterRenderInFlightRef.current = false - if (!cancelled && splitAfterPendingFrameRef.current !== null) { - void renderPendingSplitAfter() + if (splitAfterRenderOwnerRef.current === renderGeneration) { + splitAfterRenderOwnerRef.current = null + } + if (splitAfterPendingFrameRef.current !== null) { + queueMicrotask(() => splitAfterRenderPumpRef.current()) } } } - void renderPendingSplitAfter() + splitAfterRenderPumpRef.current = () => { + void renderPendingSplitAfter() + } + splitAfterRenderPumpRef.current() return () => { cancelled = true + if (splitAfterRenderGenerationRef.current === renderGeneration) { + splitAfterRenderGenerationRef.current += 1 + } } }, [ comparisonTargetFrame, ensureSplitAfterRenderer, + fastScrubRendererStructureKey, gpuEffectsCanvasRef, + isComparisonSourceBindingReady, livePreviewEdits, stageColorGradeComparisonMode, ]) - useEffect(() => () => disposeSplitAfterRenderer(), [disposeSplitAfterRenderer]) - const livePlayerFrame = playerRef.current?.getCurrentFrame() const normalizedLivePlayerFrame = livePlayerFrame === undefined || !Number.isFinite(livePlayerFrame) @@ -944,9 +1056,11 @@ const VideoPreviewBase = memo(function VideoPreviewBase({ const isColorGradeComparisonActive = stageColorGradeComparisonMode !== 'off' const isSplitGradeComparison = stageColorGradeComparisonMode === 'split' const isColorGradeComparisonFrameReady = + isComparisonSourceBindingReady && comparisonDisplayedFrame === comparisonTargetFrame && (isSplitGradeComparison - ? splitAfterRenderedFrame === comparisonTargetFrame + ? splitAfterPresentation?.frame === comparisonTargetFrame && + splitAfterPresentation.structureKey === fastScrubRendererStructureKey : stageColorGradeComparisonMode === 'before' || effectivePlayerDisplayedFrame === comparisonTargetFrame) const stageRenderedOverlayVisible = isColorGradeComparisonActive diff --git a/src/features/preview/hooks/use-preview-composition-model.test.ts b/src/features/preview/hooks/use-preview-composition-model.test.ts index a1edca27c..b296527ac 100644 --- a/src/features/preview/hooks/use-preview-composition-model.test.ts +++ b/src/features/preview/hooks/use-preview-composition-model.test.ts @@ -3,11 +3,122 @@ import { describe, expect, it } from 'vite-plus/test' import type { TimelineTrack } from '@/types/timeline' import { + buildPreviewSourceBindings, buildPreviewCompositionData, mergeLiveItemPresentation, mergeLiveItemPreview, } from './use-preview-composition-model' +describe('buildPreviewSourceBindings', () => { + const directTrack: TimelineTrack = { + id: 'root-track', + name: 'Root', + height: 80, + locked: false, + visible: true, + muted: false, + solo: false, + order: 0, + items: [ + { + id: 'direct-video', + trackId: 'root-track', + type: 'video', + mediaId: 'media-direct', + src: 'blob:stale-direct', + label: 'Direct', + from: 0, + durationInFrames: 30, + }, + { + id: 'compound', + trackId: 'root-track', + type: 'composition', + compositionId: 'composition-a', + compositionWidth: 1920, + compositionHeight: 1080, + label: 'Compound', + from: 0, + durationInFrames: 30, + }, + { + id: 'missing-compound', + trackId: 'root-track', + type: 'composition', + compositionId: 'missing-composition', + compositionWidth: 1920, + compositionHeight: 1080, + label: 'Missing', + from: 0, + durationInFrames: 30, + }, + ], + } + + it('walks reachable nested media deterministically and survives cycles and missing references', () => { + const compositionById = { + 'composition-a': { + id: 'composition-a', + items: [ + { + id: 'nested-video', + trackId: 'nested-track', + type: 'video' as const, + mediaId: 'media-nested', + src: 'blob:stale-nested', + label: 'Nested', + from: 0, + durationInFrames: 30, + }, + { + id: 'cycle-to-b', + trackId: 'nested-track', + type: 'composition' as const, + compositionId: 'composition-b', + compositionWidth: 1920, + compositionHeight: 1080, + label: 'B', + from: 0, + durationInFrames: 30, + }, + ], + }, + 'composition-b': { + id: 'composition-b', + items: [ + { + id: 'cycle-to-a', + trackId: 'nested-track', + type: 'composition' as const, + compositionId: 'composition-a', + compositionWidth: 1920, + compositionHeight: 1080, + label: 'A', + from: 0, + durationInFrames: 30, + }, + ], + }, + } + + const result = buildPreviewSourceBindings({ + tracks: [directTrack], + compositionById, + getEpoch: (mediaId) => `epoch:${mediaId}`, + getUrl: (mediaId) => `blob:current:${mediaId}`, + }) + + expect([...result.urls]).toEqual([ + ['media-direct', 'blob:current:media-direct'], + ['media-nested', 'blob:current:media-nested'], + ]) + expect(JSON.parse(result.identity)).toEqual([ + ['media-direct', 'epoch:media-direct', 'blob:current:media-direct'], + ['media-nested', 'epoch:media-nested', 'blob:current:media-nested'], + ]) + }) +}) + describe('mergeLiveItemPreview', () => { it('merges live shape properties into the canvas renderer snapshot', () => { const shape = { @@ -114,7 +225,7 @@ describe('buildPreviewCompositionData', () => { { frame: 10, srcs: ['blob://video'] }, { frame: 70, srcs: ['blob://video'] }, ]) - expect(result.totalFrames).toBe(220) + expect(result.totalFrames).toBe(70) const playbackVideoItem = result.inputProps.tracks[0]?.items[0] const scrubVideoItem = result.fastScrubInputProps.tracks[0]?.items[0] expect(playbackVideoItem?.type).toBe('video') @@ -127,6 +238,62 @@ describe('buildPreviewCompositionData', () => { } }) + it('uses the exclusive end of adjacent clips as the canonical player duration', () => { + const track: TimelineTrack = { + id: 'track-1', + name: 'Video', + height: 80, + locked: false, + visible: true, + muted: false, + solo: false, + order: 1, + items: [ + { + id: 'red', + trackId: 'track-1', + type: 'video', + mediaId: 'media-red', + src: 'blob:red', + label: 'Red', + from: 1, + durationInFrames: 90, + }, + { + id: 'blue', + trackId: 'track-1', + type: 'video', + mediaId: 'media-blue', + src: 'blob:blue', + label: 'Blue', + from: 91, + durationInFrames: 90, + }, + ], + } + + const result = buildPreviewCompositionData({ + combinedTracks: [track], + fps: 30, + items: track.items, + keyframes: [], + transitions: [], + resolvedUrls: new Map([ + ['media-red', 'blob:red'], + ['media-blue', 'blob:blue'], + ]), + useProxy: false, + blobUrlVersion: 0, + project: { width: 1920, height: 1080, backgroundColor: '#000000' }, + }) + + expect(result.totalFrames).toBe(181) + expect(result.playbackVideoSourceSpans).toEqual([ + { src: 'blob:red', startFrame: 1, endFrame: 91 }, + { src: 'blob:blue', startFrame: 91, endFrame: 181 }, + ]) + }) + it('uses proxy media for playback and fast scrubbing when proxies are enabled', () => { const track: TimelineTrack = { id: 'track-1', @@ -268,7 +435,7 @@ describe('buildPreviewCompositionData', () => { expect(result.renderSize).toEqual({ width: 1504, height: 846 }) }) - it('uses an already-acquired blob URL before resolvedUrls catches up', () => { + it('uses an already-acquired blob URL before a stale resolvedUrls entry', () => { const track: TimelineTrack = { id: 'track-1', name: 'Video', @@ -298,7 +465,7 @@ describe('buildPreviewCompositionData', () => { items: track.items, keyframes: [], transitions: [], - resolvedUrls: new Map(), + resolvedUrls: new Map([['media-1', 'blob://stale-resolved']]), useProxy: false, blobUrlVersion: 1, project: { width: 1920, height: 1080 }, diff --git a/src/features/preview/hooks/use-preview-composition-model.ts b/src/features/preview/hooks/use-preview-composition-model.ts index e99bb8cf2..53e7a03f9 100644 --- a/src/features/preview/hooks/use-preview-composition-model.ts +++ b/src/features/preview/hooks/use-preview-composition-model.ts @@ -9,7 +9,11 @@ import { blobUrlManager } from '@/infrastructure/browser/blob-url-manager' import { isColorGradeEffectType } from '@/infrastructure/gpu-effects' import { usePlaybackStore } from '@/shared/state/playback' import { resolveEffectiveTrackStates } from '@/features/preview/deps/timeline-utils' -import { useCompositionsStore, useItemsStore } from '@/features/preview/deps/timeline-store' +import { + useCompositionsStore, + useItemsStore, + type SubComposition, +} from '@/features/preview/deps/timeline-store' import { appendVirtualTranscriptCaptionTrack } from '@/features/preview/deps/caption-items' import { useCornerPinStore } from '../stores/corner-pin-store' import { useGizmoStore, type ItemPreview } from '../stores/gizmo-store' @@ -78,6 +82,7 @@ interface BuildPreviewCompositionDataParams { previewRenderSize?: PreviewPlayerSize resolveProxyUrlFn?: (mediaId: string) => string | null getBlobUrlFn?: (mediaId: string) => string | null + authoritativeSourceUrls?: ReadonlyMap } interface UsePreviewCompositionModelParams { @@ -101,6 +106,63 @@ interface UsePreviewCompositionBaseModelParams { mediaById: Record[0]> } +interface PreviewSourceBindings { + identity: string + urls: ReadonlyMap +} + +function getRenderableMediaId(item: TimelineItem): string | null { + if (!item.mediaId) return null + switch (item.type) { + case 'video': + case 'audio': + case 'image': + case 'lottie': + return item.mediaId + default: + return null + } +} + +export function buildPreviewSourceBindings({ + tracks, + compositionById, + getEpoch, + getUrl, +}: { + tracks: TimelineTrack[] + compositionById: Readonly | undefined>> + getEpoch: (mediaId: string) => string + getUrl: (mediaId: string) => string | null +}): PreviewSourceBindings { + const urls = new Map() + const identities: Array<[mediaId: string, epoch: string, url: string]> = [] + const reachableMediaIds = new Set() + const visitedCompositionIds = new Set() + + const visitItems = (items: readonly TimelineItem[]) => { + for (const item of items) { + const mediaId = getRenderableMediaId(item) + if (mediaId) reachableMediaIds.add(mediaId) + + if (!item.compositionId || visitedCompositionIds.has(item.compositionId)) continue + visitedCompositionIds.add(item.compositionId) + const composition = compositionById[item.compositionId] + if (composition) visitItems(composition.items) + } + } + + for (const track of tracks) visitItems(track.items) + + for (const mediaId of [...reachableMediaIds].sort()) { + const url = getUrl(mediaId) ?? '' + if (url) urls.set(mediaId, url) + identities.push([mediaId, getEpoch(mediaId), url]) + } + + return { identity: JSON.stringify(identities), urls } +} + /** * Apply transient panel edits to the item snapshot consumed by the canvas * renderer. The DOM player subscribes to the same preview store directly, but @@ -224,6 +286,20 @@ export function usePreviewCompositionModel({ () => ({ width: previewRenderWidth, height: previewRenderHeight }), [previewRenderHeight, previewRenderWidth], ) + const compositionById = useCompositionsStore((state) => state.compositionById) + const sourceBindings = useMemo(() => { + // Blob URL notifications are synchronous external-store updates. Snapshot + // the active media epochs and URLs during render so a relink/invalidation + // cannot leave the passive-effect-backed resolvedUrls map owning a retired + // source for the next layout/presentation phase. + void blobUrlVersion + return buildPreviewSourceBindings({ + tracks: combinedTracks, + compositionById, + getEpoch: (mediaId) => blobUrlManager.getEpoch(mediaId), + getUrl: (mediaId) => blobUrlManager.get(mediaId), + }) + }, [blobUrlVersion, combinedTracks, compositionById]) const { playbackVideoSourceSpans, scrubVideoSourceSpans, @@ -252,6 +328,7 @@ export function usePreviewCompositionModel({ blobUrlVersion, project, previewRenderSize, + authoritativeSourceUrls: sourceBindings.urls, }) }, [ blobUrlVersion, @@ -264,6 +341,7 @@ export function usePreviewCompositionModel({ previewRenderSize, proxyReadyCount, resolvedUrls, + sourceBindings.urls, transitions, useProxy, ]) @@ -374,6 +452,7 @@ export function usePreviewCompositionModel({ fastScrubInputProps, fastScrubPreviewItems, fastScrubTracksTopologyFingerprint, + sourceBindingIdentity: sourceBindings.identity, getPreviewTransformOverride, getPreviewEffectsOverride, getPreviewCornerPinOverride, @@ -397,6 +476,7 @@ export function buildPreviewCompositionData({ previewRenderSize, resolveProxyUrlFn = resolveProxyUrl, getBlobUrlFn = (mediaId: string) => blobUrlManager.get(mediaId), + authoritativeSourceUrls, }: BuildPreviewCompositionDataParams) { void blobUrlVersion const resolvedTrackList: CompositionInputProps['tracks'] = [] @@ -423,9 +503,13 @@ export function buildPreviewCompositionData({ continue } - const sourceUrl = resolvedUrls.get(item.mediaId) ?? getBlobUrlFn(item.mediaId) ?? '' + const sourceUrl = authoritativeSourceUrls + ? (authoritativeSourceUrls.get(item.mediaId) ?? '') + : (getBlobUrlFn(item.mediaId) ?? resolvedUrls.get(item.mediaId) ?? '') const proxyUrl = - item.type === 'video' ? resolveProxyUrlFn(item.mediaId) || sourceUrl : sourceUrl + item.type === 'video' && (!authoritativeSourceUrls || sourceUrl) + ? resolveProxyUrlFn(item.mediaId) || sourceUrl + : sourceUrl const resolvedSrc = useProxy && item.type === 'video' ? proxyUrl : sourceUrl const fastScrubSrc = resolvedSrc const hasMatchingAudioSrc = item.type !== 'video' || item.audioSrc === sourceUrl @@ -509,7 +593,7 @@ export function buildPreviewCompositionData({ (max, item) => Math.max(max, item.from + item.durationInFrames), 0, ) - const totalFrames = furthestItemEndFrame === 0 ? 900 : furthestItemEndFrame + fps * 5 + const totalFrames = furthestItemEndFrame === 0 ? 900 : furthestItemEndFrame const inputProps: CompositionInputProps = { fps, width: project.width, diff --git a/src/features/preview/hooks/use-preview-media-resolution.test.tsx b/src/features/preview/hooks/use-preview-media-resolution.test.tsx index ac9c58cce..e2b1005a7 100644 --- a/src/features/preview/hooks/use-preview-media-resolution.test.tsx +++ b/src/features/preview/hooks/use-preview-media-resolution.test.tsx @@ -4,17 +4,52 @@ import { useMediaDependencyStore } from '@/features/preview/deps/timeline-store' import type { TimelineTrack } from '@/types/timeline' import { usePreviewMediaResolution } from './use-preview-media-resolution' -vi.mock('../utils/media-resolver', () => ({ +const resolverHarness = vi.hoisted(() => ({ + epoch: 0, resolveMediaUrl: vi.fn(() => new Promise(() => {})), })) +vi.mock('../utils/media-resolver', () => ({ + resolveMediaUrl: resolverHarness.resolveMediaUrl, +})) + vi.mock('@/infrastructure/browser/blob-url-manager', () => ({ blobUrlManager: { get: (mediaId: string) => (mediaId === 'media-priority' ? 'blob:priority' : null), + getEpoch: () => String(resolverHarness.epoch), invalidateAll: vi.fn(), }, })) +function deferred() { + let resolve!: (value: T) => void + const promise = new Promise((res) => { + resolve = res + }) + return { promise, resolve } +} + +function makeHookParams() { + return { + fps: 30, + combinedTracks: [] as TimelineTrack[], + mediaResolveCostById: new Map(), + mediaDependencyVersion: 0, + blobUrlVersion: 0, + brokenMediaCount: 0, + previewPerfRef: { + current: { + resolveSamples: 0, + resolveTotalMs: 0, + resolveTotalIds: 0, + resolveLastMs: 0, + resolveLastIds: 0, + }, + }, + isGizmoInteractingRef: { current: false }, + } +} + const combinedTracks = [ { id: 'track-video', @@ -41,6 +76,9 @@ const combinedTracks = [ describe('usePreviewMediaResolution', () => { afterEach(() => { + resolverHarness.epoch = 0 + resolverHarness.resolveMediaUrl.mockReset() + resolverHarness.resolveMediaUrl.mockImplementation(() => new Promise(() => {})) useMediaDependencyStore.setState({ mediaIds: [], mediaDependencyVersion: 0 }) }) @@ -88,4 +126,38 @@ describe('usePreviewMediaResolution', () => { unmount() }) + + it('deduplicates only within the current media epoch while an old request drains', async () => { + const oldResolution = deferred() + const newResolution = deferred() + resolverHarness.resolveMediaUrl + .mockReturnValueOnce(oldResolution.promise) + .mockReturnValueOnce(newResolution.promise) + const { result } = renderHook(() => usePreviewMediaResolution(makeHookParams())) + + const firstBatch = result.current.resolveMediaBatch(['media-race']) + await waitFor(() => expect(resolverHarness.resolveMediaUrl).toHaveBeenCalledTimes(1)) + + resolverHarness.epoch += 1 + const secondBatch = result.current.resolveMediaBatch(['media-race']) + await waitFor(() => expect(resolverHarness.resolveMediaUrl).toHaveBeenCalledTimes(2)) + + const thirdBatch = result.current.resolveMediaBatch(['media-race']) + expect(resolverHarness.resolveMediaUrl).toHaveBeenCalledTimes(2) + + oldResolution.resolve(null) + await expect(firstBatch).resolves.toEqual({ + resolvedEntries: [], + failedIds: ['media-race'], + }) + expect(resolverHarness.resolveMediaUrl).toHaveBeenCalledTimes(2) + + newResolution.resolve('blob:new-source') + const expected = { + resolvedEntries: [{ mediaId: 'media-race', url: 'blob:new-source' }], + failedIds: [], + } + await expect(secondBatch).resolves.toEqual(expected) + await expect(thirdBatch).resolves.toEqual(expected) + }) }) diff --git a/src/features/preview/hooks/use-preview-media-resolution.ts b/src/features/preview/hooks/use-preview-media-resolution.ts index 1b52ee656..5700e38f5 100644 --- a/src/features/preview/hooks/use-preview-media-resolution.ts +++ b/src/features/preview/hooks/use-preview-media-resolution.ts @@ -24,6 +24,11 @@ type ResolveMediaBatchResult = { failedIds: string[] } +interface PendingPreviewResolve { + epoch: string + promise: Promise +} + interface UsePreviewMediaResolutionParams { fps: number combinedTracks: TimelineTrack[] @@ -58,7 +63,7 @@ export function usePreviewMediaResolution({ const unresolvedMediaIdsRef = useRef([]) const unresolvedMediaIdSetRef = useRef>(new Set()) - const pendingResolvePromisesRef = useRef>>(new Map()) + const pendingResolvePromisesRef = useRef>(new Map()) const preloadResolveInFlightRef = useRef(false) const preloadBurstRemainingRef = useRef(0) const preloadScanTrackCursorRef = useRef(0) @@ -234,19 +239,23 @@ export function usePreviewMediaResolution({ const resolveMediaUrlDeduped = useCallback((mediaId: string): Promise => { const pendingMap = pendingResolvePromisesRef.current - const existingPromise = pendingMap.get(mediaId) - if (existingPromise) { - return existingPromise + const epoch = blobUrlManager.getEpoch(mediaId) + const existingRequest = pendingMap.get(mediaId) + if (existingRequest?.epoch === epoch) { + return existingRequest.promise } - const promise = resolveMediaUrl(mediaId) + let promise!: Promise + promise = resolveMediaUrl(mediaId) .then((url) => url ?? null) .catch(() => null) .finally(() => { - pendingMap.delete(mediaId) + if (pendingMap.get(mediaId)?.promise === promise) { + pendingMap.delete(mediaId) + } }) - pendingMap.set(mediaId, promise) + pendingMap.set(mediaId, { epoch, promise }) return promise }, []) diff --git a/src/features/preview/hooks/use-preview-render-pump-controller.ts b/src/features/preview/hooks/use-preview-render-pump-controller.ts index 6a9f2cd79..ca560ba46 100644 --- a/src/features/preview/hooks/use-preview-render-pump-controller.ts +++ b/src/features/preview/hooks/use-preview-render-pump-controller.ts @@ -2490,6 +2490,9 @@ export function usePreviewRenderPump({ const playStateChanged = state.isPlaying !== prev.isPlaying || renderedPlaybackActive !== renderedPlaybackWasActive const isAtomicScrubTarget = isAtomicPreviewTarget(state) + const visibleOverlayNeedsCurrentFrame = + showFastScrubOverlayRef.current && + usePreviewBridgeStore.getState().displayedFrame !== state.currentFrame // Pointer release keeps the same numerical target, but it is still a // first-class committed request. Let it refresh the latest-target @@ -2498,7 +2501,8 @@ export function usePreviewRenderPump({ if ( targetFrame === prevTargetFrame && !playStateChanged && - settlingReleasedScrubFrame === null + settlingReleasedScrubFrame === null && + !visibleOverlayNeedsCurrentFrame ) { return } @@ -2633,7 +2637,14 @@ export function usePreviewRenderPump({ const requiresRenderedPath = forceFastScrubOverlay || shouldPreserveHighFidelityBackwardPreview(state.currentFrame) if (showFastScrubOverlayRef.current) { - if (settlingReleasedScrubFrame !== null && requiresRenderedPath) { + if ( + (settlingReleasedScrubFrame !== null && requiresRenderedPath) || + visibleOverlayNeedsCurrentFrame + ) { + // A prior skim canvas still covers the Player. A direct paused + // seek can move the Player to the exact frame without changing + // overlay ownership, so repaint that visible canvas as well; + // otherwise its previous clip remains on top indefinitely. scrubRequestedFrameRef.current = state.currentFrame void pumpRenderLoop() } diff --git a/src/features/preview/utils/media-resolver.test.ts b/src/features/preview/utils/media-resolver.test.ts index 1f5b70755..4d40b0bd0 100644 --- a/src/features/preview/utils/media-resolver.test.ts +++ b/src/features/preview/utils/media-resolver.test.ts @@ -47,6 +47,16 @@ vi.mock('@/features/media-library/stores/media-library-store', () => ({ let blobUrlCounter = 0 +function deferred() { + let resolve!: (value: T) => void + let reject!: (error: unknown) => void + const promise = new Promise((res, rej) => { + resolve = res + reject = rej + }) + return { promise, resolve, reject } +} + beforeEach(() => { vi.clearAllMocks() blobUrlManager.releaseAll() @@ -257,6 +267,34 @@ describe('resolveMediaUrl', () => { // Service should only be called once (second call uses pending promise) expect(mediaLibraryService.getMedia).toHaveBeenCalledTimes(1) }) + + it('keeps a late pre-invalidation request from replacing the new source', async () => { + const oldBlob = deferred() + const newBlob = deferred() + ;(mediaLibraryService.getMedia as Mock) + .mockResolvedValueOnce({ id: 'media-1', fileName: 'old.mp4' }) + .mockResolvedValueOnce({ id: 'media-1', fileName: 'new.mp4' }) + ;(mediaLibraryService.getMediaFile as Mock) + .mockReturnValueOnce(oldBlob.promise) + .mockReturnValueOnce(newBlob.promise) + + const oldResolution = resolveMediaUrl('media-1') + await vi.waitFor(() => expect(mediaLibraryService.getMediaFile).toHaveBeenCalledTimes(1)) + + blobUrlManager.invalidate('media-1') + const newResolution = resolveMediaUrl('media-1') + await vi.waitFor(() => expect(mediaLibraryService.getMediaFile).toHaveBeenCalledTimes(2)) + + newBlob.resolve(new Blob(['new-source'])) + await expect(newResolution).resolves.toBe('blob:test-1') + expect(blobUrlManager.get('media-1')).toBe('blob:test-1') + + oldBlob.resolve(new Blob(['old-source'])) + await expect(oldResolution).resolves.toBe('') + expect(blobUrlManager.get('media-1')).toBe('blob:test-1') + expect(blobUrlCounter).toBe(1) + expect(mockMarkMediaHealthy).toHaveBeenCalledTimes(1) + }) }) describe('resolveMediaUrls', () => { @@ -479,3 +517,49 @@ describe('relinking regression', () => { expect(relinkedUrl).not.toBe(originalUrl) }) }) + +describe('abortable bulk resolution', () => { + it('rejects promptly when its signal aborts while media is pending', async () => { + let resolveFile!: (file: Blob) => void + ;(mediaLibraryService.getMedia as Mock).mockResolvedValue({ + id: 'media-1', + fileName: 'video.mp4', + }) + ;(mediaLibraryService.getMediaFile as Mock).mockReturnValue( + new Promise((resolve) => { + resolveFile = resolve + }), + ) + + const controller = new AbortController() + const tracks = [ + { + id: 'track-1', + name: 'Track 1', + height: 40, + locked: false, + visible: true, + muted: false, + solo: false, + order: 0, + items: [ + { + id: 'item-1', + type: 'video' as const, + trackId: 'track-1', + from: 0, + durationInFrames: 30, + mediaId: 'media-1', + src: '', + label: 'clip', + }, + ], + }, + ] + + const pending = resolveMediaUrls(tracks, { useProxy: false, signal: controller.signal }) + controller.abort() + await expect(pending).rejects.toMatchObject({ name: 'AbortError' }) + resolveFile(new Blob(['late'])) + }) +}) diff --git a/src/features/preview/workers/consume-video-samples.test.ts b/src/features/preview/workers/consume-video-samples.test.ts new file mode 100644 index 000000000..39ff34613 --- /dev/null +++ b/src/features/preview/workers/consume-video-samples.test.ts @@ -0,0 +1,40 @@ +// @vitest-environment node + +import { describe, expect, it, vi } from 'vite-plus/test' +import { consumeVideoSamples } from './consume-video-samples' + +describe('consumeVideoSamples', () => { + it('closes a yielded sample when cancellation wins before consumption', async () => { + const sample = { close: vi.fn() } + let iteratorFinalized = false + async function* samples() { + try { + yield sample + } finally { + iteratorFinalized = true + } + } + const consume = vi.fn() + + await consumeVideoSamples(samples(), [1], () => false, consume) + + expect(consume).not.toHaveBeenCalled() + expect(sample.close).toHaveBeenCalledOnce() + expect(iteratorFinalized).toBe(true) + }) + + it('skips a null sample and keeps consuming later timestamps', async () => { + const sample = { close: vi.fn() } + async function* samples() { + yield null + yield sample + } + const consume = vi.fn() + + await consumeVideoSamples(samples(), [1, 2], () => true, consume) + + expect(consume).toHaveBeenCalledOnce() + expect(consume).toHaveBeenCalledWith(sample, 2) + expect(sample.close).toHaveBeenCalledOnce() + }) +}) diff --git a/src/features/preview/workers/consume-video-samples.ts b/src/features/preview/workers/consume-video-samples.ts new file mode 100644 index 000000000..27b6dcc0a --- /dev/null +++ b/src/features/preview/workers/consume-video-samples.ts @@ -0,0 +1,32 @@ +interface ClosableSample { + close?: () => void +} + +/** + * Consume a timestamp-aligned VideoSample iterator with one explicit ownership + * boundary. A sample that arrives just as cancellation wins is still closed + * before the iterator is torn down. + */ +export async function consumeVideoSamples( + iterator: AsyncGenerator, + timestamps: readonly number[], + shouldContinue: () => boolean, + consume: (sample: T, timestamp: number) => void, +): Promise { + let index = 0 + try { + for await (const sample of iterator) { + try { + if (!shouldContinue()) break + const timestamp = timestamps[index] + index += 1 + if (!sample || timestamp === undefined) continue + consume(sample, timestamp) + } finally { + sample?.close?.() + } + } + } finally { + await iterator.return?.() + } +} diff --git a/src/features/preview/workers/decoder-prewarm-worker.ts b/src/features/preview/workers/decoder-prewarm-worker.ts index 053ade595..a569ba010 100644 --- a/src/features/preview/workers/decoder-prewarm-worker.ts +++ b/src/features/preview/workers/decoder-prewarm-worker.ts @@ -7,6 +7,7 @@ */ import { createMediabunnyInputSource } from '@/infrastructure/browser/mediabunny-input-source' +import { consumeVideoSamples } from './consume-video-samples' import type { ObjectUrlSourceMetadata } from '@/infrastructure/browser/object-url-registry' const TIMESTAMP_EPSILON = 1e-4 @@ -266,11 +267,10 @@ function resetSampleIterator(state: ExtractorState, startTimestamp: number): voi // decoding a range. Starting at the requested presentation timestamp keeps // that necessary GOP decode inside the sink without yielding a keyframe-to- // target runway that this worker would only close and discard. - state.sampleIterator = state.sink.samples(Math.max(0, startTimestamp), Infinity) as AsyncGenerator< - WorkerSample, - void, - unknown - > + state.sampleIterator = state.sink.samples( + Math.max(0, startTimestamp), + Infinity, + ) as AsyncGenerator state.iteratorDone = false state.lastRequestedTimestamp = null } @@ -560,31 +560,12 @@ async function batchPreseek( // samplesAtTimestamps uses an optimized pipeline that shares decoder // state across the batch — each packet decoded at most once. const iterator = state.sink.samplesAtTimestamps(timestamps) - let i = 0 - try { - for await (const sample of iterator) { - if (!shouldContinue()) break - const timestamp = timestamps[i] - i++ - - if (!sample) { - continue - } - - try { - // Defensive: mediabunny should yield at most one sample per requested - // timestamp, but an over-producing iterator must not leak the extra - // VideoSample while the stream is being torn down. - if (timestamp === undefined) continue - const bitmap = renderSampleToBitmap(state, sample, maxDimension) - if (bitmap) results.set(timestamp, bitmap) - } finally { - sample.close?.() - } + await consumeVideoSamples(iterator, timestamps, shouldContinue, (sample, timestamp) => { + if (sample) { + const bitmap = renderSampleToBitmap(state, sample, maxDimension) + if (bitmap) results.set(timestamp, bitmap) } - } finally { - await iterator.return?.() - } + }) } catch { // Batch decode failed — return whatever we got } @@ -624,10 +605,7 @@ self.onmessage = async (event: MessageEvent) => { if (src) { activePreviewGenerationBySrc.set( src, - Math.max( - activePreviewGenerationBySrc.get(src) ?? 0, - Number(msg.generation) || 0, - ), + Math.max(activePreviewGenerationBySrc.get(src) ?? 0, Number(msg.generation) || 0), ) } return @@ -687,10 +665,7 @@ self.onmessage = async (event: MessageEvent) => { if (isActivePreviewRequest) { activePreviewGenerationBySrc.set( msg.src, - Math.max( - activePreviewGenerationBySrc.get(msg.src) ?? 0, - Number(msg.generation) || 0, - ), + Math.max(activePreviewGenerationBySrc.get(msg.src) ?? 0, Number(msg.generation) || 0), ) } diff --git a/src/features/timeline/components/timeline-content.test.tsx b/src/features/timeline/components/timeline-content.test.tsx index 280013279..75ad99df0 100644 --- a/src/features/timeline/components/timeline-content.test.tsx +++ b/src/features/timeline/components/timeline-content.test.tsx @@ -3,6 +3,7 @@ import { act, fireEvent, render, waitFor } from '@testing-library/react' import { beforeAll, beforeEach, describe, expect, it, vi } from 'vite-plus/test' import { useEditorStore } from '@/shared/state/editor' +import { useMicRecordingStore } from '@/shared/state/mic-recording-store' import { usePlaybackStore } from '@/shared/state/playback' import { resetPlaybackPreviewState } from '@/shared/state/playback-preview-test-helpers' import { useSelectionStore } from '@/shared/state/selection' @@ -58,7 +59,7 @@ vi.mock('./timeline-markers', () => ({ })) vi.mock('./timeline-playhead', () => ({ - TimelinePlayhead: () =>
, + TimelinePlayhead: () =>
, })) vi.mock('./timeline-preview-scrubber', () => ({ @@ -67,6 +68,7 @@ vi.mock('./timeline-preview-scrubber', () => ({ vi.mock('./timeline-track', async () => { const { useState } = await import('react') + const { usePlaybackStore } = await import('@/shared/state/playback') const { createTimelineTrackContentLayerRef } = await import('../utils/timeline-live-geometry') return { @@ -76,6 +78,14 @@ vi.mock('./timeline-track', async () => { return (
+
{ + event.stopPropagation() + usePlaybackStore.getState().finishScrub(1) + }} + />
) }, @@ -173,6 +183,7 @@ function resetStores() { }) resetPlaybackPreviewState() + useMicRecordingStore.getState().reset() useTimelineStore.setState({ fps: 30, @@ -651,6 +662,66 @@ describe('TimelineContent playback selection behavior', () => { cancelAnimationFrameSpy.mockRestore() }) + it('cancels a queued hover preview when playhead scrubbing starts', () => { + let nextFrameId = 1 + const scheduledFrames = new Map() + const animationFrameSpy = vi + .spyOn(window, 'requestAnimationFrame') + .mockImplementation((callback) => { + const id = nextFrameId++ + scheduledFrames.set(id, callback) + return id + }) + const cancelAnimationFrameSpy = vi + .spyOn(window, 'cancelAnimationFrame') + .mockImplementation((id) => { + scheduledFrames.delete(id) + }) + + const { container, getByTestId, unmount } = render( + , + ) + const scrollContainer = container.querySelector('[data-timeline-scroll-container]') + if (!(scrollContainer instanceof HTMLDivElement)) { + throw new Error('Expected timeline scroll container') + } + + Object.defineProperty(scrollContainer, 'getBoundingClientRect', { + configurable: true, + value: () => ({ + left: 0, + top: 0, + right: 400, + bottom: 200, + width: 400, + height: 200, + x: 0, + y: 0, + toJSON: () => ({}), + }), + }) + scheduledFrames.clear() + + fireEvent.mouseMove(scrollContainer, { clientX: 180, clientY: 48 }) + const previewFrameId = [...scheduledFrames.keys()].at(-1) + expect(previewFrameId).toBeDefined() + + fireEvent.mouseDown(getByTestId('unified-timeline-playhead'), { button: 0 }) + + expect(cancelAnimationFrameSpy).toHaveBeenCalledWith(previewFrameId) + act(() => { + for (const [id, callback] of [...scheduledFrames]) { + scheduledFrames.delete(id) + callback(performance.now()) + } + }) + expect(usePlaybackStore.getState().previewFrame).toBeNull() + + unmount() + animationFrameSpy.mockRestore() + cancelAnimationFrameSpy.mockRestore() + }) + it('gives dense timelines a short window to cancel hover preview before zoom', () => { vi.useFakeTimers() const denseItems = Array.from({ length: 80 }, (_, index) => ({ @@ -992,6 +1063,193 @@ describe('TimelineContent playback selection behavior', () => { expect(usePlaybackStore.getState().previewFrame).toBe(24) }) + it('commits the hover preview when the timeline body is clicked', () => { + const { container } = render() + + act(() => { + usePlaybackStore.getState().setCurrentFrame(90) + usePlaybackStore.getState().setPreviewFrame(24) + usePlaybackStore.getState().play() + }) + + const track = container.querySelector(`[data-track-id="${VIDEO_TRACK.id}"]`) + expect(track).toBeTruthy() + + fireEvent.click(track!, { button: 0, clientX: 80, clientY: 100 }) + + expect(usePlaybackStore.getState().currentFrame).toBe(24) + expect(usePlaybackStore.getState().previewFrame).toBeNull() + expect(usePlaybackStore.getState().isPlaying).toBe(false) + }) + + it('rejects a deferred hover from the click interaction but allows a new move', () => { + vi.useFakeTimers() + const denseItems = Array.from({ length: 80 }, (_, index) => ({ + ...VIDEO_ITEM, + id: `clip-video-${index}`, + from: index * VIDEO_ITEM.durationInFrames, + })) + act(() => { + useItemsStore.getState().setItems(denseItems) + }) + + const frameCallbacks: FrameRequestCallback[] = [] + const animationFrameSpy = vi + .spyOn(window, 'requestAnimationFrame') + .mockImplementation((callback) => { + frameCallbacks.push(callback) + return frameCallbacks.length + }) + const cancelAnimationFrameSpy = vi + .spyOn(window, 'cancelAnimationFrame') + .mockImplementation(() => { + // Model a callback that was already dequeued when cancellation arrived. + }) + + const { container, getByTestId, unmount } = render( + , + ) + const scrollContainer = container.querySelector('[data-timeline-scroll-container]') + if (!(scrollContainer instanceof HTMLDivElement)) { + throw new Error('Expected timeline scroll container') + } + vi.spyOn(scrollContainer, 'getBoundingClientRect').mockReturnValue({ + x: 0, + y: 0, + left: 0, + top: 0, + right: 400, + bottom: 200, + width: 400, + height: 200, + toJSON: () => ({}), + } as DOMRect) + frameCallbacks.length = 0 + + const item = getByTestId('mock-timeline-item') + act(() => { + usePlaybackStore.getState().setCurrentFrame(91) + usePlaybackStore.getState().setPreviewFrame(91, VIDEO_ITEM.id) + }) + + fireEvent.mouseMove(item, { clientX: 7, clientY: 48 }) + act(() => vi.advanceTimersByTime(150)) + const staleClickPreview = frameCallbacks.at(-1) + expect(staleClickPreview).toBeDefined() + frameCallbacks.length = 0 + + fireEvent.mouseDown(item, { button: 0, clientX: 3, clientY: 48 }) + fireEvent.click(item, { button: 0, clientX: 3, clientY: 48 }) + expect(usePlaybackStore.getState()).toMatchObject({ + currentFrame: 1, + previewFrame: null, + previewItemId: null, + }) + + act(() => staleClickPreview?.(performance.now())) + expect(usePlaybackStore.getState()).toMatchObject({ + currentFrame: 1, + previewFrame: null, + previewItemId: null, + }) + + fireEvent.mouseMove(item, { clientX: 7, clientY: 48 }) + act(() => vi.advanceTimersByTime(150)) + const freshPreview = frameCallbacks.at(-1) + expect(freshPreview).toBeDefined() + frameCallbacks.length = 0 + act(() => freshPreview?.(performance.now())) + expect(usePlaybackStore.getState()).toMatchObject({ + currentFrame: 1, + previewFrame: 2, + previewItemId: VIDEO_ITEM.id, + }) + + fireEvent.mouseMove(item, { clientX: 10, clientY: 48 }) + act(() => vi.advanceTimersByTime(150)) + const cancelledPreview = frameCallbacks.at(-1) + expect(cancelledPreview).toBeDefined() + frameCallbacks.length = 0 + fireEvent.mouseLeave(scrollContainer) + act(() => cancelledPreview?.(performance.now())) + expect(usePlaybackStore.getState().previewFrame).toBeNull() + + fireEvent.mouseMove(item, { clientX: 7, clientY: 48 }) + act(() => vi.advanceTimersByTime(150)) + const unmountedPreview = frameCallbacks.at(-1) + expect(unmountedPreview).toBeDefined() + unmount() + act(() => unmountedPreview?.(performance.now())) + expect(usePlaybackStore.getState().previewFrame).toBeNull() + + animationFrameSpy.mockRestore() + cancelAnimationFrameSpy.mockRestore() + vi.useRealTimers() + }) + + it('does not pause or seek when the timeline body is clicked during a microphone take', () => { + const { container } = render() + const pause = vi.spyOn(usePlaybackStore.getState(), 'pause') + + act(() => { + usePlaybackStore.setState({ currentFrame: 90, isPlaying: true }) + useMicRecordingStore.setState({ status: 'recording' }) + }) + act(() => { + usePlaybackStore.setState({ previewFrame: 24 }) + }) + + const track = container.querySelector(`[data-track-id="${VIDEO_TRACK.id}"]`) + expect(track).toBeTruthy() + + fireEvent.mouseDown(track!, { button: 0, clientX: 80, clientY: 100 }) + fireEvent.click(track!, { button: 0, clientX: 80, clientY: 100 }) + + expect(pause).not.toHaveBeenCalled() + expect(usePlaybackStore.getState()).toMatchObject({ + currentFrame: 90, + previewFrame: 24, + isPlaying: true, + }) + }) + + it('does not restore the marquee release preview after a timeline body click', () => { + const { container } = render() + const frameCallbacks: FrameRequestCallback[] = [] + const animationFrameSpy = vi + .spyOn(window, 'requestAnimationFrame') + .mockImplementation((callback) => { + frameCallbacks.push(callback) + return frameCallbacks.length + }) + + act(() => { + usePlaybackStore.getState().setCurrentFrame(90) + usePlaybackStore.getState().setPreviewFrame(24) + }) + + const track = container.querySelector(`[data-track-id="${VIDEO_TRACK.id}"]`) + expect(track).toBeTruthy() + + fireEvent.mouseDown(track!, { button: 0, clientX: 80, clientY: 100 }) + act(() => { + marqueeMocks.onGestureEnd?.( + new MouseEvent('mouseup', { button: 0, clientX: 80, clientY: 100 }), + false, + ) + }) + expect(frameCallbacks).toHaveLength(1) + + fireEvent.click(track!, { button: 0, clientX: 80, clientY: 100 }) + act(() => { + frameCallbacks.splice(0).forEach((callback) => callback(performance.now())) + }) + + expect(usePlaybackStore.getState().currentFrame).toBe(24) + expect(usePlaybackStore.getState().previewFrame).toBeNull() + animationFrameSpy.mockRestore() + }) + it('locks the skim preview from track mousedown until the marquee gesture ends', () => { const { container } = render() diff --git a/src/features/timeline/components/timeline-content.tsx b/src/features/timeline/components/timeline-content.tsx index 726bb5668..96a2c57e1 100644 --- a/src/features/timeline/components/timeline-content.tsx +++ b/src/features/timeline/components/timeline-content.tsx @@ -9,6 +9,7 @@ import { useTimelineSettingsStore } from '../stores/timeline-settings-store' import { useTimelineViewportStore } from '../stores/timeline-viewport-store' import { registerZoomTo100, useZoomStore } from '../stores/zoom-store' import { usePlaybackStore } from '@/shared/state/playback' +import { isMicRecordingActive, useMicRecordingStore } from '@/shared/state/mic-recording-store' import { useEditorStore } from '@/shared/state/editor' import { useSelectionStore } from '@/shared/state/selection' @@ -90,6 +91,64 @@ const DENSE_TIMELINE_HOVER_PREVIEW_DELAY_MS = 150 type TrackScrollbarSection = 'video' | 'audio' | 'single' +function shouldIgnoreTimelineContainerClick( + target: HTMLElement, + interactionJustFinished: boolean, +): boolean { + return ( + interactionJustFinished || + Boolean(target.closest('[role="menu"]')) || + isMicRecordingActive(useMicRecordingStore.getState().status) + ) +} + +function shouldIgnoreTimelineMouseDownCapture(button: number): boolean { + return button !== 0 || isMicRecordingActive(useMicRecordingStore.getState().status) +} + +function resolveTimelineContainerClickFrame( + clientX: number, + container: HTMLDivElement | null, + pixelsToFrame: (pixels: number) => number, + maxTimelineFrame: number, +): number { + const playback = usePlaybackStore.getState() + if (playback.previewFrame !== null) return playback.previewFrame + if (!container) return playback.currentFrame + + const localX = clientX - container.getBoundingClientRect().left + container.scrollLeft + return Math.max(0, Math.min(Math.round(pixelsToFrame(localX)), maxTimelineFrame)) +} + +function seekTimelineTrackAtPointer({ + target, + clientX, + container, + pixelsToFrame, + maxTimelineFrame, +}: { + target: HTMLElement + clientX: number + container: HTMLDivElement | null + pixelsToFrame: (pixels: number) => number + maxTimelineFrame: number +}): void { + if (!target.closest('[data-track-id]')) return + if (useSelectionStore.getState().activeTool === 'razor') return + if (isMicRecordingActive(useMicRecordingStore.getState().status)) return + + const playback = usePlaybackStore.getState() + const frame = resolveTimelineContainerClickFrame( + clientX, + container, + pixelsToFrame, + maxTimelineFrame, + ) + playback.pause() + playback.setPreviewFrame(null) + playback.setCurrentFrame(frame) +} + function revealTrackInScrollContainer(container: HTMLDivElement | null, trackId: string): boolean { if (!container) { return false @@ -823,9 +882,11 @@ export const TimelineContent = memo(function TimelineContent({ const setPreviewFrame = usePlaybackStore((s) => s.setPreviewFrame) const setPreviewFrameRef = useRef(setPreviewFrame) setPreviewFrameRef.current = setPreviewFrame + const previewInteractionEpochRef = useRef(0) const previewRafRef = useRef(null) const previewDelayTimeoutRef = useRef | null>(null) const cancelPendingHoverPreview = useCallback(() => { + previewInteractionEpochRef.current += 1 if (previewDelayTimeoutRef.current !== null) { clearTimeout(previewDelayTimeoutRef.current) previewDelayTimeoutRef.current = null @@ -834,6 +895,10 @@ export const TimelineContent = memo(function TimelineContent({ cancelAnimationFrame(previewRafRef.current) previewRafRef.current = null } + if (marqueeReleaseRafRef.current !== null) { + cancelAnimationFrame(marqueeReleaseRafRef.current) + marqueeReleaseRafRef.current = null + } }, []) useTimelineAudioSkimPreview() @@ -1275,6 +1340,7 @@ export const TimelineContent = memo(function TimelineContent({ const target = e.target as HTMLElement // Check if mousedown is on a playhead handle or timeline ruler if (target.closest('[data-playhead-handle]') || target.closest('.timeline-ruler')) { + cancelPendingHoverPreview() scrubWasActiveRef.current = true } } @@ -1306,32 +1372,54 @@ export const TimelineContent = memo(function TimelineContent({ scrubTimeoutRef.current = null } } - }, []) + }, [cancelPendingHoverPreview]) - // Click empty space to deselect items and markers (but preserve track selection) + // Commit the hover skimmer on a normal timeline click. Ruler clicks own their + // own scrub path, while drag/marquee/razor gestures must not move playback. const handleContainerClick = (e: React.MouseEvent) => { - // Don't deselect if marquee selection, drag, or scrubbing just finished - if (marqueeWasActiveRef.current || dragWasActiveRef.current || scrubWasActiveRef.current) { - return - } - - // Don't deselect if clicking inside a context menu portal (Radix renders - // menus in a portal outside the timeline DOM, but React synthetic events - // still bubble through the component tree) const target = e.target as HTMLElement - if (target.closest('[role="menu"]')) { + const interactionJustFinished = + marqueeWasActiveRef.current || dragWasActiveRef.current || scrubWasActiveRef.current + // Radix menus render outside the timeline DOM, but their synthetic events + // still bubble through this component tree. + if (shouldIgnoreTimelineContainerClick(target, interactionJustFinished)) { return } - // Deselect items and markers if NOT clicking on a timeline item + // A normal background click arrives after the marquee mouseup callback. + // Cancel its queued preview restore before committing the click so the + // program monitor follows currentFrame instead of resurrecting the hover + // frame on the next animation frame. + cancelPendingHoverPreview() const clickedOnItem = target.closest('[data-item-id]') + seekTimelineTrackAtPointer({ + target, + clientX: e.clientX, + container: containerRef.current, + pixelsToFrame: pixelsToFrameRef.current, + maxTimelineFrame: maxTimelineFrameRef.current, + }) + // Deselect items and markers if NOT clicking on a timeline item. if (!clickedOnItem) { clearItemSelection() selectMarker(null) // Also clear marker selection } } + const handleTimelineClickCapture = useCallback( + (e: React.MouseEvent) => { + if (e.button !== 0) return + const target = e.target as HTMLElement + if (!target.closest('[data-track-id]')) return + + // Item clicks stop propagation, so invalidate hover work here before the + // item's click handler commits its own geometry-derived seek. + cancelPendingHoverPreview() + }, + [cancelPendingHoverPreview], + ) + // Build snap targets for razor shift-snap (item edges, grid, playhead, markers) // Called on-demand during mouse move — reads stores directly to avoid subscriptions const buildRazorSnapTargets = useCallback((): RazorSnapTarget[] => { @@ -1356,37 +1444,33 @@ export const TimelineContent = memo(function TimelineContent({ }, []) // Preview scrubber: show ghost playhead on hover - const handleTimelineMouseDownCapture = useCallback((e: React.MouseEvent) => { - if (e.button !== 0) return + const handleTimelineMouseDownCapture = useCallback( + (e: React.MouseEvent) => { + if (shouldIgnoreTimelineMouseDownCapture(e.button)) return - const target = e.target as HTMLElement - if ( - !target.closest('[data-track-id]') || - target.closest('[data-item-id]') || - target.closest('[data-timeline-density-bucket]') - ) { - return - } + const target = e.target as HTMLElement + if (!target.closest('[data-track-id]')) return - // A press on track background is a potential marquee gesture. Freeze the - // skim target immediately so the few pixels before marquee activation do - // not briefly seek the preview away from the mouse-down frame. - marqueePointerDownRef.current = true - if (marqueeReleaseRafRef.current !== null) { - cancelAnimationFrame(marqueeReleaseRafRef.current) - marqueeReleaseRafRef.current = null - } - const playback = usePlaybackStore.getState() - marqueeStartPreviewFrameRef.current = playback.previewFrame - marqueeReleasePreviewRef.current = - playback.previewFrame === null - ? null - : { frame: playback.previewFrame, itemId: playback.previewItemId ?? undefined } - if (previewRafRef.current !== null) { - cancelAnimationFrame(previewRafRef.current) - previewRafRef.current = null - } - }, []) + // Start a new interaction epoch before item/background handlers run. A + // hover callback already dequeued by the browser can no longer take display + // ownership during this pointer interaction. + cancelPendingHoverPreview() + if (target.closest('[data-item-id]') || target.closest('[data-timeline-density-bucket]')) + return + + // A press on track background is a potential marquee gesture. Freeze the + // skim target immediately so the few pixels before marquee activation do + // not briefly seek the preview away from the mouse-down frame. + marqueePointerDownRef.current = true + const playback = usePlaybackStore.getState() + marqueeStartPreviewFrameRef.current = playback.previewFrame + marqueeReleasePreviewRef.current = + playback.previewFrame === null + ? null + : { frame: playback.previewFrame, itemId: playback.previewItemId ?? undefined } + }, + [cancelPendingHoverPreview], + ) const finishMarqueePointerGesture = useCallback((e: MouseEvent) => { const wasMarqueePointerGesture = marqueePointerDownRef.current @@ -1409,10 +1493,15 @@ export const TimelineContent = memo(function TimelineContent({ if (pointerIsInsideTimeline && releasePreview) { // Complete marquee teardown first. Its mouseup path may clear transient // preview state later in the same event dispatch. - marqueeReleaseRafRef.current = requestAnimationFrame(() => { - marqueeReleaseRafRef.current = null + const previewEpoch = previewInteractionEpochRef.current + const releaseRafId = requestAnimationFrame(() => { + if (marqueeReleaseRafRef.current === releaseRafId) { + marqueeReleaseRafRef.current = null + } + if (previewEpoch !== previewInteractionEpochRef.current) return setPreviewFrameRef.current(releasePreview.frame, releasePreview.itemId) }) + marqueeReleaseRafRef.current = releaseRafId } else { setPreviewFrameRef.current(null) } @@ -1534,21 +1623,29 @@ export const TimelineContent = memo(function TimelineContent({ // normal hover responsive while allowing Ctrl/Cmd-wheel to cancel the // pending preview before it can compete with the first zoom frame. cancelPendingHoverPreview() + const previewEpoch = previewInteractionEpochRef.current const schedulePreviewFrame = () => { - previewDelayTimeoutRef.current = null - previewRafRef.current = requestAnimationFrame(() => { - previewRafRef.current = null + if (previewEpoch !== previewInteractionEpochRef.current) return + const previewRafId = requestAnimationFrame(() => { + if (previewRafRef.current === previewRafId) { + previewRafRef.current = null + } + if (previewEpoch !== previewInteractionEpochRef.current) return withPerfMeasure('tl.raf.previewHover', () => setPreviewFrameRef.current(frame, itemId)) }) + previewRafRef.current = previewRafId } if ( useItemsStore.getState().items.length >= DENSE_TIMELINE_TRACK_ITEM_THRESHOLD && usePlaybackStore.getState().previewFrame === null ) { - previewDelayTimeoutRef.current = setTimeout( - schedulePreviewFrame, - DENSE_TIMELINE_HOVER_PREVIEW_DELAY_MS, - ) + const previewDelayTimeout = setTimeout(() => { + if (previewDelayTimeoutRef.current === previewDelayTimeout) { + previewDelayTimeoutRef.current = null + } + schedulePreviewFrame() + }, DENSE_TIMELINE_HOVER_PREVIEW_DELAY_MS) + previewDelayTimeoutRef.current = previewDelayTimeout } else { schedulePreviewFrame() } @@ -2141,6 +2238,7 @@ export const TimelineContent = memo(function TimelineContent({ willChange: 'scroll-position', }} onMouseDownCapture={handleTimelineMouseDownCapture} + onClickCapture={handleTimelineClickCapture} onClick={handleContainerClick} onMouseMove={handleTimelineMouseMove} onMouseLeave={handleTimelineMouseLeave} diff --git a/src/features/timeline/components/timeline-item/use-timeline-item-pointer-handlers.test.tsx b/src/features/timeline/components/timeline-item/use-timeline-item-pointer-handlers.test.tsx index 0f35021e6..98ee53983 100644 --- a/src/features/timeline/components/timeline-item/use-timeline-item-pointer-handlers.test.tsx +++ b/src/features/timeline/components/timeline-item/use-timeline-item-pointer-handlers.test.tsx @@ -3,6 +3,8 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vite-plus/test' import type { CompositionItem, TextItem, VideoItem } from '@/types/timeline' import { useSelectionStore } from '@/shared/state/selection' import { useEditorStore } from '@/shared/state/editor' +import { usePlaybackStore } from '@/shared/state/playback' +import { useMicRecordingStore } from '@/shared/state/mic-recording-store' import { useSourcePlayerStore } from '@/shared/state/source-player' import { useTimelineStore } from '../../stores/timeline-store' import { useCompositionNavigationStore } from '../../stores/composition-navigation-store' @@ -114,6 +116,13 @@ describe('useTimelineItemPointerHandlers', () => { vi.clearAllMocks() // Deterministic single-item selection (linked selection expands target ids) useEditorStore.getState().setLinkedSelectionEnabled(false) + usePlaybackStore.setState({ + currentFrame: 0, + previewFrame: null, + previewItemId: null, + isPlaying: false, + }) + useMicRecordingStore.setState({ status: 'idle' }) }) afterEach(() => { @@ -141,6 +150,37 @@ describe('useTimelineItemPointerHandlers', () => { expect(selectItems).toHaveBeenCalledWith(['item-1']) }) + it('seeks from click geometry when the hover preview is stale at the next boundary', () => { + usePlaybackStore.setState({ + currentFrame: 0, + previewFrame: 50, + previewItemId: 'item-1', + isPlaying: true, + }) + const handlers = renderHandlers(makeInput({ activeTool: 'select' })) + + handlers.handleClick(makeMouseEvent({ clientX: 2 })) + + expect(usePlaybackStore.getState().currentFrame).toBe(20) + expect(usePlaybackStore.getState().previewFrame).toBeNull() + expect(usePlaybackStore.getState().isPlaying).toBe(false) + }) + + it('selects without seeking or pausing while a microphone take is active', () => { + usePlaybackStore.setState({ currentFrame: 12, previewFrame: 34, isPlaying: true }) + useMicRecordingStore.setState({ status: 'recording' }) + const handlers = renderHandlers(makeInput({ activeTool: 'select' })) + + handlers.handleClick(makeMouseEvent()) + + expect(useSelectionStore.getState().selectedItemIds).toEqual(['item-1']) + expect(usePlaybackStore.getState()).toMatchObject({ + currentFrame: 12, + previewFrame: 34, + isPlaying: true, + }) + }) + it('splits the item at the cursor with the razor tool', () => { const splitItem = vi.spyOn(useTimelineStore.getState(), 'splitItem') const handlers = renderHandlers(makeInput({ activeTool: 'razor' })) diff --git a/src/features/timeline/components/timeline-item/use-timeline-item-pointer-handlers.ts b/src/features/timeline/components/timeline-item/use-timeline-item-pointer-handlers.ts index 173d19663..1621f61a8 100644 --- a/src/features/timeline/components/timeline-item/use-timeline-item-pointer-handlers.ts +++ b/src/features/timeline/components/timeline-item/use-timeline-item-pointer-handlers.ts @@ -2,6 +2,7 @@ import { useCallback, type Dispatch, type RefObject, type SetStateAction } from import type { TimelineItem as TimelineItemType } from '@/types/timeline' import type { SelectionState } from '@/shared/state/selection' import { usePlaybackStore } from '@/shared/state/playback' +import { isMicRecordingActive, useMicRecordingStore } from '@/shared/state/mic-recording-store' import { useEditorStore } from '@/shared/state/editor' import { useSourcePlayerStore } from '@/shared/state/source-player' import { useSelectionStore } from '@/shared/state/selection' @@ -156,6 +157,26 @@ export function useTimelineItemPointerHandlers({ return } + // Clip clicks stop propagation for selection, so they must explicitly + // seek from this click's own geometry. The hover skimmer can still hold + // the previous pointer event (including the next clip boundary), so it + // must never own the committed click frame. + if (!isMicRecordingActive(useMicRecordingStore.getState().status)) { + const playback = usePlaybackStore.getState() + const rect = e.currentTarget.getBoundingClientRect() + const relativeX = Math.max(0, Math.min(e.clientX - rect.left, rect.width)) + const frameOffset = + rect.width > 0 + ? Math.min( + Math.max(0, item.durationInFrames - 1), + Math.floor((relativeX / rect.width) * item.durationInFrames), + ) + : 0 + const clickedFrame = Math.max(0, item.from + frameOffset) + playback.pause() + playback.finishScrub(clickedFrame) + } + if (activeToolRef.current === 'select' || activeToolRef.current === 'trim-edit') { const bridgedHandle = smartTrimIntentToHandle(smartTrimIntentRef.current) if (bridgedHandle) { @@ -193,7 +214,15 @@ export function useTimelineItemPointerHandlers({ selectItems(targetIds) } }, - [activeToolRef, dragWasActiveRef, trackLocked, item.from, item.id, smartTrimIntentRef], + [ + activeToolRef, + dragWasActiveRef, + trackLocked, + item.durationInFrames, + item.from, + item.id, + smartTrimIntentRef, + ], ) // Double-click: open media in source monitor with clip's source range as I/O diff --git a/src/features/timeline/components/timeline-markers.test.tsx b/src/features/timeline/components/timeline-markers.test.tsx index bbe6783a6..e6699c419 100644 --- a/src/features/timeline/components/timeline-markers.test.tsx +++ b/src/features/timeline/components/timeline-markers.test.tsx @@ -219,6 +219,37 @@ describe('TimelineMarkers ruler scrub cancellation', () => { expect(usePlaybackStore.getState().previewFrame).toBe(30) }) + it('does not restore a queued ruler hover after click-seek release', () => { + const frameCallbacks: FrameRequestCallback[] = [] + vi.spyOn(window, 'requestAnimationFrame').mockImplementation((callback) => { + frameCallbacks.push(callback) + return frameCallbacks.length + }) + const { container } = render( +
+ +
, + ) + const ruler = container.querySelector('[style*="cursor: ew-resize"]') as HTMLDivElement + ruler.getBoundingClientRect = () => + ({ + left: 0, + right: 1000, + top: 0, + bottom: 34, + width: 1000, + height: 34, + }) as DOMRect + + fireEvent.mouseMove(ruler, { clientX: 100 }) + fireEvent.mouseDown(ruler, { button: 0, clientX: 260 }) + fireEvent.mouseUp(document, { clientX: 260 }) + act(() => frameCallbacks.splice(0).forEach((callback) => callback(performance.now()))) + + expect(usePlaybackStore.getState().currentFrame).toBe(78) + expect(usePlaybackStore.getState().previewFrame).toBeNull() + }) + it('keeps the IO strip in its own lane above the viewport ruler canvas', () => { useTimelineStore.setState({ inPoint: 15, outPoint: 45 }) diff --git a/src/features/timeline/components/timeline-markers.tsx b/src/features/timeline/components/timeline-markers.tsx index 9fa308c0c..2991b4895 100644 --- a/src/features/timeline/components/timeline-markers.tsx +++ b/src/features/timeline/components/timeline-markers.tsx @@ -18,6 +18,7 @@ import { beginTimelineSkimmerScrub, endTimelineSkimmerScrub, mainTimelineScrubActiveRef, + timelineSkimmerScrubSignal, } from '@/shared/timeline/main-timeline-scrub' import { getTimelineScrubViewportProgress, @@ -912,6 +913,19 @@ export const TimelineMarkers = memo(function TimelineMarkers({ [], ) + useEffect( + () => + timelineSkimmerScrubSignal.subscribe(() => { + if (!timelineSkimmerScrubSignal.current) return + if (hoverPreviewRafRef.current !== null) { + cancelAnimationFrame(hoverPreviewRafRef.current) + hoverPreviewRafRef.current = null + } + pendingHoverPreviewFrameRef.current = null + }), + [], + ) + const handleRangeMouseDown = useCallback( (e: React.PointerEvent) => { const startIn = inPointRef.current @@ -997,6 +1011,16 @@ export const TimelineMarkers = memo(function TimelineMarkers({ // without moving the mic audio would desync the recording irreparably. if (isMicRecordingActive(useMicRecordingStore.getState().status)) return + // A pointer move immediately before mousedown may still have a hover + // publication queued for the next animation frame. The click scrub now + // owns this pointer sample, so cancel that older preview before it can + // resurrect transient state after mouseup clears the scrub preview. + if (hoverPreviewRafRef.current !== null) { + cancelAnimationFrame(hoverPreviewRafRef.current) + hoverPreviewRafRef.current = null + } + pendingHoverPreviewFrameRef.current = null + // Clear marker selection when clicking on ruler (only if a marker is selected) const { selectedMarkerId } = useSelectionStore.getState() if (selectedMarkerId) { diff --git a/src/features/timeline/hooks/shortcuts/use-clipboard-shortcuts.test.tsx b/src/features/timeline/hooks/shortcuts/use-clipboard-shortcuts.test.tsx new file mode 100644 index 000000000..7bac058d4 --- /dev/null +++ b/src/features/timeline/hooks/shortcuts/use-clipboard-shortcuts.test.tsx @@ -0,0 +1,458 @@ +import { act, render } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vite-plus/test' +import { HOTKEYS } from '@/config/hotkeys' +import { useClipboardStore } from '@/shared/state/clipboard' +import { useSelectionStore } from '@/shared/state/selection' +import type { AudioItem, TextItem, TimelineItem, TimelineTrack, VideoItem } from '@/types/timeline' +import { useCompositionNavigationStore } from '../../stores/composition-navigation-store' +import { useKeyframeSelectionStore } from '../../stores/keyframe-selection-store' +import { useTimelineStore } from '../../stores/timeline-store' +import { useTimelineCommandStore } from '../../stores/timeline-command-store' +import { useClipboardShortcuts } from './use-clipboard-shortcuts' + +const { addItemsMock, playbackState, useHotkeysMock } = vi.hoisted(() => ({ + addItemsMock: vi.fn(), + playbackState: { + currentFrame: 200, + setCurrentFrame: vi.fn(), + setBusAudioEq: vi.fn(), + setMasterBusDb: vi.fn(), + }, + useHotkeysMock: vi.fn(), +})) + +vi.mock('react-hotkeys-hook', () => ({ + useHotkeys: useHotkeysMock, +})) + +vi.mock('@/shared/state/playback', () => ({ + usePlaybackStore: { + getState: () => playbackState, + }, +})) + +vi.mock('sonner', () => ({ + toast: { + success: vi.fn(), + }, +})) + +vi.mock('../../stores/timeline-actions', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + addItems: addItemsMock, + } +}) + +const TARGET_TRACK: TimelineTrack = { + id: 'target-track', + name: 'V1', + kind: 'video', + order: 0, + height: 80, + locked: false, + visible: true, + muted: false, + solo: false, + items: [], +} + +const AUDIO_TRACK: TimelineTrack = { + ...TARGET_TRACK, + id: 'target-audio', + name: 'A1', + kind: 'audio', + order: 1, +} + +const SECOND_VIDEO_TRACK: TimelineTrack = { + ...TARGET_TRACK, + id: 'target-video-2', + name: 'V2', + order: 1, +} +const SECOND_AUDIO_TRACK: TimelineTrack = { + ...AUDIO_TRACK, + id: 'target-audio-2', + name: 'A2', + order: 3, +} + +function makeVideoItem(overrides: Partial = {}): VideoItem { + return { + id: 'clip-1', + type: 'video', + trackId: TARGET_TRACK.id, + from: 0, + durationInFrames: 10, + label: 'Clip', + src: 'clip.mp4', + ...overrides, + } +} + +function makeAudioItem(overrides: Partial = {}): AudioItem { + return { + id: 'audio-1', + type: 'audio', + trackId: AUDIO_TRACK.id, + from: 0, + durationInFrames: 10, + label: 'Audio', + src: 'clip.mp4', + ...overrides, + } +} + +function makeCaptionItem(overrides: Partial = {}): TextItem { + return { + id: 'caption-1', + type: 'text', + trackId: 'missing-caption-track', + from: 0, + durationInFrames: 10, + label: 'Caption', + text: 'Caption', + color: '#fff', + textRole: 'caption', + ...overrides, + } +} + +function ShortcutHarness() { + useClipboardShortcuts() + return null +} + +type HotkeyCallback = (event: { preventDefault: () => void }) => void + +function getPasteCallback(): HotkeyCallback { + const registration = useHotkeysMock.mock.calls.find(([keys]) => keys === HOTKEYS.PASTE) + expect(registration).toBeDefined() + return registration?.[1] as HotkeyCallback +} + +function getPlannedItems(): TimelineItem[] { + expect(addItemsMock).toHaveBeenCalledTimes(1) + return addItemsMock.mock.calls[0]?.[0] as TimelineItem[] +} + +describe('useClipboardShortcuts paste placement', () => { + beforeEach(() => { + addItemsMock.mockClear() + useHotkeysMock.mockClear() + vi.spyOn(window, 'requestAnimationFrame').mockImplementation(() => 0) + + useTimelineStore.setState({ + tracks: [TARGET_TRACK], + items: [], + transitions: [], + keyframes: [], + markers: [], + }) + useSelectionStore.setState({ + selectedItemIds: [], + selectedItemIdSet: new Set(), + selectedTransitionId: null, + activeTrackId: TARGET_TRACK.id, + }) + useKeyframeSelectionStore.setState({ + selectedKeyframes: [], + clipboard: null, + isCut: false, + }) + useCompositionNavigationStore.setState({ activeCompositionId: null }) + useClipboardStore.setState({ itemsClipboard: null, transitionClipboard: null }) + playbackState.currentFrame = 200 + useTimelineCommandStore.getState().clearHistory() + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + + it('anchors the earliest copied item at the playhead and preserves relative offsets', () => { + useClipboardStore + .getState() + .copyItems( + [ + makeVideoItem({ id: 'early', label: 'Early', from: 40 }), + makeVideoItem({ id: 'late', label: 'Late', from: 70 }), + ], + 0, + 'copy', + ) + + render() + act(() => getPasteCallback()({ preventDefault: vi.fn() })) + + expect(getPlannedItems().map((item) => ({ label: item.label, from: item.from }))).toEqual([ + { label: 'Early', from: 200 }, + { label: 'Late', from: 230 }, + ]) + }) + + it('checks already-planned pasted items when source tracks map to one target track', () => { + useClipboardStore + .getState() + .copyItems( + [ + makeVideoItem({ id: 'first', label: 'First', trackId: 'missing-v1', from: 40 }), + makeVideoItem({ id: 'second', label: 'Second', trackId: 'missing-v1', from: 45 }), + ], + 0, + 'copy', + ) + + render() + act(() => getPasteCallback()({ preventDefault: vi.fn() })) + + const plannedItems = getPlannedItems() + expect(plannedItems.map((item) => item.trackId)).toEqual([TARGET_TRACK.id, TARGET_TRACK.id]) + expect(plannedItems.map((item) => item.from)).toEqual([200, 210]) + expect(plannedItems[0]!.from + plannedItems[0]!.durationInFrames).toBeLessThanOrEqual( + plannedItems[1]!.from, + ) + }) + + it('moves a linked video/audio pair together when one target track collides', () => { + useTimelineStore.setState({ + tracks: [TARGET_TRACK, AUDIO_TRACK], + items: [makeVideoItem({ id: 'occupied', from: 200 })], + }) + useClipboardStore + .getState() + .copyItems( + [ + makeVideoItem({ id: 'video', from: 40, linkedGroupId: 'linked-source' }), + makeAudioItem({ id: 'audio', from: 40, linkedGroupId: 'linked-source' }), + ], + 0, + 'copy', + ) + + render() + act(() => getPasteCallback()({ preventDefault: vi.fn() })) + + const plannedItems = getPlannedItems() + expect(plannedItems.map((item) => item.from)).toEqual([210, 210]) + expect(plannedItems[0]!.linkedGroupId).toBeTruthy() + expect(plannedItems[1]!.linkedGroupId).toBe(plannedItems[0]!.linkedGroupId) + }) + + it('maps linked A/V items with absent source IDs to separate compatible lanes', () => { + useTimelineStore.setState({ tracks: [TARGET_TRACK, AUDIO_TRACK] }) + useClipboardStore + .getState() + .copyItems( + [ + makeVideoItem({ id: 'missing-video', trackId: 'source-v', linkedGroupId: 'pair' }), + makeAudioItem({ id: 'missing-audio', trackId: 'source-a', linkedGroupId: 'pair' }), + ], + 0, + 'copy', + ) + + render() + act(() => getPasteCallback()({ preventDefault: vi.fn() })) + + expect(getPlannedItems().map((item) => item.trackId)).toEqual([TARGET_TRACK.id, AUDIO_TRACK.id]) + }) + + it('preserves lane ordinals for multiple linked pairs and keeps captions on video lanes', () => { + useTimelineStore.setState({ + tracks: [TARGET_TRACK, SECOND_VIDEO_TRACK, AUDIO_TRACK, SECOND_AUDIO_TRACK], + }) + useClipboardStore + .getState() + .copyItems( + [ + makeVideoItem({ id: 'v1', trackId: 'source-v1', linkedGroupId: 'pair-1' }), + makeAudioItem({ id: 'a1', trackId: 'source-a1', linkedGroupId: 'pair-1' }), + makeVideoItem({ id: 'v2', trackId: 'source-v2', from: 20, linkedGroupId: 'pair-2' }), + makeAudioItem({ id: 'a2', trackId: 'source-a2', from: 20, linkedGroupId: 'pair-2' }), + makeCaptionItem({ id: 'caption', trackId: 'source-v2', from: 20 }), + ], + 0, + 'copy', + ) + + render() + act(() => getPasteCallback()({ preventDefault: vi.fn() })) + + const plannedItems = getPlannedItems() + expect(plannedItems.map((item) => item.trackId)).toEqual([ + TARGET_TRACK.id, + AUDIO_TRACK.id, + SECOND_VIDEO_TRACK.id, + SECOND_AUDIO_TRACK.id, + SECOND_VIDEO_TRACK.id, + ]) + expect(plannedItems.filter((item) => item.type === 'text')[0]?.trackId).toBe( + SECOND_VIDEO_TRACK.id, + ) + }) + + it('uses surviving IDs while resolving missing linked members by kind', () => { + useTimelineStore.setState({ tracks: [TARGET_TRACK, AUDIO_TRACK] }) + useClipboardStore + .getState() + .copyItems( + [ + makeVideoItem({ id: 'surviving-video', trackId: TARGET_TRACK.id, linkedGroupId: 'pair' }), + makeAudioItem({ id: 'missing-audio', trackId: 'source-a', linkedGroupId: 'pair' }), + ], + 0, + 'copy', + ) + + render() + act(() => getPasteCallback()({ preventDefault: vi.fn() })) + + expect(getPlannedItems().map((item) => item.trackId)).toEqual([TARGET_TRACK.id, AUDIO_TRACK.id]) + }) + + it('keeps source ordinals separate when malformed A/V lanes reuse an id', () => { + useTimelineStore.setState({ + tracks: [TARGET_TRACK, AUDIO_TRACK, SECOND_AUDIO_TRACK], + }) + useClipboardStore + .getState() + .copyItems( + [ + makeVideoItem({ id: 'shared-video', trackId: 'shared-missing', linkedGroupId: 'pair' }), + makeAudioItem({ id: 'shared-audio', trackId: 'shared-missing', linkedGroupId: 'pair' }), + makeAudioItem({ id: 'second-audio', trackId: 'second-missing', from: 20 }), + ], + 0, + 'copy', + ) + + render() + act(() => getPasteCallback()({ preventDefault: vi.fn() })) + + expect(getPlannedItems().map((item) => item.trackId)).toEqual([ + TARGET_TRACK.id, + AUDIO_TRACK.id, + SECOND_AUDIO_TRACK.id, + ]) + }) + + it('reserves a later surviving lane before assigning an earlier missing source', () => { + useTimelineStore.setState({ tracks: [TARGET_TRACK] }) + useClipboardStore + .getState() + .copyItems( + [ + makeVideoItem({ id: 'missing-first', label: 'Missing', trackId: 'missing-video' }), + makeVideoItem({ id: 'surviving-second', label: 'Surviving', trackId: TARGET_TRACK.id }), + ], + 0, + 'copy', + ) + + render() + act(() => getPasteCallback()({ preventDefault: vi.fn() })) + + const pastedItems = useTimelineStore.getState().items + expect(pastedItems.find((item) => item.label === 'Surviving')?.trackId).toBe(TARGET_TRACK.id) + expect(new Set(pastedItems.map((item) => item.trackId)).size).toBe(2) + expect(useTimelineStore.getState().tracks).toHaveLength(2) + }) + + it('splits malformed overlapping members of one linked group safely', () => { + useClipboardStore + .getState() + .copyItems( + [ + makeVideoItem({ id: 'overlap-1', trackId: 'same-source', linkedGroupId: 'bad-group' }), + makeVideoItem({ id: 'overlap-2', trackId: 'same-source', linkedGroupId: 'bad-group' }), + ], + 0, + 'copy', + ) + + render() + act(() => getPasteCallback()({ preventDefault: vi.fn() })) + + const plannedItems = getPlannedItems() + expect(plannedItems.map((item) => item.from)).toEqual([200, 210]) + expect(plannedItems[0]!.from + plannedItems[0]!.durationInFrames).toBeLessThanOrEqual( + plannedItems[1]!.from, + ) + }) + + it('creates deterministic compatible lanes and undoes tracks and items together', () => { + useClipboardStore + .getState() + .copyItems( + [ + makeVideoItem({ id: 'lane-1', trackId: 'missing-v1', from: 0 }), + makeVideoItem({ id: 'lane-2', trackId: 'missing-v2', from: 0 }), + ], + 0, + 'copy', + ) + + render() + act(() => getPasteCallback()({ preventDefault: vi.fn() })) + + expect(useTimelineStore.getState().tracks.map((track) => track.kind)).toEqual([ + 'video', + 'video', + ]) + expect(useTimelineStore.getState().items).toHaveLength(2) + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(1) + + act(() => useTimelineCommandStore.getState().undo()) + expect(useTimelineStore.getState().tracks).toHaveLength(1) + expect(useTimelineStore.getState().tracks[0]?.id).toBe(TARGET_TRACK.id) + expect(useTimelineStore.getState().items).toEqual([]) + }) + + it('creates and atomically undoes a missing linked audio lane after a video collision', () => { + useTimelineStore.setState({ + tracks: [TARGET_TRACK], + items: [makeVideoItem({ id: 'occupied-video', from: 200 })], + }) + useClipboardStore.getState().copyItems( + [ + makeVideoItem({ + id: 'linked-video', + trackId: 'missing-video', + linkedGroupId: 'source-pair', + }), + makeAudioItem({ + id: 'linked-audio', + trackId: 'missing-audio', + linkedGroupId: 'source-pair', + }), + ], + 0, + 'copy', + ) + + render() + act(() => getPasteCallback()({ preventDefault: vi.fn() })) + + const pasted = useTimelineStore.getState().items.filter((item) => item.id !== 'occupied-video') + expect(pasted).toHaveLength(2) + expect(pasted.map((item) => item.from)).toEqual([210, 210]) + expect(pasted[0]!.linkedGroupId).toBeTruthy() + expect(pasted[1]!.linkedGroupId).toBe(pasted[0]!.linkedGroupId) + expect(useTimelineStore.getState().tracks.map((track) => track.kind)).toEqual([ + 'video', + 'audio', + ]) + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(1) + + act(() => useTimelineCommandStore.getState().undo()) + expect(useTimelineStore.getState().tracks).toHaveLength(1) + expect(useTimelineStore.getState().tracks[0]).toMatchObject({ + id: TARGET_TRACK.id, + kind: 'video', + }) + expect(useTimelineStore.getState().items.map((item) => item.id)).toEqual(['occupied-video']) + }) +}) diff --git a/src/features/timeline/hooks/shortcuts/use-clipboard-shortcuts.ts b/src/features/timeline/hooks/shortcuts/use-clipboard-shortcuts.ts index d0f05df13..4c3742bfd 100644 --- a/src/features/timeline/hooks/shortcuts/use-clipboard-shortcuts.ts +++ b/src/features/timeline/hooks/shortcuts/use-clipboard-shortcuts.ts @@ -14,13 +14,255 @@ import { useCompositionsStore } from '../../stores/compositions-store' import { useKeyframeSelectionStore } from '../../stores/keyframe-selection-store' import { HOTKEY_OPTIONS } from '@/config/hotkeys' import type { Transition } from '@/types/transition' -import type { TimelineItem } from '@/types/timeline' +import type { TimelineItem, TimelineTrack } from '@/types/timeline' import { useResolvedHotkeys } from '@/features/timeline/deps/settings' import { isCompositionWrapperItem, wouldCreateCompositionCycle, } from '../../utils/composition-graph' import { handleTranscriptClipboardCopy } from '../../utils/transcript-copy-bridge' +import { createClassicTrack, getTrackKind } from '../../utils/classic-tracks' + +interface PastePlacementPlan { + itemData: Omit + targetTrackId: string + desiredFrom: number + sourceIndex: number +} + +function placementsOverlap( + left: { trackId: string; from: number; durationInFrames: number }, + right: { trackId: string; from: number; durationInFrames: number }, +): boolean { + return ( + left.trackId === right.trackId && + left.from < right.from + right.durationInFrames && + left.from + left.durationInFrames > right.from + ) +} + +function hasInternalPlacementOverlap(plans: PastePlacementPlan[]): boolean { + return plans.some((plan, index) => + plans.slice(index + 1).some((candidate) => + placementsOverlap( + { + trackId: plan.targetTrackId, + from: plan.desiredFrom, + durationInFrames: plan.itemData.durationInFrames, + }, + { + trackId: candidate.targetTrackId, + from: candidate.desiredFrom, + durationInFrames: candidate.itemData.durationInFrames, + }, + ), + ), + ) +} + +type PasteTrackKind = 'video' | 'audio' + +interface PasteSourceLane { + key: string + kind: PasteTrackKind + sourceTrackId: string + ordinal: number +} + +function getPasteTrackKind(item: Omit): PasteTrackKind { + return item.type === 'audio' ? 'audio' : 'video' +} + +function isCompatiblePasteTrack(track: TimelineTrack, kind: PasteTrackKind): boolean { + const trackKind = getTrackKind(track) + return kind === 'audio' ? trackKind === 'audio' : trackKind !== 'audio' +} + +function getPasteDestinationTrackIds(tracks: TimelineTrack[], kind: PasteTrackKind): string[] { + return tracks + .filter((track) => !track.isGroup && isCompatiblePasteTrack(track, kind)) + .sort((left, right) => left.order - right.order) + .map((track) => track.id) +} + +function collectPasteSourceLanes(pasteItems: Array>) { + const lanesByKind: Record = { video: [], audio: [] } + const laneByKey = new Map() + const laneKeyByItemIndex = new Map() + + for (const [sourceIndex, itemData] of pasteItems.entries()) { + const kind = getPasteTrackKind(itemData) + const key = `${kind}:${itemData.trackId}` + let lane = laneByKey.get(key) + if (!lane) { + lane = { + key, + kind, + sourceTrackId: itemData.trackId, + ordinal: lanesByKind[kind].length, + } + laneByKey.set(key, lane) + lanesByKind[kind].push(lane) + } + laneKeyByItemIndex.set(sourceIndex, key) + } + + return { lanesByKind, laneKeyByItemIndex } +} + +function findExactPasteTrack( + lane: PasteSourceLane, + tracks: TimelineTrack[], +): TimelineTrack | undefined { + return tracks.find( + (track) => track.id === lane.sourceTrackId && isCompatiblePasteTrack(track, lane.kind), + ) +} + +function appendPasteDestinationTrack( + tracks: TimelineTrack[], + kind: PasteTrackKind, +): { trackId: string; tracks: TimelineTrack[] } { + const minOrder = Math.min(...tracks.map((track) => track.order), 0) + const maxOrder = Math.max(...tracks.map((track) => track.order), 0) + const newTrack = createClassicTrack({ + tracks, + kind, + order: kind === 'video' ? minOrder - 1 : maxOrder + 1, + }) + return { trackId: newTrack.id, tracks: [...tracks, newTrack] } +} + +function planSingleSourceSection(params: { + activeTrackId: string | null + kind: PasteTrackKind + lanes: PasteSourceLane[] + tracks: TimelineTrack[] +}): { assignments: Map; tracks: TimelineTrack[] } { + const { activeTrackId, kind, lanes } = params + let plannedTracks = params.tracks + const assignments = new Map() + const candidates = getPasteDestinationTrackIds(plannedTracks, kind) + const activeTrack = plannedTracks.find( + (track) => track.id === activeTrackId && isCompatiblePasteTrack(track, kind), + ) + + for (const lane of lanes) { + let targetTrackId = + activeTrack?.id ?? findExactPasteTrack(lane, plannedTracks)?.id ?? candidates[0] + if (!targetTrackId) { + const created = appendPasteDestinationTrack(plannedTracks, kind) + plannedTracks = created.tracks + targetTrackId = created.trackId + candidates.push(created.trackId) + } + assignments.set(lane.key, targetTrackId) + } + + return { assignments, tracks: plannedTracks } +} + +function planPreservedSourceSection(params: { + kind: PasteTrackKind + lanes: PasteSourceLane[] + tracks: TimelineTrack[] +}): { assignments: Map; tracks: TimelineTrack[] } { + const { kind, lanes } = params + let plannedTracks = params.tracks + const assignments = new Map() + const candidates = getPasteDestinationTrackIds(plannedTracks, kind) + const usedTrackIds = new Set() + + for (const lane of lanes) { + const exactTrack = findExactPasteTrack(lane, plannedTracks) + if (!exactTrack) continue + assignments.set(lane.key, exactTrack.id) + usedTrackIds.add(exactTrack.id) + } + + for (const lane of lanes) { + if (assignments.has(lane.key)) continue + const ordinalCandidate = candidates[lane.ordinal] + const availableCandidate = + ordinalCandidate && !usedTrackIds.has(ordinalCandidate) + ? ordinalCandidate + : candidates.find((candidate) => !usedTrackIds.has(candidate)) + let targetTrackId = availableCandidate + if (!targetTrackId) { + const created = appendPasteDestinationTrack(plannedTracks, kind) + plannedTracks = created.tracks + targetTrackId = created.trackId + candidates.push(created.trackId) + } + assignments.set(lane.key, targetTrackId) + usedTrackIds.add(targetTrackId) + } + + return { assignments, tracks: plannedTracks } +} + +function buildPasteTrackPlan( + pasteItems: Array>, + tracks: TimelineTrack[], + activeTrackId: string | null, +): { plan: Map; tracks: TimelineTrack[] } { + let plannedTracks = tracks + const { lanesByKind, laneKeyByItemIndex } = collectPasteSourceLanes(pasteItems) + const preserveSourceTracks = new Set(pasteItems.map((item) => item.trackId)).size > 1 + const plan = new Map() + const assignedTrackBySourceLane = new Map() + + for (const kind of ['video', 'audio'] as const) { + const sectionPlan = preserveSourceTracks + ? planPreservedSourceSection({ kind, lanes: lanesByKind[kind], tracks: plannedTracks }) + : planSingleSourceSection({ + activeTrackId, + kind, + lanes: lanesByKind[kind], + tracks: plannedTracks, + }) + plannedTracks = sectionPlan.tracks + for (const [laneKey, trackId] of sectionPlan.assignments) { + assignedTrackBySourceLane.set(laneKey, trackId) + } + } + + for (const [sourceIndex, laneKey] of laneKeyByItemIndex) { + const targetTrackId = assignedTrackBySourceLane.get(laneKey) + if (targetTrackId) plan.set(sourceIndex, targetTrackId) + } + + return { plan, tracks: plannedTracks } +} + +function findSharedPlacementShift( + plans: PastePlacementPlan[], + occupiedItems: TimelineItem[], +): number { + let shift = 0 + while (true) { + let requiredShift = 0 + for (const plan of plans) { + const from = plan.desiredFrom + shift + for (const occupied of occupiedItems) { + if ( + placementsOverlap( + { + trackId: plan.targetTrackId, + from, + durationInFrames: plan.itemData.durationInFrames, + }, + occupied, + ) + ) { + requiredShift = Math.max(requiredShift, occupied.from + occupied.durationInFrames - from) + } + } + } + if (requiredShift <= 0) return shift + shift += requiredShift + } +} function revealPastedItems(itemIds: readonly string[]): void { if (itemIds.length === 0) { @@ -75,6 +317,7 @@ export function useClipboardShortcuts() { const transitions = useTimelineStore((s) => s.transitions) const tracks = useTimelineStore((s) => s.tracks) const addItems = useTimelineStore((s) => s.addItems) + const addItemsOnNewTracks = useTimelineStore((s) => s.addItemsOnNewTracks) const removeItems = useTimelineStore((s) => s.removeItems) const updateTransition = useTimelineStore((s) => s.updateTransition) const copyTransition = useClipboardStore((s) => s.copyTransition) @@ -200,89 +443,84 @@ export function useClipboardShortcuts() { ) if (pasteItems.length === 0) return - // When the clipboard spans more than one source track (e.g. a linked - // video+audio pair copied from the transcript), preserve each item's own - // track so the pair lands on video/audio tracks separately. A - // single-track copy still pastes onto the active track as before. - const preserveSourceTracks = new Set(pasteItems.map((item) => item.trackId)).size > 1 - - const findNextAvailableSpace = ( - trackId: string, - startFrame: number, - duration: number, - ): number => { - const trackItems = storeItems - .filter((item) => item.trackId === trackId) - .sort((a, b) => a.from - b.from) - - let candidateFrame = startFrame - - for (const item of trackItems) { - const itemEnd = item.from + item.durationInFrames - if (candidateFrame < itemEnd && candidateFrame + duration > item.from) { - candidateFrame = itemEnd - } - } - - return candidateFrame - } + // Resolve every source lane by media section/kind. Exact IDs win when + // they survive in this sequence; otherwise the source lane ordinal is + // mapped to the corresponding destination lane. This keeps linked A/V + // members in separate sections even when both source IDs are absent. + const { plan: trackPlan, tracks: plannedTracks } = buildPasteTrackPlan( + pasteItems, + tracks, + activeTrackId, + ) + const placementPlans = pasteItems.flatMap((itemData, sourceIndex) => { + const targetTrackId = trackPlan.get(sourceIndex) + return targetTrackId + ? [ + { + itemData, + targetTrackId, + desiredFrom: currentFrame + itemData.from, + sourceIndex, + }, + ] + : [] + }) - const hasSpaceAt = (trackId: string, startFrame: number, duration: number): boolean => { - const trackItems = storeItems.filter((item) => item.trackId === trackId) - for (const item of trackItems) { - const itemEnd = item.from + item.durationInFrames - if (startFrame < itemEnd && startFrame + duration > item.from) { - return false - } + // Keep an ordinary multi-item paste as one rigid block. If invalid or + // missing source tracks collapse overlapping items onto one target, + // fall back to linked groups/singletons so placement can still make + // progress without separating a valid linked A/V pair. + let placementGroups: PastePlacementPlan[][] = [placementPlans] + if (hasInternalPlacementOverlap(placementPlans)) { + const grouped = new Map() + for (const plan of placementPlans) { + const key = plan.itemData.linkedGroupId + ? `linked:${plan.itemData.linkedGroupId}` + : `item:${plan.sourceIndex}` + const group = grouped.get(key) ?? [] + group.push(plan) + grouped.set(key, group) } - return true + placementGroups = [...grouped.values()].flatMap((group) => + hasInternalPlacementOverlap(group) ? group.map((plan) => [plan]) : [group], + ) } - for (const itemData of pasteItems) { - const newId = crypto.randomUUID() - newItemIds.push(newId) + const occupiedItems = [...storeItems] + for (const group of placementGroups) { + const sharedShift = findSharedPlacementShift(group, occupiedItems) + for (const plan of group) { + const { itemData, targetTrackId, desiredFrom } = plan + const newId = crypto.randomUUID() + newItemIds.push(newId) + const newItem = { + ...itemData, + id: newId, + from: desiredFrom + sharedShift, + trackId: targetTrackId, + originId: newId, + linkedGroupId: itemData.linkedGroupId + ? (linkedGroupMap.get(itemData.linkedGroupId) ?? + linkedGroupMap + .set(itemData.linkedGroupId, crypto.randomUUID()) + .get(itemData.linkedGroupId)) + : undefined, + } as TimelineItem - let targetTrackId = preserveSourceTracks ? itemData.trackId : activeTrackId - if (!targetTrackId || !tracks.some((t) => t.id === targetTrackId)) { - targetTrackId = itemData.trackId - } - const trackExists = tracks.some((t) => t.id === targetTrackId) - if (!trackExists && tracks.length > 0) { - targetTrackId = tracks[0]!.id + newItems.push(newItem) + occupiedItems.push(newItem) + usedTrackIds.add(targetTrackId) } - - const desiredFrom = currentFrame - const duration = itemData.durationInFrames - - let newFrom: number - if (hasSpaceAt(targetTrackId, desiredFrom, duration)) { - newFrom = desiredFrom - } else { - newFrom = findNextAvailableSpace(targetTrackId, desiredFrom, duration) - } - - const newItem = { - ...itemData, - id: newId, - from: newFrom, - trackId: targetTrackId, - originId: newId, - linkedGroupId: itemData.linkedGroupId - ? (linkedGroupMap.get(itemData.linkedGroupId) ?? - linkedGroupMap - .set(itemData.linkedGroupId, crypto.randomUUID()) - .get(itemData.linkedGroupId)) - : undefined, - } - - newItems.push(newItem as TimelineItem) - usedTrackIds.add(targetTrackId) } // Add every pasted item in a single ADD_ITEMS command so one Ctrl+Z // undoes the whole paste (including a linked A/V pair), not item-by-item. if (newItems.length > 0) { - addItems(newItems) + if (plannedTracks.length > tracks.length) { + addItemsOnNewTracks(newItems, plannedTracks) + } else { + addItems(newItems) + } } if (newItemIds.length > 0) { @@ -321,6 +559,7 @@ export function useClipboardShortcuts() { itemsClipboard, tracks, addItems, + addItemsOnNewTracks, selectItems, activeTrackId, selectedKeyframes.length, diff --git a/src/features/timeline/hooks/shortcuts/use-playback-shortcuts.test.tsx b/src/features/timeline/hooks/shortcuts/use-playback-shortcuts.test.tsx new file mode 100644 index 000000000..87abbe379 --- /dev/null +++ b/src/features/timeline/hooks/shortcuts/use-playback-shortcuts.test.tsx @@ -0,0 +1,160 @@ +import { act, render } from '@testing-library/react' +import { beforeEach, describe, expect, it, vi } from 'vite-plus/test' +import { HOTKEYS } from '@/config/hotkeys' +import type { SourcePlayerMethods } from '@/shared/state/source-player/types' +import type { VideoItem } from '@/types/timeline' +import { usePlaybackShortcuts } from './use-playback-shortcuts' + +const { itemsState, playbackState, sourcePlayerState, useHotkeysMock } = vi.hoisted(() => { + const playbackState = { + currentFrame: 0, + isPlaying: false, + togglePlayPause: vi.fn(), + shuttleForward: vi.fn(), + shuttleReverse: vi.fn(), + pause: vi.fn(), + setCurrentFrame: vi.fn((frame: number) => { + playbackState.currentFrame = frame + }), + setPreviewFrame: vi.fn(), + } + + return { + itemsState: { items: [] as Array<{ from: number; durationInFrames: number }> }, + playbackState, + sourcePlayerState: { + hoveredPanel: null as 'source' | null, + playerMethods: null as SourcePlayerMethods | null, + }, + useHotkeysMock: vi.fn(), + } +}) + +vi.mock('react-hotkeys-hook', () => ({ + useHotkeys: useHotkeysMock, +})) + +vi.mock('@/features/timeline/deps/settings', () => ({ + useResolvedHotkeys: () => ({ + PLAY_PAUSE: 'space', + PREVIOUS_FRAME: 'left', + NEXT_FRAME: 'right', + GO_TO_START: 'home', + GO_TO_END: 'end', + NEXT_SNAP_POINT: 'down', + PREVIOUS_SNAP_POINT: 'up', + }), +})) + +vi.mock('@/shared/state/playback', () => ({ + usePlaybackStore: Object.assign( + (selector: (state: typeof playbackState) => unknown) => selector(playbackState), + { getState: () => playbackState }, + ), +})) + +vi.mock('@/shared/state/preview-bridge', () => ({ + usePreviewBridgeStore: (selector: (state: { setDisplayedFrame: () => void }) => unknown) => + selector({ setDisplayedFrame: vi.fn() }), +})) + +vi.mock('@/shared/state/source-player', () => ({ + useSourcePlayerStore: { + getState: () => sourcePlayerState, + }, +})) + +vi.mock('../../stores/items-store', () => ({ + useItemsStore: { + getState: () => itemsState, + }, +})) + +type HotkeyCallback = (event: { preventDefault: () => void }) => void + +function makeVideoItem(overrides: Partial = {}): VideoItem { + return { + id: 'clip-1', + type: 'video', + trackId: 'track-1', + from: 10, + durationInFrames: 5, + label: 'Clip', + src: 'clip.mp4', + ...overrides, + } +} + +function ShortcutHarness() { + usePlaybackShortcuts({}) + return null +} + +function getHotkeyCallback(binding: string): HotkeyCallback { + const registration = useHotkeysMock.mock.calls.find(([keys]) => keys === binding) + expect(registration).toBeDefined() + return registration?.[1] as HotkeyCallback +} + +function trigger(callback: HotkeyCallback) { + act(() => callback({ preventDefault: vi.fn() })) +} + +describe('usePlaybackShortcuts frame boundaries', () => { + beforeEach(() => { + vi.clearAllMocks() + playbackState.currentFrame = 0 + playbackState.isPlaying = false + itemsState.items = [ + makeVideoItem(), + makeVideoItem({ id: 'clip-2', from: 0, durationInFrames: 7 }), + ] + sourcePlayerState.hoveredPanel = null + sourcePlayerState.playerMethods = null + }) + + it('clamps timeline ArrowRight to the final valid frame', () => { + playbackState.currentFrame = 13 + render() + + const nextFrame = getHotkeyCallback(HOTKEYS.NEXT_FRAME) + trigger(nextFrame) + expect(playbackState.currentFrame).toBe(14) + + trigger(nextFrame) + expect(playbackState.currentFrame).toBe(14) + }) + + it('seeks timeline End to the maximum inclusive item frame, or zero when empty', () => { + render() + + const goToEnd = getHotkeyCallback(HOTKEYS.GO_TO_END) + trigger(goToEnd) + expect(playbackState.currentFrame).toBe(14) + + itemsState.items = [] + trigger(goToEnd) + expect(playbackState.currentFrame).toBe(0) + }) + + it('clamps source-player End to a nonnegative frame', () => { + const playerMethods: SourcePlayerMethods = { + toggle: vi.fn(), + pause: vi.fn(), + isPlaying: vi.fn(() => false), + shuttleForward: vi.fn(), + shuttleReverse: vi.fn(), + seek: vi.fn(), + frameBack: vi.fn(), + frameForward: vi.fn(), + getDurationInFrames: vi.fn(() => 0), + } + sourcePlayerState.hoveredPanel = 'source' + sourcePlayerState.playerMethods = playerMethods + render() + + trigger(getHotkeyCallback(HOTKEYS.GO_TO_END)) + + expect(playerMethods.seek).toHaveBeenCalledWith(0) + }) +}) diff --git a/src/features/timeline/hooks/shortcuts/use-playback-shortcuts.ts b/src/features/timeline/hooks/shortcuts/use-playback-shortcuts.ts index aaa25969f..5dbccd54b 100644 --- a/src/features/timeline/hooks/shortcuts/use-playback-shortcuts.ts +++ b/src/features/timeline/hooks/shortcuts/use-playback-shortcuts.ts @@ -34,6 +34,12 @@ function getSnapPoints(): number[] { return Array.from(points).sort((a, b) => a - b) } +function getFinalTimelineFrame(): number { + return useItemsStore + .getState() + .items.reduce((maxFrame, item) => Math.max(maxFrame, item.from + item.durationInFrames - 1), 0) +} + export function usePlaybackShortcuts(callbacks: TimelineShortcutCallbacks) { const hotkeys = useResolvedHotkeys() const togglePlayPause = usePlaybackStore((s) => s.togglePlayPause) @@ -170,7 +176,7 @@ export function usePlaybackShortcuts(callbacks: TimelineShortcutCallbacks) { return } const currentFrame = usePlaybackStore.getState().currentFrame - commitTimelineSeek(currentFrame + 1) + commitTimelineSeek(Math.min(currentFrame + 1, getFinalTimelineFrame())) }, HOTKEY_OPTIONS, [commitTimelineSeek], @@ -199,15 +205,10 @@ export function usePlaybackShortcuts(callbacks: TimelineShortcutCallbacks) { event.preventDefault() const { hoveredPanel, playerMethods } = useSourcePlayerStore.getState() if (hoveredPanel === 'source' && playerMethods) { - playerMethods.seek(playerMethods.getDurationInFrames() - 1) + playerMethods.seek(Math.max(0, playerMethods.getDurationInFrames() - 1)) return } - const currentItems = useItemsStore.getState().items - const lastFrame = currentItems.reduce((max, item) => { - const itemEnd = item.from + item.durationInFrames - return Math.max(max, itemEnd) - }, 0) - commitTimelineSeek(lastFrame) + commitTimelineSeek(getFinalTimelineFrame()) }, HOTKEY_OPTIONS, [commitTimelineSeek], diff --git a/src/features/timeline/stores/actions/export-snapshot.ts b/src/features/timeline/stores/actions/export-snapshot.ts index e49cf22c8..fd1e2e074 100644 --- a/src/features/timeline/stores/actions/export-snapshot.ts +++ b/src/features/timeline/stores/actions/export-snapshot.ts @@ -52,6 +52,10 @@ function furthestItemEnd(items: TimelineItem[]): number { return Math.max(...items.map((item) => item.from + item.durationInFrames)) } +function cloneAudioEq(busAudioEq: AudioEqSettings | undefined): AudioEqSettings | undefined { + return busAudioEq ? { ...busAudioEq } : undefined +} + /** The active top-level tab (null = Main) — the picker's default selection. */ export function getActiveExportSequenceId(): string | null { return getActiveTabId(useCompositionNavigationStore.getState().breadcrumbs) @@ -87,7 +91,6 @@ export function getExportableSequence(sequenceId: string | null): ExportableSequ const activeTabId = getActiveTabId(nav.breadcrumbs) const playback = usePlaybackStore.getState() const markersState = useMarkersStore.getState() - const isActiveTab = sequenceId === activeTabId // The markers store holds the range of whatever timeline is *loaded* — the // deepest drill level, which is not the tab root once you drill into a comp. // Keying this on the tab id reported a drilled-into comp's range as Main's, @@ -124,8 +127,16 @@ export function getExportableSequence(sequenceId: string | null): ExportableSequ if (sequenceId === null) { const root = getRootTimelineSnapshot(current) const metadata = useProjectStore.getState().currentProject?.metadata - // Main's audio bus / range are live when Main is active, else held aside. - const busAudioEq = activeTabId === null ? playback.busAudioEq : nav.mainHolder?.busAudioEq + // Main owns the live mixer/range only when Main itself is the loaded + // composition. While drilling from Main, its complete snapshot is the + // null-composition root stash; while another tab is active it is held in + // mainHolder. The active tab alone cannot distinguish Main from a drilled + // child because both retain a null root breadcrumb. + const heldRoot = + activeTabId === null + ? nav.stashStack.find((stash) => stash.compositionId === null) + : nav.mainHolder + const busAudioEq = nav.activeCompositionId === null ? playback.busAudioEq : heldRoot?.busAudioEq return { id: null, name: MAIN_LABEL, @@ -137,10 +148,10 @@ export function getExportableSequence(sequenceId: string | null): ExportableSequ width: metadata?.width ?? DEFAULT_PROJECT_WIDTH, height: metadata?.height ?? DEFAULT_PROJECT_HEIGHT, backgroundColor: metadata?.backgroundColor, - busAudioEq, + busAudioEq: cloneAudioEq(busAudioEq), masterBusDb: playback.masterBusDb, durationFrames: furthestItemEnd(root.items), - ...range(nav.mainHolder), + ...range(heldRoot), } } @@ -160,9 +171,12 @@ export function getExportableSequence(sequenceId: string | null): ExportableSequ width: comp.width, height: comp.height, backgroundColor: comp.backgroundColor, - // Live mixer edits live in the playback store for the active sequence; the - // registry entry is only up to date once we've switched away from it. - busAudioEq: isActiveTab ? playback.busAudioEq : comp.busAudioEq, + // Live mixer edits belong to the deepest composition being edited, not the + // top-level tab. A drilled child therefore owns playback.busAudioEq while + // its tab root must continue using the registry snapshot. + busAudioEq: cloneAudioEq( + sequenceId === nav.activeCompositionId ? playback.busAudioEq : comp.busAudioEq, + ), masterBusDb: playback.masterBusDb, durationFrames: comp.durationInFrames || furthestItemEnd(comp.items), ...range(comp), diff --git a/src/features/timeline/stores/export-snapshot.test.ts b/src/features/timeline/stores/export-snapshot.test.ts index c2479e148..036e3693d 100644 --- a/src/features/timeline/stores/export-snapshot.test.ts +++ b/src/features/timeline/stores/export-snapshot.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it } from 'vite-plus/test' +import type { CompositionItem } from '@/types/timeline' import { makeTimelineTrack as makeTrack, makeTimelineVideoItem as makeVideoItem, @@ -9,6 +10,8 @@ import { useItemsStore } from './items-store' import { useCompositionsStore } from './compositions-store' import { useSequencesStore } from './sequences-store' import { useCompositionNavigationStore } from './composition-navigation-store' +import { useMarkersStore } from './markers-store' +import { usePlaybackStore } from '@/shared/state/playback' import { getActiveExportSequenceId, getExportableSequence, @@ -31,6 +34,84 @@ function seedSequence(id: string, itemId: string, width = 1280, height = 720): v useSequencesStore.getState().addTopLevelSequence(id) } +function seedNestedSequence(): void { + seedSequence('seq-a', 'a-clip') + useCompositionsStore.getState().addComposition({ + id: 'child', + name: 'child', + tracks: [makeTrack({ id: 'child-v1', name: 'V1', kind: 'video', order: 0 })], + items: [makeVideoItem({ id: 'child-clip', trackId: 'child-v1', durationInFrames: 30 })], + transitions: [], + keyframes: [], + fps: 24, + width: 640, + height: 360, + durationInFrames: 30, + busAudioEq: { enabled: true, lowGainDb: 2 }, + }) + useCompositionsStore.getState().updateComposition('seq-a', { + items: [ + { + ...makeVideoItem({ id: 'child-entry', trackId: 'seq-a-v1', durationInFrames: 30 }), + type: 'composition', + compositionId: 'child', + compositionWidth: 640, + compositionHeight: 360, + } as unknown as CompositionItem, + ], + busAudioEq: { enabled: true, lowGainDb: 4 }, + }) +} + +function makeCompositionEntry( + id: string, + compositionId: string, + trackId: string, + durationInFrames: number, +): CompositionItem { + return { + ...makeVideoItem({ id, trackId, durationInFrames }), + type: 'composition', + compositionId, + compositionWidth: 640, + compositionHeight: 360, + } as unknown as CompositionItem +} + +function seedMainChildGrandchild(): void { + useCompositionsStore.getState().addComposition({ + id: 'grandchild', + name: 'grandchild', + tracks: [makeTrack({ id: 'grandchild-v1', name: 'V1', kind: 'video', order: 0 })], + items: [ + makeVideoItem({ + id: 'grandchild-clip', + trackId: 'grandchild-v1', + durationInFrames: 20, + }), + ], + transitions: [], + keyframes: [], + fps: 24, + width: 640, + height: 360, + durationInFrames: 20, + }) + useCompositionsStore.getState().addComposition({ + id: 'child', + name: 'child', + tracks: [makeTrack({ id: 'child-v1', name: 'V1', kind: 'video', order: 0 })], + items: [makeCompositionEntry('grandchild-entry', 'grandchild', 'child-v1', 20)], + transitions: [], + keyframes: [], + fps: 24, + width: 640, + height: 360, + durationInFrames: 20, + }) + useItemsStore.getState().setItems([makeCompositionEntry('child-entry', 'child', 'track-v1', 20)]) +} + describe('export-snapshot sourcing', () => { beforeEach(() => { resetTimelineCompositionTestState() @@ -111,4 +192,62 @@ describe('export-snapshot sourcing', () => { const seq = getExportableSequence('seq-a') expect(seq.items.map((i) => i.id)).toEqual(['a-clip']) }) + + it('binds a drilled child export to the live child mixer without contaminating Main or its tab root', () => { + const mainEq = { enabled: true, lowGainDb: 1 } + const sequenceEq = { enabled: true, lowGainDb: 4 } + const childEq = { enabled: true, lowGainDb: 9 } + seedNestedSequence() + usePlaybackStore.getState().setBusAudioEq(mainEq) + useCompositionsStore.getState().updateComposition('seq-a', { busAudioEq: sequenceEq }) + useCompositionNavigationStore.getState().switchToSequence('seq-a') + useCompositionNavigationStore.getState().enterComposition('child', 'child', 'child-entry') + usePlaybackStore.getState().setBusAudioEq(childEq) + + const child = getExportableSequence('child') + const sequence = getExportableSequence('seq-a') + const main = getExportableSequence(null) + + expect(child.busAudioEq).toEqual(childEq) + expect(sequence.busAudioEq).toEqual(sequenceEq) + expect(main.busAudioEq).toEqual(mainEq) + + // Returned EQ data is a snapshot: later live edits do not rewrite prior exports. + usePlaybackStore.getState().setBusAudioEq({ enabled: true, lowGainDb: 12 }) + expect(child.busAudioEq).toEqual(childEq) + expect(sequence.busAudioEq).toEqual(sequenceEq) + expect(main.busAudioEq).toEqual(mainEq) + }) + + it('keeps Main, child, and grandchild EQ and ranges owned by their actual composition', () => { + const mainEq = { enabled: true, lowGainDb: 1 } + const childEq = { enabled: true, lowGainDb: 5 } + const grandchildEq = { enabled: true, lowGainDb: 9 } + seedMainChildGrandchild() + + usePlaybackStore.getState().setBusAudioEq(mainEq) + useMarkersStore.getState().setInOutPoints(2, 18) + useCompositionNavigationStore.getState().enterComposition('child', 'child', 'child-entry') + + usePlaybackStore.getState().setBusAudioEq(childEq) + useMarkersStore.getState().setInOutPoints(3, 15) + useCompositionNavigationStore + .getState() + .enterComposition('grandchild', 'grandchild', 'grandchild-entry') + + usePlaybackStore.getState().setBusAudioEq(grandchildEq) + useMarkersStore.getState().setInOutPoints(4, 12) + + const main = getExportableSequence(null) + const child = getExportableSequence('child') + const grandchild = getExportableSequence('grandchild') + + expect(main).toMatchObject({ busAudioEq: mainEq, inPoint: 2, outPoint: 18 }) + expect(child).toMatchObject({ busAudioEq: childEq, inPoint: 3, outPoint: 15 }) + expect(grandchild).toMatchObject({ + busAudioEq: grandchildEq, + inPoint: 4, + outPoint: 12, + }) + }) }) diff --git a/src/features/timeline/stores/timeline-persistence.ts b/src/features/timeline/stores/timeline-persistence.ts index b94717301..c0313cbf4 100644 --- a/src/features/timeline/stores/timeline-persistence.ts +++ b/src/features/timeline/stores/timeline-persistence.ts @@ -38,10 +38,7 @@ import { useMarkersStore } from './markers-store' import { useTimelineSettingsStore } from './timeline-settings-store' import { ROOT_HISTORY_CONTEXT, useTimelineCommandStore } from './timeline-command-store' import { useCompositionsStore, type SubComposition } from './compositions-store' -import { - getActiveTabId, - useCompositionNavigationStore, -} from './composition-navigation-store' +import { getActiveTabId, useCompositionNavigationStore } from './composition-navigation-store' import { useSequencesStore } from './sequences-store' import { getProject, updateProject, saveProjectThumbnail } from '@/infrastructure/storage' import { @@ -727,6 +724,10 @@ interface TimelinePersistenceSnapshot { isRootTimelineLive: boolean } +function cloneAudioEq(busAudioEq: AudioEqSettings | undefined): AudioEqSettings | undefined { + return busAudioEq ? { ...busAudioEq } : undefined +} + /** * Capture a Main-rooted project snapshot without navigating the live editor. * @@ -773,7 +774,7 @@ function captureTimelinePersistenceSnapshot(): TimelinePersistenceSnapshot { return { ...composition, durationInFrames, - busAudioEq: playback.busAudioEq, + busAudioEq: cloneAudioEq(playback.busAudioEq), markers: markers.markers, inPoint: markers.inPoint, outPoint: markers.outPoint, @@ -785,9 +786,8 @@ function captureTimelinePersistenceSnapshot(): TimelinePersistenceSnapshot { compositions, currentFrame: heldRoot ? heldRoot.currentFrame : playback.currentFrame, zoomLevel: heldRoot?.zoomLevel ?? rootView?.zoomLevel ?? zoom.level, - scrollPosition: - heldRoot?.scrollPosition ?? rootView?.scrollPosition ?? settings.scrollPosition, - busAudioEq: heldRoot ? heldRoot.busAudioEq : playback.busAudioEq, + scrollPosition: heldRoot?.scrollPosition ?? rootView?.scrollPosition ?? settings.scrollPosition, + busAudioEq: cloneAudioEq(heldRoot ? heldRoot.busAudioEq : playback.busAudioEq), masterBusDb: playback.masterBusDb, markers: heldRoot ? heldRoot.markers : markers.markers, inPoint: heldRoot ? heldRoot.inPoint : markers.inPoint, @@ -1230,10 +1230,7 @@ function getTimelineLoadKey(projectId: string, options: LoadTimelineOptions): st return JSON.stringify([projectId, options.allowProjectUpgrade === true]) } -export function loadTimeline( - projectId: string, - options: LoadTimelineOptions = {}, -): Promise { +export function loadTimeline(projectId: string, options: LoadTimelineOptions = {}): Promise { const loadKey = getTimelineLoadKey(projectId, options) const inFlightLoad = inFlightTimelineLoads.get(loadKey) if (inFlightLoad) return inFlightLoad diff --git a/src/features/timeline/stores/timeline-store-facade.ts b/src/features/timeline/stores/timeline-store-facade.ts index c3cab19b8..4379c0459 100644 --- a/src/features/timeline/stores/timeline-store-facade.ts +++ b/src/features/timeline/stores/timeline-store-facade.ts @@ -113,6 +113,7 @@ function getSnapshot(): TimelineState & TimelineActions { addItems: timelineActions.addItems, addItemWithLinkedAudio: timelineActions.addItemWithLinkedAudio, addItemOnNewTrack: timelineActions.addItemOnNewTrack, + addItemsOnNewTracks: timelineActions.addItemsOnNewTracks, updateItem: timelineActions.updateItem, removeItems: timelineActions.removeItems, rippleDeleteItems: timelineActions.rippleDeleteItems, diff --git a/src/features/timeline/types.ts b/src/features/timeline/types.ts index cec02666f..e4cc36f90 100644 --- a/src/features/timeline/types.ts +++ b/src/features/timeline/types.ts @@ -68,6 +68,7 @@ export interface TimelineActions { addItems: (items: TimelineItem[]) => void addItemWithLinkedAudio: (video: VideoItem) => void addItemOnNewTrack: (item: TimelineItem, tracks: TimelineTrack[]) => void + addItemsOnNewTracks: (items: TimelineItem[], tracks: TimelineTrack[]) => void updateItem: (id: string, updates: Partial) => void removeItems: (ids: string[]) => void rippleDeleteItems: (ids: string[]) => void diff --git a/src/infrastructure/browser/blob-url-manager.test.ts b/src/infrastructure/browser/blob-url-manager.test.ts index 17ac2c918..20d5ecdd7 100644 --- a/src/infrastructure/browser/blob-url-manager.test.ts +++ b/src/infrastructure/browser/blob-url-manager.test.ts @@ -96,8 +96,12 @@ describe('BlobUrlManager', () => { }) it('is a no-op for unknown mediaId', () => { + const epoch = blobUrlManager.getEpoch('unknown') + const version = blobUrlManager.getSnapshot() blobUrlManager.invalidate('unknown') expect(blobUrlManager.size).toBe(0) + expect(blobUrlManager.getEpoch('unknown')).not.toBe(epoch) + expect(blobUrlManager.getSnapshot()).toBeGreaterThan(version) }) it('allows re-acquiring after invalidation', () => { @@ -153,5 +157,15 @@ describe('BlobUrlManager', () => { expect(revokedUrls.has(url1)).toBe(true) expect(revokedUrls.has(url2)).toBe(true) }) + + it('retires pending generations for every media id', () => { + const firstEpoch = blobUrlManager.getEpoch('media-1') + const secondEpoch = blobUrlManager.getEpoch('media-2') + + blobUrlManager.releaseAll() + + expect(blobUrlManager.getEpoch('media-1')).not.toBe(firstEpoch) + expect(blobUrlManager.getEpoch('media-2')).not.toBe(secondEpoch) + }) }) }) diff --git a/src/infrastructure/browser/blob-url-manager.ts b/src/infrastructure/browser/blob-url-manager.ts index cc0db975b..d3271c3a5 100644 --- a/src/infrastructure/browser/blob-url-manager.ts +++ b/src/infrastructure/browser/blob-url-manager.ts @@ -29,6 +29,8 @@ interface BlobUrlEntry { class BlobUrlManager { private entries = new Map() private version = 0 + private invalidationEpoch = 0 + private mediaInvalidationEpochs = new Map() private listeners = new Set<() => void>() /** Notify React subscribers that blob URLs have changed */ @@ -106,6 +108,19 @@ class BlobUrlManager { return this.entries.has(mediaId) } + /** + * Token identifying the currently valid source generation for one media id. + * Resolvers capture this before async storage reads and must discard a + * completion when invalidation advances either component. + */ + getEpoch(mediaId: string): string { + return `${this.invalidationEpoch}:${this.mediaInvalidationEpochs.get(mediaId) ?? 0}` + } + + private advanceMediaEpoch(mediaId: string): void { + this.mediaInvalidationEpochs.set(mediaId, (this.mediaInvalidationEpochs.get(mediaId) ?? 0) + 1) + } + /** * Reverse-lookup: find the mediaId that owns a given blob URL. * Returns null if the URL is not tracked. @@ -122,10 +137,12 @@ class BlobUrlManager { * Used when the underlying media file has changed (e.g., after relinking). */ invalidate(mediaId: string): void { + this.advanceMediaEpoch(mediaId) const entry = this.entries.get(mediaId) - if (!entry) return - this.revokeEntry(entry) - this.entries.delete(mediaId) + if (entry) { + this.revokeEntry(entry) + this.entries.delete(mediaId) + } this.notify() } @@ -159,6 +176,8 @@ class BlobUrlManager { * Consumers will re-acquire fresh URLs on next resolve. */ invalidateAll(): void { + this.invalidationEpoch++ + this.mediaInvalidationEpochs.clear() for (const entry of this.entries.values()) { this.revokeEntry(entry) } @@ -170,6 +189,8 @@ class BlobUrlManager { * Release all blob URLs (e.g., on project cleanup). */ releaseAll(): void { + this.invalidationEpoch++ + this.mediaInvalidationEpochs.clear() for (const [mediaId, entry] of this.entries) { this.revokeEntry(entry) logger.debug(`Revoked blob URL for media ${mediaId}`) @@ -196,3 +217,12 @@ export const blobUrlManager = new BlobUrlManager() export function useBlobUrlVersion(): number { return useSyncExternalStore(blobUrlManager.subscribe, blobUrlManager.getSnapshot) } + +/** + * Subscribe to the source generation owned by one media id. Unlike the global + * version, this stays stable when another media item acquires or releases a + * URL, so source-local canvases can retain their decoded frame state. + */ +export function useBlobUrlEpoch(mediaId: string): string { + return useSyncExternalStore(blobUrlManager.subscribe, () => blobUrlManager.getEpoch(mediaId)) +} diff --git a/src/infrastructure/gpu-shapes/shape-render-pipeline.ts b/src/infrastructure/gpu-shapes/shape-render-pipeline.ts index fa35c9529..b63a4d38a 100644 --- a/src/infrastructure/gpu-shapes/shape-render-pipeline.ts +++ b/src/infrastructure/gpu-shapes/shape-render-pipeline.ts @@ -205,11 +205,11 @@ fn fragmentMain(input: VertexOutput) -> @location(0) vec4f { outlineProgress = fract(outlineProgress - u.trimParams.z + 1.0); let trimStart = u.trimParams.x; let trimEnd = u.trimParams.y; - strokeVisible = select( - outlineProgress >= trimStart || outlineProgress < trimEnd, - outlineProgress >= trimStart && outlineProgress < trimEnd, - trimEnd >= trimStart, - ); + if (trimEnd >= trimStart) { + strokeVisible = outlineProgress >= trimStart && outlineProgress < trimEnd; + } else { + strokeVisible = outlineProgress >= trimStart || outlineProgress < trimEnd; + } let visibleLength = select(1.0 - trimStart + trimEnd, trimEnd - trimStart, trimEnd >= trimStart); taperProgress = clamp(fract(outlineProgress - trimStart + 1.0) / max(visibleLength, 0.001), 0.0, 1.0); } diff --git a/src/runtime/player/clock/Clock.test.ts b/src/runtime/player/clock/Clock.test.ts index da4435226..860eabf0f 100644 --- a/src/runtime/player/clock/Clock.test.ts +++ b/src/runtime/player/clock/Clock.test.ts @@ -186,4 +186,44 @@ describe('Clock playback timing', () => { clock.dispose() }) + + it('crosses adjacent clip sources before applying genuine end or loop behavior', () => { + const sources = [ + { from: 1, end: 91, picture: 'red', audioHz: 440 }, + { from: 91, end: 181, picture: 'blue', audioHz: 880 }, + ] + const sourceAt = (frame: number) => + sources.find((source) => frame >= source.from && frame < source.end) ?? null + const clock = new Clock({ + fps: 30, + durationInFrames: 181, + initialFrame: 90, + }) + + expect(sourceAt(clock.currentFrame)).toMatchObject({ picture: 'red', audioHz: 440 }) + clock.play() + runNextAnimationFrame(34) + expect(clock.currentFrame).toBe(91) + expect(sourceAt(clock.currentFrame)).toMatchObject({ picture: 'blue', audioHz: 880 }) + + runNextAnimationFrame(4_000) + expect(clock.currentFrame).toBe(180) + expect(sourceAt(clock.currentFrame)).toMatchObject({ picture: 'blue', audioHz: 880 }) + expect(clock.isPlaying).toBe(false) + clock.dispose() + + nowMs = 0 + const loopingClock = new Clock({ + fps: 30, + durationInFrames: 181, + initialFrame: 180, + loop: true, + }) + loopingClock.play() + expect(loopingClock.currentFrame).toBe(0) + runNextAnimationFrame(34) + expect(loopingClock.currentFrame).toBe(1) + expect(sourceAt(loopingClock.currentFrame)).toMatchObject({ picture: 'red', audioHz: 440 }) + loopingClock.dispose() + }) })