Add microphone noise suppression options#777
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds microphone noise suppression with RNNoise, Speex, and disabled modes across audio processing, recording persistence, native/browser capture, and launch UI. It also adds localized labels and rebuilds macOS native binaries. ChangesNoise suppression recording flow
macOS native binary rebuilds
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant MicPopover
participant useScreenRecorder
participant getUserMedia
participant NoiseSuppressedStream
participant Recorder
User->>MicPopover: Select noise suppression mode
MicPopover->>useScreenRecorder: setNoiseSuppressionMode(mode)
useScreenRecorder->>useScreenRecorder: Persist recording preference
useScreenRecorder->>getUserMedia: Acquire raw microphone stream
getUserMedia-->>useScreenRecorder: Return raw microphone stream
useScreenRecorder->>NoiseSuppressedStream: Process microphone stream
NoiseSuppressedStream-->>useScreenRecorder: Return processed microphone stream
useScreenRecorder->>Recorder: Provide processed microphone audio
Possibly related PRs
🚥 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/lib/audio/noiseSuppressedStream.ts (1)
45-48: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftConsider
AudioWorkletNodeinstead of the deprecatedScriptProcessorNode.
ScriptProcessorNodeis deprecated and runs the suppression callback on the main thread;AudioWorkletNoderuns on the dedicated audio rendering thread and avoids the main-thread contention this file is already defending against withFRAME_BUDGET_MS/long-frame logging. Since this is new code, it may be worth building on the non-deprecated API instead of ScriptProcessorNode.🤖 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/lib/audio/noiseSuppressedStream.ts` around lines 45 - 48, Replace the deprecated ScriptProcessorNode created in the noise-suppressed stream setup with an AudioWorkletNode-based implementation, moving the suppression processing into an AudioWorklet processor while preserving the existing source, destination, buffer, and frame-budget 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/hooks/useScreenRecorder.ts`:
- Around line 618-634: Update prepareMicrophoneStream to stop every track on the
acquired rawStream when createNoiseSuppressedMicrophoneStream throws, then
rethrow the error so both existing callers retain their current handling. Keep
successful processing and noiseSuppressedMicrophoneStreams registration
unchanged.
In `@src/lib/audio/noiseSuppressedStream.ts`:
- Around line 121-134: The reportWarnings function hardcodes English text before
it reaches the UI, bypassing localization. Change it to return a structured
warning containing an i18n key and any required parameters instead of a
formatted message, then update its caller in useScreenRecorder to translate that
warning before passing it to toast.warning while preserving the existing
disabled and fallback cases.
In `@src/lib/audio/noiseSuppression.ts`:
- Around line 62-81: Update processAudioFrame to retain samples that do not
complete an RNNoise frame across calls: buffer the trailing remainder, prepend
it to the next input frame, process every complete frame, and return output
aligned with the input without exposing unprocessed remainder samples. Preserve
the initialization check and native frameSize processing.
---
Nitpick comments:
In `@src/lib/audio/noiseSuppressedStream.ts`:
- Around line 45-48: Replace the deprecated ScriptProcessorNode created in the
noise-suppressed stream setup with an AudioWorkletNode-based implementation,
moving the suppression processing into an AudioWorklet processor while
preserving the existing source, destination, buffer, and frame-budget behavior.
🪄 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 Plus
Run ID: cf092073-337c-4d7e-ad2e-1179c72c5e2d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (28)
electron/electron-env.d.tselectron/ipc/register/settings.tselectron/native/bin/darwin-arm64/recordly-native-cursor-monitorelectron/native/bin/darwin-arm64/recordly-screencapturekit-helperelectron/native/bin/darwin-arm64/recordly-system-cursorselectron/native/bin/darwin-arm64/recordly-window-listelectron/native/bin/darwin-x64/recordly-native-cursor-monitorelectron/native/bin/darwin-x64/recordly-screencapturekit-helperelectron/native/bin/darwin-x64/recordly-system-cursorselectron/native/bin/darwin-x64/recordly-window-listelectron/preload.tspackage.jsonsrc/components/launch/LaunchWindow.tsxsrc/components/launch/popovers/MicPopover.tsxsrc/hooks/useScreenRecorder.tssrc/i18n/locales/en/launch.jsonsrc/i18n/locales/es/launch.jsonsrc/i18n/locales/fr/launch.jsonsrc/i18n/locales/it/launch.jsonsrc/i18n/locales/ko/launch.jsonsrc/i18n/locales/nl/launch.jsonsrc/i18n/locales/pt-BR/launch.jsonsrc/i18n/locales/ru/launch.jsonsrc/i18n/locales/zh-CN/launch.jsonsrc/i18n/locales/zh-TW/launch.jsonsrc/lib/audio/noiseSuppressedStream.tssrc/lib/audio/noiseSuppression.test.tssrc/lib/audio/noiseSuppression.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/lib/audio/noiseSuppressedStream.ts (1)
112-161: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare the worklet's outgoing message type instead of duck-typing
event.data.
processor.port.onmessagehere checksmessage.typeagainst string literals with no static contract tying it to whatnoiseSuppressionWorklet.tsactually posts ("ready" | "error" | "processing-error" | "long-frame"). Exporting a union type for these outgoing messages fromnoiseSuppressionWorklet.ts(mirroring the existingNoiseSuppressionWorkletMessagefor incoming messages) and typingevent: MessageEvent<...>here would let the type checker catch contract drift between the two files.🤖 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/lib/audio/noiseSuppressedStream.ts` around lines 112 - 161, Export a union type for the worklet’s outgoing messages from noiseSuppressionWorklet.ts, mirroring the existing NoiseSuppressionWorkletMessage definition and covering ready, error, processing-error, and long-frame payloads. Import that type into initializeNoiseSuppressionWorklet and type processor.port.onmessage as MessageEvent of the shared outgoing-message union instead of treating event.data as untyped data.src/lib/audio/noiseSuppressionWorklet.ts (1)
98-139: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftReduce per-callback allocations in the real-time audio path.
process()callsthis.outputBuffer.slice(readableLength)on essentially every render quantum (~every 2.7ms at 48kHz), andprocessBufferedFrame()/appendFloat32Arraysallocate a fresh combinedFloat32Arrayon every processed frame. This happens inside the same path that tracks a 12msframeBudgetMsand logs long frames — repeated allocation/copy here adds GC pressure exactly where real-time budget is already tight, which can contribute to the very glitches the long-frame logging is meant to surface.Consider replacing the append/slice pattern with a fixed-size ring buffer (head/tail indices into a pre-allocated
Float32Array) so no new arrays are allocated during steady-stateprocess()/processBufferedFrame()calls.Also applies to: 144-182, 198-213
🤖 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/lib/audio/noiseSuppressionWorklet.ts` around lines 98 - 139, Replace the allocation-heavy output buffering used by process(), processBufferedFrame(), and appendFloat32Arrays with a pre-allocated Float32Array ring buffer managed by head/tail indices. Enqueue processed frame samples without creating combined arrays, and dequeue samples directly into output without outputBuffer.slice(); preserve existing ordering, zero-fill behavior, and frame-budget processing semantics while ensuring steady-state render callbacks allocate no arrays.
🤖 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/lib/audio/noiseSuppressedStream.ts`:
- Around line 63-79: Update the noise-suppressed stream setup around context,
source, addModule, and initializeNoiseSuppressionWorklet to catch setup
failures, close the AudioContext, and stop every sourceStream track before
rethrowing. Add a bounded timeout to initializeNoiseSuppressionWorklet’s
ready/error handshake so a nonresponsive worklet rejects and triggers the same
cleanup path, while preserving normal successful initialization.
In `@src/lib/audio/noiseSuppressionWorklet.ts`:
- Around line 29-43: Update the outputBuffer declaration in
NoiseSuppressionProcessor to use plain Float32Array, removing the
Float32Array<ArrayBufferLike> generic syntax so it remains compatible with
TypeScript 5.2.2.
---
Nitpick comments:
In `@src/lib/audio/noiseSuppressedStream.ts`:
- Around line 112-161: Export a union type for the worklet’s outgoing messages
from noiseSuppressionWorklet.ts, mirroring the existing
NoiseSuppressionWorkletMessage definition and covering ready, error,
processing-error, and long-frame payloads. Import that type into
initializeNoiseSuppressionWorklet and type processor.port.onmessage as
MessageEvent of the shared outgoing-message union instead of treating event.data
as untyped data.
In `@src/lib/audio/noiseSuppressionWorklet.ts`:
- Around line 98-139: Replace the allocation-heavy output buffering used by
process(), processBufferedFrame(), and appendFloat32Arrays with a pre-allocated
Float32Array ring buffer managed by head/tail indices. Enqueue processed frame
samples without creating combined arrays, and dequeue samples directly into
output without outputBuffer.slice(); preserve existing ordering, zero-fill
behavior, and frame-budget processing semantics while ensuring steady-state
render callbacks allocate no arrays.
🪄 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 Plus
Run ID: 5fc773c2-6b7e-49d0-8599-440f056b8ace
📒 Files selected for processing (7)
electron/ipc/register/settings.tssrc/components/launch/LaunchWindow.tsxsrc/components/launch/popovers/MicPopover.tsxsrc/hooks/useScreenRecorder.tssrc/lib/audio/noiseSuppressedStream.tssrc/lib/audio/noiseSuppression.tssrc/lib/audio/noiseSuppressionWorklet.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/components/launch/LaunchWindow.tsx
- electron/ipc/register/settings.ts
- src/components/launch/popovers/MicPopover.tsx
- src/lib/audio/noiseSuppression.ts
- src/hooks/useScreenRecorder.ts
1 participant
Description
Motivation
Creators need cleaner microphone audio without leaving Recordly. RNNoise is the default, Speex is available as a fallback, and users can disable processing when they want native/raw microphone behavior.
Type of Change
Related Issue(s)
No linked issue.
Screenshots / Video
Not applicable.
Testing Guide
npx tsc --noEmit.npm test.npx vitest --run src/lib/audio/noiseSuppression.test.ts.npx biome check src/lib/audio/noiseSuppression.ts src/lib/audio/noiseSuppressedStream.ts.Checklist
Notes
Summary by CodeRabbit