fix(rn): perf and stability fixes for video-filters#2216
Merged
santhoshvai merged 15 commits intomainfrom Apr 28, 2026
Merged
Conversation
|
Contributor
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds async image/filter loading, explicit native unregistering, instance-based YUV converters with explicit disposal, reapplication of native filters when media tracks change, and multiple lifecycle/cleanup fixes across React Native, Android, and iOS components. Changes
sequenceDiagram
participant RN as React Native UI
participant SDK as BackgroundFilters.tsx
participant Native as VideoFiltersReactNative (JS bridge)
participant Provider as ProcessorProvider / Native Filters
participant Tracks as MediaStream / VideoTracks
RN->>SDK: apply / disable filter
SDK->>Native: registerFilter / unregisterFilter (sets lastAppliedFilterNameRef)
Native->>Provider: add processor (store name in registeredNames)
SDK->>Tracks: subscribe to track changes
Tracks-->>SDK: tracks replaced
SDK->>Native: reapply recorded filter name to new tracks
RN->>SDK: call unregisterAllFilters
SDK->>Native: unregisterAllFilters
Native->>Provider: remove all processors in registeredNames (clear set)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/react-native-sdk/src/contexts/BackgroundFilters.tsx (1)
88-178:⚠️ Potential issue | 🟠 MajorGuard async filter application against stale operations.
Each apply path awaits registration before setting
lastAppliedFilterNameRef, applying the native effect, and updating state. If the user disables filters or selects another filter while registration is in flight, the older promise can resolve later and re-apply a stale filter.Suggested change
const isVideoBlurRegisteredRef = useRef(false); const registeredImageFiltersSetRef = useRef(new Set<string>()); + const filterOperationIdRef = useRef(0); @@ const applyBackgroundBlurFilter = useCallback( async (blurIntensity: BlurIntensity) => { + const operationId = ++filterOperationIdRef.current; if (!isSupported) { return; } if (!isBackgroundBlurRegisteredRef.current) { await videoFiltersModule?.registerBackgroundBlurVideoFilters(); + if (operationId !== filterOperationIdRef.current) return; isBackgroundBlurRegisteredRef.current = true; } @@ const applyVideoBlurFilter = useCallback( async (blurIntensity: BlurIntensity) => { + const operationId = ++filterOperationIdRef.current; if (!isSupported) { return; } if (!isVideoBlurRegisteredRef.current) { await videoFiltersModule?.registerBlurVideoFilters(); + if (operationId !== filterOperationIdRef.current) return; isVideoBlurRegisteredRef.current = true; } @@ const applyBackgroundImageFilter = useCallback( async (imageSource: ImageSourceType) => { + const operationId = ++filterOperationIdRef.current; if (!isSupported) { return; } @@ if (!registeredImageFiltersSet.has(imageUri)) { await videoFiltersModule?.registerVirtualBackgroundFilter(imageSource); + if (operationId !== filterOperationIdRef.current) return; registeredImageFiltersSetRef.current.add(imageUri); } @@ const disableAllFilters = useCallback(() => { + filterOperationIdRef.current += 1; if (!isSupported) { return; }As per coding guidelines,
**/*.{ts,tsx}: Use AbortController to cancel network requests and media operations on component unmount; check instance IDs and timestamps before state updates to avoid race conditions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-native-sdk/src/contexts/BackgroundFilters.tsx` around lines 88 - 178, The async apply functions (applyBackgroundBlurFilter, applyVideoBlurFilter, applyBackgroundImageFilter) can finish after the user has changed/disabled filters and thus re-apply stale effects; fix this by introducing an operation token (e.g., opCounterRef or AbortController) that you capture at the start of each apply* call, increment when disableAllFilters or a new apply starts, and then after any await (registration or image resolution) verify the token still matches before setting lastAppliedFilterNameRef, calling track._setVideoEffect, or calling setCurrentBackgroundFilter; also update disableAllFilters to bump/invalidate the token so in-flight operations abort applying their results.packages/video-filters-react-native/android/src/main/java/com/streamio/videofiltersreactnative/common/YuvFrame.kt (1)
12-17:⚠️ Potential issue | 🟠 MajorAdd explicit buffer cleanup to YuvFrame since it now holds per-instance resources.
The
libYuvRotatedI420BufferandlibYuvAbgrBufferare only released when frame dimensions change (lines 66, 73, 84), not when the processor is destroyed. SinceYuvFrameis now instantiated per-processor inVideoFrameProcessorWithBitmapFilter(line 21), these buffers leak whenever a processor is torn down without processing frames of different dimensions.Suggested direction
class YuvFrame { @@ private fun createLibYuvAbgrBuffer() { @@ libYuvRotatedI420Buffer!!.convertTo(libYuvAbgrBuffer!!) } + + fun close() { + libYuvRotatedI420Buffer?.close() + libYuvRotatedI420Buffer = null + libYuvAbgrBuffer?.close() + libYuvAbgrBuffer = null + }Then call
yuvFrame.close()from the processor/filter teardown if the externalVideoFrameProcessorinterface provides a lifecycle hook. Confirm availability with the WebRTC module dependency.Also applies to: 65-88
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/video-filters-react-native/android/src/main/java/com/streamio/videofiltersreactnative/common/YuvFrame.kt` around lines 12 - 17, YuvFrame holds per-instance native buffers (libYuvRotatedI420Buffer, libYuvAbgrBuffer) that are only released on dimension change; add an explicit cleanup method (e.g., fun close() or release()) on YuvFrame that releases and nulls libYuvRotatedI420Buffer and libYuvAbgrBuffer (and any other held native resources) to avoid leaks, and call that method from the processor/filter teardown in VideoFrameProcessorWithBitmapFilter (or wherever the processor is destroyed); reference the existing buffer fields (libYuvRotatedI420Buffer, libYuvAbgrBuffer) and the YuvFrame class so reviewers can locate and wire up the lifecycle hook.
🤖 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/contexts/BackgroundFilters.tsx`:
- Around line 181-207: Replace the manual RxJS subscription in the effect with
the React hook that exposes camera state: call
useCallStateHooks()/useCameraState() to get const { mediaStream } =
useCameraState() and use mediaStream directly inside the effect instead of
subscribing to call.camera.state.mediaStream$; update the effect dependencies to
include mediaStream (e.g. [call, mediaStream]), remove subscription
creation/unsubscription, and keep the same logic that iterates
mediaStream?.getVideoTracks() calling track._setVideoEffect(name) on mount and
track._setVideoEffect(null) in the cleanup while still resetting
lastAppliedFilterNameRef.current, isBackgroundBlurRegisteredRef.current,
isVideoBlurRegisteredRef.current, and clearing
registeredImageFiltersSetRef.current so the behavior around
lastAppliedFilterNameRef, registeredImageFiltersSetRef,
isBackgroundBlurRegisteredRef and isVideoBlurRegisteredRef remains unchanged.
In
`@packages/video-filters-react-native/android/src/main/java/com/streamio/videofiltersreactnative/factories/VirtualBackgroundFactory.kt`:
- Around line 73-86: In loadBackgroundImage within VirtualBackgroundFactory,
replace the direct URLConnection + openInputStream call with a URLConnection
where you set connect and read timeouts (e.g., setConnectTimeout,
setReadTimeout), obtain the InputStream and decode it inside a
try-with-resources style block (Kotlin .use { ... }) so the stream is always
closed, and change the error log that currently includes
backgroundImageUrlString to avoid printing the raw URL (log a masked value,
hostname only, or a constant like "[REDACTED]" alongside the exception); update
BitmapFactory.decodeStream usage accordingly to use the closed-safe stream.
In
`@packages/video-filters-react-native/ios/VideoFrameProcessors/ImageBackgroundVideoFrameProcessor.swift`:
- Around line 83-86: The failure branch that currently logs the full
backgroundImageUrl (inside the Data(contentsOf: url) handling where bgUIImage is
set) can leak sensitive tokens; change the log to avoid printing the full URL
and instead log a non-sensitive identifier such as the URL host or filename, or
a sanitized/hashed representation, or simply log "Failed to convert background
image URI" along with the error/URL length or last N characters, ensuring you
reference backgroundImageUrl only in a redacted form; update the NSLog call in
ImageBackgroundVideoFrameProcessor.swift where bgUIImage is set to reflect this.
---
Outside diff comments:
In `@packages/react-native-sdk/src/contexts/BackgroundFilters.tsx`:
- Around line 88-178: The async apply functions (applyBackgroundBlurFilter,
applyVideoBlurFilter, applyBackgroundImageFilter) can finish after the user has
changed/disabled filters and thus re-apply stale effects; fix this by
introducing an operation token (e.g., opCounterRef or AbortController) that you
capture at the start of each apply* call, increment when disableAllFilters or a
new apply starts, and then after any await (registration or image resolution)
verify the token still matches before setting lastAppliedFilterNameRef, calling
track._setVideoEffect, or calling setCurrentBackgroundFilter; also update
disableAllFilters to bump/invalidate the token so in-flight operations abort
applying their results.
In
`@packages/video-filters-react-native/android/src/main/java/com/streamio/videofiltersreactnative/common/YuvFrame.kt`:
- Around line 12-17: YuvFrame holds per-instance native buffers
(libYuvRotatedI420Buffer, libYuvAbgrBuffer) that are only released on dimension
change; add an explicit cleanup method (e.g., fun close() or release()) on
YuvFrame that releases and nulls libYuvRotatedI420Buffer and libYuvAbgrBuffer
(and any other held native resources) to avoid leaks, and call that method from
the processor/filter teardown in VideoFrameProcessorWithBitmapFilter (or
wherever the processor is destroyed); reference the existing buffer fields
(libYuvRotatedI420Buffer, libYuvAbgrBuffer) and the YuvFrame class so reviewers
can locate and wire up the lifecycle hook.
🪄 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: b6b52b4a-ca8d-446f-a851-1a40b1972485
📒 Files selected for processing (7)
packages/react-native-sdk/src/contexts/BackgroundFilters.tsxpackages/video-filters-react-native/android/src/main/java/com/streamio/videofiltersreactnative/common/VideoFrameWithBitmapFilter.ktpackages/video-filters-react-native/android/src/main/java/com/streamio/videofiltersreactnative/common/YuvFrame.ktpackages/video-filters-react-native/android/src/main/java/com/streamio/videofiltersreactnative/factories/VirtualBackgroundFactory.ktpackages/video-filters-react-native/ios/VideoFrameProcessors/BlurBackgroundVideoFrameProcessor.swiftpackages/video-filters-react-native/ios/VideoFrameProcessors/ImageBackgroundVideoFrameProcessor.swiftpackages/video-filters-react-native/ios/VideoFrameProcessors/Utils/VideoFilters.swift
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/video-filters-react-native/android/src/main/java/com/streamio/videofiltersreactnative/VideoFiltersReactNativeModule.kt`:
- Around line 13-15: The current cleanup unconditionally removes names in the
instance-scoped registeredNames from the global ProcessorProvider in
unregisterAllFilters(), which can unregister processors still used by other
owners; change this to scope cleanup to the owner by implementing an owner token
or reference-counted registry: replace direct use of the global
ProcessorProvider removal with a per-owner token (or increment/decrement a
ref-count map keyed by processor name) so that unregisterAllFilters() only
decrements/refcounts and removes a processor from ProcessorProvider when its
count reaches zero; update references to registeredNames,
unregisterAllFilters(), and any global ProcessorProvider.remove/unregister calls
to use the new token/ref-count logic.
In `@packages/video-filters-react-native/ios/VideoFiltersReactNative.swift`:
- Around line 4-6: registeredNames is a global Set<String> used by
BackgroundFiltersProvider and leads to unsafe blanket cleanup via
unregisterAllFilters(); change to per-owner tracking: associate registrations
with an owner token (e.g., a UUID) when calling registerProcessor/registerFilter
so BackgroundFiltersProvider creates a unique ownerId on init and uses it on
cleanup; replace the single registeredNames with a dictionary Map<String,
Set<String>> or [ownerId: Set<processorName>] and update register/unregister
functions (references: registeredNames, BackgroundFiltersProvider,
unregisterAllFilters, registerProcessor) to add/remove only the names tied to
that ownerId, and ensure unregisterAllFilters accepts an ownerId to only remove
that owner's processors.
In
`@packages/video-filters-react-native/ios/VideoFrameProcessors/ImageBackgroundVideoFrameProcessor.swift`:
- Around line 74-85: The current loadBackgroundImage function uses
Data(contentsOf:) which can block; replace the synchronous loader with an async
URLSession dataTask created from a URLRequest that sets timeoutInterval, assign
the returned URLSessionDataTask to a property (e.g., backgroundImageTask) so it
can be cancelled, and on success convert the received Data into a UIImage
exactly where bgUIImage is set today; also update the processor cleanup/teardown
(where ImageBackgroundVideoFrameProcessor is cleaned up) to call
backgroundImageTask?.cancel() to prevent stale network activity.
🪄 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: 25b65f89-7e46-47e7-9589-04fd04a76dcb
⛔ Files ignored due to path filters (2)
sample-apps/react-native/dogfood/ios/Podfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (13)
packages/react-native-sdk/package.jsonpackages/react-native-sdk/src/contexts/BackgroundFilters.tsxpackages/video-filters-react-native/android/src/main/java/com/streamio/videofiltersreactnative/VideoFiltersReactNativeModule.ktpackages/video-filters-react-native/android/src/main/java/com/streamio/videofiltersreactnative/common/VideoFrameWithBitmapFilter.ktpackages/video-filters-react-native/android/src/main/java/com/streamio/videofiltersreactnative/common/YuvFrame.ktpackages/video-filters-react-native/android/src/main/java/com/streamio/videofiltersreactnative/factories/VirtualBackgroundFactory.ktpackages/video-filters-react-native/ios/VideoFiltersReactNative.mmpackages/video-filters-react-native/ios/VideoFiltersReactNative.swiftpackages/video-filters-react-native/ios/VideoFrameProcessors/BlurBackgroundVideoFrameProcessor.swiftpackages/video-filters-react-native/ios/VideoFrameProcessors/ImageBackgroundVideoFrameProcessor.swiftpackages/video-filters-react-native/ios/VideoFrameProcessors/Utils/BackgroundImageFilterProcessor.swiftpackages/video-filters-react-native/src/index.tssample-apps/react-native/dogfood/package.json
✅ Files skipped from review due to trivial changes (2)
- sample-apps/react-native/dogfood/package.json
- packages/react-native-sdk/package.json
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/video-filters-react-native/android/src/main/java/com/streamio/videofiltersreactnative/common/VideoFrameWithBitmapFilter.kt
- packages/video-filters-react-native/android/src/main/java/com/streamio/videofiltersreactnative/common/YuvFrame.kt
- packages/react-native-sdk/src/contexts/BackgroundFilters.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/contexts/BackgroundFilters.tsx`:
- Around line 80-83: Replace the stale-apply detection that only checks
lastAppliedFilterNameRef with a monotonic request token: add a requestIdRef
(number) that you increment for every new apply call, capture the current
requestId in the async apply (e.g., inside applyBackgroundBlurFilter /
applyBackgroundReplacement / related applyX functions), and before any mutation
of is*RegisteredRef or setState verify the captured requestId still equals
requestIdRef.current; also store the requestId alongside
lastAppliedFilterNameRef (e.g., {name, id}) so older promises that happen to
request the same name cannot win; apply the same pattern where name-only checks
occur (around the other occurrences noted) to prevent race conditions.
In
`@packages/video-filters-react-native/android/src/main/java/com/streamio/videofiltersreactnative/factories/VirtualBackgroundFactory.kt`:
- Around line 69-95: The async image decode started in init (Thread {
loadBackgroundImage() }.start()) can complete after close() and repopulate
virtualBackgroundBitmap, so add a thread-safe cancellation check: introduce a
volatile/AtomicBoolean flag (e.g., isClosed or cancelled) set to true in
close(), and in loadBackgroundImage() check this flag both before assigning
virtualBackgroundBitmap and immediately after decoding the Bitmap (before any
assignment) to avoid writing the late result; ensure access to
virtualBackgroundBitmap is synchronized/atomic where needed so the check and
assignment are race-free and skip/recycle the decoded bitmap when cancelled.
🪄 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: f486323e-ffce-4a6e-83ae-bc0250080b04
📒 Files selected for processing (6)
packages/react-native-sdk/src/contexts/BackgroundFilters.tsxpackages/video-filters-react-native/android/src/main/java/com/streamio/videofiltersreactnative/common/BitmapVideoFilter.ktpackages/video-filters-react-native/android/src/main/java/com/streamio/videofiltersreactnative/common/VideoFrameWithBitmapFilter.ktpackages/video-filters-react-native/android/src/main/java/com/streamio/videofiltersreactnative/factories/BackgroundBlurFactory.ktpackages/video-filters-react-native/android/src/main/java/com/streamio/videofiltersreactnative/factories/VirtualBackgroundFactory.ktpackages/video-filters-react-native/ios/VideoFrameProcessors/ImageBackgroundVideoFrameProcessor.swift
✅ Files skipped from review due to trivial changes (1)
- packages/video-filters-react-native/android/src/main/java/com/streamio/videofiltersreactnative/common/BitmapVideoFilter.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/video-filters-react-native/ios/VideoFrameProcessors/ImageBackgroundVideoFrameProcessor.swift
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
Correctness, perf, and cleanup fixes in the React Native background-filter pipeline.
📝 Implementation notes
NotificationCenterobserver leak.pattern. Unblocks the capture thread so preview stays at 30 fps on older chips (e.g. iPhone XS) where Vision was previously eating most of the frame budget.
YuvFrameinstance (fixes camera-flip race); GL/libyuv resource cleanup via a newVideoFrameProcessor.dispose()lifecycle hook.unregisterAllFilters()on unmount so native processors can deallocate.Requires the paired react-native-webrtc PR for the
dispose()hook and a camera-flip regression fix.🎫 Ticket: https://linear.app/stream/issue/XYZ-123
📑 Docs: https://github.com/GetStream/docs-content/pull/
Summary by CodeRabbit
New Features
Bug Fixes
Performance Improvements