[libcamera-devel] [PATCH v4 2/5] libcamera: Initialise the ScalerCropMaximum property

[Laurent Pinchart]

Hi David,

Thank you for the patch.

On Mon, Oct 19, 2020 at 01:51:53PM +0100, David Plowman wrote:
> Add a default initialisation according to the sensor resolution,
> though it will need updating when the camera mode changes.
> 
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  src/libcamera/camera_sensor.cpp | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 78c7ceec..ae25c5c6 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -279,6 +279,12 @@ int CameraSensor::init()
>  	 */
>  	resolution_ = sizes_.back();
>  
> +	/*
> +	 * Set a default value for the ScalerCropMaximum, though it will have to
> +	 * be updated when new camera modes are chosen.
> +	 */
> +	properties_.set(properties::ScalerCropMaximum, Rectangle{ 0, 0, resolution_ });

I wonder if we actually need this. The properties of the CameraSensor
class itself are not updated by the pipeline handler, which updates the
properties stored in CameraData instead, so this only serves as a way to
initialize the ScalerCropMaximum property. It's not even required in
order to be able to update the property at configure time, as calling

	data->properties_.set(properties::ScalerCropMaximum, ...);

works without having to set it here.

Furthermore, wouldn't it make sense to only report the property when the
pipeline handler supports digital zoom ? If so, I think it could be best
to initialize ScalerCropMaximum in the pipeline handler instead. This
could be done in PipelineHandlerRPi::match(), right after

	data->properties_ = data->sensor_->properties();

> +
>  	return 0;
>  }
>  

-- 
Regards,

Laurent Pinchart