| Message ID | 20240322131451.3092931-10-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:50PM +0000, Daniel Scally wrote: > Now that we have a MeanLuminanceAgc class that centralises our AEGC > algorithm, derive the RkISP1'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/rkisp1/algorithms/agc.cpp | 91 +++++++++++++++++++++++++++++-- > src/ipa/rkisp1/algorithms/agc.h | 5 +- > src/ipa/rkisp1/ipa_context.h | 5 ++ > src/ipa/rkisp1/rkisp1.cpp | 2 +- > 4 files changed, 96 insertions(+), 7 deletions(-) > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > index d380194d..3389c471 100644 > --- a/src/ipa/rkisp1/algorithms/agc.cpp > +++ b/src/ipa/rkisp1/algorithms/agc.cpp > @@ -64,6 +64,36 @@ Agc::Agc() > supportsRaw_ = true; > } > > +/** > + * \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; > + > + parseRelativeLuminanceTarget(tuningData); > + > + ret = parseConstraintModes(tuningData); > + if (ret) > + return ret; > + > + ret = parseExposureModes(tuningData); > + if (ret) > + return ret; > + Same comment as for ipu3 > + context.ctrlMap.merge(controls()); > + > + return 0; > +} > + > /** > * \brief Configure the AGC given a configInfo > * \param[in] context The shared IPA context > @@ -81,6 +111,13 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) > context.activeState.agc.manual.exposure = context.activeState.agc.automatic.exposure; > context.activeState.agc.autoEnabled = !context.configuration.raw; > > + /* > + * \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; > + > /* > * Define the measurement window for AGC as a centered rectangle > * covering 3/4 of the image width and height. > @@ -95,6 +132,15 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) > * frame index > */ > frameCount_ = 0; > + > + for (auto &[id, helper] : exposureModeHelpers()) { > + /* \todo Run this again when FrameDurationLimits is passed in */ > + helper->configure(context.configuration.sensor.minShutterSpeed, > + context.configuration.sensor.maxShutterSpeed, Not part of this patch, but why are these called minShutterSpeed and not minExposureTime? > + context.configuration.sensor.minAnalogueGain, > + context.configuration.sensor.maxAnalogueGain); > + } > + > return 0; > } > > @@ -234,7 +280,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext, > double yGain, double iqMeanGain) > { > IPASessionConfiguration &configuration = context.configuration; > - IPAActiveState &activeState = context.activeState; > > /* Get the effective exposure and gain applied on the sensor. */ > uint32_t exposure = frameContext.sensor.exposure; > @@ -300,10 +345,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext, > LOG(RkISP1Agc, Debug) << "Divided up shutter and gain are " > << shutterTime << " and " > << stepGain; > - > - /* Update the estimated exposure and gain. */ > - activeState.agc.automatic.exposure = shutterTime / configuration.sensor.lineDuration; > - activeState.agc.automatic.gain = stepGain; > } > > /** > @@ -383,6 +424,19 @@ void Agc::parseStatistics(const rkisp1_stat_buffer *stats, > context.hw->numHistogramBins)); > } > > +double Agc::estimateLuminance(double gain) > +{ > + double ySum = 0.0; > + > + /* Sum the averages, saturated to 255. */ > + for (uint8_t expMean : expMeans_) > + ySum += std::min(expMean * gain, 255.0); > + > + /* \todo Weight with the AWB gains */ > + > + return ySum / expMeans_.size() / 255; > +} > + > /** > * \brief Process RkISP1 statistics, and run AGC operations > * \param[in] context The shared IPA context > @@ -448,6 +502,33 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > computeExposure(context, frameContext, yGain, iqMeanGain); > frameCount_++; > > + parseStatistics(stats, context); Regarding my previous mail. Here expMean_ would be assigned... > + > + /* > + * 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(RkISP1Agc, 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.automatic.exposure = shutterTime / context.configuration.sensor.lineDuration; > + activeState.agc.automatic.gain = aGain; > + > fillMetadata(context, frameContext, metadata); ...and here it would be reset. Cheers, Stefan > } > > diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h > index b0de4898..1271741e 100644 > --- a/src/ipa/rkisp1/algorithms/agc.h > +++ b/src/ipa/rkisp1/algorithms/agc.h > @@ -14,6 +14,7 @@ > > #include <libcamera/geometry.h> > > +#include "libipa/agc.h" > #include "libipa/histogram.h" > > #include "algorithm.h" > @@ -22,12 +23,13 @@ namespace libcamera { > > namespace ipa::rkisp1::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 IPACameraSensorInfo &configInfo) override; > void queueRequest(IPAContext &context, > const uint32_t frame, > @@ -51,6 +53,7 @@ private: > ControlList &metadata); > void parseStatistics(const rkisp1_stat_buffer *stats, > IPAContext &context); > + double estimateLuminance(double gain) override; > > uint64_t frameCount_; > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > index 10d8f38c..256b75eb 100644 > --- a/src/ipa/rkisp1/ipa_context.h > +++ b/src/ipa/rkisp1/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> > @@ -67,6 +68,8 @@ struct IPAActiveState { > } automatic; > > bool autoEnabled; > + uint32_t constraintMode; > + uint32_t exposureMode; > } agc; > > struct { > @@ -151,6 +154,8 @@ struct IPAContext { > IPAActiveState activeState; > > FCQueue<IPAFrameContext> frameContexts; > + > + ControlInfoMap::Map ctrlMap; > }; > > } /* namespace ipa::rkisp1 */ > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > index 9dc5f53c..5f74762f 100644 > --- a/src/ipa/rkisp1/rkisp1.cpp > +++ b/src/ipa/rkisp1/rkisp1.cpp > @@ -119,7 +119,7 @@ const ControlInfoMap::Map rkisp1Controls{ > } /* namespace */ > > IPARkISP1::IPARkISP1() > - : context_({ {}, {}, {}, { kMaxFrameContexts } }) > + : context_({ {}, {}, {}, { kMaxFrameContexts }, {} }) > { > } > > -- > 2.34.1 >
On Wed, Mar 27, 2024 at 05:11:04PM +0100, Stefan Klug wrote: > Hi Daniel, > > thanks for the patch. > > On Fri, Mar 22, 2024 at 01:14:50PM +0000, Daniel Scally wrote: > > Now that we have a MeanLuminanceAgc class that centralises our AEGC > > algorithm, derive the RkISP1'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/rkisp1/algorithms/agc.cpp | 91 +++++++++++++++++++++++++++++-- > > src/ipa/rkisp1/algorithms/agc.h | 5 +- > > src/ipa/rkisp1/ipa_context.h | 5 ++ > > src/ipa/rkisp1/rkisp1.cpp | 2 +- > > 4 files changed, 96 insertions(+), 7 deletions(-) > > > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > > index d380194d..3389c471 100644 > > --- a/src/ipa/rkisp1/algorithms/agc.cpp > > +++ b/src/ipa/rkisp1/algorithms/agc.cpp > > @@ -64,6 +64,36 @@ Agc::Agc() > > supportsRaw_ = true; > > } > > > > +/** > > + * \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; > > + > > + parseRelativeLuminanceTarget(tuningData); > > + > > + ret = parseConstraintModes(tuningData); > > + if (ret) > > + return ret; > > + > > + ret = parseExposureModes(tuningData); > > + if (ret) > > + return ret; > > + > > Same comment as for ipu3 > > > + context.ctrlMap.merge(controls()); > > + > > + return 0; > > +} > > + > > /** > > * \brief Configure the AGC given a configInfo > > * \param[in] context The shared IPA context > > @@ -81,6 +111,13 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) > > context.activeState.agc.manual.exposure = context.activeState.agc.automatic.exposure; > > context.activeState.agc.autoEnabled = !context.configuration.raw; > > > > + /* > > + * \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; > > + > > /* > > * Define the measurement window for AGC as a centered rectangle > > * covering 3/4 of the image width and height. > > @@ -95,6 +132,15 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) > > * frame index > > */ > > frameCount_ = 0; > > + > > + for (auto &[id, helper] : exposureModeHelpers()) { > > + /* \todo Run this again when FrameDurationLimits is passed in */ > > + helper->configure(context.configuration.sensor.minShutterSpeed, > > + context.configuration.sensor.maxShutterSpeed, > > Not part of this patch, but why are these called minShutterSpeed and not > minExposureTime? Let's bite the bullet and define a standard vocabulary. > > + context.configuration.sensor.minAnalogueGain, > > + context.configuration.sensor.maxAnalogueGain); > > + } > > + > > return 0; > > } > > > > @@ -234,7 +280,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext, > > double yGain, double iqMeanGain) > > { > > IPASessionConfiguration &configuration = context.configuration; > > - IPAActiveState &activeState = context.activeState; > > > > /* Get the effective exposure and gain applied on the sensor. */ > > uint32_t exposure = frameContext.sensor.exposure; > > @@ -300,10 +345,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext, > > LOG(RkISP1Agc, Debug) << "Divided up shutter and gain are " > > << shutterTime << " and " > > << stepGain; > > - > > - /* Update the estimated exposure and gain. */ > > - activeState.agc.automatic.exposure = shutterTime / configuration.sensor.lineDuration; > > - activeState.agc.automatic.gain = stepGain; > > } > > > > /** > > @@ -383,6 +424,19 @@ void Agc::parseStatistics(const rkisp1_stat_buffer *stats, > > context.hw->numHistogramBins)); > > } > > > > +double Agc::estimateLuminance(double gain) > > +{ > > + double ySum = 0.0; > > + > > + /* Sum the averages, saturated to 255. */ > > + for (uint8_t expMean : expMeans_) > > + ySum += std::min(expMean * gain, 255.0); > > + > > + /* \todo Weight with the AWB gains */ How so ? > > + > > + return ySum / expMeans_.size() / 255; > > +} > > + > > /** > > * \brief Process RkISP1 statistics, and run AGC operations > > * \param[in] context The shared IPA context > > @@ -448,6 +502,33 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > > computeExposure(context, frameContext, yGain, iqMeanGain); > > frameCount_++; > > > > + parseStatistics(stats, context); > > Regarding my previous mail. Here expMean_ would be assigned... > > > + > > + /* > > + * 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(RkISP1Agc, 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.automatic.exposure = shutterTime / context.configuration.sensor.lineDuration; > > + activeState.agc.automatic.gain = aGain; > > + > > fillMetadata(context, frameContext, metadata); > > ...and here it would be reset. > > > } > > > > diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h > > index b0de4898..1271741e 100644 > > --- a/src/ipa/rkisp1/algorithms/agc.h > > +++ b/src/ipa/rkisp1/algorithms/agc.h > > @@ -14,6 +14,7 @@ > > > > #include <libcamera/geometry.h> > > > > +#include "libipa/agc.h" > > #include "libipa/histogram.h" > > > > #include "algorithm.h" > > @@ -22,12 +23,13 @@ namespace libcamera { > > > > namespace ipa::rkisp1::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 IPACameraSensorInfo &configInfo) override; > > void queueRequest(IPAContext &context, > > const uint32_t frame, > > @@ -51,6 +53,7 @@ private: > > ControlList &metadata); > > void parseStatistics(const rkisp1_stat_buffer *stats, > > IPAContext &context); > > + double estimateLuminance(double gain) override; > > > > uint64_t frameCount_; > > > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > > index 10d8f38c..256b75eb 100644 > > --- a/src/ipa/rkisp1/ipa_context.h > > +++ b/src/ipa/rkisp1/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> > > @@ -67,6 +68,8 @@ struct IPAActiveState { > > } automatic; > > > > bool autoEnabled; > > + uint32_t constraintMode; > > + uint32_t exposureMode; > > } agc; > > > > struct { > > @@ -151,6 +154,8 @@ struct IPAContext { > > IPAActiveState activeState; > > > > FCQueue<IPAFrameContext> frameContexts; > > + > > + ControlInfoMap::Map ctrlMap; > > }; > > > > } /* namespace ipa::rkisp1 */ > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > > index 9dc5f53c..5f74762f 100644 > > --- a/src/ipa/rkisp1/rkisp1.cpp > > +++ b/src/ipa/rkisp1/rkisp1.cpp > > @@ -119,7 +119,7 @@ const ControlInfoMap::Map rkisp1Controls{ > > } /* namespace */ > > > > IPARkISP1::IPARkISP1() > > - : context_({ {}, {}, {}, { kMaxFrameContexts } }) > > + : context_({ {}, {}, {}, { kMaxFrameContexts }, {} }) > > { > > } > >
Hi Laurent On 06/04/2024 02:56, Laurent Pinchart wrote: > On Wed, Mar 27, 2024 at 05:11:04PM +0100, Stefan Klug wrote: >> Hi Daniel, >> >> thanks for the patch. >> >> On Fri, Mar 22, 2024 at 01:14:50PM +0000, Daniel Scally wrote: >>> Now that we have a MeanLuminanceAgc class that centralises our AEGC >>> algorithm, derive the RkISP1'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/rkisp1/algorithms/agc.cpp | 91 +++++++++++++++++++++++++++++-- >>> src/ipa/rkisp1/algorithms/agc.h | 5 +- >>> src/ipa/rkisp1/ipa_context.h | 5 ++ >>> src/ipa/rkisp1/rkisp1.cpp | 2 +- >>> 4 files changed, 96 insertions(+), 7 deletions(-) >>> >>> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp >>> index d380194d..3389c471 100644 >>> --- a/src/ipa/rkisp1/algorithms/agc.cpp >>> +++ b/src/ipa/rkisp1/algorithms/agc.cpp >>> @@ -64,6 +64,36 @@ Agc::Agc() >>> supportsRaw_ = true; >>> } >>> >>> +/** >>> + * \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; >>> + >>> + parseRelativeLuminanceTarget(tuningData); >>> + >>> + ret = parseConstraintModes(tuningData); >>> + if (ret) >>> + return ret; >>> + >>> + ret = parseExposureModes(tuningData); >>> + if (ret) >>> + return ret; >>> + >> Same comment as for ipu3 >> >>> + context.ctrlMap.merge(controls()); >>> + >>> + return 0; >>> +} >>> + >>> /** >>> * \brief Configure the AGC given a configInfo >>> * \param[in] context The shared IPA context >>> @@ -81,6 +111,13 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) >>> context.activeState.agc.manual.exposure = context.activeState.agc.automatic.exposure; >>> context.activeState.agc.autoEnabled = !context.configuration.raw; >>> >>> + /* >>> + * \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; >>> + >>> /* >>> * Define the measurement window for AGC as a centered rectangle >>> * covering 3/4 of the image width and height. >>> @@ -95,6 +132,15 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) >>> * frame index >>> */ >>> frameCount_ = 0; >>> + >>> + for (auto &[id, helper] : exposureModeHelpers()) { >>> + /* \todo Run this again when FrameDurationLimits is passed in */ >>> + helper->configure(context.configuration.sensor.minShutterSpeed, >>> + context.configuration.sensor.maxShutterSpeed, >> Not part of this patch, but why are these called minShutterSpeed and not >> minExposureTime? > Let's bite the bullet and define a standard vocabulary. Actually thinking about it rather than equally applicable vocabulary that we need to choose between they just seem to be named wrongly at the moment. The variables are set in IPARkISP1::configure() like so: const auto itExp = sensorControls_.find(V4L2_CID_EXPOSURE); int32_t minExposure = itExp->second.min().get<int32_t>(); int32_t maxExposure = itExp->second.max().get<int32_t>(); ... context_.configuration.sensor.minShutterSpeed = minExposure * context_.configuration.sensor.lineDuration; context_.configuration.sensor.maxShutterSpeed = maxExposure * context_.configuration.sensor.lineDuration; But that means they're backwards - the minimum (i.e. slowest) shutter speed should produce the _longest_ exposure time and vice versa...we could reverse the min/max prefixes instead but that's even more confusing. So I think this definitely should be updated to minExposureTime and maxExposureTime. >>> + context.configuration.sensor.minAnalogueGain, >>> + context.configuration.sensor.maxAnalogueGain); >>> + } >>> + >>> return 0; >>> } >>> >>> @@ -234,7 +280,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext, >>> double yGain, double iqMeanGain) >>> { >>> IPASessionConfiguration &configuration = context.configuration; >>> - IPAActiveState &activeState = context.activeState; >>> >>> /* Get the effective exposure and gain applied on the sensor. */ >>> uint32_t exposure = frameContext.sensor.exposure; >>> @@ -300,10 +345,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext, >>> LOG(RkISP1Agc, Debug) << "Divided up shutter and gain are " >>> << shutterTime << " and " >>> << stepGain; >>> - >>> - /* Update the estimated exposure and gain. */ >>> - activeState.agc.automatic.exposure = shutterTime / configuration.sensor.lineDuration; >>> - activeState.agc.automatic.gain = stepGain; >>> } >>> >>> /** >>> @@ -383,6 +424,19 @@ void Agc::parseStatistics(const rkisp1_stat_buffer *stats, >>> context.hw->numHistogramBins)); >>> } >>> >>> +double Agc::estimateLuminance(double gain) >>> +{ >>> + double ySum = 0.0; >>> + >>> + /* Sum the averages, saturated to 255. */ >>> + for (uint8_t expMean : expMeans_) >>> + ySum += std::min(expMean * gain, 255.0); >>> + >>> + /* \todo Weight with the AWB gains */ > How so ? The comment is copied from the existing implementation, but it's something that the IPU3 and RPi implementations do as well. I assume that without the colour correction the luminance estimation from RGB values is corrupted. The RkISP1 doesn't use RGB values here, but internally it estimates Y here using (R+G+B) x 0.332, and possibly those need correcting. I think that's only necessary depending on the position in the pipeline that the statistics are drawn from though; possibly they're post auto-white-balance anyway in which case it ought not matter, but I'm not sure where the tap points are for RkISP1. > >>> + >>> + return ySum / expMeans_.size() / 255; >>> +} >>> + >>> /** >>> * \brief Process RkISP1 statistics, and run AGC operations >>> * \param[in] context The shared IPA context >>> @@ -448,6 +502,33 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, >>> computeExposure(context, frameContext, yGain, iqMeanGain); >>> frameCount_++; >>> >>> + parseStatistics(stats, context); >> Regarding my previous mail. Here expMean_ would be assigned... >> >>> + >>> + /* >>> + * 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(RkISP1Agc, 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.automatic.exposure = shutterTime / context.configuration.sensor.lineDuration; >>> + activeState.agc.automatic.gain = aGain; >>> + >>> fillMetadata(context, frameContext, metadata); >> ...and here it would be reset. >> >>> } >>> >>> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h >>> index b0de4898..1271741e 100644 >>> --- a/src/ipa/rkisp1/algorithms/agc.h >>> +++ b/src/ipa/rkisp1/algorithms/agc.h >>> @@ -14,6 +14,7 @@ >>> >>> #include <libcamera/geometry.h> >>> >>> +#include "libipa/agc.h" >>> #include "libipa/histogram.h" >>> >>> #include "algorithm.h" >>> @@ -22,12 +23,13 @@ namespace libcamera { >>> >>> namespace ipa::rkisp1::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 IPACameraSensorInfo &configInfo) override; >>> void queueRequest(IPAContext &context, >>> const uint32_t frame, >>> @@ -51,6 +53,7 @@ private: >>> ControlList &metadata); >>> void parseStatistics(const rkisp1_stat_buffer *stats, >>> IPAContext &context); >>> + double estimateLuminance(double gain) override; >>> >>> uint64_t frameCount_; >>> >>> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h >>> index 10d8f38c..256b75eb 100644 >>> --- a/src/ipa/rkisp1/ipa_context.h >>> +++ b/src/ipa/rkisp1/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> >>> @@ -67,6 +68,8 @@ struct IPAActiveState { >>> } automatic; >>> >>> bool autoEnabled; >>> + uint32_t constraintMode; >>> + uint32_t exposureMode; >>> } agc; >>> >>> struct { >>> @@ -151,6 +154,8 @@ struct IPAContext { >>> IPAActiveState activeState; >>> >>> FCQueue<IPAFrameContext> frameContexts; >>> + >>> + ControlInfoMap::Map ctrlMap; >>> }; >>> >>> } /* namespace ipa::rkisp1 */ >>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp >>> index 9dc5f53c..5f74762f 100644 >>> --- a/src/ipa/rkisp1/rkisp1.cpp >>> +++ b/src/ipa/rkisp1/rkisp1.cpp >>> @@ -119,7 +119,7 @@ const ControlInfoMap::Map rkisp1Controls{ >>> } /* namespace */ >>> >>> IPARkISP1::IPARkISP1() >>> - : context_({ {}, {}, {}, { kMaxFrameContexts } }) >>> + : context_({ {}, {}, {}, { kMaxFrameContexts }, {} }) >>> { >>> } >>>
On Fri, Mar 22, 2024 at 01:14:50PM +0000, Daniel Scally wrote: > Now that we have a MeanLuminanceAgc class that centralises our AEGC > algorithm, derive the RkISP1'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/rkisp1/algorithms/agc.cpp | 91 +++++++++++++++++++++++++++++-- > src/ipa/rkisp1/algorithms/agc.h | 5 +- > src/ipa/rkisp1/ipa_context.h | 5 ++ > src/ipa/rkisp1/rkisp1.cpp | 2 +- > 4 files changed, 96 insertions(+), 7 deletions(-) > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > index d380194d..3389c471 100644 > --- a/src/ipa/rkisp1/algorithms/agc.cpp > +++ b/src/ipa/rkisp1/algorithms/agc.cpp > @@ -64,6 +64,36 @@ Agc::Agc() > supportsRaw_ = true; > } > > +/** > + * \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; > + > + parseRelativeLuminanceTarget(tuningData); > + > + ret = parseConstraintModes(tuningData); > + if (ret) > + return ret; > + > + ret = parseExposureModes(tuningData); > + if (ret) > + return ret; > + > + context.ctrlMap.merge(controls()); Ah, is this how you gather the available controls + values from the algorithms so that the IPA can report then? That might arguably be better than what I did instead on top. Paul > + > + return 0; > +} > + > /** > * \brief Configure the AGC given a configInfo > * \param[in] context The shared IPA context > @@ -81,6 +111,13 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) > context.activeState.agc.manual.exposure = context.activeState.agc.automatic.exposure; > context.activeState.agc.autoEnabled = !context.configuration.raw; > > + /* > + * \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; > + > /* > * Define the measurement window for AGC as a centered rectangle > * covering 3/4 of the image width and height. > @@ -95,6 +132,15 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) > * frame index > */ > frameCount_ = 0; > + > + for (auto &[id, helper] : exposureModeHelpers()) { > + /* \todo Run this again when FrameDurationLimits is passed in */ > + helper->configure(context.configuration.sensor.minShutterSpeed, > + context.configuration.sensor.maxShutterSpeed, > + context.configuration.sensor.minAnalogueGain, > + context.configuration.sensor.maxAnalogueGain); > + } > + > return 0; > } > > @@ -234,7 +280,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext, > double yGain, double iqMeanGain) > { > IPASessionConfiguration &configuration = context.configuration; > - IPAActiveState &activeState = context.activeState; > > /* Get the effective exposure and gain applied on the sensor. */ > uint32_t exposure = frameContext.sensor.exposure; > @@ -300,10 +345,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext, > LOG(RkISP1Agc, Debug) << "Divided up shutter and gain are " > << shutterTime << " and " > << stepGain; > - > - /* Update the estimated exposure and gain. */ > - activeState.agc.automatic.exposure = shutterTime / configuration.sensor.lineDuration; > - activeState.agc.automatic.gain = stepGain; > } > > /** > @@ -383,6 +424,19 @@ void Agc::parseStatistics(const rkisp1_stat_buffer *stats, > context.hw->numHistogramBins)); > } > > +double Agc::estimateLuminance(double gain) > +{ > + double ySum = 0.0; > + > + /* Sum the averages, saturated to 255. */ > + for (uint8_t expMean : expMeans_) > + ySum += std::min(expMean * gain, 255.0); > + > + /* \todo Weight with the AWB gains */ > + > + return ySum / expMeans_.size() / 255; > +} > + > /** > * \brief Process RkISP1 statistics, and run AGC operations > * \param[in] context The shared IPA context > @@ -448,6 +502,33 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > computeExposure(context, frameContext, yGain, iqMeanGain); > frameCount_++; > > + parseStatistics(stats, context); > + > + /* > + * 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(RkISP1Agc, 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.automatic.exposure = shutterTime / context.configuration.sensor.lineDuration; > + activeState.agc.automatic.gain = aGain; > + > fillMetadata(context, frameContext, metadata); > } > > diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h > index b0de4898..1271741e 100644 > --- a/src/ipa/rkisp1/algorithms/agc.h > +++ b/src/ipa/rkisp1/algorithms/agc.h > @@ -14,6 +14,7 @@ > > #include <libcamera/geometry.h> > > +#include "libipa/agc.h" > #include "libipa/histogram.h" > > #include "algorithm.h" > @@ -22,12 +23,13 @@ namespace libcamera { > > namespace ipa::rkisp1::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 IPACameraSensorInfo &configInfo) override; > void queueRequest(IPAContext &context, > const uint32_t frame, > @@ -51,6 +53,7 @@ private: > ControlList &metadata); > void parseStatistics(const rkisp1_stat_buffer *stats, > IPAContext &context); > + double estimateLuminance(double gain) override; > > uint64_t frameCount_; > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h > index 10d8f38c..256b75eb 100644 > --- a/src/ipa/rkisp1/ipa_context.h > +++ b/src/ipa/rkisp1/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> > @@ -67,6 +68,8 @@ struct IPAActiveState { > } automatic; > > bool autoEnabled; > + uint32_t constraintMode; > + uint32_t exposureMode; > } agc; > > struct { > @@ -151,6 +154,8 @@ struct IPAContext { > IPAActiveState activeState; > > FCQueue<IPAFrameContext> frameContexts; > + > + ControlInfoMap::Map ctrlMap; > }; > > } /* namespace ipa::rkisp1 */ > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > index 9dc5f53c..5f74762f 100644 > --- a/src/ipa/rkisp1/rkisp1.cpp > +++ b/src/ipa/rkisp1/rkisp1.cpp > @@ -119,7 +119,7 @@ const ControlInfoMap::Map rkisp1Controls{ > } /* namespace */ > > IPARkISP1::IPARkISP1() > - : context_({ {}, {}, {}, { kMaxFrameContexts } }) > + : context_({ {}, {}, {}, { kMaxFrameContexts }, {} }) > { > } > > -- > 2.34.1 >
Hi Paul On 09/04/2024 11:13, Paul Elder wrote: > On Fri, Mar 22, 2024 at 01:14:50PM +0000, Daniel Scally wrote: >> Now that we have a MeanLuminanceAgc class that centralises our AEGC >> algorithm, derive the RkISP1'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/rkisp1/algorithms/agc.cpp | 91 +++++++++++++++++++++++++++++-- >> src/ipa/rkisp1/algorithms/agc.h | 5 +- >> src/ipa/rkisp1/ipa_context.h | 5 ++ >> src/ipa/rkisp1/rkisp1.cpp | 2 +- >> 4 files changed, 96 insertions(+), 7 deletions(-) >> >> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp >> index d380194d..3389c471 100644 >> --- a/src/ipa/rkisp1/algorithms/agc.cpp >> +++ b/src/ipa/rkisp1/algorithms/agc.cpp >> @@ -64,6 +64,36 @@ Agc::Agc() >> supportsRaw_ = true; >> } >> >> +/** >> + * \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; >> + >> + parseRelativeLuminanceTarget(tuningData); >> + >> + ret = parseConstraintModes(tuningData); >> + if (ret) >> + return ret; >> + >> + ret = parseExposureModes(tuningData); >> + if (ret) >> + return ret; >> + >> + context.ctrlMap.merge(controls()); > Ah, is this how you gather the available controls + values from the > algorithms so that the IPA can report then? That might arguably be > better than what I did instead on top. Yes...although not actually implemented here (I'll do it properly in the v2) I just added a ctrlMap.merge(context_.ctrlMap) to the IPA's ::init() to include these in the ControlInfoMap that it constructs. > > > Paul > >> + >> + return 0; >> +} >> + >> /** >> * \brief Configure the AGC given a configInfo >> * \param[in] context The shared IPA context >> @@ -81,6 +111,13 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) >> context.activeState.agc.manual.exposure = context.activeState.agc.automatic.exposure; >> context.activeState.agc.autoEnabled = !context.configuration.raw; >> >> + /* >> + * \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; >> + >> /* >> * Define the measurement window for AGC as a centered rectangle >> * covering 3/4 of the image width and height. >> @@ -95,6 +132,15 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) >> * frame index >> */ >> frameCount_ = 0; >> + >> + for (auto &[id, helper] : exposureModeHelpers()) { >> + /* \todo Run this again when FrameDurationLimits is passed in */ >> + helper->configure(context.configuration.sensor.minShutterSpeed, >> + context.configuration.sensor.maxShutterSpeed, >> + context.configuration.sensor.minAnalogueGain, >> + context.configuration.sensor.maxAnalogueGain); >> + } >> + >> return 0; >> } >> >> @@ -234,7 +280,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext, >> double yGain, double iqMeanGain) >> { >> IPASessionConfiguration &configuration = context.configuration; >> - IPAActiveState &activeState = context.activeState; >> >> /* Get the effective exposure and gain applied on the sensor. */ >> uint32_t exposure = frameContext.sensor.exposure; >> @@ -300,10 +345,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext, >> LOG(RkISP1Agc, Debug) << "Divided up shutter and gain are " >> << shutterTime << " and " >> << stepGain; >> - >> - /* Update the estimated exposure and gain. */ >> - activeState.agc.automatic.exposure = shutterTime / configuration.sensor.lineDuration; >> - activeState.agc.automatic.gain = stepGain; >> } >> >> /** >> @@ -383,6 +424,19 @@ void Agc::parseStatistics(const rkisp1_stat_buffer *stats, >> context.hw->numHistogramBins)); >> } >> >> +double Agc::estimateLuminance(double gain) >> +{ >> + double ySum = 0.0; >> + >> + /* Sum the averages, saturated to 255. */ >> + for (uint8_t expMean : expMeans_) >> + ySum += std::min(expMean * gain, 255.0); >> + >> + /* \todo Weight with the AWB gains */ >> + >> + return ySum / expMeans_.size() / 255; >> +} >> + >> /** >> * \brief Process RkISP1 statistics, and run AGC operations >> * \param[in] context The shared IPA context >> @@ -448,6 +502,33 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, >> computeExposure(context, frameContext, yGain, iqMeanGain); >> frameCount_++; >> >> + parseStatistics(stats, context); >> + >> + /* >> + * 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(RkISP1Agc, 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.automatic.exposure = shutterTime / context.configuration.sensor.lineDuration; >> + activeState.agc.automatic.gain = aGain; >> + >> fillMetadata(context, frameContext, metadata); >> } >> >> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h >> index b0de4898..1271741e 100644 >> --- a/src/ipa/rkisp1/algorithms/agc.h >> +++ b/src/ipa/rkisp1/algorithms/agc.h >> @@ -14,6 +14,7 @@ >> >> #include <libcamera/geometry.h> >> >> +#include "libipa/agc.h" >> #include "libipa/histogram.h" >> >> #include "algorithm.h" >> @@ -22,12 +23,13 @@ namespace libcamera { >> >> namespace ipa::rkisp1::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 IPACameraSensorInfo &configInfo) override; >> void queueRequest(IPAContext &context, >> const uint32_t frame, >> @@ -51,6 +53,7 @@ private: >> ControlList &metadata); >> void parseStatistics(const rkisp1_stat_buffer *stats, >> IPAContext &context); >> + double estimateLuminance(double gain) override; >> >> uint64_t frameCount_; >> >> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h >> index 10d8f38c..256b75eb 100644 >> --- a/src/ipa/rkisp1/ipa_context.h >> +++ b/src/ipa/rkisp1/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> >> @@ -67,6 +68,8 @@ struct IPAActiveState { >> } automatic; >> >> bool autoEnabled; >> + uint32_t constraintMode; >> + uint32_t exposureMode; >> } agc; >> >> struct { >> @@ -151,6 +154,8 @@ struct IPAContext { >> IPAActiveState activeState; >> >> FCQueue<IPAFrameContext> frameContexts; >> + >> + ControlInfoMap::Map ctrlMap; >> }; >> >> } /* namespace ipa::rkisp1 */ >> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp >> index 9dc5f53c..5f74762f 100644 >> --- a/src/ipa/rkisp1/rkisp1.cpp >> +++ b/src/ipa/rkisp1/rkisp1.cpp >> @@ -119,7 +119,7 @@ const ControlInfoMap::Map rkisp1Controls{ >> } /* namespace */ >> >> IPARkISP1::IPARkISP1() >> - : context_({ {}, {}, {}, { kMaxFrameContexts } }) >> + : context_({ {}, {}, {}, { kMaxFrameContexts }, {} }) >> { >> } >> >> -- >> 2.34.1 >>
diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp index d380194d..3389c471 100644 --- a/src/ipa/rkisp1/algorithms/agc.cpp +++ b/src/ipa/rkisp1/algorithms/agc.cpp @@ -64,6 +64,36 @@ Agc::Agc() supportsRaw_ = true; } +/** + * \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; + + parseRelativeLuminanceTarget(tuningData); + + ret = parseConstraintModes(tuningData); + if (ret) + return ret; + + ret = parseExposureModes(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 @@ -81,6 +111,13 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) context.activeState.agc.manual.exposure = context.activeState.agc.automatic.exposure; context.activeState.agc.autoEnabled = !context.configuration.raw; + /* + * \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; + /* * Define the measurement window for AGC as a centered rectangle * covering 3/4 of the image width and height. @@ -95,6 +132,15 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) * frame index */ frameCount_ = 0; + + for (auto &[id, helper] : exposureModeHelpers()) { + /* \todo Run this again when FrameDurationLimits is passed in */ + helper->configure(context.configuration.sensor.minShutterSpeed, + context.configuration.sensor.maxShutterSpeed, + context.configuration.sensor.minAnalogueGain, + context.configuration.sensor.maxAnalogueGain); + } + return 0; } @@ -234,7 +280,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext, double yGain, double iqMeanGain) { IPASessionConfiguration &configuration = context.configuration; - IPAActiveState &activeState = context.activeState; /* Get the effective exposure and gain applied on the sensor. */ uint32_t exposure = frameContext.sensor.exposure; @@ -300,10 +345,6 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext, LOG(RkISP1Agc, Debug) << "Divided up shutter and gain are " << shutterTime << " and " << stepGain; - - /* Update the estimated exposure and gain. */ - activeState.agc.automatic.exposure = shutterTime / configuration.sensor.lineDuration; - activeState.agc.automatic.gain = stepGain; } /** @@ -383,6 +424,19 @@ void Agc::parseStatistics(const rkisp1_stat_buffer *stats, context.hw->numHistogramBins)); } +double Agc::estimateLuminance(double gain) +{ + double ySum = 0.0; + + /* Sum the averages, saturated to 255. */ + for (uint8_t expMean : expMeans_) + ySum += std::min(expMean * gain, 255.0); + + /* \todo Weight with the AWB gains */ + + return ySum / expMeans_.size() / 255; +} + /** * \brief Process RkISP1 statistics, and run AGC operations * \param[in] context The shared IPA context @@ -448,6 +502,33 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, computeExposure(context, frameContext, yGain, iqMeanGain); frameCount_++; + parseStatistics(stats, context); + + /* + * 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(RkISP1Agc, 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.automatic.exposure = shutterTime / context.configuration.sensor.lineDuration; + activeState.agc.automatic.gain = aGain; + fillMetadata(context, frameContext, metadata); } diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h index b0de4898..1271741e 100644 --- a/src/ipa/rkisp1/algorithms/agc.h +++ b/src/ipa/rkisp1/algorithms/agc.h @@ -14,6 +14,7 @@ #include <libcamera/geometry.h> +#include "libipa/agc.h" #include "libipa/histogram.h" #include "algorithm.h" @@ -22,12 +23,13 @@ namespace libcamera { namespace ipa::rkisp1::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 IPACameraSensorInfo &configInfo) override; void queueRequest(IPAContext &context, const uint32_t frame, @@ -51,6 +53,7 @@ private: ControlList &metadata); void parseStatistics(const rkisp1_stat_buffer *stats, IPAContext &context); + double estimateLuminance(double gain) override; uint64_t frameCount_; diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index 10d8f38c..256b75eb 100644 --- a/src/ipa/rkisp1/ipa_context.h +++ b/src/ipa/rkisp1/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> @@ -67,6 +68,8 @@ struct IPAActiveState { } automatic; bool autoEnabled; + uint32_t constraintMode; + uint32_t exposureMode; } agc; struct { @@ -151,6 +154,8 @@ struct IPAContext { IPAActiveState activeState; FCQueue<IPAFrameContext> frameContexts; + + ControlInfoMap::Map ctrlMap; }; } /* namespace ipa::rkisp1 */ diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 9dc5f53c..5f74762f 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -119,7 +119,7 @@ const ControlInfoMap::Map rkisp1Controls{ } /* namespace */ IPARkISP1::IPARkISP1() - : context_({ {}, {}, {}, { kMaxFrameContexts } }) + : context_({ {}, {}, {}, { kMaxFrameContexts }, {} }) { }
Now that we have a MeanLuminanceAgc class that centralises our AEGC algorithm, derive the RkISP1'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/rkisp1/algorithms/agc.cpp | 91 +++++++++++++++++++++++++++++-- src/ipa/rkisp1/algorithms/agc.h | 5 +- src/ipa/rkisp1/ipa_context.h | 5 ++ src/ipa/rkisp1/rkisp1.cpp | 2 +- 4 files changed, 96 insertions(+), 7 deletions(-)