[v5,30/36] ipa: libipa: lsc: Template LscAlgorithm with register format

27337 diff mbox series

The LscPolynomial::samplePolynomial function expands the radial
polynomial assuming the RkISP1 Q<2, 10> register format:

       /*
	* The hardware uses 2.10 fixed point format and limits
	* the legal values to [1..3.999]. Scale and clamp the
	* sampled value accordingly.
	*/
       int v = static_cast<int>(
	       poly.sampleAtNormalizedPixelPos(xp, yp) *
	       1024);
       v = std::clamp(v, 1024, 4095);

In order to remove all assumptions about the platform register format
template the LscAlgorithm class with the fixed point format of the
ISP Lsc engine registers.

       float sample = static_cast<float>
		       (poly.sampleAtNormalizedPixelPos(xp, yp));

       samples.push_back(U(sample).quantized());

In order to template the LscPolynomial class, template the whole class
hierarchy including the LscTable class that doesn't need (yet) the
template argument but will use it once re-sampling LscTable will be
implemented.

Split all involved classes (LscAlgorithm, LscTable and LscPolynomial) in
two classes: a Lsc*Base class for non-templated function implementations
and a Lsc* class for templated functions implementations.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 src/ipa/libipa/lsc.cpp            | 151 +++++++++++++-------------------------
 src/ipa/libipa/lsc.h              |  81 +++++++++++++++++---
 src/ipa/libipa/lsc_base.cpp       |   1 +
 src/ipa/libipa/lsc_base.h         |   1 +
 src/ipa/libipa/lsc_polynomial.cpp |  90 +++++++----------------
 src/ipa/libipa/lsc_polynomial.h   |  67 +++++++++++++++--
 src/ipa/libipa/lsc_table.cpp      |  49 +++++++++----
 src/ipa/libipa/lsc_table.h        |  35 ++++++---
 src/ipa/rkisp1/algorithms/lsc.h   |   3 +-
 9 files changed, 270 insertions(+), 208 deletions(-)

Quoting Jacopo Mondi (2026-07-08 17:51:12)
> The LscPolynomial::samplePolynomial function expands the radial
> polynomial assuming the RkISP1 Q<2, 10> register format:
> 
>        /*
>         * The hardware uses 2.10 fixed point format and limits
>         * the legal values to [1..3.999]. Scale and clamp the
>         * sampled value accordingly.
>         */
>        int v = static_cast<int>(
>                poly.sampleAtNormalizedPixelPos(xp, yp) *
>                1024);
>        v = std::clamp(v, 1024, 4095);
> 
> In order to remove all assumptions about the platform register format
> template the LscAlgorithm class with the fixed point format of the
> ISP Lsc engine registers.
> 
>        float sample = static_cast<float>
>                        (poly.sampleAtNormalizedPixelPos(xp, yp));
> 
>        samples.push_back(U(sample).quantized());
> 
> In order to template the LscPolynomial class, template the whole class
> hierarchy including the LscTable class that doesn't need (yet) the
> template argument but will use it once re-sampling LscTable will be
> implemented.
> 
> Split all involved classes (LscAlgorithm, LscTable and LscPolynomial) in
> two classes: a Lsc*Base class for non-templated function implementations
> and a Lsc* class for templated functions implementations.

I'm wondering if this is really necessary. Making the implementations
templated and hardware format aware introduces quite some complexity.

The polynomial resampling only happens at configure() time. So we could
also just return a table of floats. I see two options there:

a) Conversion from float to fixed happens in the  LscAlgorithm class
In this case, the interpolation would still be done on integers. Side
question: Can we guarantee that all fixed point formats can be
interpolated based on integers? I can confirm that for the rkisp1 but
didn't think about the other ones.

b) Conversion from float to fixed happens in the platform specific
implementation. This would have the benefit that we could get rid of all
the templating. But has the downside that the interpolation would be
done in floats and after interpolation the conversion to fixed point
(That feelsexpensive, but I don't know numbers).

Only issue is that the tables are currently stored as ints in the tuning
file and would need a temporary int -> float -> int conversion until fe
store floats in the tuning file.

What do you think?

Best regards,
Stefan

> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  src/ipa/libipa/lsc.cpp            | 151 +++++++++++++-------------------------
>  src/ipa/libipa/lsc.h              |  81 +++++++++++++++++---
>  src/ipa/libipa/lsc_base.cpp       |   1 +
>  src/ipa/libipa/lsc_base.h         |   1 +
>  src/ipa/libipa/lsc_polynomial.cpp |  90 +++++++----------------
>  src/ipa/libipa/lsc_polynomial.h   |  67 +++++++++++++++--
>  src/ipa/libipa/lsc_table.cpp      |  49 +++++++++----
>  src/ipa/libipa/lsc_table.h        |  35 ++++++---
>  src/ipa/rkisp1/algorithms/lsc.h   |   3 +-
>  9 files changed, 270 insertions(+), 208 deletions(-)
> 
> diff --git a/src/ipa/libipa/lsc.cpp b/src/ipa/libipa/lsc.cpp
> index 13b1b738106b..f5646937c60a 100644
> --- a/src/ipa/libipa/lsc.cpp
> +++ b/src/ipa/libipa/lsc.cpp
> @@ -7,13 +7,6 @@
>  
>  #include "lsc.h"
>  
> -#include <libcamera/base/log.h>
> -
> -#include <libcamera/control_ids.h>
> -
> -#include "lsc_polynomial.h"
> -#include "lsc_table.h"
> -
>  /**
>   * \file lsc.h
>   * \brief libipa LSC algorithm
> @@ -48,9 +41,56 @@ namespace lsc {
>  
>  } /* namespace lsc */
>  
> +/**
> + * \class LscAlgorithmBase
> + * \brief Base class for LscAlgorithm
> + *
> + * Base class for LscAlgorithm for non-templated functions implementation
> + */
> +
> +/**
> + * \brief Queue a request to the LSC algorithm
> + * \param[in] state The LSC active state
> + * \param[in] context The LSC frame context
> + * \param[in] controls The list of controls associated with a Request
> + *
> + * Queue a new list of \a controls to the LSC algorithm.
> + * The only supported control is controls::LensShadingCorrectionEnable.
> + */
> +void LscAlgorithmBase::queueRequest(lsc::ActiveState &state,
> +                                   lsc::FrameContext &context,
> +                                   const ControlList &controls)
> +{
> +       const auto &lscEnable = controls.get(controls::LensShadingCorrectionEnable);
> +       if (lscEnable && *lscEnable != state.enabled) {
> +               state.enabled = *lscEnable;
> +
> +               LOG(Lsc, Debug)
> +                       << (state.enabled ? "Enabling" : "Disabling") << " Lsc";
> +
> +               context.update = true;
> +       }
> +
> +       context.enabled = state.enabled;
> +}
> +
> +/**
> + * \brief Populate the list of LSC metadata
> + * \param[in] context The LSC frame context
> + * \param[in] metadata The list of metadata
> + *
> + * Populates the list of \a metadata with controls handled by the LscAlgorithm
> + * class. The only supported metadata is controls::LensShadingCorrectionEnable.
> + */
> +void LscAlgorithmBase::process(lsc::FrameContext &context, ControlList &metadata)
> +{
> +       metadata.set(controls::LensShadingCorrectionEnable, context.enabled);
> +}
> +
>  /**
>   * \class LscAlgorithm
>   * \brief libIPA LSC algorithm implementation
> + * \tparam U The fixedpoint LSC engine register format
>   *
>   * Due to the optical characteristics of the lens, the light intensity received
>   * by the sensor is not uniform. The Lens Shading Correction algorithm applies
> @@ -200,6 +240,7 @@ namespace lsc {
>   */
>  
>  /**
> + * \fn LscAlgorithm::init()
>   * \param[in] tuningData The tuning data
>   * \param[in] controls The IPA list of supported controls
>   * \param[in] descriptor The LSC engine descriptor
> @@ -209,42 +250,9 @@ namespace lsc {
>   *
>   * \return 0 on success, a negative error code otherwise
>   */
> -int LscAlgorithm::init(const ValueNode &tuningData, ControlInfoMap::Map &controls,
> -                      const LscDescriptor &descriptor)
> -{
> -       polynomial_ = false;
> -
> -       std::string type = tuningData["type"].get<std::string>("table");
> -       if (type == "table") {
> -               impl_ = std::make_unique<LscTable>();
> -               LOG(Lsc, Debug) << "Using table-based Lsc";
> -       } else if (type == "polynomial") {
> -               impl_ = std::make_unique<LscPolynomial>();
> -               polynomial_ = true;
> -               LOG(Lsc, Debug) << "Using polynomial Lsc";
> -       } else {
> -               LOG(Lsc, Error) << "Unsupported Lsc algorithm '"
> -                               << type << "'";
> -               return -EINVAL;
> -       }
> -
> -       const ValueNode &yamlSets = tuningData["sets"];
> -       if (!yamlSets.isList()) {
> -               LOG(Lsc, Error) << "'sets' parameter not found in tuning file";
> -               return -EINVAL;
> -       }
> -
> -       int ret = impl_->parseLscData(yamlSets, descriptor);
> -       if (ret)
> -               return ret;
> -
> -       controls[&controls::LensShadingCorrectionEnable] =
> -               ControlInfo(false, true, true);
> -
> -       return 0;
> -}
>  
>  /**
> + * \fn LscAlgorithm::configure()
>   * \param[in] state The LSC active state
>   * \param[in] analogCrop The current sensor analog crop rectangle
>   * \param[in] xPos List of horizontal positions of the LSC grid nodes
> @@ -265,67 +273,6 @@ int LscAlgorithm::init(const ValueNode &tuningData, ControlInfoMap::Map &control
>   *
>   * \return 0 on success, a negative error code otherwise
>   */
> -int LscAlgorithm::configure(lsc::ActiveState &state, const Rectangle &analogCrop,
> -                           const std::vector<double> &xPos,
> -                           const std::vector<double> &yPos)
> -{
> -       LOG(Lsc, Debug) << "Sample Lsc data for " << analogCrop;
> -       lsc::ComponentsMap lscData =
> -               impl_->sampleForCrop(analogCrop, xPos, yPos);
> -
> -       /*
> -        * Retain a copy of the components table.
> -        *
> -        * We could avoid a copy here if getComponents() could
> -        * return sets_.data() but I wasn't able to work around the
> -        * compiler refusing it.
> -        */
> -       lscData_ = lscData;
> -
> -       sets_.setData(std::move(lscData));
> -       state.enabled = true;
> -
> -       return 0;
> -}
> -
> -/**
> - * \brief Queue a request to the lsc algorithm
> - * \param[in] state The lsc active state
> - * \param[in] context The lsc frame context
> - * \param[in] controls The list of controls associated with a Request
> - *
> - * Queue a new list of \a controls to the lsc algorithm.
> - * The only supported control is controls::LensShadingCorrectionEnable.
> - */
> -void LscAlgorithm::queueRequest(lsc::ActiveState &state,
> -                               lsc::FrameContext &context,
> -                               const ControlList &controls)
> -{
> -       const auto &lscEnable = controls.get(controls::LensShadingCorrectionEnable);
> -       if (lscEnable && *lscEnable != state.enabled) {
> -               state.enabled = *lscEnable;
> -
> -               LOG(Lsc, Debug)
> -                       << (state.enabled ? "Enabling" : "Disabling") << " Lsc";
> -
> -               context.update = true;
> -       }
> -
> -       context.enabled = state.enabled;
> -}
> -
> -/**
> - * \brief Populate the list of lsc metadata
> - * \param[in] context The lsc frame context
> - * \param[in] metadata The list of metadata
> - *
> - * Populates the list of \a metadata with controls handled by the LscAlgorithm
> - * class. The only supported metadata is controls::LensShadingCorrectionEnable.
> - */
> -void LscAlgorithm::process(lsc::FrameContext &context, ControlList &metadata)
> -{
> -       metadata.set(controls::LensShadingCorrectionEnable, context.enabled);
> -}
>  
>  /**
>   * \fn LscAlgorithm::interpolateComponents
> diff --git a/src/ipa/libipa/lsc.h b/src/ipa/libipa/lsc.h
> index 9fe8ad67ea33..d48336b169f4 100644
> --- a/src/ipa/libipa/lsc.h
> +++ b/src/ipa/libipa/lsc.h
> @@ -8,18 +8,25 @@
>  #pragma once
>  
>  #include <memory>
> +#include <string>
>  #include <vector>
>  
> +#include <libcamera/base/log.h>
> +
> +#include <libcamera/control_ids.h>
>  #include <libcamera/controls.h>
>  #include <libcamera/geometry.h>
>  
>  #include "libcamera/internal/value_node.h"
>  
>  #include "interpolator.h"
> -#include "lsc_base.h"
> +#include "lsc_polynomial.h"
> +#include "lsc_table.h"
>  
>  namespace libcamera {
>  
> +LOG_DECLARE_CATEGORY(Lsc)
> +
>  namespace ipa {
>  
>  namespace lsc {
> @@ -35,19 +42,75 @@ struct FrameContext {
>  
>  } /* namespace lsc */
>  
> -class LscAlgorithm
> +class LscAlgorithmBase
> +{
> +public:
> +       void queueRequest(lsc::ActiveState &state, lsc::FrameContext &context,
> +                         const ControlList &controls);
> +       void process(lsc::FrameContext &context, ControlList &metadata);
> +};
> +
> +template<typename U>
> +class LscAlgorithm : public LscAlgorithmBase
>  {
>  public:
>         int init(const ValueNode &tuningData, ControlInfoMap::Map &controls,
> -                const LscDescriptor &descriptor);
> +                const LscDescriptor &descriptor)
> +       {
> +               polynomial_ = false;
> +
> +               std::string type = tuningData["type"].get<std::string>("table");
> +               if (type == "table") {
> +                       impl_ = std::make_unique<LscTable<U>>();
> +                       LOG(Lsc, Debug) << "Using table-based Lsc";
> +               } else if (type == "polynomial") {
> +                       impl_ = std::make_unique<LscPolynomial<U>>();
> +                       polynomial_ = true;
> +                       LOG(Lsc, Debug) << "Using polynomial Lsc";
> +               } else {
> +                       LOG(Lsc, Error) << "Unsupported Lsc algorithm '"
> +                                       << type << "'";
> +                       return -EINVAL;
> +               }
> +
> +               const ValueNode &yamlSets = tuningData["sets"];
> +               if (!yamlSets.isList()) {
> +                       LOG(Lsc, Error) << "'sets' parameter not found in tuning file";
> +                       return -EINVAL;
> +               }
> +
> +               int ret = impl_->parseLscData(yamlSets, descriptor);
> +               if (ret)
> +                       return ret;
> +
> +               controls[&controls::LensShadingCorrectionEnable] =
> +                       ControlInfo(false, true, true);
> +
> +               return 0;
> +       }
>  
>         int configure(lsc::ActiveState &state, const Rectangle &analogCrop,
>                       const std::vector<double> &xPos,
> -                     const std::vector<double> &yPos);
> -
> -       void queueRequest(lsc::ActiveState &state, lsc::FrameContext &context,
> -                         const ControlList &controls);
> -       void process(lsc::FrameContext &context, ControlList &metadata);
> +                     const std::vector<double> &yPos)
> +       {
> +               LOG(Lsc, Debug) << "Sample Lsc data for " << analogCrop;
> +               lsc::ComponentsMap lscData =
> +                       impl_->sampleForCrop(analogCrop, xPos, yPos);
> +
> +               /*
> +                * Retain a copy of the components table.
> +                *
> +                * We could avoid a copy here if getComponents() could
> +                * return sets_.data() but I wasn't able to work around the
> +                * compiler refusing it.
> +                */
> +               lscData_ = lscData;
> +
> +               sets_.setData(std::move(lscData));
> +               state.enabled = true;
> +
> +               return 0;
> +       }
>  
>         const lsc::Components interpolateComponents(unsigned int ct)
>         {
> @@ -60,7 +123,7 @@ public:
>         }
>  
>  private:
> -       std::unique_ptr<LscImplementation> impl_;
> +       std::unique_ptr<LscImplementation<U>> impl_;
>         Interpolator<lsc::Components> sets_;
>         lsc::ComponentsMap lscData_;
>         bool polynomial_;
> diff --git a/src/ipa/libipa/lsc_base.cpp b/src/ipa/libipa/lsc_base.cpp
> index adc09c15752a..b91da4e5be9d 100644
> --- a/src/ipa/libipa/lsc_base.cpp
> +++ b/src/ipa/libipa/lsc_base.cpp
> @@ -86,6 +86,7 @@ void Interpolator<lsc::Components>::
>  /**
>   * \class LscImplementation
>   * \brief Pure virtual base class for LSC algorithm implementations
> + * \tparam U The fixedpoint LSC engine register format
>   *
>   * Defines the interface for the LSC algorithm implementation.
>   */
> diff --git a/src/ipa/libipa/lsc_base.h b/src/ipa/libipa/lsc_base.h
> index 48307455362a..721fd137ef21 100644
> --- a/src/ipa/libipa/lsc_base.h
> +++ b/src/ipa/libipa/lsc_base.h
> @@ -57,6 +57,7 @@ struct LscDescriptor {
>         Size sensorSize;
>  };
>  
> +template<typename U>
>  class LscImplementation
>  {
>  public:
> diff --git a/src/ipa/libipa/lsc_polynomial.cpp b/src/ipa/libipa/lsc_polynomial.cpp
> index c61c85fbf2b2..060fba9826a0 100644
> --- a/src/ipa/libipa/lsc_polynomial.cpp
> +++ b/src/ipa/libipa/lsc_polynomial.cpp
> @@ -113,14 +113,10 @@ void Polynomial::setReferenceImageSize(const Size &size)
>  } /* namespace lsc */
>  
>  /**
> - * \class LscPolynomial
> - * \brief Radial Polynomial LSC algorithm implementation
> - *
> - * Polynomial-based LSC algorithm implementation. The LscPolynomial class
> - * implements LSC support using a Polynomial to represent the shading artifacts
> - * map.
> + * \class LscPolynomialBase
> + * \brief Base class for LscPolynomial
>   *
> - * \sa LscImplementation
> + * Base class for LscPolynomial for non-templated functions.
>   */
>  
>  /**
> @@ -132,8 +128,8 @@ void Polynomial::setReferenceImageSize(const Size &size)
>   *
>   * \return 0 on success or a negative error number otherwise
>   */
> -int LscPolynomial::parseLscData(const ValueNode &sets,
> -                               const LscDescriptor &descriptor)
> +int LscPolynomialBase::parseLscData(const ValueNode &sets,
> +                                   const LscDescriptor &descriptor)
>  {
>         for (const auto &set : sets.asList()) {
>                 uint32_t ct = set["ct"].get<uint32_t>(0);
> @@ -174,6 +170,27 @@ int LscPolynomial::parseLscData(const ValueNode &sets,
>  }
>  
>  /**
> + * \var LscPolynomialBase::lscData_
> + * \brief The polynomial LSC data
> + *
> + * Maps colour temperatures to per-colour radial polynomial definitions.
> + */
> +
> +/**
> + * \class LscPolynomial
> + * \brief Radial Polynomial LSC algorithm implementation
> + * \tparam U The fixedpoint format used to convert gain values generated by
> + * polynomial expansion to the register format
> + *
> + * Polynomial-based LSC algorithm implementation. The LscPolynomial class
> + * implements LSC support using Polynomial to represent the shading artifacts
> + * map.
> + *
> + * \sa LscImplementation
> + */
> +
> +/**
> + * \fn LscPolynomial::sampleForCrop()
>   * \brief Re-sample the LSC components for \a cropRectangle
>   * \param[in] cropRectangle The sensor analogue crop rectangle
>   * \param[in] xPos List of horizontal positions of the LSC grid nodes
> @@ -195,62 +212,7 @@ int LscPolynomial::parseLscData(const ValueNode &sets,
>   * [0, 0.0625, 0.125, 0.1875, ... , 1]. It is expected that the first position
>   * is 0 and the last position is 1.
>   */
> -lsc::ComponentsMap
> -LscPolynomial::sampleForCrop(const Rectangle &cropRectangle,
> -                            std::vector<double> xPos, std::vector<double> yPos)
> -{
> -
> -       lsc::ComponentsMap components;
> -
> -       for (const auto &[t, c] : lscData_) {
> -               lsc::Components comp;
> -
> -               for (const auto &[k, p] : c) {
> -                       comp.emplace(std::piecewise_construct,
> -                                    std::forward_as_tuple(k),
> -                                    std::forward_as_tuple(samplePolynomial(p, xPos, yPos,
> -                                                                           cropRectangle)));
> -               }
>  
> -               components[t] = comp;
> -       }
> -
> -       return components;
> -}
> -
> -std::vector<uint16_t>
> -LscPolynomial::samplePolynomial(const lsc::Polynomial &poly,
> -                               Span<const double> xPositions,
> -                               Span<const double> yPositions,
> -                               const Rectangle &cropRectangle)
> -{
> -       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> samples;
> -
> -       samples.reserve(xPositions.size() * yPositions.size());
> -
> -       for (double y : yPositions) {
> -               for (double x : xPositions) {
> -                       double xp = x0 + x * w;
> -                       double yp = y0 + y * h;
> -                       /*
> -                        * The hardware uses 2.10 fixed point format and limits
> -                        * the legal values to [1..3.999]. Scale and clamp the
> -                        * sampled value accordingly.
> -                        */
> -                       int v = static_cast<int>(
> -                               poly.sampleAtNormalizedPixelPos(xp, yp) *
> -                               1024);
> -                       v = std::clamp(v, 1024, 4095);
> -                       samples.push_back(v);
> -               }
> -       }
> -       return samples;
> -}
>  
>  } /* namespace ipa */
>  
> diff --git a/src/ipa/libipa/lsc_polynomial.h b/src/ipa/libipa/lsc_polynomial.h
> index 6d19e46a50b7..04d7465b3af0 100644
> --- a/src/ipa/libipa/lsc_polynomial.h
> +++ b/src/ipa/libipa/lsc_polynomial.h
> @@ -8,6 +8,7 @@
>  
>  #include <array>
>  #include <map>
> +#include <tuple>
>  #include <vector>
>  
>  #include <libcamera/base/span.h>
> @@ -50,26 +51,80 @@ private:
>  
>  } /* namespace lsc */
>  
> -class LscPolynomial : public LscImplementation
> +class LscPolynomialBase
>  {
>  private:
>         using PolynomialComponents = std::map<std::string, lsc::Polynomial>;
>         using PolynomialComponentsMap = std::map<unsigned int, PolynomialComponents>;
>  
> +protected:
> +       int parseLscData(const ValueNode &yamlSets,
> +                        const LscDescriptor &descriptor);
> +
> +       PolynomialComponentsMap lscData_;
> +};
> +
> +template<typename U>
> +class LscPolynomial : public LscPolynomialBase, public LscImplementation<U>
> +{
>  public:
> -       int parseLscData(const ValueNode &sets,
> -                        const LscDescriptor &descriptor) override;
> +       int parseLscData(const ValueNode &yamlSets,
> +                        const LscDescriptor &descriptor) override
> +       {
> +               return LscPolynomialBase::parseLscData(yamlSets, descriptor);
> +       }
>  
>         lsc::ComponentsMap
>         sampleForCrop(const Rectangle &cropRectangle,
> -                     std::vector<double> xPos, std::vector<double> yPos) override;
> +                     std::vector<double> xPos, std::vector<double> yPos) override
> +       {
> +               lsc::ComponentsMap components;
> +
> +               for (const auto &[t, c] : lscData_) {
> +                       lsc::Components comp;
> +
> +                       for (const auto &[k, p] : c) {
> +                               comp.emplace(std::piecewise_construct,
> +                                            std::forward_as_tuple(k),
> +                                            std::forward_as_tuple(samplePolynomial(p, xPos, yPos,
> +                                                                                   cropRectangle)));
> +                       }
> +
> +                       components[t] = comp;
> +               }
> +
> +               return components;
> +       }
>  
>  private:
>         std::vector<uint16_t> samplePolynomial(const lsc::Polynomial &poly,
>                                                Span<const double> xPositions,
>                                                Span<const double> yPositions,
> -                                              const Rectangle &cropRectangle);
> -       PolynomialComponentsMap lscData_;
> +                                              const Rectangle &cropRectangle)
> +       {
> +               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> samples;
> +
> +               samples.reserve(xPositions.size() * yPositions.size());
> +
> +               for (double y : yPositions) {
> +                       for (double x : xPositions) {
> +                               double xp = x0 + x * w;
> +                               double yp = y0 + y * h;
> +
> +                               float sample = static_cast<float>
> +                                               (poly.sampleAtNormalizedPixelPos(xp, yp));
> +
> +                               samples.push_back(U(sample).quantized());
> +                       }
> +               }
> +
> +               return samples;
> +       }
>  };
>  
>  } /* namespace ipa */
> diff --git a/src/ipa/libipa/lsc_table.cpp b/src/ipa/libipa/lsc_table.cpp
> index cdd2fea5ebf3..0e8f144cb873 100644
> --- a/src/ipa/libipa/lsc_table.cpp
> +++ b/src/ipa/libipa/lsc_table.cpp
> @@ -14,13 +14,10 @@ LOG_DEFINE_CATEGORY(LscTable)
>  namespace ipa {
>  
>  /**
> - * \class LscTable
> - * \brief Table based LSC algorithm implementation
> - *
> - * Table based LSC algorithm implementation. The LSCTable class implements LSC
> - * support using tabular LSC data.
> + * \class LscTableBase
> + * \brief Base class for LscTable
>   *
> - * \sa LscImplementation
> + * Base class for LscTable for non-templated functions.
>   */
>  
>  /**
> @@ -32,8 +29,8 @@ namespace ipa {
>   *
>   * \return 0 on success or a negative error number otherwise
>   */
> -int LscTable::parseLscData(const ValueNode &sets,
> -                          const LscDescriptor &descriptor)
> +int LscTableBase::parseLscData(const ValueNode &sets,
> +                              const LscDescriptor &descriptor)
>  {
>         for (const auto &set : sets.asList()) {
>                 uint32_t ct = set["ct"].get<uint32_t>(0);
> @@ -51,8 +48,8 @@ int LscTable::parseLscData(const ValueNode &sets,
>         return 0;
>  }
>  
> -int LscTable::parseLscComponent(const ValueNode &yamlSet,
> -                               unsigned int ct, const LscDescriptor &descriptor)
> +int LscTableBase::parseLscComponent(const ValueNode &yamlSet,
> +                                   unsigned int ct, const LscDescriptor &descriptor)
>  {
>         lsc::Components component;
>         for (auto &k : descriptor.keys) {
> @@ -82,10 +79,10 @@ int LscTable::parseLscComponent(const ValueNode &yamlSet,
>         return 0;
>  }
>  
> -std::vector<uint16_t> LscTable::parseTable(const ValueNode &tuningData,
> -                                          const char *prop,
> -                                          unsigned int numHCells,
> -                                          unsigned int numVCells)
> +std::vector<uint16_t> LscTableBase::parseTable(const ValueNode &tuningData,
> +                                              const char *prop,
> +                                              unsigned int numHCells,
> +                                              unsigned int numVCells)
>  {
>         unsigned int lscNumSamples = numHCells * numVCells;
>  
> @@ -102,6 +99,30 @@ std::vector<uint16_t> LscTable::parseTable(const ValueNode &tuningData,
>         return table;
>  }
>  
> +/**
> + * \var LscTableBase::lscData_
> + * \brief The tabular LSC data
> + *
> + * Maps colour temperatures to per-colour channel vector of gains
> + */
> +
> +/**
> + * \class LscTable
> + * \brief Table based LSC algorithm implementation
> + * \tparam U The fixedpoint LSC engine register format
> + *
> + * Table based LSC algorithm implementation. The LscTable class implements LSC
> + * support using tabular LSC data.
> + *
> + * \sa LscImplementation
> + */
> +
> +/**
> + * \fn LscTable::parseLscData()
> + *
> + * \copydoc LscTableBase::parseLscData()
> + */
> +
>  } /* namespace ipa */
>  
>  } /* namespace libcamera */
> diff --git a/src/ipa/libipa/lsc_table.h b/src/ipa/libipa/lsc_table.h
> index 3fe52afbad7b..65e04b09c3ff 100644
> --- a/src/ipa/libipa/lsc_table.h
> +++ b/src/ipa/libipa/lsc_table.h
> @@ -23,11 +23,32 @@ LOG_DECLARE_CATEGORY(LscTable)
>  
>  namespace ipa {
>  
> -class LscTable : public LscImplementation
> +class LscTableBase
> +{
> +protected:
> +       int parseLscData(const ValueNode &sets,
> +                        const LscDescriptor &descriptor);
> +
> +private:
> +       int parseLscComponent(const ValueNode &yamlSet,
> +                             unsigned int ct, const LscDescriptor &descriptor);
> +       std::vector<uint16_t> parseTable(const ValueNode &tuningData,
> +                                        const char *prop,
> +                                        unsigned int numHCells,
> +                                        unsigned int numVCells);
> +protected:
> +       lsc::ComponentsMap lscData_;
> +};
> +
> +template<typename U>
> +class LscTable : public LscTableBase, public LscImplementation<U>
>  {
>  public:
>         int parseLscData(const ValueNode &sets,
> -                        const LscDescriptor &descriptor) override;
> +                        const LscDescriptor &descriptor) override
> +       {
> +               return LscTableBase::parseLscData(sets, descriptor);
> +       }
>  
>         lsc::ComponentsMap
>         sampleForCrop([[maybe_unused]] const Rectangle &cropRectangle,
> @@ -38,16 +59,6 @@ public:
>                         << "Tabular LSC data doesn't support resampling";
>                 return lscData_;
>         }
> -
> -private:
> -       int parseLscComponent(const ValueNode &yamlSet,
> -                             unsigned int ct, const LscDescriptor &descriptor);
> -       std::vector<uint16_t> parseTable(const ValueNode &tuningData,
> -                                        const char *prop,
> -                                        unsigned int numHCells,
> -                                        unsigned int numVCells);
> -
> -       lsc::ComponentsMap lscData_;
>  };
>  
>  } /* namespace ipa */
> diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h
> index e7d3091a36c5..31156f56cd21 100644
> --- a/src/ipa/rkisp1/algorithms/lsc.h
> +++ b/src/ipa/rkisp1/algorithms/lsc.h
> @@ -13,6 +13,7 @@
>  
>  #include "libcamera/internal/value_node.h"
>  
> +#include "libipa/fixedpoint.h"
>  #include "libipa/lsc.h"
>  
>  #include "algorithm.h"
> @@ -58,7 +59,7 @@ private:
>         unsigned int lastAppliedCt_;
>         unsigned int lastAppliedQuantizedCt_;
>  
> -       LscAlgorithm lscAlgo_;
> +       LscAlgorithm<UQ<2, 10>> lscAlgo_;
>  };
>  
>  } /* namespace ipa::rkisp1::algorithms */
> 
> -- 
> 2.54.0
>

Hi Stefan

On Tue, Jul 14, 2026 at 04:15:26PM +0200, Stefan Klug wrote:
> Quoting Jacopo Mondi (2026-07-08 17:51:12)
> > The LscPolynomial::samplePolynomial function expands the radial
> > polynomial assuming the RkISP1 Q<2, 10> register format:
> >
> >        /*
> >         * The hardware uses 2.10 fixed point format and limits
> >         * the legal values to [1..3.999]. Scale and clamp the
> >         * sampled value accordingly.
> >         */
> >        int v = static_cast<int>(
> >                poly.sampleAtNormalizedPixelPos(xp, yp) *
> >                1024);
> >        v = std::clamp(v, 1024, 4095);
> >
> > In order to remove all assumptions about the platform register format
> > template the LscAlgorithm class with the fixed point format of the
> > ISP Lsc engine registers.
> >
> >        float sample = static_cast<float>
> >                        (poly.sampleAtNormalizedPixelPos(xp, yp));
> >
> >        samples.push_back(U(sample).quantized());
> >
> > In order to template the LscPolynomial class, template the whole class
> > hierarchy including the LscTable class that doesn't need (yet) the
> > template argument but will use it once re-sampling LscTable will be
> > implemented.
> >
> > Split all involved classes (LscAlgorithm, LscTable and LscPolynomial) in
> > two classes: a Lsc*Base class for non-templated function implementations
> > and a Lsc* class for templated functions implementations.
>
> I'm wondering if this is really necessary. Making the implementations
> templated and hardware format aware introduces quite some complexity.
>
> The polynomial resampling only happens at configure() time. So we could
> also just return a table of floats. I see two options there:
>
> a) Conversion from float to fixed happens in the  LscAlgorithm class

So we should here populate a vector of potentially thoudsands of
entries, and then iterate over it in the LscAlgorithm to quantize it ?

> In this case, the interpolation would still be done on integers. Side
> question: Can we guarantee that all fixed point formats can be
> interpolated based on integers? I can confirm that for the rkisp1 but
> didn't think about the other ones.

we don't interpoalte fixed point formats. We intepolate integers, and
the quantize them. I don't think I fully understood your question

>
> b) Conversion from float to fixed happens in the platform specific
> implementation. This would have the benefit that we could get rid of all

Likewise, populate a vector of thousands of integers and then iterate
over it to quantize them. Why ?

> the templating. But has the downside that the interpolation would be
> done in floats and after interpolation the conversion to fixed point
> (That feelsexpensive, but I don't know numbers).
>
> Only issue is that the tables are currently stored as ints in the tuning
> file and would need a temporary int -> float -> int conversion until fe
> store floats in the tuning file.
>
> What do you think?

I don't underand the reasons to make this less efficient. And no, "It's
complex" it's not a good enough reason.

>
> Best regards,
> Stefan
>
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > ---
> >  src/ipa/libipa/lsc.cpp            | 151 +++++++++++++-------------------------
> >  src/ipa/libipa/lsc.h              |  81 +++++++++++++++++---
> >  src/ipa/libipa/lsc_base.cpp       |   1 +
> >  src/ipa/libipa/lsc_base.h         |   1 +
> >  src/ipa/libipa/lsc_polynomial.cpp |  90 +++++++----------------
> >  src/ipa/libipa/lsc_polynomial.h   |  67 +++++++++++++++--
> >  src/ipa/libipa/lsc_table.cpp      |  49 +++++++++----
> >  src/ipa/libipa/lsc_table.h        |  35 ++++++---
> >  src/ipa/rkisp1/algorithms/lsc.h   |   3 +-
> >  9 files changed, 270 insertions(+), 208 deletions(-)
> >
> > diff --git a/src/ipa/libipa/lsc.cpp b/src/ipa/libipa/lsc.cpp
> > index 13b1b738106b..f5646937c60a 100644
> > --- a/src/ipa/libipa/lsc.cpp
> > +++ b/src/ipa/libipa/lsc.cpp
> > @@ -7,13 +7,6 @@
> >
> >  #include "lsc.h"
> >
> > -#include <libcamera/base/log.h>
> > -
> > -#include <libcamera/control_ids.h>
> > -
> > -#include "lsc_polynomial.h"
> > -#include "lsc_table.h"
> > -
> >  /**
> >   * \file lsc.h
> >   * \brief libipa LSC algorithm
> > @@ -48,9 +41,56 @@ namespace lsc {
> >
> >  } /* namespace lsc */
> >
> > +/**
> > + * \class LscAlgorithmBase
> > + * \brief Base class for LscAlgorithm
> > + *
> > + * Base class for LscAlgorithm for non-templated functions implementation
> > + */
> > +
> > +/**
> > + * \brief Queue a request to the LSC algorithm
> > + * \param[in] state The LSC active state
> > + * \param[in] context The LSC frame context
> > + * \param[in] controls The list of controls associated with a Request
> > + *
> > + * Queue a new list of \a controls to the LSC algorithm.
> > + * The only supported control is controls::LensShadingCorrectionEnable.
> > + */
> > +void LscAlgorithmBase::queueRequest(lsc::ActiveState &state,
> > +                                   lsc::FrameContext &context,
> > +                                   const ControlList &controls)
> > +{
> > +       const auto &lscEnable = controls.get(controls::LensShadingCorrectionEnable);
> > +       if (lscEnable && *lscEnable != state.enabled) {
> > +               state.enabled = *lscEnable;
> > +
> > +               LOG(Lsc, Debug)
> > +                       << (state.enabled ? "Enabling" : "Disabling") << " Lsc";
> > +
> > +               context.update = true;
> > +       }
> > +
> > +       context.enabled = state.enabled;
> > +}
> > +
> > +/**
> > + * \brief Populate the list of LSC metadata
> > + * \param[in] context The LSC frame context
> > + * \param[in] metadata The list of metadata
> > + *
> > + * Populates the list of \a metadata with controls handled by the LscAlgorithm
> > + * class. The only supported metadata is controls::LensShadingCorrectionEnable.
> > + */
> > +void LscAlgorithmBase::process(lsc::FrameContext &context, ControlList &metadata)
> > +{
> > +       metadata.set(controls::LensShadingCorrectionEnable, context.enabled);
> > +}
> > +
> >  /**
> >   * \class LscAlgorithm
> >   * \brief libIPA LSC algorithm implementation
> > + * \tparam U The fixedpoint LSC engine register format
> >   *
> >   * Due to the optical characteristics of the lens, the light intensity received
> >   * by the sensor is not uniform. The Lens Shading Correction algorithm applies
> > @@ -200,6 +240,7 @@ namespace lsc {
> >   */
> >
> >  /**
> > + * \fn LscAlgorithm::init()
> >   * \param[in] tuningData The tuning data
> >   * \param[in] controls The IPA list of supported controls
> >   * \param[in] descriptor The LSC engine descriptor
> > @@ -209,42 +250,9 @@ namespace lsc {
> >   *
> >   * \return 0 on success, a negative error code otherwise
> >   */
> > -int LscAlgorithm::init(const ValueNode &tuningData, ControlInfoMap::Map &controls,
> > -                      const LscDescriptor &descriptor)
> > -{
> > -       polynomial_ = false;
> > -
> > -       std::string type = tuningData["type"].get<std::string>("table");
> > -       if (type == "table") {
> > -               impl_ = std::make_unique<LscTable>();
> > -               LOG(Lsc, Debug) << "Using table-based Lsc";
> > -       } else if (type == "polynomial") {
> > -               impl_ = std::make_unique<LscPolynomial>();
> > -               polynomial_ = true;
> > -               LOG(Lsc, Debug) << "Using polynomial Lsc";
> > -       } else {
> > -               LOG(Lsc, Error) << "Unsupported Lsc algorithm '"
> > -                               << type << "'";
> > -               return -EINVAL;
> > -       }
> > -
> > -       const ValueNode &yamlSets = tuningData["sets"];
> > -       if (!yamlSets.isList()) {
> > -               LOG(Lsc, Error) << "'sets' parameter not found in tuning file";
> > -               return -EINVAL;
> > -       }
> > -
> > -       int ret = impl_->parseLscData(yamlSets, descriptor);
> > -       if (ret)
> > -               return ret;
> > -
> > -       controls[&controls::LensShadingCorrectionEnable] =
> > -               ControlInfo(false, true, true);
> > -
> > -       return 0;
> > -}
> >
> >  /**
> > + * \fn LscAlgorithm::configure()
> >   * \param[in] state The LSC active state
> >   * \param[in] analogCrop The current sensor analog crop rectangle
> >   * \param[in] xPos List of horizontal positions of the LSC grid nodes
> > @@ -265,67 +273,6 @@ int LscAlgorithm::init(const ValueNode &tuningData, ControlInfoMap::Map &control
> >   *
> >   * \return 0 on success, a negative error code otherwise
> >   */
> > -int LscAlgorithm::configure(lsc::ActiveState &state, const Rectangle &analogCrop,
> > -                           const std::vector<double> &xPos,
> > -                           const std::vector<double> &yPos)
> > -{
> > -       LOG(Lsc, Debug) << "Sample Lsc data for " << analogCrop;
> > -       lsc::ComponentsMap lscData =
> > -               impl_->sampleForCrop(analogCrop, xPos, yPos);
> > -
> > -       /*
> > -        * Retain a copy of the components table.
> > -        *
> > -        * We could avoid a copy here if getComponents() could
> > -        * return sets_.data() but I wasn't able to work around the
> > -        * compiler refusing it.
> > -        */
> > -       lscData_ = lscData;
> > -
> > -       sets_.setData(std::move(lscData));
> > -       state.enabled = true;
> > -
> > -       return 0;
> > -}
> > -
> > -/**
> > - * \brief Queue a request to the lsc algorithm
> > - * \param[in] state The lsc active state
> > - * \param[in] context The lsc frame context
> > - * \param[in] controls The list of controls associated with a Request
> > - *
> > - * Queue a new list of \a controls to the lsc algorithm.
> > - * The only supported control is controls::LensShadingCorrectionEnable.
> > - */
> > -void LscAlgorithm::queueRequest(lsc::ActiveState &state,
> > -                               lsc::FrameContext &context,
> > -                               const ControlList &controls)
> > -{
> > -       const auto &lscEnable = controls.get(controls::LensShadingCorrectionEnable);
> > -       if (lscEnable && *lscEnable != state.enabled) {
> > -               state.enabled = *lscEnable;
> > -
> > -               LOG(Lsc, Debug)
> > -                       << (state.enabled ? "Enabling" : "Disabling") << " Lsc";
> > -
> > -               context.update = true;
> > -       }
> > -
> > -       context.enabled = state.enabled;
> > -}
> > -
> > -/**
> > - * \brief Populate the list of lsc metadata
> > - * \param[in] context The lsc frame context
> > - * \param[in] metadata The list of metadata
> > - *
> > - * Populates the list of \a metadata with controls handled by the LscAlgorithm
> > - * class. The only supported metadata is controls::LensShadingCorrectionEnable.
> > - */
> > -void LscAlgorithm::process(lsc::FrameContext &context, ControlList &metadata)
> > -{
> > -       metadata.set(controls::LensShadingCorrectionEnable, context.enabled);
> > -}
> >
> >  /**
> >   * \fn LscAlgorithm::interpolateComponents
> > diff --git a/src/ipa/libipa/lsc.h b/src/ipa/libipa/lsc.h
> > index 9fe8ad67ea33..d48336b169f4 100644
> > --- a/src/ipa/libipa/lsc.h
> > +++ b/src/ipa/libipa/lsc.h
> > @@ -8,18 +8,25 @@
> >  #pragma once
> >
> >  #include <memory>
> > +#include <string>
> >  #include <vector>
> >
> > +#include <libcamera/base/log.h>
> > +
> > +#include <libcamera/control_ids.h>
> >  #include <libcamera/controls.h>
> >  #include <libcamera/geometry.h>
> >
> >  #include "libcamera/internal/value_node.h"
> >
> >  #include "interpolator.h"
> > -#include "lsc_base.h"
> > +#include "lsc_polynomial.h"
> > +#include "lsc_table.h"
> >
> >  namespace libcamera {
> >
> > +LOG_DECLARE_CATEGORY(Lsc)
> > +
> >  namespace ipa {
> >
> >  namespace lsc {
> > @@ -35,19 +42,75 @@ struct FrameContext {
> >
> >  } /* namespace lsc */
> >
> > -class LscAlgorithm
> > +class LscAlgorithmBase
> > +{
> > +public:
> > +       void queueRequest(lsc::ActiveState &state, lsc::FrameContext &context,
> > +                         const ControlList &controls);
> > +       void process(lsc::FrameContext &context, ControlList &metadata);
> > +};
> > +
> > +template<typename U>
> > +class LscAlgorithm : public LscAlgorithmBase
> >  {
> >  public:
> >         int init(const ValueNode &tuningData, ControlInfoMap::Map &controls,
> > -                const LscDescriptor &descriptor);
> > +                const LscDescriptor &descriptor)
> > +       {
> > +               polynomial_ = false;
> > +
> > +               std::string type = tuningData["type"].get<std::string>("table");
> > +               if (type == "table") {
> > +                       impl_ = std::make_unique<LscTable<U>>();
> > +                       LOG(Lsc, Debug) << "Using table-based Lsc";
> > +               } else if (type == "polynomial") {
> > +                       impl_ = std::make_unique<LscPolynomial<U>>();
> > +                       polynomial_ = true;
> > +                       LOG(Lsc, Debug) << "Using polynomial Lsc";
> > +               } else {
> > +                       LOG(Lsc, Error) << "Unsupported Lsc algorithm '"
> > +                                       << type << "'";
> > +                       return -EINVAL;
> > +               }
> > +
> > +               const ValueNode &yamlSets = tuningData["sets"];
> > +               if (!yamlSets.isList()) {
> > +                       LOG(Lsc, Error) << "'sets' parameter not found in tuning file";
> > +                       return -EINVAL;
> > +               }
> > +
> > +               int ret = impl_->parseLscData(yamlSets, descriptor);
> > +               if (ret)
> > +                       return ret;
> > +
> > +               controls[&controls::LensShadingCorrectionEnable] =
> > +                       ControlInfo(false, true, true);
> > +
> > +               return 0;
> > +       }
> >
> >         int configure(lsc::ActiveState &state, const Rectangle &analogCrop,
> >                       const std::vector<double> &xPos,
> > -                     const std::vector<double> &yPos);
> > -
> > -       void queueRequest(lsc::ActiveState &state, lsc::FrameContext &context,
> > -                         const ControlList &controls);
> > -       void process(lsc::FrameContext &context, ControlList &metadata);
> > +                     const std::vector<double> &yPos)
> > +       {
> > +               LOG(Lsc, Debug) << "Sample Lsc data for " << analogCrop;
> > +               lsc::ComponentsMap lscData =
> > +                       impl_->sampleForCrop(analogCrop, xPos, yPos);
> > +
> > +               /*
> > +                * Retain a copy of the components table.
> > +                *
> > +                * We could avoid a copy here if getComponents() could
> > +                * return sets_.data() but I wasn't able to work around the
> > +                * compiler refusing it.
> > +                */
> > +               lscData_ = lscData;
> > +
> > +               sets_.setData(std::move(lscData));
> > +               state.enabled = true;
> > +
> > +               return 0;
> > +       }
> >
> >         const lsc::Components interpolateComponents(unsigned int ct)
> >         {
> > @@ -60,7 +123,7 @@ public:
> >         }
> >
> >  private:
> > -       std::unique_ptr<LscImplementation> impl_;
> > +       std::unique_ptr<LscImplementation<U>> impl_;
> >         Interpolator<lsc::Components> sets_;
> >         lsc::ComponentsMap lscData_;
> >         bool polynomial_;
> > diff --git a/src/ipa/libipa/lsc_base.cpp b/src/ipa/libipa/lsc_base.cpp
> > index adc09c15752a..b91da4e5be9d 100644
> > --- a/src/ipa/libipa/lsc_base.cpp
> > +++ b/src/ipa/libipa/lsc_base.cpp
> > @@ -86,6 +86,7 @@ void Interpolator<lsc::Components>::
> >  /**
> >   * \class LscImplementation
> >   * \brief Pure virtual base class for LSC algorithm implementations
> > + * \tparam U The fixedpoint LSC engine register format
> >   *
> >   * Defines the interface for the LSC algorithm implementation.
> >   */
> > diff --git a/src/ipa/libipa/lsc_base.h b/src/ipa/libipa/lsc_base.h
> > index 48307455362a..721fd137ef21 100644
> > --- a/src/ipa/libipa/lsc_base.h
> > +++ b/src/ipa/libipa/lsc_base.h
> > @@ -57,6 +57,7 @@ struct LscDescriptor {
> >         Size sensorSize;
> >  };
> >
> > +template<typename U>
> >  class LscImplementation
> >  {
> >  public:
> > diff --git a/src/ipa/libipa/lsc_polynomial.cpp b/src/ipa/libipa/lsc_polynomial.cpp
> > index c61c85fbf2b2..060fba9826a0 100644
> > --- a/src/ipa/libipa/lsc_polynomial.cpp
> > +++ b/src/ipa/libipa/lsc_polynomial.cpp
> > @@ -113,14 +113,10 @@ void Polynomial::setReferenceImageSize(const Size &size)
> >  } /* namespace lsc */
> >
> >  /**
> > - * \class LscPolynomial
> > - * \brief Radial Polynomial LSC algorithm implementation
> > - *
> > - * Polynomial-based LSC algorithm implementation. The LscPolynomial class
> > - * implements LSC support using a Polynomial to represent the shading artifacts
> > - * map.
> > + * \class LscPolynomialBase
> > + * \brief Base class for LscPolynomial
> >   *
> > - * \sa LscImplementation
> > + * Base class for LscPolynomial for non-templated functions.
> >   */
> >
> >  /**
> > @@ -132,8 +128,8 @@ void Polynomial::setReferenceImageSize(const Size &size)
> >   *
> >   * \return 0 on success or a negative error number otherwise
> >   */
> > -int LscPolynomial::parseLscData(const ValueNode &sets,
> > -                               const LscDescriptor &descriptor)
> > +int LscPolynomialBase::parseLscData(const ValueNode &sets,
> > +                                   const LscDescriptor &descriptor)
> >  {
> >         for (const auto &set : sets.asList()) {
> >                 uint32_t ct = set["ct"].get<uint32_t>(0);
> > @@ -174,6 +170,27 @@ int LscPolynomial::parseLscData(const ValueNode &sets,
> >  }
> >
> >  /**
> > + * \var LscPolynomialBase::lscData_
> > + * \brief The polynomial LSC data
> > + *
> > + * Maps colour temperatures to per-colour radial polynomial definitions.
> > + */
> > +
> > +/**
> > + * \class LscPolynomial
> > + * \brief Radial Polynomial LSC algorithm implementation
> > + * \tparam U The fixedpoint format used to convert gain values generated by
> > + * polynomial expansion to the register format
> > + *
> > + * Polynomial-based LSC algorithm implementation. The LscPolynomial class
> > + * implements LSC support using Polynomial to represent the shading artifacts
> > + * map.
> > + *
> > + * \sa LscImplementation
> > + */
> > +
> > +/**
> > + * \fn LscPolynomial::sampleForCrop()
> >   * \brief Re-sample the LSC components for \a cropRectangle
> >   * \param[in] cropRectangle The sensor analogue crop rectangle
> >   * \param[in] xPos List of horizontal positions of the LSC grid nodes
> > @@ -195,62 +212,7 @@ int LscPolynomial::parseLscData(const ValueNode &sets,
> >   * [0, 0.0625, 0.125, 0.1875, ... , 1]. It is expected that the first position
> >   * is 0 and the last position is 1.
> >   */
> > -lsc::ComponentsMap
> > -LscPolynomial::sampleForCrop(const Rectangle &cropRectangle,
> > -                            std::vector<double> xPos, std::vector<double> yPos)
> > -{
> > -
> > -       lsc::ComponentsMap components;
> > -
> > -       for (const auto &[t, c] : lscData_) {
> > -               lsc::Components comp;
> > -
> > -               for (const auto &[k, p] : c) {
> > -                       comp.emplace(std::piecewise_construct,
> > -                                    std::forward_as_tuple(k),
> > -                                    std::forward_as_tuple(samplePolynomial(p, xPos, yPos,
> > -                                                                           cropRectangle)));
> > -               }
> >
> > -               components[t] = comp;
> > -       }
> > -
> > -       return components;
> > -}
> > -
> > -std::vector<uint16_t>
> > -LscPolynomial::samplePolynomial(const lsc::Polynomial &poly,
> > -                               Span<const double> xPositions,
> > -                               Span<const double> yPositions,
> > -                               const Rectangle &cropRectangle)
> > -{
> > -       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> samples;
> > -
> > -       samples.reserve(xPositions.size() * yPositions.size());
> > -
> > -       for (double y : yPositions) {
> > -               for (double x : xPositions) {
> > -                       double xp = x0 + x * w;
> > -                       double yp = y0 + y * h;
> > -                       /*
> > -                        * The hardware uses 2.10 fixed point format and limits
> > -                        * the legal values to [1..3.999]. Scale and clamp the
> > -                        * sampled value accordingly.
> > -                        */
> > -                       int v = static_cast<int>(
> > -                               poly.sampleAtNormalizedPixelPos(xp, yp) *
> > -                               1024);
> > -                       v = std::clamp(v, 1024, 4095);
> > -                       samples.push_back(v);
> > -               }
> > -       }
> > -       return samples;
> > -}
> >
> >  } /* namespace ipa */
> >
> > diff --git a/src/ipa/libipa/lsc_polynomial.h b/src/ipa/libipa/lsc_polynomial.h
> > index 6d19e46a50b7..04d7465b3af0 100644
> > --- a/src/ipa/libipa/lsc_polynomial.h
> > +++ b/src/ipa/libipa/lsc_polynomial.h
> > @@ -8,6 +8,7 @@
> >
> >  #include <array>
> >  #include <map>
> > +#include <tuple>
> >  #include <vector>
> >
> >  #include <libcamera/base/span.h>
> > @@ -50,26 +51,80 @@ private:
> >
> >  } /* namespace lsc */
> >
> > -class LscPolynomial : public LscImplementation
> > +class LscPolynomialBase
> >  {
> >  private:
> >         using PolynomialComponents = std::map<std::string, lsc::Polynomial>;
> >         using PolynomialComponentsMap = std::map<unsigned int, PolynomialComponents>;
> >
> > +protected:
> > +       int parseLscData(const ValueNode &yamlSets,
> > +                        const LscDescriptor &descriptor);
> > +
> > +       PolynomialComponentsMap lscData_;
> > +};
> > +
> > +template<typename U>
> > +class LscPolynomial : public LscPolynomialBase, public LscImplementation<U>
> > +{
> >  public:
> > -       int parseLscData(const ValueNode &sets,
> > -                        const LscDescriptor &descriptor) override;
> > +       int parseLscData(const ValueNode &yamlSets,
> > +                        const LscDescriptor &descriptor) override
> > +       {
> > +               return LscPolynomialBase::parseLscData(yamlSets, descriptor);
> > +       }
> >
> >         lsc::ComponentsMap
> >         sampleForCrop(const Rectangle &cropRectangle,
> > -                     std::vector<double> xPos, std::vector<double> yPos) override;
> > +                     std::vector<double> xPos, std::vector<double> yPos) override
> > +       {
> > +               lsc::ComponentsMap components;
> > +
> > +               for (const auto &[t, c] : lscData_) {
> > +                       lsc::Components comp;
> > +
> > +                       for (const auto &[k, p] : c) {
> > +                               comp.emplace(std::piecewise_construct,
> > +                                            std::forward_as_tuple(k),
> > +                                            std::forward_as_tuple(samplePolynomial(p, xPos, yPos,
> > +                                                                                   cropRectangle)));
> > +                       }
> > +
> > +                       components[t] = comp;
> > +               }
> > +
> > +               return components;
> > +       }
> >
> >  private:
> >         std::vector<uint16_t> samplePolynomial(const lsc::Polynomial &poly,
> >                                                Span<const double> xPositions,
> >                                                Span<const double> yPositions,
> > -                                              const Rectangle &cropRectangle);
> > -       PolynomialComponentsMap lscData_;
> > +                                              const Rectangle &cropRectangle)
> > +       {
> > +               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> samples;
> > +
> > +               samples.reserve(xPositions.size() * yPositions.size());
> > +
> > +               for (double y : yPositions) {
> > +                       for (double x : xPositions) {
> > +                               double xp = x0 + x * w;
> > +                               double yp = y0 + y * h;
> > +
> > +                               float sample = static_cast<float>
> > +                                               (poly.sampleAtNormalizedPixelPos(xp, yp));
> > +
> > +                               samples.push_back(U(sample).quantized());
> > +                       }
> > +               }
> > +
> > +               return samples;
> > +       }
> >  };
> >
> >  } /* namespace ipa */
> > diff --git a/src/ipa/libipa/lsc_table.cpp b/src/ipa/libipa/lsc_table.cpp
> > index cdd2fea5ebf3..0e8f144cb873 100644
> > --- a/src/ipa/libipa/lsc_table.cpp
> > +++ b/src/ipa/libipa/lsc_table.cpp
> > @@ -14,13 +14,10 @@ LOG_DEFINE_CATEGORY(LscTable)
> >  namespace ipa {
> >
> >  /**
> > - * \class LscTable
> > - * \brief Table based LSC algorithm implementation
> > - *
> > - * Table based LSC algorithm implementation. The LSCTable class implements LSC
> > - * support using tabular LSC data.
> > + * \class LscTableBase
> > + * \brief Base class for LscTable
> >   *
> > - * \sa LscImplementation
> > + * Base class for LscTable for non-templated functions.
> >   */
> >
> >  /**
> > @@ -32,8 +29,8 @@ namespace ipa {
> >   *
> >   * \return 0 on success or a negative error number otherwise
> >   */
> > -int LscTable::parseLscData(const ValueNode &sets,
> > -                          const LscDescriptor &descriptor)
> > +int LscTableBase::parseLscData(const ValueNode &sets,
> > +                              const LscDescriptor &descriptor)
> >  {
> >         for (const auto &set : sets.asList()) {
> >                 uint32_t ct = set["ct"].get<uint32_t>(0);
> > @@ -51,8 +48,8 @@ int LscTable::parseLscData(const ValueNode &sets,
> >         return 0;
> >  }
> >
> > -int LscTable::parseLscComponent(const ValueNode &yamlSet,
> > -                               unsigned int ct, const LscDescriptor &descriptor)
> > +int LscTableBase::parseLscComponent(const ValueNode &yamlSet,
> > +                                   unsigned int ct, const LscDescriptor &descriptor)
> >  {
> >         lsc::Components component;
> >         for (auto &k : descriptor.keys) {
> > @@ -82,10 +79,10 @@ int LscTable::parseLscComponent(const ValueNode &yamlSet,
> >         return 0;
> >  }
> >
> > -std::vector<uint16_t> LscTable::parseTable(const ValueNode &tuningData,
> > -                                          const char *prop,
> > -                                          unsigned int numHCells,
> > -                                          unsigned int numVCells)
> > +std::vector<uint16_t> LscTableBase::parseTable(const ValueNode &tuningData,
> > +                                              const char *prop,
> > +                                              unsigned int numHCells,
> > +                                              unsigned int numVCells)
> >  {
> >         unsigned int lscNumSamples = numHCells * numVCells;
> >
> > @@ -102,6 +99,30 @@ std::vector<uint16_t> LscTable::parseTable(const ValueNode &tuningData,
> >         return table;
> >  }
> >
> > +/**
> > + * \var LscTableBase::lscData_
> > + * \brief The tabular LSC data
> > + *
> > + * Maps colour temperatures to per-colour channel vector of gains
> > + */
> > +
> > +/**
> > + * \class LscTable
> > + * \brief Table based LSC algorithm implementation
> > + * \tparam U The fixedpoint LSC engine register format
> > + *
> > + * Table based LSC algorithm implementation. The LscTable class implements LSC
> > + * support using tabular LSC data.
> > + *
> > + * \sa LscImplementation
> > + */
> > +
> > +/**
> > + * \fn LscTable::parseLscData()
> > + *
> > + * \copydoc LscTableBase::parseLscData()
> > + */
> > +
> >  } /* namespace ipa */
> >
> >  } /* namespace libcamera */
> > diff --git a/src/ipa/libipa/lsc_table.h b/src/ipa/libipa/lsc_table.h
> > index 3fe52afbad7b..65e04b09c3ff 100644
> > --- a/src/ipa/libipa/lsc_table.h
> > +++ b/src/ipa/libipa/lsc_table.h
> > @@ -23,11 +23,32 @@ LOG_DECLARE_CATEGORY(LscTable)
> >
> >  namespace ipa {
> >
> > -class LscTable : public LscImplementation
> > +class LscTableBase
> > +{
> > +protected:
> > +       int parseLscData(const ValueNode &sets,
> > +                        const LscDescriptor &descriptor);
> > +
> > +private:
> > +       int parseLscComponent(const ValueNode &yamlSet,
> > +                             unsigned int ct, const LscDescriptor &descriptor);
> > +       std::vector<uint16_t> parseTable(const ValueNode &tuningData,
> > +                                        const char *prop,
> > +                                        unsigned int numHCells,
> > +                                        unsigned int numVCells);
> > +protected:
> > +       lsc::ComponentsMap lscData_;
> > +};
> > +
> > +template<typename U>
> > +class LscTable : public LscTableBase, public LscImplementation<U>
> >  {
> >  public:
> >         int parseLscData(const ValueNode &sets,
> > -                        const LscDescriptor &descriptor) override;
> > +                        const LscDescriptor &descriptor) override
> > +       {
> > +               return LscTableBase::parseLscData(sets, descriptor);
> > +       }
> >
> >         lsc::ComponentsMap
> >         sampleForCrop([[maybe_unused]] const Rectangle &cropRectangle,
> > @@ -38,16 +59,6 @@ public:
> >                         << "Tabular LSC data doesn't support resampling";
> >                 return lscData_;
> >         }
> > -
> > -private:
> > -       int parseLscComponent(const ValueNode &yamlSet,
> > -                             unsigned int ct, const LscDescriptor &descriptor);
> > -       std::vector<uint16_t> parseTable(const ValueNode &tuningData,
> > -                                        const char *prop,
> > -                                        unsigned int numHCells,
> > -                                        unsigned int numVCells);
> > -
> > -       lsc::ComponentsMap lscData_;
> >  };
> >
> >  } /* namespace ipa */
> > diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h
> > index e7d3091a36c5..31156f56cd21 100644
> > --- a/src/ipa/rkisp1/algorithms/lsc.h
> > +++ b/src/ipa/rkisp1/algorithms/lsc.h
> > @@ -13,6 +13,7 @@
> >
> >  #include "libcamera/internal/value_node.h"
> >
> > +#include "libipa/fixedpoint.h"
> >  #include "libipa/lsc.h"
> >
> >  #include "algorithm.h"
> > @@ -58,7 +59,7 @@ private:
> >         unsigned int lastAppliedCt_;
> >         unsigned int lastAppliedQuantizedCt_;
> >
> > -       LscAlgorithm lscAlgo_;
> > +       LscAlgorithm<UQ<2, 10>> lscAlgo_;
> >  };
> >
> >  } /* namespace ipa::rkisp1::algorithms */
> >
> > --
> > 2.54.0
> >

Hi Jacopo,

Quoting Jacopo Mondi (2026-07-14 18:01:05)
> Hi Stefan
> 
> On Tue, Jul 14, 2026 at 04:15:26PM +0200, Stefan Klug wrote:
> > Quoting Jacopo Mondi (2026-07-08 17:51:12)
> > > The LscPolynomial::samplePolynomial function expands the radial
> > > polynomial assuming the RkISP1 Q<2, 10> register format:
> > >
> > >        /*
> > >         * The hardware uses 2.10 fixed point format and limits
> > >         * the legal values to [1..3.999]. Scale and clamp the
> > >         * sampled value accordingly.
> > >         */
> > >        int v = static_cast<int>(
> > >                poly.sampleAtNormalizedPixelPos(xp, yp) *
> > >                1024);
> > >        v = std::clamp(v, 1024, 4095);
> > >
> > > In order to remove all assumptions about the platform register format
> > > template the LscAlgorithm class with the fixed point format of the
> > > ISP Lsc engine registers.
> > >
> > >        float sample = static_cast<float>
> > >                        (poly.sampleAtNormalizedPixelPos(xp, yp));
> > >
> > >        samples.push_back(U(sample).quantized());
> > >
> > > In order to template the LscPolynomial class, template the whole class
> > > hierarchy including the LscTable class that doesn't need (yet) the
> > > template argument but will use it once re-sampling LscTable will be
> > > implemented.
> > >
> > > Split all involved classes (LscAlgorithm, LscTable and LscPolynomial) in
> > > two classes: a Lsc*Base class for non-templated function implementations
> > > and a Lsc* class for templated functions implementations.
> >
> > I'm wondering if this is really necessary. Making the implementations
> > templated and hardware format aware introduces quite some complexity.
> >
> > The polynomial resampling only happens at configure() time. So we could
> > also just return a table of floats. I see two options there:
> >
> > a) Conversion from float to fixed happens in the  LscAlgorithm class
> 
> So we should here populate a vector of potentially thoudsands of
> entries, and then iterate over it in the LscAlgorithm to quantize it ?

Yes, that would be the idea. Given that we do the calculation on float
anyways, the overhead is the temporary storage.

> 
> > In this case, the interpolation would still be done on integers. Side
> > question: Can we guarantee that all fixed point formats can be
> > interpolated based on integers? I can confirm that for the rkisp1 but
> > didn't think about the other ones.
> 
> we don't interpoalte fixed point formats. We intepolate integers, and
> the quantize them. I don't think I fully understood your question

We convert all the gain values to hardware specific fixed point formats
UQ<2, 10> on rkispa1 and UQ<2, 6> on Mali. Then we interpolate the
resulting uint16 or unt8 values with double intermediates in
interpolateVector. I was looking for cases where this interpolation
could go wrong depending on the underlying fixed point format. I
couldn't spot any. It's all fine.

> 
> >
> > b) Conversion from float to fixed happens in the platform specific
> > implementation. This would have the benefit that we could get rid of all
> 
> Likewise, populate a vector of thousands of integers and then iterate
> over it to quantize them. Why ?
> 
> > the templating. But has the downside that the interpolation would be
> > done in floats and after interpolation the conversion to fixed point
> > (That feelsexpensive, but I don't know numbers).
> >
> > Only issue is that the tables are currently stored as ints in the tuning
> > file and would need a temporary int -> float -> int conversion until fe
> > store floats in the tuning file.
> >
> > What do you think?
> 
> I don't underand the reasons to make this less efficient. And no, "It's
> complex" it's not a good enough reason.

I did give it a try.
The simplification would be:
LscPolynomial no template
LscPolynomialBase dropped
LscTable no template
LscImplementation no template

The cost would be ~90 microseconds (880us vs 970us) on a imx8mp at
configure() time. I would trade the simpler code and less templates for
that. 

But I can understand if you see it differently.
Up to you.

Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>

Best regards,
Stefan

> 
> >
> > Best regards,
> > Stefan
> >
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > > ---
> > >  src/ipa/libipa/lsc.cpp            | 151 +++++++++++++-------------------------
> > >  src/ipa/libipa/lsc.h              |  81 +++++++++++++++++---
> > >  src/ipa/libipa/lsc_base.cpp       |   1 +
> > >  src/ipa/libipa/lsc_base.h         |   1 +
> > >  src/ipa/libipa/lsc_polynomial.cpp |  90 +++++++----------------
> > >  src/ipa/libipa/lsc_polynomial.h   |  67 +++++++++++++++--
> > >  src/ipa/libipa/lsc_table.cpp      |  49 +++++++++----
> > >  src/ipa/libipa/lsc_table.h        |  35 ++++++---
> > >  src/ipa/rkisp1/algorithms/lsc.h   |   3 +-
> > >  9 files changed, 270 insertions(+), 208 deletions(-)
> > >
> > > diff --git a/src/ipa/libipa/lsc.cpp b/src/ipa/libipa/lsc.cpp
> > > index 13b1b738106b..f5646937c60a 100644
> > > --- a/src/ipa/libipa/lsc.cpp
> > > +++ b/src/ipa/libipa/lsc.cpp
> > > @@ -7,13 +7,6 @@
> > >
> > >  #include "lsc.h"
> > >
> > > -#include <libcamera/base/log.h>
> > > -
> > > -#include <libcamera/control_ids.h>
> > > -
> > > -#include "lsc_polynomial.h"
> > > -#include "lsc_table.h"
> > > -
> > >  /**
> > >   * \file lsc.h
> > >   * \brief libipa LSC algorithm
> > > @@ -48,9 +41,56 @@ namespace lsc {
> > >
> > >  } /* namespace lsc */
> > >
> > > +/**
> > > + * \class LscAlgorithmBase
> > > + * \brief Base class for LscAlgorithm
> > > + *
> > > + * Base class for LscAlgorithm for non-templated functions implementation
> > > + */
> > > +
> > > +/**
> > > + * \brief Queue a request to the LSC algorithm
> > > + * \param[in] state The LSC active state
> > > + * \param[in] context The LSC frame context
> > > + * \param[in] controls The list of controls associated with a Request
> > > + *
> > > + * Queue a new list of \a controls to the LSC algorithm.
> > > + * The only supported control is controls::LensShadingCorrectionEnable.
> > > + */
> > > +void LscAlgorithmBase::queueRequest(lsc::ActiveState &state,
> > > +                                   lsc::FrameContext &context,
> > > +                                   const ControlList &controls)
> > > +{
> > > +       const auto &lscEnable = controls.get(controls::LensShadingCorrectionEnable);
> > > +       if (lscEnable && *lscEnable != state.enabled) {
> > > +               state.enabled = *lscEnable;
> > > +
> > > +               LOG(Lsc, Debug)
> > > +                       << (state.enabled ? "Enabling" : "Disabling") << " Lsc";
> > > +
> > > +               context.update = true;
> > > +       }
> > > +
> > > +       context.enabled = state.enabled;
> > > +}
> > > +
> > > +/**
> > > + * \brief Populate the list of LSC metadata
> > > + * \param[in] context The LSC frame context
> > > + * \param[in] metadata The list of metadata
> > > + *
> > > + * Populates the list of \a metadata with controls handled by the LscAlgorithm
> > > + * class. The only supported metadata is controls::LensShadingCorrectionEnable.
> > > + */
> > > +void LscAlgorithmBase::process(lsc::FrameContext &context, ControlList &metadata)
> > > +{
> > > +       metadata.set(controls::LensShadingCorrectionEnable, context.enabled);
> > > +}
> > > +
> > >  /**
> > >   * \class LscAlgorithm
> > >   * \brief libIPA LSC algorithm implementation
> > > + * \tparam U The fixedpoint LSC engine register format
> > >   *
> > >   * Due to the optical characteristics of the lens, the light intensity received
> > >   * by the sensor is not uniform. The Lens Shading Correction algorithm applies
> > > @@ -200,6 +240,7 @@ namespace lsc {
> > >   */
> > >
> > >  /**
> > > + * \fn LscAlgorithm::init()
> > >   * \param[in] tuningData The tuning data
> > >   * \param[in] controls The IPA list of supported controls
> > >   * \param[in] descriptor The LSC engine descriptor
> > > @@ -209,42 +250,9 @@ namespace lsc {
> > >   *
> > >   * \return 0 on success, a negative error code otherwise
> > >   */
> > > -int LscAlgorithm::init(const ValueNode &tuningData, ControlInfoMap::Map &controls,
> > > -                      const LscDescriptor &descriptor)
> > > -{
> > > -       polynomial_ = false;
> > > -
> > > -       std::string type = tuningData["type"].get<std::string>("table");
> > > -       if (type == "table") {
> > > -               impl_ = std::make_unique<LscTable>();
> > > -               LOG(Lsc, Debug) << "Using table-based Lsc";
> > > -       } else if (type == "polynomial") {
> > > -               impl_ = std::make_unique<LscPolynomial>();
> > > -               polynomial_ = true;
> > > -               LOG(Lsc, Debug) << "Using polynomial Lsc";
> > > -       } else {
> > > -               LOG(Lsc, Error) << "Unsupported Lsc algorithm '"
> > > -                               << type << "'";
> > > -               return -EINVAL;
> > > -       }
> > > -
> > > -       const ValueNode &yamlSets = tuningData["sets"];
> > > -       if (!yamlSets.isList()) {
> > > -               LOG(Lsc, Error) << "'sets' parameter not found in tuning file";
> > > -               return -EINVAL;
> > > -       }
> > > -
> > > -       int ret = impl_->parseLscData(yamlSets, descriptor);
> > > -       if (ret)
> > > -               return ret;
> > > -
> > > -       controls[&controls::LensShadingCorrectionEnable] =
> > > -               ControlInfo(false, true, true);
> > > -
> > > -       return 0;
> > > -}
> > >
> > >  /**
> > > + * \fn LscAlgorithm::configure()
> > >   * \param[in] state The LSC active state
> > >   * \param[in] analogCrop The current sensor analog crop rectangle
> > >   * \param[in] xPos List of horizontal positions of the LSC grid nodes
> > > @@ -265,67 +273,6 @@ int LscAlgorithm::init(const ValueNode &tuningData, ControlInfoMap::Map &control
> > >   *
> > >   * \return 0 on success, a negative error code otherwise
> > >   */
> > > -int LscAlgorithm::configure(lsc::ActiveState &state, const Rectangle &analogCrop,
> > > -                           const std::vector<double> &xPos,
> > > -                           const std::vector<double> &yPos)
> > > -{
> > > -       LOG(Lsc, Debug) << "Sample Lsc data for " << analogCrop;
> > > -       lsc::ComponentsMap lscData =
> > > -               impl_->sampleForCrop(analogCrop, xPos, yPos);
> > > -
> > > -       /*
> > > -        * Retain a copy of the components table.
> > > -        *
> > > -        * We could avoid a copy here if getComponents() could
> > > -        * return sets_.data() but I wasn't able to work around the
> > > -        * compiler refusing it.
> > > -        */
> > > -       lscData_ = lscData;
> > > -
> > > -       sets_.setData(std::move(lscData));
> > > -       state.enabled = true;
> > > -
> > > -       return 0;
> > > -}
> > > -
> > > -/**
> > > - * \brief Queue a request to the lsc algorithm
> > > - * \param[in] state The lsc active state
> > > - * \param[in] context The lsc frame context
> > > - * \param[in] controls The list of controls associated with a Request
> > > - *
> > > - * Queue a new list of \a controls to the lsc algorithm.
> > > - * The only supported control is controls::LensShadingCorrectionEnable.
> > > - */
> > > -void LscAlgorithm::queueRequest(lsc::ActiveState &state,
> > > -                               lsc::FrameContext &context,
> > > -                               const ControlList &controls)
> > > -{
> > > -       const auto &lscEnable = controls.get(controls::LensShadingCorrectionEnable);
> > > -       if (lscEnable && *lscEnable != state.enabled) {
> > > -               state.enabled = *lscEnable;
> > > -
> > > -               LOG(Lsc, Debug)
> > > -                       << (state.enabled ? "Enabling" : "Disabling") << " Lsc";
> > > -
> > > -               context.update = true;
> > > -       }
> > > -
> > > -       context.enabled = state.enabled;
> > > -}
> > > -
> > > -/**
> > > - * \brief Populate the list of lsc metadata
> > > - * \param[in] context The lsc frame context
> > > - * \param[in] metadata The list of metadata
> > > - *
> > > - * Populates the list of \a metadata with controls handled by the LscAlgorithm
> > > - * class. The only supported metadata is controls::LensShadingCorrectionEnable.
> > > - */
> > > -void LscAlgorithm::process(lsc::FrameContext &context, ControlList &metadata)
> > > -{
> > > -       metadata.set(controls::LensShadingCorrectionEnable, context.enabled);
> > > -}
> > >
> > >  /**
> > >   * \fn LscAlgorithm::interpolateComponents
> > > diff --git a/src/ipa/libipa/lsc.h b/src/ipa/libipa/lsc.h
> > > index 9fe8ad67ea33..d48336b169f4 100644
> > > --- a/src/ipa/libipa/lsc.h
> > > +++ b/src/ipa/libipa/lsc.h
> > > @@ -8,18 +8,25 @@
> > >  #pragma once
> > >
> > >  #include <memory>
> > > +#include <string>
> > >  #include <vector>
> > >
> > > +#include <libcamera/base/log.h>
> > > +
> > > +#include <libcamera/control_ids.h>
> > >  #include <libcamera/controls.h>
> > >  #include <libcamera/geometry.h>
> > >
> > >  #include "libcamera/internal/value_node.h"
> > >
> > >  #include "interpolator.h"
> > > -#include "lsc_base.h"
> > > +#include "lsc_polynomial.h"
> > > +#include "lsc_table.h"
> > >
> > >  namespace libcamera {
> > >
> > > +LOG_DECLARE_CATEGORY(Lsc)
> > > +
> > >  namespace ipa {
> > >
> > >  namespace lsc {
> > > @@ -35,19 +42,75 @@ struct FrameContext {
> > >
> > >  } /* namespace lsc */
> > >
> > > -class LscAlgorithm
> > > +class LscAlgorithmBase
> > > +{
> > > +public:
> > > +       void queueRequest(lsc::ActiveState &state, lsc::FrameContext &context,
> > > +                         const ControlList &controls);
> > > +       void process(lsc::FrameContext &context, ControlList &metadata);
> > > +};
> > > +
> > > +template<typename U>
> > > +class LscAlgorithm : public LscAlgorithmBase
> > >  {
> > >  public:
> > >         int init(const ValueNode &tuningData, ControlInfoMap::Map &controls,
> > > -                const LscDescriptor &descriptor);
> > > +                const LscDescriptor &descriptor)
> > > +       {
> > > +               polynomial_ = false;
> > > +
> > > +               std::string type = tuningData["type"].get<std::string>("table");
> > > +               if (type == "table") {
> > > +                       impl_ = std::make_unique<LscTable<U>>();
> > > +                       LOG(Lsc, Debug) << "Using table-based Lsc";
> > > +               } else if (type == "polynomial") {
> > > +                       impl_ = std::make_unique<LscPolynomial<U>>();
> > > +                       polynomial_ = true;
> > > +                       LOG(Lsc, Debug) << "Using polynomial Lsc";
> > > +               } else {
> > > +                       LOG(Lsc, Error) << "Unsupported Lsc algorithm '"
> > > +                                       << type << "'";
> > > +                       return -EINVAL;
> > > +               }
> > > +
> > > +               const ValueNode &yamlSets = tuningData["sets"];
> > > +               if (!yamlSets.isList()) {
> > > +                       LOG(Lsc, Error) << "'sets' parameter not found in tuning file";
> > > +                       return -EINVAL;
> > > +               }
> > > +
> > > +               int ret = impl_->parseLscData(yamlSets, descriptor);
> > > +               if (ret)
> > > +                       return ret;
> > > +
> > > +               controls[&controls::LensShadingCorrectionEnable] =
> > > +                       ControlInfo(false, true, true);
> > > +
> > > +               return 0;
> > > +       }
> > >
> > >         int configure(lsc::ActiveState &state, const Rectangle &analogCrop,
> > >                       const std::vector<double> &xPos,
> > > -                     const std::vector<double> &yPos);
> > > -
> > > -       void queueRequest(lsc::ActiveState &state, lsc::FrameContext &context,
> > > -                         const ControlList &controls);
> > > -       void process(lsc::FrameContext &context, ControlList &metadata);
> > > +                     const std::vector<double> &yPos)
> > > +       {
> > > +               LOG(Lsc, Debug) << "Sample Lsc data for " << analogCrop;
> > > +               lsc::ComponentsMap lscData =
> > > +                       impl_->sampleForCrop(analogCrop, xPos, yPos);
> > > +
> > > +               /*
> > > +                * Retain a copy of the components table.
> > > +                *
> > > +                * We could avoid a copy here if getComponents() could
> > > +                * return sets_.data() but I wasn't able to work around the
> > > +                * compiler refusing it.
> > > +                */
> > > +               lscData_ = lscData;
> > > +
> > > +               sets_.setData(std::move(lscData));
> > > +               state.enabled = true;
> > > +
> > > +               return 0;
> > > +       }
> > >
> > >         const lsc::Components interpolateComponents(unsigned int ct)
> > >         {
> > > @@ -60,7 +123,7 @@ public:
> > >         }
> > >
> > >  private:
> > > -       std::unique_ptr<LscImplementation> impl_;
> > > +       std::unique_ptr<LscImplementation<U>> impl_;
> > >         Interpolator<lsc::Components> sets_;
> > >         lsc::ComponentsMap lscData_;
> > >         bool polynomial_;
> > > diff --git a/src/ipa/libipa/lsc_base.cpp b/src/ipa/libipa/lsc_base.cpp
> > > index adc09c15752a..b91da4e5be9d 100644
> > > --- a/src/ipa/libipa/lsc_base.cpp
> > > +++ b/src/ipa/libipa/lsc_base.cpp
> > > @@ -86,6 +86,7 @@ void Interpolator<lsc::Components>::
> > >  /**
> > >   * \class LscImplementation
> > >   * \brief Pure virtual base class for LSC algorithm implementations
> > > + * \tparam U The fixedpoint LSC engine register format
> > >   *
> > >   * Defines the interface for the LSC algorithm implementation.
> > >   */
> > > diff --git a/src/ipa/libipa/lsc_base.h b/src/ipa/libipa/lsc_base.h
> > > index 48307455362a..721fd137ef21 100644
> > > --- a/src/ipa/libipa/lsc_base.h
> > > +++ b/src/ipa/libipa/lsc_base.h
> > > @@ -57,6 +57,7 @@ struct LscDescriptor {
> > >         Size sensorSize;
> > >  };
> > >
> > > +template<typename U>
> > >  class LscImplementation
> > >  {
> > >  public:
> > > diff --git a/src/ipa/libipa/lsc_polynomial.cpp b/src/ipa/libipa/lsc_polynomial.cpp
> > > index c61c85fbf2b2..060fba9826a0 100644
> > > --- a/src/ipa/libipa/lsc_polynomial.cpp
> > > +++ b/src/ipa/libipa/lsc_polynomial.cpp
> > > @@ -113,14 +113,10 @@ void Polynomial::setReferenceImageSize(const Size &size)
> > >  } /* namespace lsc */
> > >
> > >  /**
> > > - * \class LscPolynomial
> > > - * \brief Radial Polynomial LSC algorithm implementation
> > > - *
> > > - * Polynomial-based LSC algorithm implementation. The LscPolynomial class
> > > - * implements LSC support using a Polynomial to represent the shading artifacts
> > > - * map.
> > > + * \class LscPolynomialBase
> > > + * \brief Base class for LscPolynomial
> > >   *
> > > - * \sa LscImplementation
> > > + * Base class for LscPolynomial for non-templated functions.
> > >   */
> > >
> > >  /**
> > > @@ -132,8 +128,8 @@ void Polynomial::setReferenceImageSize(const Size &size)
> > >   *
> > >   * \return 0 on success or a negative error number otherwise
> > >   */
> > > -int LscPolynomial::parseLscData(const ValueNode &sets,
> > > -                               const LscDescriptor &descriptor)
> > > +int LscPolynomialBase::parseLscData(const ValueNode &sets,
> > > +                                   const LscDescriptor &descriptor)
> > >  {
> > >         for (const auto &set : sets.asList()) {
> > >                 uint32_t ct = set["ct"].get<uint32_t>(0);
> > > @@ -174,6 +170,27 @@ int LscPolynomial::parseLscData(const ValueNode &sets,
> > >  }
> > >
> > >  /**
> > > + * \var LscPolynomialBase::lscData_
> > > + * \brief The polynomial LSC data
> > > + *
> > > + * Maps colour temperatures to per-colour radial polynomial definitions.
> > > + */
> > > +
> > > +/**
> > > + * \class LscPolynomial
> > > + * \brief Radial Polynomial LSC algorithm implementation
> > > + * \tparam U The fixedpoint format used to convert gain values generated by
> > > + * polynomial expansion to the register format
> > > + *
> > > + * Polynomial-based LSC algorithm implementation. The LscPolynomial class
> > > + * implements LSC support using Polynomial to represent the shading artifacts
> > > + * map.
> > > + *
> > > + * \sa LscImplementation
> > > + */
> > > +
> > > +/**
> > > + * \fn LscPolynomial::sampleForCrop()
> > >   * \brief Re-sample the LSC components for \a cropRectangle
> > >   * \param[in] cropRectangle The sensor analogue crop rectangle
> > >   * \param[in] xPos List of horizontal positions of the LSC grid nodes
> > > @@ -195,62 +212,7 @@ int LscPolynomial::parseLscData(const ValueNode &sets,
> > >   * [0, 0.0625, 0.125, 0.1875, ... , 1]. It is expected that the first position
> > >   * is 0 and the last position is 1.
> > >   */
> > > -lsc::ComponentsMap
> > > -LscPolynomial::sampleForCrop(const Rectangle &cropRectangle,
> > > -                            std::vector<double> xPos, std::vector<double> yPos)
> > > -{
> > > -
> > > -       lsc::ComponentsMap components;
> > > -
> > > -       for (const auto &[t, c] : lscData_) {
> > > -               lsc::Components comp;
> > > -
> > > -               for (const auto &[k, p] : c) {
> > > -                       comp.emplace(std::piecewise_construct,
> > > -                                    std::forward_as_tuple(k),
> > > -                                    std::forward_as_tuple(samplePolynomial(p, xPos, yPos,
> > > -                                                                           cropRectangle)));
> > > -               }
> > >
> > > -               components[t] = comp;
> > > -       }
> > > -
> > > -       return components;
> > > -}
> > > -
> > > -std::vector<uint16_t>
> > > -LscPolynomial::samplePolynomial(const lsc::Polynomial &poly,
> > > -                               Span<const double> xPositions,
> > > -                               Span<const double> yPositions,
> > > -                               const Rectangle &cropRectangle)
> > > -{
> > > -       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> samples;
> > > -
> > > -       samples.reserve(xPositions.size() * yPositions.size());
> > > -
> > > -       for (double y : yPositions) {
> > > -               for (double x : xPositions) {
> > > -                       double xp = x0 + x * w;
> > > -                       double yp = y0 + y * h;
> > > -                       /*
> > > -                        * The hardware uses 2.10 fixed point format and limits
> > > -                        * the legal values to [1..3.999]. Scale and clamp the
> > > -                        * sampled value accordingly.
> > > -                        */
> > > -                       int v = static_cast<int>(
> > > -                               poly.sampleAtNormalizedPixelPos(xp, yp) *
> > > -                               1024);
> > > -                       v = std::clamp(v, 1024, 4095);
> > > -                       samples.push_back(v);
> > > -               }
> > > -       }
> > > -       return samples;
> > > -}
> > >
> > >  } /* namespace ipa */
> > >
> > > diff --git a/src/ipa/libipa/lsc_polynomial.h b/src/ipa/libipa/lsc_polynomial.h
> > > index 6d19e46a50b7..04d7465b3af0 100644
> > > --- a/src/ipa/libipa/lsc_polynomial.h
> > > +++ b/src/ipa/libipa/lsc_polynomial.h
> > > @@ -8,6 +8,7 @@
> > >
> > >  #include <array>
> > >  #include <map>
> > > +#include <tuple>
> > >  #include <vector>
> > >
> > >  #include <libcamera/base/span.h>
> > > @@ -50,26 +51,80 @@ private:
> > >
> > >  } /* namespace lsc */
> > >
> > > -class LscPolynomial : public LscImplementation
> > > +class LscPolynomialBase
> > >  {
> > >  private:
> > >         using PolynomialComponents = std::map<std::string, lsc::Polynomial>;
> > >         using PolynomialComponentsMap = std::map<unsigned int, PolynomialComponents>;
> > >
> > > +protected:
> > > +       int parseLscData(const ValueNode &yamlSets,
> > > +                        const LscDescriptor &descriptor);
> > > +
> > > +       PolynomialComponentsMap lscData_;
> > > +};
> > > +
> > > +template<typename U>
> > > +class LscPolynomial : public LscPolynomialBase, public LscImplementation<U>
> > > +{
> > >  public:
> > > -       int parseLscData(const ValueNode &sets,
> > > -                        const LscDescriptor &descriptor) override;
> > > +       int parseLscData(const ValueNode &yamlSets,
> > > +                        const LscDescriptor &descriptor) override
> > > +       {
> > > +               return LscPolynomialBase::parseLscData(yamlSets, descriptor);
> > > +       }
> > >
> > >         lsc::ComponentsMap
> > >         sampleForCrop(const Rectangle &cropRectangle,
> > > -                     std::vector<double> xPos, std::vector<double> yPos) override;
> > > +                     std::vector<double> xPos, std::vector<double> yPos) override
> > > +       {
> > > +               lsc::ComponentsMap components;
> > > +
> > > +               for (const auto &[t, c] : lscData_) {
> > > +                       lsc::Components comp;
> > > +
> > > +                       for (const auto &[k, p] : c) {
> > > +                               comp.emplace(std::piecewise_construct,
> > > +                                            std::forward_as_tuple(k),
> > > +                                            std::forward_as_tuple(samplePolynomial(p, xPos, yPos,
> > > +                                                                                   cropRectangle)));
> > > +                       }
> > > +
> > > +                       components[t] = comp;
> > > +               }
> > > +
> > > +               return components;
> > > +       }
> > >
> > >  private:
> > >         std::vector<uint16_t> samplePolynomial(const lsc::Polynomial &poly,
> > >                                                Span<const double> xPositions,
> > >                                                Span<const double> yPositions,
> > > -                                              const Rectangle &cropRectangle);
> > > -       PolynomialComponentsMap lscData_;
> > > +                                              const Rectangle &cropRectangle)
> > > +       {
> > > +               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> samples;
> > > +
> > > +               samples.reserve(xPositions.size() * yPositions.size());
> > > +
> > > +               for (double y : yPositions) {
> > > +                       for (double x : xPositions) {
> > > +                               double xp = x0 + x * w;
> > > +                               double yp = y0 + y * h;
> > > +
> > > +                               float sample = static_cast<float>
> > > +                                               (poly.sampleAtNormalizedPixelPos(xp, yp));
> > > +
> > > +                               samples.push_back(U(sample).quantized());
> > > +                       }
> > > +               }
> > > +
> > > +               return samples;
> > > +       }
> > >  };
> > >
> > >  } /* namespace ipa */
> > > diff --git a/src/ipa/libipa/lsc_table.cpp b/src/ipa/libipa/lsc_table.cpp
> > > index cdd2fea5ebf3..0e8f144cb873 100644
> > > --- a/src/ipa/libipa/lsc_table.cpp
> > > +++ b/src/ipa/libipa/lsc_table.cpp
> > > @@ -14,13 +14,10 @@ LOG_DEFINE_CATEGORY(LscTable)
> > >  namespace ipa {
> > >
> > >  /**
> > > - * \class LscTable
> > > - * \brief Table based LSC algorithm implementation
> > > - *
> > > - * Table based LSC algorithm implementation. The LSCTable class implements LSC
> > > - * support using tabular LSC data.
> > > + * \class LscTableBase
> > > + * \brief Base class for LscTable
> > >   *
> > > - * \sa LscImplementation
> > > + * Base class for LscTable for non-templated functions.
> > >   */
> > >
> > >  /**
> > > @@ -32,8 +29,8 @@ namespace ipa {
> > >   *
> > >   * \return 0 on success or a negative error number otherwise
> > >   */
> > > -int LscTable::parseLscData(const ValueNode &sets,
> > > -                          const LscDescriptor &descriptor)
> > > +int LscTableBase::parseLscData(const ValueNode &sets,
> > > +                              const LscDescriptor &descriptor)
> > >  {
> > >         for (const auto &set : sets.asList()) {
> > >                 uint32_t ct = set["ct"].get<uint32_t>(0);
> > > @@ -51,8 +48,8 @@ int LscTable::parseLscData(const ValueNode &sets,
> > >         return 0;
> > >  }
> > >
> > > -int LscTable::parseLscComponent(const ValueNode &yamlSet,
> > > -                               unsigned int ct, const LscDescriptor &descriptor)
> > > +int LscTableBase::parseLscComponent(const ValueNode &yamlSet,
> > > +                                   unsigned int ct, const LscDescriptor &descriptor)
> > >  {
> > >         lsc::Components component;
> > >         for (auto &k : descriptor.keys) {
> > > @@ -82,10 +79,10 @@ int LscTable::parseLscComponent(const ValueNode &yamlSet,
> > >         return 0;
> > >  }
> > >
> > > -std::vector<uint16_t> LscTable::parseTable(const ValueNode &tuningData,
> > > -                                          const char *prop,
> > > -                                          unsigned int numHCells,
> > > -                                          unsigned int numVCells)
> > > +std::vector<uint16_t> LscTableBase::parseTable(const ValueNode &tuningData,
> > > +                                              const char *prop,
> > > +                                              unsigned int numHCells,
> > > +                                              unsigned int numVCells)
> > >  {
> > >         unsigned int lscNumSamples = numHCells * numVCells;
> > >
> > > @@ -102,6 +99,30 @@ std::vector<uint16_t> LscTable::parseTable(const ValueNode &tuningData,
> > >         return table;
> > >  }
> > >
> > > +/**
> > > + * \var LscTableBase::lscData_
> > > + * \brief The tabular LSC data
> > > + *
> > > + * Maps colour temperatures to per-colour channel vector of gains
> > > + */
> > > +
> > > +/**
> > > + * \class LscTable
> > > + * \brief Table based LSC algorithm implementation
> > > + * \tparam U The fixedpoint LSC engine register format
> > > + *
> > > + * Table based LSC algorithm implementation. The LscTable class implements LSC
> > > + * support using tabular LSC data.
> > > + *
> > > + * \sa LscImplementation
> > > + */
> > > +
> > > +/**
> > > + * \fn LscTable::parseLscData()
> > > + *
> > > + * \copydoc LscTableBase::parseLscData()
> > > + */
> > > +
> > >  } /* namespace ipa */
> > >
> > >  } /* namespace libcamera */
> > > diff --git a/src/ipa/libipa/lsc_table.h b/src/ipa/libipa/lsc_table.h
> > > index 3fe52afbad7b..65e04b09c3ff 100644
> > > --- a/src/ipa/libipa/lsc_table.h
> > > +++ b/src/ipa/libipa/lsc_table.h
> > > @@ -23,11 +23,32 @@ LOG_DECLARE_CATEGORY(LscTable)
> > >
> > >  namespace ipa {
> > >
> > > -class LscTable : public LscImplementation
> > > +class LscTableBase
> > > +{
> > > +protected:
> > > +       int parseLscData(const ValueNode &sets,
> > > +                        const LscDescriptor &descriptor);
> > > +
> > > +private:
> > > +       int parseLscComponent(const ValueNode &yamlSet,
> > > +                             unsigned int ct, const LscDescriptor &descriptor);
> > > +       std::vector<uint16_t> parseTable(const ValueNode &tuningData,
> > > +                                        const char *prop,
> > > +                                        unsigned int numHCells,
> > > +                                        unsigned int numVCells);
> > > +protected:
> > > +       lsc::ComponentsMap lscData_;
> > > +};
> > > +
> > > +template<typename U>
> > > +class LscTable : public LscTableBase, public LscImplementation<U>
> > >  {
> > >  public:
> > >         int parseLscData(const ValueNode &sets,
> > > -                        const LscDescriptor &descriptor) override;
> > > +                        const LscDescriptor &descriptor) override
> > > +       {
> > > +               return LscTableBase::parseLscData(sets, descriptor);
> > > +       }
> > >
> > >         lsc::ComponentsMap
> > >         sampleForCrop([[maybe_unused]] const Rectangle &cropRectangle,
> > > @@ -38,16 +59,6 @@ public:
> > >                         << "Tabular LSC data doesn't support resampling";
> > >                 return lscData_;
> > >         }
> > > -
> > > -private:
> > > -       int parseLscComponent(const ValueNode &yamlSet,
> > > -                             unsigned int ct, const LscDescriptor &descriptor);
> > > -       std::vector<uint16_t> parseTable(const ValueNode &tuningData,
> > > -                                        const char *prop,
> > > -                                        unsigned int numHCells,
> > > -                                        unsigned int numVCells);
> > > -
> > > -       lsc::ComponentsMap lscData_;
> > >  };
> > >
> > >  } /* namespace ipa */
> > > diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h
> > > index e7d3091a36c5..31156f56cd21 100644
> > > --- a/src/ipa/rkisp1/algorithms/lsc.h
> > > +++ b/src/ipa/rkisp1/algorithms/lsc.h
> > > @@ -13,6 +13,7 @@
> > >
> > >  #include "libcamera/internal/value_node.h"
> > >
> > > +#include "libipa/fixedpoint.h"
> > >  #include "libipa/lsc.h"
> > >
> > >  #include "algorithm.h"
> > > @@ -58,7 +59,7 @@ private:
> > >         unsigned int lastAppliedCt_;
> > >         unsigned int lastAppliedQuantizedCt_;
> > >
> > > -       LscAlgorithm lscAlgo_;
> > > +       LscAlgorithm<UQ<2, 10>> lscAlgo_;
> > >  };
> > >
> > >  } /* namespace ipa::rkisp1::algorithms */
> > >
> > > --
> > > 2.54.0
> > >