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

fix: Fix Frame.orientation, it should always be fixed #3077

Merged
merged 5 commits into from
Jul 12, 2024

Conversation

mrousavy
Copy link
Owner

@mrousavy mrousavy commented Jul 12, 2024

What

So this has been a topic for a while, people constantly asked questions about this.

@xulihang also pointed out that Frame.orientation is a bit confusing.

So I am making this (kinda breaking) change (sorry!) to get this right.

First we need to understand a few things:

  1. Preview will always be rotated accordingly to screen/view rotation. User can lock this using the 🔒 button in control center.
  2. Photo and Video can either be the same as the Preview (outputOrientation="preview"), or be allowed to rotate even if the phone is locked (outputOrientation="device", this follows the gyroscope/accelerometer). This allows building UIs like the native iOS Camera app, where the view all stays the same, but only the buttons can actually rotate (again, taking gyro/accelerometer into consideration).
  3. Code Scanner should follow the Preview orientation, since the coordinates it returns are always respective to screen coordinates (this is now fixed with this PR! previously it was not the case)
  4. Frame Processors allow full control, and their Frame buffers are always in native sensor orientation, meaning it is always some fixed orientation (e.g. landscape-left), no matter if the phone is in portrait or landscape.

Now for Point 4 we really need to understand that the Frame cannot by physically rotated (rotating pixels in a 4k buffer is expensive), so that's why it is always in a fixed orientation, which is just coming from the native device sensor. (e.g. landscape-left)

In order to make Frame Processor Plugins (e.g. MLKit face detection) work, we need to tell the plugin in what orientation the Frame is, so it can internally know how to read the pixels in the Frame.
If it is portrait, it can assume that a Face is upright.

This graphic explains it quite well:

Portrait Landscape

The blue frame is always in the same orientation as you can see. The phone changes orientation, and the face inside the Frame changes orientation, but the Frame buffer itself is the same.

So now with this PR, Frame.orientation is always just the native device sensor orientation, and the user can then either just rotate it by this orientation (then it will just always be "up-right" in the phone/iPad's natural portrait orientation), or also rotate it more by previewOrientation or outputOrientation - this is fully up to the user depending on the use-case.

For example to detect faces you might want to rotate by outputOrientation, because otherwise the face is sideways in a portrait Frame and MLKit cannot detect faces sideways - only "up-right" faces.

For Frame Processor Plugin authors

If you are a Frame Processor Plugin author, make sure that you use Frame.orientation correctly now.

For example, if you use Apple's Vision-SDK for Face/Image/Text recognition, set the VisionImage.orientation exactly to the Frame.orientation.

public override func callback(_ frame: Frame, withArguments arguments: [AnyHashable: Any]?) -> Any {
  let image = VisionImage(buffer: frame.buffer)
  image.orientation = frame.orientation

This is now in it's native portrait orientation.

If the user holds his phone sideways (aka not in it's native portrait orientation), then you would also need to rotate that Orientation.

VisionCamera does not do that automatically, because each use case is different - for example in a Skia Frame Processor you want to lock the orientation to portrait, otherwise the canvas would rotate within itself.

So if you also want to support orientation in your Frame Processor, you need to rotate the Frame.orientation by whatever the current output orientation is.

To get the current output orientation you can just add it as a parameter to your Frame Processor Plugin - then the user can also decide whether he wants to support it or not.

In your plugin:

public override func callback(_ frame: Frame, withArguments arguments: [AnyHashable: Any]?) -> Any {
  let image = VisionImage(buffer: frame.buffer)
  image.orientation = frame.orientation

  if let outputOrientation = arguments["outputOrientation"] as? String {
    // parse outputOrientation String and then rotate image.orientation by that
  }

From the user perspective:

function App() {
  const outputOrientation = useSharedValue('portrait')

  const frameProcessor = useFrameProcessor((frame) => {
    'worklet'
    myCustomFpPlugin(frame, { outputOrientation: outputOrientation.value })
  })

  return <Camera {...props} onOutputOrientationChanged={(o) => outputOrientation.value = o} />

Note: In a future version of VisionCamera I might add outputOrientation to Frame directly, so you don't have to ask the user for it.
Then you can decide whether to use the raw orientation (e.g. for skia plugins), the preview's orientation, or the output orientation.

Changes

Tested on

Related issues

Copy link

vercel bot commented Jul 12, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-vision-camera ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 12, 2024 1:17pm

@mrousavy mrousavy merged commit f39ca07 into main Jul 12, 2024
13 checks passed
@mrousavy mrousavy deleted the fix/frame-orientation branch July 12, 2024 17:08
@mrousavy
Copy link
Owner Author

btw; In a future version of VisionCamera (maybe 5.x.x), Frame will be a Swift-based class that has better accessors (for PixelFormat and Orientation), and will also hold preview- and output-orientation.

This will also be more efficient, as I will skip the Objective-C route.

I am working on building this foundation in a private repo elsewhere, the idea is that this will be much more efficient 😄

@Flocurry
Copy link

Hello @mrousavy

Can you explain me what is the problem please ?

#3051 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants