Message ID | 20220617092315.120781-6-fsylvestre@baylibre.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Florian, Thank you for the patch. On Fri, Jun 17, 2022 at 11:23:15AM +0200, Florian Sylvestre wrote: > Get default values for Black Level Correction algorithm form YAML s/form/from/ I notice you have picked only part of the suggested spelling corrections from previous reviews in multiple occasions. Is there a way I could express them more clearly to make sure they are correctly understood ? > tuning file. > > Signed-off-by: Florian Sylvestre <fsylvestre@baylibre.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/ipa/rkisp1/algorithms/blc.cpp | 62 +++++++++++++++++++++++++++---- > src/ipa/rkisp1/algorithms/blc.h | 10 ++++- > 2 files changed, 63 insertions(+), 9 deletions(-) > > diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp > index 0c5948ff..aa4fe8c6 100644 > --- a/src/ipa/rkisp1/algorithms/blc.cpp > +++ b/src/ipa/rkisp1/algorithms/blc.cpp > @@ -7,6 +7,10 @@ > > #include "blc.h" > > +#include <libcamera/base/log.h> > + > +#include "libcamera/internal/yaml_parser.h" > + > /** > * \file blc.h > */ > @@ -29,6 +33,48 @@ namespace ipa::rkisp1::algorithms { > * isn't currently supported. > */ > > +LOG_DEFINE_CATEGORY(RkISP1Blc) > + > +BlackLevelCorrection::BlackLevelCorrection() > + : tuningParameters_(false) > +{ > +} > + > +/** > + * \copydoc libcamera::ipa::Algorithm::init > + */ > +int BlackLevelCorrection::init([[maybe_unused]] IPAContext &context, > + const YamlObject ¶ms) > +{ > + /* Parse property "BlackLevelCorrection". */ > + if (!params.contains("BlackLevelCorrection")) { > + LOG(RkISP1Blc, Debug) << "No tuning parameters found for BlackLevelCorrection"; > + return 0; > + } > + > + const YamlObject &blcObject = params["BlackLevelCorrection"]; > + > + if (!blcObject.isDictionary()) { > + LOG(RkISP1Blc, Error) << "'BlackLevelCorrection' in tuning file is not a dictionnary"; > + return -EINVAL; > + } > + > + blackLevelRed_ = blcObject["R"].get<int16_t>(256); > + blackLevelGreenR_ = blcObject["Gr"].get<int16_t>(256); > + blackLevelGreenB_ = blcObject["Gb"].get<int16_t>(256); > + blackLevelBlue_ = blcObject["B"].get<int16_t>(256); > + > + tuningParameters_ = true; > + > + LOG(RkISP1Blc, Debug) > + << " Read black levels red " << blackLevelRed_ > + << ", green (red) " << blackLevelGreenR_ > + << ", green (blue) " << blackLevelGreenB_ > + << ", blue " << blackLevelBlue_; > + > + return 0; > +} > + > /** > * \copydoc libcamera::ipa::Algorithm::prepare > */ > @@ -37,15 +83,15 @@ void BlackLevelCorrection::prepare(IPAContext &context, > { > if (context.frameContext.frameCount > 0) > return; > - /* > - * Substract fixed values taken from imx219 tuning file. > - * \todo Use a configuration file for it ? > - */ > + > + if (!tuningParameters_) > + return; > + > params->others.bls_config.enable_auto = 0; > - params->others.bls_config.fixed_val.r = 256; > - params->others.bls_config.fixed_val.gr = 256; > - params->others.bls_config.fixed_val.gb = 256; > - params->others.bls_config.fixed_val.b = 256; > + params->others.bls_config.fixed_val.r = blackLevelRed_; > + params->others.bls_config.fixed_val.gr = blackLevelGreenR_; > + params->others.bls_config.fixed_val.gb = blackLevelGreenB_; > + params->others.bls_config.fixed_val.b = blackLevelBlue_; > > params->module_en_update |= RKISP1_CIF_ISP_MODULE_BLS; > params->module_ens |= RKISP1_CIF_ISP_MODULE_BLS; > diff --git a/src/ipa/rkisp1/algorithms/blc.h b/src/ipa/rkisp1/algorithms/blc.h > index 69874d8f..c979d327 100644 > --- a/src/ipa/rkisp1/algorithms/blc.h > +++ b/src/ipa/rkisp1/algorithms/blc.h > @@ -20,10 +20,18 @@ namespace ipa::rkisp1::algorithms { > class BlackLevelCorrection : public Algorithm > { > public: > - BlackLevelCorrection() = default; > + BlackLevelCorrection(); > ~BlackLevelCorrection() = default; > > + int init(IPAContext &context, const YamlObject ¶ms) override; > void prepare(IPAContext &context, rkisp1_params_cfg *params) override; > + > +private: > + bool tuningParameters_; > + int16_t blackLevelRed_; > + int16_t blackLevelGreenR_; > + int16_t blackLevelGreenB_; > + int16_t blackLevelBlue_; > }; > > } /* namespace ipa::rkisp1::algorithms */
diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp index 0c5948ff..aa4fe8c6 100644 --- a/src/ipa/rkisp1/algorithms/blc.cpp +++ b/src/ipa/rkisp1/algorithms/blc.cpp @@ -7,6 +7,10 @@ #include "blc.h" +#include <libcamera/base/log.h> + +#include "libcamera/internal/yaml_parser.h" + /** * \file blc.h */ @@ -29,6 +33,48 @@ namespace ipa::rkisp1::algorithms { * isn't currently supported. */ +LOG_DEFINE_CATEGORY(RkISP1Blc) + +BlackLevelCorrection::BlackLevelCorrection() + : tuningParameters_(false) +{ +} + +/** + * \copydoc libcamera::ipa::Algorithm::init + */ +int BlackLevelCorrection::init([[maybe_unused]] IPAContext &context, + const YamlObject ¶ms) +{ + /* Parse property "BlackLevelCorrection". */ + if (!params.contains("BlackLevelCorrection")) { + LOG(RkISP1Blc, Debug) << "No tuning parameters found for BlackLevelCorrection"; + return 0; + } + + const YamlObject &blcObject = params["BlackLevelCorrection"]; + + if (!blcObject.isDictionary()) { + LOG(RkISP1Blc, Error) << "'BlackLevelCorrection' in tuning file is not a dictionnary"; + return -EINVAL; + } + + blackLevelRed_ = blcObject["R"].get<int16_t>(256); + blackLevelGreenR_ = blcObject["Gr"].get<int16_t>(256); + blackLevelGreenB_ = blcObject["Gb"].get<int16_t>(256); + blackLevelBlue_ = blcObject["B"].get<int16_t>(256); + + tuningParameters_ = true; + + LOG(RkISP1Blc, Debug) + << " Read black levels red " << blackLevelRed_ + << ", green (red) " << blackLevelGreenR_ + << ", green (blue) " << blackLevelGreenB_ + << ", blue " << blackLevelBlue_; + + return 0; +} + /** * \copydoc libcamera::ipa::Algorithm::prepare */ @@ -37,15 +83,15 @@ void BlackLevelCorrection::prepare(IPAContext &context, { if (context.frameContext.frameCount > 0) return; - /* - * Substract fixed values taken from imx219 tuning file. - * \todo Use a configuration file for it ? - */ + + if (!tuningParameters_) + return; + params->others.bls_config.enable_auto = 0; - params->others.bls_config.fixed_val.r = 256; - params->others.bls_config.fixed_val.gr = 256; - params->others.bls_config.fixed_val.gb = 256; - params->others.bls_config.fixed_val.b = 256; + params->others.bls_config.fixed_val.r = blackLevelRed_; + params->others.bls_config.fixed_val.gr = blackLevelGreenR_; + params->others.bls_config.fixed_val.gb = blackLevelGreenB_; + params->others.bls_config.fixed_val.b = blackLevelBlue_; params->module_en_update |= RKISP1_CIF_ISP_MODULE_BLS; params->module_ens |= RKISP1_CIF_ISP_MODULE_BLS; diff --git a/src/ipa/rkisp1/algorithms/blc.h b/src/ipa/rkisp1/algorithms/blc.h index 69874d8f..c979d327 100644 --- a/src/ipa/rkisp1/algorithms/blc.h +++ b/src/ipa/rkisp1/algorithms/blc.h @@ -20,10 +20,18 @@ namespace ipa::rkisp1::algorithms { class BlackLevelCorrection : public Algorithm { public: - BlackLevelCorrection() = default; + BlackLevelCorrection(); ~BlackLevelCorrection() = default; + int init(IPAContext &context, const YamlObject ¶ms) override; void prepare(IPAContext &context, rkisp1_params_cfg *params) override; + +private: + bool tuningParameters_; + int16_t blackLevelRed_; + int16_t blackLevelGreenR_; + int16_t blackLevelGreenB_; + int16_t blackLevelBlue_; }; } /* namespace ipa::rkisp1::algorithms */