<div dir="ltr"><div dir="ltr">Hi Laurent,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 6 Apr 2022 at 11:11, Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">From: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
<br>
Under the right circumstances, the alsc calculations could spread the colour<br>
errors across the entire image as lambda remains unbound. This would cause the<br>
corrected image chroma values to slowly drift to incorrect values.<br>
<br>
This change adds a config parameter (alsc.lambda_bound) that provides an upper<br>
and lower bound to the lambda value at every stage of the calculation. With this<br>
change, we now adjust the lambda values so that the average across the entire<br>
grid is 1 instead of normalising to the minimum value.<br>
<br>
Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
Reviewed-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
Reviewed-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>><br>
Signed-off-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>><br>
---<br>
Changes since v2:<br>
<br>
- Fix typo<br>
- Use Span and std::accumulate<br>
<br>
Naush, could you please test this to make sure I haven't messed up<br>
anything ?<br></blockquote><div><br></div><div>Looks good!</div><div><br></div><div>Tested-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com">naush@raspberrypi.com</a>></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
 src/ipa/raspberrypi/controller/rpi/alsc.cpp | 58 +++++++++++++++------<br>
 src/ipa/raspberrypi/controller/rpi/alsc.hpp |  1 +<br>
 2 files changed, 44 insertions(+), 15 deletions(-)<br>
<br>
diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.cpp b/src/ipa/raspberrypi/controller/rpi/alsc.cpp<br>
index be3d1ae476cd..e575c14a92db 100644<br>
--- a/src/ipa/raspberrypi/controller/rpi/alsc.cpp<br>
+++ b/src/ipa/raspberrypi/controller/rpi/alsc.cpp<br>
@@ -4,9 +4,12 @@<br>
  *<br>
  * alsc.cpp - ALSC (auto lens shading correction) control algorithm<br>
  */<br>
+<br>
 #include <math.h><br>
+#include <numeric><br>
<br>
 #include <libcamera/base/log.h><br>
+#include <libcamera/base/span.h><br>
<br>
 #include "../awb_status.h"<br>
 #include "alsc.hpp"<br>
@@ -149,6 +152,7 @@ void Alsc::Read(boost::property_tree::ptree const &params)<br>
        read_calibrations(config_.calibrations_Cb, params, "calibrations_Cb");<br>
        config_.default_ct = params.get<double>("default_ct", 4500.0);<br>
        config_.threshold = params.get<double>("threshold", 1e-3);<br>
+       config_.lambda_bound = params.get<double>("lambda_bound", 0.05);<br>
 }<br>
<br>
 static double get_ct(Metadata *metadata, double default_ct);<br>
@@ -610,30 +614,47 @@ static double compute_lambda_top_end(int i, double const M[XY][4],<br>
<br>
 // Gauss-Seidel iteration with over-relaxation.<br>
 static double gauss_seidel2_SOR(double const M[XY][4], double omega,<br>
-                               double lambda[XY])<br>
+                               double lambda[XY], double lambda_bound)<br>
 {<br>
+       const double min = 1 - lambda_bound, max = 1 + lambda_bound;<br>
        double old_lambda[XY];<br>
        int i;<br>
        for (i = 0; i < XY; i++)<br>
                old_lambda[i] = lambda[i];<br>
        lambda[0] = compute_lambda_bottom_start(0, M, lambda);<br>
-       for (i = 1; i < X; i++)<br>
+       lambda[0] = std::clamp(lambda[0], min, max);<br>
+       for (i = 1; i < X; i++) {<br>
                lambda[i] = compute_lambda_bottom(i, M, lambda);<br>
-       for (; i < XY - X; i++)<br>
+               lambda[i] = std::clamp(lambda[i], min, max);<br>
+       }<br>
+       for (; i < XY - X; i++) {<br>
                lambda[i] = compute_lambda_interior(i, M, lambda);<br>
-       for (; i < XY - 1; i++)<br>
+               lambda[i] = std::clamp(lambda[i], min, max);<br>
+       }<br>
+       for (; i < XY - 1; i++) {<br>
                lambda[i] = compute_lambda_top(i, M, lambda);<br>
+               lambda[i] = std::clamp(lambda[i], min, max);<br>
+       }<br>
        lambda[i] = compute_lambda_top_end(i, M, lambda);<br>
+       lambda[i] = std::clamp(lambda[i], min, max);<br>
        // Also solve the system from bottom to top, to help spread the updates<br>
        // better.<br>
        lambda[i] = compute_lambda_top_end(i, M, lambda);<br>
-       for (i = XY - 2; i >= XY - X; i--)<br>
+       lambda[i] = std::clamp(lambda[i], min, max);<br>
+       for (i = XY - 2; i >= XY - X; i--) {<br>
                lambda[i] = compute_lambda_top(i, M, lambda);<br>
-       for (; i >= X; i--)<br>
+               lambda[i] = std::clamp(lambda[i], min, max);<br>
+       }<br>
+       for (; i >= X; i--) {<br>
                lambda[i] = compute_lambda_interior(i, M, lambda);<br>
-       for (; i >= 1; i--)<br>
+               lambda[i] = std::clamp(lambda[i], min, max);<br>
+       }<br>
+       for (; i >= 1; i--) {<br>
                lambda[i] = compute_lambda_bottom(i, M, lambda);<br>
+               lambda[i] = std::clamp(lambda[i], min, max);<br>
+       }<br>
        lambda[0] = compute_lambda_bottom_start(0, M, lambda);<br>
+       lambda[0] = std::clamp(lambda[0], min, max);<br>
        double max_diff = 0;<br>
        for (i = 0; i < XY; i++) {<br>
                lambda[i] = old_lambda[i] + (lambda[i] - old_lambda[i]) * omega;<br>
@@ -653,15 +674,24 @@ static void normalise(double *ptr, size_t n)<br>
                ptr[i] /= minval;<br>
 }<br>
<br>
+// Rescale the values so that the average value is 1.<br>
+static void reaverage(Span<double> data)<br>
+{<br>
+       double sum = std::accumulate(data.begin(), data.end(), 0.0);<br>
+       double ratio = 1 / (sum / data.size());<br>
+       for (double &d : data)<br>
+               d *= ratio;<br>
+}<br>
+<br>
 static void run_matrix_iterations(double const C[XY], double lambda[XY],<br>
                                  double const W[XY][4], double omega,<br>
-                                 int n_iter, double threshold)<br>
+                                 int n_iter, double threshold, double lambda_bound)<br>
 {<br>
        double M[XY][4];<br>
        construct_M(C, W, M);<br>
        double last_max_diff = std::numeric_limits<double>::max();<br>
        for (int i = 0; i < n_iter; i++) {<br>
-               double max_diff = fabs(gauss_seidel2_SOR(M, omega, lambda));<br>
+               double max_diff = fabs(gauss_seidel2_SOR(M, omega, lambda, lambda_bound));<br>
                if (max_diff < threshold) {<br>
                        LOG(RPiAlsc, Debug)<br>
                                << "Stop after " << i + 1 << " iterations";<br>
@@ -675,10 +705,8 @@ static void run_matrix_iterations(double const C[XY], double lambda[XY],<br>
                                << last_max_diff << " to " << max_diff;<br>
                last_max_diff = max_diff;<br>
        }<br>
-       // We're going to normalise the lambdas so the smallest is 1. Not sure<br>
-       // this is really necessary as they get renormalised later, but I<br>
-       // suppose it does stop these quantities from wandering off...<br>
-       normalise(lambda, XY);<br>
+       // We're going to normalise the lambdas so the total average is 1.<br>
+       reaverage({ lambda, XY });<br>
 }<br>
<br>
 static void add_luminance_rb(double result[XY], double const lambda[XY],<br>
@@ -737,9 +765,9 @@ void Alsc::doAlsc()<br>
        compute_W(Cb, config_.sigma_Cb, Wb);<br>
        // Run Gauss-Seidel iterations over the resulting matrix, for R and B.<br>
        run_matrix_iterations(Cr, lambda_r_, Wr, config_.omega, config_.n_iter,<br>
-                             config_.threshold);<br>
+                             config_.threshold, config_.lambda_bound);<br>
        run_matrix_iterations(Cb, lambda_b_, Wb, config_.omega, config_.n_iter,<br>
-                             config_.threshold);<br>
+                             config_.threshold, config_.lambda_bound);<br>
        // Fold the calibrated gains into our final lambda values. (Note that on<br>
        // the next run, we re-start with the lambda values that don't have the<br>
        // calibration gains included.)<br>
diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.hpp b/src/ipa/raspberrypi/controller/rpi/alsc.hpp<br>
index 9616b99ea7ca..d1dbe0d1d22d 100644<br>
--- a/src/ipa/raspberrypi/controller/rpi/alsc.hpp<br>
+++ b/src/ipa/raspberrypi/controller/rpi/alsc.hpp<br>
@@ -41,6 +41,7 @@ struct AlscConfig {<br>
        std::vector<AlscCalibration> calibrations_Cb;<br>
        double default_ct; // colour temperature if no metadata found<br>
        double threshold; // iteration termination threshold<br>
+       double lambda_bound; // upper/lower bound for lambda from a value of 1<br>
 };<br>
<br>
 class Alsc : public Algorithm<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
<br>
</blockquote></div></div>