Conversation
Replaces title-cased placeholder fragments in media.embeddedSubtitles, media.picker, media.subtitleScan, media.transcribe, and timeline track hints with real English copy, matching the structure used by other locales (including interpolation tokens).
Match the Pen Tool and Path Edit toolbar height to the combined alignment + playback row height (60px) so the preview canvas no longer reflows when toggling edit modes.
items-store.ts grew to 1746 lines mixing the Zustand store with audio EQ normalization, frame-field rounding, derived index builders, and subtitle cue helpers. Extract: - items-store-normalize.ts: normalizeFrameFields, normalizeItemUpdates, normalizeTrack, subtitle cue trim helpers, roundFrame helpers. - items-store-indexes.ts: buildItemsByTrackId, buildItemById, buildLinkedItemsByItemId, withItemIndexes, buildRippleShiftByItemId, getTransitionLinkedIds, and supporting predicates. items-store.ts (now 897 lines) contains the store factory plus its media-dependency and in/out-point side-effect subscriptions. No behavior or public API changes — useItemsStore remains the only export and is imported unchanged by all 65 consumers.
normalizeFrameFields and normalizeItemUpdates each repeated ~30 nearly identical conditional clamps for the 9 EQ bands plus audio fades, pitch, and frame fields. Replace the duplicated code with EQ_BANDS + a single OPTIONAL_FIELD_CLAMPS table, then iterate it once per call. Adding a new band or audio field is now a single entry instead of two parallel edits. items-store-normalize.ts: 533 -> 347 lines.
… items TimelineItem subscribed to several preview stores 3-7 times each, once per field. Each subscription is a separate Zustand listener that re-runs on every store mutation (slide drag, rolling edit, effect drop drag fire many per second). Collapse the multi-field reads into single useShallow snapshots: - Slide preview: 7 -> 1 (isPrimary / isLeftNeighbor / isRightNeighbor / slideDelta / minDelta / maxDelta / primary-side neighbor ids) - Rolling edit preview: 3 -> 1 (delta / handle / constrained) - Effect drop preview: 3 -> 1 (isSingle / isMulti / hoveredMultiCount) - Media library: drop a separate mediaFileName subscription that did its own mediaItems.find() scan; derive it from the already-subscribed mediaForItem record. Eliminates one O(n) array scan per render. Net: 46 -> 35 store subscriptions per TimelineItem. isLinkedSlideCompanion moves from a slide-store selector (which read items-store inside) to a useMemo over the consolidated slidePreview snapshot - same firing cadence, one fewer subscription. No behavior change; full timeline test suite (648/648) and lint pass.
The recent refactor that collapsed audioEq band clamping into a data-driven table had no dedicated tests — only six EQ/fade/crop mentions across 803 lines of items-store.test.ts. A typo in the EQ_BANDS table (wrong freq range, missing hasGain, wrong qDefault) would slip through. Add focused unit tests for items-store-normalize covering: - roundFrame / roundDuration / roundOptionalFrame / normalizeOptionalFps edge cases (NaN, infinity, negatives, fallback) - normalizeFrameFields: frame field rounding, source/trim rounding, legacy split sourceStart inference, EQ gain/Q/frequency clamping, cut slope snapping, enabled-flag boolean coercion, shape mask blend mode override - normalizeItemUpdates: defined-only mutation, legacy sourceEnd backfill, no spurious field insertion - normalizeTrack: volume clamp range, undefined passthrough - trimSubtitleCuesAtStart / End: drop/truncate/rebase semantics 30 tests, all green.
…ler from TimelineItem Pull the caption/transcription surface (~165 lines) out of the TimelineItem god component into a dedicated hook + child component: - use-caption-dialog-state.ts owns the dialog state machine, transcript selectors, embedded-subtitle extraction, and per-cue consolidation. - transcribe-dialog-controller.tsx renders the TranscribeDialog from the hook output and is memo-gated by canManageCaptions. Host file shrinks from 3,794 to 3,588 lines and the caption logic is now isolated from the rest of the clip-render path.
Pull the four parallel fade-edit state machines (video fade, audio fade, audio fade curve, audio volume) plus their refs, commit effects, and ~500 lines of pointer handlers out of TimelineItem into a single hook. The audio volume CSS-variable preview stays imperative (no React state on the drag hot path), and all four state machines preserve their isCommitting cycle that auto-clears once persisted values match the preview. index.tsx shrinks from 3,588 to ~2,800 lines.
Move the ~180 lines of fade-ratio, curve-point, curve-path, and audio volume visualization derivations from TimelineItem into a pure-math hook that takes the displayed* values from useFadeEditors plus a few other primitives. The hook is gated by skipFadeComputation so narrow inactive clips still skip the expensive curve-path computations. index.tsx shrinks by another ~120 lines.
… useEditPreviewShifts Move the six per-item preview-store subscriptions (linked-edit, rolling, ripple, track-push, slip, slide) plus the slide neighbor lookups into a single hook. The subscriptions still use the same useShallow selectors, so per-clip re-renders only fire when this item's participation in the active gesture changes — same as before — but the timeline-item host file no longer has 200 lines of inline preview-store boilerplate. index.tsx shrinks by another ~190 lines.
…mBounds Move the leftFrame/rightFrame computation, the contentPreviewItem derivation chain (trim/roll/slide/slip with source-frame adjustments), and the visualLeftFrame/visualWidthFrames calculation into a single hook that takes the preview shifts and trim/stretch state. Source-frame conversions, slide-continuity math, and isCompactWidth calculation all live in the hook now.
…over Move hoveredEdge / smartTrimIntent / smartBodyIntent state machines, the activeTool-change reset effect, the mouse-move intent resolution, and handleMouseLeave into a hook scoped to the TimelineItem. The smartTrimIntentRef is still exposed so pointer handlers (handleClick, handleMouseDown, handleSmartTrimStart) can read the latest intent at gesture-start without re-render churn.
Move closerEdge state + handleContextMenu out of TimelineItem into a small focused hook. The larger pointer handlers (handleClick, handleMouseDown, handleDoubleClick, handleSmartTrimStart) stay in the host file because they are tightly coupled to local trim/stretch/slip state and consume too many host-level dependencies to extract without introducing churn in their dep arrays. Pull-out is intentionally minimal here: the bigger sub-component split in the next change does the rest of the de-coupling work.
…'d children Extract the active edge halos block and the transition drop ghost into small memo'd sub-components. Both take narrow, primitive props and will now skip render work when their state hasn't changed. The rest of the JSX (audio/video fade overlays, ClipContent, ClipIndicators, trim/stretch/fade/volume handles) is still inline in the host file — those sub-component splits are best done together with their handler hooks and are left as follow-up work.
- Add shared DEFAULT_PROJECT_WIDTH/HEIGHT/FPS constants and replace hardcoded 1920/1080/30 fallbacks across timeline, editor, export, media-library, projects, and runtime modules - Extract FULLSCREEN_QUAD_WGSL into gpu-shared and dedupe the fullscreen-quad vertex shader across 6 GPU pipeline modules - Extract filmstrip cache config constants and FilmstripMetricsAccumulator into sibling modules (filmstrip-cache: 2725 -> 2603 lines) - Extract useDopesheetViewport and useElementSize hooks from dopesheet-editor (index.tsx: 3028 -> 2955 lines) - Expand gpu-transitions and gpu-effects test coverage with registry contract, alignment, and defensive-input checks (16 -> 30 tests)
…tion
- use-caption-dialog-state: stop gating canExtractEmbeddedSubtitles on
canManageCaptions so linked audio (MKV/WebM) regains the Extract
Embedded Subtitles menu entry
- use-edit-preview-shifts: zero slideDelta for items unrelated to the
active slide so useShallow comparison passes across mousemove ticks,
eliminating an O(N) per-frame re-render storm during slide drags
- use-element-size: stop resetting size to {0,0} on disable so the
graph pane keeps its last observed dimensions across visualization-
mode toggles, avoiding a zero-sized flash on re-enter
Three cleanup findings from the prior audit that were initially deferred: - Extract the OPTIONAL_FIELD_CLAMPS table to shared/timeline/item-clamps so the runtime items-store normalizer and the project-load migration share one source of truth; collapses ~150 lines of repeated if-clamp branches in migrations/normalize.ts. - Move replaceableCaptionClipIds out of withItemIndexes' hot path: it now rebuilds lazily via selectReplaceableCaptionClipIds, keyed on items identity, so per-frame drag mutations skip the O(N) legacy-caption detection pass. - Drop the redundant top-level xxxRef.current = xxx writes in use-smart-trim-hover; the syncXxx helpers were already updating the refs, the duplicate writes only existed to catch the tool-change useEffect's direct setX calls which now route through syncXxx.
- Replace `mediaItems.find(...)` lookups with O(1) `mediaById[id]` across runtime, timeline, preview, and media-library call sites (incl. two playback hot paths in composition-runtime). - preview-area: use precomputed `useItemsStore.maxItemEndFrame` instead of looping all items inside a Zustand selector on every store update. - timeline-track: replace per-row `resolveEffectiveTrackStates(s.tracks)` call with a direct O(1) parent-group lookup so visible tracks don't do O(T^2) work on every store mutation. - inline-composition-preview: drop the redundant `compositionById` subscription and read it lazily inside the resolution effect so unrelated composition edits no longer re-trigger media URL resolution. - media-resolver: swap `JSON.parse(JSON.stringify(tracks))` for `structuredClone(tracks)`. - use-preview-view-model: count proxy "ready" statuses in `useMemo` keyed on `proxyStatus` instead of looping inside the selector on every media-library mutation. Test mocks updated to expose `mediaById` alongside `mediaItems`.
… jank ClipContent drove its filmstrip/waveform tile grid from the live per-frame pixelsPerSecond, so it re-rendered on every wheel/momentum frame during a zoom gesture. Profiling a prod build (21 visible clips) showed this was ~73% of zoom cost — ~2,388 ClipContent renders per gesture, ~104ms/frame (~10fps). Switch ClipContent to contentPixelsPerSecond (settled, updates ~100ms after the gesture ends). The clip shell still resizes smoothly mid-gesture via the --timeline-px-per-frame CSS variable, and the existing cover-frame background / overflow clipping hide the transient scale mismatch until it snaps sharp on settle. preferImmediateRendering opts back into live pps for active edit previews (trim/slide) where settle lag would distract — matching the intent the existing tests already documented. Result: ~2,388 -> 60 ClipContent renders per gesture, ~5,200ms -> ~1,400ms for a 50-frame zoom (~10fps -> ~33fps). Also adds User Timing instrumentation used to find this: withPerfMeasure wraps the timeline RAF loops and mutation execute()/applyTransitionRepairs as tl.raf.* / tl.action.* entries; perfMarkRender adds opt-in tl.render.* marks (gated on window.__TL_RENDER_MARKS__); __DEBUG__.perfSummary()/perfClear() aggregate them.
…m-out lag Zoom-out was ~5x slower than zoom-in (226ms vs 40ms per frame with 21 clips). Render marks ruled out React render count; the cost is that zooming out grows the visible-clip count, and mounting each newly-visible clip's filmstrip tile grid + waveform (canvas draws) dominates. Disabling filmstrips dropped zoom-out to 19ms/frame, confirming the source. ClipContent now reads isZoomInteracting once at mount (via getState, not a reactive subscription) into deferVisual. A clip that first mounts mid-gesture renders only its colored shell — no filmstrip/waveform — until the zoom settles, then a one-shot subscription flips it on. Reading at mount rather than subscribing is essential: already-mounted clips must not re-render when the flag flips, or they would flash empty. Zoom-out: ~226ms/frame -> ~42ms/frame (clean runs 27-37ms). Clips appear as colored blocks during a fast zoom-out and fill with thumbnails on settle.
…de path audioBufferToWavBlob used a per-sample DataView.setInt16 loop (1129ms self on the main thread in a profile — the single biggest app function), and int16ToFloat32 divided per sample. Both run on the main thread during audio decode and froze the UI for ~2s. Write PCM through an Int16Array view over the data region (byte-identical output; browsers are little-endian, which WAV requires) and convert Int16-> Float32 via precomputed reciprocals.
The full preview-audio decode (mediabunny decode + downmix + downsample + Int16) ran on the main thread, freezing the UI for ~1-2s on import/first load. Move it into a worker that streams transferable Int16 bins back; the main thread only persists bins and wraps the assembled result in an AudioBuffer (cheap memcpy) — AudioBuffer can't cross the worker boundary. - audio-decode-dsp.ts: pure, AudioBuffer-free DSP shared by both paths so worker and main-thread output stay byte-identical - audio-decode-worker.ts: mirrors the existing waveform-worker pattern (mediabunny, AC-3, createMediabunnyInputSource) - audio-decode-cache.ts: prefer the worker, fall back to the existing main-thread decode when Worker is unavailable (tests) or the worker errors The latency-sensitive partial/streaming playback path is unchanged (Phase 2). Playback correctness still needs manual verification in the running app.
The fast-start playback window decode (decodeAudioWindow) ran on the main thread, competing with audio scheduling and video rendering at the exact moment playback begins. Move it into the worker via a new decode-window message that returns a Float32 stereo slice (not persisted, so no Int16 quantization); the main thread only wraps it in an AudioBuffer. - Worker uses AudioSampleSink + getSample/samples(start,end) for the window (AudioBuffer/AudioBufferSink are unavailable in workers), mirroring the main-thread seek/dedup/coverage logic - Dedicated foreground worker lane so a window decode is never queued behind a slow background full decode on the same thread - decodeAudioWindowPreferWorker falls back to the existing main-thread decodeAudioWindow when Worker is unavailable (tests) or the worker errors; the outer timeout-race + full-decode fallback is unchanged Playback correctness still needs manual verification in the running app.
After Phase 1 the worker decoded off-thread but the main thread still wrote every bin to the workspace (the bulk of the ~950ms of storage I/O in the profile: getFileHandle, createWritable, write per bin x3 files). Hand the workspace FileSystemDirectoryHandle to the worker (it's structured-cloneable and keeps its permission grant) so the worker writes bins itself. - decoded-preview-audio.ts: extract writeDecodedPreviewAudioToRoot(root, data) so the worker reuses the exact path/format logic; saveDecodedPreviewAudio delegates with requireWorkspaceRoot() - worker writes each bin before transferring it back (transfer neuters the buffer, so the write must read it first), then streams it for AudioBuffer assembly - main thread skips per-bin saves when it handed over the handle; the tiny meta JSON and the conform WAV stay on the main thread - falls back to main-thread persistence when no workspace root is available (e.g. tests) or the worker errors Playback + on-disk cache correctness still needs manual verification in app.
When an AC-3 clip scrolled back into view, startPreviewAudioConform always ran ensureDecodeStarted (a ~800ms loadFromBins Int16->Float32 rebuild on the main thread) and re-encoded the WAV, even when the conform asset was already persisted. LoAF profiling showed these as ~800ms blocking Promise.resolve storms during scroll. - Bail out of startPreviewAudioConform before the decode when the asset is already conformed (isPreviewAudioConformed guard). - Skip already-conformed assets in persistConform; fixes a latent double WAV encode in the old path. - Encode the conform WAV directly from the persisted Int16 bins (int16StereoToWavBlob) instead of round-tripping Int16->Float32->Int16. - int16ToFloat32Into writes straight into the channel buffer, dropping a throwaway Float32Array allocation and an extra copy per bin at all three AudioBuffer assembly sites. Verified live: re-scroll blocking dropped from ~1616ms to ~150ms.
loadFromDisk only short-circuited on isComplete, so an incomplete (long-video) filmstrip re-read every persisted tile from OPFS and re-minted ~230 object URLs on every scroll remount. Live profiling showed ~230 getFile calls per scroll pass purely from this path. Skip loadFromDisk when the singleton in-memory cache already holds frames for the media. The live extraction path keeps that cache current via notifyUpdate, and a fully-evicted entry (which is the only path that revokes the object URLs, and only runs with no subscribers) resets the in-memory frames to empty so a genuine reload still runs when needed. Verified live: filmstrip getFile reads during a re-scroll dropped from ~230 to 0, with no broken tiles.
loadFromBins (the cache-hit path) dequantized persisted Int16 bins into an AudioBuffer synchronously on the main thread — a ~900ms freeze that recurs whenever a clip scrolls back into view. Move the Int16->Float32 assembly to the existing decode worker via a new assemble-bins message (reusing the shared int16ToFloat32Into DSP for byte-identical output), then copyToChannel the result. Bin buffers are copied rather than transferred so a worker failure falls back to the original synchronous loop.
Two layout-thrash sources surfaced in a DevTools trace, both firing during React commits on scroll/zoom frames: - use-fade-editors read getBoundingClientRect per audio clip inside a useLayoutEffect to pixel-snap the volume line. Many clips mounting at once (zoom-out) meant one forced reflow each, on the commit critical path. The line now carries a `top: var(--timeline-audio-volume-line-y)` CSS fallback so it paints in the right place without JS, and the snap moves to a post-paint useEffect (refinement only, no flash). - syncViewportFromContainer read clientWidth/clientHeight every scroll/zoom frame even though both are invariant under scroll and horizontal zoom. Cache them, refreshed by a ResizeObserver on the container + tracks wrapper, with a live-read fallback until the first measurement so culling can't regress.
…xing mixCompoundClipWaveformPeaks runs Math.hypot per output sample. Peaks are normalized to [0,1] so there is no overflow to guard against, and Math.sqrt(a*a + b*b) is several times faster than V8's variadic Math.hypot.
…t-visibility The zoom re-anchor scrollLeft write forces a synchronous reflow whose cost scales with the interior layout (label, filmstrip, waveform SVG, fade overlays) of every mounted clip — including the ~2/3 that sit in the 2000px cull buffer but off-screen. Adding `content-visibility: auto` to the clip shell lets the browser skip laying out those off-screen interiors; the box stays correctly sized from inset-y + explicit left/width, so nothing collapses. Measured on a zoom-heavy trace: `set scrollLeft` self-time dropped from ~910ms to ~278ms (~68% less per zoom frame) with no regression elsewhere.
…r race - Gate withPerfMeasure behind window.__TL_PERF__ (off by default) so the User Timing buffer no longer grows unbounded in prod — measure entries were never cleared and ran on every timeline action + RAF frame. Matches the existing perfMarkRender gating. - ClipContent deferVisual now re-checks isZoomInteracting on effect attach, so a clip whose zoom gesture settles between the mount-time read and the subscription no longer stays stuck shell-only.
Bundle container/codec/quality/resolution into Maximum/Recommended/Balanced/Small presets so users don't need codec knowledge to pick good defaults. Presets keep the project aspect ratio (scale only) to avoid distorted output; manually changing any control flips the indicator to Custom. Localized across all 9 languages.
…e partials Split the 6.5k-line missing.json catch-all into export/media/preview partials and fold its editor/effects/timeline slices into the existing per-feature files. Since missing.json was the lowest-priority partial, only its keys that no other partial defined were carried over (shadowed keys were dead). The merged translation tree is byte-identical across all 9 languages. Also simplified the loader's partial ordering to plain alphabetical now that the missing-first fallback is gone.
…embed gating - Captions: add a default caption style preset setting (picker in Settings → AI); transcript and AI caption generation now apply it on creation. Move caption-style-presets to shared/typography so settings/media-library can use it within feature boundaries. - Media library: aggregate proxy/transcription progress bars expand to per-item rows (name + %); analysis cancel reads "Cancel all" when multiple items run. - Export: subtitle-embed toggle now disables with a note on containers that can't carry subtitles (MOV) instead of blocking the export.
Contributor
|
Too many files changed for review. ( |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds shared project defaults, perf-mark utilities and debug API, migrates media lookups to ChangesUnified Defaults, Perf, Media Map, and Audio Worker
Sequence DiagramsequenceDiagram
participant UI as Main Thread UI
participant Store as Stores
participant Worker as AudioDecodeWorker
participant FS as Workspace FS
UI->>Store: request decode/conform(mediaId)
UI->>Worker: decode (source + metadata)
Worker-->>UI: bin responses (Int16 buffers) & complete
Worker->>FS: persist bins
UI->>Worker: assemble-bins
Worker-->>UI: assembled Float32 buffers
UI->>FS: write WAV (int16 or AudioBuffer)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
src/features/preview/hooks/use-preview-view-model.ts (1)
80-87: ⚡ Quick winKeep
proxyReadyCountderived inside the selector.Line 80 subscribes to
proxyStatusby reference; if that map is ever mutated in place, Line 81–87 can keep a stale count. Deriving the count in the selector avoids that edge case and preserves update robustness.Suggested change
- const proxyStatus = useMediaLibraryStore((s) => s.proxyStatus) - const proxyReadyCount = useMemo(() => { - let count = 0 - for (const status of proxyStatus.values()) { - if (status === 'ready') count++ - } - return count - }, [proxyStatus]) + const proxyReadyCount = useMediaLibraryStore((s) => { + let count = 0 + for (const status of s.proxyStatus.values()) { + if (status === 'ready') count++ + } + return count + })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/preview/hooks/use-preview-view-model.ts` around lines 80 - 87, proxyReadyCount is computed from proxyStatus after subscribing to proxyStatus by reference, which can miss updates if the Map is mutated in place; move the derivation into the selector passed to useMediaLibraryStore so the hook subscribes to the derived value instead of the Map reference. Change the current useMemo + proxyStatus usage to a selector like useMediaLibraryStore(s => /* compute ready count from s.proxyStatus */) so the selector returns the count (proxyReadyCount) directly and the component only subscribes to that derived number.src/infrastructure/gpu-effects/index.test.ts (1)
161-168: ⚡ Quick winStrengthen the empty-category assertion.
This test currently validates only that the return type is an array, so it can pass even when the category is not actually empty. Assert the returned list is truly empty for a guaranteed-missing key.
♻️ Suggested test tightening
- it('returns empty list for an empty category', () => { + it('returns empty list for an unknown category', () => { // Categories with no registered effects (none today) should return [], // not throw. Pass a name that is in the union but lacks effects — if // none exists, any unknown key returns [] via nullish coalescing. // We assert the well-typed surface area here. - const result = getGpuEffectsByCategory('color') - expect(Array.isArray(result)).toBe(true) + const result = getGpuEffectsByCategory( + '__unknown_category__' as Parameters<typeof getGpuEffectsByCategory>[0], + ) + expect(result).toEqual([]) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/infrastructure/gpu-effects/index.test.ts` around lines 161 - 168, Update the test that calls getGpuEffectsByCategory('color') to assert the array is actually empty rather than only checking its type: replace or augment the Array.isArray check with a strict emptiness assertion such as expect(result).toHaveLength(0) or expect(result).toEqual([]) so the test fails if any effects are returned from getGpuEffectsByCategory('color').src/runtime/composition-runtime/utils/audio-decode-dsp.test.ts (1)
89-105: 💤 Low valueTest assertion may not match actual conversion behavior.
Line 104 uses
Math.trunc(0.5 * 0x7fff)butfloat32ToInt16doesn't explicitly truncate—it assigns directly toInt16Arraywhich performs ToInt16. For positive values this is effectively truncation, but the test comment could be clearer about why this works.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/runtime/composition-runtime/utils/audio-decode-dsp.test.ts` around lines 89 - 105, The test's expected value uses Math.trunc(0.5 * 0x7fff) but the implementation converts floats by assigning into an Int16Array (which performs JS ToInt16 semantics); update the assertion to compute the expected value using the same conversion pipeline as the code under test—for example, replace Math.trunc(...) with a conversion that mirrors assignment into an Int16Array (e.g., new Int16Array([0.5 * 0x7fff])[0]) so the test for produceDecodedBin's left channel uses the identical conversion as float-to-int16 conversion in the runtime.src/runtime/composition-runtime/utils/audio-decode-cache.ts (1)
674-753: ⚖️ Poor tradeoffEvent listener cleanup is tied to message receipt but not to promise rejection paths.
If
finalizeDecodedAudiothrows aftercleanup()is called (line 717), the rejection propagates correctly. However, if the worker never responds (e.g., worker crashes silently without firingerrorevent), the promise will hang indefinitely and listeners remain attached.Consider adding a timeout or using
AbortControllerfor long-running decode operations, or at minimum document this as a known limitation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/runtime/composition-runtime/utils/audio-decode-cache.ts` around lines 674 - 753, The promise in decodeFullAudioViaWorker can hang and leave listeners attached if the worker never responds; add a cancellation/timeout path that calls cleanup(), rejects the promise, and stops further processing. Modify decodeFullAudioViaWorker to accept an optional AbortSignal or create an internal timeout timer that (a) removes the onMessage/onError listeners via cleanup(), (b) rejects with a descriptive Error (e.g., "Audio decode timed out" or "aborted"), and (c) aborts any outstanding persistPromises; ensure onMessage and onError clear the timer/abort handler when they complete and keep the existing finalizeDecodedAudio try/catch behavior. Use the function names requestId, onMessage, onError, cleanup, finalizeDecodedAudio and variables persistPromises/persistedBins to locate where to wire the timeout/AbortController logic.src/runtime/composition-runtime/utils/audio-decode-dsp.ts (1)
109-113: 💤 Low valueDead code: LFE branch can never execute.
lfeGainis hardcoded to0at line 84, so the conditionlfeGain !== 0 && LFEis always false. The block at lines 110-113 is unreachable. Consider removing this dead code or adding a comment explaining it's preserved for potential future configurability.♻️ Suggested cleanup
- if (lfeGain !== 0 && LFE) { - const lfe = LFE[i]! * lfeGain - l += lfe - r += lfe - } + // LFE channel (index 3) intentionally discarded for preview — adds mud + // without improving intelligibility at preview sample rates.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/runtime/composition-runtime/utils/audio-decode-dsp.ts` around lines 109 - 113, The branch guarded by lfeGain and LFE in audio-decode-dsp.ts is dead because lfeGain is initialized to 0 (making "if (lfeGain !== 0 && LFE)" always false); either remove the unreachable block that adds LFE to l and r (the `if (lfeGain !== 0 && LFE) { const lfe = LFE[i]! * lfeGain; l += lfe; r += lfe; }` section) or make lfeGain configurable (expose it as a parameter/option and use that value here) — if you choose to keep the code for future use, add a clear comment/TODO next to the lfeGain/LFE usage explaining why it's preserved and under what configuration it will be enabled.src/shared/logging/perf-marks.ts (1)
63-80: ⚡ Quick winFix nested
withPerfMeasuretiming collisions by uniquifying mark names
withPerfMeasureuses${name}:s/${name}:eand then callsperformance.clearMarks(startMark)/clearMarks(endMark). SinceclearMarks(markName)removes allPerformanceMarkentries matching that name, nested/re-entrant calls using the samenamecan clear the outer call’s marks before the outerperformance.measure(...)runs, dropping/mangling the outer measurement.♻️ Proposed fix
+let perfMeasureSeq = 0 + export function withPerfMeasure<T>(name: string, fn: () => T): T { if (!HAS_PERF || !(globalThis as { __TL_PERF__?: boolean }).__TL_PERF__) return fn() - const startMark = `${name}:s` - const endMark = `${name}:e` + const runId = `${name}:${++perfMeasureSeq}` + const startMark = `${runId}:s` + const endMark = `${runId}:e` try { performance.mark(startMark) } catch { return fn() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/shared/logging/perf-marks.ts` around lines 63 - 80, withPerfMeasure currently uses non-unique mark names (`${name}:s` / `${name}:e`) which lets nested calls clobber outer marks; change it to generate unique start/end mark names per invocation (e.g., append a small unique suffix from a module-scoped counter or crypto.randomUUID()) so each call uses `const startMark = `${name}:s:${unique}`` and `const endMark = `${name}:e:${unique}``; use those unique names in performance.mark, performance.measure, and performance.clearMarks so inner calls cannot remove outer marks; keep the measurement name (the first arg to performance.measure) as the original `name` if desired.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/features/timeline/components/timeline-item/transcribe-dialog-controller.tsx`:
- Around line 73-83: The onStart handler calls markCaptionStarted() then invokes
onGenerate(...) but doesn't guard against synchronous throws, leaving caption
state open and no error shown; wrap the onGenerate(...) call in a try/catch so
any synchronous exception triggers markCaptionEnded() and sets
setDialogError(...) (using isTranscriptionOutOfMemoryError and
TRANSCRIPTION_OOM_HINT the same way as the async callback), while preserving the
existing onGenerate callback that also calls markCaptionEnded() and
setDialogError(...) for async failures.
In `@src/features/timeline/components/timeline-item/use-context-menu-state.ts`:
- Around line 27-43: The current isCurrentSelection uses targetIds.some(...) so
any partial overlap prevents selection updates; change the membership check to
require full containment by using targetIds.every(id =>
selectedItemIds.includes(id)) (i.e., update the isCurrentSelection definition).
Keep the rest of the logic (selectItems, expandSelectionWithLinkedItems, items,
selectedItemIds, item.id) unchanged so only fully-contained target sets are
treated as current selection.
In `@src/features/timeline/components/timeline-track.tsx`:
- Around line 294-306: Replace the inline parent-group logic in the
useTimelineStore selector for trackInteractionState with a call to the shared
resolver: import and call resolveEffectiveTrackStates (from group-utils.ts)
passing the current track and the store's tracks collection to obtain the
canonical effective track object, then compute the same bitmask using
(effective.locked ? 1 : 0) | (getIsTrackDisabled(effective) ? 2 : 0); keep this
inside the same useTimelineStore selector (trackInteractionState) and retain
getIsTrackDisabled usage so group gating behavior always flows through
resolveEffectiveTrackStates.
In `@src/i18n/locales/partials/preview.json`:
- Line 279: The German translation for the key "frameSaved" mixes a German
opening quote („) with an escaped ASCII closing quote (\")—update the value for
"frameSaved" to use a consistent quote style (either both German typographic
quotes „…“ or both ASCII quotes "…") so it matches the rest of the locale
strings; modify the string in src/i18n/locales/partials/preview.json at the
"frameSaved" key accordingly.
---
Nitpick comments:
In `@src/features/preview/hooks/use-preview-view-model.ts`:
- Around line 80-87: proxyReadyCount is computed from proxyStatus after
subscribing to proxyStatus by reference, which can miss updates if the Map is
mutated in place; move the derivation into the selector passed to
useMediaLibraryStore so the hook subscribes to the derived value instead of the
Map reference. Change the current useMemo + proxyStatus usage to a selector like
useMediaLibraryStore(s => /* compute ready count from s.proxyStatus */) so the
selector returns the count (proxyReadyCount) directly and the component only
subscribes to that derived number.
In `@src/infrastructure/gpu-effects/index.test.ts`:
- Around line 161-168: Update the test that calls
getGpuEffectsByCategory('color') to assert the array is actually empty rather
than only checking its type: replace or augment the Array.isArray check with a
strict emptiness assertion such as expect(result).toHaveLength(0) or
expect(result).toEqual([]) so the test fails if any effects are returned from
getGpuEffectsByCategory('color').
In `@src/runtime/composition-runtime/utils/audio-decode-cache.ts`:
- Around line 674-753: The promise in decodeFullAudioViaWorker can hang and
leave listeners attached if the worker never responds; add a
cancellation/timeout path that calls cleanup(), rejects the promise, and stops
further processing. Modify decodeFullAudioViaWorker to accept an optional
AbortSignal or create an internal timeout timer that (a) removes the
onMessage/onError listeners via cleanup(), (b) rejects with a descriptive Error
(e.g., "Audio decode timed out" or "aborted"), and (c) aborts any outstanding
persistPromises; ensure onMessage and onError clear the timer/abort handler when
they complete and keep the existing finalizeDecodedAudio try/catch behavior. Use
the function names requestId, onMessage, onError, cleanup, finalizeDecodedAudio
and variables persistPromises/persistedBins to locate where to wire the
timeout/AbortController logic.
In `@src/runtime/composition-runtime/utils/audio-decode-dsp.test.ts`:
- Around line 89-105: The test's expected value uses Math.trunc(0.5 * 0x7fff)
but the implementation converts floats by assigning into an Int16Array (which
performs JS ToInt16 semantics); update the assertion to compute the expected
value using the same conversion pipeline as the code under test—for example,
replace Math.trunc(...) with a conversion that mirrors assignment into an
Int16Array (e.g., new Int16Array([0.5 * 0x7fff])[0]) so the test for
produceDecodedBin's left channel uses the identical conversion as float-to-int16
conversion in the runtime.
In `@src/runtime/composition-runtime/utils/audio-decode-dsp.ts`:
- Around line 109-113: The branch guarded by lfeGain and LFE in
audio-decode-dsp.ts is dead because lfeGain is initialized to 0 (making "if
(lfeGain !== 0 && LFE)" always false); either remove the unreachable block that
adds LFE to l and r (the `if (lfeGain !== 0 && LFE) { const lfe = LFE[i]! *
lfeGain; l += lfe; r += lfe; }` section) or make lfeGain configurable (expose it
as a parameter/option and use that value here) — if you choose to keep the code
for future use, add a clear comment/TODO next to the lfeGain/LFE usage
explaining why it's preserved and under what configuration it will be enabled.
In `@src/shared/logging/perf-marks.ts`:
- Around line 63-80: withPerfMeasure currently uses non-unique mark names
(`${name}:s` / `${name}:e`) which lets nested calls clobber outer marks; change
it to generate unique start/end mark names per invocation (e.g., append a small
unique suffix from a module-scoped counter or crypto.randomUUID()) so each call
uses `const startMark = `${name}:s:${unique}`` and `const endMark =
`${name}:e:${unique}``; use those unique names in performance.mark,
performance.measure, and performance.clearMarks so inner calls cannot remove
outer marks; keep the measurement name (the first arg to performance.measure) as
the original `name` if desired.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9bf91648-acb4-4ec5-9cfd-5c2c5152745d
📒 Files selected for processing (122)
.gitignoreCLAUDE.mdsrc/app/debug/project-debug.tssrc/features/editor/components/media-sidebar.tsxsrc/features/editor/components/preview-area.tsxsrc/features/editor/components/properties-sidebar/canvas-panel/index.tsxsrc/features/editor/components/properties-sidebar/clip-panel/caption-style-controls.tsxsrc/features/editor/components/properties-sidebar/clip-panel/index.tsxsrc/features/editor/components/properties-sidebar/clip-panel/layout-section.tsxsrc/features/editor/components/properties-sidebar/clip-panel/subtitle-section.tsxsrc/features/editor/components/properties-sidebar/clip-panel/video-section.tsxsrc/features/editor/components/settings-dialog.tsxsrc/features/export/components/export-dialog.tsxsrc/features/export/hooks/use-client-render.tssrc/features/export/utils/canvas-render-orchestrator.tssrc/features/export/utils/client-renderer.tssrc/features/export/utils/shared-video-extractor.tssrc/features/keyframes/components/dopesheet-editor/index.tsxsrc/features/keyframes/components/dopesheet-editor/use-dopesheet-viewport.tssrc/features/keyframes/components/dopesheet-editor/use-element-size.tssrc/features/media-library/components/background-task-progress.tsxsrc/features/media-library/components/media-card.tsxsrc/features/media-library/components/media-library.tsxsrc/features/media-library/services/media-analysis-service.tssrc/features/media-library/services/media-captioning-service.tssrc/features/media-library/services/media-transcription-service.tssrc/features/media-library/services/subtitle-sidecar-service.tssrc/features/media-library/utils/caption-items.tssrc/features/media-library/utils/media-resolver.tssrc/features/media-library/workers/media-processor.worker.tssrc/features/preview/components/inline-composition-preview.tsxsrc/features/preview/components/source-monitor.test.tsxsrc/features/preview/components/source-monitor.tsxsrc/features/preview/hooks/use-canvas-media-drop.tssrc/features/preview/hooks/use-preview-view-model.tssrc/features/projects/components/project-card.tsxsrc/features/projects/utils/validation.tssrc/features/settings/stores/settings-store.tssrc/features/timeline/components/bento-layout-dialog.tsxsrc/features/timeline/components/keyframe-graph-panel.tsxsrc/features/timeline/components/timeline-content.tsxsrc/features/timeline/components/timeline-item/clip-content.test.tsxsrc/features/timeline/components/timeline-item/clip-content.tsxsrc/features/timeline/components/timeline-item/edge-halos.tsxsrc/features/timeline/components/timeline-item/index.tsxsrc/features/timeline/components/timeline-item/transcribe-dialog-controller.tsxsrc/features/timeline/components/timeline-item/transition-drop-ghost.tsxsrc/features/timeline/components/timeline-item/use-caption-dialog-state.tssrc/features/timeline/components/timeline-item/use-context-menu-state.tssrc/features/timeline/components/timeline-item/use-edit-preview-shifts.tssrc/features/timeline/components/timeline-item/use-fade-editors.tssrc/features/timeline/components/timeline-item/use-fade-math.tssrc/features/timeline/components/timeline-item/use-smart-trim-hover.tssrc/features/timeline/components/timeline-item/use-timeline-item-actions.tssrc/features/timeline/components/timeline-item/use-timeline-item-bounds.tssrc/features/timeline/components/timeline-markers.tsxsrc/features/timeline/components/timeline-media-drop-zone.tsxsrc/features/timeline/components/timeline-playhead.tsxsrc/features/timeline/components/timeline-track.tsxsrc/features/timeline/components/transition-item.tsxsrc/features/timeline/hooks/use-filmstrip.tssrc/features/timeline/services/filmstrip-cache-config.tssrc/features/timeline/services/filmstrip-cache-metrics.tssrc/features/timeline/services/filmstrip-cache.tssrc/features/timeline/services/reverse-conform-service.tssrc/features/timeline/stores/actions/composition-actions.tssrc/features/timeline/stores/actions/edit/freeze-frame-actions.tssrc/features/timeline/stores/actions/shared.tssrc/features/timeline/stores/actions/source-edit-actions.tssrc/features/timeline/stores/items-store-indexes.tssrc/features/timeline/stores/items-store-normalize.test.tssrc/features/timeline/stores/items-store-normalize.tssrc/features/timeline/stores/items-store.test.tssrc/features/timeline/stores/items-store.tssrc/features/timeline/stores/timeline-persistence.tssrc/features/timeline/utils/compound-clip-waveform.tssrc/i18n/index.tssrc/i18n/locales/de.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/ko.jsonsrc/i18n/locales/partials/editor.jsonsrc/i18n/locales/partials/effects.jsonsrc/i18n/locales/partials/export.jsonsrc/i18n/locales/partials/media.jsonsrc/i18n/locales/partials/missing.jsonsrc/i18n/locales/partials/preview.jsonsrc/i18n/locales/partials/timeline.jsonsrc/i18n/locales/pt-BR.jsonsrc/i18n/locales/tr.jsonsrc/i18n/locales/zh.jsonsrc/infrastructure/gpu-compositor/compositor-pipeline.tssrc/infrastructure/gpu-effects/common.tssrc/infrastructure/gpu-effects/effects-pipeline.tssrc/infrastructure/gpu-effects/index.test.tssrc/infrastructure/gpu-masks/mask-combine-pipeline.tssrc/infrastructure/gpu-media/media-blend-pipeline.tssrc/infrastructure/gpu-media/media-render-pipeline.tssrc/infrastructure/gpu-shapes/shape-render-pipeline.tssrc/infrastructure/gpu-shared/fullscreen-quad.tssrc/infrastructure/gpu-transitions/common.tssrc/infrastructure/gpu-transitions/index.test.tssrc/infrastructure/storage/workspace-fs/decoded-preview-audio.tssrc/runtime/composition-runtime/components/item.tsxsrc/runtime/composition-runtime/components/stable-video-sequence.test.tsxsrc/runtime/composition-runtime/components/stable-video-sequence.tsxsrc/runtime/composition-runtime/components/text-content.tsxsrc/runtime/composition-runtime/utils/audio-buffer-wav.tssrc/runtime/composition-runtime/utils/audio-decode-cache.test.tssrc/runtime/composition-runtime/utils/audio-decode-cache.tssrc/runtime/composition-runtime/utils/audio-decode-dsp.test.tssrc/runtime/composition-runtime/utils/audio-decode-dsp.tssrc/runtime/composition-runtime/utils/audio-decode-worker.tssrc/runtime/composition-runtime/utils/audio-decode-worker.types.tssrc/runtime/composition-runtime/utils/preview-audio-conform.tssrc/shared/logging/perf-marks.tssrc/shared/projects/defaults.tssrc/shared/projects/migrations/normalize.tssrc/shared/timeline/item-clamps.tssrc/shared/typography/caption-style-presets.ts
Comment on lines
+294
to
306
| // Direct parent-group lookup keeps this O(1); calling resolveEffectiveTrackStates | ||
| // here would scan the whole tracks array per row on every store mutation. | ||
| const trackInteractionState = useTimelineStore((s) => { | ||
| const effective = resolveEffectiveTrackStates(s.tracks).find((t) => t.id === track.id) ?? track | ||
| const parentGroup = track.parentTrackId | ||
| ? s.tracks.find((t) => t.id === track.parentTrackId && t.isGroup) | ||
| : undefined | ||
| const effectiveLocked = track.locked || parentGroup?.locked || false | ||
| const effectiveMuted = track.muted || parentGroup?.muted || false | ||
| const effectiveVisible = track.visible !== false && parentGroup?.visible !== false | ||
| const effective: TimelineTrackType = parentGroup | ||
| ? { ...track, locked: effectiveLocked, muted: effectiveMuted, visible: effectiveVisible } | ||
| : track | ||
| return (effective.locked ? 1 : 0) | (getIsTrackDisabled(effective) ? 2 : 0) |
Contributor
There was a problem hiding this comment.
Use resolveEffectiveTrackStates() for group gating instead of inline logic.
This block bypasses the shared propagation path and can drift from canonical mute/visible/locked behavior for child tracks. Please route effective state resolution through group-utils.ts here.
As per coding guidelines, "Track groups are 1-level only (no nested groups). Gate behavior (mute/visible/locked) propagates from group to children via resolveEffectiveTrackStates() in group-utils.ts."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/timeline/components/timeline-track.tsx` around lines 294 - 306,
Replace the inline parent-group logic in the useTimelineStore selector for
trackInteractionState with a call to the shared resolver: import and call
resolveEffectiveTrackStates (from group-utils.ts) passing the current track and
the store's tracks collection to obtain the canonical effective track object,
then compute the same bitmask using (effective.locked ? 1 : 0) |
(getIsTrackDisabled(effective) ? 2 : 0); keep this inside the same
useTimelineStore selector (trackInteractionState) and retain getIsTrackDisabled
usage so group gating behavior always flows through resolveEffectiveTrackStates.
- transcribe dialog: catch synchronous onGenerate throws so caption state resets and an OOM-aware error surfaces like the async path - context menu: require full containment (every) so a partially-selected linked set expands to the full set on right-click - i18n: use matching German typographic quotes in preview frameSaved - audio-decode test: mirror Int16Array conversion instead of Math.trunc - perf-marks: unique start/end mark names so nested calls don't clobber
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rolls up 36 commits from
developintostaging. Spans new editor features, an export quality-preset workflow, a large audio/timeline performance pass, and a TimelineItem hook-composition refactor.Features
Performance
Math.sqrtoverMath.hypotin waveform mixingRefactors
Fixes
Test plan
Summary by CodeRabbit
New Features
Improvements
Localization