[libcamera-devel] [PATCH v2] android: camera_device: Fix value of orientation metadata

[Jacopo Mondi]

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 at uajain.com>
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart at 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 at jmondi.org>
> >
> > > +	}
> > >
> > >  	int ret = camera_->acquire();
> > >  	if (ret) {
>
> --
> Regards,
>
> Laurent Pinchart