[libcamera-devel] [RFC PATCH 06/14] android: Plumb AeEnable control

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Fri, Jun 18, 2021 at 07:33:43PM +0900, Paul Elder wrote:
> Plumb the AeEnable control into the HAL for CONTROL_AE_AVAILABLE_MODES
> for static metadata, and CONTROL_AE_MODE for result metadata.
> 
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=42
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>  src/android/camera_device.cpp | 28 +++++++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 86f47e00..3446cdb6 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -10,6 +10,7 @@
>  #include "camera_ops.h"
>  #include "post_processor.h"
>  
> +#include <algorithm>
>  #include <array>
>  #include <cmath>
>  #include <fstream>
> @@ -877,9 +878,24 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  	staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_ANTIBANDING_MODES,
>  				  aeAvailableAntiBandingModes);
>  
> -	std::vector<uint8_t> aeAvailableModes = {
> -		ANDROID_CONTROL_AE_MODE_ON,
> -	};
> +	std::vector<uint8_t> aeAvailableModes;
> +	aeAvailableModes.reserve(2);
> +	const auto &aeModesInfo = controlsInfo.find(&controls::AeEnable);
> +	if (aeModesInfo != controlsInfo.end()) {
> +		aeAvailableModes.push_back(aeModesInfo->second.min().get<bool>());
> +		if (aeModesInfo->second.min() != aeModesInfo->second.max())
> +			aeAvailableModes.push_back(aeModesInfo->second.max().get<bool>());
> +	} else {
> +		aeAvailableModes.push_back(ANDROID_CONTROL_AE_MODE_ON);
> +	}
> +
> +	if (std::find(aeAvailableModes.begin(),
> +		      aeAvailableModes.end(),
> +		      ANDROID_CONTROL_AE_MODE_OFF) == aeAvailableModes.end()) {
> +		fullSupport = false;
> +		/* \todo Double check if this is the correct capability */

Seems fine to me.

> +		capabilities[ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR] = false;
> +	}
>  	staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_MODES,
>  				  aeAvailableModes);
>  
> @@ -2333,8 +2349,10 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
>  	value = ANDROID_CONTROL_AE_LOCK_OFF;
>  	resultMetadata->addEntry(ANDROID_CONTROL_AE_LOCK, value);
>  
> -	value = ANDROID_CONTROL_AE_MODE_ON;
> -	resultMetadata->addEntry(ANDROID_CONTROL_AE_MODE, value);
> +	if (metadata.contains(controls::AeEnable)) {
> +		uint8_t aeMode = metadata.get<bool>(controls::AeEnable);

I'd make this more explicit, with

		uint8_t aeMode = metadata.get<bool>(controls::AeEnable)
			       ? ANDROID_CONTROL_AE_MODE_ON
			       : ANDROID_CONTROL_AE_MODE_OFF;

It would also be nice for the code above to do something similar.

> +		resultMetadata->addEntry(ANDROID_CONTROL_AE_MODE, aeMode);
> +	}
>  
>  	if (settings.getEntry(ANDROID_CONTROL_AE_TARGET_FPS_RANGE, &entry))
>  		/*

-- 
Regards,

Laurent Pinchart