[libcamera-devel] [PATCH 6/6] android: camera_device: Fix preview template

[Jacopo Mondi]

Hi Laurent,

On Fri, Jul 24, 2020 at 07:07:15PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Fri, Jul 24, 2020 at 04:21:20PM +0200, Jacopo Mondi wrote:
> > Add 5 controls to the generate preview template to comply with the
> > camera3 specification.
> >
> > This change fixes CTS 9.0.r12 test [14/253]
> > android.hardware.camera2.cts.CameraDeviceTest#testCameraDevicePreviewTemplate
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/android/camera_device.cpp | 32 +++++++++++++++++++++++++++++---
> >  1 file changed, 29 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index b8cb118a960e..58901a119c18 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -377,7 +377,7 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()
> >  	 * Currently: 50 entries, 647 bytes of static metadata
> >  	 */
> >  	uint32_t numEntries = 50;
> > -	uint32_t byteSize = 647;
> > +	uint32_t byteSize = 667;
> >
> >  	/*
> >  	 * Calculate space occupation in bytes for dynamically built metadata
> > @@ -779,6 +779,8 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >  		ANDROID_CONTROL_AE_MODE,
> >  		ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION,
> >  		ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER,
> > +		ANDROID_CONTROL_AE_TARGET_FPS_RANGE,
> > +		ANDROID_CONTROL_AE_ANTIBANDING_MODE,
> >  		ANDROID_CONTROL_AE_LOCK,
> >  		ANDROID_CONTROL_AF_TRIGGER,
> >  		ANDROID_CONTROL_AWB_MODE,
> > @@ -787,6 +789,9 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
> >  		ANDROID_STATISTICS_FACE_DETECT_MODE,
> >  		ANDROID_NOISE_REDUCTION_MODE,
> >  		ANDROID_COLOR_CORRECTION_ABERRATION_MODE,
> > +		ANDROID_LENS_APERTURE,
> > +		ANDROID_LENS_OPTICAL_STABILIZATION_MODE,
> > +		ANDROID_CONTROL_MODE,
> >  		ANDROID_CONTROL_CAPTURE_INTENT,
> >  	};
> >  	staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_REQUEST_KEYS,
> > @@ -825,9 +830,9 @@ const CameraMetadata *CameraDevice::captureTemplatePreview()
> >  {
> >  	/*
> >  	 * \todo Keep this in sync with the actual number of entries.
> > -	 * Currently: 12 entries, 15 bytes
> > +	 * Currently: 20 entries, 35 bytes
> >  	 */
> > -	CameraMetadata *requestTemplate = new CameraMetadata(15, 20);
> > +	CameraMetadata *requestTemplate = new CameraMetadata(20, 35);
>
> It's lovely how (15, 20) didn't match the comment :-) I'll be happy when
> we'll get a better API.
>

It's a real pain, yes

> >  	if (!requestTemplate->isValid())
> >  		return nullptr;
> >
> > @@ -847,6 +852,17 @@ const CameraMetadata *CameraDevice::captureTemplatePreview()
> >  	requestTemplate->addEntry(ANDROID_CONTROL_AE_LOCK,
> >  				  &aeLock, 1);
> >
> > +	std::vector<int32_t> aeFpsTarget = {
> > +		15, 30,
> > +	};
> > +	requestTemplate->addEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE,
> > +				  aeFpsTarget.data(),
> > +				  aeFpsTarget.size());
> > +
> > +	uint8_t aeAntibandingMode = ANDROID_CONTROL_AE_ANTIBANDING_MODE_AUTO;
>
> Should we report OFF instead, as we don't support antibanding at the
> moment ?

CTS complains if I report OFF.

I'm not sure I got why, as we report OFF in the available anti-banding
modes reported with ANDROID_CONTROL_AE_AVAILABLE_ANTIBANDING_MODES.

I think I'll stick with AUTO for the moment.

>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> > +	requestTemplate->addEntry(ANDROID_CONTROL_AE_ANTIBANDING_MODE,
> > +				  &aeAntibandingMode, 1);
> > +
> >  	uint8_t afTrigger = ANDROID_CONTROL_AF_TRIGGER_IDLE;
> >  	requestTemplate->addEntry(ANDROID_CONTROL_AF_TRIGGER,
> >  				  &afTrigger, 1);
> > @@ -875,6 +891,16 @@ const CameraMetadata *CameraDevice::captureTemplatePreview()
> >  	requestTemplate->addEntry(ANDROID_COLOR_CORRECTION_ABERRATION_MODE,
> >  				  &aberrationMode, 1);
> >
> > +	uint8_t controlMode = ANDROID_CONTROL_MODE_AUTO;
> > +	requestTemplate->addEntry(ANDROID_CONTROL_MODE, &controlMode, 1);
> > +
> > +	float lensAperture = 2.53 / 100;
> > +	requestTemplate->addEntry(ANDROID_LENS_APERTURE, &lensAperture, 1);
> > +
> > +	uint8_t opticalStabilization = ANDROID_LENS_OPTICAL_STABILIZATION_MODE_OFF;
> > +	requestTemplate->addEntry(ANDROID_LENS_OPTICAL_STABILIZATION_MODE,
> > +				  &opticalStabilization, 1);
> > +
> >  	uint8_t captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_PREVIEW;
> >  	requestTemplate->addEntry(ANDROID_CONTROL_CAPTURE_INTENT,
> >  				  &captureIntent, 1);
>
> --
> Regards,
>
> Laurent Pinchart