[libcamera-devel] [PATCH v3 6/6] android: camera_device: Do not default pixel array properties

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wed, Dec 30, 2020 at 07:01:20PM +0100, Jacopo Mondi wrote:
> Now that the pixel array properties have been defaulted in the
> CameraSensor class, they will always be reported by the
> libcamera::Camera and there's no need to default them in the
> Camera HAL.
> 
> Remove defaults and assume properties are always there.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/android/camera_device.cpp | 20 ++------------------
>  1 file changed, 2 insertions(+), 18 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 7678d4485ce9..9912875ea046 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -851,7 +851,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  	staticMetadata_->addEntry(ANDROID_JPEG_MAX_SIZE, &maxJpegBufferSize_, 1);
>  
>  	/* Sensor static metadata. */
> -	if (properties.contains(properties::PixelArraySize)) {
> +	{
>  		const Size &size =
>  			properties.get(properties::PixelArraySize);
>  		std::vector<int32_t> data{
> @@ -860,17 +860,9 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  		};
>  		staticMetadata_->addEntry(ANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE,
>  					  data.data(), data.size());
> -	} else {
> -		/*
> -		 * \todo Drop the default once the ov5670 and ov13858 drivers
> -		 * are updated to report the pixel array size.
> -		 */

Good, one less \todo :-) However, that reminds me that you should add
one in the patch that makes validation of sensor properties a soft
failure.

> -		int32_t data[] = { 2592, 1944 };
> -		staticMetadata_->addEntry(ANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE,
> -					  data, 2);
>  	}
>  
> -	if (properties.contains(properties::PixelArrayActiveAreas)) {
> +	{
>  		const Span<const Rectangle> &rects =
>  			properties.get(properties::PixelArrayActiveAreas);
>  		std::vector<int32_t> data{
> @@ -881,14 +873,6 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  		};
>  		staticMetadata_->addEntry(ANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE,
>  					  data.data(), data.size());
> -	} else {
> -		/*
> -		 * \todo Drop the default once the ov5670 and ov13858 drivers
> -		 * are updated to report the pixel array size.
> -		 */
> -		int32_t data[] = { 0, 0, 2560, 1920 };
> -		staticMetadata_->addEntry(ANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE,
> -					  data, 4);
>  	}

We need to update the UVC pipeline handler first. With a patch to do so
before this patch,

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

>  
>  	int32_t sensitivityRange[] = {

-- 
Regards,

Laurent Pinchart