[PATCH v3 3/6] ipa: simple: Report the ColourGains in metadata

[Laurent Pinchart]

Hi Milan,

Thank you for the patch.

On Mon, Jan 13, 2025 at 02:34:02PM +0100, Milan Zamazal wrote:
> From: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> Provide the determined colour gains back into the metadata
> to add to completed requests.
> 
> Metadata must be set before computing the new gain values in order to
> report the metadata used to process the image rather than the metadata
> determined from the image (and to be used for processing the next
> image).

That's not right. Setting the metadata before computing the gains won't
magically solve the synchronization problem. The code can be kept as-is
(ideally with a comment to indicate synchronization needs to be fixed),
but the commit message shouldn't be misleading.

> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  src/ipa/simple/algorithms/awb.cpp | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp
> index 195de41d..95ff4434 100644
> --- a/src/ipa/simple/algorithms/awb.cpp
> +++ b/src/ipa/simple/algorithms/awb.cpp
> @@ -14,6 +14,8 @@
>  
>  #include "simple/ipa_context.h"
>  
> +#include "control_ids.h"
> +
>  namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(IPASoftAwb)
> @@ -33,10 +35,16 @@ void Awb::process(IPAContext &context,
>  		  [[maybe_unused]] const uint32_t frame,
>  		  [[maybe_unused]] IPAFrameContext &frameContext,
>  		  const SwIspStats *stats,
> -		  [[maybe_unused]] ControlList &metadata)
> +		  ControlList &metadata)
>  {
>  	const SwIspStats::Histogram &histogram = stats->yHistogram;
>  	const uint8_t blackLevel = context.activeState.blc.level;
> +	auto &gains = context.activeState.gains;
> +
> +	const float maxGain = 1024.0;
> +	const float mdGains[] = { static_cast<float>(gains.red / maxGain),
> +				  static_cast<float>(gains.blue / maxGain) };
> +	metadata.set(controls::ColourGains, mdGains);
>  
>  	/*
>  	 * Black level must be subtracted to get the correct AWB ratios, they
> @@ -54,7 +62,6 @@ void Awb::process(IPAContext &context,
>  	 * Calculate red and blue gains for AWB.
>  	 * Clamp max gain at 4.0, this also avoids 0 division.
>  	 */
> -	auto &gains = context.activeState.gains;
>  	gains.red = sumR <= sumG / 4 ? 4.0 : static_cast<double>(sumG) / sumR;
>  	gains.blue = sumB <= sumG / 4 ? 4.0 : static_cast<double>(sumG) / sumB;
>  	/* Green gain is fixed to 1.0 */

-- 
Regards,

Laurent Pinchart