[libcamera-devel] [PATCH v2 4/5] ipa: ipu3: af: Use Span for y_table_item_t

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Wed, Mar 23, 2022 at 01:56:13PM +0000, Kieran Bingham via libcamera-devel wrote:
> Convert the y_table_item_t to a Span and use that for iteration when
> estimating variance of the table.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  src/ipa/ipu3/algorithms/af.cpp | 34 +++++++++++++++++-----------------
>  src/ipa/ipu3/algorithms/af.h   |  4 ++--
>  2 files changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp
> index 68e100fdc504..ff5e9fb5b3c5 100644
> --- a/src/ipa/ipu3/algorithms/af.cpp
> +++ b/src/ipa/ipu3/algorithms/af.cpp
> @@ -336,29 +336,29 @@ void Af::afIgnoreFrameReset()
>   *
>   * \return The variance of the values in the data set \a y_item selected by \a isY1
>   */
> -double Af::afEstimateVariance(const y_table_item_t *y_item, uint32_t len,
> -			      bool isY1)
> +double Af::afEstimateVariance(Span<const y_table_item_t> y_items, bool isY1)
>  {
> -	uint32_t z = 0;
>  	uint32_t total = 0;
>  	double mean;
>  	double var_sum = 0;
>  
> -	for (z = 0; z < len; z++) {
> +	for (auto y : y_items) {
>  		if (isY1)
> -			total += y_item[z].y1_avg;
> +			total += y.y1_avg;
>  		else
> -			total += y_item[z].y2_avg;
> +			total += y.y2_avg;
>  	}
> -	mean = total / len;
> -	for (z = 0; z < len; z++) {
> +
> +	mean = total / y_items.size();
> +
> +	for (auto y : y_items) {
>  		if (isY1)
> -			var_sum += pow((y_item[z].y1_avg - mean), 2);
> +			var_sum += pow((y.y1_avg - mean), 2);

While at it you could remove the extra parentheses.

>  		else
> -			var_sum += pow((y_item[z].y2_avg - mean), 2);
> +			var_sum += pow((y.y2_avg - mean), 2);

Same here.

>  	}
>  
> -	return var_sum / static_cast<double>(len);
> +	return var_sum / static_cast<double>(y_items.size());

The cast could also be dropped.

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

Did I mention I like the span class ? :-)

>  }
>  
>  /**
> @@ -406,21 +406,21 @@ bool Af::afIsOutOfFocus(IPAContext context)
>   */
>  void Af::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
>  {
> -	const y_table_item_t *y_item = reinterpret_cast<const y_table_item_t *>(&stats->af_raw_buffer.y_table);
> -	uint32_t afRawBufferLen;
> -
>  	/* Evaluate the AF buffer length */
> -	afRawBufferLen = context.configuration.af.afGrid.width *
> -			 context.configuration.af.afGrid.height;
> +	uint32_t afRawBufferLen = context.configuration.af.afGrid.width *
> +				  context.configuration.af.afGrid.height;
>  
>  	ASSERT(afRawBufferLen < IPU3_UAPI_AF_Y_TABLE_MAX_SIZE);
>  
> +	Span<const y_table_item_t> y_items(reinterpret_cast<const y_table_item_t *>(&stats->af_raw_buffer.y_table),
> +					   afRawBufferLen);
> +
>  	/*
>  	 * Calculate the mean and the variance of AF statistics for a given grid.
>  	 * For coarse: y1 are used.
>  	 * For fine: y2 results are used.
>  	 */
> -	currentVariance_ = afEstimateVariance(y_item, afRawBufferLen, !coarseCompleted_);
> +	currentVariance_ = afEstimateVariance(y_items, !coarseCompleted_);
>  
>  	if (!context.frameContext.af.stable) {
>  		afCoarseScan(context);
> diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h
> index 3b5758e868ea..b85cf94163ff 100644
> --- a/src/ipa/ipu3/algorithms/af.h
> +++ b/src/ipa/ipu3/algorithms/af.h
> @@ -41,8 +41,8 @@ private:
>  	void afReset(IPAContext &context);
>  	bool afNeedIgnoreFrame();
>  	void afIgnoreFrameReset();
> -	double afEstimateVariance(const y_table_item_t *y_item, uint32_t len,
> -				  bool isY1);
> +	double afEstimateVariance(Span<const y_table_item_t> y_items, bool isY1);
> +
>  	bool afIsOutOfFocus(IPAContext context);
>  
>  	/* VCM step configuration. It is the current setting of the VCM step. */

-- 
Regards,

Laurent Pinchart