chore: caught up with the upstream#7
Conversation
GladosBlueWallet
left a comment
GladosBlueWallet
left a comment
There was a problem hiding this comment.
Upstream face-detection port with iOS Vision instead of ML Kit — fine hypothesis, sloppy execution. Android got a costume rack of dead types, iOS IDs shuffle every frame like test subjects in a centrifuge, and your bonus onWindowFocusChanged camera init can double-bind. One chore: commit hides a feature drop; no CONTRIBUTING.md exists, but the PR template still wanted proof, not poetry.
| import com.facebook.react.uimanager.events.Event | ||
| import com.rncamerakit.FacePayload | ||
|
|
||
| class FaceDetectedEvent( |
There was a problem hiding this comment.
[HIGH] Thirty-six lines of Android face-event theater, zero dispatches. Upstream cosplay without ML Kit is bloat, not architecture — delete the corpse or wire it.
| @@ -0,0 +1,14 @@ | |||
| package com.rncamerakit | |||
|
|
|||
| data class FacePayload( | |||
There was a problem hiding this comment.
[HIGH] Whole FacePayload data class exists solely so a constant can rent an apartment. Move the constant; evict the unused tenant.
| } | ||
| } | ||
|
|
||
| fun setFaceDetectionThrottleMs(throttleMs: Int) { |
There was a problem hiding this comment.
[HIGH] faceDetectionThrottleMs is written, never read — decorative state on a platform that doesn't detect faces. Stub props shouldn't pretend to work.
| cameraProvider?.unbindAll() | ||
| } | ||
|
|
||
| override fun onWindowFocusChanged(hasWindowFocus: Boolean) { |
There was a problem hiding this comment.
[HIGH] onWindowFocusChanged races onAttachedToWindow: two setupCamera() calls, duplicate orientation listeners, double bindCameraUseCases. Congratulations — you fixed blank preview by inventing a leak.
| } | ||
|
|
||
| let observations = request.results ?? [] | ||
| return observations.enumerated().map { build(from: $1, id: $0) } |
There was a problem hiding this comment.
[HIGH] id is array index, not a face. Order shuffles → IDs swap → your example's key={face.id} remounts overlays every frame. Tracking API cosplaying as identity.
| DispatchQueue.main.async { | ||
| let layer = self.cameraPreview.previewLayer | ||
| let layerSize = layer.bounds.size | ||
| guard layerSize.width > 0, layerSize.height > 0 else { return } |
There was a problem hiding this comment.
[MEDIUM] Throttle slot consumed on visionQueue, then main bails on zero layer/frame rect — events vanish silently until the next window. Face detection with Schrodinger callbacks.
| kCVPixelBufferPixelFormatTypeKey as String: kCVPixelFormatType_32BGRA, | ||
| ] | ||
| self.videoDataOutput.setSampleBufferDelegate(self, queue: self.visionQueue) | ||
| if self.session.canAddOutput(self.videoDataOutput) { |
There was a problem hiding this comment.
[MEDIUM] canAddOutput fails → isVideoDataOutputAttached stays false, JS thinks face detection is on, zero frames forever. No error event. Silent failure — my favorite kind, for me.
| "registry": "https://registry.npmjs.org/" | ||
| }, | ||
| "version": "17.0.4", | ||
| "version": "18.0.0", |
There was a problem hiding this comment.
[LOW] Single commit labeled chore: for v18 + iOS face detection API. Mislabeled releases are how future you discovers the cake was a lie.
2 participants
i pulled fresh upstream.
they have new facedetection api that again relies on mlkit on android - my agent removed that