[libcamera-devel] ipa: raspberrypi: awb: Update colour temperature whenever manual gains change
diff mbox series

Message ID 20221027113956.7854-1-david.plowman@raspberrypi.com
State Accepted
Commit 375a70d43eaa30e0c4f59f3da209a0f9fdf2b889
Headers show
Series
  • [libcamera-devel] ipa: raspberrypi: awb: Update colour temperature whenever manual gains change
Related show

Commit Message

David Plowman Oct. 27, 2022, 11:39 a.m. UTC 
Previously we only did this when the system starts (on the first
switch_mode). Now we do it whenever the manual colour gains are
updated. To facilitate this, this R/B vs. colour temperature inverse
functions are stored persistently in the AwbConfig.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/ipa/raspberrypi/controller/rpi/awb.cpp | 24 +++++++++-------------
 src/ipa/raspberrypi/controller/rpi/awb.h   |  3 ++-
 2 files changed, 12 insertions(+), 15 deletions(-)

Comments

Naushir Patuck Oct. 28, 2022, 10:53 a.m. UTC | #1 
Hi David,

Thank you for this work.

On Thu, 27 Oct 2022 at 12:40, David Plowman via libcamera-devel <
libcamera-devel@lists.libcamera.org> wrote:

> Previously we only did this when the system starts (on the first
> switch_mode). Now we do it whenever the manual colour gains are
> updated. To facilitate this, this R/B vs. colour temperature inverse
> functions are stored persistently in the AwbConfig.
>
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
>

It's good to be consistent with this behavior.

Reviewed-by: Naushir Patuck <naush@raspberrypi.com>



> ---
>  src/ipa/raspberrypi/controller/rpi/awb.cpp | 24 +++++++++-------------
>  src/ipa/raspberrypi/controller/rpi/awb.h   |  3 ++-
>  2 files changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp
> b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> index 2b88c3b0..8d8ddf09 100644
> --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> @@ -104,6 +104,9 @@ int AwbConfig::read(const libcamera::YamlObject
> &params)
>                 ret = readCtCurve(ctR, ctB, params["ct_curve"]);
>                 if (ret)
>                         return ret;
> +               /* We will want the inverse functions of these too. */
> +               ctRInverse = ctR.inverse();
> +               ctBInverse = ctB.inverse();
>         }
>
>         if (params.contains("priors")) {
> @@ -174,7 +177,6 @@ Awb::Awb(Controller *controller)
>         asyncAbort_ = asyncStart_ = asyncStarted_ = asyncFinished_ = false;
>         mode_ = nullptr;
>         manualR_ = manualB_ = 0.0;
> -       firstSwitchMode_ = true;
>         asyncThread_ = std::thread(std::bind(&Awb::asyncFunc, this));
>  }
>
> @@ -270,27 +272,21 @@ void Awb::setManualGains(double manualR, double
> manualB)
>                 syncResults_.gainR = prevSyncResults_.gainR = manualR_;
>                 syncResults_.gainG = prevSyncResults_.gainG = 1.0;
>                 syncResults_.gainB = prevSyncResults_.gainB = manualB_;
> +               if (config_.bayes) {
> +                       /* Also estimate the best corresponding colour
> temperature from the curves. */
> +                       double ctR =
> config_.ctRInverse.eval(config_.ctRInverse.domain().clip(1 / manualR_));
> +                       double ctB =
> config_.ctBInverse.eval(config_.ctBInverse.domain().clip(1 / manualB_));
> +                       prevSyncResults_.temperatureK = (ctR + ctB) / 2;
> +                       syncResults_.temperatureK =
> prevSyncResults_.temperatureK;
> +               }
>         }
>  }
>
>  void Awb::switchMode([[maybe_unused]] CameraMode const &cameraMode,
>                      Metadata *metadata)
>  {
> -       /*
> -        * On the first mode switch we'll have no meaningful colour
> -        * temperature, so try to dead reckon one if in manual mode.
> -        */
> -       if (!isAutoEnabled() && firstSwitchMode_ && config_.bayes) {
> -               Pwl ctRInverse = config_.ctR.inverse();
> -               Pwl ctBInverse = config_.ctB.inverse();
> -               double ctR = ctRInverse.eval(ctRInverse.domain().clip(1 /
> manualR_));
> -               double ctB = ctBInverse.eval(ctBInverse.domain().clip(1 /
> manualB_));
> -               prevSyncResults_.temperatureK = (ctR + ctB) / 2;
> -               syncResults_.temperatureK = prevSyncResults_.temperatureK;
> -       }
>         /* Let other algorithms know the current white balance values. */
>         metadata->set("awb.status", prevSyncResults_);
> -       firstSwitchMode_ = false;
>  }
>
>  bool Awb::isAutoEnabled() const
> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.h
> b/src/ipa/raspberrypi/controller/rpi/awb.h
> index cb4cfd1b..30acd89d 100644
> --- a/src/ipa/raspberrypi/controller/rpi/awb.h
> +++ b/src/ipa/raspberrypi/controller/rpi/awb.h
> @@ -42,6 +42,8 @@ struct AwbConfig {
>         bool fast; /* "fast" mode uses a 16x16 rather than 32x32 grid */
>         Pwl ctR; /* function maps CT to r (= R/G) */
>         Pwl ctB; /* function maps CT to b (= B/G) */
> +       Pwl ctRInverse; /* inverse of ctR */
> +       Pwl ctBInverse; /* inverse of ctB */
>         /* table of illuminant priors at different lux levels */
>         std::vector<AwbPrior> priors;
>         /* AWB "modes" (determines the search range) */
> @@ -168,7 +170,6 @@ private:
>         double manualR_;
>         /* manual b setting */
>         double manualB_;
> -       bool firstSwitchMode_; /* is this the first call to SwitchMode? */
>  };
>
>  static inline Awb::RGB operator+(Awb::RGB const &a, Awb::RGB const &b)
> --
> 2.30.2
>
>
Kieran Bingham Oct. 28, 2022, 1:26 p.m. UTC | #2 
Quoting Naushir Patuck via libcamera-devel (2022-10-28 11:53:00)
> Hi David,
> 
> Thank you for this work.
> 
> On Thu, 27 Oct 2022 at 12:40, David Plowman via libcamera-devel <
> libcamera-devel@lists.libcamera.org> wrote:
> 
> > Previously we only did this when the system starts (on the first
> > switch_mode). Now we do it whenever the manual colour gains are
> > updated. To facilitate this, this R/B vs. colour temperature inverse
> > functions are stored persistently in the AwbConfig.
> >
> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> >
> 
> It's good to be consistent with this behavior.
> 
> Reviewed-by: Naushir Patuck <naush@raspberrypi.com>

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


> > ---
> >  src/ipa/raspberrypi/controller/rpi/awb.cpp | 24 +++++++++-------------
> >  src/ipa/raspberrypi/controller/rpi/awb.h   |  3 ++-
> >  2 files changed, 12 insertions(+), 15 deletions(-)
> >
> > diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp
> > b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> > index 2b88c3b0..8d8ddf09 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> > @@ -104,6 +104,9 @@ int AwbConfig::read(const libcamera::YamlObject
> > &params)
> >                 ret = readCtCurve(ctR, ctB, params["ct_curve"]);
> >                 if (ret)
> >                         return ret;
> > +               /* We will want the inverse functions of these too. */
> > +               ctRInverse = ctR.inverse();
> > +               ctBInverse = ctB.inverse();
> >         }
> >
> >         if (params.contains("priors")) {
> > @@ -174,7 +177,6 @@ Awb::Awb(Controller *controller)
> >         asyncAbort_ = asyncStart_ = asyncStarted_ = asyncFinished_ = false;
> >         mode_ = nullptr;
> >         manualR_ = manualB_ = 0.0;
> > -       firstSwitchMode_ = true;
> >         asyncThread_ = std::thread(std::bind(&Awb::asyncFunc, this));
> >  }
> >
> > @@ -270,27 +272,21 @@ void Awb::setManualGains(double manualR, double
> > manualB)
> >                 syncResults_.gainR = prevSyncResults_.gainR = manualR_;
> >                 syncResults_.gainG = prevSyncResults_.gainG = 1.0;
> >                 syncResults_.gainB = prevSyncResults_.gainB = manualB_;
> > +               if (config_.bayes) {
> > +                       /* Also estimate the best corresponding colour
> > temperature from the curves. */
> > +                       double ctR =
> > config_.ctRInverse.eval(config_.ctRInverse.domain().clip(1 / manualR_));
> > +                       double ctB =
> > config_.ctBInverse.eval(config_.ctBInverse.domain().clip(1 / manualB_));
> > +                       prevSyncResults_.temperatureK = (ctR + ctB) / 2;
> > +                       syncResults_.temperatureK =
> > prevSyncResults_.temperatureK;
> > +               }
> >         }
> >  }
> >
> >  void Awb::switchMode([[maybe_unused]] CameraMode const &cameraMode,
> >                      Metadata *metadata)
> >  {
> > -       /*
> > -        * On the first mode switch we'll have no meaningful colour
> > -        * temperature, so try to dead reckon one if in manual mode.
> > -        */
> > -       if (!isAutoEnabled() && firstSwitchMode_ && config_.bayes) {
> > -               Pwl ctRInverse = config_.ctR.inverse();
> > -               Pwl ctBInverse = config_.ctB.inverse();
> > -               double ctR = ctRInverse.eval(ctRInverse.domain().clip(1 /
> > manualR_));
> > -               double ctB = ctBInverse.eval(ctBInverse.domain().clip(1 /
> > manualB_));
> > -               prevSyncResults_.temperatureK = (ctR + ctB) / 2;
> > -               syncResults_.temperatureK = prevSyncResults_.temperatureK;
> > -       }
> >         /* Let other algorithms know the current white balance values. */
> >         metadata->set("awb.status", prevSyncResults_);
> > -       firstSwitchMode_ = false;
> >  }
> >
> >  bool Awb::isAutoEnabled() const
> > diff --git a/src/ipa/raspberrypi/controller/rpi/awb.h
> > b/src/ipa/raspberrypi/controller/rpi/awb.h
> > index cb4cfd1b..30acd89d 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/awb.h
> > +++ b/src/ipa/raspberrypi/controller/rpi/awb.h
> > @@ -42,6 +42,8 @@ struct AwbConfig {
> >         bool fast; /* "fast" mode uses a 16x16 rather than 32x32 grid */
> >         Pwl ctR; /* function maps CT to r (= R/G) */
> >         Pwl ctB; /* function maps CT to b (= B/G) */
> > +       Pwl ctRInverse; /* inverse of ctR */
> > +       Pwl ctBInverse; /* inverse of ctB */
> >         /* table of illuminant priors at different lux levels */
> >         std::vector<AwbPrior> priors;
> >         /* AWB "modes" (determines the search range) */
> > @@ -168,7 +170,6 @@ private:
> >         double manualR_;
> >         /* manual b setting */
> >         double manualB_;
> > -       bool firstSwitchMode_; /* is this the first call to SwitchMode? */
> >  };
> >
> >  static inline Awb::RGB operator+(Awb::RGB const &a, Awb::RGB const &b)
> > --
> > 2.30.2
> >
> >

Patch
diff mbox series 

diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp b/src/ipa/raspberrypi/controller/rpi/awb.cpp
index 2b88c3b0..8d8ddf09 100644
--- a/src/ipa/raspberrypi/controller/rpi/awb.cpp
+++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp
@@ -104,6 +104,9 @@  int AwbConfig::read(const libcamera::YamlObject &params)
 		ret = readCtCurve(ctR, ctB, params["ct_curve"]);
 		if (ret)
 			return ret;
+		/* We will want the inverse functions of these too. */
+		ctRInverse = ctR.inverse();
+		ctBInverse = ctB.inverse();
 	}
 
 	if (params.contains("priors")) {
@@ -174,7 +177,6 @@  Awb::Awb(Controller *controller)
 	asyncAbort_ = asyncStart_ = asyncStarted_ = asyncFinished_ = false;
 	mode_ = nullptr;
 	manualR_ = manualB_ = 0.0;
-	firstSwitchMode_ = true;
 	asyncThread_ = std::thread(std::bind(&Awb::asyncFunc, this));
 }
 
@@ -270,27 +272,21 @@  void Awb::setManualGains(double manualR, double manualB)
 		syncResults_.gainR = prevSyncResults_.gainR = manualR_;
 		syncResults_.gainG = prevSyncResults_.gainG = 1.0;
 		syncResults_.gainB = prevSyncResults_.gainB = manualB_;
+		if (config_.bayes) {
+			/* Also estimate the best corresponding colour temperature from the curves. */
+			double ctR = config_.ctRInverse.eval(config_.ctRInverse.domain().clip(1 / manualR_));
+			double ctB = config_.ctBInverse.eval(config_.ctBInverse.domain().clip(1 / manualB_));
+			prevSyncResults_.temperatureK = (ctR + ctB) / 2;
+			syncResults_.temperatureK = prevSyncResults_.temperatureK;
+		}
 	}
 }
 
 void Awb::switchMode([[maybe_unused]] CameraMode const &cameraMode,
 		     Metadata *metadata)
 {
-	/*
-	 * On the first mode switch we'll have no meaningful colour
-	 * temperature, so try to dead reckon one if in manual mode.
-	 */
-	if (!isAutoEnabled() && firstSwitchMode_ && config_.bayes) {
-		Pwl ctRInverse = config_.ctR.inverse();
-		Pwl ctBInverse = config_.ctB.inverse();
-		double ctR = ctRInverse.eval(ctRInverse.domain().clip(1 / manualR_));
-		double ctB = ctBInverse.eval(ctBInverse.domain().clip(1 / manualB_));
-		prevSyncResults_.temperatureK = (ctR + ctB) / 2;
-		syncResults_.temperatureK = prevSyncResults_.temperatureK;
-	}
 	/* Let other algorithms know the current white balance values. */
 	metadata->set("awb.status", prevSyncResults_);
-	firstSwitchMode_ = false;
 }
 
 bool Awb::isAutoEnabled() const
diff --git a/src/ipa/raspberrypi/controller/rpi/awb.h b/src/ipa/raspberrypi/controller/rpi/awb.h
index cb4cfd1b..30acd89d 100644
--- a/src/ipa/raspberrypi/controller/rpi/awb.h
+++ b/src/ipa/raspberrypi/controller/rpi/awb.h
@@ -42,6 +42,8 @@  struct AwbConfig {
 	bool fast; /* "fast" mode uses a 16x16 rather than 32x32 grid */
 	Pwl ctR; /* function maps CT to r (= R/G) */
 	Pwl ctB; /* function maps CT to b (= B/G) */
+	Pwl ctRInverse; /* inverse of ctR */
+	Pwl ctBInverse; /* inverse of ctB */
 	/* table of illuminant priors at different lux levels */
 	std::vector<AwbPrior> priors;
 	/* AWB "modes" (determines the search range) */
@@ -168,7 +170,6 @@  private:
 	double manualR_;
 	/* manual b setting */
 	double manualB_;
-	bool firstSwitchMode_; /* is this the first call to SwitchMode? */
 };
 
 static inline Awb::RGB operator+(Awb::RGB const &a, Awb::RGB const &b)