[libcamera-devel] [PATCH v3 2/5] ipa: ipu3: af: Use geometry classes to perform grid centering

[Jean-Michel Hautbois]

Hi Kieran,

Thanks for the patch !

On 25/03/2022 10:25, Kieran Bingham wrote:
> Use our geometry classes for Rectangle, Size and Point to identify
> the region of interest for the autofocus, and center it on the BDS output.
> 
> This will facilitate custom ROI being passed in through controls at a later
> time.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>

> ---
>   src/ipa/ipu3/algorithms/af.cpp | 23 +++++++++++++++--------
>   1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp
> index 634d0f2e3176..4476857fa772 100644
> --- a/src/ipa/ipu3/algorithms/af.cpp
> +++ b/src/ipa/ipu3/algorithms/af.cpp
> @@ -141,16 +141,23 @@ int Af::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>   	grid.block_height_log2 = kAfMinGridBlockHeight;
>   	grid.height_per_slice = kAfDefaultHeightPerSlice;
>   
> -	/* x_start and y start are default to BDS center */
> -	grid.x_start = (configInfo.bdsOutputSize.width / 2) -
> -		       (((grid.width << grid.block_width_log2) / 2));
> -	grid.y_start = (configInfo.bdsOutputSize.height / 2) -
> -		       (((grid.height << grid.block_height_log2) / 2));
> +	/* Position the AF grid in the center of the BDS output. */
> +	Rectangle bds(configInfo.bdsOutputSize);
> +	Size gridSize(grid.width << grid.block_width_log2,
> +		      grid.height << grid.block_height_log2);
> +
> +	/*
> +	 * \todo - Support request metadata
> +	 * - Set the ROI based on any input controls in the request
> +	 * - Return the AF ROI as metadata in the Request
> +	 */
> +	Rectangle roi = gridSize.centeredTo(bds.center());
> +	Point start = roi.topLeft();
>   
>   	/* x_start and y_start should be even */
> -	grid.x_start = (grid.x_start / 2) * 2;
> -	grid.y_start = (grid.y_start / 2) * 2;
> -	grid.y_start = grid.y_start | IPU3_UAPI_GRID_Y_START_EN;
> +	grid.x_start = utils::alignDown(start.x, 2);
> +	grid.y_start = utils::alignDown(start.y, 2);
> +	grid.y_start |= IPU3_UAPI_GRID_Y_START_EN;
>   
>   	/* Initial max focus step */
>   	maxStep_ = kMaxFocusSteps;