[v1,3/5] ipa: rkisp1: awb: Unconditionally fill metadata
diff mbox series

Message ID 20240712143227.3036702-4-stefan.klug@ideasonboard.com
State Accepted
Headers show
Series
  • A few fixes for the rkisp1 ipa
Related show

Commit Message

Stefan Klug July 12, 2024, 2:32 p.m. UTC 
When the colour temperature estimation gets skipped, the metadata isn't
populated. Fix that by filling the metadata early in the function.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 src/ipa/rkisp1/algorithms/awb.cpp | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

Comments

Dan Scally July 12, 2024, 3:03 p.m. UTC | #1 
Hi Stefan

On 12/07/2024 15:32, Stefan Klug wrote:
> When the colour temperature estimation gets skipped, the metadata isn't
> populated. Fix that by filling the metadata early in the function.
>
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---


Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

>   src/ipa/rkisp1/algorithms/awb.cpp | 17 ++++++++++-------
>   1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> index 18f750207793..4ccafd48dedd 100644
> --- a/src/ipa/rkisp1/algorithms/awb.cpp
> +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> @@ -222,6 +222,12 @@ void Awb::process(IPAContext &context,
>   	double redMean;
>   	double blueMean;
>   
> +	metadata.set(controls::AwbEnable, frameContext.awb.autoEnabled);
> +	metadata.set(controls::ColourGains, {
> +			static_cast<float>(frameContext.awb.gains.red),
> +			static_cast<float>(frameContext.awb.gains.blue)
> +		});
> +
>   	if (rgbMode_) {
>   		greenMean = awb->awb_mean[0].mean_y_or_g;
>   		redMean = awb->awb_mean[0].mean_cr_or_r;
> @@ -277,11 +283,15 @@ void Awb::process(IPAContext &context,
>   	 */
>   	if (redMean < kMeanMinThreshold && greenMean < kMeanMinThreshold &&
>   	    blueMean < kMeanMinThreshold) {
> +		metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);
>   		return;
>   	}
>   
>   	activeState.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);
>   
> +	/* Metadata shall contain the up to date measurement */
> +	metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);
> +
>   	/*
>   	 * Estimate the red and blue gains to apply in a grey world. The green
>   	 * gain is hardcoded to 1.0. Avoid divisions by zero by clamping the
> @@ -308,13 +318,6 @@ void Awb::process(IPAContext &context,
>   	activeState.awb.gains.automatic.blue = blueGain;
>   	activeState.awb.gains.automatic.green = 1.0;
>   
> -	metadata.set(controls::AwbEnable, frameContext.awb.autoEnabled);
> -	metadata.set(controls::ColourGains, {
> -			static_cast<float>(frameContext.awb.gains.red),
> -			static_cast<float>(frameContext.awb.gains.blue)
> -		});
> -	metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);
> -
>   	LOG(RkISP1Awb, Debug) << std::showpoint
>   		<< "Means [" << redMean << ", " << greenMean << ", " << blueMean
>   		<< "], gains [" << activeState.awb.gains.automatic.red << ", "
Kieran Bingham July 14, 2024, 11:51 p.m. UTC | #2 
Quoting Stefan Klug (2024-07-12 15:32:04)
> When the colour temperature estimation gets skipped, the metadata isn't
> populated. Fix that by filling the metadata early in the function.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>  src/ipa/rkisp1/algorithms/awb.cpp | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> index 18f750207793..4ccafd48dedd 100644
> --- a/src/ipa/rkisp1/algorithms/awb.cpp
> +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> @@ -222,6 +222,12 @@ void Awb::process(IPAContext &context,
>         double redMean;
>         double blueMean;
>  
> +       metadata.set(controls::AwbEnable, frameContext.awb.autoEnabled);
> +       metadata.set(controls::ColourGains, {
> +                       static_cast<float>(frameContext.awb.gains.red),
> +                       static_cast<float>(frameContext.awb.gains.blue)
> +               });
> +
>         if (rgbMode_) {
>                 greenMean = awb->awb_mean[0].mean_y_or_g;
>                 redMean = awb->awb_mean[0].mean_cr_or_r;
> @@ -277,11 +283,15 @@ void Awb::process(IPAContext &context,
>          */
>         if (redMean < kMeanMinThreshold && greenMean < kMeanMinThreshold &&
>             blueMean < kMeanMinThreshold) {
> +               metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);

Maybe the braces aren't so redundant after all. Perhaps the metadata
could have been updated in the previous patch as it was so closely
related - but I won't object to this.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


>                 return;
>         }
>  
>         activeState.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);
>  
> +       /* Metadata shall contain the up to date measurement */
> +       metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);
> +
>         /*
>          * Estimate the red and blue gains to apply in a grey world. The green
>          * gain is hardcoded to 1.0. Avoid divisions by zero by clamping the
> @@ -308,13 +318,6 @@ void Awb::process(IPAContext &context,
>         activeState.awb.gains.automatic.blue = blueGain;
>         activeState.awb.gains.automatic.green = 1.0;
>  
> -       metadata.set(controls::AwbEnable, frameContext.awb.autoEnabled);
> -       metadata.set(controls::ColourGains, {
> -                       static_cast<float>(frameContext.awb.gains.red),
> -                       static_cast<float>(frameContext.awb.gains.blue)
> -               });
> -       metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);
> -
>         LOG(RkISP1Awb, Debug) << std::showpoint
>                 << "Means [" << redMean << ", " << greenMean << ", " << blueMean
>                 << "], gains [" << activeState.awb.gains.automatic.red << ", "
> -- 
> 2.43.0
>
Paul Elder July 19, 2024, 7:19 a.m. UTC | #3 
On Fri, Jul 12, 2024 at 04:32:04PM +0200, Stefan Klug wrote:
> When the colour temperature estimation gets skipped, the metadata isn't
> populated. Fix that by filling the metadata early in the function.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/ipa/rkisp1/algorithms/awb.cpp | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> index 18f750207793..4ccafd48dedd 100644
> --- a/src/ipa/rkisp1/algorithms/awb.cpp
> +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> @@ -222,6 +222,12 @@ void Awb::process(IPAContext &context,
>  	double redMean;
>  	double blueMean;
>  
> +	metadata.set(controls::AwbEnable, frameContext.awb.autoEnabled);
> +	metadata.set(controls::ColourGains, {
> +			static_cast<float>(frameContext.awb.gains.red),
> +			static_cast<float>(frameContext.awb.gains.blue)
> +		});
> +
>  	if (rgbMode_) {
>  		greenMean = awb->awb_mean[0].mean_y_or_g;
>  		redMean = awb->awb_mean[0].mean_cr_or_r;
> @@ -277,11 +283,15 @@ void Awb::process(IPAContext &context,
>  	 */
>  	if (redMean < kMeanMinThreshold && greenMean < kMeanMinThreshold &&
>  	    blueMean < kMeanMinThreshold) {
> +		metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);
>  		return;
>  	}
>  
>  	activeState.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);
>  
> +	/* Metadata shall contain the up to date measurement */
> +	metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);
> +
>  	/*
>  	 * Estimate the red and blue gains to apply in a grey world. The green
>  	 * gain is hardcoded to 1.0. Avoid divisions by zero by clamping the
> @@ -308,13 +318,6 @@ void Awb::process(IPAContext &context,
>  	activeState.awb.gains.automatic.blue = blueGain;
>  	activeState.awb.gains.automatic.green = 1.0;
>  
> -	metadata.set(controls::AwbEnable, frameContext.awb.autoEnabled);
> -	metadata.set(controls::ColourGains, {
> -			static_cast<float>(frameContext.awb.gains.red),
> -			static_cast<float>(frameContext.awb.gains.blue)
> -		});
> -	metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);
> -
>  	LOG(RkISP1Awb, Debug) << std::showpoint
>  		<< "Means [" << redMean << ", " << greenMean << ", " << blueMean
>  		<< "], gains [" << activeState.awb.gains.automatic.red << ", "
> -- 
> 2.43.0
>

Patch
diff mbox series 

diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
index 18f750207793..4ccafd48dedd 100644
--- a/src/ipa/rkisp1/algorithms/awb.cpp
+++ b/src/ipa/rkisp1/algorithms/awb.cpp
@@ -222,6 +222,12 @@  void Awb::process(IPAContext &context,
 	double redMean;
 	double blueMean;
 
+	metadata.set(controls::AwbEnable, frameContext.awb.autoEnabled);
+	metadata.set(controls::ColourGains, {
+			static_cast<float>(frameContext.awb.gains.red),
+			static_cast<float>(frameContext.awb.gains.blue)
+		});
+
 	if (rgbMode_) {
 		greenMean = awb->awb_mean[0].mean_y_or_g;
 		redMean = awb->awb_mean[0].mean_cr_or_r;
@@ -277,11 +283,15 @@  void Awb::process(IPAContext &context,
 	 */
 	if (redMean < kMeanMinThreshold && greenMean < kMeanMinThreshold &&
 	    blueMean < kMeanMinThreshold) {
+		metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);
 		return;
 	}
 
 	activeState.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);
 
+	/* Metadata shall contain the up to date measurement */
+	metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);
+
 	/*
 	 * Estimate the red and blue gains to apply in a grey world. The green
 	 * gain is hardcoded to 1.0. Avoid divisions by zero by clamping the
@@ -308,13 +318,6 @@  void Awb::process(IPAContext &context,
 	activeState.awb.gains.automatic.blue = blueGain;
 	activeState.awb.gains.automatic.green = 1.0;
 
-	metadata.set(controls::AwbEnable, frameContext.awb.autoEnabled);
-	metadata.set(controls::ColourGains, {
-			static_cast<float>(frameContext.awb.gains.red),
-			static_cast<float>(frameContext.awb.gains.blue)
-		});
-	metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);
-
 	LOG(RkISP1Awb, Debug) << std::showpoint
 		<< "Means [" << redMean << ", " << greenMean << ", " << blueMean
 		<< "], gains [" << activeState.awb.gains.automatic.red << ", "