[{"id":31206,"web_url":"https://patchwork.libcamera.org/comment/31206/","msgid":"<172622090109.3474483.6506921137362980982@ping.linuxembedded.co.uk>","date":"2024-09-13T09:48:21","subject":"Re: [PATCH v2 5/9] ipa: rkisp1: Use interpolator in lsc","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Stefan Klug (2024-09-13 08:57:23)\n> Now, that the generic interpolator is available, use it to do the\n> interpolation of the lens shading tables. This makes the algorithm\n> easier to read and remove some duplicate code.\n> \n> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> ---\n>  src/ipa/rkisp1/algorithms/lsc.cpp | 166 +++++++++---------------------\n>  src/ipa/rkisp1/algorithms/lsc.h   |  13 +--\n>  2 files changed, 57 insertions(+), 122 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> index 5f3a0388075b..87a04ec048f8 100644\n> --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> @@ -24,6 +24,36 @@\n>  \n>  namespace libcamera {\n>  \n> +namespace ipa {\n> +\n> +constexpr int kColourTemperatureChangeThreshhold = 10;\n> +\n> +template<typename T>\n> +void interpolateVector(const std::vector<T> &a, const std::vector<T> &b,\n> +                      std::vector<T> &dest, double lambda)\n> +{\n> +       assert(a.size() == b.size());\n\nI wonder if we should do\n\tif (a.size() != b.size()\n\t\tLOG(RkISP1Lsc, Fatal) << \"Interpolation must be identical sizes\";\n\nI assume this should never be possible to happen even if people have\n'faulty' tuning files?\n\nBut if that's the case, I guess it would have to be handled at a higher\nlayer than the core interpolate implementation.\n\n\n> +       dest.resize(a.size());\n> +       for (size_t i = 0; i < a.size(); i++) {\n> +               dest[i] = a[i] * (1.0 - lambda) + b[i] * lambda;\n> +       }\n> +}\n> +\n> +template<>\n> +void Interpolator<rkisp1::algorithms::LensShadingCorrection::Components>::\n> +       interpolate(const rkisp1::algorithms::LensShadingCorrection::Components &a,\n> +                   const rkisp1::algorithms::LensShadingCorrection::Components &b,\n> +                   rkisp1::algorithms::LensShadingCorrection::Components &dest,\n> +                   double lambda)\n> +{\n> +       interpolateVector(a.r, b.r, dest.r, lambda);\n> +       interpolateVector(a.gr, b.gr, dest.gr, lambda);\n> +       interpolateVector(a.gb, b.gb, dest.gb, lambda);\n> +       interpolateVector(a.b, b.b, dest.b, lambda);\n> +}\n\nI like how easy it is to make custom interpolators now.\n\n> +\n> +} // namespace ipa\n> +\n>  namespace ipa::rkisp1::algorithms {\n>  \n>  /**\n> @@ -90,8 +120,9 @@ static std::vector<uint16_t> parseTable(const YamlObject &tuningData,\n>  }\n>  \n>  LensShadingCorrection::LensShadingCorrection()\n> -       : lastCt_({ 0, 0 })\n> +       : lastAppliedCt_(0), lastAppliedQuantizedCt_(0)\n>  {\n> +       sets_.setQuantization(kColourTemperatureChangeThreshhold);\n>  }\n>  \n>  /**\n> @@ -115,17 +146,18 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n>         }\n>  \n>         const auto &sets = yamlSets.asList();\n> +       std::map<unsigned int, Components> lscData;\n>         for (const auto &yamlSet : sets) {\n>                 uint32_t ct = yamlSet[\"ct\"].get<uint32_t>(0);\n>  \n> -               if (sets_.count(ct)) {\n> +               if (lscData.count(ct)) {\n>                         LOG(RkISP1Lsc, Error)\n>                                 << \"Multiple sets found for color temperature \"\n>                                 << ct;\n>                         return -EINVAL;\n>                 }\n>  \n> -               Components &set = sets_[ct];\n> +               Components &set = lscData[ct];\n>  \n\nIs this where we can do some validation on the input data to\nensure/guarantee we don't assert later on ? Or reject the file and\nreport to the user?\n\nWorrying about input validation is the only issue blocking me here.\n\n\n\n>                 set.ct = ct;\n>                 set.r = parseTable(yamlSet, \"r\");\n> @@ -142,11 +174,13 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n>                 }\n>         }\n>  \n> -       if (sets_.empty()) {\n> +       if (lscData.empty()) {\n>                 LOG(RkISP1Lsc, Error) << \"Failed to load any sets\";\n>                 return -EINVAL;\n>         }\n>  \n> +       sets_.setData(std::move(lscData));\n> +\n>         return 0;\n>  }\n>  \n> @@ -202,134 +236,34 @@ void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config,\n>         std::copy(set.b.begin(), set.b.end(), &config.b_data_tbl[0][0]);\n>  }\n>  \n> -/*\n> - * Interpolate LSC parameters based on color temperature value.\n> - */\n> -void LensShadingCorrection::interpolateTable(rkisp1_cif_isp_lsc_config &config,\n> -                                            const Components &set0,\n> -                                            const Components &set1,\n> -                                            const uint32_t ct)\n> -{\n> -       double coeff0 = (set1.ct - ct) / static_cast<double>(set1.ct - set0.ct);\n> -       double coeff1 = (ct - set0.ct) / static_cast<double>(set1.ct - set0.ct);\n> -\n> -       for (unsigned int i = 0; i < RKISP1_CIF_ISP_LSC_SAMPLES_MAX; ++i) {\n> -               for (unsigned int j = 0; j < RKISP1_CIF_ISP_LSC_SAMPLES_MAX; ++j) {\n> -                       unsigned int sample = i * RKISP1_CIF_ISP_LSC_SAMPLES_MAX + j;\n> -\n> -                       config.r_data_tbl[i][j] =\n> -                               set0.r[sample] * coeff0 +\n> -                               set1.r[sample] * coeff1;\n> -\n> -                       config.gr_data_tbl[i][j] =\n> -                               set0.gr[sample] * coeff0 +\n> -                               set1.gr[sample] * coeff1;\n> -\n> -                       config.gb_data_tbl[i][j] =\n> -                               set0.gb[sample] * coeff0 +\n> -                               set1.gb[sample] * coeff1;\n> -\n> -                       config.b_data_tbl[i][j] =\n> -                               set0.b[sample] * coeff0 +\n> -                               set1.b[sample] * coeff1;\n> -               }\n> -       }\n> -}\n> -\n>  /**\n>   * \\copydoc libcamera::ipa::Algorithm::prepare\n>   */\n>  void LensShadingCorrection::prepare(IPAContext &context,\n> -                                   const uint32_t frame,\n> +                                   [[maybe_unused]] const uint32_t frame,\n>                                     [[maybe_unused]] IPAFrameContext &frameContext,\n>                                     RkISP1Params *params)\n>  {\n> -       /*\n> -        * If there is only one set, the configuration has already been done\n> -        * for first frame.\n> -        */\n> -       if (sets_.size() == 1 && frame > 0)\n> -               return;\n> -\n> -       /*\n> -        * If there is only one set, pick it. We can ignore lastCt_, as it will\n> -        * never be relevant.\n> -        */\n> -       if (sets_.size() == 1) {\n> -               auto config = params->block<BlockType::Lsc>();\n> -               config.setEnabled(true);\n> -\n> -               setParameters(*config);\n> -               copyTable(*config, sets_.cbegin()->second);\n> -               return;\n> -       }\n> -\n>         uint32_t ct = context.activeState.awb.temperatureK;\n> -       ct = std::clamp(ct, sets_.cbegin()->first, sets_.crbegin()->first);\n> -\n> -       /*\n> -        * If the original is the same, then it means the same adjustment would\n> -        * be made. If the adjusted is the same, then it means that it's the\n> -        * same as what was actually applied. Thus in these cases we can skip\n> -        * reprogramming the LSC.\n> -        *\n> -        * original == adjusted can only happen if an interpolation\n> -        * happened, or if original has an exact entry in sets_. This means\n> -        * that if original != adjusted, then original was adjusted to\n> -        * the nearest available entry in sets_, resulting in adjusted.\n> -        * Clearly, any ct value that is in between original and adjusted\n> -        * will be adjusted to the same adjusted value, so we can skip\n> -        * reprogramming the LSC table.\n> -        *\n> -        * We also skip updating the original value, as the last one had a\n> -        * larger bound and thus a larger range of ct values that will be\n> -        * adjusted to the same adjusted.\n> -        */\n> -       if ((lastCt_.original <= ct && ct <= lastCt_.adjusted) ||\n> -           (lastCt_.adjusted <= ct && ct <= lastCt_.original))\n> +       if (std::abs(static_cast<int>(ct) - static_cast<int>(lastAppliedCt_)) <\n> +           kColourTemperatureChangeThreshhold)\n> +               return;\n> +       unsigned int quantizedCt;\n> +       const Components &set = sets_.getInterpolated(ct, &quantizedCt);\n> +       if (lastAppliedQuantizedCt_ == quantizedCt)\n>                 return;\n>  \n>         auto config = params->block<BlockType::Lsc>();\n>         config.setEnabled(true);\n>         setParameters(*config);\n> +       copyTable(*config, set);\n>  \n> -       /*\n> -        * The color temperature matches exactly one of the available LSC tables.\n> -        */\n> -       if (sets_.count(ct)) {\n> -               copyTable(*config, sets_[ct]);\n> -               lastCt_ = { ct, ct };\n> -               return;\n> -       }\n> +       lastAppliedCt_ = ct;\n> +       lastAppliedQuantizedCt_ = quantizedCt;\n>  \n> -       /* No shortcuts left; we need to round or interpolate */\n> -       auto iter = sets_.upper_bound(ct);\n> -       const Components &set1 = iter->second;\n> -       const Components &set0 = (--iter)->second;\n> -       uint32_t ct0 = set0.ct;\n> -       uint32_t ct1 = set1.ct;\n> -       uint32_t diff0 = ct - ct0;\n> -       uint32_t diff1 = ct1 - ct;\n> -       static constexpr double kThreshold = 0.1;\n> -       float threshold = kThreshold * (ct1 - ct0);\n> -\n> -       if (diff0 < threshold || diff1 < threshold) {\n> -               const Components &set = diff0 < diff1 ? set0 : set1;\n> -               LOG(RkISP1Lsc, Debug) << \"using LSC table for \" << set.ct;\n> -               copyTable(*config, set);\n> -               lastCt_ = { ct, set.ct };\n> -               return;\n> -       }\n> -\n> -       /*\n> -        * ct is not within 10% of the difference between the neighbouring\n> -        * color temperatures, so we need to interpolate.\n> -        */\n>         LOG(RkISP1Lsc, Debug)\n> -               << \"ct is \" << ct << \", interpolating between \"\n> -               << ct0 << \" and \" << ct1;\n> -       interpolateTable(*config, set0, set1, ct);\n> -       lastCt_ = { ct, ct };\n> +               << \"ct is \" << ct << \", quantized to \"\n> +               << quantizedCt;\n>  }\n>  \n>  REGISTER_IPA_ALGORITHM(LensShadingCorrection, \"LensShadingCorrection\")\n> diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h\n> index a9c7a230e0fc..5a0824e36dd5 100644\n> --- a/src/ipa/rkisp1/algorithms/lsc.h\n> +++ b/src/ipa/rkisp1/algorithms/lsc.h\n> @@ -9,6 +9,8 @@\n>  \n>  #include <map>\n>  \n> +#include \"libipa/interpolator.h\"\n> +\n>  #include \"algorithm.h\"\n>  \n>  namespace libcamera {\n> @@ -27,7 +29,6 @@ public:\n>                      IPAFrameContext &frameContext,\n>                      RkISP1Params *params) override;\n>  \n> -private:\n>         struct Components {\n>                 uint32_t ct;\n>                 std::vector<uint16_t> r;\n> @@ -36,23 +37,23 @@ private:\n>                 std::vector<uint16_t> b;\n>         };\n>  \n> +private:\n>         void setParameters(rkisp1_cif_isp_lsc_config &config);\n>         void copyTable(rkisp1_cif_isp_lsc_config &config, const Components &set0);\n>         void interpolateTable(rkisp1_cif_isp_lsc_config &config,\n>                               const Components &set0, const Components &set1,\n>                               const uint32_t ct);\n>  \n> -       std::map<uint32_t, Components> sets_;\n> +       ipa::Interpolator<Components> sets_;\n>         std::vector<double> xSize_;\n>         std::vector<double> ySize_;\n>         uint16_t xGrad_[RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE];\n>         uint16_t yGrad_[RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE];\n>         uint16_t xSizes_[RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE];\n>         uint16_t ySizes_[RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE];\n> -       struct {\n> -               uint32_t original;\n> -               uint32_t adjusted;\n> -       } lastCt_;\n> +\n> +       unsigned int lastAppliedCt_;\n> +       unsigned int lastAppliedQuantizedCt_;\n>  };\n>  \n>  } /* namespace ipa::rkisp1::algorithms */\n> -- \n> 2.43.0\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id BE2EDC3257\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 13 Sep 2024 09:48:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A15E6634F4;\n\tFri, 13 Sep 2024 11:48:26 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F1E38634F4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 13 Sep 2024 11:48:24 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 89E7A1083;\n\tFri, 13 Sep 2024 11:47:05 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"nr2NTebR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1726220825;\n\tbh=G4NE2OQyLvMqnm9hfzcuen3URUBUHRoXbu44+TS9jew=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=nr2NTebRX/ssQfTv424r2SSnnV91PnDDWhyKHFw1Ihwb3bdAb60e4ZyBQSWOK/L6B\n\tAa0YACMjXtsZ1mywRNwQPNy5kvts3dZncLj5nRrul1ylBxo2QIJwmbqpwuoViu6nLY\n\th2OzmqiFg8HqVV1WxkyFN/OaD3ZxgXIgW3Q7AbM0=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240913075750.35115-6-stefan.klug@ideasonboard.com>","References":"<20240913075750.35115-1-stefan.klug@ideasonboard.com>\n\t<20240913075750.35115-6-stefan.klug@ideasonboard.com>","Subject":"Re: [PATCH v2 5/9] ipa: rkisp1: Use interpolator in lsc","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 13 Sep 2024 10:48:21 +0100","Message-ID":"<172622090109.3474483.6506921137362980982@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":31208,"web_url":"https://patchwork.libcamera.org/comment/31208/","msgid":"<172622139983.3474483.5893832578255111620@ping.linuxembedded.co.uk>","date":"2024-09-13T09:56:39","subject":"Re: [PATCH v2 5/9] ipa: rkisp1: Use interpolator in lsc","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Kieran Bingham (2024-09-13 10:48:21)\n> Quoting Stefan Klug (2024-09-13 08:57:23)\n> > Now, that the generic interpolator is available, use it to do the\n> > interpolation of the lens shading tables. This makes the algorithm\n> > easier to read and remove some duplicate code.\n> > \n> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> > ---\n> >  src/ipa/rkisp1/algorithms/lsc.cpp | 166 +++++++++---------------------\n> >  src/ipa/rkisp1/algorithms/lsc.h   |  13 +--\n> >  2 files changed, 57 insertions(+), 122 deletions(-)\n> > \n> > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > index 5f3a0388075b..87a04ec048f8 100644\n> > --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > @@ -24,6 +24,36 @@\n> >  \n> >  namespace libcamera {\n> >  \n> > +namespace ipa {\n> > +\n> > +constexpr int kColourTemperatureChangeThreshhold = 10;\n> > +\n> > +template<typename T>\n> > +void interpolateVector(const std::vector<T> &a, const std::vector<T> &b,\n> > +                      std::vector<T> &dest, double lambda)\n> > +{\n> > +       assert(a.size() == b.size());\n> \n> I wonder if we should do\n>         if (a.size() != b.size()\n>                 LOG(RkISP1Lsc, Fatal) << \"Interpolation must be identical sizes\";\n> \n> I assume this should never be possible to happen even if people have\n> 'faulty' tuning files?\n> \n> But if that's the case, I guess it would have to be handled at a higher\n> layer than the core interpolate implementation.\n\nI think I've convinced myself that this is covered by the YAML loader -\nso a plain assert is fine to enforce the contract.\n\n> \n> \n> > +       dest.resize(a.size());\n> > +       for (size_t i = 0; i < a.size(); i++) {\n> > +               dest[i] = a[i] * (1.0 - lambda) + b[i] * lambda;\n> > +       }\n> > +}\n> > +\n> > +template<>\n> > +void Interpolator<rkisp1::algorithms::LensShadingCorrection::Components>::\n> > +       interpolate(const rkisp1::algorithms::LensShadingCorrection::Components &a,\n> > +                   const rkisp1::algorithms::LensShadingCorrection::Components &b,\n> > +                   rkisp1::algorithms::LensShadingCorrection::Components &dest,\n> > +                   double lambda)\n> > +{\n> > +       interpolateVector(a.r, b.r, dest.r, lambda);\n> > +       interpolateVector(a.gr, b.gr, dest.gr, lambda);\n> > +       interpolateVector(a.gb, b.gb, dest.gb, lambda);\n> > +       interpolateVector(a.b, b.b, dest.b, lambda);\n> > +}\n> \n> I like how easy it is to make custom interpolators now.\n> \n> > +\n> > +} // namespace ipa\n> > +\n> >  namespace ipa::rkisp1::algorithms {\n> >  \n> >  /**\n> > @@ -90,8 +120,9 @@ static std::vector<uint16_t> parseTable(const YamlObject &tuningData,\n> >  }\n> >  \n> >  LensShadingCorrection::LensShadingCorrection()\n> > -       : lastCt_({ 0, 0 })\n> > +       : lastAppliedCt_(0), lastAppliedQuantizedCt_(0)\n> >  {\n> > +       sets_.setQuantization(kColourTemperatureChangeThreshhold);\n> >  }\n> >  \n> >  /**\n> > @@ -115,17 +146,18 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n> >         }\n> >  \n> >         const auto &sets = yamlSets.asList();\n> > +       std::map<unsigned int, Components> lscData;\n> >         for (const auto &yamlSet : sets) {\n> >                 uint32_t ct = yamlSet[\"ct\"].get<uint32_t>(0);\n> >  \n> > -               if (sets_.count(ct)) {\n> > +               if (lscData.count(ct)) {\n> >                         LOG(RkISP1Lsc, Error)\n> >                                 << \"Multiple sets found for color temperature \"\n> >                                 << ct;\n> >                         return -EINVAL;\n> >                 }\n> >  \n> > -               Components &set = sets_[ct];\n> > +               Components &set = lscData[ct];\n> >  \n> \n> Is this where we can do some validation on the input data to\n> ensure/guarantee we don't assert later on ? Or reject the file and\n> report to the user?\n> \n> Worrying about input validation is the only issue blocking me here.\n> \n> \n\nAnd I think I didn't see it in this patch as the yaml loader isn't\naffected, but I have convinced myself that this is safe because it's\nhandled there.\n\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> \n> >                 set.ct = ct;\n> >                 set.r = parseTable(yamlSet, \"r\");\n> > @@ -142,11 +174,13 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n> >                 }\n> >         }\n> >  \n> > -       if (sets_.empty()) {\n> > +       if (lscData.empty()) {\n> >                 LOG(RkISP1Lsc, Error) << \"Failed to load any sets\";\n> >                 return -EINVAL;\n> >         }\n> >  \n> > +       sets_.setData(std::move(lscData));\n> > +\n> >         return 0;\n> >  }\n> >  \n> > @@ -202,134 +236,34 @@ void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config,\n> >         std::copy(set.b.begin(), set.b.end(), &config.b_data_tbl[0][0]);\n> >  }\n> >  \n> > -/*\n> > - * Interpolate LSC parameters based on color temperature value.\n> > - */\n> > -void LensShadingCorrection::interpolateTable(rkisp1_cif_isp_lsc_config &config,\n> > -                                            const Components &set0,\n> > -                                            const Components &set1,\n> > -                                            const uint32_t ct)\n> > -{\n> > -       double coeff0 = (set1.ct - ct) / static_cast<double>(set1.ct - set0.ct);\n> > -       double coeff1 = (ct - set0.ct) / static_cast<double>(set1.ct - set0.ct);\n> > -\n> > -       for (unsigned int i = 0; i < RKISP1_CIF_ISP_LSC_SAMPLES_MAX; ++i) {\n> > -               for (unsigned int j = 0; j < RKISP1_CIF_ISP_LSC_SAMPLES_MAX; ++j) {\n> > -                       unsigned int sample = i * RKISP1_CIF_ISP_LSC_SAMPLES_MAX + j;\n> > -\n> > -                       config.r_data_tbl[i][j] =\n> > -                               set0.r[sample] * coeff0 +\n> > -                               set1.r[sample] * coeff1;\n> > -\n> > -                       config.gr_data_tbl[i][j] =\n> > -                               set0.gr[sample] * coeff0 +\n> > -                               set1.gr[sample] * coeff1;\n> > -\n> > -                       config.gb_data_tbl[i][j] =\n> > -                               set0.gb[sample] * coeff0 +\n> > -                               set1.gb[sample] * coeff1;\n> > -\n> > -                       config.b_data_tbl[i][j] =\n> > -                               set0.b[sample] * coeff0 +\n> > -                               set1.b[sample] * coeff1;\n> > -               }\n> > -       }\n> > -}\n> > -\n> >  /**\n> >   * \\copydoc libcamera::ipa::Algorithm::prepare\n> >   */\n> >  void LensShadingCorrection::prepare(IPAContext &context,\n> > -                                   const uint32_t frame,\n> > +                                   [[maybe_unused]] const uint32_t frame,\n> >                                     [[maybe_unused]] IPAFrameContext &frameContext,\n> >                                     RkISP1Params *params)\n> >  {\n> > -       /*\n> > -        * If there is only one set, the configuration has already been done\n> > -        * for first frame.\n> > -        */\n> > -       if (sets_.size() == 1 && frame > 0)\n> > -               return;\n> > -\n> > -       /*\n> > -        * If there is only one set, pick it. We can ignore lastCt_, as it will\n> > -        * never be relevant.\n> > -        */\n> > -       if (sets_.size() == 1) {\n> > -               auto config = params->block<BlockType::Lsc>();\n> > -               config.setEnabled(true);\n> > -\n> > -               setParameters(*config);\n> > -               copyTable(*config, sets_.cbegin()->second);\n> > -               return;\n> > -       }\n> > -\n> >         uint32_t ct = context.activeState.awb.temperatureK;\n> > -       ct = std::clamp(ct, sets_.cbegin()->first, sets_.crbegin()->first);\n> > -\n> > -       /*\n> > -        * If the original is the same, then it means the same adjustment would\n> > -        * be made. If the adjusted is the same, then it means that it's the\n> > -        * same as what was actually applied. Thus in these cases we can skip\n> > -        * reprogramming the LSC.\n> > -        *\n> > -        * original == adjusted can only happen if an interpolation\n> > -        * happened, or if original has an exact entry in sets_. This means\n> > -        * that if original != adjusted, then original was adjusted to\n> > -        * the nearest available entry in sets_, resulting in adjusted.\n> > -        * Clearly, any ct value that is in between original and adjusted\n> > -        * will be adjusted to the same adjusted value, so we can skip\n> > -        * reprogramming the LSC table.\n> > -        *\n> > -        * We also skip updating the original value, as the last one had a\n> > -        * larger bound and thus a larger range of ct values that will be\n> > -        * adjusted to the same adjusted.\n> > -        */\n> > -       if ((lastCt_.original <= ct && ct <= lastCt_.adjusted) ||\n> > -           (lastCt_.adjusted <= ct && ct <= lastCt_.original))\n> > +       if (std::abs(static_cast<int>(ct) - static_cast<int>(lastAppliedCt_)) <\n> > +           kColourTemperatureChangeThreshhold)\n> > +               return;\n> > +       unsigned int quantizedCt;\n> > +       const Components &set = sets_.getInterpolated(ct, &quantizedCt);\n> > +       if (lastAppliedQuantizedCt_ == quantizedCt)\n> >                 return;\n> >  \n> >         auto config = params->block<BlockType::Lsc>();\n> >         config.setEnabled(true);\n> >         setParameters(*config);\n> > +       copyTable(*config, set);\n> >  \n> > -       /*\n> > -        * The color temperature matches exactly one of the available LSC tables.\n> > -        */\n> > -       if (sets_.count(ct)) {\n> > -               copyTable(*config, sets_[ct]);\n> > -               lastCt_ = { ct, ct };\n> > -               return;\n> > -       }\n> > +       lastAppliedCt_ = ct;\n> > +       lastAppliedQuantizedCt_ = quantizedCt;\n> >  \n> > -       /* No shortcuts left; we need to round or interpolate */\n> > -       auto iter = sets_.upper_bound(ct);\n> > -       const Components &set1 = iter->second;\n> > -       const Components &set0 = (--iter)->second;\n> > -       uint32_t ct0 = set0.ct;\n> > -       uint32_t ct1 = set1.ct;\n> > -       uint32_t diff0 = ct - ct0;\n> > -       uint32_t diff1 = ct1 - ct;\n> > -       static constexpr double kThreshold = 0.1;\n> > -       float threshold = kThreshold * (ct1 - ct0);\n> > -\n> > -       if (diff0 < threshold || diff1 < threshold) {\n> > -               const Components &set = diff0 < diff1 ? set0 : set1;\n> > -               LOG(RkISP1Lsc, Debug) << \"using LSC table for \" << set.ct;\n> > -               copyTable(*config, set);\n> > -               lastCt_ = { ct, set.ct };\n> > -               return;\n> > -       }\n> > -\n> > -       /*\n> > -        * ct is not within 10% of the difference between the neighbouring\n> > -        * color temperatures, so we need to interpolate.\n> > -        */\n> >         LOG(RkISP1Lsc, Debug)\n> > -               << \"ct is \" << ct << \", interpolating between \"\n> > -               << ct0 << \" and \" << ct1;\n> > -       interpolateTable(*config, set0, set1, ct);\n> > -       lastCt_ = { ct, ct };\n> > +               << \"ct is \" << ct << \", quantized to \"\n> > +               << quantizedCt;\n> >  }\n> >  \n> >  REGISTER_IPA_ALGORITHM(LensShadingCorrection, \"LensShadingCorrection\")\n> > diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h\n> > index a9c7a230e0fc..5a0824e36dd5 100644\n> > --- a/src/ipa/rkisp1/algorithms/lsc.h\n> > +++ b/src/ipa/rkisp1/algorithms/lsc.h\n> > @@ -9,6 +9,8 @@\n> >  \n> >  #include <map>\n> >  \n> > +#include \"libipa/interpolator.h\"\n> > +\n> >  #include \"algorithm.h\"\n> >  \n> >  namespace libcamera {\n> > @@ -27,7 +29,6 @@ public:\n> >                      IPAFrameContext &frameContext,\n> >                      RkISP1Params *params) override;\n> >  \n> > -private:\n> >         struct Components {\n> >                 uint32_t ct;\n> >                 std::vector<uint16_t> r;\n> > @@ -36,23 +37,23 @@ private:\n> >                 std::vector<uint16_t> b;\n> >         };\n> >  \n> > +private:\n> >         void setParameters(rkisp1_cif_isp_lsc_config &config);\n> >         void copyTable(rkisp1_cif_isp_lsc_config &config, const Components &set0);\n> >         void interpolateTable(rkisp1_cif_isp_lsc_config &config,\n> >                               const Components &set0, const Components &set1,\n> >                               const uint32_t ct);\n> >  \n> > -       std::map<uint32_t, Components> sets_;\n> > +       ipa::Interpolator<Components> sets_;\n> >         std::vector<double> xSize_;\n> >         std::vector<double> ySize_;\n> >         uint16_t xGrad_[RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE];\n> >         uint16_t yGrad_[RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE];\n> >         uint16_t xSizes_[RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE];\n> >         uint16_t ySizes_[RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE];\n> > -       struct {\n> > -               uint32_t original;\n> > -               uint32_t adjusted;\n> > -       } lastCt_;\n> > +\n> > +       unsigned int lastAppliedCt_;\n> > +       unsigned int lastAppliedQuantizedCt_;\n> >  };\n> >  \n> >  } /* namespace ipa::rkisp1::algorithms */\n> > -- \n> > 2.43.0\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 012C7C324C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 13 Sep 2024 09:56:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AEA2463500;\n\tFri, 13 Sep 2024 11:56:44 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0E608634F5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 13 Sep 2024 11:56:43 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 309B49FF;\n\tFri, 13 Sep 2024 11:55:24 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"uxCkbCO+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1726221324;\n\tbh=Y7sTd/gvnyhpAcikRNb5Y8kCVnHdf2igvyodXv+hAq0=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=uxCkbCO+mpYR8hNRT1sHEoTx0OYgpZMmP2QWqPehYO3UK55G2Us/2fgNLEkF3OcWY\n\tK29M55u78SJdQq9LWkeW2UN+kKvGw5vwwtRHWii54risMxNlqz1n2UBySA7Fr4rBKZ\n\t2yHYnItDPEWIc+GuhsTxerowsTCS0IbQi5ywcHjE=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<172622090109.3474483.6506921137362980982@ping.linuxembedded.co.uk>","References":"<20240913075750.35115-1-stefan.klug@ideasonboard.com>\n\t<20240913075750.35115-6-stefan.klug@ideasonboard.com>\n\t<172622090109.3474483.6506921137362980982@ping.linuxembedded.co.uk>","Subject":"Re: [PATCH v2 5/9] ipa: rkisp1: Use interpolator in lsc","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 13 Sep 2024 10:56:39 +0100","Message-ID":"<172622139983.3474483.5893832578255111620@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":31214,"web_url":"https://patchwork.libcamera.org/comment/31214/","msgid":"<ZuQT9bvET04tRO8d@pyrite.rasen.tech>","date":"2024-09-13T10:29:09","subject":"Re: [PATCH v2 5/9] ipa: rkisp1: Use interpolator in lsc","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Fri, Sep 13, 2024 at 09:57:23AM +0200, Stefan Klug wrote:\n> Now, that the generic interpolator is available, use it to do the\n> interpolation of the lens shading tables. This makes the algorithm\n> easier to read and remove some duplicate code.\n> \n> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> ---\n>  src/ipa/rkisp1/algorithms/lsc.cpp | 166 +++++++++---------------------\n>  src/ipa/rkisp1/algorithms/lsc.h   |  13 +--\n>  2 files changed, 57 insertions(+), 122 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> index 5f3a0388075b..87a04ec048f8 100644\n> --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> @@ -24,6 +24,36 @@\n>  \n>  namespace libcamera {\n>  \n> +namespace ipa {\n> +\n> +constexpr int kColourTemperatureChangeThreshhold = 10;\n> +\n> +template<typename T>\n> +void interpolateVector(const std::vector<T> &a, const std::vector<T> &b,\n> +\t\t       std::vector<T> &dest, double lambda)\n> +{\n> +\tassert(a.size() == b.size());\n> +\tdest.resize(a.size());\n> +\tfor (size_t i = 0; i < a.size(); i++) {\n> +\t\tdest[i] = a[i] * (1.0 - lambda) + b[i] * lambda;\n> +\t}\n> +}\n> +\n> +template<>\n> +void Interpolator<rkisp1::algorithms::LensShadingCorrection::Components>::\n> +\tinterpolate(const rkisp1::algorithms::LensShadingCorrection::Components &a,\n> +\t\t    const rkisp1::algorithms::LensShadingCorrection::Components &b,\n> +\t\t    rkisp1::algorithms::LensShadingCorrection::Components &dest,\n> +\t\t    double lambda)\n> +{\n> +\tinterpolateVector(a.r, b.r, dest.r, lambda);\n> +\tinterpolateVector(a.gr, b.gr, dest.gr, lambda);\n> +\tinterpolateVector(a.gb, b.gb, dest.gb, lambda);\n> +\tinterpolateVector(a.b, b.b, dest.b, lambda);\n> +}\n> +\n> +} // namespace ipa\n\nI think this should be /* namespace ipa */\n\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\n> +\n>  namespace ipa::rkisp1::algorithms {\n>  \n>  /**\n> @@ -90,8 +120,9 @@ static std::vector<uint16_t> parseTable(const YamlObject &tuningData,\n>  }\n>  \n>  LensShadingCorrection::LensShadingCorrection()\n> -\t: lastCt_({ 0, 0 })\n> +\t: lastAppliedCt_(0), lastAppliedQuantizedCt_(0)\n>  {\n> +\tsets_.setQuantization(kColourTemperatureChangeThreshhold);\n>  }\n>  \n>  /**\n> @@ -115,17 +146,18 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n>  \t}\n>  \n>  \tconst auto &sets = yamlSets.asList();\n> +\tstd::map<unsigned int, Components> lscData;\n>  \tfor (const auto &yamlSet : sets) {\n>  \t\tuint32_t ct = yamlSet[\"ct\"].get<uint32_t>(0);\n>  \n> -\t\tif (sets_.count(ct)) {\n> +\t\tif (lscData.count(ct)) {\n>  \t\t\tLOG(RkISP1Lsc, Error)\n>  \t\t\t\t<< \"Multiple sets found for color temperature \"\n>  \t\t\t\t<< ct;\n>  \t\t\treturn -EINVAL;\n>  \t\t}\n>  \n> -\t\tComponents &set = sets_[ct];\n> +\t\tComponents &set = lscData[ct];\n>  \n>  \t\tset.ct = ct;\n>  \t\tset.r = parseTable(yamlSet, \"r\");\n> @@ -142,11 +174,13 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,\n>  \t\t}\n>  \t}\n>  \n> -\tif (sets_.empty()) {\n> +\tif (lscData.empty()) {\n>  \t\tLOG(RkISP1Lsc, Error) << \"Failed to load any sets\";\n>  \t\treturn -EINVAL;\n>  \t}\n>  \n> +\tsets_.setData(std::move(lscData));\n> +\n>  \treturn 0;\n>  }\n>  \n> @@ -202,134 +236,34 @@ void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config,\n>  \tstd::copy(set.b.begin(), set.b.end(), &config.b_data_tbl[0][0]);\n>  }\n>  \n> -/*\n> - * Interpolate LSC parameters based on color temperature value.\n> - */\n> -void LensShadingCorrection::interpolateTable(rkisp1_cif_isp_lsc_config &config,\n> -\t\t\t\t\t     const Components &set0,\n> -\t\t\t\t\t     const Components &set1,\n> -\t\t\t\t\t     const uint32_t ct)\n> -{\n> -\tdouble coeff0 = (set1.ct - ct) / static_cast<double>(set1.ct - set0.ct);\n> -\tdouble coeff1 = (ct - set0.ct) / static_cast<double>(set1.ct - set0.ct);\n> -\n> -\tfor (unsigned int i = 0; i < RKISP1_CIF_ISP_LSC_SAMPLES_MAX; ++i) {\n> -\t\tfor (unsigned int j = 0; j < RKISP1_CIF_ISP_LSC_SAMPLES_MAX; ++j) {\n> -\t\t\tunsigned int sample = i * RKISP1_CIF_ISP_LSC_SAMPLES_MAX + j;\n> -\n> -\t\t\tconfig.r_data_tbl[i][j] =\n> -\t\t\t\tset0.r[sample] * coeff0 +\n> -\t\t\t\tset1.r[sample] * coeff1;\n> -\n> -\t\t\tconfig.gr_data_tbl[i][j] =\n> -\t\t\t\tset0.gr[sample] * coeff0 +\n> -\t\t\t\tset1.gr[sample] * coeff1;\n> -\n> -\t\t\tconfig.gb_data_tbl[i][j] =\n> -\t\t\t\tset0.gb[sample] * coeff0 +\n> -\t\t\t\tset1.gb[sample] * coeff1;\n> -\n> -\t\t\tconfig.b_data_tbl[i][j] =\n> -\t\t\t\tset0.b[sample] * coeff0 +\n> -\t\t\t\tset1.b[sample] * coeff1;\n> -\t\t}\n> -\t}\n> -}\n> -\n>  /**\n>   * \\copydoc libcamera::ipa::Algorithm::prepare\n>   */\n>  void LensShadingCorrection::prepare(IPAContext &context,\n> -\t\t\t\t    const uint32_t frame,\n> +\t\t\t\t    [[maybe_unused]] const uint32_t frame,\n>  \t\t\t\t    [[maybe_unused]] IPAFrameContext &frameContext,\n>  \t\t\t\t    RkISP1Params *params)\n>  {\n> -\t/*\n> -\t * If there is only one set, the configuration has already been done\n> -\t * for first frame.\n> -\t */\n> -\tif (sets_.size() == 1 && frame > 0)\n> -\t\treturn;\n> -\n> -\t/*\n> -\t * If there is only one set, pick it. We can ignore lastCt_, as it will\n> -\t * never be relevant.\n> -\t */\n> -\tif (sets_.size() == 1) {\n> -\t\tauto config = params->block<BlockType::Lsc>();\n> -\t\tconfig.setEnabled(true);\n> -\n> -\t\tsetParameters(*config);\n> -\t\tcopyTable(*config, sets_.cbegin()->second);\n> -\t\treturn;\n> -\t}\n> -\n>  \tuint32_t ct = context.activeState.awb.temperatureK;\n> -\tct = std::clamp(ct, sets_.cbegin()->first, sets_.crbegin()->first);\n> -\n> -\t/*\n> -\t * If the original is the same, then it means the same adjustment would\n> -\t * be made. If the adjusted is the same, then it means that it's the\n> -\t * same as what was actually applied. Thus in these cases we can skip\n> -\t * reprogramming the LSC.\n> -\t *\n> -\t * original == adjusted can only happen if an interpolation\n> -\t * happened, or if original has an exact entry in sets_. This means\n> -\t * that if original != adjusted, then original was adjusted to\n> -\t * the nearest available entry in sets_, resulting in adjusted.\n> -\t * Clearly, any ct value that is in between original and adjusted\n> -\t * will be adjusted to the same adjusted value, so we can skip\n> -\t * reprogramming the LSC table.\n> -\t *\n> -\t * We also skip updating the original value, as the last one had a\n> -\t * larger bound and thus a larger range of ct values that will be\n> -\t * adjusted to the same adjusted.\n> -\t */\n> -\tif ((lastCt_.original <= ct && ct <= lastCt_.adjusted) ||\n> -\t    (lastCt_.adjusted <= ct && ct <= lastCt_.original))\n> +\tif (std::abs(static_cast<int>(ct) - static_cast<int>(lastAppliedCt_)) <\n> +\t    kColourTemperatureChangeThreshhold)\n> +\t\treturn;\n> +\tunsigned int quantizedCt;\n> +\tconst Components &set = sets_.getInterpolated(ct, &quantizedCt);\n> +\tif (lastAppliedQuantizedCt_ == quantizedCt)\n>  \t\treturn;\n>  \n>  \tauto config = params->block<BlockType::Lsc>();\n>  \tconfig.setEnabled(true);\n>  \tsetParameters(*config);\n> +\tcopyTable(*config, set);\n>  \n> -\t/*\n> -\t * The color temperature matches exactly one of the available LSC tables.\n> -\t */\n> -\tif (sets_.count(ct)) {\n> -\t\tcopyTable(*config, sets_[ct]);\n> -\t\tlastCt_ = { ct, ct };\n> -\t\treturn;\n> -\t}\n> +\tlastAppliedCt_ = ct;\n> +\tlastAppliedQuantizedCt_ = quantizedCt;\n>  \n> -\t/* No shortcuts left; we need to round or interpolate */\n> -\tauto iter = sets_.upper_bound(ct);\n> -\tconst Components &set1 = iter->second;\n> -\tconst Components &set0 = (--iter)->second;\n> -\tuint32_t ct0 = set0.ct;\n> -\tuint32_t ct1 = set1.ct;\n> -\tuint32_t diff0 = ct - ct0;\n> -\tuint32_t diff1 = ct1 - ct;\n> -\tstatic constexpr double kThreshold = 0.1;\n> -\tfloat threshold = kThreshold * (ct1 - ct0);\n> -\n> -\tif (diff0 < threshold || diff1 < threshold) {\n> -\t\tconst Components &set = diff0 < diff1 ? set0 : set1;\n> -\t\tLOG(RkISP1Lsc, Debug) << \"using LSC table for \" << set.ct;\n> -\t\tcopyTable(*config, set);\n> -\t\tlastCt_ = { ct, set.ct };\n> -\t\treturn;\n> -\t}\n> -\n> -\t/*\n> -\t * ct is not within 10% of the difference between the neighbouring\n> -\t * color temperatures, so we need to interpolate.\n> -\t */\n>  \tLOG(RkISP1Lsc, Debug)\n> -\t\t<< \"ct is \" << ct << \", interpolating between \"\n> -\t\t<< ct0 << \" and \" << ct1;\n> -\tinterpolateTable(*config, set0, set1, ct);\n> -\tlastCt_ = { ct, ct };\n> +\t\t<< \"ct is \" << ct << \", quantized to \"\n> +\t\t<< quantizedCt;\n>  }\n>  \n>  REGISTER_IPA_ALGORITHM(LensShadingCorrection, \"LensShadingCorrection\")\n> diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h\n> index a9c7a230e0fc..5a0824e36dd5 100644\n> --- a/src/ipa/rkisp1/algorithms/lsc.h\n> +++ b/src/ipa/rkisp1/algorithms/lsc.h\n> @@ -9,6 +9,8 @@\n>  \n>  #include <map>\n>  \n> +#include \"libipa/interpolator.h\"\n> +\n>  #include \"algorithm.h\"\n>  \n>  namespace libcamera {\n> @@ -27,7 +29,6 @@ public:\n>  \t\t     IPAFrameContext &frameContext,\n>  \t\t     RkISP1Params *params) override;\n>  \n> -private:\n>  \tstruct Components {\n>  \t\tuint32_t ct;\n>  \t\tstd::vector<uint16_t> r;\n> @@ -36,23 +37,23 @@ private:\n>  \t\tstd::vector<uint16_t> b;\n>  \t};\n>  \n> +private:\n>  \tvoid setParameters(rkisp1_cif_isp_lsc_config &config);\n>  \tvoid copyTable(rkisp1_cif_isp_lsc_config &config, const Components &set0);\n>  \tvoid interpolateTable(rkisp1_cif_isp_lsc_config &config,\n>  \t\t\t      const Components &set0, const Components &set1,\n>  \t\t\t      const uint32_t ct);\n>  \n> -\tstd::map<uint32_t, Components> sets_;\n> +\tipa::Interpolator<Components> sets_;\n>  \tstd::vector<double> xSize_;\n>  \tstd::vector<double> ySize_;\n>  \tuint16_t xGrad_[RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE];\n>  \tuint16_t yGrad_[RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE];\n>  \tuint16_t xSizes_[RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE];\n>  \tuint16_t ySizes_[RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE];\n> -\tstruct {\n> -\t\tuint32_t original;\n> -\t\tuint32_t adjusted;\n> -\t} lastCt_;\n> +\n> +\tunsigned int lastAppliedCt_;\n> +\tunsigned int lastAppliedQuantizedCt_;\n>  };\n>  \n>  } /* namespace ipa::rkisp1::algorithms */\n> -- \n> 2.43.0\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D09C6C3257\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 13 Sep 2024 10:29:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D2E70634FB;\n\tFri, 13 Sep 2024 12:29:14 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EB05A634E3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 13 Sep 2024 12:29:12 +0200 (CEST)","from pyrite.rasen.tech (213-229-8-243.static.upcbusiness.at\n\t[213.229.8.243])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E544BE0D;\n\tFri, 13 Sep 2024 12:27:53 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"dndtET2+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1726223274;\n\tbh=9+4lwzhA97JgPxevr6UtHY3qy1Mm/sx9zbwbAztmrEo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=dndtET2++jE2VBhXL2Iiujn0ZK5RjP17SGKpM8efsqNN8pNkaTr8TupyULatxvf6w\n\tidZVbxpN2IXLqHRzoIe1OmhNLNM/IVK03trPJnPaeeQ5q32l3ZRfF2WUv6PLke/jNR\n\tGY0V0XKPdxTibuARErOAefvL95dqqElEU9h7EI24=","Date":"Fri, 13 Sep 2024 12:29:09 +0200","From":"Paul Elder <paul.elder@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 5/9] ipa: rkisp1: Use interpolator in lsc","Message-ID":"<ZuQT9bvET04tRO8d@pyrite.rasen.tech>","References":"<20240913075750.35115-1-stefan.klug@ideasonboard.com>\n\t<20240913075750.35115-6-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20240913075750.35115-6-stefan.klug@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]