[PATCH v2 9/9] ipa: rkisp1: Add polynomial LSC loader
Stefan Klug
stefan.klug at ideasonboard.com
Tue Sep 17 09:25:44 CEST 2024
Hi Kieran,
Thank you for the review.
On Fri, Sep 13, 2024 at 11:35:07AM +0100, Kieran Bingham wrote:
> Quoting Stefan Klug (2024-09-13 08:57:27)
> > Add a loader that is capable to load polynomial coefficients from the
> > tuning files. The polynoms are sampled at load time to reduce the
>
> I think 'polynom' might be a german term? I haven't heard it used before
> and google suddenly started giving me lots of non-english hits.
>
> Perhaps just "The polynomials are sampled" ... but I understood it
> anyway so I don't mind keeping it ;-)
>
> > computational overhead at runtime.
> >
> > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > ---
> > src/ipa/rkisp1/algorithms/lsc.cpp | 126 +++++++++++++++++++++++++++++-
> > 1 file changed, 125 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
> > index 12867b8f7d0f..b0ad2234cd11 100644
> > --- a/src/ipa/rkisp1/algorithms/lsc.cpp
> > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp
> > @@ -16,6 +16,7 @@
> >
> > #include "libcamera/internal/yaml_parser.h"
> >
> > +#include "libipa/lsc_polynomial.h"
> > #include "linux/rkisp1-config.h"
> >
> > /**
> > @@ -70,6 +71,123 @@ namespace ipa::rkisp1::algorithms {
> >
> > LOG_DEFINE_CATEGORY(RkISP1Lsc)
> >
> > +class LscPolynomialLoader
> > +{
> > +public:
> > + LscPolynomialLoader(const Size &sensorSize,
> > + const Rectangle &cropRectangle,
> > + const std::vector<double> &xSizes,
> > + const std::vector<double> &ySizes)
> > + : sensorSize_(sensorSize),
> > + cropRectangle_(cropRectangle),
> > + xSizes_(xSizes),
> > + ySizes_(ySizes)
> > + {
> > + }
> > +
> > + int parseLscData(const YamlObject &yamlSets,
> > + std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> > + {
> > + const auto &sets = yamlSets.asList();
> > + for (const auto &yamlSet : sets) {
> > + std::optional<LscPolynomial> pr, pgr, pgb, pb;
> > + uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> > +
> > + if (lscData.count(ct)) {
> > + LOG(RkISP1Lsc, Error)
> > + << "Multiple sets found for "
> > + << "color temperature " << ct;
> > + return -EINVAL;
> > + }
> > +
> > + LensShadingCorrection::Components &set = lscData[ct];
> > + pr = yamlSet["r"].get<LscPolynomial>();
> > + pgr = yamlSet["gr"].get<LscPolynomial>();
> > + pgb = yamlSet["gb"].get<LscPolynomial>();
> > + pb = yamlSet["b"].get<LscPolynomial>();
> > +
> > + if (!(pr || pgr || pgb || pb)) {
> > + LOG(RkISP1Lsc, Error)
> > + << "Failed to parse polynomial for "
> > + << "colour temperature " << ct;
> > + return -EINVAL;
> > + }
> > +
> > + set.ct = ct;
> > + pr->setReferenceImageSize(sensorSize_);
> > + pgr->setReferenceImageSize(sensorSize_);
> > + pgb->setReferenceImageSize(sensorSize_);
> > + pb->setReferenceImageSize(sensorSize_);
> > + set.r = samplePolynomial(*pr);
> > + set.gr = samplePolynomial(*pgr);
> > + set.gb = samplePolynomial(*pgb);
> > + set.b = samplePolynomial(*pb);
> > + }
> > +
> > + if (lscData.empty()) {
> > + LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > + }
> > +
> > +private:
>
> It's private implementation detail, but this might be a good place to
> add a comment or explicit reference to any external documentation that
> details the mirroring going on here.
>
> I was confused to start with ...
I added documentation for that in v3.
>
> > + std::vector<double> sizesListToPositions(const std::vector<double> &sizes)
> > + {
> > + const int half = sizes.size();
> > + std::vector<double> res(half * 2 + 1);
> > + double x = 0.0;
> > +
> > + res[half] = 0.5;
> > + for (int i = 1; i <= half; i++) {
> > + x += sizes[half - i];
> > + res[half - i] = 0.5 - x;
> > + res[half + i] = 0.5 + x;
> > + }
> > +
> > + return res;
> > + }
> > +
> > + std::vector<uint16_t> samplePolynomial(const LscPolynomial &poly)
> > + {
> > + constexpr int k = RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> > +
> > + double m = poly.getM();
> > + double x0 = cropRectangle_.x / m;
> > + double y0 = cropRectangle_.y / m;
> > + double w = cropRectangle_.width / m;
> > + double h = cropRectangle_.height / m;
> > + std::vector<uint16_t> res;
> > +
> > + assert(xSizes_.size() * 2 + 1 == k);
>
> Do we evaluate/validate this when loading ?
Yes, this get's checked in parseSizes(). For completeness sake I added
the same assert for ySizes.
>
> > +
> > + res.reserve(k * k);
> > +
> > + std::vector<double> xPos(sizesListToPositions(xSizes_));
> > + std::vector<double> yPos(sizesListToPositions(ySizes_));
> > +
> > + for (int x = 0; x < k; x++) {
> > + for (int y = 0; y < k; y++) {
> > + double xp = x0 + xPos[x] * w;
> > + double yp = y0 + yPos[y] * h;
> > + int v = static_cast<int>(
> > + poly.sampleAtNormalizedPixelPos(xp, yp) *
> > + 1024);
> > +
> > + v = std::min(std::max(v, 1024), 4095);
> > + res.push_back(v);
> > + }
> > + }
> > + return res;
> > + }
> > +
> > + Size sensorSize_;
> > + Rectangle cropRectangle_;
> > + const std::vector<double> &xSizes_;
> > + const std::vector<double> &ySizes_;
> > +};
> > +
> > class LscClassicLoader
> > {
> > public:
> > @@ -193,11 +311,17 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
> > std::map<unsigned int, Components> lscData;
> > int res = 0;
> > std::optional<std::string> type = tuningData["type"].get<std::string>();
> > - if (!type.has_value()) {
> > + if (!tuningData.contains("type")) {
>
> Does this need to change? We're not removing type variable so we can
> still use it in both cases right?
Rereading that sentence a few times I realized that this check shouldn't
have gone into patch 6. I cleaned that up for v3.
Best regards,
Stefan
>
> > LOG(RkISP1Lsc, Warning) << "LSC data is in classic format. "
> > << "This will be deprecated soon.";
> > auto loader = LscClassicLoader();
> > res = loader.parseLscData(yamlSets, lscData);
> > + } else if (*type == "polynomial") {
> > + auto loader = LscPolynomialLoader(context.sensorInfo.activeAreaSize,
> > + context.sensorInfo.analogCrop,
> > + xSize_,
> > + ySize_);
> > + res = loader.parseLscData(yamlSets, lscData);
> > } else {
> > LOG(RkISP1Lsc, Error) << "Unsupported LSC type '" << *type << "'";
> > res = -EINVAL;
> > --
> > 2.43.0
> >
More information about the libcamera-devel
mailing list