[libcamera-devel] [libcamera-devel 5/5] ipa: rkisp1: add support of Black Level Correction default values
Florian Sylvestre
fsylvestre at baylibre.com
Wed Jun 1 10:19:44 CEST 2022
>> + const YamlObject &blcObject = (*params)["BlackLevelCorrection"];
>> +
>> + if (!blcObject.isDictionary())
>> + return -EINVAL;
>> +
>> + blackLevelRed_ = blcObject["R"].get<int32_t>(256);
>Can the value be negative ? If not, let go to uint32_t. Actually, you'll
>have uint16_t support when rebasing on top of the YAML parser
>improvements, so you can use that.
The values can be negatives.
FYI: Extracted from .h comments:
* struct rkisp1_cif_isp_bls_fixed_val - BLS fixed subtraction values
*
* The values will be subtracted from the sensor
* values. Therefore a negative value means addition instead of subtraction!
I have seen the [RFC] on modifications for Yaml Parser to support int16,
but since it's 'only' at [RFC] state, should we not provide this version and
update it when the support of int16 will be available?
Regards,
Florian Sylvestre
Le jeu. 26 mai 2022 à 13:04, Laurent Pinchart <
laurent.pinchart at ideasonboard.com> a écrit :
> Hi Florian,
>
> Thank you for the patch.
>
> On Mon, May 23, 2022 at 11:24:35AM +0200, Florian Sylvestre via
> libcamera-devel wrote:
> > Get default values for Black Level Correction algorithm form Yaml
> > configuration file.
>
> s/form Yaml configuration/from YAML tuning/
>
> > Signed-off-by: Florian Sylvestre <fsylvestre at baylibre.com>
> > ---
> > src/ipa/rkisp1/algorithms/blc.cpp | 46 ++++++++++++++++++++++++++++---
> > src/ipa/rkisp1/algorithms/blc.h | 9 ++++++
> > 2 files changed, 51 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/ipa/rkisp1/algorithms/blc.cpp
> b/src/ipa/rkisp1/algorithms/blc.cpp
> > index 0c5948ff..177bc22e 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>
>
> For internal headers we use "" instead of <>.
>
> > +
> > /**
> > * \file blc.h
> > */
> > @@ -29,6 +33,40 @@ namespace ipa::rkisp1::algorithms {
> > * isn't currently supported.
> > */
> >
> > +LOG_DEFINE_CATEGORY(RkISP1Blc)
> > +
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::init
> > + */
> > +int BlackLevelCorrection::init(IPAContext &context,
> > + const YamlObject *params)
> > +{
> > + if (context.frameContext.frameCount > 0)
> > + return -EBUSY;
> > +
> > + /* Parse property "BlackLevelCorrection" */
> > + if (!params->contains("BlackLevelCorrection"))
> > + return -EINVAL;
> > +
> > + const YamlObject &blcObject = (*params)["BlackLevelCorrection"];
> > +
> > + if (!blcObject.isDictionary())
> > + return -EINVAL;
> > +
> > + blackLevelRed_ = blcObject["R"].get<int32_t>(256);
>
> Can the value be negative ? If not, let go to uint32_t. Actually, you'll
> have uint16_t support when rebasing on top of the YAML parser
> improvements, so you can use that.
>
> > + blackLevelGreenR_ = blcObject["Gr"].get<int32_t>(256);
> > + blackLevelGreenB_ = blcObject["Gb"].get<int32_t>(256);
> > + blackLevelBlue_ = blcObject["B"].get<int32_t>(256);
> > +
> > + LOG(RkISP1Blc, Debug)
> > + << " Read black levels red " << blackLevelRed_
> > + << " green (red) " << blackLevelGreenR_
> > + << " green (blue) " << blackLevelGreenB_
> > + << " blue " << blackLevelBlue_;
>
> << " Read black levels red " << blackLevelRed_
> << ", green (red) " << blackLevelGreenR_
> << ", green (blue) " << blackLevelGreenB_
> << ", blue " << blackLevelBlue_;
>
> > +
> > + return 0;
> > +}
> > +
> > /**
> > * \copydoc libcamera::ipa::Algorithm::prepare
> > */
> > @@ -42,10 +80,10 @@ void BlackLevelCorrection::prepare(IPAContext
> &context,
> > * \todo Use a configuration file for it ?
> > */
> > 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..2f6e2d82 100644
> > --- a/src/ipa/rkisp1/algorithms/blc.h
> > +++ b/src/ipa/rkisp1/algorithms/blc.h
> > @@ -13,6 +13,8 @@
> >
> > namespace libcamera {
> >
> > +class YamlObject;
>
> You can drop this as it's alread forward-declared by algorithm.h;
>
> > +
> > struct IPACameraSensorInfo;
> >
> > namespace ipa::rkisp1::algorithms {
> > @@ -23,7 +25,14 @@ public:
> > BlackLevelCorrection() = default;
> > ~BlackLevelCorrection() = default;
> >
> > + int init(IPAContext &context, const YamlObject *params) override;
> > void prepare(IPAContext &context, rkisp1_params_cfg *params)
> override;
> > +
> > +private:
> > + int16_t blackLevelRed_;
> > + int16_t blackLevelGreenR_;
> > + int16_t blackLevelGreenB_;
> > + int16_t blackLevelBlue_;
>
> If the values can't be negative let's use uint16_t here.
>
> > };
> >
> > } /* namespace ipa::rkisp1::algorithms */
>
> --
> Regards,
>
> Laurent Pinchart
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220601/3e9eee5e/attachment.htm>
More information about the libcamera-devel
mailing list