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

feat: Implement Orientation #2932

Merged
merged 60 commits into from
Jun 6, 2024
Merged

feat: Implement Orientation #2932

merged 60 commits into from
Jun 6, 2024

Conversation

mrousavy
Copy link
Owner

@mrousavy mrousavy commented Jun 3, 2024

What

Implements orientation for VisionCamera.

  1. A new prop is introduced (outputOrientation), which specifies how VisionCamera should treat orientation:
    • device; use physical device orientation, even if screen is locked
    • preview; use preview orientation, respects screen-lock
    • portrait, landscape-left, landscape-right, portrait-upside-down; locked to a specific orientation
  2. The current orientation prop has been removed to avoid confusion. (it never really worked anyways)
  3. Adds documentation for Orientation to explain how it works
  4. Make Frame.orientation actually reflect it's relative orientation.
  5. Stops rotating CMSampleBuffers on iOS. This makes the video and frame processor pipeline much more lightweight and use less energy, as it now no longer physically rotates pixels in a buffer.
    • For Videos, rotation will be passed to the AVAssetWriter (EXIF flag)
    • For Frame Processors, Frame.orientation now represents it's rotation relative to the target rotation (like on Android)
  6. In a previous PR, Skia Frame Processors now properly rotate their transform matrix to support orientation (fix: Fix Skia Frame Processor transformation matrix #2931)

Changes

Tested on

Related issues

Copy link

vercel bot commented Jun 3, 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 Jun 6, 2024 3:15pm

@mrousavy mrousavy merged commit 24e7739 into main Jun 6, 2024
12 of 13 checks passed
@mrousavy mrousavy deleted the feat/orientation-buffers-ios branch June 6, 2024 15:14
@xulihang
Copy link
Contributor

xulihang commented Jun 7, 2024

Stops rotating CMSampleBuffers on iOS. This makes the video and frame processor pipeline much more lightweight and use less energy, as it now no longer physically rotates pixels in a buffer.

This seems like a breaking change for frame processors. For text recognition, the frame processor has to rotate the image to match the preview in portrait mode if it does not accept passing the orientation.

Maybe adding an option to enable rotation is a good idea to make old frame processors compatible with the new version.

@mrousavy
Copy link
Owner Author

mrousavy commented Jun 7, 2024

Hm, I am not sure - I think for portrait nothing changed, but whereas frame processors previously didnt work when the phone is rotated, they now work.

Not entirely sure though, need to test it...

@laxminarayana-capgemini
Copy link

Hi @mrousavy, we trying to record the video in Landscape, but the output video is in Portrait. Whatever the orientation we are recording it is in Portrait only. We are running react-native-vision-camera in Android application, but it is not working. Hereby I have attached a sample code. Kindly help us with a solution.
<Camera style={StyleSheet.absoluteFill} device={device} isActive={isAppForeground} video={true} audio={true} ref={camera} format={format} exposure={0} zoom={0} orientation='landscape-left' />

@pke
Copy link

pke commented Jun 8, 2024

thanks @mrousavy! It seems onCodeScanned of useCodeScanner has not had its frame.bounds fixed with this PR? The bounds should be aligned in React space always at 0,0 being top left of the detected code.

@jkaufman
Copy link
Contributor

jkaufman commented Jun 8, 2024

@laxminarayana-capgemini The name of the orientation prop changed with this PR.

@laxminarayana-capgemini

@mrousavy and @jkaufman. thanks, the issue is resolved.

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.

✨ Implement Orientation ($3,000)
8 participants