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: Remove sensorOrientation hack on iOS, it's always portrait #3096

Merged
merged 1 commit into from
Jul 24, 2024

Conversation

mrousavy
Copy link
Owner

What

So either I was completely drunk when building this initially, or I was way too tired already.

But I just re-visited the orientation implementation on iOS, and it seems like .sensorOrientation always returns .portrait!

I just tested this on both my iPhone 15 Pro and my iPhone 8 and they work in every orientation - no matter if landscape or portrait, no matter if front or back camera.


I revisited the orientation implementation because I wanted to to get rid of the sensorOrientation hack I built.
The hack was that everytime I want to know the sensorOrientation of an AVCaptureDevice, I have to build up an AVCaptureSession and attach a Video Output to it - which was slow (~5ms per call, 20 calls on an iPhone 15 Pro at startup), and was insanely hacky.
It also crashed when you don't have camera permission yet, or the device is invalid.

This is the hacky code btw:

var sensorOrientation: Orientation {
// TODO: There is no iOS API to get native sensor orientation.
// - The new `RotationCoordinator` API is a blackbox, and cannot be used statically.
// - Dynamically creating an AVCaptureSession is very hacky and has a runtime overhead.
// Hopefully iOS adds an API to get sensor orientation soon so we can use that!
// 0. If we don't have Camera permission yet, we cannot create a temporary AVCaptureSession.
// So we just return the default orientation as a workaround...
let cameraPermissionStatus = AVCaptureDevice.authorizationStatus(for: .video)
if cameraPermissionStatus != .authorized {
return DEFAULT_SENSOR_ORIENTATION
}
// 1. Create a capture session
let session = AVCaptureSession()
if session.canSetSessionPreset(.low) {
session.sessionPreset = .low
}
// 2. Add this device as an input
guard let input = try? AVCaptureDeviceInput(device: self) else {
VisionLogger.log(level: .error, message: "Cannot dynamically determine \(uniqueID)'s sensorOrientation, " +
"because the AVCaptureDeviceInput cannot be created. Falling back to \(DEFAULT_SENSOR_ORIENTATION)...")
return DEFAULT_SENSOR_ORIENTATION
}
guard session.canAddInput(input) else {
VisionLogger.log(level: .error, message: "Cannot dynamically determine \(uniqueID)'s sensorOrientation, because " +
"it cannot be added to the temporary AVCaptureSession. Falling back to \(DEFAULT_SENSOR_ORIENTATION)...")
return DEFAULT_SENSOR_ORIENTATION
}
session.addInput(input)
// 3. Add an output (e.g. video data output)
let output = AVCaptureVideoDataOutput()
output.automaticallyConfiguresOutputBufferDimensions = false
output.deliversPreviewSizedOutputBuffers = true
guard session.canAddOutput(output) else {
VisionLogger.log(level: .error, message: "Cannot dynamically determine \(uniqueID)'s sensorOrientation, because " +
"the AVCaptureVideoDataOutput cannot be added to the AVCaptureSession. Falling back to \(DEFAULT_SENSOR_ORIENTATION)...")
return DEFAULT_SENSOR_ORIENTATION
}
session.addOutput(output)
// 4. Inspect the default orientation of the output
let defaultOrientation = output.orientation
// 5. Rotate the default orientation by the default sensor orientation we know of
var sensorOrientation = defaultOrientation.rotatedBy(orientation: DEFAULT_SENSOR_ORIENTATION)
// 6. If we are on the front Camera, AVCaptureVideoDataOutput.orientation is mirrored.
if position == .front {
sensorOrientation = sensorOrientation.flipped()
}
return sensorOrientation
}

And now I just return .portrait - which works and speeds up the app! :)

Changes

So now sensorOrientation doesn

Tested on

  • iPhone 15 Pro
  • iPhone 8

Related issues

Copy link

vercel bot commented Jul 23, 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 23, 2024 8:53pm

@mrousavy mrousavy merged commit d670ca9 into main Jul 24, 2024
8 checks passed
@mrousavy mrousavy deleted the feat/remove-sensorOrientation branch July 24, 2024 12:41
isaaccolson pushed a commit to isaaccolson/deliveries-mobile that referenced this pull request Oct 30, 2024
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.

1 participant