Conversation
|
Contributor
📝 WalkthroughWalkthroughAdds a browser no-audio detector and integrates it into MicrophoneManager to emit Changes
Sequence Diagram(s)sequenceDiagram
participant MM as MicrophoneManager
participant NAD as NoAudioDetector
participant TR as Tracer/StreamClient
participant UI as MicCaptureErrorNotification
MM->>NAD: createNoAudioDetector(mediaStream, options)
NAD->>NAD: sample audio periodically
alt audio present
NAD->>TR: emit mic.capture_report (capturesAudio: true)
TR->>UI: deliver event
UI->>UI: hide notification
else prolonged silence
NAD->>TR: emit mic.capture_report (capturesAudio: false, noAudioDurationMs)
TR->>UI: deliver event
UI->>UI: show notification
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/client/src/devices/MicrophoneManager.ts`:
- Around line 118-147: The no-audio detector created by createNoAudioDetector
(stored in this.noAudioDetectorCleanup) can remain running because it's only
stopped in the status/mediaStream subscription; update MicrophoneManager.dispose
to explicitly call and await this.noAudioDetectorCleanup (if present) and handle
errors (e.g., try/catch or .catch logging via this.logger.warn) before clearing
this.noAudioDetectorCleanup so the detector's timers/audio context are always
cleaned up when the manager is disposed.
In `@packages/client/src/helpers/no-audio-detector.ts`:
- Around line 288-289: The RN detector is being constructed with new
RNSpeechDetector() which opens a new mic stream; change the initialization so
the detector uses the provided audioStream (e.g., new
RNSpeechDetector(audioStream) or call speechDetector.setStream(audioStream)
after creation) so detection reflects the active track and avoids extra
permission prompts; update any related logic that assumes internal stream
ownership (references: RNSpeechDetector, speechDetector, RNDetectionState,
audioStream).
In `@packages/react-sdk/src/components/Notification/Notification.tsx`:
- Line 46: Typo in the wrapper className: change the misspelled
"str-video__notiication-wrapper" to the correct BEM-style
"str-video__notification-wrapper" on the div in Notification component (the JSX
element using refs.setReference) so styles apply; update any
references/tests/SCSS that expect the corrected class name to keep them
consistent.
🧹 Nitpick comments (3)
packages/react-sdk/src/components/Notification/MicCaptureErrorNotification.tsx (1)
22-27: Ensure mic.capture_report subscriptions always clean up.Line 22–27 assumes
call.on(...)returns an unsubscribe. If it doesn’t, handlers will accumulate on unmount or call changes. Consider explicitoffcleanup for safety.♻️ Suggested defensive cleanup
useEffect(() => { if (!call) return; - return call.on('mic.capture_report', (event) => { - setIsVisible(!event.capturesAudio); - }); + const handler = (event: { capturesAudio: boolean }) => { + setIsVisible(!event.capturesAudio); + }; + call.on('mic.capture_report', handler); + return () => { + call.off?.('mic.capture_report', handler); + }; }, [call]);As per coding guidelines, ensure event listeners are cleaned up in effects.
packages/client/src/helpers/no-audio-detector.ts (1)
195-213: Consider stopping the RN detector once audio is confirmed.
AftercapturesAudio: true, the RN path keeps the underlying speech detector running; consider clearing the interval and stopping detection to save battery/CPU and align with the browser path’s “stop on audio” behavior.packages/client/src/devices/MicrophoneManager.ts (1)
262-272: Consider applying threshold changes immediately when the detector is active.
Right now the new threshold only takes effect after a status/stream change. If you expect runtime tuning, consider restarting the detector when the mic is already enabled.
packages/react-sdk/src/components/Notification/Notification.tsx
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/client/detectors-api.md`:
- Around line 287-289: The frequency resolution and latency lines currently use
Nyquist (22050 Hz) instead of the actual sample rate; update the "Frequency
resolution" and "Latency" calculations to use Sample Rate / fftSize and fftSize
/ Sample Rate respectively (e.g., at 44100 Hz: 44100 / 256 = ~172.3 Hz per bin
and 256 / 44100 = ~5.8 ms; at 48000 Hz: 48000 / 256 = ~187.5 Hz per bin and 256
/ 48000 = ~5.3 ms), or explicitly state the assumed sample rate if the docs are
locked to 44100 Hz so readers know which formula/values apply.
In `@packages/client/src/devices/__tests__/MicrophoneManager.test.ts`:
- Line 23: The tests call setupAudioContextMock() which stubs the global
AudioContext via vi.stubGlobal() and is not cleaned up; import
cleanupAudioContextMock from './web-audio.mocks' and add an afterEach hook that
calls cleanupAudioContextMock() to restore the global and prevent cross-test
leakage (ensure the afterEach runs in the same test file where
setupAudioContextMock() is used).
In `@packages/client/src/helpers/__tests__/no-audio-detector.test.ts`:
- Around line 513-538: The test fails because emitIntervalMs is hardcoded to
5000 instead of defaulting to noAudioThresholdMs per the JSDoc; update the
createNoAudioDetector implementation to set emitIntervalMs =
options.emitIntervalMs ?? options.noAudioThresholdMs (or equivalent using the
internal option names) so when emitIntervalMs is omitted it inherits
noAudioThresholdMs, ensuring behavior matches the JSDoc and the test
(references: createNoAudioDetector, emitIntervalMs, noAudioThresholdMs).
🧹 Nitpick comments (1)
packages/client/detectors-api.md (1)
472-503: Consider adding language specifiers to fenced code blocks.The event flow diagrams at lines 472, 484, and 496 are missing language specifiers. While not critical, adding
textorplaintextidentifiers would satisfy linting rules and improve consistency.📝 Suggested formatting improvement
-``` +```text 1. User enables microphone 2. Detection starts (500ms interval) ...Apply the same pattern to the code blocks at lines 484 and 496.
packages/client/src/helpers/__tests__/no-audio-detector.test.ts
Outdated
Show resolved
Hide resolved
jdimovska
approved these changes
Jan 22, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/client/src/helpers/no-audio-detector.ts`:
- Around line 5-45: The JSDoc defaults for NoAudioDetectorOptions
(detectionFrequencyInMs = 500, audioLevelThreshold = 5) don't match the actual
defaults used in createBrowserDetector; update createBrowserDetector so its
default values for detectionFrequencyInMs and audioLevelThreshold match the
documented values (set detectionFrequencyInMs to 500 and audioLevelThreshold to
5) so the implementation and the NoAudioDetectorOptions docs are consistent.
🧹 Nitpick comments (3)
packages/client/src/helpers/RNSpeechDetector.ts (1)
179-210: Consider using a small threshold instead of zero for noise detection.Using
normalizedAudioLevel > 0may be overly sensitive—even minimal background noise or sensor artifacts can produce non-zero values. Consider aligning with a small threshold (similar to the browser implementation's approach) for more reliable detection.💡 Suggested improvement
+const NOISE_THRESHOLD = 0.001; // Minimal threshold to filter sensor noise + const normalizedAudioLevel = typeof audioLevel === 'number' ? audioLevel : 0; onSoundDetectedStateChanged({ - isSoundDetected: normalizedAudioLevel > 0, + isSoundDetected: normalizedAudioLevel > NOISE_THRESHOLD, audioLevel: normalizedAudioLevel, });packages/client/src/helpers/no-audio-detector.ts (2)
178-191: State reset is unreachable when audio is detected.When
wasInNoAudioStateis true, the function returns early without resetting state (lines 186-188 are unreachable). While the detector stops afterward, resetting state before returning would be cleaner and safer if the control flow ever changes.♻️ Suggested improvement
const handleAudioDetected = ( state: DetectionState, ): CaptureStatusEvent | undefined => { const wasInNoAudioState = state.noAudioStartTime !== null; + + // Reset timing state + state.noAudioStartTime = null; + state.lastEmitTime = null; + if (wasInNoAudioState) { return { capturesAudio: true }; } - - // Reset timing state - state.noAudioStartTime = null; - state.lastEmitTime = null; - return undefined; };
196-214: Interval not cleared directly when audio is detected.When
wasInNoAudioStateis true,shouldStopis set butcheckIntervalIdis not cleared (lines 200-203 return early). The interval continues firing (returning early due toshouldStop) untilstopDetectoris called asynchronously. Consider clearing the interval directly for immediate cleanup.♻️ Suggested improvement
const handleAudioDetectedRN = ( state: RNDetectionState, ): CaptureStatusEvent | undefined => { const wasInNoAudioState = state.noAudioStartTime !== null; + + // Reset timing state and clear interval + state.noAudioStartTime = null; + state.lastEmitTime = null; + if (state.checkIntervalId) { + clearInterval(state.checkIntervalId); + state.checkIntervalId = undefined; + } + if (wasInNoAudioState) { state.shouldStop = true; return { capturesAudio: true }; } - - // Reset timing state - state.noAudioStartTime = null; - state.lastEmitTime = null; - if (state.checkIntervalId) { - clearInterval(state.checkIntervalId); - state.checkIntervalId = undefined; - } - return undefined; };
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@packages/client/src/devices/MicrophoneManager.ts`:
- Around line 264-274: The setSilenceThreshold method only assigns
silenceThresholdMs but doesn’t stop an already-running detector, so when
thresholdMs <= 0 the detector must be stopped; update
setSilenceThreshold(thresholdMs: number) in MicrophoneManager to set
this.silenceThresholdMs = thresholdMs and if thresholdMs <= 0 call the existing
detector stop/cleanup method (e.g.,
stopSilenceDetector/clearSilenceTimeout/stopDetector — whichever function is
responsible for tearing down the active detector) to disable detection
immediately; additionally, if you want to support changing to a positive
threshold, ensure you (re)start the detector when thresholdMs > 0 by invoking
the detector initialization routine used elsewhere in the class.
In `@packages/client/src/helpers/no-audio-detector.ts`:
- Around line 3-42: The NoAudioDetectorOptions type currently requires
noAudioThresholdMs and emitIntervalMs but callers omit them and
createNoAudioDetector doesn't apply defaults; make noAudioThresholdMs and
emitIntervalMs optional in NoAudioDetectorOptions, set defaults in
createNoAudioDetector (e.g., default noAudioThresholdMs = 5000 and
emitIntervalMs = noAudioThresholdMs when not provided), normalize the options
into a single object and pass that normalized options object into
transitionState (and any other internal callers) so comparisons use defined
numbers and TypeScript reflects the optional defaults.
- Around line 110-119: transitionState currently only emits when audioDetected
and state.kind is 'IDLE' or 'EMITTING', which leaves 'DETECTING' stuck; change
the logic in transitionState so that any audioDetected returns emit(true, state)
immediately (use the existing emit(...) helper) regardless of the current
state.kind (including 'DETECTING'), otherwise fall back to existing
noEmit(state) behavior to stop/continue intervals.
In `@packages/client/src/helpers/RNSpeechDetector.ts`:
- Around line 26-31: In RNSpeechDetector, guard against null ICE candidates in
the 'icecandidate' event handlers for this.pc1 and this.pc2 before calling
addIceCandidate: check that e?.candidate is non-null (or truthy) and only then
call this.pc2.addIceCandidate(e.candidate) /
this.pc1.addIceCandidate(e.candidate); optionally wrap the addIceCandidate call
in a try/catch to quietly handle implementations that still throw. This mirrors
the null-candidate protection used in BasePeerConnection.ts.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@packages/client/src/devices/MicrophoneManager.ts`:
- Around line 296-317: In performTest, if createNoAudioDetector throws the
acquired MediaStream is leaked; wrap the call to createNoAudioDetector in a
try/catch, and on catch call disposeOfMediaStream(stream), log the error via
this.logger.warn/error, and reject or resolve the promiseWithResolvers (e.g.,
resolve(false) or reject(err)) to ensure the returned promise always settles.
Reference performTest, promiseWithResolvers, createNoAudioDetector,
disposeOfMediaStream and the local cleanup variable so the stream is always
disposed when detector setup fails.
In `@packages/client/src/helpers/no-audio-detector.ts`:
- Around line 1-2: The code in no-audio-detector.ts uses the web-only
AudioContext unguarded; wrap any creation or reference to AudioContext (and
related web-audio calls within the module, including the block around lines
116-146) with platform checks and throw a clear error when running in
non-browser environments: use the existing platform helpers (isReactNative(),
isMobileSafari(), isSafari(), isFirefox()) to detect React Native or non-browser
contexts and avoid calling new AudioContext(); if AudioContext is required only
in browsers, guard creation behind if (!isReactNative() && typeof AudioContext
!== "undefined") { /* create */ } else { throw new Error("AudioContext
unavailable: running in non-browser environment (React Native)"); } and add a
videoLoggerSystem.error with the same descriptive message before throwing.
In `@sample-apps/react/react-dogfood/CLAUDE.md`:
- Around line 195-197: The fenced code block in CLAUDE.md currently lacks a
language identifier; update the triple-backtick fence that wraps the lines
"?coordinator_url=..." and "?use_local_coordinator=true" to include a language
(e.g., bash) so syntax highlighting works. Locate the code block in the
CLAUDE.md content and change the opening ``` to ```bash, leaving the block
contents unchanged.
In `@sample-apps/react/react-dogfood/components/Inspector/DevicesDash.tsx`:
- Around line 188-268: The device label span in the DevicesDash list is not
keyboard-accessible and the test control lacks an accessible name; replace the
clickable <span> that calls props.manager?.select(device.deviceId) with a native
<button type="button"> (preserve the inline styles or CSS class) so it can be
focused and activated by keyboard, and add an appropriate ARIA state (e.g.,
aria-pressed or aria-current) that reflects device.deviceId ===
props.selectedDevice; also give the test control (props.onTestDevice handler /
the current test button) a clear accessible name (aria-label like "Test
microphone for {device.label}" or visually hidden text) and ensure the button
remains disabled/announced correctly when isTesting/props.testingDeviceId is
set.
🧹 Nitpick comments (1)
sample-apps/react/react-dogfood/CLAUDE.md (1)
73-84: Consider documenting the microphone capture notification feature.The AI summary indicates that this PR adds
MicCaptureErrorNotificationcomponent and mic capture detection features to the dogfood app. While this is general guidance documentation, mentioning this new notification/debugging feature in the component organization section could help developers understand the full scope of available components.Optional addition to component list
- **Debug/** - Debug panels and inspection tools - **Inspector/** - Deep SDK state inspection - **Settings/** - Audio/video/effects configuration +- **Notifications/** - Error notifications (e.g., MicCaptureErrorNotification for broken mic detection)
This was referenced Feb 4, 2026
Merged
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
2 participants
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.
💡 Overview
Adds a broken microphone setup integration. Upon unmuting or switching a device, we start polling for audio levels.
If we don't get any captured audio after a certain threshold (default is 5 seconds), we emit a
mic.capture_reportevent withcapturesAudio: false.Integrators can use this event to show a warning message to the end users so they can adjust their setup (or switch to a different device).
While being silent, the SDK will keep publishing this event. Once sound is detected, this poller will emit
mic.capture_reportwithcapturesAudio: trueright away, and will stop emitting further events until there is a device switch or unmute of the current one.📝 Implementation notes
🎫 Ticket: https://linear.app/stream/issue/REACT-759/detectors-api-detection-of-broken-microphone-setup
📑 Docs: https://github.com/GetStream/docs-content/pull/968
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation