Conversation
WalkthroughAdds Vision Camera external-source support with per-track backpressure, delivery APIs, and per-track stats. Extends iOS native module/spec and JS/TS bridge, reworks VisionRTCView, adds example camera preview, test framework, and project/config hygiene updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Example UI
participant VC as VisionCamera (RN)
participant JS as VisionRTC JS API
participant TM as TurboModule (ObjC++)
participant SW as VisionRTC (Swift)
participant W as WebRTC Track
rect rgb(240,248,255)
note over UI,SW: Initialize & bind external source
UI->>VC: ensurePermissions + mount Camera
UI->>JS: createVisionCameraSource(cameraRef)
JS->>TM: createSource(...)
TM->>SW: bind source -> sourceId
TM-->>JS: sourceId
UI->>JS: createWebRTCTrack({sourceId, backpressure, fps})
JS->>TM: createTrack(opts)
TM->>SW: createTrack(opts + bind source)
SW-->>TM: trackId
TM-->>JS: trackId
JS-->>UI: render VisionRTCView(trackId)
end
rect rgb(245,255,240)
note over VC,SW: External frame delivery & backpressure
VC-->>JS: frame -> getNativeBuffer + ts
JS->>TM: deliverFrame(sourceId, pixelBuffer, ts)
TM->>SW: deliverFrame -> deliverExternalFrame
alt backpressure = latest-wins
SW->>SW: replace buffered frame for track
else backpressure = throttle
SW->>SW: gate frames by time
else drop-late
SW->>SW: drop frame if busy
end
SW->>W: enqueue/frame emitted to renderer
SW->>SW: update produced/delivered/dropped counters
end
rect rgb(255,248,240)
note over UI,SW: Controls & stats
UI->>JS: updateSource(sourceId,{position,torch,maxFps})
JS->>TM: updateSource(...)
TM->>SW: apply changes, reconfigure bindings
UI->>JS: updateTrack(trackId,{fps,backpressure})
JS->>TM: updateTrack(...)
TM->>SW: set constraints
UI->>JS: getStats(trackId?)
JS->>TM: getStatsForTrack / getStats
TM->>SW: return per-track/global stats
TM-->>JS: stats
JS-->>UI: display stats
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ios/VisionRTCModule.swift (1)
236-252: Dispose track must clean per-source bindings and per-track statsLeads to stale mappings, retained pixel buffers, and skewed aggregates.
@objc(disposeTrack:resolver:rejecter:) func disposeTrack(trackId: NSString, resolver: RCTPromiseResolveBlock, rejecter: RCTPromiseRejectBlock) { stateQueue.async(flags: .barrier) { self.activeTrackIds.remove(trackId as String) self.trackStates.removeValue(forKey: trackId as String) self.tracks.removeValue(forKey: trackId as String) self.sources.removeValue(forKey: trackId as String) self.capturers.removeValue(forKey: trackId as String) self.lastSent.removeValue(forKey: trackId as String) + // Unbind from source + if let sid = self.trackToSourceId.removeValue(forKey: trackId as String) { + var set = self.sourceToTrackIds[sid] ?? Set<String>() + set.remove(trackId as String) + if set.isEmpty { + self.sourceToTrackIds.removeValue(forKey: sid) + } else { + self.sourceToTrackIds[sid] = set + } + } + // Clear per-track buffers and stats + self.latestBufferByTrack.removeValue(forKey: trackId as String) + self.producedThisSecond.removeValue(forKey: trackId as String) + self.deliveredThisSecond.removeValue(forKey: trackId as String) + self.producedFpsByTrack.removeValue(forKey: trackId as String) + self.deliveredFpsByTrack.removeValue(forKey: trackId as String) + self.droppedFramesByTrack.removeValue(forKey: trackId as String) + self.lastSecondWallClock.removeValue(forKey: trackId as String) + self.pausedForReconfig.remove(trackId as String) }
🧹 Nitpick comments (7)
example/ios/VisionRtcExample/Info.plist (1)
52-55: iOS privacy strings added correctly.Camera/mic usage descriptions are present. If you don’t request mic permission in the example, you can defer NSMicrophoneUsageDescription to avoid review noise.
example/index.js (1)
6-12: Safe area provider at root: LGTM.Optional: pass initialWindowMetrics if you want to avoid first-render inset jumps on cold start.
src/vision-rtc-view.tsx (1)
33-41: Avoid hard-coded absolute positioning in container.Hard-coding absolute fill on the container can surprise consumers and complicate layout. Let the parent control positioning.
Apply:
const styles = StyleSheet.create({ container: { - flex: 1, - position: 'absolute', - top: 0, - left: 0, - right: 0, - bottom: 0, + flex: 1, }, });example/src/App.tsx (1)
173-201: Optional: disable controls based on state.Improve UX and avoid redundant calls by disabling buttons when not applicable (e.g., Start while creating/active, Stop when idle, Flip/Torch without source, FPS/Backpressure without track).
Example:
- <Button title="Start" onPress={onStart} /> + <Button title="Start" onPress={onStart} disabled={creating || !!trackId || !device} /> ... - <Button title="Stop" onPress={onStop} /> + <Button title="Stop" onPress={onStop} disabled={!trackId && !sourceId} /> ... - <Button title="Flip" onPress={onFlip} /> + <Button title="Flip" onPress={onFlip} disabled={!sourceId} /> ... - <Button title="15 fps" onPress={() => onFps(15)} /> + <Button title="15 fps" onPress={() => onFps(15)} disabled={!trackId} />src/index.ts (2)
116-126: Capabilities may misreport hardware encoder availabilityHardcoding
{h264: true, vp8: true}can mislead on devices without those encoders. Consider detecting via native factories or defaulting to false until probed.
128-144: Throwing plain object, notErrorIf callers expect
instanceof Error, consider throwing anErrorsubclass carrying the code. Otherwise keep as-is.- const err: VisionRtcError = { - code: 'ERR_EXPO_GO', - message: 'Expo Go is not supported. Use Expo Dev Client.', - }; - throw err; + const e = new Error('Expo Go is not supported. Use Expo Dev Client.') as Error & VisionRtcError; + (e as any).code = 'ERR_EXPO_GO'; + throw e;ios/VisionRTCModule.swift (1)
564-569: SwiftLint: unused closure parameter
fpsNowis unused. Replace with_to silence the warning.- }, onFps: { [weak self] fpsNow, dropped in + }, onFps: { [weak self] _, dropped in
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
example/ios/Podfile.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (12)
.gitignore(1 hunks)example/index.js(1 hunks)example/ios/VisionRtcExample/Info.plist(1 hunks)example/package.json(1 hunks)example/src/App.tsx(4 hunks)ios/VisionRTC+Spec.mm(5 hunks)ios/VisionRTCModule.swift(10 hunks)ios/VisionRTCViewManager.m(1 hunks)src/NativeVisionRtc.ts(2 hunks)src/index.ts(4 hunks)src/types.ts(2 hunks)src/vision-rtc-view.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
src/vision-rtc-view.tsx (2)
src/index.ts (1)
VisionRTCView(25-25)android/src/main/java/com/visionrtc/VisionRtcViewManager.kt (2)
name(12-42)name(28-36)
src/NativeVisionRtc.ts (1)
android/src/main/java/com/visionrtc/VisionRtcModule.kt (1)
source(48-53)
src/types.ts (2)
src/index.ts (3)
Capabilities(20-20)VisionRtcError(21-21)TrackStats(22-22)android/src/main/java/com/visionrtc/VisionRtcModule.kt (1)
source(48-53)
example/src/App.tsx (5)
ios/VisionRTCModule.swift (5)
createVisionCameraSource(57-68)disposeTrack(236-252)disposeSource(102-109)getStats(254-267)updateSource(70-100)src/index.ts (7)
createVisionCameraSource(27-31)createWebRTCTrack(44-49)disposeTrack(73-75)disposeSource(40-42)getStats(77-95)updateSource(33-38)VisionRTCView(25-25)android/src/main/java/com/visionrtc/VisionRtcModule.kt (4)
createVisionCameraSource(63-68)disposeTrack(198-207)getStats(209-214)name(27-240)src/vision-rtc-view.tsx (1)
VisionRTCView(21-31)android/src/main/java/com/visionrtc/VisionRtcViewManager.kt (1)
name(12-42)
ios/VisionRTC+Spec.mm (1)
ios/VisionRTCTrackRegistry.swift (1)
VisionRTCTrackRegistry(4-30)
ios/VisionRTCModule.swift (3)
src/index.ts (2)
updateSource(33-38)disposeSource(40-42)android/src/main/java/com/visionrtc/VisionRtcModule.kt (2)
createTrack(70-123)source(48-53)ios/VisionRTCTrackRegistry.swift (2)
track(23-29)register(11-15)
example/index.js (1)
example/src/App.tsx (1)
App(16-204)
ios/VisionRTCViewManager.m (3)
ios/VisionRTCTrackRegistry.swift (2)
unregister(17-21)VisionRTCTrackRegistry(4-30)android/src/main/java/com/visionrtc/VisionRtcViewManager.kt (1)
name(12-42)android/src/main/java/com/visionrtc/VisionRtcModule.kt (1)
disposeTrack(198-207)
src/index.ts (3)
ios/VisionRTCModule.swift (5)
updateSource(70-100)disposeSource(102-109)replaceSenderTrack(162-166)disposeTrack(236-252)getStats(254-267)android/src/main/java/com/visionrtc/VisionRtcModule.kt (4)
source(48-53)replaceSenderTrack(132-134)disposeTrack(198-207)getStats(209-214)src/types.ts (6)
VisionCameraSource(13-13)TrackOptions(3-11)VisionRTCTrack(37-39)TrackStats(62-69)Capabilities(43-49)VisionRtcError(57-60)
🪛 SwiftLint (0.57.0)
ios/VisionRTCModule.swift
[Warning] 564-564: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-ios
- GitHub Check: build-android
🔇 Additional comments (15)
.gitignore (1)
9-11: Ignore Cursor workspace artifactsAdding
.cursor/keeps the repo clean of Cursor IDE metadata. ✅ios/VisionRTCViewManager.m (1)
11-16: Good: prevent renderer leaks on unmount.Detaching the renderer in dealloc avoids dangling sinks. Solid cleanup.
src/types.ts (4)
6-11: Backpressure added to TrackOptions: LGTM.Type union aligns with usages across the PR.
41-49: New public types (Backpressure/Capabilities): LGTM.Clear, minimal surface.
51-61: VisionRtcError types: LGTM.Consistent error surface for consumers.
62-69: TrackStats shape: LGTM.Matches getStats mapping behavior.
src/NativeVisionRtc.ts (1)
48-53: Verify Android omission of getStatsForTrack is intentional. iOS implements it in ios/VisionRTCModule.swift (line 517), but no Kotlin/Java implementation exists—index.ts will fall back.src/index.ts (3)
33-38: Source management API looks goodRuntime source updates/disposal are correctly wired to native module.
77-95: Confirm Android fallback used
getStatsForTrackis only implemented in the iOS native module; Android lacks this method, so the fallback path will correctly engage.
66-71: No direct calls tosetTrackConstraintsoutsideupdateTrackios/VisionRTC+Spec.mm (2)
205-216: Per-track stats bridge LGTMBridged method forwards to Swift with proper Swift-availability guard.
218-244: Source lifecycle bridge LGTM
updateSource/disposeSourceare correctly bridged with Swift-availability checks.ios/VisionRTCModule.swift (3)
70-100: Source update/reset logic LGTMPer-track pause, buffer reset, and timed unpause after reconfig are sound.
517-528: Per-track stats API LGTMConsistent with TS
TrackStatsand JS surface.
418-516: External frame backpressure path looks correctCadencing and per-track counters are guarded and rolled per second; timestamp selection for latest-wins is sensible.
| const cameraRef = React.useRef<Camera>(null); | ||
| const device = useCameraDevice('back'); |
There was a problem hiding this comment.
Type error: useRef should allow null.
This won’t type-check in TS.
Apply:
- const cameraRef = React.useRef<Camera>(null);
+ const cameraRef = React.useRef<Camera | null>(null);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const cameraRef = React.useRef<Camera>(null); | |
| const device = useCameraDevice('back'); | |
| const cameraRef = React.useRef<Camera | null>(null); | |
| const device = useCameraDevice('back'); |
🤖 Prompt for AI Agents
In example/src/App.tsx around lines 18-19, the useRef generic currently excludes
null which causes a TypeScript type error; update the ref type to allow null
(e.g., change the generic to Camera | null) so the initial null value
type-checks, and ensure any usages are null-checked or non-null asserted where
appropriate.
| await ensurePermissions(); | ||
| const node = findNodeHandle(cameraRef.current); | ||
| if (!node) throw new Error('Camera view not ready'); | ||
| const {__nativeSourceId} = await createVisionCameraSource(node); | ||
| setSourceId(__nativeSourceId); | ||
| const created = await createWebRTCTrack( | ||
| {__nativeSourceId}, | ||
| { | ||
| fps: 30, | ||
| resolution: {width: 1280, height: 720}, | ||
| backpressure, | ||
| } | ||
| ); | ||
| newId = id; | ||
| setTrackId(id); | ||
| newId = created.trackId; | ||
| setTrackId(created.trackId); | ||
| } catch (err) { |
There was a problem hiding this comment.
Dispose source on failure to avoid leaks.
If track creation fails after creating the Vision source, the source is never disposed.
Apply:
setCreating(true);
let newId: string | null = null;
+ let createdSourceId: string | null = null;
try {
await ensurePermissions();
const node = findNodeHandle(cameraRef.current);
if (!node) throw new Error('Camera view not ready');
- const {__nativeSourceId} = await createVisionCameraSource(node);
- setSourceId(__nativeSourceId);
+ const {__nativeSourceId} = await createVisionCameraSource(node);
+ createdSourceId = __nativeSourceId;
+ setSourceId(__nativeSourceId);
const created = await createWebRTCTrack(
{__nativeSourceId},
{
fps: 30,
resolution: {width: 1280, height: 720},
backpressure,
}
);
newId = created.trackId;
setTrackId(created.trackId);
} catch (err) {
if (newId) {
try {
await disposeTrack(newId);
} catch {}
}
+ if (createdSourceId) {
+ try {
+ await disposeSource(createdSourceId);
+ } catch {}
+ }
console.error('Failed to start WebRTC track', err);
} finally {
setCreating(false);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await ensurePermissions(); | |
| const node = findNodeHandle(cameraRef.current); | |
| if (!node) throw new Error('Camera view not ready'); | |
| const {__nativeSourceId} = await createVisionCameraSource(node); | |
| setSourceId(__nativeSourceId); | |
| const created = await createWebRTCTrack( | |
| {__nativeSourceId}, | |
| { | |
| fps: 30, | |
| resolution: {width: 1280, height: 720}, | |
| backpressure, | |
| } | |
| ); | |
| newId = id; | |
| setTrackId(id); | |
| newId = created.trackId; | |
| setTrackId(created.trackId); | |
| } catch (err) { | |
| setCreating(true); | |
| let newId: string | null = null; | |
| let createdSourceId: string | null = null; | |
| try { | |
| await ensurePermissions(); | |
| const node = findNodeHandle(cameraRef.current); | |
| if (!node) throw new Error('Camera view not ready'); | |
| const {__nativeSourceId} = await createVisionCameraSource(node); | |
| createdSourceId = __nativeSourceId; | |
| setSourceId(__nativeSourceId); | |
| const created = await createWebRTCTrack( | |
| {__nativeSourceId}, | |
| { | |
| fps: 30, | |
| resolution: {width: 1280, height: 720}, | |
| backpressure, | |
| } | |
| ); | |
| newId = created.trackId; | |
| setTrackId(created.trackId); | |
| } catch (err) { | |
| if (newId) { | |
| try { | |
| await disposeTrack(newId); | |
| } catch {} | |
| } | |
| if (createdSourceId) { | |
| try { | |
| await disposeSource(createdSourceId); | |
| } catch {} | |
| } | |
| console.error('Failed to start WebRTC track', err); | |
| } finally { | |
| setCreating(false); | |
| } |
🤖 Prompt for AI Agents
In example/src/App.tsx around lines 47 to 62, the Vision camera source created
via createVisionCameraSource is not disposed if subsequent createWebRTCTrack
fails, leaking native resources; update the try/catch (or add a finally) to
store the created __nativeSourceId in an outer variable and, on error or in
finally when track creation did not complete, call and await the appropriate
source-disposal API (e.g., disposeVisionCameraSource or equivalent) only if
__nativeSourceId exists, then clear the variable; ensure disposal errors are
caught/logged but do not mask the original error.
| container: { | ||
| flex: 1, | ||
| backgroundColor: '#ffff', | ||
| justifyContent: 'center', | ||
| alignItems: 'center', | ||
| }, | ||
| controls: { |
There was a problem hiding this comment.
Color value typo.
'#ffff' is unusual; use '#fff' or '#ffffff'.
Apply:
- backgroundColor: '#ffff',
+ backgroundColor: '#fff',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| container: { | |
| flex: 1, | |
| backgroundColor: '#ffff', | |
| justifyContent: 'center', | |
| alignItems: 'center', | |
| }, | |
| controls: { | |
| container: { | |
| flex: 1, | |
| backgroundColor: '#fff', | |
| justifyContent: 'center', | |
| alignItems: 'center', | |
| }, | |
| controls: { |
🤖 Prompt for AI Agents
In example/src/App.tsx around lines 207 to 213, the container style uses an
invalid/typo color value '#ffff'; replace it with a valid 3- or 6-digit hex like
'#fff' or '#ffffff' (e.g., set backgroundColor to '#ffffff') so the style uses a
correct hex color.
| NSString *bp = opts.backpressure(); | ||
| if (bp != nil) { | ||
| optsDict[@"backpressure"] = bp; | ||
| } | ||
| [self.swift createTrack:source opts:optsDict resolver:resolve rejecter:reject]; |
There was a problem hiding this comment.
Compile-time bug: incorrect reading of optional backpressure
opts.backpressure() returns an optional; assigning to NSString * is invalid. Use the same pattern as fps()/resolution().
- NSString *bp = opts.backpressure();
- if (bp != nil) {
- optsDict[@"backpressure"] = bp;
- }
+ if (auto bp = opts.backpressure()) {
+ optsDict[@"backpressure"] = *bp;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| NSString *bp = opts.backpressure(); | |
| if (bp != nil) { | |
| optsDict[@"backpressure"] = bp; | |
| } | |
| [self.swift createTrack:source opts:optsDict resolver:resolve rejecter:reject]; | |
| if (auto bp = opts.backpressure()) { | |
| optsDict[@"backpressure"] = *bp; | |
| } | |
| [self.swift createTrack:source opts:optsDict resolver:resolve rejecter:reject]; |
🤖 Prompt for AI Agents
In ios/VisionRTC+Spec.mm around lines 105–109, opts.backpressure() returns an
optional and cannot be assigned directly to NSString *; follow the same pattern
as fps()/resolution() by capturing the optional (e.g., auto bp =
opts.backpressure()), check whether it has a value, and if so
extract/dereference the contained NSString and assign that to
optsDict[@"backpressure"] before calling createTrack; update the conditional
accordingly.
| NSString *bp = opts.backpressure(); | ||
| if (bp != nil) { | ||
| optsDict[@"backpressure"] = bp; | ||
| } |
There was a problem hiding this comment.
Same optional handling bug in setTrackConstraints
Mirror the fix here too.
- NSString *bp = opts.backpressure();
- if (bp != nil) {
- optsDict[@"backpressure"] = bp;
- }
+ if (auto bp = opts.backpressure()) {
+ optsDict[@"backpressure"] = *bp;
+ }🤖 Prompt for AI Agents
In ios/VisionRTC+Spec.mm around lines 173-176, the optional backpressure string
is being assigned without guarding against null/CFNull; mirror the previous fix
by retrieving the value into NSString *bp = opts.backpressure(); then only set
optsDict[@"backpressure"] when bp is non-nil and not kCFNull (e.g. if (bp != nil
&& bp != (id)kCFNull) { optsDict[@"backpressure"] = bp; }), ensuring you avoid
inserting null placeholders into the dictionary.
| readonly updateSource: ( | ||
| sourceId: string, | ||
| opts: {position?: 'front' | 'back'; torch?: boolean; maxFps?: number} | ||
| ) => Promise<void>; | ||
| readonly disposeSource: (sourceId: string) => Promise<void>; | ||
| readonly createTrack: ( |
There was a problem hiding this comment.
🧩 Analysis chain
New source APIs declared; confirm native parity.
Ensure both iOS and Android implement updateSource/disposeSource; Android snippets didn’t show them.
Run:
🏁 Script executed:
#!/bin/bash
rg -n -C2 --type=kt '\boverride fun (updateSource|disposeSource)\b'
rg -n -C2 --type=swift '@objc\((updateSource|disposeSource):'Length of output: 721
🏁 Script executed:
#!/bin/bash
# Search Android implementation for updateSource and disposeSource in Kotlin or Java
rg -nC2 '\bupdateSource\b' -g '*.kt' -g '*.java'
rg -nC2 '\bdisposeSource\b' -g '*.kt' -g '*.java'Length of output: 0
Implement updateSource and disposeSource in Android native module
iOS defines updateSource(_:opts:resolver:rejecter:) and disposeSource(_:resolver:rejecter:), but no Android counterparts were found. Add these @ReactMethod implementations in your Android VisionRTCModule to ensure parity.
🤖 Prompt for AI Agents
In src/NativeVisionRtc.ts around lines 22 to 27, the TypeScript interface
declares updateSource and disposeSource but the Android native module lacks
corresponding implementations; add two @ReactMethod functions in the Android
VisionRTCModule: updateSource(String sourceId, ReadableMap opts, Promise
promise) and disposeSource(String sourceId, Promise promise). Inside
updateSource, validate sourceId, extract opts.position/torch/maxFps from the
ReadableMap, apply changes to the underlying source object (update camera
position, toggle torch, adjust fps) with proper thread/context handling, and
resolve the promise on success or reject with a clear error on failure. In
disposeSource, validate sourceId, perform cleanup/removal of the source from the
module’s tracking structures, release resources on the correct thread, and
resolve or reject the promise accordingly so behavior matches the existing iOS
methods.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
example/src/App.tsx (2)
20-205: Hook the camera device to thefacingstate
Fliptogglesfacing, butuseCameraDevice('back')never re-runs, so the preview (and Vision camera source) stays on the back camera. Derive the device fromfacinginstead.- const cameraRef = React.useRef<Camera>(null); - const device = useCameraDevice('back'); + const cameraRef = React.useRef<Camera | null>(null); + const [facing, setFacing] = React.useState<'front' | 'back'>('back'); + const device = useCameraDevice(facing); @@ - const [torch, setTorch] = React.useState(false); - const [facing, setFacing] = React.useState<'front' | 'back'>('back'); + const [torch, setTorch] = React.useState(false);(Move the
facingstate beforeuseCameraDeviceso the hook has access to the current value.)
90-99: Dispose the source on unmount
Unmount cleanup only tears down the track; any active Vision camera source survives, leaking native state. Mirror theonStoplogic and dispose the source too.- React.useEffect(() => { - return () => { - if (trackId) { - disposeTrack(trackId).catch(() => {}); - } - }; - }, [trackId]); + React.useEffect(() => { + return () => { + if (trackId) { + disposeTrack(trackId).catch(() => {}); + } + if (sourceId) { + disposeSource(sourceId).catch(() => {}); + } + }; + }, [trackId, sourceId]);ios/VisionRTCModule.swift (1)
246-266: Remove disposed tracks from source bindings
disposeTrackclears the track but leavestrackToSourceIdandsourceToTrackIdsentries behind. Over time this builds stale bindings that keep delivering attempts and retains memory. Drop the mapping when the track goes away.stateQueue.async(flags: .barrier) { self.activeTrackIds.remove(trackId as String) self.trackStates.removeValue(forKey: trackId as String) self.tracks.removeValue(forKey: trackId as String) self.sources.removeValue(forKey: trackId as String) self.capturers.removeValue(forKey: trackId as String) self.lastSent.removeValue(forKey: trackId as String) + if let sourceId = self.trackToSourceId.removeValue(forKey: trackId as String) { + var set = self.sourceToTrackIds[sourceId] ?? Set<String>() + set.remove(trackId as String) + if set.isEmpty { + self.sourceToTrackIds.removeValue(forKey: sourceId) + } else { + self.sourceToTrackIds[sourceId] = set + } + } self.latestBufferByTrack.removeValue(forKey: trackId as String) self.producedThisSecond.removeValue(forKey: trackId as String) self.deliveredThisSecond.removeValue(forKey: trackId as String) self.lastSecondWallClock.removeValue(forKey: trackId as String) self.deliveredFpsByTrack.removeValue(forKey: trackId as String) self.producedFpsByTrack.removeValue(forKey: trackId as String) self.droppedFramesByTrack.removeValue(forKey: trackId as String) }
♻️ Duplicate comments (5)
src/NativeVisionRtc.ts (1)
22-27: Android implementation missing for new source APIsLines 22‑27 extend the native Spec with
updateSource/disposeSource, butVisionRtcModule.ktstill lacks the corresponding overrides (confirmed by the current Android source). Since the Kotlin class subclassesNativeVisionRtcSpec, the project now fails to compile on Android and JS calls would reject even if you stubbed them. Add@ReactMethodimplementations forupdateSourceanddisposeSource(plus wire their behavior) before landing.ios/VisionRTC+Spec.mm (2)
110-113: Fix optional backpressure extraction in createTrack
opts.backpressure()returns an optional; assigning it straight toNSString *causes a compile error. Mirror the fps/resolution pattern:- NSString *bp = opts.backpressure(); - if (bp != nil) { - optsDict[@"backpressure"] = bp; - } + if (auto bp = opts.backpressure()) { + optsDict[@"backpressure"] = *bp; + }Without this, the iOS target won’t build.
178-181: Apply the same optional fix in setTrackConstraintsLine 178 repeats the optional assignment problem, so
setTrackConstraintsalso fails to compile. Use the sameif (auto bp = opts.backpressure()) { … }pattern here.example/src/App.tsx (2)
31-145: Dispose the created source when track creation fails
IfcreateWebRTCTrackthrows aftercreateVisionCameraSourceresolves, the native source leaks andsourceIdstate still points to it. Capture the source id, dispose it inside thecatch, and clear the React state (already requested earlier).- const onStart = async () => { + const onStart = async () => { if (creating || trackId) return; setCreating(true); let newId: string | null = null; + let createdSourceId: string | null = null; try { await ensurePermissions(); const node = findNodeHandle(cameraRef.current); if (!node) throw new Error('Camera view not ready'); - const {__nativeSourceId} = await createVisionCameraSource(node); - setSourceId(__nativeSourceId); + const {__nativeSourceId} = await createVisionCameraSource(node); + createdSourceId = __nativeSourceId; + setSourceId(__nativeSourceId); const created = await createWebRTCTrack( {__nativeSourceId}, { fps: 30, resolution: {width: 1280, height: 720}, backpressure, } ); newId = created.trackId; setTrackId(created.trackId); } catch (err) { if (newId) { try { await disposeTrack(newId); } catch {} } + if (createdSourceId) { + try { + await disposeSource(createdSourceId); + } catch {} + setSourceId(null); + } console.error('Failed to start WebRTC track', err); } finally { setCreating(false); } };
19-21: Fix thecameraReftyping
The ref is initialised withnull; forcing it toCamerastill violates strict null checks and keeps the TypeScript error alive (previously flagged).- const cameraRef = React.useRef<Camera>(null); + const cameraRef = React.useRef<Camera | null>(null);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
example/ios/VisionRtcExample.xcodeproj/project.pbxproj(2 hunks)example/src/App.tsx(4 hunks)example/src/test-framework.tsx(1 hunks)example/src/vision-camera-frame-processor.ts(1 hunks)ios/VisionRTC+Spec.mm(5 hunks)ios/VisionRTCModule.swift(12 hunks)lefthook.yml(1 hunks)src/NativeVisionRtc.ts(2 hunks)src/index.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
example/src/test-framework.tsx (3)
example/src/vision-camera-frame-processor.ts (4)
processFrame(17-47)setSourceId(7-10)getFrameCount(49-51)clearSourceId(12-15)ios/VisionRTCModule.swift (4)
createVisionCameraSource(57-68)getStats(270-283)disposeTrack(245-268)disposeSource(102-116)src/index.ts (5)
createVisionCameraSource(27-31)createWebRTCTrack(44-49)getStats(88-106)disposeTrack(73-75)disposeSource(40-42)
example/src/vision-camera-frame-processor.ts (2)
ios/VisionRTCModule.swift (1)
deliverFrame(555-585)src/index.ts (1)
deliverFrame(77-86)
ios/VisionRTCModule.swift (3)
src/index.ts (3)
updateSource(33-38)disposeSource(40-42)deliverFrame(77-86)android/src/main/java/com/visionrtc/VisionRtcModule.kt (2)
createTrack(70-123)source(48-53)ios/VisionRTCTrackRegistry.swift (2)
track(23-29)register(11-15)
src/index.ts (4)
ios/VisionRTCModule.swift (6)
updateSource(70-100)disposeSource(102-116)replaceSenderTrack(169-173)disposeTrack(245-268)deliverFrame(555-585)getStats(270-283)android/src/main/java/com/visionrtc/VisionRtcModule.kt (4)
source(48-53)replaceSenderTrack(132-134)disposeTrack(198-207)getStats(209-214)src/types.ts (7)
VisionCameraSource(13-13)NativePixelSource(32-35)TrackOptions(3-11)VisionRTCTrack(37-39)TrackStats(62-69)Capabilities(43-49)VisionRtcError(57-60)src/vision-rtc-view.tsx (1)
VisionRTCView(3-9)
ios/VisionRTC+Spec.mm (1)
ios/VisionRTCTrackRegistry.swift (1)
VisionRTCTrackRegistry(4-30)
src/NativeVisionRtc.ts (1)
android/src/main/java/com/visionrtc/VisionRtcModule.kt (2)
source(48-53)name(27-240)
🪛 GitHub Check: lint
example/src/test-framework.tsx
[failure] 199-199:
Argument of type '(value: unknown) => void' is not assignable to parameter of type '() => void'.
[failure] 1-1:
'React' is declared but its value is never read.
🪛 SwiftLint (0.57.0)
ios/VisionRTCModule.swift
[Warning] 621-621: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-ios
- GitHub Check: build-android
| import React, {useState, useRef, useCallback, useEffect} from 'react'; | ||
| import {View, Text, Button, StyleSheet, ScrollView, Alert} from 'react-native'; | ||
| import {SafeAreaView, useSafeAreaInsets} from 'react-native-safe-area-context'; | ||
| import { | ||
| Camera, | ||
| useCameraDevice, | ||
| useFrameProcessor, | ||
| } from 'react-native-vision-camera'; | ||
| import { | ||
| createVisionCameraSource, | ||
| createWebRTCTrack, | ||
| disposeTrack, | ||
| disposeSource, | ||
| getStats, | ||
| } from 'react-native-vision-rtc'; |
There was a problem hiding this comment.
Remove unused default React import
Lint is failing with 'React' is declared but its value is never read. Drop the default import so the pipeline passes.
-import React, {useState, useRef, useCallback, useEffect} from 'react';
+import {useState, useRef, useCallback, useEffect} from 'react';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import React, {useState, useRef, useCallback, useEffect} from 'react'; | |
| import {View, Text, Button, StyleSheet, ScrollView, Alert} from 'react-native'; | |
| import {SafeAreaView, useSafeAreaInsets} from 'react-native-safe-area-context'; | |
| import { | |
| Camera, | |
| useCameraDevice, | |
| useFrameProcessor, | |
| } from 'react-native-vision-camera'; | |
| import { | |
| createVisionCameraSource, | |
| createWebRTCTrack, | |
| disposeTrack, | |
| disposeSource, | |
| getStats, | |
| } from 'react-native-vision-rtc'; | |
| // example/src/test-framework.tsx | |
| import {useState, useRef, useCallback, useEffect} from 'react'; | |
| import {View, Text, Button, StyleSheet, ScrollView, Alert} from 'react-native'; | |
| import {SafeAreaView, useSafeAreaInsets} from 'react-native-safe-area-context'; | |
| import { | |
| Camera, | |
| useCameraDevice, | |
| useFrameProcessor, | |
| } from 'react-native-vision-camera'; | |
| import { | |
| createVisionCameraSource, | |
| createWebRTCTrack, | |
| disposeTrack, | |
| disposeSource, | |
| getStats, | |
| } from 'react-native-vision-rtc'; |
🧰 Tools
🪛 GitHub Check: lint
[failure] 1-1:
'React' is declared but its value is never read.
🤖 Prompt for AI Agents
In example/src/test-framework.tsx around lines 1 to 15, the default React import
is unused causing a lint error; remove the default import token "React" from the
import statement and keep the named imports (e.g. import {useState, useRef,
useCallback, useEffect} from 'react';) so the file uses only the consumed hooks
and the linter error is resolved.
| const cameraRef = useRef<Camera>(null); | ||
| const device = useCameraDevice('back'); |
There was a problem hiding this comment.
Allow cameraRef to store null
useRef<Camera>(null) violates the type contract under strictNullChecks and breaks the TypeScript build. Let the ref accept null.
- const cameraRef = useRef<Camera>(null);
+ const cameraRef = useRef<Camera | null>(null);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const cameraRef = useRef<Camera>(null); | |
| const device = useCameraDevice('back'); | |
| const cameraRef = useRef<Camera | null>(null); | |
| const device = useCameraDevice('back'); |
🤖 Prompt for AI Agents
In example/src/test-framework.tsx around lines 43 to 44, the ref is declared as
useRef<Camera>(null) which violates strictNullChecks; change the ref type to
allow null (e.g., useRef<Camera | null>(null) or the equivalent RefObject type)
so the ref can store null without breaking the TypeScript build.
| while (attempts < maxAttempts) { | ||
| await new Promise((resolve) => setTimeout(resolve, 100)); | ||
| const currentFrameCount = getFrameCount(); | ||
|
|
There was a problem hiding this comment.
Fix setTimeout callback signature
Passing resolve directly gives it the type (value: unknown) => void, which doesn’t satisfy () => void and triggers the lint error. Wrap the call so the handler has the right shape.
- await new Promise((resolve) => setTimeout(resolve, 100));
+ await new Promise<void>((resolve) =>
+ setTimeout(() => resolve(), 100)
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| while (attempts < maxAttempts) { | |
| await new Promise((resolve) => setTimeout(resolve, 100)); | |
| const currentFrameCount = getFrameCount(); | |
| while (attempts < maxAttempts) { | |
| await new Promise<void>((resolve) => | |
| setTimeout(() => resolve(), 100) | |
| ); | |
| const currentFrameCount = getFrameCount(); |
🧰 Tools
🪛 GitHub Check: lint
[failure] 199-199:
Argument of type '(value: unknown) => void' is not assignable to parameter of type '() => void'.
🤖 Prompt for AI Agents
In example/src/test-framework.tsx around lines 198 to 201, the setTimeout call
passes resolve directly which gives the callback the wrong signature; wrap
resolve in a zero-argument function so the timer handler is () => void (e.g.,
setTimeout(() => resolve(), 100)) to satisfy the expected callback type and
eliminate the lint error.
| import type {Frame} from 'react-native-vision-camera'; | ||
| import {deliverFrame} from 'react-native-vision-rtc'; | ||
|
|
||
| let sourceId: string | null = null; | ||
| let frameCount = 0; | ||
|
|
||
| export function setSourceId(id: string) { | ||
| sourceId = id; | ||
| frameCount = 0; | ||
| } | ||
|
|
||
| export function clearSourceId() { | ||
| sourceId = null; | ||
| frameCount = 0; | ||
| } | ||
|
|
||
| export function processFrame(frame: Frame): void { | ||
| 'worklet'; | ||
|
|
||
| if (!sourceId) { | ||
| console.log('VisionCamera: No source ID set, skipping frame delivery'); | ||
| return; | ||
| } | ||
|
|
||
| frameCount++; | ||
|
|
||
| // Only deliver every 2nd frame to avoid overwhelming the system during testing | ||
| if (frameCount % 2 !== 0) { | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| // Get the native pixel buffer reference | ||
| const pixelBufferRef = (frame as Frame).getNativeBuffer(); | ||
| const timestampNs = frame.timestamp * 1000000; // Convert to nanoseconds | ||
|
|
||
| if (pixelBufferRef) { | ||
| deliverFrame(sourceId, pixelBufferRef, timestampNs).catch( | ||
| (error: unknown) => { | ||
| console.warn('VisionCamera: Failed to deliver frame:', error); | ||
| } | ||
| ); | ||
| } | ||
| } catch (error) { | ||
| console.warn('VisionCamera: Error processing frame:', error); | ||
| } | ||
| } |
There was a problem hiding this comment.
Route deliverFrame through runOnJS
Frame processors run in a worklet; calling a regular JS promise (deliverFrame) directly will throw at runtime. Bounce back to JS via runOnJS and keep the existing error handling there.
-import type {Frame} from 'react-native-vision-camera';
-import {deliverFrame} from 'react-native-vision-rtc';
+import type {Frame} from 'react-native-vision-camera';
+import {runOnJS} from 'react-native-reanimated';
+import {deliverFrame} from 'react-native-vision-rtc';
let sourceId: string | null = null;
let frameCount = 0;
+function deliverFrameFromWorklet(
+ id: string,
+ pixelBuffer: unknown,
+ timestampNs: number
+) {
+ deliverFrame(id, pixelBuffer, timestampNs).catch((error: unknown) => {
+ console.warn('VisionCamera: Failed to deliver frame:', error);
+ });
+}
+
export function setSourceId(id: string) {
sourceId = id;
frameCount = 0;
}
@@
- if (pixelBufferRef) {
- deliverFrame(sourceId, pixelBufferRef, timestampNs).catch(
- (error: unknown) => {
- console.warn('VisionCamera: Failed to deliver frame:', error);
- }
- );
- }
+ if (pixelBufferRef) {
+ runOnJS(deliverFrameFromWorklet)(
+ sourceId,
+ pixelBufferRef,
+ timestampNs
+ );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import type {Frame} from 'react-native-vision-camera'; | |
| import {deliverFrame} from 'react-native-vision-rtc'; | |
| let sourceId: string | null = null; | |
| let frameCount = 0; | |
| export function setSourceId(id: string) { | |
| sourceId = id; | |
| frameCount = 0; | |
| } | |
| export function clearSourceId() { | |
| sourceId = null; | |
| frameCount = 0; | |
| } | |
| export function processFrame(frame: Frame): void { | |
| 'worklet'; | |
| if (!sourceId) { | |
| console.log('VisionCamera: No source ID set, skipping frame delivery'); | |
| return; | |
| } | |
| frameCount++; | |
| // Only deliver every 2nd frame to avoid overwhelming the system during testing | |
| if (frameCount % 2 !== 0) { | |
| return; | |
| } | |
| try { | |
| // Get the native pixel buffer reference | |
| const pixelBufferRef = (frame as Frame).getNativeBuffer(); | |
| const timestampNs = frame.timestamp * 1000000; // Convert to nanoseconds | |
| if (pixelBufferRef) { | |
| deliverFrame(sourceId, pixelBufferRef, timestampNs).catch( | |
| (error: unknown) => { | |
| console.warn('VisionCamera: Failed to deliver frame:', error); | |
| } | |
| ); | |
| } | |
| } catch (error) { | |
| console.warn('VisionCamera: Error processing frame:', error); | |
| } | |
| } | |
| import type {Frame} from 'react-native-vision-camera'; | |
| import {runOnJS} from 'react-native-reanimated'; | |
| import {deliverFrame} from 'react-native-vision-rtc'; | |
| let sourceId: string | null = null; | |
| let frameCount = 0; | |
| function deliverFrameFromWorklet( | |
| id: string, | |
| pixelBuffer: unknown, | |
| timestampNs: number | |
| ) { | |
| deliverFrame(id, pixelBuffer, timestampNs).catch((error: unknown) => { | |
| console.warn('VisionCamera: Failed to deliver frame:', error); | |
| }); | |
| } | |
| export function setSourceId(id: string) { | |
| sourceId = id; | |
| frameCount = 0; | |
| } | |
| export function clearSourceId() { | |
| sourceId = null; | |
| frameCount = 0; | |
| } | |
| export function processFrame(frame: Frame): void { | |
| 'worklet'; | |
| if (!sourceId) { | |
| console.log('VisionCamera: No source ID set, skipping frame delivery'); | |
| return; | |
| } | |
| frameCount++; | |
| // Only deliver every 2nd frame to avoid overwhelming the system during testing | |
| if (frameCount % 2 !== 0) { | |
| return; | |
| } | |
| try { | |
| // Get the native pixel buffer reference | |
| const pixelBufferRef = (frame as Frame).getNativeBuffer(); | |
| const timestampNs = frame.timestamp * 1000000; // Convert to nanoseconds | |
| if (pixelBufferRef) { | |
| runOnJS(deliverFrameFromWorklet)( | |
| sourceId, | |
| pixelBufferRef, | |
| timestampNs | |
| ); | |
| } | |
| } catch (error) { | |
| console.warn('VisionCamera: Error processing frame:', error); | |
| } | |
| } |
| const Constants = require('expo-constants').default; | ||
| return Constants?.appOwnership === 'expo'; | ||
| } catch { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| function hasVisionCamera(): boolean { | ||
| try { | ||
| const vc = require('react-native-vision-camera'); | ||
| return !!vc; | ||
| } catch { | ||
| return false; |
There was a problem hiding this comment.
Optional Expo detection breaks Metro builds
Lines 111‑123 call require('expo-constants'). Metro resolves static requires at bundle time, so any consumer without Expo will now hit “Unable to resolve module 'expo-constants'” even though the call sits inside a try/catch. Please switch to a detection strategy that doesn’t require the package (e.g., inspect NativeModules.ExponentConstants or gate the require via a lazy globalThis.__r?. hook) so non-Expo apps keep building.
🤖 Prompt for AI Agents
In src/index.ts around lines 111 to 123, the optional Expo detection uses a
static require('expo-constants') which breaks Metro bundling; change the
detection to avoid static requires by checking runtime-native exports instead
(e.g., inspect NativeModules.ExponentConstants or use a guarded dynamic require
via globalThis.__r?. or a conditional typeof globalThis.__r === 'function' ?
globalThis.__r('expo-constants') : undefined) so Metro does not try to resolve
the module at bundle time; update the hasExpo detection to read from
NativeModules or use the guarded dynamic require and return the same boolean
result, keeping the try/catch logic only around the guarded access.
1 participant
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores