Conversation
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds cross-platform screen-share audio mixing: Android MediaProjection-based capture and RN bridge methods, iOS in‑app capturer + mixer integration, a JS manager and hook to control lifecycle and noise‑cancellation, UI prop to opt into in‑app/broadcast modes with audio, and sample app wiring. Changes
Sequence DiagramsequenceDiagram
participant User
participant Button as ScreenShareToggleButton
participant Hook as useScreenShareButton
participant MixerHook as useScreenShareAudioMixing
participant Manager as ScreenShareAudioManager
participant Native as NativeModule
participant Capture as ScreenAudioCapture / ScreenShareAudioMixer
User->>Button: Tap screen share
Button->>Hook: start flow with screenShareOptions
Hook->>Hook: set includeAudio / type
Hook->>MixerHook: update desired mixing state
alt iOS in‑app + includeAudio
Hook->>Manager: startInAppScreenCapture(includeAudio)
Manager->>Native: startInAppScreenCapture()
Native->>Capture: init in‑app capturer
else Android or broadcast
Hook->>Hook: request MediaProjection / show system picker
end
alt mixing requested
MixerHook->>Manager: startScreenShareAudioMixing()
Manager->>Native: startScreenShareAudioMixing()
Native->>Capture: start capture / mixer
Capture->>Native: supply audio bytes to WebRTC mic path
end
User->>Button: Stop share
Hook->>MixerHook: stop mixing
MixerHook->>Manager: stopScreenShareAudioMixing()
Manager->>Native: stopScreenShareAudioMixing()
Native->>Capture: stop and cleanup
alt iOS in‑app
Hook->>Manager: stopInAppScreenCapture()
Manager->>Native: stopInAppScreenCapture()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 |
greenfrvr
requested review from
oliverlaz and
santhoshvai
and removed request for
oliverlaz
![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: 1
🧹 Nitpick comments (3)
packages/react-native-sdk/src/hooks/useScreenShareButton.ts (1)
156-167: Inconsistent handler invocation pattern.In the iOS in-app branch (line 163), the handler is called directly as
onScreenShareStartedHandler?.(), while in the broadcast mode event listener (line 120), it's called via the ref asonScreenShareStartedHandlerRef.current?.(). The ref pattern was intentionally used to avoid stale closure issues in the effect. Consider using the ref consistently.💡 Suggested fix for consistency
await screenShareAudioMixingManager.startInAppScreenCapture( includeAudio, ); await call?.screenShare.enable(); - onScreenShareStartedHandler?.(); + onScreenShareStartedHandlerRef.current?.(); } catch (error) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-native-sdk/src/hooks/useScreenShareButton.ts` around lines 156 - 167, The iOS in-app branch in useScreenShareButton calls the start handler directly (onScreenShareStartedHandler?.()), which is inconsistent with the broadcast branch that uses the ref to avoid stale closures; change the direct invocation to use the ref (onScreenShareStartedHandlerRef.current?.()) after successful start so both branches use the same onScreenShareStartedHandlerRef pattern used elsewhere in useScreenShareButton.packages/react-native-sdk/ios/StreamVideoReactNative.m (1)
688-727: Consider handling the case whencaptureris nil.When
activeInAppScreenCapturerisnil, the method still starts mixing and bypasses voice processing but never sets up theaudioBufferHandler. This could leave the system in a partially configured state where mixing is active but no audio data flows.💡 Suggested improvement
// Wire audio buffer handler on the active capturer → mixer.enqueue InAppScreenCapturer *capturer = options.activeInAppScreenCapturer; if (capturer) { capturer.audioBufferHandler = ^(CMSampleBufferRef sampleBuffer) { ScreenShareAudioMixer *currentMixer = [WebRTCModuleOptions sharedInstance].screenShareAudioMixer; if (currentMixer) { [currentMixer enqueue:sampleBuffer]; } }; + } else { + // No active capturer — log warning but continue; audio may start flowing later + NSLog(@"[StreamVideoReactNative] startScreenShareAudioMixing: No active capturer available"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-native-sdk/ios/StreamVideoReactNative.m` around lines 688 - 727, The method startScreenShareAudioMixing currently starts mixing and bypasses voice processing before checking activeInAppScreenCapturer, which can leave mixing active with no audio handler; modify startScreenShareAudioMixing (and use WebRTCModuleOptions, InAppScreenCapturer, startMixing, setVoiceProcessingBypassed, audioBufferHandler) to check options.activeInAppScreenCapturer before enabling mixing/VP bypass and, if nil, do not start mixing or bypass voice processing and instead reject the promise (or return an error) with a clear code/message; alternatively, if you must start mixing first, add cleanup logic to stopMixing and restore _vpBypassedBeforeMixing (via audioDeviceModule.setVoiceProcessingBypassed:) before rejecting so the system is not left partially configured.packages/react-native-sdk/android/src/main/java/com/streamvideo/reactnative/StreamVideoReactNativeModule.kt (1)
495-545: Consider checking ifScreenAudioCapture.start()actually succeeded.After
ScreenAudioCapture(mediaProjection).also { it.start() }on line 530, theaudioRecordinside may still benullif initialization failed (as handled inScreenAudioCapture.start()). ThescreenAudioBytesProvidercallback on line 536 will then always returnnull, but the method resolves successfully, potentially misleading the caller.💡 Suggested improvement to verify capture started
- screenAudioCapture = ScreenAudioCapture(mediaProjection).also { it.start() } + val capture = ScreenAudioCapture(mediaProjection) + capture.start() + + // Verify capture actually started (audioRecord initialized successfully) + if (capture.getScreenAudioBytes(0) == null) { + // This is a lightweight check - getScreenAudioBytes returns null when audioRecord is null + Log.w(NAME, "Screen audio capture may not have initialized correctly") + } + screenAudioCapture = captureAlternatively, expose an
isActiveproperty onScreenAudioCaptureto check initialization state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-native-sdk/android/src/main/java/com/streamvideo/reactnative/StreamVideoReactNativeModule.kt` around lines 495 - 545, The startScreenShareAudioMixing method currently assumes ScreenAudioCapture.start() succeeded; change the flow to verify the capture actually started before resolving and setting WebRTCModuleOptions.screenAudioBytesProvider: after creating ScreenAudioCapture (symbol: ScreenAudioCapture) call start() and check a success indicator (either a boolean return from start(), an exposed property like isActive on ScreenAudioCapture, or by verifying internal audioRecord != null on the instance referenced by screenAudioCapture) and if initialization failed reject the promise with a descriptive error and do not assign screenAudioBytesProvider; only assign the provider, log success, and resolve the promise when the capture is confirmed active (references: function startScreenShareAudioMixing, field screenAudioCapture, method ScreenAudioCapture.start(), and WebRTCModuleOptions.screenAudioBytesProvider).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/react-native-sdk/src/hooks/useScreenShareAudioMixing.ts`:
- Around line 107-129: The cleanup effect can miss stopping in-progress mixing
because isMixingActiveRef.current is set after an await in startMixing; ensure
the component can cancel an in-flight startMixing on unmount by adding an
AbortController (or set isMixingActiveRef.current = true before awaiting) inside
startMixing and/or the start/stop useEffect, pass the controller/signal to any
async media calls, and in the cleanup use the controller.abort() then call
screenShareAudioMixingManager.stopScreenShareAudioMixing() and
restoreNoiseCancellation() if needed; update references to startMixing,
isMixingActiveRef, ncWasEnabledRef,
screenShareAudioMixingManager.stopScreenShareAudioMixing, and
restoreNoiseCancellation to use the abort signal and ensure deterministic
cleanup.
---
Nitpick comments:
In
`@packages/react-native-sdk/android/src/main/java/com/streamvideo/reactnative/StreamVideoReactNativeModule.kt`:
- Around line 495-545: The startScreenShareAudioMixing method currently assumes
ScreenAudioCapture.start() succeeded; change the flow to verify the capture
actually started before resolving and setting
WebRTCModuleOptions.screenAudioBytesProvider: after creating ScreenAudioCapture
(symbol: ScreenAudioCapture) call start() and check a success indicator (either
a boolean return from start(), an exposed property like isActive on
ScreenAudioCapture, or by verifying internal audioRecord != null on the instance
referenced by screenAudioCapture) and if initialization failed reject the
promise with a descriptive error and do not assign screenAudioBytesProvider;
only assign the provider, log success, and resolve the promise when the capture
is confirmed active (references: function startScreenShareAudioMixing, field
screenAudioCapture, method ScreenAudioCapture.start(), and
WebRTCModuleOptions.screenAudioBytesProvider).
In `@packages/react-native-sdk/ios/StreamVideoReactNative.m`:
- Around line 688-727: The method startScreenShareAudioMixing currently starts
mixing and bypasses voice processing before checking activeInAppScreenCapturer,
which can leave mixing active with no audio handler; modify
startScreenShareAudioMixing (and use WebRTCModuleOptions, InAppScreenCapturer,
startMixing, setVoiceProcessingBypassed, audioBufferHandler) to check
options.activeInAppScreenCapturer before enabling mixing/VP bypass and, if nil,
do not start mixing or bypass voice processing and instead reject the promise
(or return an error) with a clear code/message; alternatively, if you must start
mixing first, add cleanup logic to stopMixing and restore
_vpBypassedBeforeMixing (via audioDeviceModule.setVoiceProcessingBypassed:)
before rejecting so the system is not left partially configured.
In `@packages/react-native-sdk/src/hooks/useScreenShareButton.ts`:
- Around line 156-167: The iOS in-app branch in useScreenShareButton calls the
start handler directly (onScreenShareStartedHandler?.()), which is inconsistent
with the broadcast branch that uses the ref to avoid stale closures; change the
direct invocation to use the ref (onScreenShareStartedHandlerRef.current?.())
after successful start so both branches use the same
onScreenShareStartedHandlerRef pattern used elsewhere in useScreenShareButton.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4af24a58-88d7-47a5-a055-a48049448684
📒 Files selected for processing (10)
packages/react-native-sdk/android/src/main/java/com/streamvideo/reactnative/StreamVideoReactNativeModule.ktpackages/react-native-sdk/android/src/main/java/com/streamvideo/reactnative/screenshare/ScreenAudioCapture.ktpackages/react-native-sdk/ios/StreamVideoReactNative.mpackages/react-native-sdk/src/components/Call/CallControls/ScreenShareToggleButton.tsxpackages/react-native-sdk/src/hooks/index.tspackages/react-native-sdk/src/hooks/useScreenShareAudioMixing.tspackages/react-native-sdk/src/hooks/useScreenShareButton.tspackages/react-native-sdk/src/modules/ScreenShareAudioManager.tspackages/react-native-sdk/src/providers/StreamCall/index.tsxsample-apps/react-native/dogfood/src/components/CallControlls/BottomControls.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/react-native-sdk/ios/StreamVideoReactNative.m (1)
676-683: Make in-app stop path defensively teardown mixer wiring
stopInAppScreenCapture(Line 676-683) only flips option flags. If JS call order breaks or partial failures occur, handler/delegate cleanup may be skipped and mixing can remain wired longer than intended. Prefer idempotent teardown from this path too.Also applies to: 718-743
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-native-sdk/ios/StreamVideoReactNative.m` around lines 676 - 683, The stopInAppScreenCapture method currently only flips WebRTCModuleOptions flags; update it to perform an idempotent teardown of any mixer/wiring and handlers so lingering delegates or active mixing cannot persist: call the shared module's cleanup/stop methods (e.g., stop any in-app capture session, disconnect the ScreenShareMixer or mixController, remove/nil out delegate references and observers) and make those teardown calls safe to run multiple times; ensure you reference WebRTCModuleOptions sharedInstance plus the relevant mixer/manager instance (the module or ScreenShareMixer/mixController used elsewhere) and invoke its stop/teardown and delegate-nil methods before resolving the promise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/react-native-sdk/ios/StreamVideoReactNative.m`:
- Around line 687-716: Ensure startScreenShareAudioMixing and
stopScreenShareAudioMixing validate prerequisites before resolving: check
WebRTCModule (via [self.bridge moduleForClass:[WebRTCModule class]]), its
audioDeviceModule, and the screenShareAudioMixer and reject the promise if any
are nil; verify WebRTCModuleOptions.sharedInstance.includeScreenShareAudio is
true at the start and reject if false; when wiring the audio processing module,
ensure options.audioProcessingModule is of class RTCDefaultAudioProcessingModule
and reject if not (do not silently continue), and return early after calling
reject in each failure path so JS state stays consistent with native state.
- Around line 10-19: The import of InAppScreenCapturer.h in
StreamVideoReactNative.m is unconditional while the Swift header is guarded, and
the podspec lacks the version constraint for stream-react-native-webrtc that
package.json requires; update stream-video-react-native.podspec to add the
version constraint (>= 137.1.0) for the stream-react-native-webrtc dependency
and also wrap the InAppScreenCapturer.h import in StreamVideoReactNative.m with
the same `#if` __has_include guard (matching the Swift header guard) so the
Objective-C header is only included when available.
---
Nitpick comments:
In `@packages/react-native-sdk/ios/StreamVideoReactNative.m`:
- Around line 676-683: The stopInAppScreenCapture method currently only flips
WebRTCModuleOptions flags; update it to perform an idempotent teardown of any
mixer/wiring and handlers so lingering delegates or active mixing cannot
persist: call the shared module's cleanup/stop methods (e.g., stop any in-app
capture session, disconnect the ScreenShareMixer or mixController, remove/nil
out delegate references and observers) and make those teardown calls safe to run
multiple times; ensure you reference WebRTCModuleOptions sharedInstance plus the
relevant mixer/manager instance (the module or ScreenShareMixer/mixController
used elsewhere) and invoke its stop/teardown and delegate-nil methods before
resolving the promise.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 401622be-bd9e-405f-b7e0-dc25842c573a
📒 Files selected for processing (1)
packages/react-native-sdk/ios/StreamVideoReactNative.m
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/react-native-sdk/src/hooks/useScreenShareButton.ts (1)
165-165: Inconsistent handler invocation: direct call vs ref.Lines 120 and 129 use
onScreenShareStartedHandlerRef.current?.()andonScreenShareStoppedHandlerRef.current?.()to avoid stale closure issues in the effect. However, lines 165 and 191 call the handlers directly (onScreenShareStartedHandler?.()andonScreenShareStoppedHandler?.()). This inconsistency is safe withinonPresssince it's not captured in a closure, but using the refs consistently would be clearer.♻️ Suggested consistency fix
- onScreenShareStartedHandler?.(); + onScreenShareStartedHandlerRef.current?.();And at line 191:
- onScreenShareStoppedHandler?.(); + onScreenShareStoppedHandlerRef.current?.();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-native-sdk/src/hooks/useScreenShareButton.ts` at line 165, The onPress handler in useScreenShareButton calls the handlers directly (onScreenShareStartedHandler?.() and onScreenShareStoppedHandler?.()) which is inconsistent with other places using refs; update the onPress logic to invoke the ref-stored callbacks instead (use onScreenShareStartedHandlerRef.current?.() and onScreenShareStoppedHandlerRef.current?.()) and do the same for the stopped handler (onScreenShareStoppedHandlerRef.current?.()) so all invocations consistently use the ref-backed callbacks and avoid stale closure issues.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/react-native-sdk/src/hooks/useScreenShareButton.ts`:
- Around line 160-169: The code starts native capture with
screenShareAudioMixingManager.startInAppScreenCapture then enables call screen
share, but if call?.screenShare.enable() throws the native capture keeps
running; modify the try block so that after startInAppScreenCapture succeeds you
attempt call?.screenShare.enable() in its own try/catch and on any failure await
a cleanup call on the audio mixing manager (e.g.
screenShareAudioMixingManager.stopInAppScreenCapture or the appropriate stop
method) to stop native capture, catch/log any cleanup errors via
videoLoggerSystem.getLogger('useScreenShareButton'), and only call
onScreenShareStartedHandler after both startInAppScreenCapture and
call?.screenShare.enable() have completed successfully.
- Around line 154-156: The call to disableScreenShareAudio() in
useScreenShareButton is not awaited, causing a race with subsequent
screenShare.enable() calls; update the handler in useScreenShareButton to await
call?.screenShare.disableScreenShareAudio() (making the enclosing function async
if it isn't already) so the Promise resolves before calling screenShare.enable()
or proceeding with state changes; ensure error handling (try/catch) or propagate
the Promise as appropriate so failures don't leave the app in an inconsistent
state.
---
Nitpick comments:
In `@packages/react-native-sdk/src/hooks/useScreenShareButton.ts`:
- Line 165: The onPress handler in useScreenShareButton calls the handlers
directly (onScreenShareStartedHandler?.() and onScreenShareStoppedHandler?.())
which is inconsistent with other places using refs; update the onPress logic to
invoke the ref-stored callbacks instead (use
onScreenShareStartedHandlerRef.current?.() and
onScreenShareStoppedHandlerRef.current?.()) and do the same for the stopped
handler (onScreenShareStoppedHandlerRef.current?.()) so all invocations
consistently use the ref-backed callbacks and avoid stale closure issues.
🪄 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: d47ebc65-8d38-485e-84d2-7ee6065f6f31
📒 Files selected for processing (1)
packages/react-native-sdk/src/hooks/useScreenShareButton.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/react-native-sdk/src/hooks/useScreenShareButton.ts (1)
116-124:⚠️ Potential issue | 🟠 MajorAwait the screen-share startup calls before signaling success.
disableScreenShareAudio()is still fire-and-forget in the button handler, and the new broadcast-start effect adds the same pattern forenableScreenShareAudio()plusscreenShare.enable(). If any of those promises reject, this hook can emit the started callback before anything is actually published and leave an unhandled rejection on the event/effect path.🧭 Proposed fix
if ( iosScreenShareStartedFromSystem && !prevIosScreenShareStartedFromSystem ) { - onScreenShareStartedHandlerRef.current?.(); - if (includeAudio) { - call?.screenShare.enableScreenShareAudio(); - } - call?.screenShare.enable(); + void (async () => { + try { + if (includeAudio) { + await call?.screenShare.enableScreenShareAudio(); + } + await call?.screenShare.enable(); + onScreenShareStartedHandlerRef.current?.(); + } catch (error) { + videoLoggerSystem + .getLogger('useScreenShareButton') + .warn('Failed to publish iOS broadcast screen share', error); + } + })(); } else if ( !iosScreenShareStartedFromSystem && prevIosScreenShareStartedFromSystem ) { onScreenShareStoppedHandlerRef.current?.(); @@ if (!hasPublishedScreenShare) { - // Set audio mixing preference before starting screen share - if (includeAudio) { - call?.screenShare.enableScreenShareAudio(); - } else { - call?.screenShare.disableScreenShareAudio(); - } + try { + if (includeAudio) { + await call?.screenShare.enableScreenShareAudio(); + } else { + await call?.screenShare.disableScreenShareAudio(); + } + } catch (error) { + videoLoggerSystem + .getLogger('useScreenShareButton') + .warn('Failed to configure screen share audio', error); + return; + }Run this read-only check to confirm the method signatures and current call sites:
#!/bin/bash rg -n -C3 --type=ts --type=tsx '\b(enableScreenShareAudio|disableScreenShareAudio)\s*\(' printf '\n--- useScreenShareButton call sites ---\n' rg -n -C3 --type=ts --type=tsx '\bscreenShare\.enable\s*\(|\benableScreenShareAudio\s*\(|\bdisableScreenShareAudio\s*\(' packages/react-native-sdk/src/hooks/useScreenShareButton.tsAlso applies to: 150-156
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-native-sdk/src/hooks/useScreenShareButton.ts` around lines 116 - 124, The effect and button handler currently call call?.screenShare.enable(), call?.screenShare.enableScreenShareAudio(), and call?.screenShare.disableScreenShareAudio() without awaiting them, causing onScreenShareStartedHandlerRef.current to fire before the operations complete and leaving unhandled rejections; update the broadcast-start effect and the button handler in useScreenShareButton to await those promises and wrap them in try/catch (e.g., await call.screenShare.enable() and await call.screenShare.enableScreenShareAudio()/disableScreenShareAudio() when call is defined), only invoke onScreenShareStartedHandlerRef.current after the awaited calls succeed, and handle/log any errors to prevent unhandled rejections while preserving existing null-safe checks (call?).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/react-native-sdk/src/hooks/useScreenShareButton.ts`:
- Around line 193-196: The call to
screenShareAudioMixingManager.stopInAppScreenCapture() can reject and prevent
call?.screenShare.disable(true) from running, leaving the call still publishing;
wrap the stopInAppScreenCapture() invocation in a try-catch-finally (or
try/finally) inside useScreenShareButton so that any error from
stopInAppScreenCapture() is caught/logged but call?.screenShare.disable(true) is
always executed in the finally block; reference the existing symbols
screenShareAudioMixingManager.stopInAppScreenCapture() and
call?.screenShare.disable(true) and ensure errors are handled without blocking
the disable call.
- Around line 171-175: The iOS branch in useScreenShareButton calls
findNodeHandle(screenCapturePickerViewiOSRef.current) and awaits
NativeModules.ScreenCapturePickerViewManager.show(...) without guarding null or
handling errors; update the handler in useScreenShareButton to first check that
reactTag (from findNodeHandle on screenCapturePickerViewiOSRef) is non-null
before calling ScreenCapturePickerViewManager.show, wrap the await in a
try/catch, log or handle the error via the existing state updater (e.g., the
local sharing/button state) to revert any optimistic UI changes, and ensure
failure paths match the other branches' behavior so the button isn't left in an
ambiguous state.
---
Duplicate comments:
In `@packages/react-native-sdk/src/hooks/useScreenShareButton.ts`:
- Around line 116-124: The effect and button handler currently call
call?.screenShare.enable(), call?.screenShare.enableScreenShareAudio(), and
call?.screenShare.disableScreenShareAudio() without awaiting them, causing
onScreenShareStartedHandlerRef.current to fire before the operations complete
and leaving unhandled rejections; update the broadcast-start effect and the
button handler in useScreenShareButton to await those promises and wrap them in
try/catch (e.g., await call.screenShare.enable() and await
call.screenShare.enableScreenShareAudio()/disableScreenShareAudio() when call is
defined), only invoke onScreenShareStartedHandlerRef.current after the awaited
calls succeed, and handle/log any errors to prevent unhandled rejections while
preserving existing null-safe checks (call?).
🪄 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: 1c0eb4da-174b-40c5-93fe-00d2ee181f27
📒 Files selected for processing (1)
packages/react-native-sdk/src/hooks/useScreenShareButton.ts
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
1 participant
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
This PR contains implementation for screen share audio capturing and mixing.
For Android we intercept webrtc audio buffer and mix it with audio data captured by AudioRecord converted to PCM 16-bit format. (note: investigate stereo output case)
For iOS we presented new screen share mode – in app screen sharing, which limits for capturing only app content. For in app capturing mode we modify audio engine graph – we intercept audio signal in the end of processing chain and mix captured audio from RPScreenRecorder.
📝 Implementation notes
Corresponding PR for webrtc package: GetStream/react-native-webrtc#28
🎫 Ticket: https://linear.app/stream/issue/RN-371/screen-share-audio-capture
📑 Docs: https://github.com/GetStream/docs-content/pull/1098
Summary by CodeRabbit
New Features
Chores