[libcamera-devel] [PATCH v2 5/6] src: ipa: raspberrypi: Estimate the colour temerature if starting with fixed colour gains

[Naushir Patuck]

Hi David,

Thank you for your patch.

On Mon, 7 Dec 2020 at 18:02, David Plowman <david.plowman at raspberrypi.com>
wrote:

> When the AWB is started from "cold" with fixed colour gains, we try to
> estimate the colour temperature this corresponds to (if a calibrated
> CT curve was supplied). When fixed colour gains are set after the AWB
> has been running, we leave the CT estimate alone, as the one we have
> is probably sensible.
>
> This estimated colour is passed out in the metadata for other
> algorithms - notably ALSC - to use.
>
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  src/ipa/raspberrypi/controller/rpi/alsc.cpp |  6 +++++-
>  src/ipa/raspberrypi/controller/rpi/awb.cpp  | 14 ++++++++++++++
>  src/ipa/raspberrypi/controller/rpi/awb.hpp  |  1 +
>  3 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> b/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> index 183a0c95..c354c985 100644
> --- a/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> @@ -146,6 +146,7 @@ void Alsc::Read(boost::property_tree::ptree const
> &params)
>         config_.threshold = params.get<double>("threshold", 1e-3);
>  }
>
> +static double get_ct(Metadata *metadata, double default_ct);
>  static void get_cal_table(double ct,
>                           std::vector<AlscCalibration> const &calibrations,
>                           double cal_table[XY]);
> @@ -210,6 +211,9 @@ void Alsc::SwitchMode(CameraMode const &camera_mode,
>         // change.
>         bool reset_tables = first_time_ || compare_modes(camera_mode_,
> camera_mode);
>
> +       // Believe the colour temperature from the AWB, if there is one.
> +       ct_ = get_ct(metadata, ct_);
> +
>         // Ensure the other thread isn't running while we do this.
>         waitForAysncThread();
>
> @@ -254,7 +258,7 @@ void Alsc::fetchAsyncResults()
>         memcpy(sync_results_, async_results_, sizeof(sync_results_));
>  }
>
> -static double get_ct(Metadata *metadata, double default_ct)
> +double get_ct(Metadata *metadata, double default_ct)
>  {
>         AwbStatus awb_status;
>         awb_status.temperature_K = default_ct; // in case nothing found
> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp
> b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> index 6b359ac5..2266c07b 100644
> --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> @@ -19,6 +19,9 @@ using namespace RPiController;
>
>  const double Awb::RGB::INVALID = -1.0;
>
> +// todo - the locking in this algorithm needs some tidying up as has been
> done
> +// elsewhere (ALSC and AGC).
> +
>  void AwbMode::Read(boost::property_tree::ptree const &params)
>  {
>         ct_lo = params.get<double>("lo");
> @@ -121,6 +124,7 @@ Awb::Awb(Controller *controller)
>         async_abort_ = async_start_ = async_started_ = async_finished_ =
> false;
>         mode_ = nullptr;
>         manual_r_ = manual_b_ = 0.0;
> +       first_switch_mode_ = true;
>         async_thread_ = std::thread(std::bind(&Awb::asyncFunc, this));
>  }
>
> @@ -201,9 +205,19 @@ void Awb::SwitchMode([[maybe_unused]] CameraMode
> const &camera_mode,
>                 prev_sync_results_.gain_r = manual_r_;
>                 prev_sync_results_.gain_g = 1.0;
>                 prev_sync_results_.gain_b = manual_b_;
> +               // If we're starting up for the first time, try and
> +               // "dead reckon" the corresponding colour temperature.
> +               if (first_switch_mode_ && config_.bayes) {
> +                       Pwl ct_r_inverse =
> std::move(config_.ct_r.Inverse());
> +                       Pwl ct_b_inverse =
> std::move(config_.ct_b.Inverse());
>

Not a review comment just for my curiosity - will std::move do anything
here?  Would C++ Return Value Optimisation ensure that we don't make any
extra copies without the std::move?

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


> +                       double ct_r =
> ct_r_inverse.Eval(ct_r_inverse.Domain().Clip(1 / manual_r_));
> +                       double ct_b =
> ct_b_inverse.Eval(ct_b_inverse.Domain().Clip(1 / manual_b_));
> +                       prev_sync_results_.temperature_K = (ct_r + ct_b) /
> 2;
> +               }
>                 sync_results_ = prev_sync_results_;
>         }
>         metadata->Set("awb.status", prev_sync_results_);
> +       first_switch_mode_ = false;
>  }
>
>  void Awb::fetchAsyncResults()
> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.hpp
> b/src/ipa/raspberrypi/controller/rpi/awb.hpp
> index d86b9598..8525af32 100644
> --- a/src/ipa/raspberrypi/controller/rpi/awb.hpp
> +++ b/src/ipa/raspberrypi/controller/rpi/awb.hpp
> @@ -159,6 +159,7 @@ private:
>         double manual_r_;
>         // manual b setting
>         double manual_b_;
> +       bool first_switch_mode_; // is this the first call to SwitchMode?
>  };
>
>  static inline Awb::RGB operator+(Awb::RGB const &a, Awb::RGB const &b)
> --
> 2.20.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20201208/c6d07b11/attachment-0001.htm>