feat: pre-call testing - #2235
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds loopback recording support across the client, native recorders, a React Native hook, and a dogfood test flow. ChangesSFU Loopback Track Recording
Sequence Diagram(s)sequenceDiagram
participant App as TestRecordingScreen
participant Hook as useLoopbackRecording
participant Call as Call
participant Native as TracksRecorderManager
participant SFU
App->>Call: getOrCreate()
App->>Hook: startRecording({ includeVideo })
Hook->>Call: join({ allowOwnTracksLoopback: true })
Call->>SFU: publish selfSubAudioVideo
SFU-->>Call: loopback tracks
Hook->>Native: startTrackRecording(...)
Native->>Native: encode audio/video to MP4
App->>Hook: stopRecording()
Hook->>Native: stopTrackRecording()
Native-->>Hook: file:// URI
App->>App: navigate to results screen
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 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 |
|
oliverlaz
reviewed
Jun 23, 2026
greenfrvr
marked this pull request as ready for review
June 24, 2026 12:34
![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: 11
🧹 Nitpick comments (4)
sample-apps/react-native/dogfood/src/screens/TestRecording/components/LoopbackPanel.tsx (1)
46-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: use a theme token for the active indicator color.
The "ready" green
#4CAF50is hardcoded here while the rest of the panel usesappTheme.colors. Pulling it from the theme keeps status colors consistent.🤖 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 `@sample-apps/react-native/dogfood/src/screens/TestRecording/components/LoopbackPanel.tsx` around lines 46 - 69, The active status indicator color in LoopbackPanel is hardcoded, which is inconsistent with the rest of the component’s theme usage. Update the dot styling in the badge/status rows to use an existing appTheme.colors token for the “ready” green instead of the literal value, and keep the Mic and Video indicator logic unchanged while applying the themed color wherever the active state is rendered.sample-apps/react-native/dogfood/src/screens/TestRecording/TestRecordingScreen.tsx (1)
42-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDebug global assignment left in.
globalThis.call = call(with the@ts-expect-error) is a debugging artifact. Consider gating it behind__DEV__or removing it so it doesn't leak the call instance in every build.🤖 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 `@sample-apps/react-native/dogfood/src/screens/TestRecording/TestRecordingScreen.tsx` around lines 42 - 43, The TestRecordingScreen debug global assignment is left enabled in all builds, leaking the call instance via globalThis.call. Update the logic around the globalThis.call assignment in TestRecordingScreen so it is either removed entirely or wrapped in a __DEV__-only guard, and keep the `@ts-expect-error` scoped only if the debug path remains necessary.sample-apps/react-native/dogfood/types.ts (1)
19-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCentralize
TestRecordingStackParamListusage from this file.The new shared type is good, but related files still redeclare the same shape. Import this exported type in those files to avoid route-param drift.
🤖 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 `@sample-apps/react-native/dogfood/types.ts` around lines 19 - 23, The route param shape is now exported as TestRecordingStackParamList, but other files still duplicate the same definition and can drift. Update the related React Native dogfood files to import and reuse TestRecordingStackParamList from this module instead of redeclaring it, and keep any screen/stack typings wired through that shared type.packages/react-native-sdk/ios/TracksRecorder/AudioPipeline.swift (1)
104-110: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid the second PCM deep copy on the audio callback path.
RecorderAudioRenderTapalready creates and fills a freshAVAudioPCMBuffer; capture that buffer into the queue block instead of allocating/copying it again.Suggested change
- guard let copy = AudioPipeline.deepCopyPCMBuffer(pcmBuffer) else { return } guard let host = host else { return } // `DispatchTime.now().uptimeNanoseconds` is the monotonic clock @@ // shared time origin works coherently across both pipelines. let captureTimeNs = DispatchTime.now().uptimeNanoseconds - host.queue.async { [weak self] in - self?.handleAudioBufferOnQueue(pcmBuffer: copy, captureTimeNs: captureTimeNs) + host.queue.async { [weak self, pcmBuffer] in + self?.handleAudioBufferOnQueue(pcmBuffer: pcmBuffer, captureTimeNs: captureTimeNs) }🤖 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/ios/TracksRecorder/AudioPipeline.swift` around lines 104 - 110, The audio callback path in `AudioPipeline.handleAudioBuffer(pcmBuffer:)` is doing an unnecessary second deep copy even though `RecorderAudioRenderTap` already provides a freshly allocated and filled `AVAudioPCMBuffer`. Update the queue handoff to capture and use that existing buffer directly instead of calling `AudioPipeline.deepCopyPCMBuffer`, while preserving the lifetime guarantees needed across the async boundary.
🤖 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/client/src/Call.ts`:
- Around line 1091-1097: Set and clear the loopback flag around the ringing flow
in Call’s call setup path: `ringingSubject.next(true)` can synchronously trigger
`handleRingingCall()`/`registerOutgoingCall()` before `allowOwnTracksLoopback`
is assigned, so move the flag assignment to happen before any ringing side
effects and make sure it is reset when the call ends or leaves. Update the
relevant call lifecycle logic in `Call` so reusable instances do not carry
`allowOwnTracksLoopback` forward into later calls.
In
`@packages/react-native-sdk/android/src/main/java/com/streamvideo/reactnative/recorder/TracksRecorderManager.kt`:
- Around line 163-177: The stop/cleanup path in TracksRecorderManager should not
resolve a terminal state with no MP4 as success. Update the empty-recording
branch that calls fireTerminalCompletion and the later completion path so they
emit a RecordingError when resolved is null and no file was produced, instead of
passing null/null through to startTrackRecording(). Ensure the logic around
muxerStarted, muxerInstance, and the terminal completion callback always
distinguishes “no output” from a successful recording.
- Around line 313-323: The fatal cleanup path in cleanupAfterFailure() stops and
releases pipelines and the muxer, but leaves the partially created outputFile
behind. Update TracksRecorderManager.cleanupAfterFailure() to also remove the
failed recording file before resetting transient state, so rejected start/encode
failures do not leave a corrupt MP4 that getStreamRecordings() can later return.
- Around line 230-239: The clear-recordings flow in
TracksRecorderManager.clearRecordingsDirectory can delete the currently active
recording output, so update this path to guard against active sessions before
clearing or skip the in-use outputFile when iterating over StreamRecordings. Use
the existing TracksRecorderManager recording state and outputFile references to
detect an active recording, then either reject clearStreamRecordings() during
recording or preserve the current file while deleting only inactive recordings.
In `@packages/react-native-sdk/ios/TracksRecorder/RecorderAudioRenderTap.swift`:
- Around line 96-107: Move the per-buffer diagnostics out of
`audioProcessingProcess(audioBuffer:)` in `RecorderAudioRenderTap` because
`NSLog` on the WebRTC audio thread can block the render callback. Keep the
`_callCount` bookkeeping there, but remove the first-call and every-100th
logging from this method and emit equivalent diagnostics from `logSummary()` or
another off-thread debug path instead.
In `@packages/react-native-sdk/ios/TracksRecorder/TracksRecorderManager.swift`:
- Around line 77-79: The `TracksRecorderManager` video track lookup is too
permissive: when `videoTrackId` is provided but `webRTCModule.track(forId:)`
returns nil or a non-`RTCVideoTrack`, recording should fail instead of silently
continuing with audio-only. Update the validation around `videoTrackId` and
`videoTrack` so the recorder rejects the request with an error when the
requested track cannot be resolved or is not a video track, and keep the success
path only for a valid `RTCVideoTrack`.
- Around line 211-223: Prevent clearRecordingsDirectory from deleting an
in-progress recording by adding a guard in
TracksRecorderManager.clearRecordingsDirectory that checks whether recording is
currently active and skips clearing when it is. Use the existing recording state
and current outputURL/recording session tracking in TracksRecorderManager to
detect an active recording, and only enumerate/remove files from
recordingsDirectory when no recording is underway; keep the completion callback
behavior consistent for the skipped case.
In `@packages/react-native-sdk/src/hooks/useLoopbackRecording.ts`:
- Around line 247-255: Guard the native module accesses in useLoopbackRecording
by using optional chaining consistently for startTrackRecording,
clearStreamRecordings, and getStreamRecordings on StreamVideoReactNative,
matching the existing stopTrackRecording pattern. Wrap each native call in
try/catch inside the related callbacks so bridge rejections are handled
gracefully, and return safe fallbacks (or rethrow through your existing error
handling path) when the module is unavailable.
- Around line 175-185: Replace the two raw console.warn calls in
useLoopbackRecording with the existing scoped logger from
videoLoggerSystem.getLogger('useLoopbackRecording'). Keep the same early-return
behavior, but route both “already running” and “other participants present”
diagnostics through the hook’s logger so no unconditional console noise ships in
production. Use the same logger instance already used elsewhere in
useLoopbackRecording to keep all diagnostics consistent and env-gated.
- Around line 297-318: The `useLoopbackRecording` hook is still reading client
observables directly, so update it to use bindings-backed state instead. Replace
the `combineLatest` subscription in `useEffect` and the `waitForLoopbackStreams`
path to consume values from `useLocalParticipant`, `useCameraState`, and
`useMicrophoneState` instead of `call.state.localParticipant$`,
`call.camera.state.mediaStream$`, and `call.microphone.state.mediaStream$`. Keep
`getLoopbackStreamsFor` as the shared mapper, and if `waitForLoopbackStreams`
still needs async waiting, feed it hook-derived state or move the observable
bridge outside the RN hook state flow.
In
`@sample-apps/react-native/dogfood/src/screens/TestRecording/TestRecordingResultsScreen.tsx`:
- Around line 48-68: The button labels in TestRecordingResultsScreen are
hardcoded and should be localized; replace the visible strings for Share, Record
again, and Done with translation lookups from the app’s dictionary. Update the
relevant keys in the translation flow used by this screen (including the English
source in src/translations/en.json) and wire the screen to read those keys
instead of inline text so the Pressable labels remain localizable.
---
Nitpick comments:
In `@packages/react-native-sdk/ios/TracksRecorder/AudioPipeline.swift`:
- Around line 104-110: The audio callback path in
`AudioPipeline.handleAudioBuffer(pcmBuffer:)` is doing an unnecessary second
deep copy even though `RecorderAudioRenderTap` already provides a freshly
allocated and filled `AVAudioPCMBuffer`. Update the queue handoff to capture and
use that existing buffer directly instead of calling
`AudioPipeline.deepCopyPCMBuffer`, while preserving the lifetime guarantees
needed across the async boundary.
In
`@sample-apps/react-native/dogfood/src/screens/TestRecording/components/LoopbackPanel.tsx`:
- Around line 46-69: The active status indicator color in LoopbackPanel is
hardcoded, which is inconsistent with the rest of the component’s theme usage.
Update the dot styling in the badge/status rows to use an existing
appTheme.colors token for the “ready” green instead of the literal value, and
keep the Mic and Video indicator logic unchanged while applying the themed color
wherever the active state is rendered.
In
`@sample-apps/react-native/dogfood/src/screens/TestRecording/TestRecordingScreen.tsx`:
- Around line 42-43: The TestRecordingScreen debug global assignment is left
enabled in all builds, leaking the call instance via globalThis.call. Update the
logic around the globalThis.call assignment in TestRecordingScreen so it is
either removed entirely or wrapped in a __DEV__-only guard, and keep the
`@ts-expect-error` scoped only if the debug path remains necessary.
In `@sample-apps/react-native/dogfood/types.ts`:
- Around line 19-23: The route param shape is now exported as
TestRecordingStackParamList, but other files still duplicate the same definition
and can drift. Update the related React Native dogfood files to import and reuse
TestRecordingStackParamList from this module instead of redeclaring it, and keep
any screen/stack typings wired through that shared type.
🪄 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: 99ce0378-72bd-4976-b0ab-1af590c4fda2
⛔ Files ignored due to path filters (8)
packages/client/src/gen/google/protobuf/struct.tsis excluded by!**/gen/**packages/client/src/gen/google/protobuf/timestamp.tsis excluded by!**/gen/**packages/client/src/gen/video/sfu/event/events.tsis excluded by!**/gen/**packages/client/src/gen/video/sfu/models/models.tsis excluded by!**/gen/**packages/client/src/gen/video/sfu/signal_rpc/signal.client.tsis excluded by!**/gen/**packages/client/src/gen/video/sfu/signal_rpc/signal.tsis excluded by!**/gen/**sample-apps/react-native/dogfood/ios/Podfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (37)
packages/client/src/Call.tspackages/client/src/rtc/Publisher.tspackages/client/src/rtc/Subscriber.tspackages/react-native-sdk/android/src/main/java/com/streamvideo/reactnative/StreamVideoReactNativeModule.ktpackages/react-native-sdk/android/src/main/java/com/streamvideo/reactnative/recorder/AudioPipeline.ktpackages/react-native-sdk/android/src/main/java/com/streamvideo/reactnative/recorder/EncoderConstants.ktpackages/react-native-sdk/android/src/main/java/com/streamvideo/reactnative/recorder/PipelineHost.ktpackages/react-native-sdk/android/src/main/java/com/streamvideo/reactnative/recorder/RecorderPlaybackSamplesSink.ktpackages/react-native-sdk/android/src/main/java/com/streamvideo/reactnative/recorder/RecorderVideoSink.ktpackages/react-native-sdk/android/src/main/java/com/streamvideo/reactnative/recorder/TracksRecorderManager.ktpackages/react-native-sdk/android/src/main/java/com/streamvideo/reactnative/recorder/VideoPipeline.ktpackages/react-native-sdk/ios/StreamVideoReactNative-Bridging-Header.hpackages/react-native-sdk/ios/StreamVideoReactNative.mpackages/react-native-sdk/ios/TracksRecorder/AudioPipeline.swiftpackages/react-native-sdk/ios/TracksRecorder/PipelineHost.swiftpackages/react-native-sdk/ios/TracksRecorder/RecorderAudioRenderTap.swiftpackages/react-native-sdk/ios/TracksRecorder/RecorderVideoSink.swiftpackages/react-native-sdk/ios/TracksRecorder/TracksRecorderManager.swiftpackages/react-native-sdk/ios/TracksRecorder/VideoPipeline.swiftpackages/react-native-sdk/src/hooks/index.tspackages/react-native-sdk/src/hooks/useLoopbackRecording.tspackages/react-native-sdk/src/utils/internal/callingx/callingx.tssample-apps/react-native/dogfood/App.tsxsample-apps/react-native/dogfood/package.jsonsample-apps/react-native/dogfood/src/components/NavigationHeader.tsxsample-apps/react-native/dogfood/src/contexts/AppContext.tsxsample-apps/react-native/dogfood/src/navigators/TestRecording.tsxsample-apps/react-native/dogfood/src/screens/ChooseAppModeScreen.tsxsample-apps/react-native/dogfood/src/screens/TestRecording/TestRecordingResultsScreen.tsxsample-apps/react-native/dogfood/src/screens/TestRecording/TestRecordingScreen.tsxsample-apps/react-native/dogfood/src/screens/TestRecording/components/InlineCallStats.tsxsample-apps/react-native/dogfood/src/screens/TestRecording/components/LoopbackPanel.tsxsample-apps/react-native/dogfood/src/screens/TestRecording/components/PlaybackPanel.tsxsample-apps/react-native/dogfood/src/screens/TestRecording/components/RecordingControls.tsxsample-apps/react-native/dogfood/src/screens/TestRecording/components/index.tssample-apps/react-native/dogfood/src/translations/en.jsonsample-apps/react-native/dogfood/types.ts
oliverlaz
approved these changes
Jun 24, 2026
|
🎉 The changes from this pull request have been released. Shipped with:
|
Merged
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.
💡 Overview
This PR contains pre-call testing functionality for React Native SDK and dogfood app example how it can be used by end user. The idea of pre-call testing is to receive loopback tracks and record them to a local video file to be able to check the call from the callee's perspective.
📝 Implementation notes
To subscribe to their own published tracks, the caller should send
selfSubAudioVideoduring publishing. From public APIallowOwnTracksLoopbackshould be passed duringjoininvocation.When connected, loopback streams are routed to
localParticipant(instead of default camera/microphone tracks).All pre-call testing functionality is provided with a single hook
useLoopbackRecording. It allows to start/stop recording of loopback video, getting the loopback tracks refs, check local recordings directory and clear previously recorded videos.🎫 Ticket: https://linear.app/stream/issue/RN-389/pre-call-testing
📑 Docs: https://github.com/GetStream/docs-content/pull/1325
Summary by CodeRabbit
Summary
New Features
useLoopbackRecordinghook.Bug Fixes