[PATCH v2 5/8] ipa: ipu3: Derive ipu3::algorithms::Agc from MeanLuminanceAgc
Stefan Klug
stefan.klug at ideasonboard.com
Thu Apr 25 12:05:55 CEST 2024
On Wed, Apr 24, 2024 at 10:00:37AM +0100, Dan Scally wrote:
> Hi Stefan
>
> On 18/04/2024 08:56, Stefan Klug wrote:
> > Hi Daniel,
> >
> > thanks for the patch.
> >
> > On Wed, Apr 17, 2024 at 02:15:33PM +0100, Daniel Scally wrote:
> > > In preparation for switching to a derivation of MeanLuminanceAgc, add
> > > a function to parse and store the statistics for easy retrieval in an
> > > overriding estimateLuminance() function.
> > >
> > > Now that we have a MeanLuminanceAgc class that centralises our AEGC
> > > algorithm, derive the IPU3's Agc class from it and plumb in the
> > > necessary framework to enable it to be used. For simplicity's sake
> > > this commit switches the algorithm to use the derived class, but
> > > does not remove the bespoke functions at this time.
> > >
> > > Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
> > > ---
> > > Changes in v2:
> > >
> > > - Squashed the patch adding parseStatistics()
> > > - Used the first available constraint and exposure modes rather than
> > > assuming the "Normal" mode was available
> > > - Used a vector of RGB tuples in estimateLuminance() rather than 3
> > > vectors
> > > - Stored a copy of the bdsGrid and AWB gains rather than a pointer to
> > > the context for use in estimateLuminance()
> > > - Remembered to report the controls out to IPAIPU3::updateControls()
> > >
> > > src/ipa/ipu3/algorithms/agc.cpp | 120 ++++++++++++++++++++++++++++++--
> > > src/ipa/ipu3/algorithms/agc.h | 14 +++-
> > > src/ipa/ipu3/ipa_context.cpp | 3 +
> > > src/ipa/ipu3/ipa_context.h | 5 ++
> > > src/ipa/ipu3/ipu3.cpp | 3 +-
> > > 5 files changed, 136 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> > > index 606a237a..3b9761bd 100644
> > > --- a/src/ipa/ipu3/algorithms/agc.cpp
> > > +++ b/src/ipa/ipu3/algorithms/agc.cpp
> > > @@ -71,11 +71,33 @@ static constexpr uint32_t kNumStartupFrames = 10;
> > > static constexpr double kRelativeLuminanceTarget = 0.16;
> > > Agc::Agc()
> > > - : frameCount_(0), minShutterSpeed_(0s),
> > > - maxShutterSpeed_(0s), filteredExposure_(0s)
> > > + : minShutterSpeed_(0s), maxShutterSpeed_(0s)
> > > {
> > > }
> > > +/**
> > > + * \brief Initialise the AGC algorithm from tuning files
> > > + * \param[in] context The shared IPA context
> > > + * \param[in] tuningData The YamlObject containing Agc tuning data
> > > + *
> > > + * This function calls the base class' tuningData parsers to discover which
> > > + * control values are supported.
> > > + *
> > > + * \return 0 on success or errors from the base class
> > > + */
> > > +int Agc::init(IPAContext &context, const YamlObject &tuningData)
> > > +{
> > > + int ret;
> > > +
> > > + ret = parseTuningData(tuningData);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + context.ctrlMap.merge(controls());
> > > +
> > > + return 0;
> > > +}
> > > +
> > > /**
> > > * \brief Configure the AGC given a configInfo
> > > * \param[in] context The shared IPA context
> > > @@ -90,6 +112,7 @@ int Agc::configure(IPAContext &context,
> > > IPAActiveState &activeState = context.activeState;
> > > stride_ = configuration.grid.stride;
> > > + bdsGrid_ = configuration.grid.bdsGrid;
> > > minShutterSpeed_ = configuration.agc.minShutterSpeed;
> > > maxShutterSpeed_ = std::min(configuration.agc.maxShutterSpeed,
> > > @@ -103,6 +126,16 @@ int Agc::configure(IPAContext &context,
> > > activeState.agc.exposure = 10ms / configuration.sensor.lineDuration;
> > > frameCount_ = 0;
> > IMHO That line can be removed as you call resetFrameCount() below.
>
>
> This is actually libcamera::ipa::ipu3::algorithms::Agc::frameCount_ rather
> than libcamera::ipa::AgcMeanLuminance::frameCount_, which is what
> resetFrameCount() touches. It's removed along with the rest of the bespoke
> implementation in the next patch...does it still need to be removed here?
Oh I missed that. Sorry for the noise. Then there is no need to remove
it here.
>
> >
> > With that fixed
> > Reviewed-by: Stefan Klug <stefan.klug at ideasonboard.com>
>
>
> Thanks!
>
> >
> > Cheers,
> > Stefan
> >
> > > +
> > > + context.activeState.agc.constraintMode = constraintModes().begin()->first;
> > > + context.activeState.agc.exposureMode = exposureModeHelpers().begin()->first;
> > > +
> > > + /* \todo Run this again when FrameDurationLimits is passed in */
> > > + configureExposureModeHelpers(minShutterSpeed_, maxShutterSpeed_,
> > > + minAnalogueGain_, maxAnalogueGain_);
> > > +
> > > + resetFrameCount();
> > > +
> > > return 0;
> > > }
> > > @@ -142,6 +175,39 @@ double Agc::measureBrightness(const ipu3_uapi_stats_3a *stats,
> > > return Histogram(Span<uint32_t>(hist)).interQuantileMean(0.98, 1.0);
> > > }
> > > +Histogram Agc::parseStatistics(const ipu3_uapi_stats_3a *stats,
> > > + const ipu3_uapi_grid_config &grid)
> > > +{
> > > + uint32_t hist[knumHistogramBins] = { 0 };
> > > +
> > > + rgbTriples_.clear();
> > > +
> > > + for (unsigned int cellY = 0; cellY < grid.height; cellY++) {
> > > + for (unsigned int cellX = 0; cellX < grid.width; cellX++) {
> > > + uint32_t cellPosition = cellY * stride_ + cellX;
> > > +
> > > + const ipu3_uapi_awb_set_item *cell =
> > > + reinterpret_cast<const ipu3_uapi_awb_set_item *>(
> > > + &stats->awb_raw_buffer.meta_data[cellPosition]);
> > > +
> > > + rgbTriples_.push_back({
> > > + cell->R_avg,
> > > + (cell->Gr_avg + cell->Gb_avg) / 2,
> > > + cell->B_avg
> > > + });
> > > +
> > > + /*
> > > + * Store the average green value to estimate the
> > > + * brightness. Even the overexposed pixels are
> > > + * taken into account.
> > > + */
> > > + hist[(cell->Gr_avg + cell->Gb_avg) / 2]++;
> > > + }
> > > + }
> > > +
> > > + return Histogram(Span<uint32_t>(hist));
> > > +}
> > > +
> > > /**
> > > * \brief Apply a filter on the exposure value to limit the speed of changes
> > > * \param[in] exposureValue The target exposure from the AGC algorithm
> > > @@ -247,11 +313,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext,
> > > LOG(IPU3Agc, Debug) << "Divided up shutter and gain are "
> > > << shutterTime << " and "
> > > << stepGain;
> > > -
> > > - IPAActiveState &activeState = context.activeState;
> > > - /* Update the estimated exposure and gain. */
> > > - activeState.agc.exposure = shutterTime / configuration.sensor.lineDuration;
> > > - activeState.agc.gain = stepGain;
> > > }
> > > /**
> > > @@ -314,6 +375,23 @@ double Agc::estimateLuminance(IPAActiveState &activeState,
> > > return ySum / (grid.height * grid.width) / 255;
> > > }
> > > +double Agc::estimateLuminance(double gain)
> > > +{
> > > + double redSum = 0, greenSum = 0, blueSum = 0;
> > > +
> > > + for (unsigned int i = 0; i < rgbTriples_.size(); i++) {
> > > + redSum += std::min(std::get<0>(rgbTriples_[i]) * gain, 255.0);
> > > + greenSum += std::min(std::get<1>(rgbTriples_[i]) * gain, 255.0);
> > > + blueSum += std::min(std::get<2>(rgbTriples_[i]) * gain, 255.0);
> > > + }
> > > +
> > > + double ySum = redSum * rGain_ * 0.299
> > > + + greenSum * gGain_ * 0.587
> > > + + blueSum * bGain_ * 0.114;
> > > +
> > > + return ySum / (bdsGrid_.height * bdsGrid_.width) / 255;
> > > +}
> > > +
> > > /**
> > > * \brief Process IPU3 statistics, and run AGC operations
> > > * \param[in] context The shared IPA context
> > > @@ -366,8 +444,36 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> > > computeExposure(context, frameContext, yGain, iqMeanGain);
> > > frameCount_++;
> > > + Histogram hist = parseStatistics(stats, context.configuration.grid.bdsGrid);
> > > + rGain_ = context.activeState.awb.gains.red;
> > > + gGain_ = context.activeState.awb.gains.blue;
> > > + bGain_ = context.activeState.awb.gains.green;
> > > +
> > > + /*
> > > + * The Agc algorithm needs to know the effective exposure value that was
> > > + * applied to the sensor when the statistics were collected.
> > > + */
> > > utils::Duration exposureTime = context.configuration.sensor.lineDuration
> > > * frameContext.sensor.exposure;
> > > + double analogueGain = frameContext.sensor.gain;
> > > + utils::Duration effectiveExposureValue = exposureTime * analogueGain;
> > > +
> > > + utils::Duration shutterTime;
> > > + double aGain, dGain;
> > > + std::tie(shutterTime, aGain, dGain) =
> > > + calculateNewEv(context.activeState.agc.constraintMode,
> > > + context.activeState.agc.exposureMode, hist,
> > > + effectiveExposureValue);
> > > +
> > > + LOG(IPU3Agc, Debug)
> > > + << "Divided up shutter, analogue gain and digital gain are "
> > > + << shutterTime << ", " << aGain << " and " << dGain;
> > > +
> > > + IPAActiveState &activeState = context.activeState;
> > > + /* Update the estimated exposure and gain. */
> > > + activeState.agc.exposure = shutterTime / context.configuration.sensor.lineDuration;
> > > + activeState.agc.gain = aGain;
> > > +
> > > metadata.set(controls::AnalogueGain, frameContext.sensor.gain);
> > > metadata.set(controls::ExposureTime, exposureTime.get<std::micro>());
> > > diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> > > index 9d6e3ff1..40f32188 100644
> > > --- a/src/ipa/ipu3/algorithms/agc.h
> > > +++ b/src/ipa/ipu3/algorithms/agc.h
> > > @@ -13,6 +13,9 @@
> > > #include <libcamera/geometry.h>
> > > +#include "libipa/agc_mean_luminance.h"
> > > +#include "libipa/histogram.h"
> > > +
> > > #include "algorithm.h"
> > > namespace libcamera {
> > > @@ -21,12 +24,13 @@ struct IPACameraSensorInfo;
> > > namespace ipa::ipu3::algorithms {
> > > -class Agc : public Algorithm
> > > +class Agc : public Algorithm, public AgcMeanLuminance
> > > {
> > > public:
> > > Agc();
> > > ~Agc() = default;
> > > + int init(IPAContext &context, const YamlObject &tuningData) override;
> > > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;
> > > void process(IPAContext &context, const uint32_t frame,
> > > IPAFrameContext &frameContext,
> > > @@ -43,6 +47,9 @@ private:
> > > const ipu3_uapi_grid_config &grid,
> > > const ipu3_uapi_stats_3a *stats,
> > > double gain);
> > > + double estimateLuminance(double gain) override;
> > > + Histogram parseStatistics(const ipu3_uapi_stats_3a *stats,
> > > + const ipu3_uapi_grid_config &grid);
> > > uint64_t frameCount_;
> > > @@ -55,6 +62,11 @@ private:
> > > utils::Duration filteredExposure_;
> > > uint32_t stride_;
> > > + double rGain_;
> > > + double gGain_;
> > > + double bGain_;
> > > + ipu3_uapi_grid_config bdsGrid_;
> > > + std::vector<std::tuple<uint8_t, uint8_t, uint8_t>> rgbTriples_;
> > > };
> > > } /* namespace ipa::ipu3::algorithms */
> > > diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> > > index 959f314f..c4fb5642 100644
> > > --- a/src/ipa/ipu3/ipa_context.cpp
> > > +++ b/src/ipa/ipu3/ipa_context.cpp
> > > @@ -47,6 +47,9 @@ namespace libcamera::ipa::ipu3 {
> > > *
> > > * \var IPAContext::activeState
> > > * \brief The current state of IPA algorithms
> > > + *
> > > + * \var IPAContext::ctrlMap
> > > + * \brief A ControlInfoMap::Map of controls populated by the algorithms
> > > */
> > > /**
> > > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> > > index e9a3863b..a92cb6ce 100644
> > > --- a/src/ipa/ipu3/ipa_context.h
> > > +++ b/src/ipa/ipu3/ipa_context.h
> > > @@ -12,6 +12,7 @@
> > > #include <libcamera/base/utils.h>
> > > +#include <libcamera/controls.h>
> > > #include <libcamera/geometry.h>
> > > #include <libipa/fc_queue.h>
> > > @@ -55,6 +56,8 @@ struct IPAActiveState {
> > > struct {
> > > uint32_t exposure;
> > > double gain;
> > > + uint32_t constraintMode;
> > > + uint32_t exposureMode;
> > > } agc;
> > > struct {
> > > @@ -85,6 +88,8 @@ struct IPAContext {
> > > IPAActiveState activeState;
> > > FCQueue<IPAFrameContext> frameContexts;
> > > +
> > > + ControlInfoMap::Map ctrlMap;
> > > };
> > > } /* namespace ipa::ipu3 */
> > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> > > index 08ee6eb3..4809eb60 100644
> > > --- a/src/ipa/ipu3/ipu3.cpp
> > > +++ b/src/ipa/ipu3/ipu3.cpp
> > > @@ -189,7 +189,7 @@ private:
> > > };
> > > IPAIPU3::IPAIPU3()
> > > - : context_({ {}, {}, { kMaxFrameContexts } })
> > > + : context_({ {}, {}, { kMaxFrameContexts }, {} })
> > > {
> > > }
> > > @@ -287,6 +287,7 @@ void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,
> > > frameDurations[1],
> > > frameDurations[2]);
> > > + controls.merge(context_.ctrlMap);
> > > *ipaControls = ControlInfoMap(std::move(controls), controls::controls);
> > > }
> > > --
> > > 2.34.1
> > >
More information about the libcamera-devel
mailing list