feat(editor): add waveform, audio mastering, and webcam crop - #344
feat(editor): add waveform, audio mastering, and webcam crop#344vitaligusatinsky wants to merge 2 commits into
Conversation
|
Warning Review limit reached
Next review available in: 19 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (19)
📝 WalkthroughWalkthroughThe change adds scene-level audio processing, authored webcam cropping, editor controls, synchronized audio preview, and media-stage timeline insertion across the Rust compositor and TypeScript editor. ChangesScene media controls
Media timeline insertion
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant EditorSettings
participant SceneDescription
participant CompositorPreview
participant AACExport
EditorSettings->>SceneDescription: serialize audio and webcam crop settings
SceneDescription->>CompositorPreview: provide scene media settings
CompositorPreview->>CompositorPreview: synchronize and process preview audio
SceneDescription->>AACExport: provide SceneAudio settings
AACExport->>AACExport: finalize PCM before AAC encoding
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
|
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (2)
src/components/ai-edition/VirtualPreview.audio.test.ts (1)
14-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for an unknown duration.
src/components/ai-edition/VirtualPreview.tsxpassesaudio.duration, which isNaNuntil the media metadata loads.resolveAudioPreviewTimehandles that through theNumber.isFinitefallback, but no test covers it. ANaNcase pins the "play while the duration is still unknown" behavior.Attribution: the coding guidelines require "Add a test for every new behavior in the same package as the code under test."
💚 Proposed additional test
it("stops instead of seeking past the track", () => { expect(resolveAudioPreviewTime(9.9, -160, 10)).toEqual({ targetTimeSec: 10, shouldPlay: false, }); }); + + it("plays while the duration is still unknown", () => { + expect(resolveAudioPreviewTime(1, 0, Number.NaN)).toEqual({ + targetTimeSec: 1, + shouldPlay: true, + }); + }); });🤖 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/components/ai-edition/VirtualPreview.audio.test.ts` around lines 14 - 19, Add a test case alongside the existing resolveAudioPreviewTime tests for an unknown duration represented by NaN, asserting the fallback behavior allows playback while metadata is unavailable. Use the existing test structure and resolveAudioPreviewTime symbol, without changing production logic.Source: Coding guidelines
crates/compositor/src/audio.rs (1)
120-135: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winShift the PCM in place to avoid a second full-length buffer.
shiftedallocates a complete copy of the PCM. For a long export the assembled PCM is already large (48 kHz × channels × duration), so this doubles peak audio memory for the whole finalization step.copy_withinplus a zero fill of the vacated region gives the same result without the extra allocation.♻️ Proposed in-place shift
- let mut shifted = vec![vec![0.0f32; samples]; pcm.len()]; if shift > 0 { let destination = (shift as usize).min(samples); let count = samples - destination; - for channel in 0..pcm.len() { - shifted[channel][destination..destination + count] - .copy_from_slice(&pcm[channel][..count]); + for channel in pcm.iter_mut() { + channel.copy_within(..count, destination); + channel[..destination].fill(0.0); } } else { let source = ((-shift) as usize).min(samples); let count = samples - source; - for channel in 0..pcm.len() { - shifted[channel][..count].copy_from_slice(&pcm[channel][source..source + count]); + for channel in pcm.iter_mut() { + channel.copy_within(source.., 0); + channel[count..].fill(0.0); } } - shifted + pcm🤖 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 `@crates/compositor/src/audio.rs` around lines 120 - 135, Update the PCM shift logic to operate directly on the existing pcm buffer instead of allocating shifted in the relevant audio-processing function. Use in-place slice movement such as copy_within for both shift directions, then zero-fill the vacated region, while preserving the current clamping and output behavior.
🤖 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/components/ai-edition/v4/MediaStage.tsx`:
- Around line 98-102: add colocated Vitest coverage for MediaStage’s
addSelectedToTimeline action, verifying the selected asset ID is forwarded to
onAddToTimeline and that no call occurs when selected is absent; if rendering
MediaStage, use the jsdom environment directive, otherwise keep the default Node
environment.
- Around line 98-101: Update addSelectedToTimeline so the success toast uses the
displayed asset name, falling back to basename(selected.originalPath) when
selected.label is empty, matching the media card’s existing name-resolution
behavior.
In `@src/components/ai-edition/VirtualPreview.tsx`:
- Around line 188-216: Update the WebAudio setup effect around audioGraphRef and
createMediaElementSource to cache one MediaElementAudioSourceNode per
HTMLAudioElement in a WeakMap and reuse it across effect reruns, including
StrictMode remounts. Ensure cached nodes remain connected to the active
processing graph without recreating them or leaving elements attached to closed
contexts; preserve the existing cleanup and fallback behavior.
- Around line 175-179: Update the preparePreviewAudioTrack flow in
VirtualPreview so rejected IPC calls are handled and still mark audio probing
complete. Add a rejection path alongside the existing success handler that
clears or preserves the appropriate supplemental audio source, calls
setAudioProbeComplete(true), and prevents an unhandled promise rejection.
- Around line 218-237: Ensure the mastering-parameter effect also runs after the
audio graph is created, rather than relying on the ref update to trigger it.
Track graph creation with state and include that state in the effect
dependencies, or extract the parameter assignments from the effect and invoke
that function immediately after graph creation while preserving the existing
settings behavior.
- Around line 757-780: Stabilize the audio element ref callbacks in
VirtualPreview by wrapping the primary and supplemental ref handlers with
useCallback. Preserve assigning both the corresponding audio ref and state
setter, and ensure dependencies include the referenced setters and refs so
callbacks do not change on each render.
In `@src/i18n/locales/ar/settings.json`:
- Around line 56-59: Translate the English values for layout.webcamFraming,
layout.webcamCropZoom, layout.webcamCropX, layout.webcamCropY, and every entry
in the audio group within the Arabic settings locale, while preserving valid
JSON and the existing key structure. Verify all 13 settings locale files contain
these keys and run the provided i18n check to confirm no required values are
missing or unintentionally untranslated.
In `@src/i18n/locales/es/settings.json`:
- Around line 56-59: Translate the new user-visible webcam framing labels and
audio labels/help text, replacing the English fallback values while preserving
the existing JSON keys. Update src/i18n/locales/es/settings.json at lines 56-59
and 305-311 in Spanish, src/i18n/locales/fr/settings.json at lines 56-59 and
305-311 in French, src/i18n/locales/it/settings.json at lines 56-59 and 305-311
in Italian, src/i18n/locales/ja-JP/settings.json at lines 56-59 and 305-311 in
Japanese, src/i18n/locales/ko-KR/settings.json at lines 56-59 and 305-311 in
Korean, and src/i18n/locales/pt-BR/settings.json at lines 56-59 and 305-311 in
Brazilian Portuguese.
In `@src/i18n/locales/ru/settings.json`:
- Around line 56-59: Translate the new webcam framing/crop labels and audio
labels/help text in all affected locale files: src/i18n/locales/ru/settings.json
lines 56-59 and 304-311, src/i18n/locales/tr/settings.json lines 56-59 and
304-311, src/i18n/locales/vi/settings.json lines 56-59 and 304-311,
src/i18n/locales/zh-CN/settings.json lines 56-59 and 304-311, and
src/i18n/locales/zh-TW/settings.json lines 57-60 and 305-312. Use accurate
Russian, Turkish, Vietnamese, Simplified Chinese, and Traditional Chinese
translations, preserve JSON validity across all 13 locale files, and run the
i18n check.
In `@src/lib/ai-edition/store/editorSettings.ts`:
- Around line 258-269: Update normaliseCropRegion so x and y are clamped to 1 -
MIN_CROP_SIZE instead of 1 before width and height are calculated, preserving
the minimum crop size at the edges. Keep the existing dimension clamping and
fallback behavior unchanged.
---
Nitpick comments:
In `@crates/compositor/src/audio.rs`:
- Around line 120-135: Update the PCM shift logic to operate directly on the
existing pcm buffer instead of allocating shifted in the relevant
audio-processing function. Use in-place slice movement such as copy_within for
both shift directions, then zero-fill the vacated region, while preserving the
current clamping and output behavior.
In `@src/components/ai-edition/VirtualPreview.audio.test.ts`:
- Around line 14-19: Add a test case alongside the existing
resolveAudioPreviewTime tests for an unknown duration represented by NaN,
asserting the fallback behavior allows playback while metadata is unavailable.
Use the existing test structure and resolveAudioPreviewTime symbol, without
changing production logic.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 104f41d6-5785-49e1-be79-7bd781cc4bcf
📒 Files selected for processing (44)
crates/compositor/src/audio.rscrates/compositor/src/compositor_linux.rscrates/compositor/src/compositor_macos.rscrates/compositor/src/compositor_windows.rscrates/compositor/src/frame_geometry.rscrates/compositor/src/pipeline_linux.rscrates/compositor/src/pipeline_macos.rscrates/compositor/src/pipeline_windows.rscrates/compositor/src/scene.rssrc/components/ai-edition/NewEditorShell.module.csssrc/components/ai-edition/NewEditorShell.tsxsrc/components/ai-edition/RightPanes.tsxsrc/components/ai-edition/VirtualPreview.audio.test.tssrc/components/ai-edition/VirtualPreview.tsxsrc/components/ai-edition/v4/FloatingInspector.tsxsrc/components/ai-edition/v4/MediaStage.tsxsrc/i18n/locales/ar/editor.jsonsrc/i18n/locales/ar/settings.jsonsrc/i18n/locales/en/editor.jsonsrc/i18n/locales/en/settings.jsonsrc/i18n/locales/es/editor.jsonsrc/i18n/locales/es/settings.jsonsrc/i18n/locales/fr/editor.jsonsrc/i18n/locales/fr/settings.jsonsrc/i18n/locales/it/editor.jsonsrc/i18n/locales/it/settings.jsonsrc/i18n/locales/ja-JP/editor.jsonsrc/i18n/locales/ja-JP/settings.jsonsrc/i18n/locales/ko-KR/editor.jsonsrc/i18n/locales/ko-KR/settings.jsonsrc/i18n/locales/pt-BR/editor.jsonsrc/i18n/locales/pt-BR/settings.jsonsrc/i18n/locales/ru/editor.jsonsrc/i18n/locales/ru/settings.jsonsrc/i18n/locales/tr/editor.jsonsrc/i18n/locales/tr/settings.jsonsrc/i18n/locales/vi/editor.jsonsrc/i18n/locales/vi/settings.jsonsrc/i18n/locales/zh-CN/editor.jsonsrc/i18n/locales/zh-CN/settings.jsonsrc/i18n/locales/zh-TW/editor.jsonsrc/i18n/locales/zh-TW/settings.jsonsrc/lib/ai-edition/store/editorSettings.tssrc/native/sceneDescription.ts
|
All CodeRabbit findings are addressed in 3d1cd3f, including both nitpicks: the PCM shift is in-place and NaN media duration remains playable. Verification: full Vitest suite 1705 passed / 1 skipped, app and test TypeScript checks passed, i18n passed all 12 locales, lint passed with only pre-existing warnings, and Rust passed 129 tests. An independent verifier audited the exact diff and all 12 findings with no blocker. @coderabbitai review |
|
|
What changed
Why
The editor exposed no waveform or practical way to correct residual sync, clean up voice audio, or crop a webcam feed. Those are core finishing controls for a screen recorder and should be fast enough to use without external editing software.
Impact
Users can see where speech occurs, nudge audio without changing duration, master voice quickly, and reframe webcam footage while keeping preview and export behavior aligned.
Validation
Summary by CodeRabbit
New Features
Bug Fixes
Localization