[libcamera-devel] [PATCH 5/5] libcamera: camera_sensor: Retrieve sensor sizes

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Sat, Aug 17, 2019 at 12:59:37PM +0200, Jacopo Mondi wrote:
> Retrieve the camera sensor pixel array sizes and the active pixel sizes
> using the V4L2 selection APIs.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/camera_sensor.cpp       | 13 +++++++++++++
>  src/libcamera/include/camera_sensor.h |  2 ++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 2703d10c719e..e6b01c242328 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -114,6 +114,19 @@ int CameraSensor::init()
>  		return -EINVAL;
>  	}
>  
> +	/* Retrieve and store the sensor pixel array and active area sizes. */

Nit-picking, the active area is a rectangle, not a size unlike the pixel
array.

> +	ret = subdev_->getCropBounds(0, &activeAreaSize_);
> +	if (ret)
> +		return ret;
> +
> +	Rectangle rect = {};

I think you can skip the initialisation of rect.

> +	ret = subdev_->getNativeSize(0, &rect);
> +	if (ret)
> +		return ret;
> +
> +	pixelArraySize_.width = rect.w;
> +	pixelArraySize_.height = rect.h;
> +
>  	/* Enumerate and cache media bus codes and sizes. */
>  	const ImageFormats formats = subdev_->formats(0);
>  	if (formats.isEmpty()) {
> diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h
> index a237a1684605..f6b184bf2838 100644
> --- a/src/libcamera/include/camera_sensor.h
> +++ b/src/libcamera/include/camera_sensor.h
> @@ -58,6 +58,8 @@ private:
>  	std::vector<Size> sizes_;
>  
>  	CameraLocation location_;
> +	Size pixelArraySize_;
> +	Rectangle activeAreaSize_;

Should you provide get accessors for these (with documentation :-)) ?

Do we also need to expose the sensor physical size ?

>  };
>  
>  } /* namespace libcamera */

-- 
Regards,

Laurent Pinchart