[libcamera-devel,v2] android: camera_device: Fix value of orientation metadata
diff mbox series

Message ID 20200909104754.25940-1-email@uajain.com
State Accepted
Commit e917655d06ec86224b10b992e036460a7334b407
Headers show
Series
  • [libcamera-devel,v2] android: camera_device: Fix value of orientation metadata
Related show

Commit Message

Umang Jain Sept. 9, 2020, 10:47 a.m. UTC 
Android's orientation metadata cannot have identical numerical
value to libcamera's rotation property. This is due to the fact
that libcamera's rotation property specify the correction angle
in anticlockwise direction whereas Android's orientation metadata
specifies the value in clockwise direction. Fix that by computing
corresponding value for clockwise direction from libcamera's rotation
property.

Signed-off-by: Umang Jain <email@uajain.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/android/camera_device.cpp | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

Comments

Jacopo Mondi Sept. 9, 2020, 11:01 a.m. UTC | #1 
Hi Umang,
 sorry I got late on v1

On Wed, Sep 09, 2020 at 04:17:54PM +0530, Umang Jain wrote:
> Android's orientation metadata cannot have identical numerical
> value to libcamera's rotation property. This is due to the fact
> that libcamera's rotation property specify the correction angle

specifies

> in anticlockwise direction whereas Android's orientation metadata
> specifies the value in clockwise direction. Fix that by computing
> corresponding value for clockwise direction from libcamera's rotation
> property.
>

We used 'counterclockwise', but I think it's the same

> Signed-off-by: Umang Jain <email@uajain.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/android/camera_device.cpp | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 2582991..8be846b 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -265,12 +265,17 @@ int CameraDevice::initialize()
>  	}
>
>  	/*
> -	 * The Android orientation metadata and libcamera rotation property are
> -	 * defined differently but have identical numerical values for Android
> -	 * devices such as phones and tablets.
> +	 * The Android orientation metadata specifies its rotation correction
> +	 * value in clockwise direction whereas libcamera specifies the
> +	 * rotation property in anticlockwise direction. Read the libcamera's
> +	 * rotation property (anticlockwise) and compute the corresponding
> +	 * value for clockwise direction as required by the Android orientation
> +	 * metadata.
>  	 */
> -	if (properties.contains(properties::Rotation))
> -		orientation_ = properties.get(properties::Rotation);
> +	if (properties.contains(properties::Rotation)) {
> +		int rotation = properties.get(properties::Rotation);
> +		orientation_ = (360 - rotation) % 360;

The % 360 'should' not be necessary, but I guess it doesn't hurt.

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> +	}
>
>  	int ret = camera_->acquire();
>  	if (ret) {
> --
> 2.26.2
>
Laurent Pinchart Sept. 9, 2020, 11:10 a.m. UTC | #2 
On Wed, Sep 09, 2020 at 01:01:33PM +0200, Jacopo Mondi wrote:
> Hi Umang,
>  sorry I got late on v1
> 
> On Wed, Sep 09, 2020 at 04:17:54PM +0530, Umang Jain wrote:
> > Android's orientation metadata cannot have identical numerical
> > value to libcamera's rotation property. This is due to the fact
> > that libcamera's rotation property specify the correction angle
> 
> specifies
> 
> > in anticlockwise direction whereas Android's orientation metadata
> > specifies the value in clockwise direction. Fix that by computing
> > corresponding value for clockwise direction from libcamera's rotation
> > property.
> 
> We used 'counterclockwise', but I think it's the same

Seems anticlockwise is UK English and counterclockwise US English :-)

> > Signed-off-by: Umang Jain <email@uajain.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/android/camera_device.cpp | 15 ++++++++++-----
> >  1 file changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 2582991..8be846b 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -265,12 +265,17 @@ int CameraDevice::initialize()
> >  	}
> >
> >  	/*
> > -	 * The Android orientation metadata and libcamera rotation property are
> > -	 * defined differently but have identical numerical values for Android
> > -	 * devices such as phones and tablets.
> > +	 * The Android orientation metadata specifies its rotation correction
> > +	 * value in clockwise direction whereas libcamera specifies the
> > +	 * rotation property in anticlockwise direction. Read the libcamera's
> > +	 * rotation property (anticlockwise) and compute the corresponding
> > +	 * value for clockwise direction as required by the Android orientation
> > +	 * metadata.
> >  	 */
> > -	if (properties.contains(properties::Rotation))
> > -		orientation_ = properties.get(properties::Rotation);
> > +	if (properties.contains(properties::Rotation)) {
> > +		int rotation = properties.get(properties::Rotation);
> > +		orientation_ = (360 - rotation) % 360;
> 
> The % 360 'should' not be necessary, but I guess it doesn't hurt.

Without % 360 a 0° rotation will be translated to 360°, not 0°.

> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> > +	}
> >
> >  	int ret = camera_->acquire();
> >  	if (ret) {
Jacopo Mondi Sept. 9, 2020, 12:41 p.m. UTC | #3 
Hi Laurent,

On Wed, Sep 09, 2020 at 02:10:38PM +0300, Laurent Pinchart wrote:
> On Wed, Sep 09, 2020 at 01:01:33PM +0200, Jacopo Mondi wrote:
> > Hi Umang,
> >  sorry I got late on v1
> >
> > On Wed, Sep 09, 2020 at 04:17:54PM +0530, Umang Jain wrote:
> > > Android's orientation metadata cannot have identical numerical
> > > value to libcamera's rotation property. This is due to the fact
> > > that libcamera's rotation property specify the correction angle
> >
> > specifies
> >
> > > in anticlockwise direction whereas Android's orientation metadata
> > > specifies the value in clockwise direction. Fix that by computing
> > > corresponding value for clockwise direction from libcamera's rotation
> > > property.
> >
> > We used 'counterclockwise', but I think it's the same
>
> Seems anticlockwise is UK English and counterclockwise US English :-)
>
> > > Signed-off-by: Umang Jain <email@uajain.com>
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > >  src/android/camera_device.cpp | 15 ++++++++++-----
> > >  1 file changed, 10 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > > index 2582991..8be846b 100644
> > > --- a/src/android/camera_device.cpp
> > > +++ b/src/android/camera_device.cpp
> > > @@ -265,12 +265,17 @@ int CameraDevice::initialize()
> > >  	}
> > >
> > >  	/*
> > > -	 * The Android orientation metadata and libcamera rotation property are
> > > -	 * defined differently but have identical numerical values for Android
> > > -	 * devices such as phones and tablets.
> > > +	 * The Android orientation metadata specifies its rotation correction
> > > +	 * value in clockwise direction whereas libcamera specifies the
> > > +	 * rotation property in anticlockwise direction. Read the libcamera's
> > > +	 * rotation property (anticlockwise) and compute the corresponding
> > > +	 * value for clockwise direction as required by the Android orientation
> > > +	 * metadata.
> > >  	 */
> > > -	if (properties.contains(properties::Rotation))
> > > -		orientation_ = properties.get(properties::Rotation);
> > > +	if (properties.contains(properties::Rotation)) {
> > > +		int rotation = properties.get(properties::Rotation);
> > > +		orientation_ = (360 - rotation) % 360;
> >
> > The % 360 'should' not be necessary, but I guess it doesn't hurt.
>
> Without % 360 a 0° rotation will be translated to 360°, not 0°.
>

Ah ups. I was thinking about the fact we refuse 360 rotations in
kernel

from v4l2_fwnode_device_parse():

	props->rotation = V4L2_FWNODE_PROPERTY_UNSET;
	ret = fwnode_property_read_u32(fwnode, "rotation", &val);
	if (!ret) {
		if (val >= 360) {
			dev_warn(dev, "Unsupported device rotation: %u\n", val);
			return -EINVAL;
		}

		props->rotation = val;
		dev_dbg(dev, "device rotation: %u\n", val);
	}

but yes, 360 - 0 = 360 :)


> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> >
> > > +	}
> > >
> > >  	int ret = camera_->acquire();
> > >  	if (ret) {
>
> --
> Regards,
>
> Laurent Pinchart

Patch
diff mbox series 

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 2582991..8be846b 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -265,12 +265,17 @@  int CameraDevice::initialize()
 	}
 
 	/*
-	 * The Android orientation metadata and libcamera rotation property are
-	 * defined differently but have identical numerical values for Android
-	 * devices such as phones and tablets.
+	 * The Android orientation metadata specifies its rotation correction
+	 * value in clockwise direction whereas libcamera specifies the
+	 * rotation property in anticlockwise direction. Read the libcamera's
+	 * rotation property (anticlockwise) and compute the corresponding
+	 * value for clockwise direction as required by the Android orientation
+	 * metadata.
 	 */
-	if (properties.contains(properties::Rotation))
-		orientation_ = properties.get(properties::Rotation);
+	if (properties.contains(properties::Rotation)) {
+		int rotation = properties.get(properties::Rotation);
+		orientation_ = (360 - rotation) % 360;
+	}
 
 	int ret = camera_->acquire();
 	if (ret) {