From 503c3cc1161334252cb72de56b73b68dd9ca9480 Mon Sep 17 00:00:00 2001 From: Patrick Lu Date: Fri, 21 Aug 2026 18:56:39 -0700 Subject: [PATCH 1/4] fix(editor): keep host bridge round-trip stable for minimal clip items stableSerialize treated a present-but-undefined key as different from a missing key, and frameItemToNativeComparable unconditionally emitted volume/speed/opacity/transform keys (plus an always-present text style object). Host snapshots that omit those optional fields failed metadataUnchanged after a simple drag or trim, landing in the unsupported-edit branch, and untouched clips leaked into changed[] on multi-clip timelines. - stableSerialize now skips undefined-valued object entries. - frameItemToNativeComparable emits volume/speed/opacity/transform only when set, and the plain-text style key only when non-empty; an opacity-only native transform round-trips as the top-level opacity field the host sent. - commandIdsForChanges compares clips with synthesized source bounds on both sides so host items without sourceStart/sourceEnd do not appear changed after the native bridge fills its defaults. - deriveSupportedHostEdit batches one remove_item command per removed item (bounded by MAX_COMMANDS_PER_OPERATION) instead of only handling exactly one removal. Round-trip tests install host snapshots through the real runtime, perform store move/trim/remove operations, and assert the derived host command batches. --- src/features/editor/host/controller.test.ts | 238 ++++++++++++++++++++ src/features/editor/host/controller.ts | 43 +++- src/features/editor/host/document.ts | 49 +++- 3 files changed, 311 insertions(+), 19 deletions(-) diff --git a/src/features/editor/host/controller.test.ts b/src/features/editor/host/controller.test.ts index 91963a3b2..9c26f4366 100644 --- a/src/features/editor/host/controller.test.ts +++ b/src/features/editor/host/controller.test.ts @@ -437,4 +437,242 @@ describe('embedded FreeCut host controller', () => { }) expect(adapter.capabilities).toEqual({}) }) + + describe('host round-trip stability', () => { + async function flushReconcile(): Promise { + for (let i = 0; i < 10; i += 1) { + await new Promise((resolve) => setTimeout(resolve, 0)) + } + } + + function minimalTwoClipSnapshot(): EmbeddedEditorSnapshot { + const initial = snapshot() + const track = initial.timeline.tracks[0]! + return { + ...initial, + timeline: { + ...initial.timeline, + tracks: [ + { + ...track, + items: [ + // Minimal host clips: no volume/speed/opacity/transform keys, + // and clip-2 omits sourceStart/sourceEnd entirely. + { + type: 'video', + id: 'clip-1', + trackId: 'track-1', + mediaId: 'media-1', + from: 0, + durationInFrames: 60, + sourceStart: 0, + sourceEnd: 60, + }, + { + type: 'video', + id: 'clip-2', + trackId: 'track-1', + mediaId: 'media-1', + from: 60, + durationInFrames: 60, + }, + ], + }, + ], + }, + } + } + + it('derives a move_item command for a store drag of a minimal host clip', async () => { + const initial = minimalTwoClipSnapshot() + const harness = createFakeHost(initial) + const runtime = new EmbeddedEditorHostRuntime(harness.host, initial) + runtime.mountStores() + try { + useTimelineStore.getState().moveItem('clip-1', 30) + await flushReconcile() + + expect(harness.submitEdit).toHaveBeenCalledTimes(1) + const batch = harness.submitEdit.mock.calls[0]![0] as EditCommandBatch + expect(batch.commands).toEqual([ + expect.objectContaining({ type: 'move_item', item_id: 'clip-1' }), + ]) + // The untouched clip must not leak into the derived change set. + expect(batch.commands.some((command) => command.command_id.includes('clip-2'))).toBe( + false, + ) + } finally { + runtime.unmountStores() + } + }) + + it('derives a trim_item command for a store trim of a minimal host clip', async () => { + const initial = minimalTwoClipSnapshot() + const harness = createFakeHost(initial) + const runtime = new EmbeddedEditorHostRuntime(harness.host, initial) + runtime.mountStores() + try { + useTimelineStore.getState().updateItem('clip-1', { durationInFrames: 40, sourceEnd: 40 }) + await flushReconcile() + + expect(harness.submitEdit).toHaveBeenCalledTimes(1) + const batch = harness.submitEdit.mock.calls[0]![0] as EditCommandBatch + expect(batch.commands).toEqual([ + expect.objectContaining({ type: 'trim_item', item_id: 'clip-1', edge: 'end' }), + ]) + } finally { + runtime.unmountStores() + } + }) + + it('derives a move_item command for a host text item with the default color', async () => { + const initial = snapshot() + const track = initial.timeline.tracks[0]! + const withText: EmbeddedEditorSnapshot = { + ...initial, + timeline: { + ...initial.timeline, + tracks: [ + { + ...track, + items: [ + ...track.items, + { + type: 'text', + id: 'text-1', + trackId: 'track-1', + from: 10, + durationInFrames: 20, + text: 'Hello host', + }, + ], + }, + ], + }, + } + const harness = createFakeHost(withText) + const runtime = new EmbeddedEditorHostRuntime(harness.host, withText) + runtime.mountStores() + try { + // The native bridge defaults the text color to #ffffff; the round trip + // must not turn that default into a style mutation. + expect( + useTimelineStore.getState().items.find((item) => item.id === 'text-1'), + ).toMatchObject({ type: 'text', color: '#ffffff' }) + + useTimelineStore.getState().moveItem('text-1', 40) + await flushReconcile() + + expect(harness.submitEdit).toHaveBeenCalledTimes(1) + const batch = harness.submitEdit.mock.calls[0]![0] as EditCommandBatch + expect(batch.commands).toEqual([ + expect.objectContaining({ type: 'move_item', item_id: 'text-1' }), + ]) + } finally { + runtime.unmountStores() + } + }) + + it('keeps a clip carrying top-level opacity movable through the round trip', async () => { + const initial = snapshot() + const track = initial.timeline.tracks[0]! + const clip = track.items[0]! + if (clip.type !== 'video') throw new Error('expected a video clip') + const withOpacity: EmbeddedEditorSnapshot = { + ...initial, + timeline: { + ...initial.timeline, + tracks: [ + { + ...track, + items: [{ ...clip, opacity: 0.5 }], + }, + ], + }, + } + const harness = createFakeHost(withOpacity) + const runtime = new EmbeddedEditorHostRuntime(harness.host, withOpacity) + runtime.mountStores() + try { + useTimelineStore.getState().moveItem('clip-1', 30) + await flushReconcile() + + expect(harness.submitEdit).toHaveBeenCalledTimes(1) + const batch = harness.submitEdit.mock.calls[0]![0] as EditCommandBatch + expect(batch.commands).toEqual([ + expect.objectContaining({ type: 'move_item', item_id: 'clip-1' }), + ]) + } finally { + runtime.unmountStores() + } + }) + + it('batches one remove_item command per removed item', () => { + const initial = minimalTwoClipSnapshot() + const next: EmbeddedEditorSnapshot = { + ...initial, + timeline: { + ...initial.timeline, + tracks: [{ ...initial.timeline.tracks[0]!, items: [] }], + }, + } + + const derived = deriveSupportedHostEdit(initial.timeline, next.timeline, { + operationId: 'operation-remove-2', + idempotencyKey: 'idempotency-remove-2', + }) + + expect(derived.batch?.commands).toEqual([ + expect.objectContaining({ type: 'remove_item', item_id: 'clip-1' }), + expect.objectContaining({ type: 'remove_item', item_id: 'clip-2' }), + ]) + expect(derived.batch?.preconditions).toHaveLength(2) + }) + + it('forwards a multi-select store removal to the host as one batched operation', async () => { + const initial = minimalTwoClipSnapshot() + const harness = createFakeHost(initial) + const runtime = new EmbeddedEditorHostRuntime(harness.host, initial) + runtime.mountStores() + try { + useTimelineStore.getState().removeItems(['clip-1', 'clip-2']) + await flushReconcile() + + expect(harness.submitEdit).toHaveBeenCalledTimes(1) + const batch = harness.submitEdit.mock.calls[0]![0] as EditCommandBatch + expect(batch.commands).toEqual([ + expect.objectContaining({ type: 'remove_item', item_id: 'clip-1' }), + expect.objectContaining({ type: 'remove_item', item_id: 'clip-2' }), + ]) + } finally { + runtime.unmountStores() + } + }) + + it('rejects removals beyond the host per-operation command limit', () => { + const initial = snapshot() + const track = initial.timeline.tracks[0]! + const items = Array.from({ length: 65 }, (_, index) => ({ + type: 'video' as const, + id: `clip-${index}`, + trackId: 'track-1', + mediaId: 'media-1', + from: index * 60, + durationInFrames: 60, + })) + const previous = { + ...initial.timeline, + tracks: [{ ...track, items }], + } + const next = { + ...initial.timeline, + tracks: [{ ...track, items: [] }], + } + + const derived = deriveSupportedHostEdit(previous, next) + + expect(derived.batch).toBeNull() + expect(derived.reason).toMatch(/exceeds the 64-command host operation limit/) + }) + }) }) diff --git a/src/features/editor/host/controller.ts b/src/features/editor/host/controller.ts index e96318ba9..a7cfc3626 100644 --- a/src/features/editor/host/controller.ts +++ b/src/features/editor/host/controller.ts @@ -11,6 +11,7 @@ import { } from './contract' import { createCodePressCommandAdapter, + MAX_COMMANDS_PER_OPERATION, type EditCommand, type EditCommandBatch, type FreeCutFrameDocument, @@ -39,6 +40,10 @@ function stableSerialize(value: unknown): string { if (Array.isArray(value)) return `[${value.map(stableSerialize).join(',')}]` if (value && typeof value === 'object') { return `{${Object.entries(value as Record) + // A present-but-undefined key is equivalent to a missing key: host + // snapshots omit unset optional fields while the native round trip may + // surface them explicitly. + .filter(([, entry]) => entry !== undefined) .sort(([left], [right]) => left.localeCompare(right)) .map(([key, entry]) => `${JSON.stringify(key)}:${stableSerialize(entry)}`) .join(',')}}` @@ -65,6 +70,18 @@ function sourceBounds(item: FrameClip): [number, number] { return [start, item.sourceEnd ?? start + item.durationInFrames] } +/** + * Host snapshots may omit source bounds; the native bridge synthesizes them + * (sourceStart ?? from, sourceEnd ?? sourceStart + duration). Fill the same + * defaults on both sides so an unchanged item compares equal regardless of + * which side carried the explicit keys. + */ +function withSynthesizedSourceBounds(item: FreeCutFrameItem): FreeCutFrameItem { + if (!isFrameClip(item)) return item + const [sourceStart, sourceEnd] = sourceBounds(item) + return { ...item, sourceStart, sourceEnd } +} + function trackIndex(document: FreeCutFrameDocument, trackId: string): number { return Math.max( 0, @@ -197,7 +214,8 @@ function commandIdsForChanges( return ( before !== undefined && after !== undefined && - stableSerialize(before) !== stableSerialize(after) + stableSerialize(withSynthesizedSourceBounds(before)) !== + stableSerialize(withSynthesizedSourceBounds(after)) ) }) return { added, removed, changed } @@ -254,13 +272,24 @@ export function deriveSupportedHostEdit( changed.length === 0 ) { // Track creation is already represented by the add_track commands above. - } else if (removed.length === 1 && added.length === 0 && changed.length === 0) { - const id = removed[0]! - const before = previousItems.get(id)! - if (before.type === 'caption_cue') + } else if (removed.length >= 1 && added.length === 0 && changed.length === 0) { + const removedItems = removed.map((id) => previousItems.get(id)!) + if (removedItems.some((item) => item.type === 'caption_cue')) return { batch: null, reason: 'Caption removal is not supported' } - commands.push({ command_id: `remove-${id}`, type: 'remove_item', item_id: id }) - preconditions.push(preconditionForItem(before, fps)) + if (commands.length + removedItems.length > MAX_COMMANDS_PER_OPERATION) { + return { + batch: null, + reason: `Removing ${removedItems.length} items exceeds the ${MAX_COMMANDS_PER_OPERATION}-command host operation limit`, + } + } + for (const before of removedItems) { + commands.push({ + command_id: `remove-${before.id}`, + type: 'remove_item', + item_id: before.id, + }) + preconditions.push(preconditionForItem(before, fps)) + } } else if (added.length === 1 && removed.length === 0 && changed.length === 0) { const id = added[0]! const after = nextItems.get(id)! diff --git a/src/features/editor/host/document.ts b/src/features/editor/host/document.ts index 4db85eccb..7d0b7211b 100644 --- a/src/features/editor/host/document.ts +++ b/src/features/editor/host/document.ts @@ -47,6 +47,24 @@ function nativeTransformToFrame( } } +/** + * A frame transform whose only non-default value is opacity round-trips + * through the native item as a top-level `opacity` field (see + * nativeItemFromHostItem). Emitting a `transform` key for it would break + * round-trip equivalence with host items that carry opacity only. + */ +function isOpacityOnlyTransform(transform: Record): boolean { + return ( + (transform.x ?? 0) === 0 && + (transform.y ?? 0) === 0 && + (transform.width ?? 0) === 0 && + (transform.height ?? 0) === 0 && + (transform.anchorX ?? 0) === 0 && + (transform.anchorY ?? 0) === 0 && + (transform.rotation ?? 0) === 0 + ) +} + function frameTransformToNative( transform: Record | undefined, ): NonNullable | undefined { @@ -241,6 +259,8 @@ function frameItemToNativeComparable( | NativeTimelineConversionFailure { if (item.type === 'video' || item.type === 'audio' || item.type === 'image') { if (!item.mediaId) return { reason: `Media item "${item.id}" has no mediaId`, itemId: item.id } + const transform = nativeTransformToFrame(item.transform) + const opacityOnly = transform !== undefined && isOpacityOnlyTransform(transform) return { type: item.type, id: item.id, @@ -250,10 +270,12 @@ function frameItemToNativeComparable( durationInFrames: item.durationInFrames, sourceStart: item.sourceStart, sourceEnd: item.sourceEnd, - volume: item.volume, - speed: item.speed, - opacity: item.transform?.opacity, - transform: nativeTransformToFrame(item.transform), + // Optional fields are emitted only when set so the comparable shape + // matches host snapshots that omit them entirely. + ...(item.volume !== undefined ? { volume: item.volume } : {}), + ...(item.speed !== undefined ? { speed: item.speed } : {}), + ...(opacityOnly ? { opacity: transform.opacity } : {}), + ...(transform !== undefined && !opacityOnly ? { transform } : {}), } } @@ -280,6 +302,14 @@ function frameItemToNativeComparable( } if (item.type === 'text') { + const style = { + ...(item.fontFamily ? { font_family: item.fontFamily } : {}), + ...(item.fontSize !== undefined ? { font_size: item.fontSize } : {}), + ...(item.color && item.color !== '#ffffff' ? { color: item.color } : {}), + ...(item.textAlign ? { alignment: item.textAlign } : {}), + } + const transform = nativeTransformToFrame(item.transform) + const opacityOnly = transform !== undefined && isOpacityOnlyTransform(transform) return { type: 'text', id: item.id, @@ -287,14 +317,9 @@ function frameItemToNativeComparable( from: item.from, durationInFrames: item.durationInFrames, text: item.text, - style: { - ...(item.fontFamily ? { font_family: item.fontFamily } : {}), - ...(item.fontSize !== undefined ? { font_size: item.fontSize } : {}), - ...(item.color && item.color !== '#ffffff' ? { color: item.color } : {}), - ...(item.textAlign ? { alignment: item.textAlign } : {}), - }, - opacity: item.transform?.opacity, - transform: nativeTransformToFrame(item.transform), + ...(Object.keys(style).length > 0 ? { style } : {}), + ...(opacityOnly ? { opacity: transform.opacity } : {}), + ...(transform !== undefined && !opacityOnly ? { transform } : {}), } } From 01a80ab0e1e2b9a4cd5a92208389a19cb4ea3273 Mon Sep 17 00:00:00 2001 From: Patrick Lu Date: Fri, 21 Aug 2026 18:56:56 -0700 Subject: [PATCH 2/4] feat(editor): mount host-safe timeline shortcuts in host mode TimelineShortcutsController was only rendered outside host mode, so Delete/Backspace/Space/J/K/L and friends did nothing in the host-embedded surface. Add useHostTimelineShortcuts (composed in use-timeline-shortcuts.ts) and mount it via a HostTimelineShortcutsController when hostRuntime is present. The composition mounts only bindings that never mutate the host-owned document outside the bridge: - Playback/navigation and tool switching mount as-is (local UI state; Shift+C split crosses the bridge as a supported split_item command). - Delete/Backspace are extracted into a shared useDeleteShortcuts hook; item removal flows through the bridge as remove_item commands, now batched for multi-select. - useUIShortcuts gains an enableHistory option (default true) so host mode mounts zoom/snap without undo/redo, which would mutate the temporal store without emitting host commands. Ripple delete, clipboard, markers, in/out points, nudges, join, freeze frame, and clear-keyframes stay unmounted in host mode. The editor -> timeline edge stays within budget by exporting the new hook from the existing use-timeline-shortcuts module. --- src/features/editor/components/editor.tsx | 10 ++ .../editor/deps/timeline-hooks-contract.ts | 1 + .../hooks/shortcuts/use-delete-shortcuts.ts | 89 ++++++++++ .../hooks/shortcuts/use-editing-shortcuts.ts | 96 +---------- .../hooks/shortcuts/use-ui-shortcuts.ts | 20 ++- .../use-host-timeline-shortcuts.test.tsx | 153 ++++++++++++++++++ .../timeline/hooks/use-timeline-shortcuts.ts | 24 +++ 7 files changed, 299 insertions(+), 94 deletions(-) create mode 100644 src/features/timeline/hooks/shortcuts/use-delete-shortcuts.ts create mode 100644 src/features/timeline/hooks/use-host-timeline-shortcuts.test.tsx diff --git a/src/features/editor/components/editor.tsx b/src/features/editor/components/editor.tsx index 716696b54..a81593002 100644 --- a/src/features/editor/components/editor.tsx +++ b/src/features/editor/components/editor.tsx @@ -28,6 +28,7 @@ import { toast } from 'sonner' import { useEditorHotkeys } from '@/features/editor/hooks/use-editor-hotkeys' import { useAutoSave } from '../hooks/use-auto-save' import { + useHostTimelineShortcuts, useTimelineShortcuts, useTransitionBreakageNotifications, } from '@/features/editor/deps/timeline-hooks' @@ -429,6 +430,14 @@ const TimelineShortcutsController = memo(function TimelineShortcutsController() return null }) +// Host mode mounts only the host-safe shortcut slice (playback, tools, +// delete, zoom/snap) — undo/redo, ripple delete, clipboard, markers, and +// nudges would mutate local stores without crossing the host bridge. +const HostTimelineShortcutsController = memo(function HostTimelineShortcutsController() { + useHostTimelineShortcuts() + return null +}) + // fallow-ignore-next-line complexity export const LoadedEditor = memo(function LoadedEditor({ projectId, @@ -746,6 +755,7 @@ export const LoadedEditor = memo(function LoadedEditor({ > {!hostRuntime && } {!hostRuntime && } + {hostRuntime && } {!hostRuntime && ( )} diff --git a/src/features/editor/deps/timeline-hooks-contract.ts b/src/features/editor/deps/timeline-hooks-contract.ts index 6d436e571..62ccd27ab 100644 --- a/src/features/editor/deps/timeline-hooks-contract.ts +++ b/src/features/editor/deps/timeline-hooks-contract.ts @@ -4,6 +4,7 @@ */ export { + useHostTimelineShortcuts, useTimelineShortcuts, } from '@/features/timeline/hooks/use-timeline-shortcuts' export { diff --git a/src/features/timeline/hooks/shortcuts/use-delete-shortcuts.ts b/src/features/timeline/hooks/shortcuts/use-delete-shortcuts.ts new file mode 100644 index 000000000..fa4d0ddfa --- /dev/null +++ b/src/features/timeline/hooks/shortcuts/use-delete-shortcuts.ts @@ -0,0 +1,89 @@ +/** + * Delete shortcuts: Delete/Backspace - remove selected items, marker, or transition. + * + * Extracted from useEditingShortcuts so host-embedded surfaces can mount just + * the remove bindings: item removal flows through the host bridge as + * remove_item commands, while the remaining editing shortcuts (ripple delete, + * nudges, join, freeze frame, keyframes) are unsupported in host mode. + */ + +import { useCallback } from 'react' +import { useHotkeys } from 'react-hotkeys-hook' +import { useEditorStore } from '@/shared/state/editor' +import { useTimelineStore } from '../../stores/timeline-store' +import { useSelectionStore } from '@/shared/state/selection' +import { HOTKEY_OPTIONS } from '@/config/hotkeys' +import type { TimelineShortcutCallbacks } from '../use-timeline-shortcuts' +import { useResolvedHotkeys } from '@/features/timeline/deps/settings' +import { useKeyframeSelectionStore } from '../../stores/keyframe-selection-store' + +export function useDeleteShortcuts(callbacks: TimelineShortcutCallbacks) { + const hotkeys = useResolvedHotkeys() + const selectedItemIds = useSelectionStore((s) => s.selectedItemIds) + const selectedMarkerId = useSelectionStore((s) => s.selectedMarkerId) + const selectedTransitionId = useSelectionStore((s) => s.selectedTransitionId) + const editKeyframePanelOpen = useSelectionStore((s) => s.editKeyframePanelOpen) + const clearSelection = useSelectionStore((s) => s.clearSelection) + const selectedKeyframes = useKeyframeSelectionStore((s) => s.selectedKeyframes) + const removeItems = useTimelineStore((s) => s.removeItems) + const removeMarker = useTimelineStore((s) => s.removeMarker) + const removeTransition = useTimelineStore((s) => s.removeTransition) + const keyframeEditorShortcutScopeActive = useEditorStore( + (s) => s.keyframeEditorShortcutScopeActive, + ) + const transcriptEditorShortcutScopeActive = useEditorStore( + (s) => s.transcriptEditorShortcutScopeActive, + ) + // Another panel (keyframe or transcript editor) owns Delete/Backspace — clip + // delete must yield so it doesn't also fire and remove the timeline clip. + const deleteOwnedByPanel = + keyframeEditorShortcutScopeActive || + (editKeyframePanelOpen && selectedKeyframes.length > 0) || + transcriptEditorShortcutScopeActive + + const deleteSelection = useCallback( + (event: KeyboardEvent) => { + if (deleteOwnedByPanel) { + event.preventDefault() + event.stopPropagation() + return + } + if (selectedTransitionId) { + event.preventDefault() + removeTransition(selectedTransitionId) + clearSelection() + return + } + if (selectedMarkerId) { + event.preventDefault() + removeMarker(selectedMarkerId) + clearSelection() + return + } + if (selectedItemIds.length > 0) { + event.preventDefault() + removeItems(selectedItemIds) + if (callbacks.onDelete) { + callbacks.onDelete() + } + } + }, + [ + deleteOwnedByPanel, + selectedItemIds, + selectedMarkerId, + selectedTransitionId, + removeItems, + removeMarker, + removeTransition, + clearSelection, + callbacks, + ], + ) + + // Editing: Delete - Delete selected items, marker, or transition + useHotkeys(hotkeys.DELETE_SELECTED, deleteSelection, HOTKEY_OPTIONS, [deleteSelection]) + + // Editing: Backspace - Delete selected items, marker, or transition (alternative) + useHotkeys(hotkeys.DELETE_SELECTED_ALT, deleteSelection, HOTKEY_OPTIONS, [deleteSelection]) +} diff --git a/src/features/timeline/hooks/shortcuts/use-editing-shortcuts.ts b/src/features/timeline/hooks/shortcuts/use-editing-shortcuts.ts index 81987c096..276c038c8 100644 --- a/src/features/timeline/hooks/shortcuts/use-editing-shortcuts.ts +++ b/src/features/timeline/hooks/shortcuts/use-editing-shortcuts.ts @@ -22,18 +22,14 @@ import type { TimelineShortcutCallbacks } from '../use-timeline-shortcuts' import { useClearKeyframesDialogStore } from '@/shared/state/clear-keyframes-dialog' import { useResolvedHotkeys } from '@/features/timeline/deps/settings' import { useKeyframeSelectionStore } from '../../stores/keyframe-selection-store' +import { useDeleteShortcuts } from './use-delete-shortcuts' export function useEditingShortcuts(callbacks: TimelineShortcutCallbacks) { const hotkeys = useResolvedHotkeys() const selectedItemIds = useSelectionStore((s) => s.selectedItemIds) - const selectedMarkerId = useSelectionStore((s) => s.selectedMarkerId) - const selectedTransitionId = useSelectionStore((s) => s.selectedTransitionId) const editKeyframePanelOpen = useSelectionStore((s) => s.editKeyframePanelOpen) const clearSelection = useSelectionStore((s) => s.clearSelection) const selectedKeyframes = useKeyframeSelectionStore((s) => s.selectedKeyframes) - const removeItems = useTimelineStore((s) => s.removeItems) - const removeMarker = useTimelineStore((s) => s.removeMarker) - const removeTransition = useTimelineStore((s) => s.removeTransition) const rippleDeleteItems = useTimelineStore((s) => s.rippleDeleteItems) const updateItemsTransformMap = useTimelineStore((s) => s.updateItemsTransformMap) const joinItems = useTimelineStore((s) => s.joinItems) @@ -52,6 +48,10 @@ export function useEditingShortcuts(callbacks: TimelineShortcutCallbacks) { (editKeyframePanelOpen && selectedKeyframes.length > 0) || transcriptEditorShortcutScopeActive + // Delete/Backspace live in a dedicated hook so host-embedded surfaces can + // mount just the remove bindings. + useDeleteShortcuts(callbacks) + const nudgeSelectedVisualItems = useCallback( (deltaX: number, deltaY: number) => { if (selectedItemIds.length === 0) return @@ -78,92 +78,6 @@ export function useEditingShortcuts(callbacks: TimelineShortcutCallbacks) { [selectedItemIds, items, updateItemsTransformMap], ) - // Editing: Delete - Delete selected items, marker, or transition - useHotkeys( - hotkeys.DELETE_SELECTED, - (event) => { - if (deleteOwnedByPanel) { - event.preventDefault() - event.stopPropagation() - return - } - if (selectedTransitionId) { - event.preventDefault() - removeTransition(selectedTransitionId) - clearSelection() - return - } - if (selectedMarkerId) { - event.preventDefault() - removeMarker(selectedMarkerId) - clearSelection() - return - } - if (selectedItemIds.length > 0) { - event.preventDefault() - removeItems(selectedItemIds) - if (callbacks.onDelete) { - callbacks.onDelete() - } - } - }, - HOTKEY_OPTIONS, - [ - deleteOwnedByPanel, - selectedItemIds, - selectedMarkerId, - selectedTransitionId, - removeItems, - removeMarker, - removeTransition, - clearSelection, - callbacks, - ], - ) - - // Editing: Backspace - Delete selected items, marker, or transition (alternative) - useHotkeys( - hotkeys.DELETE_SELECTED_ALT, - (event) => { - if (deleteOwnedByPanel) { - event.preventDefault() - event.stopPropagation() - return - } - if (selectedTransitionId) { - event.preventDefault() - removeTransition(selectedTransitionId) - clearSelection() - return - } - if (selectedMarkerId) { - event.preventDefault() - removeMarker(selectedMarkerId) - clearSelection() - return - } - if (selectedItemIds.length > 0) { - event.preventDefault() - removeItems(selectedItemIds) - if (callbacks.onDelete) { - callbacks.onDelete() - } - } - }, - HOTKEY_OPTIONS, - [ - deleteOwnedByPanel, - selectedItemIds, - selectedMarkerId, - selectedTransitionId, - removeItems, - removeMarker, - removeTransition, - clearSelection, - callbacks, - ], - ) - // Editing: Ctrl+Delete - Ripple delete selected items (delete + close gap) useHotkeys( hotkeys.RIPPLE_DELETE, diff --git a/src/features/timeline/hooks/shortcuts/use-ui-shortcuts.ts b/src/features/timeline/hooks/shortcuts/use-ui-shortcuts.ts index f07d252d9..cffd9cbc7 100644 --- a/src/features/timeline/hooks/shortcuts/use-ui-shortcuts.ts +++ b/src/features/timeline/hooks/shortcuts/use-ui-shortcuts.ts @@ -10,7 +10,19 @@ import { HOTKEY_OPTIONS } from '@/config/hotkeys' import type { TimelineShortcutCallbacks } from '../use-timeline-shortcuts' import { useResolvedHotkeys, useSettingsStore } from '@/features/timeline/deps/settings' -export function useUIShortcuts(callbacks: TimelineShortcutCallbacks) { +export interface UIShortcutOptions { + /** + * Undo/redo mutate the timeline temporal store directly without emitting + * host commands, so host-embedded surfaces must mount with this disabled. + */ + enableHistory?: boolean +} + +export function useUIShortcuts( + callbacks: TimelineShortcutCallbacks, + options: UIShortcutOptions = {}, +) { + const { enableHistory = true } = options const hotkeys = useResolvedHotkeys() const toggleSnap = useTimelineStore((s) => s.toggleSnap) const zoomIn = useZoomStore((s) => s.zoomIn) @@ -29,8 +41,9 @@ export function useUIShortcuts(callbacks: TimelineShortcutCallbacks) { { ...HOTKEY_OPTIONS, enableOnFormTags: true, + enabled: enableHistory, }, - [callbacks], + [callbacks, enableHistory], ) // History: Cmd/Ctrl+Shift+Z - Redo @@ -46,8 +59,9 @@ export function useUIShortcuts(callbacks: TimelineShortcutCallbacks) { { ...HOTKEY_OPTIONS, enableOnFormTags: true, + enabled: enableHistory, }, - [callbacks], + [callbacks, enableHistory], ) // UI: S - Toggle Snap diff --git a/src/features/timeline/hooks/use-host-timeline-shortcuts.test.tsx b/src/features/timeline/hooks/use-host-timeline-shortcuts.test.tsx new file mode 100644 index 000000000..3e2518986 --- /dev/null +++ b/src/features/timeline/hooks/use-host-timeline-shortcuts.test.tsx @@ -0,0 +1,153 @@ +import { fireEvent, render } from '@testing-library/react' +import { beforeEach, describe, expect, it, vi } from 'vite-plus/test' +import { useEditorStore } from '@/shared/state/editor' +import { usePlaybackStore } from '@/shared/state/playback' +import { useSelectionStore } from '@/shared/state/selection' +import { useTimelineStore } from '../stores/timeline-store' +import { useTimelineCommandStore } from '../stores/timeline-command-store' +import { useKeyframeSelectionStore } from '../stores/keyframe-selection-store' +import { useHostTimelineShortcuts, useTimelineShortcuts } from './use-timeline-shortcuts' +import type { TimelineTrack, VideoItem } from '@/types/timeline' + +// Some machines run jsdom with an opaque origin, leaving localStorage +// undefined; the zustand persist middleware captures it at store creation +// (import time). Install a stub before imports evaluate — a no-op wherever +// the environment provides a real localStorage (e.g. CI). +vi.hoisted(() => { + if (typeof globalThis.localStorage !== 'undefined') return + const backing = new Map() + const stub: Storage = { + getItem: (key: string) => backing.get(key) ?? null, + setItem: (key: string, value: string) => void backing.set(key, String(value)), + removeItem: (key: string) => void backing.delete(key), + clear: () => backing.clear(), + key: () => null, + get length() { + return backing.size + }, + } + Object.defineProperty(globalThis, 'localStorage', { value: stub, configurable: true }) +}) + +function HostShortcutHarness() { + useHostTimelineShortcuts() + return null +} + +function FullShortcutHarness() { + useTimelineShortcuts() + return null +} + +const TRACK: TimelineTrack = { + id: 'track-1', + name: 'V1', + kind: 'video', + order: 0, + height: 80, + locked: false, + visible: true, + muted: false, + solo: false, + items: [], +} + +const ITEM: VideoItem = { + id: 'clip-1', + type: 'video', + trackId: 'track-1', + from: 0, + durationInFrames: 30, + label: 'Clip 1', + src: 'clip.mp4', +} + +describe('useHostTimelineShortcuts', () => { + beforeEach(() => { + useTimelineCommandStore.getState().clearHistory() + useSelectionStore.setState({ + selectedItemIds: [], + selectedMarkerId: null, + selectedTransitionId: null, + selectionType: null, + editKeyframePanelOpen: false, + expandedKeyframeLanes: new Set(), + }) + useKeyframeSelectionStore.setState({ + selectedKeyframes: [], + clipboard: null, + isCut: false, + }) + useEditorStore.setState({ + keyframeEditorShortcutScopeActive: false, + transcriptEditorShortcutScopeActive: false, + }) + usePlaybackStore.setState({ + isPlaying: false, + currentFrame: 0, + previewFrame: null, + previewItemId: null, + }) + useTimelineStore.setState({ + tracks: [TRACK], + items: [ITEM], + transitions: [], + keyframes: [], + markers: [], + }) + }) + + it('toggles playback on Space', () => { + render() + + expect(usePlaybackStore.getState().isPlaying).toBe(false) + fireEvent.keyDown(document, { key: ' ', code: 'Space' }) + expect(usePlaybackStore.getState().isPlaying).toBe(true) + fireEvent.keyDown(document, { key: ' ', code: 'Space' }) + expect(usePlaybackStore.getState().isPlaying).toBe(false) + }) + + it('removes the selected item on Delete', () => { + useSelectionStore.setState({ selectedItemIds: ['clip-1'], selectionType: 'item' }) + render() + + fireEvent.keyDown(document, { key: 'Delete', code: 'Delete' }) + + expect(useTimelineStore.getState().items).toHaveLength(0) + }) + + it('removes multiple selected items on Delete', () => { + const clip2: VideoItem = { ...ITEM, id: 'clip-2', from: 30 } + useTimelineStore.setState({ items: [ITEM, clip2] }) + useSelectionStore.setState({ + selectedItemIds: ['clip-1', 'clip-2'], + selectionType: 'item', + }) + render() + + fireEvent.keyDown(document, { key: 'Delete', code: 'Delete' }) + + expect(useTimelineStore.getState().items).toHaveLength(0) + }) + + it('does not undo timeline edits on Mod+Z in host mode', () => { + useTimelineStore.getState().moveItem('clip-1', 30) + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(1) + render() + + fireEvent.keyDown(document, { key: 'z', code: 'KeyZ', metaKey: true }) + + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(1) + expect(useTimelineStore.getState().items[0]).toMatchObject({ id: 'clip-1', from: 30 }) + }) + + it('still undoes on Mod+Z with the full timeline shortcuts (control)', () => { + useTimelineStore.getState().moveItem('clip-1', 30) + expect(useTimelineCommandStore.getState().undoStack).toHaveLength(1) + render() + + fireEvent.keyDown(document, { key: 'z', code: 'KeyZ', metaKey: true }) + + expect(useTimelineStore.getState().items[0]).toMatchObject({ id: 'clip-1', from: 0 }) + }) +}) diff --git a/src/features/timeline/hooks/use-timeline-shortcuts.ts b/src/features/timeline/hooks/use-timeline-shortcuts.ts index 62da88de7..146c60b02 100644 --- a/src/features/timeline/hooks/use-timeline-shortcuts.ts +++ b/src/features/timeline/hooks/use-timeline-shortcuts.ts @@ -1,5 +1,6 @@ import { usePlaybackShortcuts } from './shortcuts/use-playback-shortcuts' import { useEditingShortcuts } from './shortcuts/use-editing-shortcuts' +import { useDeleteShortcuts } from './shortcuts/use-delete-shortcuts' import { useToolShortcuts } from './shortcuts/use-tool-shortcuts' import { useMarkerShortcuts } from './shortcuts/use-marker-shortcuts' import { useInOutShortcuts } from './shortcuts/use-in-out-shortcuts' @@ -41,3 +42,26 @@ export function useTimelineShortcuts(callbacks: TimelineShortcutCallbacks = {}) useClipboardShortcuts() useSourceMonitorShortcuts() } + +/** + * Host-embedded timeline keyboard shortcuts. + * + * Composes only the bindings that are safe while a host owns the + * authoritative timeline document: + * - Playback & navigation (Space, J/K/L, arrows, Home/End, snap points) — + * local playback state that never crosses the host bridge. + * - Tools (V/T/C/R, Shift+C split) — tool switching is pure local UI; split + * flows through the bridge as a supported split_item command. + * - Delete/Backspace — flows through the bridge as remove_item commands. + * - UI zoom/snap (S, Shift+S, Cmd/Ctrl+=/-, \, Shift+\) — local view state. + * + * Deliberately excluded: undo/redo (mutate the temporal store without host + * commands), ripple delete, clipboard, markers, in/out points, nudges, join, + * freeze frame, and clear-keyframes — all unsupported by the host slice. + */ +export function useHostTimelineShortcuts(callbacks: TimelineShortcutCallbacks = {}) { + usePlaybackShortcuts(callbacks) + useDeleteShortcuts(callbacks) + useToolShortcuts(callbacks) + useUIShortcuts(callbacks, { enableHistory: false }) +} From c679db10040bc1a1739f49916ef7f5afdbca378a Mon Sep 17 00:00:00 2001 From: Patrick Lu Date: Fri, 21 Aug 2026 19:03:18 -0700 Subject: [PATCH 3/4] refactor(editor): drop redundant null guards in isOpacityOnlyTransform nativeTransformToFrame fills every key, so the ?? 0 fallbacks were dead branches that tripped the fallow changed-health complexity gate. --- src/features/editor/host/document.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/features/editor/host/document.ts b/src/features/editor/host/document.ts index 7d0b7211b..f0f8dcec0 100644 --- a/src/features/editor/host/document.ts +++ b/src/features/editor/host/document.ts @@ -51,17 +51,18 @@ function nativeTransformToFrame( * A frame transform whose only non-default value is opacity round-trips * through the native item as a top-level `opacity` field (see * nativeItemFromHostItem). Emitting a `transform` key for it would break - * round-trip equivalence with host items that carry opacity only. + * round-trip equivalence with host items that carry opacity only. The input + * always comes from nativeTransformToFrame, which fills every key. */ function isOpacityOnlyTransform(transform: Record): boolean { return ( - (transform.x ?? 0) === 0 && - (transform.y ?? 0) === 0 && - (transform.width ?? 0) === 0 && - (transform.height ?? 0) === 0 && - (transform.anchorX ?? 0) === 0 && - (transform.anchorY ?? 0) === 0 && - (transform.rotation ?? 0) === 0 + transform.x === 0 && + transform.y === 0 && + transform.width === 0 && + transform.height === 0 && + transform.anchorX === 0 && + transform.anchorY === 0 && + transform.rotation === 0 ) } From bf0f29866067674b7e0710286675eb0b03c501aa Mon Sep 17 00:00:00 2001 From: Patrick Lu Date: Fri, 21 Aug 2026 19:34:29 -0700 Subject: [PATCH 4/4] fix(preview): keep host-provided cross-origin media audible MediaElementAudioSourceNode silences cross-origin media served without CORS approval (HTML spec), so host-mode playback through the Web Audio clip graph was muted. Route cross-origin sources around the graph: applyVideoElementAudioState and NativePitchCorrectedAudio now drive the media element's volume/muted directly when the source is not WebAudio-safe (isWebAudioSafeMediaSource: same-origin, blob:, data:), with reactive volume/mute propagation on the direct path. EQ remains graph-only and is skipped on the direct path. Host runtime hardening: reset the persisted playback mute/volume on mount (the monitor volume UI is hidden in host mode, so a persisted mute silently zeroed embedded audio), and keep resilient pointerdown/keydown listeners for the whole host session that resume the shared preview AudioContext when it starts suspended. --- .../deps/composition-runtime-contract.ts | 1 + src/features/editor/host/runtime.test.tsx | 159 ++++++++++++++++++ src/features/editor/host/runtime.ts | 31 ++++ .../components/pitch-corrected-audio.test.tsx | 96 +++++++++++ .../components/pitch-corrected-audio.tsx | 32 +++- .../components/video-audio-context.test.ts | 95 +++++++++++ .../components/video-audio-context.ts | 11 ++ .../utils/media-source-origin.test.ts | 23 +++ .../utils/media-source-origin.ts | 19 +++ 9 files changed, 464 insertions(+), 3 deletions(-) create mode 100644 src/features/editor/host/runtime.test.tsx create mode 100644 src/runtime/composition-runtime/components/video-audio-context.test.ts create mode 100644 src/runtime/composition-runtime/utils/media-source-origin.test.ts create mode 100644 src/runtime/composition-runtime/utils/media-source-origin.ts diff --git a/src/features/editor/deps/composition-runtime-contract.ts b/src/features/editor/deps/composition-runtime-contract.ts index f6bd6bbf2..c812ff22f 100644 --- a/src/features/editor/deps/composition-runtime-contract.ts +++ b/src/features/editor/deps/composition-runtime-contract.ts @@ -26,3 +26,4 @@ export { } from '@/runtime/composition-runtime/utils/corner-pin' export { clearPreviewAudioCache } from '@/runtime/composition-runtime/utils/audio-decode-cache' export { deletePreviewAudioConform } from '@/runtime/composition-runtime/utils/preview-audio-conform' +export { peekSharedPreviewAudioContext } from '@/runtime/composition-runtime/utils/preview-audio-graph' diff --git a/src/features/editor/host/runtime.test.tsx b/src/features/editor/host/runtime.test.tsx new file mode 100644 index 000000000..c9d9e201d --- /dev/null +++ b/src/features/editor/host/runtime.test.tsx @@ -0,0 +1,159 @@ +import { afterEach, describe, expect, it, vi } from 'vite-plus/test' + +// Some machines run jsdom with an opaque origin, leaving localStorage +// undefined; the zustand persist middleware captures it at store creation +// (import time). Install a stub before imports evaluate — a no-op wherever +// the environment provides a real localStorage (e.g. CI). +vi.hoisted(() => { + if (typeof globalThis.localStorage !== 'undefined') return + const backing = new Map() + const stub: Storage = { + getItem: (key: string) => backing.get(key) ?? null, + setItem: (key: string, value: string) => void backing.set(key, String(value)), + removeItem: (key: string) => void backing.delete(key), + clear: () => backing.clear(), + key: () => null, + get length() { + return backing.size + }, + } + Object.defineProperty(globalThis, 'localStorage', { value: stub, configurable: true }) +}) + +const compositionRuntimeMocks = vi.hoisted(() => ({ + previewContext: null as { + state: 'suspended' | 'running' + resume: () => Promise + } | null, +})) + +vi.mock('@/features/editor/deps/composition-runtime', () => ({ + peekSharedPreviewAudioContext: () => compositionRuntimeMocks.previewContext, +})) + +import { usePlaybackStore } from '@/shared/state/playback' +import type { EditorHost, EmbeddedEditorSnapshot } from './contract' +import { EmbeddedEditorHostRuntime } from './runtime' + +function snapshot(): EmbeddedEditorSnapshot { + return { + project: { + id: 'project-1', + name: 'Host project', + width: 1920, + height: 1080, + fps: 30, + backgroundColor: '#000000', + }, + timeline: { + timelineId: 'timeline-1', + revision: 0, + fps: 30, + durationInFrames: 300, + media: [ + { + media_id: 'media-1', + media_kind: 'video', + content_hash: 'sha256:media-1', + duration_us: 10_000_000, + availability: { mode: 'cloud', cloud: { object_id: 'opaque-media-object-1' } }, + }, + ], + tracks: [ + { + id: 'track-1', + kind: 'video', + name: 'Video 1', + locked: false, + muted: false, + items: [ + { + type: 'video', + id: 'clip-1', + trackId: 'track-1', + mediaId: 'media-1', + from: 0, + durationInFrames: 60, + sourceStart: 0, + sourceEnd: 60, + }, + ], + }, + ], + width: 1920, + height: 1080, + backgroundColor: '#000000', + }, + assets: [ + { + id: 'media-1', + kind: 'video', + fileName: 'host-video.mp4', + mimeType: 'video/mp4', + durationSeconds: 10, + width: 1920, + height: 1080, + fps: 30, + contentHash: 'sha256:media-1', + }, + ], + } +} + +function createHost(snap: EmbeddedEditorSnapshot): EditorHost { + return { + capabilities: {}, + load: () => snap, + resolveMedia: async () => null, + submitEdit: async () => { + throw new Error('not used') + }, + } +} + +describe('EmbeddedEditorHostRuntime host audio', () => { + afterEach(() => { + compositionRuntimeMocks.previewContext = null + }) + + it('resets persisted playback mute/volume on mount', () => { + usePlaybackStore.setState({ muted: true, volume: 0 }) + + const runtime = new EmbeddedEditorHostRuntime(createHost(snapshot()), snapshot()) + try { + runtime.mountStores() + expect(usePlaybackStore.getState().muted).toBe(false) + expect(usePlaybackStore.getState().volume).toBe(1) + } finally { + runtime.unmountStores() + } + }) + + it('resumes a suspended preview AudioContext on every user gesture', () => { + const resume = vi.fn(() => Promise.resolve()) + compositionRuntimeMocks.previewContext = { state: 'suspended', resume } + + const runtime = new EmbeddedEditorHostRuntime(createHost(snapshot()), snapshot()) + runtime.mountStores() + try { + document.dispatchEvent(new Event('pointerdown')) + expect(resume).toHaveBeenCalledTimes(1) + + // Resilient, not one-time: a later keydown resumes again. + document.dispatchEvent(new KeyboardEvent('keydown')) + expect(resume).toHaveBeenCalledTimes(2) + + // A running context is left alone. + compositionRuntimeMocks.previewContext = { state: 'running', resume } + document.dispatchEvent(new Event('pointerdown')) + expect(resume).toHaveBeenCalledTimes(2) + } finally { + runtime.unmountStores() + } + + // Listeners are removed on unmount. + compositionRuntimeMocks.previewContext = { state: 'suspended', resume } + document.dispatchEvent(new Event('pointerdown')) + expect(resume).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/features/editor/host/runtime.ts b/src/features/editor/host/runtime.ts index 7d2960ee2..98cca9301 100644 --- a/src/features/editor/host/runtime.ts +++ b/src/features/editor/host/runtime.ts @@ -15,6 +15,7 @@ import { usePlaybackStore } from '@/shared/state/playback' import { useEditorStore } from '@/shared/state/editor' import { useSelectionStore } from '@/shared/state/selection' import { useGizmoStore, useMaskEditorStore } from '@/features/editor/deps/preview' +import { peekSharedPreviewAudioContext } from '@/features/editor/deps/composition-runtime' import { isHostCapabilityEnabled, type EmbeddedEditorSnapshot, @@ -53,6 +54,7 @@ export class EmbeddedEditorHostRuntime implements EmbeddedEditorHostRuntimeContr private applyingAuthoritative = false private reconcileScheduled = false private editInFlight = false + private gestureListenersAttached = false private unsubscribeResolver: (() => void) | null = null private unsubscribeController: (() => void) | null = null private unsubscribeTimeline: (() => void) | null = null @@ -65,10 +67,28 @@ export class EmbeddedEditorHostRuntime implements EmbeddedEditorHostRuntimeContr this.controller = new HostEditorController(host, snapshot) } + /** + * The host serves cross-origin media URLs, which rules out the Web Audio + * clip graph (non-CORS cross-origin resources are silenced through + * MediaElementAudioSourceNode) — but the shared preview AudioContext can + * still start suspended when playback did not begin with a real user + * gesture. Kept attached for the whole host session (not once) so a + * context created after the first gesture is also resumed. + */ + private readonly resumePreviewAudioOnGesture = (): void => { + const context = peekSharedPreviewAudioContext() + if (context?.state === 'suspended') { + void context.resume() + } + } + mountStores(): void { if (this.mounted) return this.mounted = true useEditorStore.setState({ hostMode: true }) + // The monitor volume UI is hidden in host mode, so a persisted local + // mute/volume preference must not silently zero embedded audio. + usePlaybackStore.setState({ muted: false, volume: 1 }) useSelectionStore.getState().clearSelection() useSelectionStore.getState().setActiveTool('select') useGizmoStore.getState().cancelInteraction() @@ -76,6 +96,12 @@ export class EmbeddedEditorHostRuntime implements EmbeddedEditorHostRuntimeContr useMaskEditorStore.getState().stopEditing() this.applySnapshotToStores(this.authoritativeSnapshot) + if (typeof document !== 'undefined') { + document.addEventListener('pointerdown', this.resumePreviewAudioOnGesture) + document.addEventListener('keydown', this.resumePreviewAudioOnGesture) + this.gestureListenersAttached = true + } + this.unsubscribeResolver = installRuntimeMediaResolver(async (mediaId) => { const asset = this.authoritativeSnapshot.assets.find((candidate) => candidate.id === mediaId) if (!asset) return null @@ -97,6 +123,11 @@ export class EmbeddedEditorHostRuntime implements EmbeddedEditorHostRuntimeContr unmountStores(): void { if (!this.mounted) return this.mounted = false + if (this.gestureListenersAttached) { + document.removeEventListener('pointerdown', this.resumePreviewAudioOnGesture) + document.removeEventListener('keydown', this.resumePreviewAudioOnGesture) + this.gestureListenersAttached = false + } this.unsubscribeTimeline?.() this.unsubscribeTimeline = null this.unsubscribeController?.() diff --git a/src/runtime/composition-runtime/components/pitch-corrected-audio.test.tsx b/src/runtime/composition-runtime/components/pitch-corrected-audio.test.tsx index 96db31dd1..0a584c8fb 100644 --- a/src/runtime/composition-runtime/components/pitch-corrected-audio.test.tsx +++ b/src/runtime/composition-runtime/components/pitch-corrected-audio.test.tsx @@ -409,3 +409,99 @@ describe('PitchCorrectedAudio', () => { expect(document.querySelector('[data-testid="pitch"]')).toBeInTheDocument() }) }) + +describe('PitchCorrectedAudio cross-origin media', () => { + beforeEach(() => { + vi.clearAllMocks() + clockRateMocks.current = 1 + playbackStateMocks.current = { + frame: 0, + fps: 30, + playing: false, + resolvedVolume: 1, + resolvedPitchShiftSemitones: 0, + resolvedAudioEqStages: [], + } + }) + + it('drives the element directly instead of the silenced Web Audio path', async () => { + const { rerender } = render( + , + ) + + await waitFor(() => { + expect(previewAudioMocks.acquirePreviewAudioElement).toHaveBeenCalledWith( + 'https://cdn.example.com/a.mp3', + ) + }) + + expect(previewGraphMocks.createPreviewClipAudioGraph).not.toHaveBeenCalled() + expect(previewGraphMocks.graph.context.createMediaElementSource).not.toHaveBeenCalled() + expect(previewAudioMocks.markPreviewAudioElementUsesWebAudio).not.toHaveBeenCalled() + + const audio = previewAudioMocks.state.current + expect(audio).not.toBeNull() + + playbackStateMocks.current = { ...playbackStateMocks.current, resolvedVolume: 0.4 } + rerender( + , + ) + + await waitFor(() => { + expect(audio?.volume).toBeCloseTo(0.4) + }) + expect(audio?.muted).toBe(false) + expect(previewGraphMocks.rampPreviewClipGain).not.toHaveBeenCalled() + + rerender( + , + ) + + await waitFor(() => { + expect(audio?.muted).toBe(true) + expect(audio?.volume).toBe(0) + }) + expect(previewGraphMocks.rampPreviewClipGain).not.toHaveBeenCalled() + }) + + it('keeps same-origin blob sources on the Web Audio graph', async () => { + render( + , + ) + + await waitFor(() => { + expect(previewGraphMocks.graph.context.createMediaElementSource).toHaveBeenCalledWith( + previewAudioMocks.state.current, + ) + }) + expect(previewAudioMocks.markPreviewAudioElementUsesWebAudio).toHaveBeenCalledWith( + previewAudioMocks.state.current, + ) + }) +}) diff --git a/src/runtime/composition-runtime/components/pitch-corrected-audio.tsx b/src/runtime/composition-runtime/components/pitch-corrected-audio.tsx index 301db6a97..f367e39dc 100644 --- a/src/runtime/composition-runtime/components/pitch-corrected-audio.tsx +++ b/src/runtime/composition-runtime/components/pitch-corrected-audio.tsx @@ -19,6 +19,7 @@ import { setPreviewClipGain, type PreviewClipAudioGraph, } from '../utils/preview-audio-graph' +import { isWebAudioSafeMediaSource } from '../utils/media-source-origin' import { SoundTouchWorkletAudio } from './soundtouch-worklet-audio' import type { AudioPlaybackProps } from './audio-playback-props' import { getBrowserMediaPlaybackRate } from '@/shared/state/playback/shuttle' @@ -221,6 +222,24 @@ export const NativePitchCorrectedAudio: React.FC = Rea useEffect(() => { const audio = acquirePreviewAudioElement(src) + + // Cross-origin media without CORS approval is silenced by + // MediaElementAudioSourceNode (HTML spec) — host-provided signed URLs + // keep the element on the direct volume/muted path (driven by the sync + // effect below) instead of the Web Audio graph. EQ is graph-only and + // does not apply on this path. + if (!isWebAudioSafeMediaSource(src)) { + audioRef.current = audio + return () => { + audioRef.current = null + if (preWarmTimerRef.current !== null) { + clearTimeout(preWarmTimerRef.current) + preWarmTimerRef.current = null + } + releasePreviewAudioElement(audio) + } + } + // Keep the media element and graph alive across EQ toggles; the EQ stages ramp in place below. const graph = createPreviewClipAudioGraph() if (!graph) { @@ -266,10 +285,17 @@ export const NativePitchCorrectedAudio: React.FC = Rea }, [mediaPlaybackRate]) useEffect(() => { - const graph = graphRef.current - if (!graph) return const clampedVolume = muted ? 0 : Math.max(0, finalVolume) - rampPreviewClipGain(graph, clampedVolume) + const graph = graphRef.current + if (graph) { + rampPreviewClipGain(graph, clampedVolume) + return + } + // Direct path (cross-origin source): drive the element itself. + const audio = audioRef.current + if (!audio) return + audio.muted = muted + audio.volume = Math.min(1, clampedVolume) }, [finalVolume, muted]) useEffect(() => { diff --git a/src/runtime/composition-runtime/components/video-audio-context.test.ts b/src/runtime/composition-runtime/components/video-audio-context.test.ts new file mode 100644 index 000000000..76ba6f6d5 --- /dev/null +++ b/src/runtime/composition-runtime/components/video-audio-context.test.ts @@ -0,0 +1,95 @@ +import { beforeEach, describe, expect, it, vi } from 'vite-plus/test' + +const previewGraphMocks = vi.hoisted(() => { + const sourceNode = { + connect: vi.fn(), + disconnect: vi.fn(), + } + const graph = { + context: { + state: 'running' as AudioContextState, + currentTime: 0, + resume: vi.fn(() => Promise.resolve()), + createMediaElementSource: vi.fn(() => sourceNode), + }, + sourceInputNode: {}, + outputGainNode: { gain: { value: 1 } }, + eqStageNodes: [], + dispose: vi.fn(), + } + return { + graph, + sourceNode, + createPreviewClipAudioGraph: vi.fn(() => graph), + rampPreviewClipEq: vi.fn(), + rampPreviewClipGain: vi.fn(), + setPreviewClipEq: vi.fn(), + setPreviewClipGain: vi.fn(), + } +}) + +vi.mock('../utils/preview-audio-graph', () => ({ + createPreviewClipAudioGraph: previewGraphMocks.createPreviewClipAudioGraph, + rampPreviewClipEq: previewGraphMocks.rampPreviewClipEq, + rampPreviewClipGain: previewGraphMocks.rampPreviewClipGain, + setPreviewClipEq: previewGraphMocks.setPreviewClipEq, + setPreviewClipGain: previewGraphMocks.setPreviewClipGain, +})) + +import { applyVideoElementAudioState } from './video-audio-context' + +function createVideoElement(src: string): HTMLVideoElement { + const element = document.createElement('video') + element.src = src + return element +} + +describe('applyVideoElementAudioState media-origin routing', () => { + beforeEach(() => { + vi.clearAllMocks() + previewGraphMocks.graph.context.state = 'running' + }) + + it('routes cross-origin sources direct to element.volume without the Web Audio graph', () => { + const video = createVideoElement('https://cdn.example.com/x.mp4') + + applyVideoElementAudioState(video, 0.8, []) + + expect(previewGraphMocks.createPreviewClipAudioGraph).not.toHaveBeenCalled() + expect(previewGraphMocks.graph.context.createMediaElementSource).not.toHaveBeenCalled() + expect(video.volume).toBeCloseTo(0.8) + expect(video.muted).toBe(false) + + // Subsequent volume changes propagate through the same direct path. + applyVideoElementAudioState(video, 0.35, []) + expect(video.volume).toBeCloseTo(0.35) + + // Muted playback maps to a zeroed element volume. + applyVideoElementAudioState(video, 0, []) + expect(video.volume).toBe(0) + expect(previewGraphMocks.graph.context.createMediaElementSource).not.toHaveBeenCalled() + }) + + it('keeps same-origin sources on the Web Audio graph', () => { + const video = createVideoElement(`${location.origin}/media/clip.mp4`) + + applyVideoElementAudioState(video, 0.8, []) + + expect(previewGraphMocks.createPreviewClipAudioGraph).toHaveBeenCalledTimes(1) + expect(previewGraphMocks.graph.context.createMediaElementSource).toHaveBeenCalledWith(video) + expect(previewGraphMocks.sourceNode.connect).toHaveBeenCalledWith( + previewGraphMocks.graph.sourceInputNode, + ) + expect(previewGraphMocks.rampPreviewClipGain).toHaveBeenCalled() + expect(video.volume).toBe(1) + }) + + it('keeps blob: sources on the Web Audio graph', () => { + const video = createVideoElement('blob:http://localhost:3000/uuid-1') + + applyVideoElementAudioState(video, 1, []) + + expect(previewGraphMocks.createPreviewClipAudioGraph).toHaveBeenCalledTimes(1) + expect(previewGraphMocks.graph.context.createMediaElementSource).toHaveBeenCalledWith(video) + }) +}) diff --git a/src/runtime/composition-runtime/components/video-audio-context.ts b/src/runtime/composition-runtime/components/video-audio-context.ts index 350e6d72f..19510a24b 100644 --- a/src/runtime/composition-runtime/components/video-audio-context.ts +++ b/src/runtime/composition-runtime/components/video-audio-context.ts @@ -26,6 +26,7 @@ import { setPreviewClipGain, type PreviewClipAudioGraph, } from '../utils/preview-audio-graph' +import { isWebAudioSafeMediaSource } from '../utils/media-source-origin' // Track video elements that have been connected to Web Audio API // A video element can only be connected to ONE MediaElementSourceNode ever @@ -60,6 +61,16 @@ export function applyVideoElementAudioState( return } + // Cross-origin media without CORS approval is silenced by + // MediaElementAudioSourceNode (HTML spec) — host-provided signed URLs take + // this direct element volume path instead of the Web Audio graph. EQ is + // graph-only and does not apply on this path. Called on every volume + // change, so element.volume stays in sync. + if (!isWebAudioSafeMediaSource(video.currentSrc || video.src)) { + video.volume = Math.min(1, safeVolume) + return + } + // Always route preview video audio through the shared clip graph so future // EQ/SFX can be inserted in one place for video and audio clips alike. try { diff --git a/src/runtime/composition-runtime/utils/media-source-origin.test.ts b/src/runtime/composition-runtime/utils/media-source-origin.test.ts new file mode 100644 index 000000000..2b1a3ac2f --- /dev/null +++ b/src/runtime/composition-runtime/utils/media-source-origin.test.ts @@ -0,0 +1,23 @@ +import { describe, expect, it } from 'vite-plus/test' +import { isWebAudioSafeMediaSource } from './media-source-origin' + +describe('isWebAudioSafeMediaSource', () => { + it('treats same-origin absolute and relative URLs as graph-safe', () => { + expect(isWebAudioSafeMediaSource(`${location.origin}/media/clip.mp4`)).toBe(true) + expect(isWebAudioSafeMediaSource('/media/clip.mp4')).toBe(true) + }) + + it('treats blob: and data: URLs as graph-safe', () => { + expect(isWebAudioSafeMediaSource('blob:http://localhost:3000/uuid-1')).toBe(true) + expect(isWebAudioSafeMediaSource('data:audio/mpeg;base64,AAAA')).toBe(true) + }) + + it('treats cross-origin URLs as unsafe for the Web Audio graph', () => { + expect(isWebAudioSafeMediaSource('https://cdn.example.com/x.mp4')).toBe(false) + expect(isWebAudioSafeMediaSource('https://signed.example.com/object?signature=1')).toBe(false) + }) + + it('treats empty and unparsable sources as graph-safe (legacy behavior)', () => { + expect(isWebAudioSafeMediaSource('')).toBe(true) + }) +}) diff --git a/src/runtime/composition-runtime/utils/media-source-origin.ts b/src/runtime/composition-runtime/utils/media-source-origin.ts new file mode 100644 index 000000000..c1eb333a0 --- /dev/null +++ b/src/runtime/composition-runtime/utils/media-source-origin.ts @@ -0,0 +1,19 @@ +/** + * Whether a media element source survives routing through + * MediaElementAudioSourceNode. Per the HTML spec, a media element whose + * resource is cross-origin and not CORS-approved outputs zeroes through the + * Web Audio graph, so cross-origin preview sources (e.g. host-provided + * signed URLs that lack Access-Control-Allow-Origin) must drive + * element.volume/muted directly instead. blob: and data: URLs inherit the + * embedding origin and are always graph-safe. + */ +export function isWebAudioSafeMediaSource(src: string): boolean { + if (!src) return true + if (src.startsWith('blob:') || src.startsWith('data:')) return true + if (typeof location === 'undefined') return true + try { + return new URL(src, location.href).origin === location.origin + } catch { + return true + } +}