[libcamera-devel] [PATCH v2 11/13] android: camera_device: Handle NOISE_REDUCTION_MODES

[Kieran Bingham]

Hi Jacopo,

On 20/10/2020 19:05, Jacopo Mondi wrote:
> Register the ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES
> static metadata property inspecting the values retuned by the pipeline
> handler.
> 
> Reserve in the static metadata pack enough space to support all the 5
> available noise reduction modes Android defines.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/android/camera_device.cpp | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 8166b09bb69a..a4d9e6ddc519 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -553,7 +553,7 @@ std::tuple<uint32_t, uint32_t> CameraDevice::calculateStaticMetadataSize()
>  	 * Currently: 51 entries, 687 bytes of static metadata
>  	 */
>  	uint32_t numEntries = 51;
> -	uint32_t byteSize = 687;
> +	uint32_t byteSize = 691;
>  
>  	/*
>  	 * Calculate space occupation in bytes for dynamically built metadata
> @@ -830,9 +830,18 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  				  &minFocusDistance, 1);
>  
>  	/* Noise reduction modes. */
> -	uint8_t noiseReductionModes = ANDROID_NOISE_REDUCTION_MODE_OFF;
> -	staticMetadata_->addEntry(ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
> -				  &noiseReductionModes, 1);
> +	{
> +		std::vector<uint8_t> data(5);
> +		const auto &infoMap = controlsInfo.find(&controls::draft::NoiseReductionMode);
> +		if (infoMap != controlsInfo.end()) {
> +			for (const auto &value : infoMap->second.values())
> +				data.push_back(value.get<int32_t>());

I'm a bit concerned by this, as it has the hidden assumption that the
control values are the same value as the ANDROID enum values.

While they're in draft that's true, but it might not be when we remove
::draft:: ... I suspect the best option here is to add a \todo: or
something to highlight that fact so it's not a hidden assumption.

Anyway, I won't block on that, as this is clearly still in ::draft::.

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> +		} else {
> +			data.push_back(ANDROID_NOISE_REDUCTION_MODE_OFF);
> +		}
> +		staticMetadata_->addEntry(ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
> +					  data.data(), data.size());
> +	}
>  
>  	/* Scaler static metadata. */
>  	float maxDigitalZoom = 1;
> 

-- 
Regards
--
Kieran