[libcamera-devel] [PATCH 09/10] android: camera_device: Use Camera properties for static Metadata

[Jacopo Mondi]

Hi Laurent,

On Wed, Dec 04, 2019 at 06:32:35PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Wed, Dec 04, 2019 at 02:21:05PM +0100, Jacopo Mondi wrote:
> > Construct two example static metadata to be reported to the Android
> > framework using the properties reported by the Camera.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/android/camera_device.cpp | 29 +++++++++++++++++++++++++++--
> >  1 file changed, 27 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 065e0292e186..674867d313ac 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -7,6 +7,9 @@
> >
> >  #include "camera_device.h"
> >
> > +#include <libcamera/controls.h>
> > +#include <libcamera/property_ids.h>
> > +
> >  #include "log.h"
> >  #include "utils.h"
> >
> > @@ -97,6 +100,8 @@ camera_metadata_t *CameraDevice::getStaticMetadata()
> >  	if (staticMetadata_)
> >  		return staticMetadata_->get();
> >
> > +	const ControlList &properties = camera_->properties();
> > +
> >  	/*
> >  	 * The here reported metadata are enough to implement a basic capture
> >  	 * example application, but a real camera implementation will require
> > @@ -261,9 +266,15 @@ camera_metadata_t *CameraDevice::getStaticMetadata()
> >  	staticMetadata_->addEntry(ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE,
> >  				  &exposureTimeRange, 2);
> >
> > +	/*
> > +	 * Android reports orientation as clockwise correction, while Linux
> > +	 * uses counter-clockwise.
>
> What Linux does isn't relevant I think, what matters here is what
> libcamera does.

Libcamera does what Linux does, might that be because the same person
did both ends ? Do we want to introduce a different handling of
rotation in libcamera ?

>
> Android defines the rotation as
>
> "Clockwise angle through which the output image needs to be rotated to
> be upright on the device screen in its native orientation."
>
> I have to say I have a hard time parsing the description of the
> libcamera rotation property:
>
> "Camera mounting rotation expressed as counterclockwise rotation degrees
> towards the axis perpendicular to the sensor surface and directed away
> from it."

That's the mounting rotation description, almost copied from the
dt-bindings property description. Might it be better to make the
libcamera rotation about the correction and not the mounting rotation
? In that case the below [(360 - rotation) % 360] would be performed
by the CameraSensor class and not here..

>
> I can't figure out what this means :-S Is it just me ?

Didn't we have this currently in review in linux-media as a
dt-property ? :)

>
> > +	 */
> >  	int32_t orientation = 0;
> > -	staticMetadata_->addEntry(ANDROID_SENSOR_ORIENTATION,
> > -				  &orientation, 1);
> > +	if (properties.contains(properties::Rotation))
> > +		orientation = (360 - properties.get(properties::Rotation))
>
> This could be made slightly more efficient if you used .find() to avoid
> the double lookup. Same below.

Indeed, I'll fix

Thanks
  j

>
> > +			    % 360;
> > +	staticMetadata_->addEntry(ANDROID_SENSOR_ORIENTATION, &orientation, 1);
> >
> >  	std::vector<int32_t> testPatterModes = {
> >  		ANDROID_SENSOR_TEST_PATTERN_MODE_OFF,
> > @@ -310,6 +321,20 @@ camera_metadata_t *CameraDevice::getStaticMetadata()
> >  				  lensApertures.size());
> >
> >  	uint8_t lensFacing = ANDROID_LENS_FACING_FRONT;
> > +	if (properties.contains(properties::Location)) {
> > +		int32_t location = properties.get(properties::Location);
> > +		switch (location) {
> > +		case CAMERA_LOCATION_FRONT:
> > +			lensFacing = ANDROID_LENS_FACING_FRONT;
> > +			break;
> > +		case CAMERA_LOCATION_BACK:
> > +			lensFacing = ANDROID_LENS_FACING_BACK;
> > +			break;
> > +		case CAMERA_LOCATION_EXTERNAL:
> > +			lensFacing = ANDROID_LENS_FACING_EXTERNAL;
> > +			break;
> > +		}
> > +	}
> >  	staticMetadata_->addEntry(ANDROID_LENS_FACING, &lensFacing, 1);
> >
> >  	std::vector<float> lensFocalLenghts = {
>
> --
> Regards,
>
> Laurent Pinchart
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20191205/66df2178/attachment.sig>