[v3,13/15] ipa: rkisp1: lsc: Resample polynomial lens shading tables at configure time
diff mbox series

Message ID 20260114-sklug-lsc-resampling-v2-dev-v3-13-80cd24f4dd61@ideasonboard.com
State Superseded
Headers show
Series
  • Add resampling support for polynomial LSC data
Related show

Commit Message

Stefan Klug Jan. 14, 2026, 11:58 a.m. UTC 
The lens shading correction is always applied based on the sensor crop
bounds. This leads to incorrect lens shading correction for analog crops
that do not cover the whole sensor.

To fix that, we need to adapt the lens shading table for the selected
analog crop at configure time. Introduce an abstract ShadingDescriptor
class that holds the lens shading information that can then be sampled
at configure time for a specific crop rectangle.

Resampling for a specific crop is only implemented for polynomial lsc
data. For tabular data, a warning is logged and the unmodified table is
returned. This matches the current functionality for tabular data and is
a huge improvement for polynomial data.

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

---

Changes in v2:
- Replaced "auto const" by "const auto"
- Replaces some vector parameters by Spans
- Changed sampleForCrop to return the components
- Replaced min/max by clamp
- Replaced map.swap() by move assignment
---
 src/ipa/rkisp1/algorithms/lsc.cpp | 233 ++++++++++++++++++++++++--------------
 src/ipa/rkisp1/algorithms/lsc.h   |  13 +++
 2 files changed, 159 insertions(+), 87 deletions(-)

Comments

Kieran Bingham Jan. 16, 2026, 12:39 p.m. UTC | #1 
Quoting Stefan Klug (2026-01-14 11:58:06)
> The lens shading correction is always applied based on the sensor crop
> bounds. This leads to incorrect lens shading correction for analog crops
> that do not cover the whole sensor.
> 
> To fix that, we need to adapt the lens shading table for the selected
> analog crop at configure time. Introduce an abstract ShadingDescriptor
> class that holds the lens shading information that can then be sampled
> at configure time for a specific crop rectangle.
> 
> Resampling for a specific crop is only implemented for polynomial lsc
> data. For tabular data, a warning is logged and the unmodified table is
> returned. This matches the current functionality for tabular data and is
> a huge improvement for polynomial data.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> 
> ---
> 
> Changes in v2:
> - Replaced "auto const" by "const auto"
> - Replaces some vector parameters by Spans
> - Changed sampleForCrop to return the components
> - Replaced min/max by clamp
> - Replaced map.swap() by move assignment
> ---
>  src/ipa/rkisp1/algorithms/lsc.cpp | 233 ++++++++++++++++++++++++--------------
>  src/ipa/rkisp1/algorithms/lsc.h   |  13 +++
>  2 files changed, 159 insertions(+), 87 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
> index 5808381fd84dc49fd8548e4d00b2fb4b7015f509..8427c48f65b2e6e200f834db506939c6c85fd2a3 100644
> --- a/src/ipa/rkisp1/algorithms/lsc.cpp
> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp
> @@ -72,38 +72,127 @@ namespace {
>  
>  constexpr int kColourTemperatureQuantization = 10;
>  
> -class LscPolynomialLoader
> +class LscPolynomialShadingDescriptor : public LensShadingCorrection::ShadingDescriptor
>  {
>  public:
> -       LscPolynomialLoader(const Size &sensorSize,
> -                           const Rectangle &cropRectangle,
> -                           const std::vector<double> &xSizes,
> -                           const std::vector<double> &ySizes)
> -               : sensorSize_(sensorSize),
> -                 cropRectangle_(cropRectangle),
> -                 xSizes_(xSizes),
> -                 ySizes_(ySizes)
> +       LscPolynomialShadingDescriptor(const LscPolynomial &pr, const LscPolynomial &pgr,
> +                                      const LscPolynomial &pgb, const LscPolynomial &pb)
> +               : pr_(pr), pgr_(pgr), pgb_(pgb), pb_(pb)
>         {
>         }
>  
> -       int parseLscData(const YamlObject &yamlSets,
> -                        std::map<unsigned int, LensShadingCorrection::Components> &lscData);
> +       LensShadingCorrection::Components sampleForCrop(const Rectangle &cropRectangle,
> +                                                       Span<const double> xSizes,
> +                                                       Span<const double> ySizes) override;
>  
>  private:
> -       std::vector<double> sizesListToPositions(const std::vector<double> &sizes);
>         std::vector<uint16_t> samplePolynomial(const LscPolynomial &poly,
>                                                Span<const double> xPositions,
>                                                Span<const double> yPositions,
>                                                const Rectangle &cropRectangle);
>  
> +       std::vector<double> sizesListToPositions(Span<const double> sizes);
> +
> +       LscPolynomial pr_;
> +       LscPolynomial pgr_;
> +       LscPolynomial pgb_;
> +       LscPolynomial pb_;
> +};
> +
> +LensShadingCorrection::Components
> +LscPolynomialShadingDescriptor::sampleForCrop(const Rectangle &cropRectangle,
> +                                             Span<const double> xSizes,
> +                                             Span<const double> ySizes)
> +{
> +       std::vector<double> xPos = sizesListToPositions(xSizes);
> +       std::vector<double> yPos = sizesListToPositions(ySizes);
> +
> +       return {
> +               .r = samplePolynomial(pr_, xPos, yPos, cropRectangle),
> +               .gr = samplePolynomial(pgr_, xPos, yPos, cropRectangle),
> +               .gb = samplePolynomial(pgb_, xPos, yPos, cropRectangle),
> +               .b = samplePolynomial(pb_, xPos, yPos, cropRectangle)
> +       };
> +}
> +
> +std::vector<uint16_t>
> +LscPolynomialShadingDescriptor::samplePolynomial(const LscPolynomial &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.
> +                        */

Looks like a good candidate for UQ<2,10> which would already clamp to 3.99902

			float s = poly.sampleAtNormalizedPixelPos(xp, yp);
			s = std::max(s, 1);
			samples.push_back(UQ<2, 10>(s).quantized());

Would that make the code more expressive or readable ?

verifying ...

+++ b/test/ipa/libipa/fixedpoint.cpp
@@ -136,6 +136,9 @@ protected:
                fails += quantizedCheck<UQ<1, 7>>(-100.0f, 0b0'0000000, 0.0f);
                fails += quantizedCheck<UQ<1, 7>>(+100.0f, 0b1'1111111, 1.99219f);
 
+               introduce<UQ<2, 10>>("UQ2.10");
+               fails += quantizedCheck<UQ<2, 10>>(4, 4095, 3.99902f);

...

UQ2.10(0 .. 3.99902)  Min: [0x0000:0] -- Max: [0x0fff:3.99902] Step:0.000976562
  Checking 4 == [0x0fff:3.99902]


But this is just a code move of existing code - so anything like that
could be on top. (Although I see the clamp is already an update to the
moving code).

Anyway, there's nothing I object to in this so:


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


> +                       int v = static_cast<int>(
> +                               poly.sampleAtNormalizedPixelPos(xp, yp) *
> +                               1024);
> +                       v = std::clamp(v, 1024, 4095);
> +                       samples.push_back(v);
> +               }
> +       }
> +       return samples;
> +}
> +
> +/*
> + * The rkisp1 LSC grid spacing is defined by the cell sizes on the first half of
> + * the grid. This is then mirrored in hardware to the other half. See
> + * parseSizes() for further details. For easier handling, this function converts
> + * the cell sizes of half the grid to a list of position of the whole grid (on
> + * one axis). Example:
> + *
> + * input:   | 0.2 | 0.3 |
> + * output: 0.0   0.2   0.5   0.8   1.0
> + */
> +std::vector<double>
> +LscPolynomialShadingDescriptor::sizesListToPositions(Span<const double> sizes)
> +{
> +       const int half = sizes.size();
> +       std::vector<double> positions(half * 2 + 1);
> +       double x = 0.0;
> +
> +       positions[half] = 0.5;
> +       for (int i = 1; i <= half; i++) {
> +               x += sizes[half - i];
> +               positions[half - i] = 0.5 - x;
> +               positions[half + i] = 0.5 + x;
> +       }
> +
> +       return positions;
> +}
> +
> +class LscPolynomialLoader
> +{
> +public:
> +       LscPolynomialLoader(const Size &sensorSize)
> +               : sensorSize_(sensorSize)
> +       {
> +       }
> +
> +       int parseLscData(const YamlObject &yamlSets,
> +                        LensShadingCorrection::ShadingDescriptorMap &lscData);
> +
> +private:
>         Size sensorSize_;
> -       Rectangle cropRectangle_;
> -       const std::vector<double> &xSizes_;
> -       const std::vector<double> &ySizes_;
>  };
>  
>  int LscPolynomialLoader::parseLscData(const YamlObject &yamlSets,
> -                                     std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> +                                     LensShadingCorrection::ShadingDescriptorMap &lscData)
>  {
>         const auto &sets = yamlSets.asList();
>         for (const auto &yamlSet : sets) {
> @@ -117,7 +206,6 @@ int LscPolynomialLoader::parseLscData(const YamlObject &yamlSets,
>                         return -EINVAL;
>                 }
>  
> -               LensShadingCorrection::Components &set = lscData[ct];
>                 pr = yamlSet["r"].get<LscPolynomial>();
>                 pgr = yamlSet["gr"].get<LscPolynomial>();
>                 pgb = yamlSet["gb"].get<LscPolynomial>();
> @@ -135,12 +223,9 @@ int LscPolynomialLoader::parseLscData(const YamlObject &yamlSets,
>                 pgb->setReferenceImageSize(sensorSize_);
>                 pb->setReferenceImageSize(sensorSize_);
>  
> -               std::vector<double> xPos = sizesListToPositions(xSizes_);
> -               std::vector<double> yPos = sizesListToPositions(ySizes_);
> -               set.r = samplePolynomial(*pr, xPos, yPos, cropRectangle_);
> -               set.gr = samplePolynomial(*pgr, xPos, yPos, cropRectangle_);
> -               set.gb = samplePolynomial(*pgb, xPos, yPos, cropRectangle_);
> -               set.b = samplePolynomial(*pb, xPos, yPos, cropRectangle_);
> +               lscData.emplace(
> +                       ct, std::make_unique<LscPolynomialShadingDescriptor>(
> +                                   *pr, *pgr, *pgb, *pb));
>         }
>  
>         if (lscData.empty()) {
> @@ -151,70 +236,33 @@ int LscPolynomialLoader::parseLscData(const YamlObject &yamlSets,
>         return 0;
>  }
>  
> -/*
> - * The rkisp1 LSC grid spacing is defined by the cell sizes on the first half of
> - * the grid. This is then mirrored in hardware to the other half. See
> - * parseSizes() for further details. For easier handling, this function converts
> - * the cell sizes of half the grid to a list of position of the whole grid (on
> - * one axis). Example:
> - *
> - * input:   | 0.2 | 0.3 |
> - * output: 0.0   0.2   0.5   0.8   1.0
> - */
> -std::vector<double> LscPolynomialLoader::sizesListToPositions(const std::vector<double> &sizes)
> +class LscTableShadingDescriptor : public LensShadingCorrection::ShadingDescriptor
>  {
> -       const int half = sizes.size();
> -       std::vector<double> positions(half * 2 + 1);
> -       double x = 0.0;
> -
> -       positions[half] = 0.5;
> -       for (int i = 1; i <= half; i++) {
> -               x += sizes[half - i];
> -               positions[half - i] = 0.5 - x;
> -               positions[half + i] = 0.5 + x;
> +public:
> +       LscTableShadingDescriptor(LensShadingCorrection::Components components)
> +               : lscData_(std::move(components))
> +       {
>         }
>  
> -       return positions;
> -}
> -
> -std::vector<uint16_t> LscPolynomialLoader::samplePolynomial(const LscPolynomial &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::min(std::max(v, 1024), 4095);
> -                       samples.push_back(v);
> -               }
> +       LensShadingCorrection::Components
> +       sampleForCrop([[maybe_unused]] const Rectangle &cropRectangle,
> +                     [[maybe_unused]] Span<const double> xSizes,
> +                     [[maybe_unused]] Span<const double> ySizes)
> +       {
> +               LOG(RkISP1Lsc, Warning)
> +                       << "Tabular LSC data doesn't support resampling.";
> +               return lscData_;
>         }
> -       return samples;
> -}
> +
> +private:
> +       LensShadingCorrection::Components lscData_;
> +};
>  
>  class LscTableLoader
>  {
>  public:
>         int parseLscData(const YamlObject &yamlSets,
> -                        std::map<unsigned int, LensShadingCorrection::Components> &lscData);
> +                        LensShadingCorrection::ShadingDescriptorMap &lscData);
>  
>  private:
>         std::vector<uint16_t> parseTable(const YamlObject &tuningData,
> @@ -222,7 +270,7 @@ private:
>  };
>  
>  int LscTableLoader::parseLscData(const YamlObject &yamlSets,
> -                                std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> +                                LensShadingCorrection::ShadingDescriptorMap &lscData)
>  {
>         const auto &sets = yamlSets.asList();
>  
> @@ -236,8 +284,7 @@ int LscTableLoader::parseLscData(const YamlObject &yamlSets,
>                         return -EINVAL;
>                 }
>  
> -               LensShadingCorrection::Components &set = lscData[ct];
> -
> +               LensShadingCorrection::Components set;
>                 set.r = parseTable(yamlSet, "r");
>                 set.gr = parseTable(yamlSet, "gr");
>                 set.gb = parseTable(yamlSet, "gb");
> @@ -250,6 +297,9 @@ int LscTableLoader::parseLscData(const YamlObject &yamlSets,
>                                 << " is missing tables";
>                         return -EINVAL;
>                 }
> +
> +               lscData.emplace(
> +                       ct, std::make_unique<LscTableShadingDescriptor>(std::move(set)));
>         }
>  
>         if (lscData.empty()) {
> @@ -341,7 +391,7 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
>                 return -EINVAL;
>         }
>  
> -       std::map<unsigned int, Components> lscData;
> +       ShadingDescriptorMap lscData;
>         int ret = 0;
>  
>         std::string type = tuningData["type"].get<std::string>("table");
> @@ -351,10 +401,11 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
>                 ret = loader.parseLscData(yamlSets, lscData);
>         } else if (type == "polynomial") {
>                 LOG(RkISP1Lsc, Debug) << "Loading polynomial LSC data.";
> -               auto loader = LscPolynomialLoader(context.sensorInfo.activeAreaSize,
> -                                                 context.sensorInfo.analogCrop,
> -                                                 xSize_,
> -                                                 ySize_);
> +               /*
> +                * \todo: Most likely the reference frame should be native_size.
> +                * Let's wait how the internal discussions progress.
> +                */
> +               auto loader = LscPolynomialLoader(context.sensorInfo.activeAreaSize);
>                 ret = loader.parseLscData(yamlSets, lscData);
>         } else {
>                 LOG(RkISP1Lsc, Error) << "Unsupported LSC data type '"
> @@ -365,7 +416,7 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
>         if (ret)
>                 return ret;
>  
> -       sets_.setData(std::move(lscData));
> +       shadingDescriptors_ = std::move(lscData);
>  
>         return 0;
>  }
> @@ -401,6 +452,14 @@ int LensShadingCorrection::configure(IPAContext &context,
>                 yGrad_[i] = std::round(32768 / ySizes_[i]);
>         }
>  
> +       LOG(RkISP1Lsc, Debug) << "Sample LSC data for " << configInfo.analogCrop;
> +       std::map<unsigned int, LensShadingCorrection::Components> shadingData;
> +       for (const auto &[t, descriptor] : shadingDescriptors_)
> +               shadingData[t] = descriptor->sampleForCrop(configInfo.analogCrop,
> +                                                          xSize_, ySize_);
> +
> +       sets_.setData(std::move(shadingData));
> +
>         context.configuration.lsc.enabled = true;
>         return 0;
>  }
> diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h
> index 7b68dda1a0d4257c345e0f1b77aa498f8183c92f..3097740a6cb2ce9063a4ba087856987a489b0ab6 100644
> --- a/src/ipa/rkisp1/algorithms/lsc.h
> +++ b/src/ipa/rkisp1/algorithms/lsc.h
> @@ -8,6 +8,7 @@
>  #pragma once
>  
>  #include <map>
> +#include <memory>
>  
>  #include "libipa/interpolator.h"
>  
> @@ -36,10 +37,22 @@ public:
>                 std::vector<uint16_t> b;
>         };
>  
> +       class ShadingDescriptor
> +       {
> +       public:
> +               virtual ~ShadingDescriptor() = default;
> +               virtual Components sampleForCrop(const Rectangle &cropRectangle,
> +                                                Span<const double> xSizes,
> +                                                Span<const double> ySizes) = 0;
> +       };
> +
> +       using ShadingDescriptorMap = std::map<unsigned int, std::unique_ptr<ShadingDescriptor>>;
> +
>  private:
>         void setParameters(rkisp1_cif_isp_lsc_config &config);
>         void copyTable(rkisp1_cif_isp_lsc_config &config, const Components &set0);
>  
> +       ShadingDescriptorMap shadingDescriptors_;
>         ipa::Interpolator<Components> sets_;
>         std::vector<double> xSize_;
>         std::vector<double> ySize_;
> 
> -- 
> 2.51.0
>
Barnabás Pőcze Jan. 16, 2026, 2:51 p.m. UTC | #2 
2026. 01. 14. 12:58 keltezéssel, Stefan Klug írta:
> The lens shading correction is always applied based on the sensor crop
> bounds. This leads to incorrect lens shading correction for analog crops
> that do not cover the whole sensor.
> 
> To fix that, we need to adapt the lens shading table for the selected
> analog crop at configure time. Introduce an abstract ShadingDescriptor
> class that holds the lens shading information that can then be sampled
> at configure time for a specific crop rectangle.
> 
> Resampling for a specific crop is only implemented for polynomial lsc
> data. For tabular data, a warning is logged and the unmodified table is
> returned. This matches the current functionality for tabular data and is
> a huge improvement for polynomial data.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> 
> ---
> 
> Changes in v2:
> - Replaced "auto const" by "const auto"
> - Replaces some vector parameters by Spans
> - Changed sampleForCrop to return the components
> - Replaced min/max by clamp
> - Replaced map.swap() by move assignment
> ---
>   src/ipa/rkisp1/algorithms/lsc.cpp | 233 ++++++++++++++++++++++++--------------
>   src/ipa/rkisp1/algorithms/lsc.h   |  13 +++
>   2 files changed, 159 insertions(+), 87 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
> index 5808381fd84dc49fd8548e4d00b2fb4b7015f509..8427c48f65b2e6e200f834db506939c6c85fd2a3 100644
> --- a/src/ipa/rkisp1/algorithms/lsc.cpp
> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp
> @@ -72,38 +72,127 @@ namespace {
> [...]
> -std::vector<double> LscPolynomialLoader::sizesListToPositions(const std::vector<double> &sizes)
> +class LscTableShadingDescriptor : public LensShadingCorrection::ShadingDescriptor
>   {
> -	const int half = sizes.size();
> -	std::vector<double> positions(half * 2 + 1);
> -	double x = 0.0;
> -
> -	positions[half] = 0.5;
> -	for (int i = 1; i <= half; i++) {
> -		x += sizes[half - i];
> -		positions[half - i] = 0.5 - x;
> -		positions[half + i] = 0.5 + x;
> +public:
> +	LscTableShadingDescriptor(LensShadingCorrection::Components components)
> +		: lscData_(std::move(components))
> +	{
>   	}
>   
> -	return positions;
> -}
> -
> -std::vector<uint16_t> LscPolynomialLoader::samplePolynomial(const LscPolynomial &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::min(std::max(v, 1024), 4095);
> -			samples.push_back(v);
> -		}
> +	LensShadingCorrection::Components
> +	sampleForCrop([[maybe_unused]] const Rectangle &cropRectangle,
> +		      [[maybe_unused]] Span<const double> xSizes,
> +		      [[maybe_unused]] Span<const double> ySizes)

Missing `override`.


> +	{
> +		LOG(RkISP1Lsc, Warning)
> +			<< "Tabular LSC data doesn't support resampling.";

Small thing, but I noticed it while testing: most log message don't have punctuation at the end.



> +		return lscData_;
>   	}
> -	return samples;
> -}
> +
> +private:
> +	LensShadingCorrection::Components lscData_;
> +};
>   
>   class LscTableLoader
>   {
>   public:
>   	int parseLscData(const YamlObject &yamlSets,
> -			 std::map<unsigned int, LensShadingCorrection::Components> &lscData);
> +			 LensShadingCorrection::ShadingDescriptorMap &lscData);
>   
>   private:
>   	std::vector<uint16_t> parseTable(const YamlObject &tuningData,
> @@ -222,7 +270,7 @@ private:
>   };
>   
>   int LscTableLoader::parseLscData(const YamlObject &yamlSets,
> -				 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> +				 LensShadingCorrection::ShadingDescriptorMap &lscData)
>   {
>   	const auto &sets = yamlSets.asList();
>   
> @@ -236,8 +284,7 @@ int LscTableLoader::parseLscData(const YamlObject &yamlSets,
>   			return -EINVAL;
>   		}
>   
> -		LensShadingCorrection::Components &set = lscData[ct];
> -
> +		LensShadingCorrection::Components set;
>   		set.r = parseTable(yamlSet, "r");
>   		set.gr = parseTable(yamlSet, "gr");
>   		set.gb = parseTable(yamlSet, "gb");
> @@ -250,6 +297,9 @@ int LscTableLoader::parseLscData(const YamlObject &yamlSets,
>   				<< " is missing tables";
>   			return -EINVAL;
>   		}
> +
> +		lscData.emplace(
> +			ct, std::make_unique<LscTableShadingDescriptor>(std::move(set)));
>   	}
>   
>   	if (lscData.empty()) {
> @@ -341,7 +391,7 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
>   		return -EINVAL;
>   	}
>   
> -	std::map<unsigned int, Components> lscData;
> +	ShadingDescriptorMap lscData;
>   	int ret = 0;
>   
>   	std::string type = tuningData["type"].get<std::string>("table");
> @@ -351,10 +401,11 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
>   		ret = loader.parseLscData(yamlSets, lscData);
>   	} else if (type == "polynomial") {
>   		LOG(RkISP1Lsc, Debug) << "Loading polynomial LSC data.";
> -		auto loader = LscPolynomialLoader(context.sensorInfo.activeAreaSize,
> -						  context.sensorInfo.analogCrop,
> -						  xSize_,
> -						  ySize_);
> +		/*
> +		 * \todo: Most likely the reference frame should be native_size.
> +		 * Let's wait how the internal discussions progress.
> +		 */
> +		auto loader = LscPolynomialLoader(context.sensorInfo.activeAreaSize);
>   		ret = loader.parseLscData(yamlSets, lscData);
>   	} else {
>   		LOG(RkISP1Lsc, Error) << "Unsupported LSC data type '"
> @@ -365,7 +416,7 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
>   	if (ret)
>   		return ret;
>   
> -	sets_.setData(std::move(lscData));
> +	shadingDescriptors_ = std::move(lscData);
>   
>   	return 0;
>   }
> @@ -401,6 +452,14 @@ int LensShadingCorrection::configure(IPAContext &context,
>   		yGrad_[i] = std::round(32768 / ySizes_[i]);
>   	}
>   
> +	LOG(RkISP1Lsc, Debug) << "Sample LSC data for " << configInfo.analogCrop;
> +	std::map<unsigned int, LensShadingCorrection::Components> shadingData;
> +	for (const auto &[t, descriptor] : shadingDescriptors_)
> +		shadingData[t] = descriptor->sampleForCrop(configInfo.analogCrop,
> +							   xSize_, ySize_);
> +
> +	sets_.setData(std::move(shadingData));
> +

As far as I understand, these previous lines are the important parts, the rest
is just to make this possible, mostly moving things. I also compared the current
and proposed source files themselves, and things look ok to me.


Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>


>   	context.configuration.lsc.enabled = true;
>   	return 0;
>   }
> diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h
> index 7b68dda1a0d4257c345e0f1b77aa498f8183c92f..3097740a6cb2ce9063a4ba087856987a489b0ab6 100644
> --- a/src/ipa/rkisp1/algorithms/lsc.h
> +++ b/src/ipa/rkisp1/algorithms/lsc.h
> @@ -8,6 +8,7 @@
>   #pragma once
>   
>   #include <map>
> +#include <memory>
>   
>   #include "libipa/interpolator.h"
>   
> @@ -36,10 +37,22 @@ public:
>   		std::vector<uint16_t> b;
>   	};
>   
> +	class ShadingDescriptor
> +	{
> +	public:
> +		virtual ~ShadingDescriptor() = default;
> +		virtual Components sampleForCrop(const Rectangle &cropRectangle,
> +						 Span<const double> xSizes,
> +						 Span<const double> ySizes) = 0;
> +	};
> +
> +	using ShadingDescriptorMap = std::map<unsigned int, std::unique_ptr<ShadingDescriptor>>;
> +
>   private:
>   	void setParameters(rkisp1_cif_isp_lsc_config &config);
>   	void copyTable(rkisp1_cif_isp_lsc_config &config, const Components &set0);
>   
> +	ShadingDescriptorMap shadingDescriptors_;
>   	ipa::Interpolator<Components> sets_;
>   	std::vector<double> xSize_;
>   	std::vector<double> ySize_;
>

Patch
diff mbox series 

diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
index 5808381fd84dc49fd8548e4d00b2fb4b7015f509..8427c48f65b2e6e200f834db506939c6c85fd2a3 100644
--- a/src/ipa/rkisp1/algorithms/lsc.cpp
+++ b/src/ipa/rkisp1/algorithms/lsc.cpp
@@ -72,38 +72,127 @@  namespace {
 
 constexpr int kColourTemperatureQuantization = 10;
 
-class LscPolynomialLoader
+class LscPolynomialShadingDescriptor : public LensShadingCorrection::ShadingDescriptor
 {
 public:
-	LscPolynomialLoader(const Size &sensorSize,
-			    const Rectangle &cropRectangle,
-			    const std::vector<double> &xSizes,
-			    const std::vector<double> &ySizes)
-		: sensorSize_(sensorSize),
-		  cropRectangle_(cropRectangle),
-		  xSizes_(xSizes),
-		  ySizes_(ySizes)
+	LscPolynomialShadingDescriptor(const LscPolynomial &pr, const LscPolynomial &pgr,
+				       const LscPolynomial &pgb, const LscPolynomial &pb)
+		: pr_(pr), pgr_(pgr), pgb_(pgb), pb_(pb)
 	{
 	}
 
-	int parseLscData(const YamlObject &yamlSets,
-			 std::map<unsigned int, LensShadingCorrection::Components> &lscData);
+	LensShadingCorrection::Components sampleForCrop(const Rectangle &cropRectangle,
+							Span<const double> xSizes,
+							Span<const double> ySizes) override;
 
 private:
-	std::vector<double> sizesListToPositions(const std::vector<double> &sizes);
 	std::vector<uint16_t> samplePolynomial(const LscPolynomial &poly,
 					       Span<const double> xPositions,
 					       Span<const double> yPositions,
 					       const Rectangle &cropRectangle);
 
+	std::vector<double> sizesListToPositions(Span<const double> sizes);
+
+	LscPolynomial pr_;
+	LscPolynomial pgr_;
+	LscPolynomial pgb_;
+	LscPolynomial pb_;
+};
+
+LensShadingCorrection::Components
+LscPolynomialShadingDescriptor::sampleForCrop(const Rectangle &cropRectangle,
+					      Span<const double> xSizes,
+					      Span<const double> ySizes)
+{
+	std::vector<double> xPos = sizesListToPositions(xSizes);
+	std::vector<double> yPos = sizesListToPositions(ySizes);
+
+	return {
+		.r = samplePolynomial(pr_, xPos, yPos, cropRectangle),
+		.gr = samplePolynomial(pgr_, xPos, yPos, cropRectangle),
+		.gb = samplePolynomial(pgb_, xPos, yPos, cropRectangle),
+		.b = samplePolynomial(pb_, xPos, yPos, cropRectangle)
+	};
+}
+
+std::vector<uint16_t>
+LscPolynomialShadingDescriptor::samplePolynomial(const LscPolynomial &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;
+}
+
+/*
+ * The rkisp1 LSC grid spacing is defined by the cell sizes on the first half of
+ * the grid. This is then mirrored in hardware to the other half. See
+ * parseSizes() for further details. For easier handling, this function converts
+ * the cell sizes of half the grid to a list of position of the whole grid (on
+ * one axis). Example:
+ *
+ * input:   | 0.2 | 0.3 |
+ * output: 0.0   0.2   0.5   0.8   1.0
+ */
+std::vector<double>
+LscPolynomialShadingDescriptor::sizesListToPositions(Span<const double> sizes)
+{
+	const int half = sizes.size();
+	std::vector<double> positions(half * 2 + 1);
+	double x = 0.0;
+
+	positions[half] = 0.5;
+	for (int i = 1; i <= half; i++) {
+		x += sizes[half - i];
+		positions[half - i] = 0.5 - x;
+		positions[half + i] = 0.5 + x;
+	}
+
+	return positions;
+}
+
+class LscPolynomialLoader
+{
+public:
+	LscPolynomialLoader(const Size &sensorSize)
+		: sensorSize_(sensorSize)
+	{
+	}
+
+	int parseLscData(const YamlObject &yamlSets,
+			 LensShadingCorrection::ShadingDescriptorMap &lscData);
+
+private:
 	Size sensorSize_;
-	Rectangle cropRectangle_;
-	const std::vector<double> &xSizes_;
-	const std::vector<double> &ySizes_;
 };
 
 int LscPolynomialLoader::parseLscData(const YamlObject &yamlSets,
-				      std::map<unsigned int, LensShadingCorrection::Components> &lscData)
+				      LensShadingCorrection::ShadingDescriptorMap &lscData)
 {
 	const auto &sets = yamlSets.asList();
 	for (const auto &yamlSet : sets) {
@@ -117,7 +206,6 @@  int LscPolynomialLoader::parseLscData(const YamlObject &yamlSets,
 			return -EINVAL;
 		}
 
-		LensShadingCorrection::Components &set = lscData[ct];
 		pr = yamlSet["r"].get<LscPolynomial>();
 		pgr = yamlSet["gr"].get<LscPolynomial>();
 		pgb = yamlSet["gb"].get<LscPolynomial>();
@@ -135,12 +223,9 @@  int LscPolynomialLoader::parseLscData(const YamlObject &yamlSets,
 		pgb->setReferenceImageSize(sensorSize_);
 		pb->setReferenceImageSize(sensorSize_);
 
-		std::vector<double> xPos = sizesListToPositions(xSizes_);
-		std::vector<double> yPos = sizesListToPositions(ySizes_);
-		set.r = samplePolynomial(*pr, xPos, yPos, cropRectangle_);
-		set.gr = samplePolynomial(*pgr, xPos, yPos, cropRectangle_);
-		set.gb = samplePolynomial(*pgb, xPos, yPos, cropRectangle_);
-		set.b = samplePolynomial(*pb, xPos, yPos, cropRectangle_);
+		lscData.emplace(
+			ct, std::make_unique<LscPolynomialShadingDescriptor>(
+				    *pr, *pgr, *pgb, *pb));
 	}
 
 	if (lscData.empty()) {
@@ -151,70 +236,33 @@  int LscPolynomialLoader::parseLscData(const YamlObject &yamlSets,
 	return 0;
 }
 
-/*
- * The rkisp1 LSC grid spacing is defined by the cell sizes on the first half of
- * the grid. This is then mirrored in hardware to the other half. See
- * parseSizes() for further details. For easier handling, this function converts
- * the cell sizes of half the grid to a list of position of the whole grid (on
- * one axis). Example:
- *
- * input:   | 0.2 | 0.3 |
- * output: 0.0   0.2   0.5   0.8   1.0
- */
-std::vector<double> LscPolynomialLoader::sizesListToPositions(const std::vector<double> &sizes)
+class LscTableShadingDescriptor : public LensShadingCorrection::ShadingDescriptor
 {
-	const int half = sizes.size();
-	std::vector<double> positions(half * 2 + 1);
-	double x = 0.0;
-
-	positions[half] = 0.5;
-	for (int i = 1; i <= half; i++) {
-		x += sizes[half - i];
-		positions[half - i] = 0.5 - x;
-		positions[half + i] = 0.5 + x;
+public:
+	LscTableShadingDescriptor(LensShadingCorrection::Components components)
+		: lscData_(std::move(components))
+	{
 	}
 
-	return positions;
-}
-
-std::vector<uint16_t> LscPolynomialLoader::samplePolynomial(const LscPolynomial &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::min(std::max(v, 1024), 4095);
-			samples.push_back(v);
-		}
+	LensShadingCorrection::Components
+	sampleForCrop([[maybe_unused]] const Rectangle &cropRectangle,
+		      [[maybe_unused]] Span<const double> xSizes,
+		      [[maybe_unused]] Span<const double> ySizes)
+	{
+		LOG(RkISP1Lsc, Warning)
+			<< "Tabular LSC data doesn't support resampling.";
+		return lscData_;
 	}
-	return samples;
-}
+
+private:
+	LensShadingCorrection::Components lscData_;
+};
 
 class LscTableLoader
 {
 public:
 	int parseLscData(const YamlObject &yamlSets,
-			 std::map<unsigned int, LensShadingCorrection::Components> &lscData);
+			 LensShadingCorrection::ShadingDescriptorMap &lscData);
 
 private:
 	std::vector<uint16_t> parseTable(const YamlObject &tuningData,
@@ -222,7 +270,7 @@  private:
 };
 
 int LscTableLoader::parseLscData(const YamlObject &yamlSets,
-				 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
+				 LensShadingCorrection::ShadingDescriptorMap &lscData)
 {
 	const auto &sets = yamlSets.asList();
 
@@ -236,8 +284,7 @@  int LscTableLoader::parseLscData(const YamlObject &yamlSets,
 			return -EINVAL;
 		}
 
-		LensShadingCorrection::Components &set = lscData[ct];
-
+		LensShadingCorrection::Components set;
 		set.r = parseTable(yamlSet, "r");
 		set.gr = parseTable(yamlSet, "gr");
 		set.gb = parseTable(yamlSet, "gb");
@@ -250,6 +297,9 @@  int LscTableLoader::parseLscData(const YamlObject &yamlSets,
 				<< " is missing tables";
 			return -EINVAL;
 		}
+
+		lscData.emplace(
+			ct, std::make_unique<LscTableShadingDescriptor>(std::move(set)));
 	}
 
 	if (lscData.empty()) {
@@ -341,7 +391,7 @@  int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
 		return -EINVAL;
 	}
 
-	std::map<unsigned int, Components> lscData;
+	ShadingDescriptorMap lscData;
 	int ret = 0;
 
 	std::string type = tuningData["type"].get<std::string>("table");
@@ -351,10 +401,11 @@  int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
 		ret = loader.parseLscData(yamlSets, lscData);
 	} else if (type == "polynomial") {
 		LOG(RkISP1Lsc, Debug) << "Loading polynomial LSC data.";
-		auto loader = LscPolynomialLoader(context.sensorInfo.activeAreaSize,
-						  context.sensorInfo.analogCrop,
-						  xSize_,
-						  ySize_);
+		/*
+		 * \todo: Most likely the reference frame should be native_size.
+		 * Let's wait how the internal discussions progress.
+		 */
+		auto loader = LscPolynomialLoader(context.sensorInfo.activeAreaSize);
 		ret = loader.parseLscData(yamlSets, lscData);
 	} else {
 		LOG(RkISP1Lsc, Error) << "Unsupported LSC data type '"
@@ -365,7 +416,7 @@  int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
 	if (ret)
 		return ret;
 
-	sets_.setData(std::move(lscData));
+	shadingDescriptors_ = std::move(lscData);
 
 	return 0;
 }
@@ -401,6 +452,14 @@  int LensShadingCorrection::configure(IPAContext &context,
 		yGrad_[i] = std::round(32768 / ySizes_[i]);
 	}
 
+	LOG(RkISP1Lsc, Debug) << "Sample LSC data for " << configInfo.analogCrop;
+	std::map<unsigned int, LensShadingCorrection::Components> shadingData;
+	for (const auto &[t, descriptor] : shadingDescriptors_)
+		shadingData[t] = descriptor->sampleForCrop(configInfo.analogCrop,
+							   xSize_, ySize_);
+
+	sets_.setData(std::move(shadingData));
+
 	context.configuration.lsc.enabled = true;
 	return 0;
 }
diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h
index 7b68dda1a0d4257c345e0f1b77aa498f8183c92f..3097740a6cb2ce9063a4ba087856987a489b0ab6 100644
--- a/src/ipa/rkisp1/algorithms/lsc.h
+++ b/src/ipa/rkisp1/algorithms/lsc.h
@@ -8,6 +8,7 @@ 
 #pragma once
 
 #include <map>
+#include <memory>
 
 #include "libipa/interpolator.h"
 
@@ -36,10 +37,22 @@  public:
 		std::vector<uint16_t> b;
 	};
 
+	class ShadingDescriptor
+	{
+	public:
+		virtual ~ShadingDescriptor() = default;
+		virtual Components sampleForCrop(const Rectangle &cropRectangle,
+						 Span<const double> xSizes,
+						 Span<const double> ySizes) = 0;
+	};
+
+	using ShadingDescriptorMap = std::map<unsigned int, std::unique_ptr<ShadingDescriptor>>;
+
 private:
 	void setParameters(rkisp1_cif_isp_lsc_config &config);
 	void copyTable(rkisp1_cif_isp_lsc_config &config, const Components &set0);
 
+	ShadingDescriptorMap shadingDescriptors_;
 	ipa::Interpolator<Components> sets_;
 	std::vector<double> xSize_;
 	std::vector<double> ySize_;