[libcamera-devel] [PATCH 3/4] libcamera: camera_sensor: Initialize PixelArray properties

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Fri, Nov 06, 2020 at 04:49:46PM +0100, Jacopo Mondi wrote:
> Initialize pixel array properties 'PixelArraySize' and
> 'PixelArrayActiveArea' inspecting the V4L2 CROP_BOUNDS and CROP_DEFAULT

s/Area/Areas/
s/inspecting/by inspecting/

> selection targets.
> 
> The properties are registered only if the sensor subdevice support
> the above mentioned selection targets.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/camera_sensor.cpp | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 49b0a026125c..266ed7e9238e 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -288,6 +288,28 @@ int CameraSensor::initProperties()
>  		propertyValue = 0;
>  	properties_.set(properties::Rotation, propertyValue);
>  
> +	Rectangle bounds{};

The default constructor initializes all members to 0, you can drop {}.
Same below.

> +	ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_BOUNDS, &bounds);
> +	if (!ret) {
> +		properties_.set(properties::PixelArraySize,
> +				Size(bounds.width, bounds.height));

You could use bounds.size().

> +	}

No need for curly braces.

> +
> +	Rectangle crop{};
> +	ret = subdev_->getSelection(pad_, V4L2_SEL_TGT_CROP_DEFAULT, &crop);
> +	if (!ret) {
> +		/*
> +		 * V4L2_SEL_TGT_CROP_DEFAULT and V4L2_SEL_TGT_CROP_BOUNDS are
> +		 * defined relatively to the sensor native pixel array size,
> +		 * while properties::PixelArrayActiveAreas is defined relatively
> +		 * to properties::PixelArraySize. Adjust them

s/them/them./

Or actually should it be "it", or "the former" ? You only adjust
PixelArrayActiveAreas.

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

> +		 */
> +		crop.x -= bounds.x;
> +		crop.y -= bounds.y;
> +		properties_.set(properties::PixelArrayActiveAreas,
> +				{ crop });
> +	}
> +
>  	return 0;
>  }
>  

-- 
Regards,

Laurent Pinchart