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

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Tue, Mar 15, 2022 at 05:53:44PM +0000, Kieran Bingham via libcamera-devel 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.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  src/ipa/ipu3/algorithms/af.cpp | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp
> index 3cedf8cbd687..469762a29937 100644
> --- a/src/ipa/ipu3/algorithms/af.cpp
> +++ b/src/ipa/ipu3/algorithms/af.cpp
> @@ -141,16 +141,24 @@ 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
> +	 * - Provide an alignedTo on a point, or Rectangle x,y position
> +	 */
> +	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 = (start.x / 2) * 2;
> +	grid.y_start = (start.y / 2) * 2;

You could use

	grid.x_start = utils::alignDown(start.x, 2);
	grid.y_start = utils::alignDown(start.y, 2);

or possibly even

	grid.x_start = start.x & ~1;
	grid.y_start = start.y & ~1;

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

> +	grid.y_start |= IPU3_UAPI_GRID_Y_START_EN;
>  
>  	/* Initial max focus step */
>  	maxStep_ = kMaxFocusSteps;

-- 
Regards,

Laurent Pinchart