[libcamera-devel] [PATCH v3 3/4] android: camera_device: Report sensor physical size

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2020-12-28 17:52:02 +0100, Jacopo Mondi wrote:
> Calculate the value of the ANDROID_SENSOR_INFO_PHYSICAL_SIZE property
> multiplying the pixel unit cell size with the number of sensor's readable
> pixels.
> 
> Maintain a default value to support pipelines that do not register
> the UnitCellSize property.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/android/camera_device.cpp | 38 ++++++++++++++++++++---------------
>  1 file changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 7678d4485ce9..9ad417ee6c3a 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -851,24 +851,37 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  	staticMetadata_->addEntry(ANDROID_JPEG_MAX_SIZE, &maxJpegBufferSize_, 1);
>  
>  	/* Sensor static metadata. */
> +	std::vector<int32_t> pixelArraySize(2);
>  	if (properties.contains(properties::PixelArraySize)) {
>  		const Size &size =
>  			properties.get(properties::PixelArraySize);
> -		std::vector<int32_t> data{
> -			static_cast<int32_t>(size.width),
> -			static_cast<int32_t>(size.height),
> -		};
> -		staticMetadata_->addEntry(ANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE,
> -					  data.data(), data.size());
> +		pixelArraySize[0] = static_cast<int32_t>(size.width);
> +		pixelArraySize[1] = static_cast<int32_t>(size.height);
>  	} else {
>  		/*
>  		 * \todo Drop the default once the ov5670 and ov13858 drivers
>  		 * are updated to report the pixel array size.
>  		 */
> -		int32_t data[] = { 2592, 1944 };
> -		staticMetadata_->addEntry(ANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE,
> -					  data, 2);
> +		pixelArraySize = { 2592, 1944 };
>  	}
> +	staticMetadata_->addEntry(ANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE,
> +				  pixelArraySize.data(), 2);
> +
> +	std::vector<float> physicalSize(2);
> +	if (properties.contains(properties::UnitCellSize)) {
> +		const Size &unitCellSize =
> +			properties.get<Size>(properties::UnitCellSize);
> +		physicalSize[0] = unitCellSize.width * pixelArraySize[0] / 1e6f;
> +		physicalSize[1] = unitCellSize.height * pixelArraySize[1] / 1e6f;
> +	} else {
> +		/*
> +		 * \todo Drop the default once all camera sensors report
> +		 * the pixel unit size.
> +		 */
> +		physicalSize = { 2.592, 1.944 };
> +	}

Instead of adding special cases for cameras that does not report all 
controls that the HAL needs would it make sens to add default values in 
the CameraSensor/SensorDatabase classes? I'm thinking this could create 
a leaner HAL implementation as the HAL is already rather complex and we 
would collect all defaults values in components that are designed to 
provided them. As an added bonus the controls would also be visible 
outside the HAL and possibly increasing the usage of them in other parts 
of the code?

> +	staticMetadata_->addEntry(ANDROID_SENSOR_INFO_PHYSICAL_SIZE,
> +				  physicalSize.data(), physicalSize.size());
>  
>  	if (properties.contains(properties::PixelArrayActiveAreas)) {
>  		const Span<const Rectangle> &rects =
> @@ -919,13 +932,6 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  				  testPatterModes.data(),
>  				  testPatterModes.size());
>  
> -	std::vector<float> physicalSize = {
> -		2592, 1944,
> -	};
> -	staticMetadata_->addEntry(ANDROID_SENSOR_INFO_PHYSICAL_SIZE,
> -				  physicalSize.data(),
> -				  physicalSize.size());
> -
>  	uint8_t timestampSource = ANDROID_SENSOR_INFO_TIMESTAMP_SOURCE_UNKNOWN;
>  	staticMetadata_->addEntry(ANDROID_SENSOR_INFO_TIMESTAMP_SOURCE,
>  				  &timestampSource, 1);
> -- 
> 2.29.2
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund