Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Pixel 7a detected face boundaries not correct #30

Closed
imelos opened this issue Apr 13, 2024 · 20 comments
Closed

[BUG] Pixel 7a detected face boundaries not correct #30

imelos opened this issue Apr 13, 2024 · 20 comments
Labels
bug Something isn't working

Comments

@imelos
Copy link

imelos commented Apr 13, 2024

i test on Xiaomi MI A2 Lite and have correct face boundaries.
My friend test on Pixel 7A and receive not correct boundaries shifted left and bottom.

I don't have access to Pixel 7a to try reproduce and fix this.
Any ideas is this possible to fix?

default example/src/index.tsx with useCameraDevice('back')
[react-native-vision-camera-face-detector] 1.4.0
[react-native-vision-camera] 4.0.0-beta.13

MI A2 Lite
right
Pixel 7a
bad

@imelos imelos added the bug Something isn't working label Apr 13, 2024
@luicfrr
Copy link
Owner

luicfrr commented Apr 15, 2024

@imelos this will be difficult to fix without access to the device, logs, and other relevant information.

However, I suspect the problem might be related to scale values.

Perhaps these values are higher than they should be? 🤔

@mrousavy
Copy link
Contributor

@nonam4 I also btw just experienced a similar issue on iOS, not sure if it's the same cause, but basically scaleX and scaleY shouldn't be done.

https://github.com/nonam4/react-native-vision-camera-face-detector/blob/22c87fe4af1fd44fdff7bf729dd9bebe41da176e/ios/VisionCameraFaceDetector.swift#L245-L246

You are making assumptions about a preview view here, whereas the coordinate system of a Frame Processor should always be the same of the Frame (.width/.height).

The user is responsible for converting that to preview coordinates. Maybe I can expose a function that does that.
But in reality it's just not as simple to scale it, because sometimes there's a pixel-shift involved, sometimes the width/height are flipped on Android, etc.

And in my case that was wrong (I explicitly patched this lib and set scaleX and scaleY to 1) because I am drawing in a Skia Frame Processor - and those use the same coordinate system as the Frame. It will be auto-scaled by the Preview later. mrousavy/react-native-vision-camera#2743

So I recommend you remove the scaling from your code, and let the user manually convert that to preview coordinates if needed. In Skia Frame Processors the user doesn't need to do that, it is just always the Frame coordinates.

@luicfrr
Copy link
Owner

luicfrr commented Apr 16, 2024

@mrousavy it would help a lot if vision camera provide a preview coordinates conversion.

about scaling, I took this expo-face-detector's code as inspiration to this feature but it seems to cause many problems. Maybe the best sollution is to let user handle this conversion on JS side.

@mrousavy
Copy link
Contributor

yup, and in JS I could expose a method to do this then.

@luicfrr
Copy link
Owner

luicfrr commented Apr 16, 2024

I already removed scaling in 67d98f4.
Now waiting for this new method so I can update my example app

@mrousavy
Copy link
Contributor

Hm, technically the user can also implement this.

We just need to scale frame.width/height to view.width/height by applying a center-crop, and then optionally also rotate starting from frame.orientation to current view rotation. That's it

@luicfrr
Copy link
Owner

luicfrr commented Apr 16, 2024

Hm, technically the user can also implement this.

We just need to scale frame.width/height to view.width/height by applying a center-crop, and then optionally also rotate starting from frame.orientation to current view rotation. That's it

you mean this removed function? 😅

@mrousavy
Copy link
Contributor

uhm yea lol I mean exactly that. On iOS the Frame.orientation is now correct I think - it's always portrait.

@imelos
Copy link
Author

imelos commented Apr 16, 2024

But this removed function has the same code as native one and will bring incorrect face boundaries as i mentioned in topic. Am i understand right?

android

if (rotation == 270 || rotation == 90) {
        scaleX = windowWidth.toDouble() / image.height
        scaleY = windowHeight.toDouble() / image.width
      } else {
        scaleX = windowWidth.toDouble() / image.width
        scaleY = windowHeight.toDouble() / image.height
      }

js

if ( !isIos && (
      degrees === 90 ||
      degrees === 270
    ) ) {
      // frame sizes are inverted due to vision camera orientation bug
      scaleX = windowWidth / frame.height
      scaleY = windowHeight / frame.width
    } else {
      scaleX = windowWidth / frame.width
      scaleY = windowHeight / frame.height
    }

@luicfrr
Copy link
Owner

luicfrr commented Apr 16, 2024

@imelos Doing this scaling on js side using Dimensions.get('window') method give us lower screen size because of android's status and bottom navigation bars.

For Android the window dimension will exclude the size used by the status bar (if not translucent) and bottom navigation bar

I'm still testing but I suppose this makes difference on scaling.

@imelos
Copy link
Author

imelos commented Apr 16, 2024

@nonam4 Now i understand. Thank you!

@mrousavy
Copy link
Contributor

My 2 cents are; if you run something on a Frame, the returned coordinates should be relative to the Frame.

The user is responsible for converting that to the Preview later. Maybe I'll add a helper function for this at some point.

@luicfrr
Copy link
Owner

luicfrr commented Apr 18, 2024

@imelos I found the problem with scalling. I'm working on a sollution right now, this issue will be closed when it's really fixed.

@mrousavy I'll make native side scalling optional.
I think this will be the best way to support skia as it use the same coordinate system as the Frame

@imelos
Copy link
Author

imelos commented Apr 18, 2024

@nonam4 i switched to js side scaling calculation and its fixed my issue. The problem was as you mentioned before - because of wrong scaling calculations between native and js

@mrousavy
Copy link
Contributor

Cool yea making it optional on the native side is an okay solution for now.
I think in the future there definitely needs to be a solution that every plugin can use. Basically all plugins should use the Frame's coordinate system for any points or coordinates they return. That way it is standardized, and depending on what the user wants he can then convert using helper functions, maybe from VisionCamera.
There's different use-cases:

  • Drawing to the Frame w/ Skia (that uses the Frame's coordinate system and orientation!)
  • Displaying something as a View over the Preview (that uses the Preview view's coordinate system and is always portrait and properly resized/fitted)

Thanks for your hard work man, I'm probably gonna use this plugin as a prime example to show off how VisionCamera Plugins work if that's cool with you?
Maybe mention it in a talk or something.

@luicfrr
Copy link
Owner

luicfrr commented Apr 19, 2024

@mrousavy Sure man, for me it's an honor to have you using my plugin 😁

@luicfrr
Copy link
Owner

luicfrr commented Apr 19, 2024

@imelos Can you please test if v1.5 branch fix the issue?

@imelos
Copy link
Author

imelos commented Apr 20, 2024

nonam4 Yes. Without autoScale it works well. Thank you.

@Nurmehemmed
Copy link

Nurmehemmed commented Apr 22, 2024

@nonam4 Hi, Nonam. I have also face bound size problems in different devices. When can i install your last solution? It is urgent little bit. Thanks very much. P.s I tested in different devices. 2 Samsung device, 2 Huawei, 1 Xiaomi devices. Almost all of them returned different sizes. I am using this version (https://www.npmjs.com/package/react-native-vision-camera-face-detector/v/1.5.0)

@luicfrr luicfrr mentioned this issue Apr 22, 2024
Merged
@Nurmehemmed
Copy link

@nonam4 hi. I hope your works are good. I get Frame Processor Error: Regular javascript function '' cannot be shared. Try decorating the function with the 'worklet' keyword to allow the javascript function
to be used as a worklet., js engine: VisionCamera
error after update to V1.6. My Camera page component is like this
` function handleFacesDetection(faces: Face[], frame: Frame) {
console.log('faces', faces.length, 'frame', frame.toString());
}

{hasPermission && (
<Camera
photoQualityBalance="speed"
photo={true}
ref={camera}
isActive={true}
onInitialized={() => {
setIsInitialized(true);
}}
style={[StyleSheet.absoluteFill]}
device={device!}
faceDetectionCallback={handleFacesDetection}
orientation="portrait"
faceDetectionOptions={{
performanceMode: 'fast',
classificationMode: 'all',
}}
zoom={0}
/>
)}`

I enabled processNestedWorklets to true already
module.exports = { presets: ['module:@react-native/babel-preset'], plugins: [ ['react-native-worklets-core/plugin'], [ 'react-native-reanimated/plugin', { processNestedWorklets: true, }, ], ], };
Can u help me please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

4 participants