[v3,2/2] ipa: rpi: awb: Add a bias to the AWB search
diff mbox series

Message ID 20241008081317.24602-2-naush@raspberrypi.com
State Accepted
Commit ea8fd63d936f19bfc773199b9fe81e8d5d1f709b
Headers show
Series
  • [v3,1/2] ipa: rpi: awb: Add a const for the default colour temperature
Related show

Commit Message

Naushir Patuck Oct. 8, 2024, 8:13 a.m. UTC 
In the case of an AWB search failure, the current algorithm logic will
return a point on the CT curve closest to where the search finisned.
This can be quite undesirable. Instead, add some bias params to the AWB
algorithm which will direct the search to a set CT value in the case
where statistics become unreliable causing the search to fail.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/ipa/rpi/controller/rpi/awb.cpp | 23 +++++++++++++++++++++--
 src/ipa/rpi/controller/rpi/awb.h   |  4 ++++
 2 files changed, 25 insertions(+), 2 deletions(-)

Comments

Kieran Bingham Oct. 8, 2024, 8:53 a.m. UTC | #1 
Quoting Naushir Patuck (2024-10-08 09:13:17)
> In the case of an AWB search failure, the current algorithm logic will
> return a point on the CT curve closest to where the search finisned.
> This can be quite undesirable. Instead, add some bias params to the AWB
> algorithm which will direct the search to a set CT value in the case
> where statistics become unreliable causing the search to fail.
> 
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

Ah, I was about to merge the v2, but now I see v3.

v2 passed the CI, so I'll shove this through and merge if nothing else
crops up.


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

> ---
>  src/ipa/rpi/controller/rpi/awb.cpp | 23 +++++++++++++++++++++--
>  src/ipa/rpi/controller/rpi/awb.h   |  4 ++++
>  2 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ipa/rpi/controller/rpi/awb.cpp b/src/ipa/rpi/controller/rpi/awb.cpp
> index 65416d05b28e..9d8e170d1bfe 100644
> --- a/src/ipa/rpi/controller/rpi/awb.cpp
> +++ b/src/ipa/rpi/controller/rpi/awb.cpp
> @@ -170,6 +170,14 @@ int AwbConfig::read(const libcamera::YamlObject &params)
>         whitepointB = params["whitepoint_b"].get<double>(0.0);
>         if (bayes == false)
>                 sensitivityR = sensitivityB = 1.0; /* nor do sensitivities make any sense */
> +       /*
> +        * The biasProportion parameter adds a small proportion of the counted
> +        * pixles to a region biased to the biasCT colour temperature.
> +        *
> +        * A typical value for biasProportion would be between 0.05 to 0.1.
> +        */
> +       biasProportion = params["bias_proportion"].get<double>(0.0);
> +       biasCT = params["bias_ct"].get<double>(kDefaultCT);
>         return 0;
>  }
>  
> @@ -410,7 +418,8 @@ void Awb::asyncFunc()
>  
>  static void generateStats(std::vector<Awb::RGB> &zones,
>                           StatisticsPtr &stats, double minPixels,
> -                         double minG, Metadata &globalMetadata)
> +                         double minG, Metadata &globalMetadata,
> +                         double biasProportion, double biasCtR, double biasCtB)
>  {
>         std::scoped_lock<RPiController::Metadata> l(globalMetadata);
>  
> @@ -423,6 +432,14 @@ static void generateStats(std::vector<Awb::RGB> &zones,
>                                 continue;
>                         zone.R = region.val.rSum / region.counted;
>                         zone.B = region.val.bSum / region.counted;
> +                       /*
> +                        * Add some bias samples to allow the search to tend to a
> +                        * bias CT in failure cases.
> +                        */
> +                       const unsigned int proportion = biasProportion * region.counted;
> +                       zone.R += proportion * biasCtR;
> +                       zone.B += proportion * biasCtB;
> +                       zone.G += proportion * 1.0;
>                         /* Factor in the ALSC applied colour shading correction if required. */
>                         const AlscStatus *alscStatus = globalMetadata.getLocked<AlscStatus>("alsc.status");
>                         if (stats->colourStatsPos == Statistics::ColourStatsPos::PreLsc && alscStatus) {
> @@ -443,7 +460,9 @@ void Awb::prepareStats()
>          * any LSC compensation.  We also ignore config_.fast in this version.
>          */
>         generateStats(zones_, statistics_, config_.minPixels,
> -                     config_.minG, getGlobalMetadata());
> +                     config_.minG, getGlobalMetadata(),
> +                     config_.biasProportion, config_.ctR.eval(config_.biasCT),
> +                     config_.ctB.eval(config_.biasCT));
>         /*
>          * apply sensitivities, so values appear to come from our "canonical"
>          * sensor.
> diff --git a/src/ipa/rpi/controller/rpi/awb.h b/src/ipa/rpi/controller/rpi/awb.h
> index ab30f4fa51c1..5d628b47c8a6 100644
> --- a/src/ipa/rpi/controller/rpi/awb.h
> +++ b/src/ipa/rpi/controller/rpi/awb.h
> @@ -87,6 +87,10 @@ struct AwbConfig {
>         double whitepointR;
>         double whitepointB;
>         bool bayes; /* use Bayesian algorithm */
> +       /* proportion of counted samples to add for the search bias */
> +       double biasProportion;
> +       /* CT target for the search bias */
> +       double biasCT;
>  };
>  
>  class Awb : public AwbAlgorithm
> -- 
> 2.34.1
>
Laurent Pinchart Oct. 8, 2024, 2:32 p.m. UTC | #2 
On Tue, Oct 08, 2024 at 09:13:17AM +0100, Naushir Patuck wrote:
> In the case of an AWB search failure, the current algorithm logic will
> return a point on the CT curve closest to where the search finisned.
> This can be quite undesirable. Instead, add some bias params to the AWB
> algorithm which will direct the search to a set CT value in the case
> where statistics become unreliable causing the search to fail.
> 
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  src/ipa/rpi/controller/rpi/awb.cpp | 23 +++++++++++++++++++++--
>  src/ipa/rpi/controller/rpi/awb.h   |  4 ++++
>  2 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ipa/rpi/controller/rpi/awb.cpp b/src/ipa/rpi/controller/rpi/awb.cpp
> index 65416d05b28e..9d8e170d1bfe 100644
> --- a/src/ipa/rpi/controller/rpi/awb.cpp
> +++ b/src/ipa/rpi/controller/rpi/awb.cpp
> @@ -170,6 +170,14 @@ int AwbConfig::read(const libcamera::YamlObject &params)
>  	whitepointB = params["whitepoint_b"].get<double>(0.0);
>  	if (bayes == false)
>  		sensitivityR = sensitivityB = 1.0; /* nor do sensitivities make any sense */
> +	/*
> +	 * The biasProportion parameter adds a small proportion of the counted
> +	 * pixles to a region biased to the biasCT colour temperature.

s/pixles/pixels/

> +	 *
> +	 * A typical value for biasProportion would be between 0.05 to 0.1.
> +	 */
> +	biasProportion = params["bias_proportion"].get<double>(0.0);
> +	biasCT = params["bias_ct"].get<double>(kDefaultCT);
>  	return 0;
>  }
>  
> @@ -410,7 +418,8 @@ void Awb::asyncFunc()
>  
>  static void generateStats(std::vector<Awb::RGB> &zones,
>  			  StatisticsPtr &stats, double minPixels,
> -			  double minG, Metadata &globalMetadata)
> +			  double minG, Metadata &globalMetadata,
> +			  double biasProportion, double biasCtR, double biasCtB)
>  {
>  	std::scoped_lock<RPiController::Metadata> l(globalMetadata);
>  
> @@ -423,6 +432,14 @@ static void generateStats(std::vector<Awb::RGB> &zones,
>  				continue;
>  			zone.R = region.val.rSum / region.counted;
>  			zone.B = region.val.bSum / region.counted;
> +			/*
> +			 * Add some bias samples to allow the search to tend to a
> +			 * bias CT in failure cases.
> +			 */
> +			const unsigned int proportion = biasProportion * region.counted;
> +			zone.R += proportion * biasCtR;
> +			zone.B += proportion * biasCtB;
> +			zone.G += proportion * 1.0;
>  			/* Factor in the ALSC applied colour shading correction if required. */
>  			const AlscStatus *alscStatus = globalMetadata.getLocked<AlscStatus>("alsc.status");
>  			if (stats->colourStatsPos == Statistics::ColourStatsPos::PreLsc && alscStatus) {
> @@ -443,7 +460,9 @@ void Awb::prepareStats()
>  	 * any LSC compensation.  We also ignore config_.fast in this version.
>  	 */
>  	generateStats(zones_, statistics_, config_.minPixels,
> -		      config_.minG, getGlobalMetadata());
> +		      config_.minG, getGlobalMetadata(),
> +		      config_.biasProportion, config_.ctR.eval(config_.biasCT),
> +		      config_.ctB.eval(config_.biasCT));
>  	/*
>  	 * apply sensitivities, so values appear to come from our "canonical"
>  	 * sensor.
> diff --git a/src/ipa/rpi/controller/rpi/awb.h b/src/ipa/rpi/controller/rpi/awb.h
> index ab30f4fa51c1..5d628b47c8a6 100644
> --- a/src/ipa/rpi/controller/rpi/awb.h
> +++ b/src/ipa/rpi/controller/rpi/awb.h
> @@ -87,6 +87,10 @@ struct AwbConfig {
>  	double whitepointR;
>  	double whitepointB;
>  	bool bayes; /* use Bayesian algorithm */
> +	/* proportion of counted samples to add for the search bias */
> +	double biasProportion;
> +	/* CT target for the search bias */
> +	double biasCT;
>  };
>  
>  class Awb : public AwbAlgorithm

Patch
diff mbox series 

diff --git a/src/ipa/rpi/controller/rpi/awb.cpp b/src/ipa/rpi/controller/rpi/awb.cpp
index 65416d05b28e..9d8e170d1bfe 100644
--- a/src/ipa/rpi/controller/rpi/awb.cpp
+++ b/src/ipa/rpi/controller/rpi/awb.cpp
@@ -170,6 +170,14 @@  int AwbConfig::read(const libcamera::YamlObject &params)
 	whitepointB = params["whitepoint_b"].get<double>(0.0);
 	if (bayes == false)
 		sensitivityR = sensitivityB = 1.0; /* nor do sensitivities make any sense */
+	/*
+	 * The biasProportion parameter adds a small proportion of the counted
+	 * pixles to a region biased to the biasCT colour temperature.
+	 *
+	 * A typical value for biasProportion would be between 0.05 to 0.1.
+	 */
+	biasProportion = params["bias_proportion"].get<double>(0.0);
+	biasCT = params["bias_ct"].get<double>(kDefaultCT);
 	return 0;
 }
 
@@ -410,7 +418,8 @@  void Awb::asyncFunc()
 
 static void generateStats(std::vector<Awb::RGB> &zones,
 			  StatisticsPtr &stats, double minPixels,
-			  double minG, Metadata &globalMetadata)
+			  double minG, Metadata &globalMetadata,
+			  double biasProportion, double biasCtR, double biasCtB)
 {
 	std::scoped_lock<RPiController::Metadata> l(globalMetadata);
 
@@ -423,6 +432,14 @@  static void generateStats(std::vector<Awb::RGB> &zones,
 				continue;
 			zone.R = region.val.rSum / region.counted;
 			zone.B = region.val.bSum / region.counted;
+			/*
+			 * Add some bias samples to allow the search to tend to a
+			 * bias CT in failure cases.
+			 */
+			const unsigned int proportion = biasProportion * region.counted;
+			zone.R += proportion * biasCtR;
+			zone.B += proportion * biasCtB;
+			zone.G += proportion * 1.0;
 			/* Factor in the ALSC applied colour shading correction if required. */
 			const AlscStatus *alscStatus = globalMetadata.getLocked<AlscStatus>("alsc.status");
 			if (stats->colourStatsPos == Statistics::ColourStatsPos::PreLsc && alscStatus) {
@@ -443,7 +460,9 @@  void Awb::prepareStats()
 	 * any LSC compensation.  We also ignore config_.fast in this version.
 	 */
 	generateStats(zones_, statistics_, config_.minPixels,
-		      config_.minG, getGlobalMetadata());
+		      config_.minG, getGlobalMetadata(),
+		      config_.biasProportion, config_.ctR.eval(config_.biasCT),
+		      config_.ctB.eval(config_.biasCT));
 	/*
 	 * apply sensitivities, so values appear to come from our "canonical"
 	 * sensor.
diff --git a/src/ipa/rpi/controller/rpi/awb.h b/src/ipa/rpi/controller/rpi/awb.h
index ab30f4fa51c1..5d628b47c8a6 100644
--- a/src/ipa/rpi/controller/rpi/awb.h
+++ b/src/ipa/rpi/controller/rpi/awb.h
@@ -87,6 +87,10 @@  struct AwbConfig {
 	double whitepointR;
 	double whitepointB;
 	bool bayes; /* use Bayesian algorithm */
+	/* proportion of counted samples to add for the search bias */
+	double biasProportion;
+	/* CT target for the search bias */
+	double biasCT;
 };
 
 class Awb : public AwbAlgorithm