feat: added audio capture for ios broadcast screensharing mode - #2346
Conversation
|
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:
📝 WalkthroughWalkthroughiOS broadcast capture now streams app audio through a dedicated socket, routes video and audio independently, and supports fallback screen-share audio mixing. The Expo plugin and Xcode project include the audio uploader, and audio session configurations enable mixing with other audio. ChangesiOS broadcast audio
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ScreenShareToggleButton
participant SampleHandler
participant AudioUploader
participant SocketConnection
participant ScreenAudioCapture
participant ScreenShareAudioMixer
ScreenShareToggleButton->>SampleHandler: start broadcast with audio
SampleHandler->>SocketConnection: open audio connection
SampleHandler->>AudioUploader: send app-audio sample
AudioUploader->>SocketConnection: write framed PCM chunks
ScreenAudioCapture->>ScreenShareAudioMixer: forward fallback PCM buffers
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Bundle sizeBuilt package output. Sizes in KB; delta vs No significant package size increase vs |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/react-native-sdk/expo-config-plugin/static/SampleHandler.swift (1)
134-151: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConnect-retry timers are unreachable after creation in both
SampleHandlercopies. TheDispatchSourceTimeris only captured by its own event handler, sobroadcastFinishedcannot cancel it and the audio retry loop keeps firing for up to ~30s past teardown;openVideoConnectionhas the same shape.
packages/react-native-sdk/expo-config-plugin/static/SampleHandler.swift#L134-L151: store the timers as properties, cancel them inbroadcastFinished, and extract one shared open-with-retry helper.sample-apps/react-native/dogfood/ios/Broadcast Extension/SampleHandler.swift#L134-L151: mirror the change to keep the dogfood extension aligned with the template.🤖 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 `@packages/react-native-sdk/expo-config-plugin/static/SampleHandler.swift` around lines 134 - 151, Update SampleHandler.swift at packages/react-native-sdk/expo-config-plugin/static/SampleHandler.swift:134-151 and sample-apps/react-native/dogfood/ios/Broadcast Extension/SampleHandler.swift:134-151 to store audio and video retry timers as properties, extract their shared retry logic into one open-with-retry helper, and cancel both timers from broadcastFinished so no retry continues after teardown.packages/react-native-sdk/expo-config-plugin/src/withIosScreenCapture/withTarget.ts (1)
182-189: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unit-test coverage for the new
AudioUploader.swiftsource entry.The modified source registers
AudioUploader.swiftalongsideSampleUploader.swift, but the plugin tests do not coverwithTarget.tsor the Xcode build-phase expectations for these static Swift sources. Add a test/snapshot so future diffing catches regressions in the modifier’s file-copy andaddSourceFileexpectations.🤖 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 `@packages/react-native-sdk/expo-config-plugin/src/withIosScreenCapture/withTarget.ts` around lines 182 - 189, Add unit-test or snapshot coverage for the withTarget modifier that verifies both AudioUploader.swift and SampleUploader.swift are copied and registered via addSourceFile in the expected Xcode target build phase. Use the existing plugin test conventions and assert the generated project output so regressions in these static Swift source entries are detected.Source: Path instructions
🤖 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 `@packages/react-native-sdk/expo-config-plugin/static/AudioUploader.swift`:
- Around line 214-225: In the planar interleaving logic around frameCount and
output, derive frameCount from the minimum mDataByteSize across all buffers
rather than buffers[0], and return early when the clamped count is zero. Apply
this same change at
packages/react-native-sdk/expo-config-plugin/static/AudioUploader.swift:214-225
and sample-apps/react-native/dogfood/ios/Broadcast
Extension/AudioUploader.swift:214-225, preserving the existing channel/frame
copy behavior.
- Around line 34-49: Serialize updates to dataToSend and byteIndex by moving
their assignments into the serialQueue.async block immediately before
sendDataChunk(), while preserving the existing readiness and return behavior.
Apply this identical change in
packages/react-native-sdk/expo-config-plugin/static/AudioUploader.swift lines
34-49 and sample-apps/react-native/dogfood/ios/Broadcast
Extension/AudioUploader.swift lines 34-49 so both AudioUploader copies
synchronize state mutations.
In `@packages/react-native-sdk/ios/StreamVideoReactNative.m`:
- Around line 686-689: Update the iOS screen-audio capture flow around
_screenAudioCapture to remove the stale non-existent reference: either provide a
valid shared Swift implementation and expose its new, onAudioBuffer, start, and
stop APIs through the React Native bridge, or mirror Android’s cleanup by
clearing the capture object after stopping it and recreating it through a valid
implementation.
---
Nitpick comments:
In
`@packages/react-native-sdk/expo-config-plugin/src/withIosScreenCapture/withTarget.ts`:
- Around line 182-189: Add unit-test or snapshot coverage for the withTarget
modifier that verifies both AudioUploader.swift and SampleUploader.swift are
copied and registered via addSourceFile in the expected Xcode target build
phase. Use the existing plugin test conventions and assert the generated project
output so regressions in these static Swift source entries are detected.
In `@packages/react-native-sdk/expo-config-plugin/static/SampleHandler.swift`:
- Around line 134-151: Update SampleHandler.swift at
packages/react-native-sdk/expo-config-plugin/static/SampleHandler.swift:134-151
and sample-apps/react-native/dogfood/ios/Broadcast
Extension/SampleHandler.swift:134-151 to store audio and video retry timers as
properties, extract their shared retry logic into one open-with-retry helper,
and cancel both timers from broadcastFinished so no retry continues after
teardown.
🪄 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 Plus
Run ID: 1460615c-ddb8-48dd-9ea0-4c6cfa02ac47
📒 Files selected for processing (8)
packages/react-native-sdk/expo-config-plugin/src/withIosScreenCapture/withTarget.tspackages/react-native-sdk/expo-config-plugin/static/AudioUploader.swiftpackages/react-native-sdk/expo-config-plugin/static/SampleHandler.swiftpackages/react-native-sdk/ios/StreamVideoReactNative.mpackages/react-native-sdk/src/hooks/useScreenShareButton.tssample-apps/react-native/dogfood/ios/Broadcast Extension/AudioUploader.swiftsample-apps/react-native/dogfood/ios/Broadcast Extension/SampleHandler.swiftsample-apps/react-native/dogfood/src/components/CallControls/BottomControls/index.tsx
|
🎉 The changes from this pull request have been released. Shipped with:
|
💡 Overview
Added audio capturing for iOS broadcast screen sharing type.
Added
.mixWithOthersdefault option to all audio session configurations, which prevents audio engine from stopping when other apps start audio session with different configuration.Changed default screen sharing type in dogfood app.
📝 Implementation notes
Added
AudioUploaderwhich works similar toSampleUploader: it sends bytes data via socket connection. Server is started on webrtc side. Bytes chunks are accumulated and pushed to ring buffer as they are ready. Audio mixing part stays as is (as it was for in-app audio capturing).🎫 Ticket: https://linear.app/stream/issue/RN-416/ios-broadcast-screen-sharing-audio-capturing
📑 Docs: https://github.com/GetStream/docs-content/pull/1457
Corresponding WebRTC PR: GetStream/react-native-webrtc#58
Summary by CodeRabbit
includeAudiooption.