[PATCH v2 2/2] ipa: rkisp1: Add Lux algorithm module

[Kieran Bingham]

Quoting Kieran Bingham (2024-12-16 11:26:44)
> Quoting Paul Elder (2024-12-16 09:49:33)
> > Add a lux algorithm module to rkisp1 IPA for estimating the lux level of
> > an image. This is reported in metadata, as well as saved in the frame
> > context so that other algorithms (mainly AGC) can use its value. It does
> > not set any controls.
> > 
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > 
> > ---
> > Changes in v2:
> > - fix bitrot
> > - fixes corresponding to changes in the previous patch
> > ---
> >  src/ipa/rkisp1/algorithms/lux.cpp     | 76 +++++++++++++++++++++++++++
> >  src/ipa/rkisp1/algorithms/lux.h       | 39 ++++++++++++++
> >  src/ipa/rkisp1/algorithms/meson.build |  1 +
> >  src/ipa/rkisp1/ipa_context.h          |  1 +
> >  4 files changed, 117 insertions(+)
> >  create mode 100644 src/ipa/rkisp1/algorithms/lux.cpp
> >  create mode 100644 src/ipa/rkisp1/algorithms/lux.h
> > 
> > diff --git a/src/ipa/rkisp1/algorithms/lux.cpp b/src/ipa/rkisp1/algorithms/lux.cpp
> > new file mode 100644
> > index 000000000000..333167b15f64
> > --- /dev/null
> > +++ b/src/ipa/rkisp1/algorithms/lux.cpp
> > @@ -0,0 +1,76 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2024, Ideas On Board
> > + *
> > + * lux.cpp - RkISP1 Lux control
> > + */
> > +
> > +#include "lux.h"
> > +
> > +#include <algorithm>
> > +#include <cmath>
> > +
> > +#include <libcamera/base/log.h>
> > +
> > +#include <libcamera/control_ids.h>
> > +
> > +#include "libcamera/internal/yaml_parser.h"
> > +
> > +#include "libipa/histogram.h"
> > +#include "libipa/lux.h"
> > +
> > +/**
> > + * \file lux.h
> > + */
> > +
> > +namespace libcamera {
> > +
> > +namespace ipa::rkisp1::algorithms {
> > +
> > +/**
> > + * \class Lux
> > + * \brief RkISP1 Lux control
> > + *
> > + * The Lux algorithm is responsible for estimating the lux level of the image.
> > + * It doesn't take or generate any controls, but it provides a lux level for
> > + * other algorithms (such as AGC) to use.
> > + */
> > +
> > +LOG_DEFINE_CATEGORY(RkISP1Lux)
> > +
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::init
> > + */
> > +int Lux::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)
> > +{
> > +       lux_.setBinSize(65535);
> > +       return lux_.readYaml(tuningData);
> > +}
> > +
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::process
> > + */
> > +void Lux::process(IPAContext &context,
> > +                 [[maybe_unused]] const uint32_t frame,
> > +                 IPAFrameContext &frameContext,
> > +                 const rkisp1_stat_buffer *stats,
> > +                 ControlList &metadata)
> > +{
> > +       utils::Duration exposureTime = context.configuration.sensor.lineDuration
> > +                                      * frameContext.sensor.exposure;
> > +       double gain = frameContext.sensor.gain;
> > +
> > +       const rkisp1_cif_isp_stat *params = &stats->params;
> > +       Histogram yHist({ params->hist.hist_bins, context.hw->numHistogramBins },
> > +                       [](uint32_t x) { return x >> 4; });
> > +
> > +       double lux = lux_.estimateLux(exposureTime, gain, 1.0, yHist);
> > +       frameContext.agc.lux = lux;
> > +       metadata.set(controls::Lux, lux);
> > +}
> > +
> > +REGISTER_IPA_ALGORITHM(Lux, "Lux")
> > +
> > +} /* namespace ipa::rkisp1::algorithms */
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/ipa/rkisp1/algorithms/lux.h b/src/ipa/rkisp1/algorithms/lux.h
> > new file mode 100644
> > index 000000000000..ea98c29120eb
> > --- /dev/null
> > +++ b/src/ipa/rkisp1/algorithms/lux.h
> > @@ -0,0 +1,39 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2024, Ideas On Board
> > + *
> > + * lux.h - RkISP1 Lux control
> > + */
> > +
> > +#pragma once
> > +
> > +#include <sys/types.h>
> > +
> > +#include "libcamera/internal/yaml_parser.h"
> > +
> > +#include "libipa/lux.h"
> > +
> > +#include "algorithm.h"
> > +
> > +namespace libcamera {
> > +
> > +namespace ipa::rkisp1::algorithms {
> > +
> > +class Lux : public Algorithm
> > +{
> > +public:
> > +       Lux() = default;
> > +       ~Lux() = default;
> > +
> > +       int init(IPAContext &context, const YamlObject &tuningData) override;
> > +       void process(IPAContext &context, const uint32_t frame,
> > +                    IPAFrameContext &frameContext,
> > +                    const rkisp1_stat_buffer *stats,
> > +                    ControlList &metadata) override;
> > +
> > +private:
> > +       ipa::Lux lux_;
> > +};
> > +
> > +} /* namespace ipa::rkisp1::algorithms */
> > +} /* namespace libcamera */
> > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build
> > index 1734a6675f78..c66b0b70b82f 100644
> > --- a/src/ipa/rkisp1/algorithms/meson.build
> > +++ b/src/ipa/rkisp1/algorithms/meson.build
> > @@ -12,4 +12,5 @@ rkisp1_ipa_algorithms = files([
> >      'goc.cpp',
> >      'gsl.cpp',
> >      'lsc.cpp',
> > +    'lux.cpp',
> >  ])
> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> > index deb8c196f1b8..e7bdc51bfa4c 100644
> > --- a/src/ipa/rkisp1/ipa_context.h
> > +++ b/src/ipa/rkisp1/ipa_context.h
> > @@ -130,6 +130,7 @@ struct IPAFrameContext : public FrameContext {
> >                 controls::AeMeteringModeEnum meteringMode;
> >                 utils::Duration maxFrameDuration;
> >                 bool updateMetering;
> > +               double lux;
> 
> Is this really part of agc ? Is it used anywhere that it needs to be
> stored in the FrameContext? (Or will it be used somewhere?)
> 
> I'm curious on what it will be used for and whether we need to guarantee
> ordering that the lux runs before AGC if it's /really/ used by the AGC.
> 
> Perhaps not storing it for now would be easier until it's needed, as
> then it would be clearer - but I don't object to storing it.
> 
> Does libtuning already prepare and store the lux calibration reference
> points ?

Having read the cover letter - it sounds like we will need to make sure
this is run before both AWB and AGC.

I'm fine with keeping it in the agc structure though, as it probably
gets a bit lonely as framecontext.lux.lux ... ? And it really is closely
related to AGC control...


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> 
> --
> 
> >         } agc;
> >  
> >         struct {
> > -- 
> > 2.39.2
> >