| Message ID | 20260708-libipa-algorithms-v5-25-0759d0359f52@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Quoting Jacopo Mondi (2026-07-08 17:51:07) > Port the RkISP1 LSC algorithm to use the LscAlgorithm class by > composition. > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> I like how these calls into libipa now work. Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> Best regards, Stefan > --- > src/ipa/rkisp1/algorithms/lsc.cpp | 72 +++++---------------------------------- > src/ipa/rkisp1/algorithms/lsc.h | 13 +++---- > src/ipa/rkisp1/ipa_context.h | 10 ++---- > 3 files changed, 18 insertions(+), 77 deletions(-) > > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp > index 7010c97db22e..0d9f1b006107 100644 > --- a/src/ipa/rkisp1/algorithms/lsc.cpp > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp > @@ -14,12 +14,6 @@ > #include <libcamera/base/log.h> > #include <libcamera/base/utils.h> > > -#include "libcamera/internal/value_node.h" > - > -#include "libipa/lsc_polynomial.h" > -#include "libipa/lsc_table.h" > -#include "linux/rkisp1-config.h" > - > /** > * \file lsc.h > */ > @@ -73,42 +67,8 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context, > xPos_ = sizesListToPositions(xSize_); > yPos_ = sizesListToPositions(ySize_); > > - /* Get all defined sets to apply. */ > - const ValueNode &sets = tuningData["sets"]; > - if (!sets.isList()) { > - LOG(RkISP1Lsc, Error) > - << "'sets' parameter not found in tuning file"; > - return -EINVAL; > - } > - > - int ret; > - > - std::string type = tuningData["type"].get<std::string>("table"); > - if (type == "table") { > - LOG(RkISP1Lsc, Debug) << "Loading tabular LSC data."; > - algo_ = std::make_unique<LscTable>(); > - ret = algo_->parseLscData(sets); > - } else if (type == "polynomial") { > - LOG(RkISP1Lsc, Debug) << "Loading polynomial LSC data."; > - /* > - * \todo: Most likely the reference frame should be native_size. > - * Let's wait how the internal discussions progress. > - */ > - algo_ = std::make_unique<LscPolynomial>(context.sensorInfo.activeAreaSize); > - ret = algo_->parseLscData(sets); > - } else { > - LOG(RkISP1Lsc, Error) << "Unsupported LSC data type '" > - << type << "'"; > - ret = -EINVAL; > - } > - > - if (ret) > - return ret; > - > - context.ctrlMap[&controls::LensShadingCorrectionEnable] = > - ControlInfo(false, true, true); > - > - return 0; > + return lscAlgo_.init(tuningData, context.sensorInfo.activeAreaSize, > + context.ctrlMap); > } > > std::vector<double> LensShadingCorrection::parseSizes(const ValueNode &tuningData, > @@ -199,13 +159,8 @@ int LensShadingCorrection::configure(IPAContext &context, > yGrad_[i] = std::round(32768 / ySizes_[i]); > } > > - LOG(RkISP1Lsc, Debug) << "Sample LSC data for " << configInfo.analogCrop; > - lsc::ComponentsMap shadingData = algo_->sampleForCrop(configInfo.analogCrop, > - xPos_, yPos_); > - sets_.setData(std::move(shadingData)); > - > - context.activeState.lsc.enabled = true; > - return 0; > + return lscAlgo_.configure(context.activeState.lsc, configInfo.analogCrop, > + xPos_, yPos_); > } > > void LensShadingCorrection::setParameters(rkisp1_cif_isp_lsc_config &config) > @@ -233,19 +188,8 @@ void LensShadingCorrection::queueRequest(IPAContext &context, > IPAFrameContext &frameContext, > const ControlList &controls) > { > - auto &lsc = context.activeState.lsc; > - > - const auto &lscEnable = controls.get(controls::LensShadingCorrectionEnable); > - if (lscEnable && *lscEnable != lsc.enabled) { > - lsc.enabled = *lscEnable; > - > - LOG(RkISP1Lsc, Debug) > - << (lsc.enabled ? "Enabling" : "Disabling") << " Lsc"; > - > - frameContext.lsc.update = true; > - } > - > - frameContext.lsc.enabled = lsc.enabled; > + lscAlgo_.queueRequest(context.activeState.lsc, frameContext.lsc, > + controls); > } > > /** > @@ -283,7 +227,7 @@ void LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context, > > setParameters(*config); > > - const lsc::Components &set = sets_.getInterpolated(quantizedCt); > + const lsc::Components &set = lscAlgo_.interpolateComponents(quantizedCt); > copyTable(*config, set); > > lastAppliedCt_ = ct; > @@ -303,7 +247,7 @@ void LensShadingCorrection::process([[maybe_unused]] IPAContext &context, > [[maybe_unused]] const rkisp1_stat_buffer *stats, > ControlList &metadata) > { > - metadata.set(controls::LensShadingCorrectionEnable, frameContext.lsc.enabled); > + lscAlgo_.process(frameContext.lsc, metadata); > } > > REGISTER_IPA_ALGORITHM(LensShadingCorrection, "LensShadingCorrection") > diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h > index 06aa7a5f133c..e7d3091a36c5 100644 > --- a/src/ipa/rkisp1/algorithms/lsc.h > +++ b/src/ipa/rkisp1/algorithms/lsc.h > @@ -7,10 +7,13 @@ > > #pragma once > > -#include <memory> > +#include <vector> > > -#include "libipa/interpolator.h" > -#include "libipa/lsc_base.h" > +#include <linux/rkisp1-config.h> > + > +#include "libcamera/internal/value_node.h" > + > +#include "libipa/lsc.h" > > #include "algorithm.h" > > @@ -55,9 +58,7 @@ private: > unsigned int lastAppliedCt_; > unsigned int lastAppliedQuantizedCt_; > > - ipa::Interpolator<lsc::Components> sets_; > - > - std::unique_ptr<LscImplementation> algo_; > + LscAlgorithm lscAlgo_; > }; > > } /* namespace ipa::rkisp1::algorithms */ > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > index 6698ccfd77cf..7b4346c54ed2 100644 > --- a/src/ipa/rkisp1/ipa_context.h > +++ b/src/ipa/rkisp1/ipa_context.h > @@ -30,6 +30,7 @@ > #include "libipa/ccm.h" > #include "libipa/fc_queue.h" > #include "libipa/fixedpoint.h" > +#include "libipa/lsc.h" > > namespace libcamera { > > @@ -141,9 +142,7 @@ struct IPAActiveState { > double strength; > } wdr; > > - struct { > - bool enabled; > - } lsc; > + ipa::lsc::ActiveState lsc; > }; > > struct IPAFrameContext : public FrameContext { > @@ -217,10 +216,7 @@ struct IPAFrameContext : public FrameContext { > double gain; > } wdr; > > - struct { > - bool enabled; > - bool update; > - } lsc; > + ipa::lsc::FrameContext lsc; > }; > > struct IPAContext { > > -- > 2.54.0 >
Quoting Stefan Klug (2026-07-13 15:55:44) > Hi Jacopo, > > Quoting Jacopo Mondi (2026-07-08 17:51:07) > > Port the RkISP1 LSC algorithm to use the LscAlgorithm class by > > composition. > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > I like how these calls into libipa now work. Reducing code duplication throughout the series is fantastic. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> > > Best regards, > Stefan > > > --- > > src/ipa/rkisp1/algorithms/lsc.cpp | 72 +++++---------------------------------- > > src/ipa/rkisp1/algorithms/lsc.h | 13 +++---- > > src/ipa/rkisp1/ipa_context.h | 10 ++---- > > 3 files changed, 18 insertions(+), 77 deletions(-) > > > > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp > > index 7010c97db22e..0d9f1b006107 100644 > > --- a/src/ipa/rkisp1/algorithms/lsc.cpp > > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp > > @@ -14,12 +14,6 @@ > > #include <libcamera/base/log.h> > > #include <libcamera/base/utils.h> > > > > -#include "libcamera/internal/value_node.h" > > - > > -#include "libipa/lsc_polynomial.h" > > -#include "libipa/lsc_table.h" > > -#include "linux/rkisp1-config.h" > > - > > /** > > * \file lsc.h > > */ > > @@ -73,42 +67,8 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context, > > xPos_ = sizesListToPositions(xSize_); > > yPos_ = sizesListToPositions(ySize_); > > > > - /* Get all defined sets to apply. */ > > - const ValueNode &sets = tuningData["sets"]; > > - if (!sets.isList()) { > > - LOG(RkISP1Lsc, Error) > > - << "'sets' parameter not found in tuning file"; > > - return -EINVAL; > > - } > > - > > - int ret; > > - > > - std::string type = tuningData["type"].get<std::string>("table"); > > - if (type == "table") { > > - LOG(RkISP1Lsc, Debug) << "Loading tabular LSC data."; > > - algo_ = std::make_unique<LscTable>(); > > - ret = algo_->parseLscData(sets); > > - } else if (type == "polynomial") { > > - LOG(RkISP1Lsc, Debug) << "Loading polynomial LSC data."; > > - /* > > - * \todo: Most likely the reference frame should be native_size. > > - * Let's wait how the internal discussions progress. > > - */ > > - algo_ = std::make_unique<LscPolynomial>(context.sensorInfo.activeAreaSize); > > - ret = algo_->parseLscData(sets); > > - } else { > > - LOG(RkISP1Lsc, Error) << "Unsupported LSC data type '" > > - << type << "'"; > > - ret = -EINVAL; > > - } > > - > > - if (ret) > > - return ret; > > - > > - context.ctrlMap[&controls::LensShadingCorrectionEnable] = > > - ControlInfo(false, true, true); > > - > > - return 0; > > + return lscAlgo_.init(tuningData, context.sensorInfo.activeAreaSize, > > + context.ctrlMap); > > } > > > > std::vector<double> LensShadingCorrection::parseSizes(const ValueNode &tuningData, > > @@ -199,13 +159,8 @@ int LensShadingCorrection::configure(IPAContext &context, > > yGrad_[i] = std::round(32768 / ySizes_[i]); > > } > > > > - LOG(RkISP1Lsc, Debug) << "Sample LSC data for " << configInfo.analogCrop; > > - lsc::ComponentsMap shadingData = algo_->sampleForCrop(configInfo.analogCrop, > > - xPos_, yPos_); > > - sets_.setData(std::move(shadingData)); > > - > > - context.activeState.lsc.enabled = true; > > - return 0; > > + return lscAlgo_.configure(context.activeState.lsc, configInfo.analogCrop, > > + xPos_, yPos_); > > } > > > > void LensShadingCorrection::setParameters(rkisp1_cif_isp_lsc_config &config) > > @@ -233,19 +188,8 @@ void LensShadingCorrection::queueRequest(IPAContext &context, > > IPAFrameContext &frameContext, > > const ControlList &controls) > > { > > - auto &lsc = context.activeState.lsc; > > - > > - const auto &lscEnable = controls.get(controls::LensShadingCorrectionEnable); > > - if (lscEnable && *lscEnable != lsc.enabled) { > > - lsc.enabled = *lscEnable; > > - > > - LOG(RkISP1Lsc, Debug) > > - << (lsc.enabled ? "Enabling" : "Disabling") << " Lsc"; > > - > > - frameContext.lsc.update = true; > > - } > > - > > - frameContext.lsc.enabled = lsc.enabled; > > + lscAlgo_.queueRequest(context.activeState.lsc, frameContext.lsc, > > + controls); > > } > > > > /** > > @@ -283,7 +227,7 @@ void LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context, > > > > setParameters(*config); > > > > - const lsc::Components &set = sets_.getInterpolated(quantizedCt); > > + const lsc::Components &set = lscAlgo_.interpolateComponents(quantizedCt); > > copyTable(*config, set); > > > > lastAppliedCt_ = ct; > > @@ -303,7 +247,7 @@ void LensShadingCorrection::process([[maybe_unused]] IPAContext &context, > > [[maybe_unused]] const rkisp1_stat_buffer *stats, > > ControlList &metadata) > > { > > - metadata.set(controls::LensShadingCorrectionEnable, frameContext.lsc.enabled); > > + lscAlgo_.process(frameContext.lsc, metadata); > > } > > > > REGISTER_IPA_ALGORITHM(LensShadingCorrection, "LensShadingCorrection") > > diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h > > index 06aa7a5f133c..e7d3091a36c5 100644 > > --- a/src/ipa/rkisp1/algorithms/lsc.h > > +++ b/src/ipa/rkisp1/algorithms/lsc.h > > @@ -7,10 +7,13 @@ > > > > #pragma once > > > > -#include <memory> > > +#include <vector> > > > > -#include "libipa/interpolator.h" > > -#include "libipa/lsc_base.h" > > +#include <linux/rkisp1-config.h> > > + > > +#include "libcamera/internal/value_node.h" > > + > > +#include "libipa/lsc.h" > > > > #include "algorithm.h" > > > > @@ -55,9 +58,7 @@ private: > > unsigned int lastAppliedCt_; > > unsigned int lastAppliedQuantizedCt_; > > > > - ipa::Interpolator<lsc::Components> sets_; > > - > > - std::unique_ptr<LscImplementation> algo_; > > + LscAlgorithm lscAlgo_; > > }; > > > > } /* namespace ipa::rkisp1::algorithms */ > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > > index 6698ccfd77cf..7b4346c54ed2 100644 > > --- a/src/ipa/rkisp1/ipa_context.h > > +++ b/src/ipa/rkisp1/ipa_context.h > > @@ -30,6 +30,7 @@ > > #include "libipa/ccm.h" > > #include "libipa/fc_queue.h" > > #include "libipa/fixedpoint.h" > > +#include "libipa/lsc.h" > > > > namespace libcamera { > > > > @@ -141,9 +142,7 @@ struct IPAActiveState { > > double strength; > > } wdr; > > > > - struct { > > - bool enabled; > > - } lsc; > > + ipa::lsc::ActiveState lsc; > > }; > > > > struct IPAFrameContext : public FrameContext { > > @@ -217,10 +216,7 @@ struct IPAFrameContext : public FrameContext { > > double gain; > > } wdr; > > > > - struct { > > - bool enabled; > > - bool update; > > - } lsc; > > + ipa::lsc::FrameContext lsc; > > }; > > > > struct IPAContext { > > > > -- > > 2.54.0 > >
diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp index 7010c97db22e..0d9f1b006107 100644 --- a/src/ipa/rkisp1/algorithms/lsc.cpp +++ b/src/ipa/rkisp1/algorithms/lsc.cpp @@ -14,12 +14,6 @@ #include <libcamera/base/log.h> #include <libcamera/base/utils.h> -#include "libcamera/internal/value_node.h" - -#include "libipa/lsc_polynomial.h" -#include "libipa/lsc_table.h" -#include "linux/rkisp1-config.h" - /** * \file lsc.h */ @@ -73,42 +67,8 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context, xPos_ = sizesListToPositions(xSize_); yPos_ = sizesListToPositions(ySize_); - /* Get all defined sets to apply. */ - const ValueNode &sets = tuningData["sets"]; - if (!sets.isList()) { - LOG(RkISP1Lsc, Error) - << "'sets' parameter not found in tuning file"; - return -EINVAL; - } - - int ret; - - std::string type = tuningData["type"].get<std::string>("table"); - if (type == "table") { - LOG(RkISP1Lsc, Debug) << "Loading tabular LSC data."; - algo_ = std::make_unique<LscTable>(); - ret = algo_->parseLscData(sets); - } else if (type == "polynomial") { - LOG(RkISP1Lsc, Debug) << "Loading polynomial LSC data."; - /* - * \todo: Most likely the reference frame should be native_size. - * Let's wait how the internal discussions progress. - */ - algo_ = std::make_unique<LscPolynomial>(context.sensorInfo.activeAreaSize); - ret = algo_->parseLscData(sets); - } else { - LOG(RkISP1Lsc, Error) << "Unsupported LSC data type '" - << type << "'"; - ret = -EINVAL; - } - - if (ret) - return ret; - - context.ctrlMap[&controls::LensShadingCorrectionEnable] = - ControlInfo(false, true, true); - - return 0; + return lscAlgo_.init(tuningData, context.sensorInfo.activeAreaSize, + context.ctrlMap); } std::vector<double> LensShadingCorrection::parseSizes(const ValueNode &tuningData, @@ -199,13 +159,8 @@ int LensShadingCorrection::configure(IPAContext &context, yGrad_[i] = std::round(32768 / ySizes_[i]); } - LOG(RkISP1Lsc, Debug) << "Sample LSC data for " << configInfo.analogCrop; - lsc::ComponentsMap shadingData = algo_->sampleForCrop(configInfo.analogCrop, - xPos_, yPos_); - sets_.setData(std::move(shadingData)); - - context.activeState.lsc.enabled = true; - return 0; + return lscAlgo_.configure(context.activeState.lsc, configInfo.analogCrop, + xPos_, yPos_); } void LensShadingCorrection::setParameters(rkisp1_cif_isp_lsc_config &config) @@ -233,19 +188,8 @@ void LensShadingCorrection::queueRequest(IPAContext &context, IPAFrameContext &frameContext, const ControlList &controls) { - auto &lsc = context.activeState.lsc; - - const auto &lscEnable = controls.get(controls::LensShadingCorrectionEnable); - if (lscEnable && *lscEnable != lsc.enabled) { - lsc.enabled = *lscEnable; - - LOG(RkISP1Lsc, Debug) - << (lsc.enabled ? "Enabling" : "Disabling") << " Lsc"; - - frameContext.lsc.update = true; - } - - frameContext.lsc.enabled = lsc.enabled; + lscAlgo_.queueRequest(context.activeState.lsc, frameContext.lsc, + controls); } /** @@ -283,7 +227,7 @@ void LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context, setParameters(*config); - const lsc::Components &set = sets_.getInterpolated(quantizedCt); + const lsc::Components &set = lscAlgo_.interpolateComponents(quantizedCt); copyTable(*config, set); lastAppliedCt_ = ct; @@ -303,7 +247,7 @@ void LensShadingCorrection::process([[maybe_unused]] IPAContext &context, [[maybe_unused]] const rkisp1_stat_buffer *stats, ControlList &metadata) { - metadata.set(controls::LensShadingCorrectionEnable, frameContext.lsc.enabled); + lscAlgo_.process(frameContext.lsc, metadata); } REGISTER_IPA_ALGORITHM(LensShadingCorrection, "LensShadingCorrection") diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h index 06aa7a5f133c..e7d3091a36c5 100644 --- a/src/ipa/rkisp1/algorithms/lsc.h +++ b/src/ipa/rkisp1/algorithms/lsc.h @@ -7,10 +7,13 @@ #pragma once -#include <memory> +#include <vector> -#include "libipa/interpolator.h" -#include "libipa/lsc_base.h" +#include <linux/rkisp1-config.h> + +#include "libcamera/internal/value_node.h" + +#include "libipa/lsc.h" #include "algorithm.h" @@ -55,9 +58,7 @@ private: unsigned int lastAppliedCt_; unsigned int lastAppliedQuantizedCt_; - ipa::Interpolator<lsc::Components> sets_; - - std::unique_ptr<LscImplementation> algo_; + LscAlgorithm lscAlgo_; }; } /* namespace ipa::rkisp1::algorithms */ diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index 6698ccfd77cf..7b4346c54ed2 100644 --- a/src/ipa/rkisp1/ipa_context.h +++ b/src/ipa/rkisp1/ipa_context.h @@ -30,6 +30,7 @@ #include "libipa/ccm.h" #include "libipa/fc_queue.h" #include "libipa/fixedpoint.h" +#include "libipa/lsc.h" namespace libcamera { @@ -141,9 +142,7 @@ struct IPAActiveState { double strength; } wdr; - struct { - bool enabled; - } lsc; + ipa::lsc::ActiveState lsc; }; struct IPAFrameContext : public FrameContext { @@ -217,10 +216,7 @@ struct IPAFrameContext : public FrameContext { double gain; } wdr; - struct { - bool enabled; - bool update; - } lsc; + ipa::lsc::FrameContext lsc; }; struct IPAContext {
Port the RkISP1 LSC algorithm to use the LscAlgorithm class by composition. Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> --- src/ipa/rkisp1/algorithms/lsc.cpp | 72 +++++---------------------------------- src/ipa/rkisp1/algorithms/lsc.h | 13 +++---- src/ipa/rkisp1/ipa_context.h | 10 ++---- 3 files changed, 18 insertions(+), 77 deletions(-)