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

(1/2) Properly support camera orientation (V.2) #2

Conversation

@peat-psuwit
Copy link
Contributor

commented Jun 18, 2019

This PR make the plugin handles the orientation of the device within the plugin itself, removing the need for applications to deal with it, and complying with what other plugins do. This PR also correct the reported camera orientation by inverting the value given from Android as it has a different meaning.

This partially fixes ubports/ubuntu-touch#370 and superseded #1.

Invert camera orientation given from Android HAL
Android's orientation means differently compared to QT's orientation.
On Android, it means "the angle that the camera image needs to be
rotated", but on QT, it means "the physical orientation of the camera
sensor". So, the value will have to be inverted.
@peat-psuwit

This comment has been minimized.

Copy link
Contributor Author

commented Jun 24, 2019

A test from @cibersheep indicates that orientation override doesn't work. Will need some time to fix this.

@peat-psuwit peat-psuwit changed the title (1/2) Properly support camera orientation (V.2) WIP: (1/2) Properly support camera orientation (V.2) Jun 24, 2019

peat-psuwit added some commits Jun 15, 2019

Add a camera orientation override for some devices
Krillin (BQ Aquaris E4.5) & vegetahd (BQ Aquaris E5) give the
incorrect orientation to us. So, I decided to override it here.
Add RotationHandler class
This class will handle device's orientation changes and calculate
required rotation for captured image/video.

Note that this implementation uses QOrientationSensor. I guess it's
usually not desirable for plugins to uses another QT feature. However,
in this case it's the easiest way to determine orientation of the
device.
Make image & video capture use RotationHandler
Previously, they uses orientation information given from application.
As the application isn't supposed to set this information (see
QTMultimedia's declarative-camera example) and other plugins from
QT itself handles rotation inside plugin, I decided to do this in
plugin too.
Remove orienataion code from MetadataWriterControl
Now that orientation is handled in code, MetadataWriterControl doesn't
need to care about orientation anymore.

@peat-psuwit peat-psuwit force-pushed the peat-psuwit:xenial_-_proper-camera-orientaion-support-v2 branch from 91f9e02 to d41d90e Jun 24, 2019

@peat-psuwit peat-psuwit changed the title WIP: (1/2) Properly support camera orientation (V.2) (1/2) Properly support camera orientation (V.2) Jun 24, 2019

@peat-psuwit

This comment has been minimized.

Copy link
Contributor Author

commented Jun 24, 2019

The problem is fixed. PR updated.

@UniversalSuperBox
Copy link
Member

left a comment

This tests fine on the FP2 with old camera module, Oneplus One, Nexus 5, and Nexus 7. We've also seen tests from the FP2 with new camera module and this actually fixes the problem. I think we can get this into devel (Assuming https://gitlab.com/ubports/apps/camera-app/merge_requests/122 is merged in tandem)

@Flohack74

This comment has been minimized.

Copy link
Member

commented Jun 26, 2019

But can we merge the change to camera-app which will update on all stable channel devices as well? Wont it affect that, and we need to wait with this app update after the OTA?

@peat-psuwit

This comment has been minimized.

Copy link
Contributor Author

commented Jun 26, 2019

@UniversalSuperBox suggested me to bump the framework version. So, it won't be installable on stable channel device. Not sure how it works in OpenStore though. Also, maybe the preinstalled click should be upgraded as well?

@Flohack74

This comment has been minimized.

Copy link
Member

commented Jun 26, 2019

I see. I will talk with him.

@cibersheep

This comment has been minimized.

Copy link

commented Jun 26, 2019

If you install the camera app without the updated framwork, Camera app desappears from the App Scope :)

@UniversalSuperBox

This comment has been minimized.

Copy link
Member

commented Jun 26, 2019

The OpenStore won't offer an incompatible app upgrade. The old camera app works with the new qtubuntu-camera.

@Flohack74 Flohack74 merged commit 21d3dfe into ubports:xenial Jun 26, 2019

2 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details

@peat-psuwit peat-psuwit deleted the peat-psuwit:xenial_-_proper-camera-orientaion-support-v2 branch Jun 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.