feat: webrtc 145 update#27
Merged
Merged
Conversation
📝 WalkthroughWalkthroughUpgrades StreamWebRTC dependencies, migrates iOS rendering to RTCVideoRenderingView and sets frameBufferPolicy, removes frame-cryption APIs/events and JS exports, syncs AudioDeviceModule mute state with local audio tracks, adds Android camera sensor-orientation helpers and dimension-swap logic, and updates degradationPreference handling. ChangesCross-platform WebRTC surface update
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 docstrings
🧪 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@stream-react-native-webrtc.podspec`:
- Line 24: The podspec contains a broken CocoaPods dependency declaration
s.dependency 'StreamWebRTC', '~>137.0.62' which will cause pod install to fail
because StreamWebRTC isn't published as a standalone pod; remove that dependency
or replace it with a valid CocoaPods pod that vendors WebRTC (e.g., use the
StreamVideo pod that includes the vendored XCFramework). Update the podspec by
deleting the s.dependency 'StreamWebRTC' line and either rely on SPM for
StreamWebRTC or add/replace with s.dependency 'StreamVideo',
'<appropriate-version-that-includes-137.0.62>' (ensure the chosen StreamVideo
version actually vendors WebRTC 137.0.62).
5 tasks
santhoshvai
pushed a commit
that referenced
this pull request
Apr 16, 2026
* Add AudioProcessingFactory to WebRTCModuleOptions * Expose media stream tracks for ios * Swift compatibility * Fix boost checksum error
ipavlidakis
reviewed
Apr 28, 2026
Co-authored-by: Ilias Pavlidakis <3liaspav@gmail.com>
# Conflicts: # ios/RCTWebRTC/WebRTCModule.m # package.json
This reverts commit 337677d. # Conflicts: # android/build.gradle # android/src/main/java/com/oney/WebRTCModule/WebRTCModule.java # livekit-react-native-webrtc.podspec # src/index.ts
This reverts commit fe2a043. # Conflicts: # android/src/main/java/com/oney/WebRTCModule/PeerConnectionObserver.java # android/src/main/java/com/oney/WebRTCModule/RTCFrameCryptor.java # android/src/main/java/com/oney/WebRTCModule/WebRTCModule.java # ios/RCTWebRTC/WebRTCModule+RTCFrameCryptor.m # ios/RCTWebRTC/WebRTCModule+RTCPeerConnection.h # ios/RCTWebRTC/WebRTCModule+RTCPeerConnection.m # ios/RCTWebRTC/WebRTCModule.h # ios/RCTWebRTC/WebRTCModule.m # livekit-react-native-webrtc.podspec # package.json # src/RTCFrameCryptor.ts fix: ios build error unnecessary import fix missing import
Promote the audio-engine lifecycle publisher and its Event enum from internal to public so app-level Cocoapod modules (CallKit integrations, in-call audio managers, etc.) can subscribe to willEnableAudioEngine / didDisableAudioEngine events and reapply AVAudioSessionConfiguration deterministically on every engine rebuild — most importantly during interruption recovery, where the WebRTC ADM tears down and rebuilds the engine and the previously-applied session config is lost.
santhoshvai
changed the title
…nce (#43) ## Summary - Add `'maintain-framerate-and-resolution'` to `DegradationPreferenceType` - Fix `toNative` / `fromNative` to use global regex replaces (the existing single-replace was a latent bug for any multi-dash token) ##⚠️ DO NOT MERGE before the M145 native SDK bump `MAINTAIN_FRAMERATE_AND_RESOLUTION` first lands in libwebrtc with Chrome M144 ([Intent to Ship](https://groups.google.com/a/chromium.org/g/blink-dev/c/FUlvv_Lmk-s)). This SDK currently pins M137 on both platforms: - iOS: `StreamWebRTC ~> 137.0.54` - Android: `io.getstream:stream-video-webrtc-android:137.0.1` On M137, passing the new value will reject the JS promise on Android (`IllegalArgumentException` from `RtpParameters.DegradationPreference.valueOf`) and silently no-op on iOS. Wait until the pending M145 native SDK bump is published before merging this PR. No native (iOS / Android) changes are needed: Android's `valueOf` and iOS's plain NSString assignment will both work once the underlying binary ships the new enum value.
…red frames getSettings() returned sensor-space (landscape) dimensions while M145 delivers frames in the device orientation (portrait). This caused a landscape-to-portrait flash on camera mute/unmute. Now both dimension sources report oriented dimensions so they agree: - getSettings() swaps width/height based on sensor orientation and display rotation, matching the frame rotation the camera session uses. - videoTrackDimensionChanged uses VideoFrame.getRotatedWidth/Height instead of raw buffer dimensions. Adds getSensorOrientation() helpers for Camera1 and Camera2. Affects local and remote track getSettings().
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/index.ts (1)
98-100: 💤 Low valueDuplicate
setupListeners()call is redundant.
audioDeviceModuleEvents.setupListeners()is already called at line 40 during module initialization. Calling it again at line 100 insideregisterGlobals()is redundant if the listeners are already registered.If
setupListeners()is idempotent (safe to call multiple times), this is harmless. Otherwise, consider removing one of the calls.🤖 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/index.ts` around lines 98 - 100, Duplicate call to audioDeviceModuleEvents.setupListeners() exists: it's invoked during module initialization and again inside registerGlobals(); remove the redundant call in registerGlobals() (or make setupListeners() idempotent and document it) so listeners are only registered once; locate the registerGlobals function and delete the audioDeviceModuleEvents.setupListeners() invocation (or add a guard inside setupListeners to no-op if already initialized).android/src/main/java/com/oney/WebRTCModule/WebRTCModule.java (1)
594-611: 💤 Low valueCaller contract relies on comment-only enforcement.
The comment "Must be called in the executor" at line 594 documents an important threading contract, but there's no runtime assertion. Callers must ensure they invoke this from
ThreadUtils.runOnExecutor()orsubmitToExecutor(). This is acceptable if all call sites are verified.🤖 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 `@android/src/main/java/com/oney/WebRTCModule/WebRTCModule.java` around lines 594 - 611, The method getStreamForReactTag has a documented threading contract ("Must be called in the executor") but lacks a runtime check; add an assertion at the start of getStreamForReactTag that verifies the current thread is the module's executor (use the same ThreadUtils or executor-check helper used elsewhere, e.g., ThreadUtils.runOnExecutor/submitToExecutor idiom or an existing check method) and throw or log an error if called off-executor so callers fail fast; update getStreamForReactTag to perform this check before accessing localStreams or mPeerConnectionObservers.
🤖 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 `@android/src/main/java/com/oney/WebRTCModule/WebRTCModule.java`:
- Around line 58-60: mCertificates is a static Map that can grow indefinitely
because certificates are never removed; add cleanup to avoid leaks by providing
a public/static removal API (e.g., removeCertificate(String id) or
removeCertificatesByIds) and/or clearing entries in WebRTCModule.invalidate() —
locate mCertificates and implement a method to remove a certificate by its key
and call mCertificates.clear() (or selectively remove) inside
WebRTCModule.invalidate() to ensure stored RtcCertificatePem instances are
released when the module is invalidated.
- Around line 263-277: The loop writing mixed audio can write past the mic
buffer when i >= micSamples because it does micShorts.put(i, ...) even for
screen-only samples; update the loop in WebRTCModule.java (the for-loop using
totalSamples, micShorts, screenShorts, micSamples, screenSamples) so that when i
>= micSamples you do not write into micShorts (e.g., skip/continue) or ensure
the mic buffer has capacity before put; in short, change the i >= micSamples
branch to skip writing to micShorts (matching the i >= screenSamples handling)
to avoid IndexOutOfBoundsException.
---
Nitpick comments:
In `@android/src/main/java/com/oney/WebRTCModule/WebRTCModule.java`:
- Around line 594-611: The method getStreamForReactTag has a documented
threading contract ("Must be called in the executor") but lacks a runtime check;
add an assertion at the start of getStreamForReactTag that verifies the current
thread is the module's executor (use the same ThreadUtils or executor-check
helper used elsewhere, e.g., ThreadUtils.runOnExecutor/submitToExecutor idiom or
an existing check method) and throw or log an error if called off-executor so
callers fail fast; update getStreamForReactTag to perform this check before
accessing localStreams or mPeerConnectionObservers.
In `@src/index.ts`:
- Around line 98-100: Duplicate call to audioDeviceModuleEvents.setupListeners()
exists: it's invoked during module initialization and again inside
registerGlobals(); remove the redundant call in registerGlobals() (or make
setupListeners() idempotent and document it) so listeners are only registered
once; locate the registerGlobals function and delete the
audioDeviceModuleEvents.setupListeners() invocation (or add a guard inside
setupListeners to no-op if already initialized).
🪄 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: a451114d-76e0-4aef-8d41-2dd6c4b21b1a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
android/build.gradleandroid/src/main/java/com/oney/WebRTCModule/CameraCaptureController.javaandroid/src/main/java/com/oney/WebRTCModule/PeerConnectionObserver.javaandroid/src/main/java/com/oney/WebRTCModule/VideoTrackAdapter.javaandroid/src/main/java/com/oney/WebRTCModule/WebRTCModule.javaandroid/src/main/java/org/webrtc/Camera1Helper.javaandroid/src/main/java/org/webrtc/Camera2Helper.javaios/RCTWebRTC/RTCVideoViewManager.mios/RCTWebRTC/Utils/AudioDeviceModule/AudioDeviceModule.swiftios/RCTWebRTC/WebRTCModule+RTCPeerConnection.hios/RCTWebRTC/WebRTCModule+RTCPeerConnection.mios/RCTWebRTC/WebRTCModule.hios/RCTWebRTC/WebRTCModule.mpackage.jsonsrc/RTCRtpSendParameters.tssrc/index.tsstream-react-native-webrtc.podspec
✅ Files skipped from review due to trivial changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- ios/RCTWebRTC/RTCVideoViewManager.m
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/index.ts (1)
98-100: 💤 Low valueDuplicate
setupListeners()call is redundant.
audioDeviceModuleEvents.setupListeners()is already called at line 40 during module initialization. Calling it again at line 100 insideregisterGlobals()is redundant if the listeners are already registered.If
setupListeners()is idempotent (safe to call multiple times), this is harmless. Otherwise, consider removing one of the calls.🤖 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/index.ts` around lines 98 - 100, Duplicate call to audioDeviceModuleEvents.setupListeners() exists: it's invoked during module initialization and again inside registerGlobals(); remove the redundant call in registerGlobals() (or make setupListeners() idempotent and document it) so listeners are only registered once; locate the registerGlobals function and delete the audioDeviceModuleEvents.setupListeners() invocation (or add a guard inside setupListeners to no-op if already initialized).android/src/main/java/com/oney/WebRTCModule/WebRTCModule.java (1)
594-611: 💤 Low valueCaller contract relies on comment-only enforcement.
The comment "Must be called in the executor" at line 594 documents an important threading contract, but there's no runtime assertion. Callers must ensure they invoke this from
ThreadUtils.runOnExecutor()orsubmitToExecutor(). This is acceptable if all call sites are verified.🤖 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 `@android/src/main/java/com/oney/WebRTCModule/WebRTCModule.java` around lines 594 - 611, The method getStreamForReactTag has a documented threading contract ("Must be called in the executor") but lacks a runtime check; add an assertion at the start of getStreamForReactTag that verifies the current thread is the module's executor (use the same ThreadUtils or executor-check helper used elsewhere, e.g., ThreadUtils.runOnExecutor/submitToExecutor idiom or an existing check method) and throw or log an error if called off-executor so callers fail fast; update getStreamForReactTag to perform this check before accessing localStreams or mPeerConnectionObservers.
🤖 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 `@android/src/main/java/com/oney/WebRTCModule/WebRTCModule.java`:
- Around line 58-60: mCertificates is a static Map that can grow indefinitely
because certificates are never removed; add cleanup to avoid leaks by providing
a public/static removal API (e.g., removeCertificate(String id) or
removeCertificatesByIds) and/or clearing entries in WebRTCModule.invalidate() —
locate mCertificates and implement a method to remove a certificate by its key
and call mCertificates.clear() (or selectively remove) inside
WebRTCModule.invalidate() to ensure stored RtcCertificatePem instances are
released when the module is invalidated.
- Around line 263-277: The loop writing mixed audio can write past the mic
buffer when i >= micSamples because it does micShorts.put(i, ...) even for
screen-only samples; update the loop in WebRTCModule.java (the for-loop using
totalSamples, micShorts, screenShorts, micSamples, screenSamples) so that when i
>= micSamples you do not write into micShorts (e.g., skip/continue) or ensure
the mic buffer has capacity before put; in short, change the i >= micSamples
branch to skip writing to micShorts (matching the i >= screenSamples handling)
to avoid IndexOutOfBoundsException.
---
Nitpick comments:
In `@android/src/main/java/com/oney/WebRTCModule/WebRTCModule.java`:
- Around line 594-611: The method getStreamForReactTag has a documented
threading contract ("Must be called in the executor") but lacks a runtime check;
add an assertion at the start of getStreamForReactTag that verifies the current
thread is the module's executor (use the same ThreadUtils or executor-check
helper used elsewhere, e.g., ThreadUtils.runOnExecutor/submitToExecutor idiom or
an existing check method) and throw or log an error if called off-executor so
callers fail fast; update getStreamForReactTag to perform this check before
accessing localStreams or mPeerConnectionObservers.
In `@src/index.ts`:
- Around line 98-100: Duplicate call to audioDeviceModuleEvents.setupListeners()
exists: it's invoked during module initialization and again inside
registerGlobals(); remove the redundant call in registerGlobals() (or make
setupListeners() idempotent and document it) so listeners are only registered
once; locate the registerGlobals function and delete the
audioDeviceModuleEvents.setupListeners() invocation (or add a guard inside
setupListeners to no-op if already initialized).
🪄 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: a451114d-76e0-4aef-8d41-2dd6c4b21b1a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
android/build.gradleandroid/src/main/java/com/oney/WebRTCModule/CameraCaptureController.javaandroid/src/main/java/com/oney/WebRTCModule/PeerConnectionObserver.javaandroid/src/main/java/com/oney/WebRTCModule/VideoTrackAdapter.javaandroid/src/main/java/com/oney/WebRTCModule/WebRTCModule.javaandroid/src/main/java/org/webrtc/Camera1Helper.javaandroid/src/main/java/org/webrtc/Camera2Helper.javaios/RCTWebRTC/RTCVideoViewManager.mios/RCTWebRTC/Utils/AudioDeviceModule/AudioDeviceModule.swiftios/RCTWebRTC/WebRTCModule+RTCPeerConnection.hios/RCTWebRTC/WebRTCModule+RTCPeerConnection.mios/RCTWebRTC/WebRTCModule.hios/RCTWebRTC/WebRTCModule.mpackage.jsonsrc/RTCRtpSendParameters.tssrc/index.tsstream-react-native-webrtc.podspec
✅ Files skipped from review due to trivial changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- ios/RCTWebRTC/RTCVideoViewManager.m
🛑 Comments failed to post (2)
android/src/main/java/com/oney/WebRTCModule/WebRTCModule.java (2)
58-60:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStatic certificate map may leak memory if certificates are never removed.
mCertificatesis a static map that stores generated certificates by ID. However, there's no visible cleanup mechanism to remove certificates when they're no longer needed. Over the lifetime of an app, this could accumulate stale certificate entries.Consider adding a method to remove certificates or clear the map during
invalidate().💡 Suggested addition in invalidate()
// 4. Dispose factory (frees C++ factory + 3 threads) if (mFactory != null) { mFactory.dispose(); mFactory = null; } + + // 5. Clear cached certificates + synchronized (mCertificates) { + mCertificates.clear(); + } return null; })🤖 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 `@android/src/main/java/com/oney/WebRTCModule/WebRTCModule.java` around lines 58 - 60, mCertificates is a static Map that can grow indefinitely because certificates are never removed; add cleanup to avoid leaks by providing a public/static removal API (e.g., removeCertificate(String id) or removeCertificatesByIds) and/or clearing entries in WebRTCModule.invalidate() — locate mCertificates and implement a method to remove a certificate by its key and call mCertificates.clear() (or selectively remove) inside WebRTCModule.invalidate() to ensure stored RtcCertificatePem instances are released when the module is invalidated.
263-277:
⚠️ Potential issue | 🔴 Critical | ⚡ Quick winPotential out-of-bounds write when screen buffer is longer than mic buffer.
When
i >= micSamples(line 265), the code writesscreenShorts.get(i)tomicShorts.put(i, ...)at line 277. However, ifmicSamples < totalSamples, the mic buffer doesn't have capacity for indexi >= micSamples, causing anIndexOutOfBoundsExceptionor silent corruption.If the intent is to extend the mic buffer with screen-only samples, the underlying
ByteBuffermust have been allocated with sufficient capacity. If not, thei >= micSamplesbranch should skip writing (similar to thei >= screenSamplesbranch).🐛 Proposed fix: Skip writing when beyond mic buffer capacity
for (int i = 0; i < totalSamples; i++) { int sum; if (i >= micSamples) { - // Screen-only: mic buffer is shorter — write screen sample directly - sum = screenShorts.get(i); + // Screen-only: mic buffer is shorter — cannot write beyond its capacity + break; } else if (i >= screenSamples) { // Mic-only: screen buffer is shorter — keep mic sample as-is break; } else { // Both buffers have data — add samples sum = micShorts.get(i) + screenShorts.get(i); } if (sum > Short.MAX_VALUE) sum = Short.MAX_VALUE; if (sum < Short.MIN_VALUE) sum = Short.MIN_VALUE; micShorts.put(i, (short) sum); }🤖 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 `@android/src/main/java/com/oney/WebRTCModule/WebRTCModule.java` around lines 263 - 277, The loop writing mixed audio can write past the mic buffer when i >= micSamples because it does micShorts.put(i, ...) even for screen-only samples; update the loop in WebRTCModule.java (the for-loop using totalSamples, micShorts, screenShorts, micSamples, screenSamples) so that when i >= micSamples you do not write into micShorts (e.g., skip/continue) or ensure the mic buffer has capacity before put; in short, change the i >= micSamples branch to skip writing to micShorts (matching the i >= screenSamples handling) to avoid IndexOutOfBoundsException.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
stream-react-native-webrtc.podspec (1)
24-24: ⚡ Quick winConsider using pessimistic version constraint instead of exact pinning.
The dependency operator was changed from
~>137.0.54(pessimistic) to= 145.10.0(exact). Exact version pinning prevents CocoaPods from resolving any other version, which can cause integration issues if:
- Other pods in a consumer's project depend on a different StreamWebRTC version
- Bug fixes or patches are released as 145.10.1, 145.10.2, etc.
Unless there's a specific API incompatibility requiring this exact version, consider using
~> 145.10.0to allow patch-level updates while maintaining major.minor compatibility.🔧 Recommended change to use pessimistic constraint
- s.dependency 'StreamWebRTC', '= 145.10.0' + s.dependency 'StreamWebRTC', '~> 145.10.0'🤖 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 `@stream-react-native-webrtc.podspec` at line 24, Change the exact version pin on the StreamWebRTC dependency to a pessimistic constraint to allow patch updates; locate the s.dependency line that currently reads "StreamWebRTC', '= 145.10.0" and update it to use the pessimistic operator (e.g., "~> 145.10.0") so CocoaPods can resolve compatible patch releases while keeping major.minor stability.ios/RCTWebRTC/Utils/AudioDeviceModule/AudioDeviceModule.swift (1)
252-254: ⚡ Quick winDocument why mute mode is no longer reset.
The
reset()method no longer configures the ADM mute mode. If this is intentional (e.g., because mute mode is now managed bysetStereoPlayoutPreferenceor track-level sync), please add a comment explaining that mute mode is preserved across resets.🤖 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 `@ios/RCTWebRTC/Utils/AudioDeviceModule/AudioDeviceModule.swift` around lines 252 - 254, The reset() implementation currently just calls source.reset() and no longer touches ADM mute mode; add a brief explanatory comment inside or above the AudioDeviceModule.reset() method indicating that mute state is intentionally preserved across resets (e.g., because mute is now managed at track-level or via setStereoPlayoutPreference) so future readers understand this is deliberate and not a bug; reference the reset() method, the call to source.reset(), and setStereoPlayoutPreference in the comment to clarify the authority for mute handling.ios/RCTWebRTC/WebRTCModule+RTCMediaStream.m (1)
621-630: ⚡ Quick winExtract duplicated ADM mute sync into a shared helper.
The ADM mute synchronization logic is duplicated across three category files with nearly identical implementations. This makes the code harder to maintain and increases the risk of inconsistencies if the logic needs to change.
♻️ Suggested refactor
Add this helper method to
WebRTCModule:// WebRTCModule.m or WebRTCModule+RTCMediaStream.m - (void)syncAudioTrackStateToADM:(RTCMediaStreamTrack *)track { if (!track || ![track.kind isEqualToString:@"audio"]) { return; } if (!self.audioDeviceModule) { NSLog(@"[WebRTCModule] AudioDeviceModule not initialized, skipping mute sync"); return; } NSError *error = nil; if (![self.audioDeviceModule setMuted:!track.isEnabled error:&error]) { NSLog(@"[WebRTCModule] Failed to sync ADM mute state: %@", error.localizedDescription); } }Then replace all three call sites with
[self syncAudioTrackStateToADM:track];🤖 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 `@ios/RCTWebRTC/WebRTCModule`+RTCMediaStream.m around lines 621 - 630, Extract the duplicated ADM mute-sync block into a shared instance method on WebRTCModule named -syncAudioTrackStateToADM:(RTCMediaStreamTrack *)track that checks track non-null and kind == @"audio", verifies self.audioDeviceModule exists (log and return if nil), and calls [self.audioDeviceModule setMuted:!track.isEnabled error:&error] logging any error.localizedDescription; then replace the three inlined implementations (including the current pcId.intValue == -1 audio branch in WebRTCModule+RTCMediaStream.m) with a call to [self syncAudioTrackStateToADM:track];.
🤖 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 `@ios/RCTWebRTC/WebRTCModule`+RTCMediaStream.m:
- Around line 621-630: The code in WebRTCModule+RTCMediaStream.m calls
[self.audioDeviceModule setMuted:!enabled error:&error] without verifying
self.audioDeviceModule is non-nil; update the conditional around the local audio
branch (pcId.intValue == -1 && [track.kind isEqualToString:@"audio"]) to first
check that self.audioDeviceModule != nil before calling setMuted:error:, and if
it's nil, skip the call and optionally NSLog a warning mentioning
audioDeviceModule is nil so callers can trace initialization issues; reference
self.audioDeviceModule and the setMuted:error: invocation when making the
change.
In `@ios/RCTWebRTC/WebRTCModule`+RTCPeerConnection.m:
- Around line 574-582: Add a defensive nil check for the audio device module
before calling setMuted:error: in the addTransceiver/mirror-audio block: ensure
self.audioDeviceModule is non-nil before invoking [self.audioDeviceModule
setMuted:!track.isEnabled error:&admError] (the same pattern used in
WebRTCModule+RTCMediaStream.m); if it's nil, skip the call or log a warning,
otherwise call setMuted and log admError.localizedDescription on failure.
In `@ios/RCTWebRTC/WebRTCModule`+Transceivers.m:
- Around line 74-82: Add a nil-check and extraction to avoid messaging nil
audioDeviceModule: update the logic that currently calls [self.audioDeviceModule
setMuted:!track.isEnabled error:&admError] (in WebRTCModule+Transceivers.m) to
first verify self.audioDeviceModule is non-nil and bail with a log if it is nil,
and refactor the repeated block (the track kind/audio mute sync) into a shared
helper method (e.g., syncADMWithTrackEnabled: or -syncADMMuteForTrack:) that
takes the RTCMediaStreamTrack *track, performs the nil-check for
self.audioDeviceModule and track.kind == @"audio", calls setMuted:error:, and
logs any admError using admError.localizedDescription.
In `@stream-react-native-webrtc.podspec`:
- Line 24: The podspec currently declares s.dependency 'StreamWebRTC', '=
145.10.0' which is invalid because StreamWebRTC is not a standalone CocoaPods
pod; replace that dependency with Stream’s CocoaPods pod that vendors the
XCFramework (use 'StreamVideo' instead of 'StreamWebRTC') and avoid exact "="
pinning — update the s.dependency line to reference 'StreamVideo' with a
flexible constraint (for example use a pessimistic operator like '~> x.y' or a
>= constraint appropriate for your supported release) so CocoaPods resolves the
correct packaged XCFramework.
---
Nitpick comments:
In `@ios/RCTWebRTC/Utils/AudioDeviceModule/AudioDeviceModule.swift`:
- Around line 252-254: The reset() implementation currently just calls
source.reset() and no longer touches ADM mute mode; add a brief explanatory
comment inside or above the AudioDeviceModule.reset() method indicating that
mute state is intentionally preserved across resets (e.g., because mute is now
managed at track-level or via setStereoPlayoutPreference) so future readers
understand this is deliberate and not a bug; reference the reset() method, the
call to source.reset(), and setStereoPlayoutPreference in the comment to clarify
the authority for mute handling.
In `@ios/RCTWebRTC/WebRTCModule`+RTCMediaStream.m:
- Around line 621-630: Extract the duplicated ADM mute-sync block into a shared
instance method on WebRTCModule named
-syncAudioTrackStateToADM:(RTCMediaStreamTrack *)track that checks track
non-null and kind == @"audio", verifies self.audioDeviceModule exists (log and
return if nil), and calls [self.audioDeviceModule setMuted:!track.isEnabled
error:&error] logging any error.localizedDescription; then replace the three
inlined implementations (including the current pcId.intValue == -1 audio branch
in WebRTCModule+RTCMediaStream.m) with a call to [self
syncAudioTrackStateToADM:track];.
In `@stream-react-native-webrtc.podspec`:
- Line 24: Change the exact version pin on the StreamWebRTC dependency to a
pessimistic constraint to allow patch updates; locate the s.dependency line that
currently reads "StreamWebRTC', '= 145.10.0" and update it to use the
pessimistic operator (e.g., "~> 145.10.0") so CocoaPods can resolve compatible
patch releases while keeping major.minor stability.
🪄 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: 63484d86-aa7f-4ea1-829a-2d9dba96192a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
ios/RCTWebRTC/Utils/AudioDeviceModule/AudioDeviceModule.swiftios/RCTWebRTC/WebRTCModule+RTCMediaStream.mios/RCTWebRTC/WebRTCModule+RTCPeerConnection.mios/RCTWebRTC/WebRTCModule+Transceivers.mios/RCTWebRTC/WebRTCModule.mpackage.jsonstream-react-native-webrtc.podspec
✅ Files skipped from review due to trivial changes (1)
- package.json
Comment on lines
+621
to
630
| } else if (pcId.intValue == -1 && [track.kind isEqualToString:@"audio"]) { | ||
| // Local microphone track. Mirror the enabled-state to the AudioDeviceModule so muting | ||
| // engages voice-processing mute (voiceProcessingInputMuted = true) while the input node | ||
| // keeps running. | ||
| NSError *error = nil; | ||
| if (![self.audioDeviceModule setMuted:!enabled error:&error]) { | ||
| NSLog(@"[WebRTCModule] Failed to mirror local audio enabled=%d to ADM mute: %@", | ||
| enabled, error.localizedDescription); | ||
| } | ||
| } |
There was a problem hiding this comment.
Add nil check for audioDeviceModule.
The code calls self.audioDeviceModule setMuted: without checking if audioDeviceModule is nil. If the ADM is not initialized, this could result in a null pointer access.
🛡️ Suggested defensive check
} else if (pcId.intValue == -1 && [track.kind isEqualToString:@"audio"]) {
// Local microphone track. Mirror the enabled-state to the AudioDeviceModule so muting
// engages voice-processing mute (voiceProcessingInputMuted = true) while the input node
// keeps running.
+ if (!self.audioDeviceModule) {
+ NSLog(@"[WebRTCModule] AudioDeviceModule not initialized, skipping mute sync");
+ return;
+ }
NSError *error = nil;
if (![self.audioDeviceModule setMuted:!enabled error:&error]) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } else if (pcId.intValue == -1 && [track.kind isEqualToString:@"audio"]) { | |
| // Local microphone track. Mirror the enabled-state to the AudioDeviceModule so muting | |
| // engages voice-processing mute (voiceProcessingInputMuted = true) while the input node | |
| // keeps running. | |
| NSError *error = nil; | |
| if (![self.audioDeviceModule setMuted:!enabled error:&error]) { | |
| NSLog(@"[WebRTCModule] Failed to mirror local audio enabled=%d to ADM mute: %@", | |
| enabled, error.localizedDescription); | |
| } | |
| } | |
| } else if (pcId.intValue == -1 && [track.kind isEqualToString:@"audio"]) { | |
| // Local microphone track. Mirror the enabled-state to the AudioDeviceModule so muting | |
| // engages voice-processing mute (voiceProcessingInputMuted = true) while the input node | |
| // keeps running. | |
| if (!self.audioDeviceModule) { | |
| NSLog(@"[WebRTCModule] AudioDeviceModule not initialized, skipping mute sync"); | |
| return; | |
| } | |
| NSError *error = nil; | |
| if (![self.audioDeviceModule setMuted:!enabled error:&error]) { | |
| NSLog(@"[WebRTCModule] Failed to mirror local audio enabled=%d to ADM mute: %@", | |
| enabled, error.localizedDescription); | |
| } | |
| } |
🤖 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 `@ios/RCTWebRTC/WebRTCModule`+RTCMediaStream.m around lines 621 - 630, The code
in WebRTCModule+RTCMediaStream.m calls [self.audioDeviceModule setMuted:!enabled
error:&error] without verifying self.audioDeviceModule is non-nil; update the
conditional around the local audio branch (pcId.intValue == -1 && [track.kind
isEqualToString:@"audio"]) to first check that self.audioDeviceModule != nil
before calling setMuted:error:, and if it's nil, skip the call and optionally
NSLog a warning mentioning audioDeviceModule is nil so callers can trace
initialization issues; reference self.audioDeviceModule and the setMuted:error:
invocation when making the change.
Comment on lines
+574
to
+582
| // Mirror a local audio track's enabled state to the ADM. On rejoin/SFU-migration the | ||
| // SDK republishes via addTransceiver on a fresh Publisher | ||
| if (track && [track.kind isEqualToString:@"audio"] && self.localTracks[trackId]) { | ||
| NSError *admError = nil; | ||
| if (![self.audioDeviceModule setMuted:!track.isEnabled error:&admError]) { | ||
| NSLog(@"[WebRTCModule] Failed to sync ADM mute on audio addTransceiver: %@", | ||
| admError.localizedDescription); | ||
| } | ||
| } |
There was a problem hiding this comment.
Add nil check for audioDeviceModule.
Similar to the sync in WebRTCModule+RTCMediaStream.m, this code calls self.audioDeviceModule setMuted: without verifying the ADM is initialized.
🛡️ Suggested defensive check
if (track && [track.kind isEqualToString:@"audio"] && self.localTracks[trackId]) {
+ if (!self.audioDeviceModule) {
+ NSLog(@"[WebRTCModule] AudioDeviceModule not initialized, skipping mute sync");
+ } else {
NSError *admError = nil;
if (![self.audioDeviceModule setMuted:!track.isEnabled error:&admError]) {
NSLog(@"[WebRTCModule] Failed to sync ADM mute on audio addTransceiver: %@",
admError.localizedDescription);
}
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Mirror a local audio track's enabled state to the ADM. On rejoin/SFU-migration the | |
| // SDK republishes via addTransceiver on a fresh Publisher | |
| if (track && [track.kind isEqualToString:@"audio"] && self.localTracks[trackId]) { | |
| NSError *admError = nil; | |
| if (![self.audioDeviceModule setMuted:!track.isEnabled error:&admError]) { | |
| NSLog(@"[WebRTCModule] Failed to sync ADM mute on audio addTransceiver: %@", | |
| admError.localizedDescription); | |
| } | |
| } | |
| // Mirror a local audio track's enabled state to the ADM. On rejoin/SFU-migration the | |
| // SDK republishes via addTransceiver on a fresh Publisher | |
| if (track && [track.kind isEqualToString:@"audio"] && self.localTracks[trackId]) { | |
| if (!self.audioDeviceModule) { | |
| NSLog(@"[WebRTCModule] AudioDeviceModule not initialized, skipping mute sync"); | |
| } else { | |
| NSError *admError = nil; | |
| if (![self.audioDeviceModule setMuted:!track.isEnabled error:&admError]) { | |
| NSLog(@"[WebRTCModule] Failed to sync ADM mute on audio addTransceiver: %@", | |
| admError.localizedDescription); | |
| } | |
| } | |
| } |
🤖 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 `@ios/RCTWebRTC/WebRTCModule`+RTCPeerConnection.m around lines 574 - 582, Add a
defensive nil check for the audio device module before calling setMuted:error:
in the addTransceiver/mirror-audio block: ensure self.audioDeviceModule is
non-nil before invoking [self.audioDeviceModule setMuted:!track.isEnabled
error:&admError] (the same pattern used in WebRTCModule+RTCMediaStream.m); if
it's nil, skip the call or log a warning, otherwise call setMuted and log
admError.localizedDescription on failure.
Comment on lines
+74
to
+82
| if (track != nil && [track.kind isEqualToString:@"audio"]) { | ||
|
|
||
| // Mirror the track's enabled state to the ADM so voice-processing mute is cleared on unmute | ||
| NSError *admError = nil; | ||
| if (![self.audioDeviceModule setMuted:!track.isEnabled error:&admError]) { | ||
| NSLog(@"[WebRTCModule] Failed to sync ADM mute on audio replaceTrack: %@", | ||
| admError.localizedDescription); | ||
| } | ||
| } |
There was a problem hiding this comment.
Add nil check for audioDeviceModule.
This is the third location that syncs track enabled state to the ADM without checking if audioDeviceModule is initialized. Consider extracting this repeated logic into a shared helper method.
🛡️ Suggested defensive check
if (track != nil && [track.kind isEqualToString:@"audio"]) {
// Mirror the track's enabled state to the ADM so voice-processing mute is cleared on unmute
+ if (!self.audioDeviceModule) {
+ NSLog(@"[WebRTCModule] AudioDeviceModule not initialized, skipping mute sync");
+ } else {
NSError *admError = nil;
if (![self.audioDeviceModule setMuted:!track.isEnabled error:&admError]) {
NSLog(@"[WebRTCModule] Failed to sync ADM mute on audio replaceTrack: %@",
admError.localizedDescription);
}
+ }
}🤖 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 `@ios/RCTWebRTC/WebRTCModule`+Transceivers.m around lines 74 - 82, Add a
nil-check and extraction to avoid messaging nil audioDeviceModule: update the
logic that currently calls [self.audioDeviceModule setMuted:!track.isEnabled
error:&admError] (in WebRTCModule+Transceivers.m) to first verify
self.audioDeviceModule is non-nil and bail with a log if it is nil, and refactor
the repeated block (the track kind/audio mute sync) into a shared helper method
(e.g., syncADMWithTrackEnabled: or -syncADMMuteForTrack:) that takes the
RTCMediaStreamTrack *track, performs the nil-check for self.audioDeviceModule
and track.kind == @"audio", calls setMuted:error:, and logs any admError using
admError.localizedDescription.
Member
Author
There was a problem hiding this comment.
ADM is always initialized; the message send is nil-safe.
There was a problem hiding this comment.
@santhoshvai, that's a fair point on both counts — audioDeviceModule is guaranteed to be initialized, and Objective-C's nil-safe message dispatch means there's no crash risk even if it weren't. I'll withdraw the suggestion.
✏️ Learnings added
Learnt from: santhoshvai
Repo: GetStream/react-native-webrtc PR: 27
File: ios/RCTWebRTC/WebRTCModule+Transceivers.m:74-82
Timestamp: 2026-06-10T16:04:53.178Z
Learning: In `ios/RCTWebRTC/WebRTCModule+Transceivers.m` (and related iOS files in the GetStream/react-native-webrtc repo), `self.audioDeviceModule` is always initialized by the time it is messaged. Additionally, Objective-C nil-safe messaging applies, so defensive nil-checks before `[self.audioDeviceModule setMuted:error:]` calls are unnecessary.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
oliverlaz
approved these changes
Jun 11, 2026
santhoshvai
added a commit
to GetStream/stream-video-js
that referenced
this pull request
Jun 11, 2026
## Overview * Bump @stream-io/react-native-webrtc to 145.0.0 (depends on [rn-webrtc#27](GetStream/react-native-webrtc#27)). * iOS: engine-lifecycle–driven audio session reconfiguration for interruption recovery. * CallKit path: session ownership guard between callingx and StreamInCallManager. * Fixes blank video on iOS PiP (only seen after 145) ## Test plan - [x] Dogfood iOS: join call, verify remote video appears quickly. - [x] Go to PiP on iOS verifiy it works - [x] Interrupt with Phone call/ Facetime / Siri; verify audio resumes. - [x] Ringing: answer via CallKit; verify audio + no session conflict when moving to in-call UI. Interrupt with Phone call/ Facetime. Siri only works without callkit. - [x] Android: smoke call + noise cancellation. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Mic mute detection and new audio-interruption/mic-mute events. * Cross-module session ownership for safer CallKit/audio handoffs. * **Bug Fixes** * More reliable audio session and engine lifecycle handling. * Improved picture‑in‑picture rendering and robust video buffer conversion/resizing. * **Chores** * Bumped WebRTC-related dependency to 145.0.0-alpha.4. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Oliver Lazoroski <oliver.lazoroski@gmail.com> Co-authored-by: Ivan Sekovanikj <ivan.sekovanikj@getstream.io>
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
3 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.
Webrtc 145 update
Underlying webrtc changelog here
Other updates
ref: [Enhancement]Improve frame rendering webrtc#70
update-1: fix: fixes objectFit="cover" rendering on iOS using the shared Metal backend webrtc#82 - fix in m145
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Breaking / Removed