| Message ID | 20240920133941.90629-10-stefan.klug@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Quoting Stefan Klug (2024-09-20 14:39:24) > Add a loader that is capable of loading polynomial coefficients from the > tuning files. The polynomial is sampled at load time to reduce the > computational overhead at runtime. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > --- > Changes in v3: > - Fixes bug with lsc table beeing rotated by 90 degrees That was just a minor bug right ;-) > - Added documentation on sizesListToPositions() > - Refactored loader selection code > --- > src/ipa/rkisp1/algorithms/lsc.cpp | 147 +++++++++++++++++++++++++++++- > 1 file changed, 145 insertions(+), 2 deletions(-) > > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp > index 44c97f0e1a10..e210b32ff380 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,132 @@ 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: > + /* > + * The lsc grid has custom spacing defined on half the range (see > + * parseSizes() for details). For easier handling this function converts > + * the spaces vector to positions and mirrors them. E.g.: > + * > + * input: | 0.2 | 0.3 | > + * output: 0.0 0.2 0.5 0.8 1.0 > + */ > + 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); > + assert(ySizes_.size() * 2 + 1 == k); > + > + res.reserve(k * k); > + > + std::vector<double> xPos(sizesListToPositions(xSizes_)); > + std::vector<double> yPos(sizesListToPositions(ySizes_)); > + > + for (int y = 0; y < k; y++) { > + for (int x = 0; x < k; x++) { > + 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); It's not immediately obvious why we're using values of 1024 and 4095 here ... or why we're clamping between v (above 1024) and 4095. I can make guesses, but acomment above would be helpful to any reader to clear it up. > + res.push_back(v); > + } > + } > + return res; > + } > + > + Size sensorSize_; > + Rectangle cropRectangle_; > + const std::vector<double> &xSizes_; > + const std::vector<double> &ySizes_; > +}; > + > class LscTableLoader > { > public: > @@ -192,8 +319,24 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context, > > std::map<unsigned int, Components> lscData; > int res = 0; > - auto loader = LscTableLoader(); > - res = loader.parseLscData(yamlSets, lscData); > + std::string type = tuningData["type"].get<std::string>("table"); > + if (type == "table") { > + LOG(RkISP1Lsc, Debug) << "Loading tabular LSC data."; > + auto loader = LscTableLoader(); > + res = 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_); > + res = loader.parseLscData(yamlSets, lscData); > + } else { > + LOG(RkISP1Lsc, Error) << "Unsupported LSC data type '" > + << type << "'"; I like the syntax and expressiveness on the default values here - so we know type is set to something else in this block. With comments above handled anyway you see fit... Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > + res = -EINVAL; > + } > + > if (res) > return res; > > -- > 2.43.0 >
Hi Stefan, Thank you for the patch. On Fri, Sep 20, 2024 at 03:39:24PM +0200, Stefan Klug wrote: > Add a loader that is capable of loading polynomial coefficients from the > tuning files. The polynomial is sampled at load time to reduce the > computational overhead at runtime. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > --- > Changes in v3: > - Fixes bug with lsc table beeing rotated by 90 degrees > - Added documentation on sizesListToPositions() > - Refactored loader selection code > --- > src/ipa/rkisp1/algorithms/lsc.cpp | 147 +++++++++++++++++++++++++++++- > 1 file changed, 145 insertions(+), 2 deletions(-) > > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp > index 44c97f0e1a10..e210b32ff380 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,132 @@ 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; > + } Don't make the functions inline. class LscPolynomialLoader { public: LscPolynomialLoader(const Size &sensorSize, const Rectangle &cropRectangle, const std::vector<double> &xSizes, const std::vector<double> &ySizes); int parseLscData(const YamlObject &yamlSets, std::map<unsigned int, LensShadingCorrection::Components> &lscData); ... }; LscPolynomialLoader::LscPolynomialLoader(const Size &sensorSize, const Rectangle &cropRectangle, const std::vector<double> &xSizes, const std::vector<double> &ySizes) { ... } ... That will reduce indentation. > + > +private: > + /* > + * The lsc grid has custom spacing defined on half the range (see s/lsc/LSC/ > + * parseSizes() for details). For easier handling this function converts > + * the spaces vector to positions and mirrors them. E.g.: > + * > + * input: | 0.2 | 0.3 | > + * output: 0.0 0.2 0.5 0.8 1.0 > + */ > + std::vector<double> sizesListToPositions(const std::vector<double> &sizes) > + { > + const int half = sizes.size(); > + std::vector<double> res(half * 2 + 1); s/res/pos/ (or positions) "res" (I assume it stands for result) is a bit like "tmp", it doesn't describe very well what the variable contains. Same below. > + 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); > + assert(ySizes_.size() * 2 + 1 == k); I think the belongs to the constructor. But better, you shouldn't assert like this. If the tuning file contains a wrong size, you'll crash. This should be handled gracefully, with a check in LensShadingCorrection::init, and the 'k' constant above moved to the beginning of the file. Furthermore, does this class actually depend on the number of samples the hardware expects ? > + > + res.reserve(k * k); > + > + std::vector<double> xPos(sizesListToPositions(xSizes_)); > + std::vector<double> yPos(sizesListToPositions(ySizes_)); > + > + for (int y = 0; y < k; y++) { > + for (int x = 0; x < k; x++) { > + 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 LscTableLoader > { > public: > @@ -192,8 +319,24 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context, > > std::map<unsigned int, Components> lscData; > int res = 0; For error codes we use "ret". > - auto loader = LscTableLoader(); > - res = loader.parseLscData(yamlSets, lscData); > + std::string type = tuningData["type"].get<std::string>("table"); > + if (type == "table") { > + LOG(RkISP1Lsc, Debug) << "Loading tabular LSC data."; > + auto loader = LscTableLoader(); > + res = 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_); > + res = loader.parseLscData(yamlSets, lscData); > + } else { > + LOG(RkISP1Lsc, Error) << "Unsupported LSC data type '" > + << type << "'"; > + res = -EINVAL; > + } > + > if (res) > return res; >
diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp index 44c97f0e1a10..e210b32ff380 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,132 @@ 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: + /* + * The lsc grid has custom spacing defined on half the range (see + * parseSizes() for details). For easier handling this function converts + * the spaces vector to positions and mirrors them. E.g.: + * + * input: | 0.2 | 0.3 | + * output: 0.0 0.2 0.5 0.8 1.0 + */ + 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); + assert(ySizes_.size() * 2 + 1 == k); + + res.reserve(k * k); + + std::vector<double> xPos(sizesListToPositions(xSizes_)); + std::vector<double> yPos(sizesListToPositions(ySizes_)); + + for (int y = 0; y < k; y++) { + for (int x = 0; x < k; x++) { + 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 LscTableLoader { public: @@ -192,8 +319,24 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context, std::map<unsigned int, Components> lscData; int res = 0; - auto loader = LscTableLoader(); - res = loader.parseLscData(yamlSets, lscData); + std::string type = tuningData["type"].get<std::string>("table"); + if (type == "table") { + LOG(RkISP1Lsc, Debug) << "Loading tabular LSC data."; + auto loader = LscTableLoader(); + res = 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_); + res = loader.parseLscData(yamlSets, lscData); + } else { + LOG(RkISP1Lsc, Error) << "Unsupported LSC data type '" + << type << "'"; + res = -EINVAL; + } + if (res) return res;
Add a loader that is capable of loading polynomial coefficients from the tuning files. The polynomial is sampled at load time to reduce the computational overhead at runtime. Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- Changes in v3: - Fixes bug with lsc table beeing rotated by 90 degrees - Added documentation on sizesListToPositions() - Refactored loader selection code --- src/ipa/rkisp1/algorithms/lsc.cpp | 147 +++++++++++++++++++++++++++++- 1 file changed, 145 insertions(+), 2 deletions(-)