| Message ID | 20240322131451.3092931-7-dan.scally@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Daniel, thanks for the patch. On Fri, Mar 22, 2024 at 01:14:47PM +0000, Daniel Scally wrote: > 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 simplicities 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@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/agc.cpp | 98 ++++++++++++++++++++++++++++++--- > src/ipa/ipu3/algorithms/agc.h | 6 +- > src/ipa/ipu3/ipa_context.cpp | 3 + > src/ipa/ipu3/ipa_context.h | 5 ++ > src/ipa/ipu3/ipu3.cpp | 2 +- > 5 files changed, 105 insertions(+), 9 deletions(-) > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > index ee9a42cf..a84534ea 100644 > --- a/src/ipa/ipu3/algorithms/agc.cpp > +++ b/src/ipa/ipu3/algorithms/agc.cpp > @@ -71,11 +71,41 @@ 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), context_(nullptr) > { > } > > +/** > + * \brief Initialise the AGC algorith 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; > + > + parseRelativeLuminanceTarget(tuningData); > + > + ret = parseConstraintModes(tuningData); > + if (ret) > + return ret; > + > + ret = parseExposureModes(tuningData); > + if (ret) > + return ret; > + If I got it right, the agc implementation in libipa requires all parseXXX to be called and even if the tuningData misses the relevant entries, dummy entries are generated. So I think these calls should be moved to MeanLuminanceAgc::parseTuningData() Cheers, Stefan > + context.ctrlMap.merge(controls()); > + context_ = &context; > + > + return 0; > +} > + > /** > * \brief Configure the AGC given a configInfo > * \param[in] context The shared IPA context > @@ -103,6 +133,20 @@ int Agc::configure(IPAContext &context, > activeState.agc.exposure = 10ms / configuration.sensor.lineDuration; > > frameCount_ = 0; > + > + /* > + * \todo We should use the first available mode rather than assume that > + * the "Normal" modes are present in tuning data. > + */ > + context.activeState.agc.constraintMode = controls::ConstraintNormal; > + context.activeState.agc.exposureMode = controls::ExposureNormal; > + > + for (auto &[id, helper] : exposureModeHelpers()) { > + /* \todo Run this again when FrameDurationLimits is passed in */ > + helper->configure(minShutterSpeed_, maxShutterSpeed_, > + minAnalogueGain_, maxAnalogueGain_); > + } > + > return 0; > } > > @@ -280,11 +324,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; > } > > /** > @@ -347,6 +386,26 @@ double Agc::estimateLuminance(IPAActiveState &activeState, > return ySum / (grid.height * grid.width) / 255; > } > > +double Agc::estimateLuminance(double gain) > +{ > + ASSERT(reds_.size() == greens_.size()); > + ASSERT(greens_.size() == blues_.size()); > + const ipu3_uapi_grid_config &grid = context_->configuration.grid.bdsGrid; > + double redSum = 0, greenSum = 0, blueSum = 0; > + > + for (unsigned int i = 0; i < reds_.size(); i++) { > + redSum += std::min(reds_[i] * gain, 255.0); > + greenSum += std::min(greens_[i] * gain, 255.0); > + blueSum += std::min(blues_[i] * gain, 255.0); > + } > + > + double ySum = redSum * context_->activeState.awb.gains.red * 0.299 > + + greenSum * context_->activeState.awb.gains.green * 0.587 > + + blueSum * context_->activeState.awb.gains.blue * 0.114; > + > + return ySum / (grid.height * grid.width) / 255; > +} > + > /** > * \brief Process IPU3 statistics, and run AGC operations > * \param[in] context The shared IPA context > @@ -399,8 +458,33 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > computeExposure(context, frameContext, yGain, iqMeanGain); > frameCount_++; > > + parseStatistics(stats, context.configuration.grid.bdsGrid); > + > + /* > + * 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 7ed0ef7a..8405da9d 100644 > --- a/src/ipa/ipu3/algorithms/agc.h > +++ b/src/ipa/ipu3/algorithms/agc.h > @@ -13,6 +13,7 @@ > > #include <libcamera/geometry.h> > > +#include "libipa/agc.h" > #include "libipa/histogram.h" > > #include "algorithm.h" > @@ -23,12 +24,13 @@ struct IPACameraSensorInfo; > > namespace ipa::ipu3::algorithms { > > -class Agc : public Algorithm > +class Agc : public Algorithm, public MeanLuminanceAgc > { > 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, > @@ -45,6 +47,7 @@ private: > const ipu3_uapi_grid_config &grid, > const ipu3_uapi_stats_3a *stats, > double gain); > + double estimateLuminance(double gain) override; > void parseStatistics(const ipu3_uapi_stats_3a *stats, > const ipu3_uapi_grid_config &grid); > > @@ -59,6 +62,7 @@ private: > utils::Duration filteredExposure_; > > uint32_t stride_; > + IPAContext *context_; > std::vector<uint8_t> reds_; > std::vector<uint8_t> blues_; > std::vector<uint8_t> greens_; > 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..2fcc0c91 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -189,7 +189,7 @@ private: > }; > > IPAIPU3::IPAIPU3() > - : context_({ {}, {}, { kMaxFrameContexts } }) > + : context_({ {}, {}, { kMaxFrameContexts }, {} }) > { > } > > -- > 2.34.1 >
Hi Stefan On 27/03/2024 15:02, Stefan Klug wrote: > Hi Daniel, > > thanks for the patch. > > On Fri, Mar 22, 2024 at 01:14:47PM +0000, Daniel Scally wrote: >> 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 simplicities 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@ideasonboard.com> >> --- >> src/ipa/ipu3/algorithms/agc.cpp | 98 ++++++++++++++++++++++++++++++--- >> src/ipa/ipu3/algorithms/agc.h | 6 +- >> src/ipa/ipu3/ipa_context.cpp | 3 + >> src/ipa/ipu3/ipa_context.h | 5 ++ >> src/ipa/ipu3/ipu3.cpp | 2 +- >> 5 files changed, 105 insertions(+), 9 deletions(-) >> >> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp >> index ee9a42cf..a84534ea 100644 >> --- a/src/ipa/ipu3/algorithms/agc.cpp >> +++ b/src/ipa/ipu3/algorithms/agc.cpp >> @@ -71,11 +71,41 @@ 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), context_(nullptr) >> { >> } >> >> +/** >> + * \brief Initialise the AGC algorith 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; >> + >> + parseRelativeLuminanceTarget(tuningData); >> + >> + ret = parseConstraintModes(tuningData); >> + if (ret) >> + return ret; >> + >> + ret = parseExposureModes(tuningData); >> + if (ret) >> + return ret; >> + > If I got it right, the agc implementation in libipa requires all > parseXXX to be called Correct > and even if the tuningData misses the relevant > entries, dummy entries are generated. Correct > So I think these calls should be > moved to MeanLuminanceAgc::parseTuningData() OK, will do - thanks for the review! > > Cheers, > Stefan > >> + context.ctrlMap.merge(controls()); >> + context_ = &context; >> + >> + return 0; >> +} >> + >> /** >> * \brief Configure the AGC given a configInfo >> * \param[in] context The shared IPA context >> @@ -103,6 +133,20 @@ int Agc::configure(IPAContext &context, >> activeState.agc.exposure = 10ms / configuration.sensor.lineDuration; >> >> frameCount_ = 0; >> + >> + /* >> + * \todo We should use the first available mode rather than assume that >> + * the "Normal" modes are present in tuning data. >> + */ >> + context.activeState.agc.constraintMode = controls::ConstraintNormal; >> + context.activeState.agc.exposureMode = controls::ExposureNormal; >> + >> + for (auto &[id, helper] : exposureModeHelpers()) { >> + /* \todo Run this again when FrameDurationLimits is passed in */ >> + helper->configure(minShutterSpeed_, maxShutterSpeed_, >> + minAnalogueGain_, maxAnalogueGain_); >> + } >> + >> return 0; >> } >> >> @@ -280,11 +324,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; >> } >> >> /** >> @@ -347,6 +386,26 @@ double Agc::estimateLuminance(IPAActiveState &activeState, >> return ySum / (grid.height * grid.width) / 255; >> } >> >> +double Agc::estimateLuminance(double gain) >> +{ >> + ASSERT(reds_.size() == greens_.size()); >> + ASSERT(greens_.size() == blues_.size()); >> + const ipu3_uapi_grid_config &grid = context_->configuration.grid.bdsGrid; >> + double redSum = 0, greenSum = 0, blueSum = 0; >> + >> + for (unsigned int i = 0; i < reds_.size(); i++) { >> + redSum += std::min(reds_[i] * gain, 255.0); >> + greenSum += std::min(greens_[i] * gain, 255.0); >> + blueSum += std::min(blues_[i] * gain, 255.0); >> + } >> + >> + double ySum = redSum * context_->activeState.awb.gains.red * 0.299 >> + + greenSum * context_->activeState.awb.gains.green * 0.587 >> + + blueSum * context_->activeState.awb.gains.blue * 0.114; >> + >> + return ySum / (grid.height * grid.width) / 255; >> +} >> + >> /** >> * \brief Process IPU3 statistics, and run AGC operations >> * \param[in] context The shared IPA context >> @@ -399,8 +458,33 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, >> computeExposure(context, frameContext, yGain, iqMeanGain); >> frameCount_++; >> >> + parseStatistics(stats, context.configuration.grid.bdsGrid); >> + >> + /* >> + * 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 7ed0ef7a..8405da9d 100644 >> --- a/src/ipa/ipu3/algorithms/agc.h >> +++ b/src/ipa/ipu3/algorithms/agc.h >> @@ -13,6 +13,7 @@ >> >> #include <libcamera/geometry.h> >> >> +#include "libipa/agc.h" >> #include "libipa/histogram.h" >> >> #include "algorithm.h" >> @@ -23,12 +24,13 @@ struct IPACameraSensorInfo; >> >> namespace ipa::ipu3::algorithms { >> >> -class Agc : public Algorithm >> +class Agc : public Algorithm, public MeanLuminanceAgc >> { >> 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, >> @@ -45,6 +47,7 @@ private: >> const ipu3_uapi_grid_config &grid, >> const ipu3_uapi_stats_3a *stats, >> double gain); >> + double estimateLuminance(double gain) override; >> void parseStatistics(const ipu3_uapi_stats_3a *stats, >> const ipu3_uapi_grid_config &grid); >> >> @@ -59,6 +62,7 @@ private: >> utils::Duration filteredExposure_; >> >> uint32_t stride_; >> + IPAContext *context_; >> std::vector<uint8_t> reds_; >> std::vector<uint8_t> blues_; >> std::vector<uint8_t> greens_; >> 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..2fcc0c91 100644 >> --- a/src/ipa/ipu3/ipu3.cpp >> +++ b/src/ipa/ipu3/ipu3.cpp >> @@ -189,7 +189,7 @@ private: >> }; >> >> IPAIPU3::IPAIPU3() >> - : context_({ {}, {}, { kMaxFrameContexts } }) >> + : context_({ {}, {}, { kMaxFrameContexts }, {} }) >> { >> } >> >> -- >> 2.34.1 >>
Hi Dan, Thank you for the patch. On Fri, Mar 22, 2024 at 01:14:47PM +0000, Daniel Scally wrote: > 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 simplicities sake s/simplicities/simplicity's/ > 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@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/agc.cpp | 98 ++++++++++++++++++++++++++++++--- > src/ipa/ipu3/algorithms/agc.h | 6 +- > src/ipa/ipu3/ipa_context.cpp | 3 + > src/ipa/ipu3/ipa_context.h | 5 ++ > src/ipa/ipu3/ipu3.cpp | 2 +- > 5 files changed, 105 insertions(+), 9 deletions(-) > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > index ee9a42cf..a84534ea 100644 > --- a/src/ipa/ipu3/algorithms/agc.cpp > +++ b/src/ipa/ipu3/algorithms/agc.cpp > @@ -71,11 +71,41 @@ 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), context_(nullptr) > { > } > > +/** > + * \brief Initialise the AGC algorith from tuning files > + * No blank line. > + * \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; > + > + parseRelativeLuminanceTarget(tuningData); > + > + ret = parseConstraintModes(tuningData); > + if (ret) > + return ret; > + > + ret = parseExposureModes(tuningData); > + if (ret) > + return ret; Turn all this into a single function. > + > + context.ctrlMap.merge(controls()); > + context_ = &context; > + > + return 0; > +} > + > /** > * \brief Configure the AGC given a configInfo > * \param[in] context The shared IPA context > @@ -103,6 +133,20 @@ int Agc::configure(IPAContext &context, > activeState.agc.exposure = 10ms / configuration.sensor.lineDuration; > > frameCount_ = 0; > + > + /* > + * \todo We should use the first available mode rather than assume that > + * the "Normal" modes are present in tuning data. > + */ > + context.activeState.agc.constraintMode = controls::ConstraintNormal; This sounds crash-prone if the tuning data file doesn't have a normal mode. Some more work is needed to define what tuning files need to provide. > + context.activeState.agc.exposureMode = controls::ExposureNormal; > + > + for (auto &[id, helper] : exposureModeHelpers()) { > + /* \todo Run this again when FrameDurationLimits is passed in */ > + helper->configure(minShutterSpeed_, maxShutterSpeed_, > + minAnalogueGain_, maxAnalogueGain_); > + } Can you move this loop to the base class ? > + > return 0; > } > > @@ -280,11 +324,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; > } > > /** > @@ -347,6 +386,26 @@ double Agc::estimateLuminance(IPAActiveState &activeState, > return ySum / (grid.height * grid.width) / 255; > } > > +double Agc::estimateLuminance(double gain) > +{ > + ASSERT(reds_.size() == greens_.size()); > + ASSERT(greens_.size() == blues_.size()); If you store the values in a single vector of RGB triplets the error won't be possible anymore. > + const ipu3_uapi_grid_config &grid = context_->configuration.grid.bdsGrid; > + double redSum = 0, greenSum = 0, blueSum = 0; > + > + for (unsigned int i = 0; i < reds_.size(); i++) { > + redSum += std::min(reds_[i] * gain, 255.0); > + greenSum += std::min(greens_[i] * gain, 255.0); > + blueSum += std::min(blues_[i] * gain, 255.0); > + } > + > + double ySum = redSum * context_->activeState.awb.gains.red * 0.299 > + + greenSum * context_->activeState.awb.gains.green * 0.587 > + + blueSum * context_->activeState.awb.gains.blue * 0.114; Align the + with the =. > + > + return ySum / (grid.height * grid.width) / 255; > +} > + > /** > * \brief Process IPU3 statistics, and run AGC operations > * \param[in] context The shared IPA context > @@ -399,8 +458,33 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > computeExposure(context, frameContext, yGain, iqMeanGain); > frameCount_++; > > + parseStatistics(stats, context.configuration.grid.bdsGrid); > + > + /* > + * 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 7ed0ef7a..8405da9d 100644 > --- a/src/ipa/ipu3/algorithms/agc.h > +++ b/src/ipa/ipu3/algorithms/agc.h > @@ -13,6 +13,7 @@ > > #include <libcamera/geometry.h> > > +#include "libipa/agc.h" > #include "libipa/histogram.h" > > #include "algorithm.h" > @@ -23,12 +24,13 @@ struct IPACameraSensorInfo; > > namespace ipa::ipu3::algorithms { > > -class Agc : public Algorithm > +class Agc : public Algorithm, public MeanLuminanceAgc > { > 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, > @@ -45,6 +47,7 @@ private: > const ipu3_uapi_grid_config &grid, > const ipu3_uapi_stats_3a *stats, > double gain); > + double estimateLuminance(double gain) override; This function should be private in the base class. > void parseStatistics(const ipu3_uapi_stats_3a *stats, > const ipu3_uapi_grid_config &grid); > > @@ -59,6 +62,7 @@ private: > utils::Duration filteredExposure_; > > uint32_t stride_; > + IPAContext *context_; No please. The context is meant to be passed to the algorithm. Don't retain a pointer. You can store a copy of the grid size, the same way we already have a copy of the stride. > std::vector<uint8_t> reds_; > std::vector<uint8_t> blues_; > std::vector<uint8_t> greens_; > 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; Populated but never used, AFAICT. > }; > > } /* namespace ipa::ipu3 */ > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > index 08ee6eb3..2fcc0c91 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -189,7 +189,7 @@ private: > }; > > IPAIPU3::IPAIPU3() > - : context_({ {}, {}, { kMaxFrameContexts } }) > + : context_({ {}, {}, { kMaxFrameContexts }, {} }) > { > } >
diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp index ee9a42cf..a84534ea 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -71,11 +71,41 @@ 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), context_(nullptr) { } +/** + * \brief Initialise the AGC algorith 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; + + parseRelativeLuminanceTarget(tuningData); + + ret = parseConstraintModes(tuningData); + if (ret) + return ret; + + ret = parseExposureModes(tuningData); + if (ret) + return ret; + + context.ctrlMap.merge(controls()); + context_ = &context; + + return 0; +} + /** * \brief Configure the AGC given a configInfo * \param[in] context The shared IPA context @@ -103,6 +133,20 @@ int Agc::configure(IPAContext &context, activeState.agc.exposure = 10ms / configuration.sensor.lineDuration; frameCount_ = 0; + + /* + * \todo We should use the first available mode rather than assume that + * the "Normal" modes are present in tuning data. + */ + context.activeState.agc.constraintMode = controls::ConstraintNormal; + context.activeState.agc.exposureMode = controls::ExposureNormal; + + for (auto &[id, helper] : exposureModeHelpers()) { + /* \todo Run this again when FrameDurationLimits is passed in */ + helper->configure(minShutterSpeed_, maxShutterSpeed_, + minAnalogueGain_, maxAnalogueGain_); + } + return 0; } @@ -280,11 +324,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; } /** @@ -347,6 +386,26 @@ double Agc::estimateLuminance(IPAActiveState &activeState, return ySum / (grid.height * grid.width) / 255; } +double Agc::estimateLuminance(double gain) +{ + ASSERT(reds_.size() == greens_.size()); + ASSERT(greens_.size() == blues_.size()); + const ipu3_uapi_grid_config &grid = context_->configuration.grid.bdsGrid; + double redSum = 0, greenSum = 0, blueSum = 0; + + for (unsigned int i = 0; i < reds_.size(); i++) { + redSum += std::min(reds_[i] * gain, 255.0); + greenSum += std::min(greens_[i] * gain, 255.0); + blueSum += std::min(blues_[i] * gain, 255.0); + } + + double ySum = redSum * context_->activeState.awb.gains.red * 0.299 + + greenSum * context_->activeState.awb.gains.green * 0.587 + + blueSum * context_->activeState.awb.gains.blue * 0.114; + + return ySum / (grid.height * grid.width) / 255; +} + /** * \brief Process IPU3 statistics, and run AGC operations * \param[in] context The shared IPA context @@ -399,8 +458,33 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, computeExposure(context, frameContext, yGain, iqMeanGain); frameCount_++; + parseStatistics(stats, context.configuration.grid.bdsGrid); + + /* + * 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 7ed0ef7a..8405da9d 100644 --- a/src/ipa/ipu3/algorithms/agc.h +++ b/src/ipa/ipu3/algorithms/agc.h @@ -13,6 +13,7 @@ #include <libcamera/geometry.h> +#include "libipa/agc.h" #include "libipa/histogram.h" #include "algorithm.h" @@ -23,12 +24,13 @@ struct IPACameraSensorInfo; namespace ipa::ipu3::algorithms { -class Agc : public Algorithm +class Agc : public Algorithm, public MeanLuminanceAgc { 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, @@ -45,6 +47,7 @@ private: const ipu3_uapi_grid_config &grid, const ipu3_uapi_stats_3a *stats, double gain); + double estimateLuminance(double gain) override; void parseStatistics(const ipu3_uapi_stats_3a *stats, const ipu3_uapi_grid_config &grid); @@ -59,6 +62,7 @@ private: utils::Duration filteredExposure_; uint32_t stride_; + IPAContext *context_; std::vector<uint8_t> reds_; std::vector<uint8_t> blues_; std::vector<uint8_t> greens_; 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..2fcc0c91 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -189,7 +189,7 @@ private: }; IPAIPU3::IPAIPU3() - : context_({ {}, {}, { kMaxFrameContexts } }) + : context_({ {}, {}, { kMaxFrameContexts }, {} }) { }
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 simplicities 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@ideasonboard.com> --- src/ipa/ipu3/algorithms/agc.cpp | 98 ++++++++++++++++++++++++++++++--- src/ipa/ipu3/algorithms/agc.h | 6 +- src/ipa/ipu3/ipa_context.cpp | 3 + src/ipa/ipu3/ipa_context.h | 5 ++ src/ipa/ipu3/ipu3.cpp | 2 +- 5 files changed, 105 insertions(+), 9 deletions(-)