feat: send native speech activity events#29
Conversation
Our StreamWebRTC framework exposes RTCAudioEngineState via engineState but does not provide RTCAudioEngineAvailability or -setEngineAvailability:. Remove the corresponding native bridge methods and their TS wrappers so the example app compiles. Also drop the unused AudioDeviceModuleObserver import and trim AudioEngineAvailability from the public exports. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 26 minutes and 1 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis change introduces speech activity detection on Android and adds comprehensive iOS audio device module controls. It includes a new Java speech detector monitoring microphone loudness, Android integration via WebRTCModule callbacks, iOS native method wrappers for audio device operations, and TypeScript bindings to expose these capabilities to JavaScript while emitting speech activity events across both platforms. Changes
Sequence Diagram(s)sequenceDiagram
participant MIC as Microphone
participant AD as Audio Device<br/>(Native)
participant SAD as SpeechActivityDetector<br/>(Android)
participant WRM as WebRTCModule<br/>(Bridge)
participant JS as JavaScript<br/>(React Native)
rect rgba(100, 150, 200, 0.5)
Note over MIC,JS: Recording Session Start
JS->>WRM: start recording
WRM->>AD: onWebRtcAudioRecordStart()
WRM->>SAD: reset()
end
rect rgba(150, 200, 100, 0.5)
Note over MIC,JS: Audio Buffer Processing
MIC->>AD: capture audio chunk
AD->>WRM: audio buffer callback
WRM->>SAD: processBuffer(buffer, bytesRead)
SAD->>SAD: compute RMS & dBFS
alt Loudness > Threshold
SAD->>SAD: update timing windows
alt Duration > START_CONFIRM_MS
SAD->>WRM: onSpeechStarted()
WRM->>JS: emit audioDeviceModuleSpeechActivity<br/>event: "started"
end
else Loudness ≤ Threshold
alt Speaking && Silence > TIMEOUT_MS
SAD->>WRM: onSpeechEnded()
WRM->>JS: emit audioDeviceModuleSpeechActivity<br/>event: "ended"
end
end
end
rect rgba(200, 150, 100, 0.5)
Note over MIC,JS: Recording Session Stop
JS->>WRM: stop recording
WRM->>AD: onWebRtcAudioRecordStop()
WRM->>SAD: onRecordStop()
alt Still Speaking
SAD->>WRM: onSpeechEnded()
WRM->>JS: emit audioDeviceModuleSpeechActivity<br/>event: "ended"
end
SAD->>SAD: reset()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/AudioDeviceModuleEvents.ts (1)
1-1:⚠️ Potential issue | 🟡 MinorRemove unused
Platformimport to fix CI failure.The pipeline is failing because
Platformis imported but no longer used after removing the Android OS guard on line 43.🔧 Proposed fix
-import { NativeEventEmitter, NativeModules, Platform } from 'react-native'; +import { NativeEventEmitter, NativeModules } from 'react-native';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/AudioDeviceModuleEvents.ts` at line 1, Remove the unused Platform import from the module's import list to fix the CI error: update the import statement that currently reads with NativeEventEmitter, NativeModules, Platform to only import NativeEventEmitter and NativeModules (i.e., remove the symbol Platform) in src/AudioDeviceModuleEvents.ts so there are no unused imports reported by the linter/CI.
🧹 Nitpick comments (2)
src/AudioDeviceModule.ts (1)
20-25: Extract the availability check once.The same platform guard is copied into every method. A tiny helper that owns the error message and returns the native module would keep this surface consistent and make future additions easier to maintain.
♻️ Possible shape
+const getAudioDeviceModule = () => { + if (Platform.OS === 'android') { + throw new Error('AudioDeviceModule is only available on iOS/macOS'); + } + if (!WebRTCModule) { + throw new Error('WebRTCModule is not available'); + } + return WebRTCModule; +}; + export class AudioDeviceModule { /** * Start audio playback */ static async startPlayout(): Promise<void> { - if (Platform.OS === 'android') { - throw new Error('AudioDeviceModule is only available on iOS/macOS'); - } - - return WebRTCModule.audioDeviceModuleStartPlayout(); + return getAudioDeviceModule().audioDeviceModuleStartPlayout(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/AudioDeviceModule.ts` around lines 20 - 25, Extract the repeated Platform.OS check into a single private helper on AudioDeviceModule (e.g., a method like getNativeAudioModuleOrThrow or ensureAudioDeviceAvailable) that performs the iOS/macOS guard and throws the standardized error, then update startPlayout to call that helper and return the native call via the helper (e.g., helper().audioDeviceModuleStartPlayout()). Replace the same pattern in any other AudioDeviceModule static methods to use this helper so the error message and native module retrieval are centralized.ios/RCTWebRTC/WebRTCModule+RTCAudioDeviceModule.m (1)
18-84: Factor the status-code promise boilerplate into one helper.All promise-based exports repeat the same
NSInteger→resolve/rejectmapping. Pulling that into a small helper would remove a lot of copy/paste and keep error formatting aligned as more ADM operations get added.♻️ Helper sketch
+static inline void ResolveADMResult(NSInteger result, + NSString *code, + NSString *action, + RCTPromiseResolveBlock resolve, + RCTPromiseRejectBlock reject) { + if (result == 0) { + resolve(nil); + return; + } + + reject(code, [NSString stringWithFormat:@"Failed to %@: %ld", action, (long)result], nil); +} + RCT_EXPORT_METHOD(audioDeviceModuleStartPlayout : (RCTPromiseResolveBlock)resolve rejecter : (RCTPromiseRejectBlock)reject) { - NSInteger result = [RAW_ADM startPlayout]; - if (result == 0) { - resolve(nil); - } else { - reject(@"playout_error", [NSString stringWithFormat:@"Failed to start playout: %ld", (long)result], nil); - } + ResolveADMResult([RAW_ADM startPlayout], @"playout_error", @"start playout", resolve, reject); }Also applies to: 88-118, 158-168, 196-208
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/RCTWebRTC/WebRTCModule`+RTCAudioDeviceModule.m around lines 18 - 84, Create a small helper in WebRTCModule+RTCAudioDeviceModule.m (e.g. - (void)handleADMResult:(NSInteger)result operation:(NSString*)op resolve:(RCTPromiseResolveBlock)resolve reject:(RCTPromiseRejectBlock)reject) that centralizes the NSInteger → resolve(nil) / reject(code, message, nil) logic and consistent error formatting, then replace the repeated blocks in audioDeviceModuleStartPlayout, audioDeviceModuleStopPlayout, audioDeviceModuleStartRecording, audioDeviceModuleStopRecording, audioDeviceModuleStartLocalRecording, audioDeviceModuleStopLocalRecording (and the other similar blocks mentioned) to call handleADMResult:operation:resolve:reject: passing the RAW_ADM return value and a short operation string like @"startPlayout" so all callers use the same message format and behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/GumTestApp/ios/GumTestApp.xcodeproj/project.pbxproj`:
- Around line 762-765: Remove the extraneous whitespace-only linker flag token
(" ") from the OTHER_LDFLAGS array so only "$(inherited)" remains; locate the
OTHER_LDFLAGS entries (the arrays containing "$(inherited)" and the literal " ")
and delete the " " element in each occurrence (e.g., the entries around the
symbols OTHER_LDFLAGS and the quoted " " token) to avoid passing an
empty/invalid ld argument.
In `@examples/GumTestApp/ios/GumTestApp/Info.plist`:
- Line 47: Update the iOS test app setup to either document or fix why React
Native New Architecture is disabled: locate the RCTNewArchEnabled key in the
Info.plist files for the iOS and tvOS test apps (currently set to false) and
either (A) add a short note in the repository README or PR describing the exact
incompatibility, affected components (e.g., TurboModule/Fabric integration), and
rollback criteria, or (B) enable the flag to true and resolve the underlying
incompatibility in native integration code so it matches the Android setting
(newArchEnabled=true) — mention RCTNewArchEnabled, the two Info.plist entries,
and the Android newArchEnabled setting to make the change discoverable.
In `@src/AudioDeviceModule.ts`:
- Around line 249-256: In setDuckingLevel, validate the incoming level before
calling the bridge: in AudioDeviceModule.setDuckingLevel check Platform.OS as
now, then ensure level is a number, Number.isInteger(level) is true, and level
is between 0 and 100 inclusive; if not, throw a descriptive
RangeError/TypeError; only after passing validation call
WebRTCModule.audioDeviceModuleSetDuckingLevel(level). This prevents non-integer
or out-of-range values from reaching the native method.
---
Outside diff comments:
In `@src/AudioDeviceModuleEvents.ts`:
- Line 1: Remove the unused Platform import from the module's import list to fix
the CI error: update the import statement that currently reads with
NativeEventEmitter, NativeModules, Platform to only import NativeEventEmitter
and NativeModules (i.e., remove the symbol Platform) in
src/AudioDeviceModuleEvents.ts so there are no unused imports reported by the
linter/CI.
---
Nitpick comments:
In `@ios/RCTWebRTC/WebRTCModule`+RTCAudioDeviceModule.m:
- Around line 18-84: Create a small helper in
WebRTCModule+RTCAudioDeviceModule.m (e.g. -
(void)handleADMResult:(NSInteger)result operation:(NSString*)op
resolve:(RCTPromiseResolveBlock)resolve reject:(RCTPromiseRejectBlock)reject)
that centralizes the NSInteger → resolve(nil) / reject(code, message, nil) logic
and consistent error formatting, then replace the repeated blocks in
audioDeviceModuleStartPlayout, audioDeviceModuleStopPlayout,
audioDeviceModuleStartRecording, audioDeviceModuleStopRecording,
audioDeviceModuleStartLocalRecording, audioDeviceModuleStopLocalRecording (and
the other similar blocks mentioned) to call
handleADMResult:operation:resolve:reject: passing the RAW_ADM return value and a
short operation string like @"startPlayout" so all callers use the same message
format and behavior.
In `@src/AudioDeviceModule.ts`:
- Around line 20-25: Extract the repeated Platform.OS check into a single
private helper on AudioDeviceModule (e.g., a method like
getNativeAudioModuleOrThrow or ensureAudioDeviceAvailable) that performs the
iOS/macOS guard and throws the standardized error, then update startPlayout to
call that helper and return the native call via the helper (e.g.,
helper().audioDeviceModuleStartPlayout()). Replace the same pattern in any other
AudioDeviceModule static methods to use this helper so the error message and
native module retrieval are centralized.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d12795ed-ebfa-4733-accb-00f0db46b6ae
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
.gitignoreandroid/src/main/java/com/oney/WebRTCModule/SpeechActivityDetector.javaandroid/src/main/java/com/oney/WebRTCModule/WebRTCModule.javaexamples/GumTestApp/ios/GumTestApp-tvOS/Info.plistexamples/GumTestApp/ios/GumTestApp.xcodeproj/project.pbxprojexamples/GumTestApp/ios/GumTestApp/Info.plistios/RCTWebRTC/WebRTCModule+RTCAudioDeviceModule.hios/RCTWebRTC/WebRTCModule+RTCAudioDeviceModule.mios/RCTWebRTC/WebRTCModule.hios/RCTWebRTC/WebRTCModule.mpackage.jsonsrc/AudioDeviceModule.tssrc/AudioDeviceModuleEvents.tssrc/index.ts
* pc: don't throw when applyConstraints on audio tracks * lint
### 💡 Overview Old mechanism - polling through rtc stats Now - speech events pushed from native side Depends on GetStream/react-native-webrtc#29 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Speech detection now uses native WebRTC event subscriptions instead of the previous detector implementation. * **Removed** * The useSpeechDetection hook has been removed and is no longer exported. * **Docs** * Hook entry for speech detection removed from platform-specific docs. * **Chores** * WebRTC dependency updated to a newer release. * **Tests** * Related unit tests updated/removed to match the new event-based approach. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary by CodeRabbit
Release Notes
New Features
Chores