feat: hi-fi audio - #50
Open
greenfrvr wants to merge 10 commits into
Open
Conversation
📝 WalkthroughWalkthroughThis PR adds per-call ChangesFactory lifecycle, media routing, and camera preview
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant App
participant CallFactory
participant WebRTCModule
participant PeerConnectionFactoryRegistry
App->>CallFactory: create(options)
CallFactory->>WebRTCModule: createCallFactory(options)
WebRTCModule->>PeerConnectionFactoryRegistry: create(bypassVoiceProcessing)
PeerConnectionFactoryRegistry-->>WebRTCModule: provider
App->>CallFactory: dispose()
CallFactory->>WebRTCModule: disposeCallFactory()
WebRTCModule->>PeerConnectionFactoryRegistry: disposeCurrent()
sequenceDiagram
participant App
participant CameraPreview
participant WebRTCModule
participant MediaTrackCreation
App->>CameraPreview: activate with camera props
CameraPreview->>WebRTCModule: register active preview
MediaTrackCreation->>WebRTCModule: request preview adoption
WebRTCModule->>CameraPreview: adopt or yield capture
CameraPreview-->>MediaTrackCreation: capture handoff
MediaTrackCreation->>MediaTrackCreation: attach capture to video track
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
greenfrvr
marked this pull request as ready for review
July 17, 2026 13:05
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/src/main/java/com/oney/WebRTCModule/GetUserMediaImpl.java (1)
241-260: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winValidate video prerequisites before creating the audio track.
For an audio-and-video request with no current Activity, the audio source and track are registered before this early return. Since no track ID reaches JavaScript, they remain leaked until factory or module teardown. Resolve the Activity first or dispose the partially created audio track before invoking the error callback.
🤖 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/GetUserMediaImpl.java` around lines 241 - 260, Update getUserMedia around createAudioTrack and the currentActivity null check so video prerequisites are validated before creating and registering the audio track. Preserve the existing InvalidStateError callback and return behavior, ensuring audio-only requests still create their track normally.
🤖 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/FanoutCapturerObserver.java`:
- Around line 49-53: Update FanoutCapturerObserver.setDownstream and the
capturer lifecycle and frame-delivery methods to use one shared lock or
executor, serializing observer attachment with started-state callbacks and
downstream frame delivery. Ensure assignment and any onCapturerStarted
notification occur atomically with respect to concurrent start/stop operations,
preventing missed or incorrect lifecycle notifications.
In
`@android/src/main/java/com/oney/WebRTCModule/PeerConnectionFactoryProvider.java`:
- Around line 188-217: Update mixScreenAudioIntoBuffer so mixing is limited to
the writable microphone sample count, micSamples. Iterate only across
overlapping samples and remove the screen-only branch that writes beyond
micShorts’ range; preserve saturation for summed microphone and screen samples.
- Around line 87-89: Track whether the ADM in the factory was created by
buildAudioDeviceModule or supplied through options.injectedAudioDeviceModule,
and only release owned instances in dispose(); preserve shared injected
instances for reuse by later factories. In mixScreenAudioIntoBuffer(), clamp the
write loop to the mic buffer/sample count so screen audio cannot cause
micShorts.put to exceed its capacity.
In
`@android/src/main/java/com/oney/WebRTCModule/PeerConnectionFactoryRegistry.java`:
- Around line 93-101: Update the replacement logic around currentFactory and
currentIsBareForkDefault to never dispose a live default factory directly.
Reject creation of the call factory while that default or its dependents remain
active, or route replacement through WebRTCModule’s ordered dependent cleanup
and disposeCallFactory before creating the new factory.
- Around line 102-105: Update the concurrent-factory branch in createCallFactory
to throw an appropriate exception instead of logging a warning and returning
existing. Ensure the native promise rejects and the second CallFactory cannot
proceed with ignored options or dispose the shared factory.
In `@android/src/main/java/com/oney/WebRTCModule/RTCCameraPreviewView.java`:
- Around line 115-125: Update commitProps and its running-configuration
comparison to snapshot captureWidth and captureHeight alongside facing and
device ID, then treat either dimension changing as configChanged so active
capture is restarted or reconfigured. Ensure the setters’ updated dimensions are
reflected in the committed configuration.
- Around line 241-261: Update yieldForAdoption() so handedOff and
clearActivePreview() are performed inside the captureExecutor task only after
confirming running, captureController, surfaceTextureHelper, and fanoutObserver
are all available. Do not mark the preview handed off or clear the active
preview before the prerequisite check succeeds, and preserve the existing
ownership transfer and null return behavior.
- Around line 268-280: Move the `fanoutObserver` snapshot and related
renderer-detachment decision into the `captureExecutor` task, so
`fanoutObserver` is read on its owning executor before calling
`fanout.setRenderer(null)`. Preserve the existing `releaseRenderer` behavior and
ensure detachment occurs for adopted captures before
`surfaceViewRenderer.release()`.
In `@android/src/main/java/com/oney/WebRTCModule/WebRTCModule.java`:
- Around line 64-65: Make activeCameraPreview thread-safe across React lifecycle
callbacks and WebRTC executor access: synchronize or otherwise safely publish
writes and reads, including the getUserMedia path and the callbacks around lines
275-287. Ensure getUserMedia observes the currently active preview before
attempting to open another camera.
- Around line 153-185: Update disposeCallFactory() to abort factory disposal
when any owned PeerConnection or track fails to dispose, rather than continuing
to factoryRegistry.disposeCurrent(). Track failures, PeerConnection failures,
and any remaining local streams must cause the promise to reject while leaving
the factory and ADM undisposed; only resolve after all dependents are
successfully removed and the ordering invariant is satisfied.
In `@ios/RCTWebRTC/RTCCameraPreviewViewManager.m`:
- Around line 195-228: Serialize all `_captureController` access in the capture
queue: update the start, stop, and constraint-update branches in
`RTCameraPreviewViewManager` to resolve the controller inside `_captureQueue`
rather than snapshotting it beforehand. Before creating or starting capture,
re-check the latest `isActive`/`_capturing` state and abort or stop when
inactive, while preserving queued stop and constraint updates.
- Around line 204-207: Update the fresh preview-controller initialization in the
capture setup around _captureController and VideoCaptureController so the
existing enableMultitaskingCameraAccess setting is also passed to an adopted
preview controller. Preserve the current capturer and constraints initialization
while ensuring both fresh-track and handoff paths use the same multitasking
camera access option.
In `@ios/RCTWebRTC/WebRTCModule.m`:
- Around line 215-219: Update disposeCallFactory to dispose all factory-owned
peer connections and media tracks before invoking self.factoryRegistry
disposeCurrent. Preserve the existing promise resolution, and ensure factory
teardown occurs only after those dependent WebRTC objects have been released.
In `@src/CallFactory.ts`:
- Around line 25-38: Scope each CallFactory handle to its native factory ID: in
src/CallFactory.ts lines 25-38, store the ID returned by createCallFactory and
pass that ID from dispose; in ios/RCTWebRTC/WebRTCModule.m lines 202-219,
resolve creation with the provider ID and require it when disposing; in
ios/RCTWebRTC/Utils/PeerConnectionFactory/PeerConnectionFactoryRegistry.swift
lines 66-86, reject a second live factory instead of reusing the existing
provider and only dispose when the requested ID matches.
---
Outside diff comments:
In `@android/src/main/java/com/oney/WebRTCModule/GetUserMediaImpl.java`:
- Around line 241-260: Update getUserMedia around createAudioTrack and the
currentActivity null check so video prerequisites are validated before creating
and registering the audio track. Preserve the existing InvalidStateError
callback and return behavior, ensuring audio-only requests still create their
track normally.
🪄 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: e0f04110-1d20-4236-b823-20142a7d54de
📒 Files selected for processing (19)
android/src/main/java/com/oney/WebRTCModule/FanoutCapturerObserver.javaandroid/src/main/java/com/oney/WebRTCModule/GetUserMediaImpl.javaandroid/src/main/java/com/oney/WebRTCModule/PeerConnectionFactoryProvider.javaandroid/src/main/java/com/oney/WebRTCModule/PeerConnectionFactoryRegistry.javaandroid/src/main/java/com/oney/WebRTCModule/RTCCameraPreviewView.javaandroid/src/main/java/com/oney/WebRTCModule/RTCCameraPreviewViewManager.javaandroid/src/main/java/com/oney/WebRTCModule/WebRTCModule.javaandroid/src/main/java/com/oney/WebRTCModule/WebRTCModulePackage.javaios/RCTWebRTC/RTCCameraPreviewViewManager.hios/RCTWebRTC/RTCCameraPreviewViewManager.mios/RCTWebRTC/Utils/AudioDeviceModule/AudioDeviceModule.swiftios/RCTWebRTC/Utils/PeerConnectionFactory/PeerConnectionFactoryProvider.swiftios/RCTWebRTC/Utils/PeerConnectionFactory/PeerConnectionFactoryRegistry.swiftios/RCTWebRTC/WebRTCModule+RTCMediaStream.mios/RCTWebRTC/WebRTCModule.hios/RCTWebRTC/WebRTCModule.msrc/CallFactory.tssrc/RTCCameraPreviewView.tssrc/index.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/src/main/java/com/oney/WebRTCModule/PeerConnectionFactoryProvider.java (1)
84-96: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRelease
admif factory construction fails.
dispose()cleans up the module on the normal path, but ifcreatePeerConnectionFactory()throws, this native resource is left allocated. Wrap factory construction and releaseadmon the failure path.🤖 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/PeerConnectionFactoryProvider.java` around lines 84 - 96, Update the factory construction flow around PeerConnectionFactory.Builder.createPeerConnectionFactory() to release the AudioDeviceModule via adm.release() if construction throws, then rethrow the original failure. Preserve the existing successful return path and normal dispose() ownership 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.
Outside diff comments:
In
`@android/src/main/java/com/oney/WebRTCModule/PeerConnectionFactoryProvider.java`:
- Around line 84-96: Update the factory construction flow around
PeerConnectionFactory.Builder.createPeerConnectionFactory() to release the
AudioDeviceModule via adm.release() if construction throws, then rethrow the
original failure. Preserve the existing successful return path and normal
dispose() ownership behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1255f264-1c0e-4f71-8f78-3c91e8a18a5b
📒 Files selected for processing (7)
Documentation/AndroidInstallation.mdandroid/src/main/java/com/oney/WebRTCModule/PeerConnectionFactoryProvider.javaandroid/src/main/java/com/oney/WebRTCModule/PeerConnectionFactoryRegistry.javaandroid/src/main/java/com/oney/WebRTCModule/WebRTCModule.javaandroid/src/main/java/com/oney/WebRTCModule/WebRTCModuleOptions.javaios/RCTWebRTC/RTCCameraPreviewViewManager.mios/RCTWebRTC/WebRTCModule.m
💤 Files with no reviewable changes (2)
- android/src/main/java/com/oney/WebRTCModule/WebRTCModuleOptions.java
- Documentation/AndroidInstallation.md
🚧 Files skipped from review as they are similar to previous changes (4)
- android/src/main/java/com/oney/WebRTCModule/PeerConnectionFactoryRegistry.java
- ios/RCTWebRTC/WebRTCModule.m
- ios/RCTWebRTC/RTCCameraPreviewViewManager.m
- android/src/main/java/com/oney/WebRTCModule/WebRTCModule.java
santhoshvai
approved these changes
Aug 7, 2026
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
android/src/main/java/com/oney/WebRTCModule/PeerConnectionFactoryProvider.java (2)
103-112: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winApply the bypass voice-profile capture settings.
When
bypassVoiceProcessingis true,stereoInputEnabledcurrently has no effect and the audio source remains the default voice-communication path instead of rawMediaRecorder.AudioSource.MIC. Enable both builders matching the documented profile, or remove the options and update callers.Proposed fix
builder.setUseHardwareAcousticEchoCanceler(false) .setUseHardwareNoiseSuppressor(false) .setUseStereoOutput(true) - // .setUseStereoInput(options.stereoInputEnabled) - // .setAudioSource(MediaRecorder.AudioSource.MIC) + .setUseStereoInput(options.stereoInputEnabled) + .setAudioSource(MediaRecorder.AudioSource.MIC) .setOutputSampleRate(nativeOutputSampleRate(options.context));🤖 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/PeerConnectionFactoryProvider.java` around lines 103 - 112, Update the bypassVoiceProcessing branch in PeerConnectionFactoryProvider so it applies options.stereoInputEnabled through setUseStereoInput and selects MediaRecorder.AudioSource.MIC via setAudioSource, while preserving the existing hardware-processing, stereo-output, and native sample-rate settings.
84-96: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRelease
admwhenbuild()fails.
createAudioDeviceModule()hands callers ownership of native audio resources. If the PeerConnectionFactory builder setup orcreatePeerConnectionFactory()throws,PeerConnectionFactoryProvider.build()returns without owning theAudioDeviceModule, soadm.release()is never called. Catch and wrap the failure path to releaseadm, then rethrow the original exception.🤖 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/PeerConnectionFactoryProvider.java` around lines 84 - 96, Update PeerConnectionFactoryProvider.build() to guard the PeerConnectionFactory builder setup and createPeerConnectionFactory() call with failure handling that invokes adm.release() when any step throws, then rethrows the original exception without altering successful creation 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.
Outside diff comments:
In
`@android/src/main/java/com/oney/WebRTCModule/PeerConnectionFactoryProvider.java`:
- Around line 103-112: Update the bypassVoiceProcessing branch in
PeerConnectionFactoryProvider so it applies options.stereoInputEnabled through
setUseStereoInput and selects MediaRecorder.AudioSource.MIC via setAudioSource,
while preserving the existing hardware-processing, stereo-output, and native
sample-rate settings.
- Around line 84-96: Update PeerConnectionFactoryProvider.build() to guard the
PeerConnectionFactory builder setup and createPeerConnectionFactory() call with
failure handling that invokes adm.release() when any step throws, then rethrows
the original exception without altering successful creation behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 696ae0ab-38b6-4699-9bd2-bff0f9a84e58
📒 Files selected for processing (1)
android/src/main/java/com/oney/WebRTCModule/PeerConnectionFactoryProvider.java
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.
Summary by CodeRabbit
CallFactoryAPI for creating and disposing call resources.