[PATCH v2 9/9] ipa: rkisp1: Add polynomial LSC loader

[Kieran Bingham]

Quoting Stefan Klug (2024-09-13 08:57:27)
> Add a loader that is capable to load polynomial coefficients from the
> tuning files. The polynoms are sampled at load time to reduce the

I think 'polynom' might be a german term? I haven't heard it used before
and google suddenly started giving me lots of non-english hits.

Perhaps just "The polynomials are sampled" ... but I understood it
anyway so I don't mind keeping it ;-)

> computational overhead at runtime.
> 
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> ---
>  src/ipa/rkisp1/algorithms/lsc.cpp | 126 +++++++++++++++++++++++++++++-
>  1 file changed, 125 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
> index 12867b8f7d0f..b0ad2234cd11 100644
> --- a/src/ipa/rkisp1/algorithms/lsc.cpp
> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp
> @@ -16,6 +16,7 @@
>  
>  #include "libcamera/internal/yaml_parser.h"
>  
> +#include "libipa/lsc_polynomial.h"
>  #include "linux/rkisp1-config.h"
>  
>  /**
> @@ -70,6 +71,123 @@ namespace ipa::rkisp1::algorithms {
>  
>  LOG_DEFINE_CATEGORY(RkISP1Lsc)
>  
> +class LscPolynomialLoader
> +{
> +public:
> +       LscPolynomialLoader(const Size &sensorSize,
> +                           const Rectangle &cropRectangle,
> +                           const std::vector<double> &xSizes,
> +                           const std::vector<double> &ySizes)
> +               : sensorSize_(sensorSize),
> +                 cropRectangle_(cropRectangle),
> +                 xSizes_(xSizes),
> +                 ySizes_(ySizes)
> +       {
> +       }
> +
> +       int parseLscData(const YamlObject &yamlSets,
> +                        std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> +       {
> +               const auto &sets = yamlSets.asList();
> +               for (const auto &yamlSet : sets) {
> +                       std::optional<LscPolynomial> pr, pgr, pgb, pb;
> +                       uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> +
> +                       if (lscData.count(ct)) {
> +                               LOG(RkISP1Lsc, Error)
> +                                       << "Multiple sets found for "
> +                                       << "color temperature " << ct;
> +                               return -EINVAL;
> +                       }
> +
> +                       LensShadingCorrection::Components &set = lscData[ct];
> +                       pr = yamlSet["r"].get<LscPolynomial>();
> +                       pgr = yamlSet["gr"].get<LscPolynomial>();
> +                       pgb = yamlSet["gb"].get<LscPolynomial>();
> +                       pb = yamlSet["b"].get<LscPolynomial>();
> +
> +                       if (!(pr || pgr || pgb || pb)) {
> +                               LOG(RkISP1Lsc, Error)
> +                                       << "Failed to parse polynomial for "
> +                                       << "colour temperature " << ct;
> +                               return -EINVAL;
> +                       }
> +
> +                       set.ct = ct;
> +                       pr->setReferenceImageSize(sensorSize_);
> +                       pgr->setReferenceImageSize(sensorSize_);
> +                       pgb->setReferenceImageSize(sensorSize_);
> +                       pb->setReferenceImageSize(sensorSize_);
> +                       set.r = samplePolynomial(*pr);
> +                       set.gr = samplePolynomial(*pgr);
> +                       set.gb = samplePolynomial(*pgb);
> +                       set.b = samplePolynomial(*pb);
> +               }
> +
> +               if (lscData.empty()) {
> +                       LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> +                       return -EINVAL;
> +               }
> +
> +               return 0;
> +       }
> +
> +private:

It's private implementation detail, but this might be a good place to
add a comment or explicit reference to any external documentation that
details the mirroring going on here.

I was confused to start with ...

> +       std::vector<double> sizesListToPositions(const std::vector<double> &sizes)
> +       {
> +               const int half = sizes.size();
> +               std::vector<double> res(half * 2 + 1);
> +               double x = 0.0;
> +
> +               res[half] = 0.5;
> +               for (int i = 1; i <= half; i++) {
> +                       x += sizes[half - i];
> +                       res[half - i] = 0.5 - x;
> +                       res[half + i] = 0.5 + x;
> +               }
> +
> +               return res;
> +       }
> +
> +       std::vector<uint16_t> samplePolynomial(const LscPolynomial &poly)
> +       {
> +               constexpr int k = RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> +
> +               double m = poly.getM();
> +               double x0 = cropRectangle_.x / m;
> +               double y0 = cropRectangle_.y / m;
> +               double w = cropRectangle_.width / m;
> +               double h = cropRectangle_.height / m;
> +               std::vector<uint16_t> res;
> +
> +               assert(xSizes_.size() * 2 + 1 == k);

Do we evaluate/validate this when loading ?

> +
> +               res.reserve(k * k);
> +
> +               std::vector<double> xPos(sizesListToPositions(xSizes_));
> +               std::vector<double> yPos(sizesListToPositions(ySizes_));
> +
> +               for (int x = 0; x < k; x++) {
> +                       for (int y = 0; y < k; y++) {
> +                               double xp = x0 + xPos[x] * w;
> +                               double yp = y0 + yPos[y] * h;
> +                               int v = static_cast<int>(
> +                                       poly.sampleAtNormalizedPixelPos(xp, yp) *
> +                                       1024);
> +
> +                               v = std::min(std::max(v, 1024), 4095);
> +                               res.push_back(v);
> +                       }
> +               }
> +               return res;
> +       }
> +
> +       Size sensorSize_;
> +       Rectangle cropRectangle_;
> +       const std::vector<double> &xSizes_;
> +       const std::vector<double> &ySizes_;
> +};
> +
>  class LscClassicLoader
>  {
>  public:
> @@ -193,11 +311,17 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
>         std::map<unsigned int, Components> lscData;
>         int res = 0;
>         std::optional<std::string> type = tuningData["type"].get<std::string>();
> -       if (!type.has_value()) {
> +       if (!tuningData.contains("type")) {

Does this need to change? We're not removing type variable so we can
still use it in both cases right?

>                 LOG(RkISP1Lsc, Warning) << "LSC data is in classic format. "
>                                         << "This will be deprecated soon.";
>                 auto loader = LscClassicLoader();
>                 res = loader.parseLscData(yamlSets, lscData);
> +       } else if (*type == "polynomial") {
> +               auto loader = LscPolynomialLoader(context.sensorInfo.activeAreaSize,
> +                                                 context.sensorInfo.analogCrop,
> +                                                 xSize_,
> +                                                 ySize_);
> +               res = loader.parseLscData(yamlSets, lscData);
>         } else {
>                 LOG(RkISP1Lsc, Error) << "Unsupported LSC type '" << *type << "'";
>                 res = -EINVAL;
> -- 
> 2.43.0
>