[PATCH 4/5] ipa: rkisp1: blc: Report sensor black levels in metadata

[Kieran Bingham]

Quoting Jacopo Mondi (2024-07-02 09:45:26)
> Hi Stefan
> 
> On Mon, Jul 01, 2024 at 04:38:27PM GMT, Stefan Klug wrote:
> > For the tuning process it is necessary to know the sensor black levels.
> > Add them to the metadata.
> >
> > Additionally enable raw support for this algorithm and add it to
> > uncalibrated.yaml, so that black levels get reported when capturing
> > tuning images.
> >
> > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > ---
> >  src/ipa/rkisp1/algorithms/blc.cpp     | 19 +++++++++++++++++++
> >  src/ipa/rkisp1/algorithms/blc.h       |  5 ++++-
> >  src/ipa/rkisp1/data/uncalibrated.yaml |  1 +
> >  3 files changed, 24 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp
> > index 0c39c3b47da5..a9f76b87d15a 100644
> > --- a/src/ipa/rkisp1/algorithms/blc.cpp
> > +++ b/src/ipa/rkisp1/algorithms/blc.cpp
> > @@ -9,6 +9,8 @@
> >
> >  #include <libcamera/base/log.h>
> >
> > +#include <libcamera/control_ids.h>
> > +
> >  #include "libcamera/internal/yaml_parser.h"
> >
> >  /**
> > @@ -38,6 +40,7 @@ LOG_DEFINE_CATEGORY(RkISP1Blc)
> >  BlackLevelCorrection::BlackLevelCorrection()
> >       : tuningParameters_(false)
> >  {
> > +     supportsRaw_ = true;
> 
> Does it support raw for real ? Doesn't BLC require going through the
> ISP ?

Oh That's interesting, - I didn't realise we had this switch

Any preference for adding it in the constructor rather than extending
the initiasliser list and putting it after tuningParameters_(false) ?

>
> >  }
> >
> >  /**
> > @@ -107,6 +110,22 @@ void BlackLevelCorrection::prepare([[maybe_unused]] IPAContext &context,
> >       params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_BLS;
> >  }
> >
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::process
> > + */
> > +void BlackLevelCorrection::process([[maybe_unused]] IPAContext &context,
> > +                                [[maybe_unused]] const uint32_t frame,
> > +                                [[maybe_unused]] IPAFrameContext &frameContext,
> > +                                [[maybe_unused]] const rkisp1_stat_buffer *stats,
> > +                                [[maybe_unused]] ControlList &metadata)
> > +{
> > +     metadata.set(controls::SensorBlackLevels,
> > +                  { static_cast<int32_t>(blackLevelRed_),
> > +                    static_cast<int32_t>(blackLevelGreenR_),
> > +                    static_cast<int32_t>(blackLevelGreenB_),
> > +                    static_cast<int32_t>(blackLevelBlue_) });
> 
> As black levels do not change you can store them in the context and
> populate the metadata in the main IPA module instead of making this
> algorithm support raw ?


If a sensor exposes a control to allow setting the black level, I expect
this component would be responsible for reporting that and then setting
it to the v4l2-subdev driver?

> 
> > +}
> > +
> >  REGISTER_IPA_ALGORITHM(BlackLevelCorrection, "BlackLevelCorrection")
> >
> >  } /* namespace ipa::rkisp1::algorithms */
> > diff --git a/src/ipa/rkisp1/algorithms/blc.h b/src/ipa/rkisp1/algorithms/blc.h
> > index 460ebcc15739..4ecac233f88b 100644
> > --- a/src/ipa/rkisp1/algorithms/blc.h
> > +++ b/src/ipa/rkisp1/algorithms/blc.h
> > @@ -23,7 +23,10 @@ public:
> >       void prepare(IPAContext &context, const uint32_t frame,
> >                    IPAFrameContext &frameContext,
> >                    rkisp1_params_cfg *params) override;
> > -
> > +     void process(IPAContext &context, const uint32_t frame,
> > +                  IPAFrameContext &frameContext,
> > +                  const rkisp1_stat_buffer *stats,
> > +                  ControlList &metadata) override;
> >  private:
> >       bool tuningParameters_;
> >       int16_t blackLevelRed_;
> > diff --git a/src/ipa/rkisp1/data/uncalibrated.yaml b/src/ipa/rkisp1/data/uncalibrated.yaml
> > index a7bbd8d84263..609012967e02 100644
> > --- a/src/ipa/rkisp1/data/uncalibrated.yaml
> > +++ b/src/ipa/rkisp1/data/uncalibrated.yaml
> > @@ -5,4 +5,5 @@ version: 1
> >  algorithms:
> >    - Agc:
> >    - Awb:
> > +  - BlackLevelCorrection:
> >  ...
> > --
> > 2.43.0
> >