| Message ID | 20260723154327.1357866-15-barnabas.pocze@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Barnabás On Thu, Jul 23, 2026 at 05:42:57PM +0200, Barnabás Pőcze wrote: > Add a new type that contains all parameters for the `calculateNewEv()` > function. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/agc.cpp | 28 +++++++++-------- > src/ipa/libipa/agc_mean_luminance.cpp | 44 ++++++++++++++++----------- > src/ipa/libipa/agc_mean_luminance.h | 12 ++++++-- > src/ipa/mali-c55/algorithms/agc.cpp | 12 +++++--- > src/ipa/rkisp1/algorithms/agc.cpp | 18 ++++++----- > 5 files changed, 68 insertions(+), 46 deletions(-) > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > index a919418d69..ac5b6e7513 100644 > --- a/src/ipa/ipu3/algorithms/agc.cpp > +++ b/src/ipa/ipu3/algorithms/agc.cpp > @@ -236,22 +236,24 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > * frameContext.sensor.exposure; > double analogueGain = frameContext.sensor.gain; > utils::Duration effectiveExposureValue = exposureTime * analogueGain; > - AgcTraits agcTraits{ > - rgbTriples_, > - {{ > - context.activeState.awb.gains.red, > - context.activeState.awb.gains.blue, > - context.activeState.awb.gains.green, > - }}, > - bdsGrid_, > - }; > > utils::Duration newExposureTime; > double aGain, qGain, dGain; > - std::tie(newExposureTime, aGain, qGain, dGain) = > - agc_.calculateNewEv(context.activeState.agc.constraintMode, > - context.activeState.agc.exposureMode, hist, > - effectiveExposureValue, agcTraits); > + std::tie(newExposureTime, aGain, qGain, dGain) = agc_.calculateNewEv({ > + .traits = AgcTraits{ > + rgbTriples_, > + {{ > + context.activeState.awb.gains.red, > + context.activeState.awb.gains.blue, > + context.activeState.awb.gains.green, > + }}, > + bdsGrid_, > + }, > + .yHist = hist, > + .effectiveExposureValue = effectiveExposureValue, > + .constraintModeIndex = context.activeState.agc.constraintMode, > + .exposureModeIndex = context.activeState.agc.exposureMode, > + }); > > LOG(IPU3Agc, Debug) > << "Divided up exposure time, analogue gain and digital gain are " > diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp > index 31636159a2..083d67eb70 100644 > --- a/src/ipa/libipa/agc_mean_luminance.cpp > +++ b/src/ipa/libipa/agc_mean_luminance.cpp > @@ -648,16 +648,30 @@ utils::Duration AgcMeanLuminance::filterExposure(utils::Duration exposureValue) > return filteredExposure_; > } > > +/** > + * \struct AgcMeanLuminance::Params > + * \brief Collection of parameters for the mean luminance AGC algorithm > + * > + * \var AgcMeanLuminance::Params::traits > + * \brief The traits object implementing the necessary functions > + * > + * \var AgcMeanLuminance::Params::yHist > + * \brief A histogram from the ISP statistics to use in constraining the calculated gain The histogram is passed by AgcMeanLuminance::calculateNewEv() to AgcMeanLuminance::constraintClampGain() which documents it as * \param[in] hist A histogram over which to calculate inter-quantile means So I guess the histogram is used for calculating means and not for constraining the result ? > + * > + * \var AgcMeanLuminance::Params::effectiveExposureValue > + * \brief The EV applied to the frame from which the statistics in use derive Or maybe \brief The ExposureValue used in the frame on which statistics have been generated on > + * > + * \var AgcMeanLuminance::Params::constraintModeIndex > + * \brief The index of the constraint mode to use > + * > + * \var AgcMeanLuminance::Params::exposureModeIndex > + * \brief The index of the exposure mode to use > + */ > + > /** > * \brief Calculate the new exposure value and split it between exposure time > * and gain > - * \param[in] constraintModeIndex The index of the current constraint mode > - * \param[in] exposureModeIndex The index of the current exposure mode > - * \param[in] yHist A Histogram from the ISP statistics to use in constraining > - * the calculated gain > - * \param[in] effectiveExposureValue The EV applied to the frame from which the > - * statistics in use derive > - * \param[in] traits The traits object implementing the necessary functions > + * \param[in] params The set of parameters for the calculation for the exposure value calculation ? > * > * Calculate a new exposure value to try to obtain the target. The calculated > * exposure value is filtered to prevent rapid changes from frame to frame, and > @@ -667,20 +681,16 @@ utils::Duration AgcMeanLuminance::filterExposure(utils::Duration exposureValue) > * gain > */ > std::tuple<utils::Duration, double, double, double> > -AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex, > - uint32_t exposureModeIndex, > - const Histogram &yHist, > - utils::Duration effectiveExposureValue, > - const Traits &traits) > +AgcMeanLuminance::calculateNewEv(const Params ¶ms) > { > /* > * The pipeline handler should validate that we have received an allowed > * value for AeExposureMode. > */ > ExposureModeHelper &exposureModeHelper = > - exposureModeHelpers_.at(exposureModeIndex); > + exposureModeHelpers_.at(params.exposureModeIndex); > > - if (effectiveExposureValue == 0s) { > + if (params.effectiveExposureValue == 0s) { > LOG(AgcMeanLuminance, Error) > << "Effective exposure value is 0. This is a bug in AGC " > "and must be fixed for proper operation."; > @@ -692,8 +702,8 @@ AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex, > return exposureModeHelper.splitExposure(10ms); > } > > - double gain = estimateInitialGain(traits); > - gain = constraintClampGain(constraintModeIndex, yHist, gain); > + double gain = estimateInitialGain(params.traits); > + gain = constraintClampGain(params.constraintModeIndex, params.yHist, gain); > > /* > * We don't check whether we're already close to the target, because > @@ -702,7 +712,7 @@ AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex, > * pass through the splitExposure() function. > */ > > - utils::Duration newExposureValue = effectiveExposureValue * gain; > + utils::Duration newExposureValue = params.effectiveExposureValue * gain; > > /* > * We filter the exposure value to make sure changes are not too jarring > diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h > index d5425cd20b..eb9d6fec2b 100644 > --- a/src/ipa/libipa/agc_mean_luminance.h > +++ b/src/ipa/libipa/agc_mean_luminance.h > @@ -79,10 +79,16 @@ public: > return controls_; > } > > + struct Params { > + const Traits &traits; > + const Histogram &yHist; > + utils::Duration effectiveExposureValue; > + uint32_t constraintModeIndex; > + uint32_t exposureModeIndex; > + }; > + > std::tuple<utils::Duration, double, double, double> > - calculateNewEv(uint32_t constraintModeIndex, uint32_t exposureModeIndex, > - const Histogram &yHist, utils::Duration effectiveExposureValue, > - const Traits &traits); > + calculateNewEv(const Params ¶ms); > > double effectiveYTarget() const; > > diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp > index 29b7c66159..fb97b87c6e 100644 > --- a/src/ipa/mali-c55/algorithms/agc.cpp > +++ b/src/ipa/mali-c55/algorithms/agc.cpp > @@ -331,14 +331,16 @@ void Agc::process(IPAContext &context, > double analogueGain = frameContext.agc.sensorGain; > utils::Duration currentShutter = exposure * configuration.sensor.lineDuration; > utils::Duration effectiveExposureValue = currentShutter * analogueGain; > - AgcTraits agcTraits(statistics_); > > utils::Duration shutterTime; > double aGain, qGain, dGain; > - std::tie(shutterTime, aGain, qGain, dGain) = > - agc_.calculateNewEv(activeState.agc.constraintMode, > - activeState.agc.exposureMode, statistics_.yHist, > - effectiveExposureValue, agcTraits); > + std::tie(shutterTime, aGain, qGain, dGain) = agc_.calculateNewEv({ > + .traits = AgcTraits(statistics_), > + .yHist = statistics_.yHist, > + .effectiveExposureValue = effectiveExposureValue, > + .constraintModeIndex = activeState.agc.constraintMode, > + .exposureModeIndex = activeState.agc.exposureMode, > + }); > > LOG(MaliC55Agc, Debug) > << "Divided up shutter, analogue gain and digital gain are " > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > index 129f5be2a5..8b4bcbd71b 100644 > --- a/src/ipa/rkisp1/algorithms/agc.cpp > +++ b/src/ipa/rkisp1/algorithms/agc.cpp > @@ -716,20 +716,22 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > /* The lower 4 bits are fractional and meant to be discarded. */ > Histogram hist({ params->hist.hist_bins, context.hw.numHistogramBins }, > [](uint32_t x) { return x >> 4; }); > - AgcTraits agcTraits{ > - { params->ae.exp_mean, context.hw.numAeCells }, > - meteringModes_.at(frameContext.agc.meteringMode), > - }; > > agc_.setExposureCompensation(pow(2.0, frameContext.agc.exposureValue)); > agc_.setLux(frameContext.lux.lux); > > utils::Duration newExposureTime; > double aGain, qGain, dGain; > - std::tie(newExposureTime, aGain, qGain, dGain) = > - agc_.calculateNewEv(frameContext.agc.constraintMode, > - frameContext.agc.exposureMode, > - hist, effectiveExposureValue, agcTraits); > + std::tie(newExposureTime, aGain, qGain, dGain) = agc_.calculateNewEv({ > + .traits = AgcTraits{ > + { params->ae.exp_mean, context.hw.numAeCells }, > + meteringModes_.at(frameContext.agc.meteringMode), > + }, > + .yHist = hist, > + .effectiveExposureValue = effectiveExposureValue, > + .constraintModeIndex = frameContext.agc.constraintMode, > + .exposureModeIndex = frameContext.agc.exposureMode, > + }); > > LOG(RkISP1Agc, Debug) > << "Divided up exposure time, analogue gain, quantization gain" Minors apart Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thanks j > -- > 2.55.0 >
2026. 07. 24. 14:23 keltezéssel, Jacopo Mondi írta: > Hi Barnabás > > On Thu, Jul 23, 2026 at 05:42:57PM +0200, Barnabás Pőcze wrote: >> Add a new type that contains all parameters for the `calculateNewEv()` >> function. >> >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> --- >> src/ipa/ipu3/algorithms/agc.cpp | 28 +++++++++-------- >> src/ipa/libipa/agc_mean_luminance.cpp | 44 ++++++++++++++++----------- >> src/ipa/libipa/agc_mean_luminance.h | 12 ++++++-- >> src/ipa/mali-c55/algorithms/agc.cpp | 12 +++++--- >> src/ipa/rkisp1/algorithms/agc.cpp | 18 ++++++----- >> 5 files changed, 68 insertions(+), 46 deletions(-) >> >> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp >> index a919418d69..ac5b6e7513 100644 >> --- a/src/ipa/ipu3/algorithms/agc.cpp >> +++ b/src/ipa/ipu3/algorithms/agc.cpp >> @@ -236,22 +236,24 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, >> * frameContext.sensor.exposure; >> double analogueGain = frameContext.sensor.gain; >> utils::Duration effectiveExposureValue = exposureTime * analogueGain; >> - AgcTraits agcTraits{ >> - rgbTriples_, >> - {{ >> - context.activeState.awb.gains.red, >> - context.activeState.awb.gains.blue, >> - context.activeState.awb.gains.green, >> - }}, >> - bdsGrid_, >> - }; >> >> utils::Duration newExposureTime; >> double aGain, qGain, dGain; >> - std::tie(newExposureTime, aGain, qGain, dGain) = >> - agc_.calculateNewEv(context.activeState.agc.constraintMode, >> - context.activeState.agc.exposureMode, hist, >> - effectiveExposureValue, agcTraits); >> + std::tie(newExposureTime, aGain, qGain, dGain) = agc_.calculateNewEv({ >> + .traits = AgcTraits{ >> + rgbTriples_, >> + {{ >> + context.activeState.awb.gains.red, >> + context.activeState.awb.gains.blue, >> + context.activeState.awb.gains.green, >> + }}, >> + bdsGrid_, >> + }, >> + .yHist = hist, >> + .effectiveExposureValue = effectiveExposureValue, >> + .constraintModeIndex = context.activeState.agc.constraintMode, >> + .exposureModeIndex = context.activeState.agc.exposureMode, >> + }); >> >> LOG(IPU3Agc, Debug) >> << "Divided up exposure time, analogue gain and digital gain are " >> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp >> index 31636159a2..083d67eb70 100644 >> --- a/src/ipa/libipa/agc_mean_luminance.cpp >> +++ b/src/ipa/libipa/agc_mean_luminance.cpp >> @@ -648,16 +648,30 @@ utils::Duration AgcMeanLuminance::filterExposure(utils::Duration exposureValue) >> return filteredExposure_; >> } >> >> +/** >> + * \struct AgcMeanLuminance::Params >> + * \brief Collection of parameters for the mean luminance AGC algorithm >> + * >> + * \var AgcMeanLuminance::Params::traits >> + * \brief The traits object implementing the necessary functions >> + * >> + * \var AgcMeanLuminance::Params::yHist >> + * \brief A histogram from the ISP statistics to use in constraining the calculated gain > > The histogram is passed by AgcMeanLuminance::calculateNewEv() to > AgcMeanLuminance::constraintClampGain() which documents it as > > * \param[in] hist A histogram over which to calculate inter-quantile means > > So I guess the histogram is used for calculating means and not for > constraining the result ? One could argue that it's both, the calculated means are used to constrain the gain. But in any case, I copied the documentation verbatim from `calculateNewEv()`, so I'm a bit wary about changing them in an intended-to-be-non-functional change. Maybe in a separate patch if the documentation updates are desirable. > > >> + * >> + * \var AgcMeanLuminance::Params::effectiveExposureValue >> + * \brief The EV applied to the frame from which the statistics in use derive > > Or maybe > \brief The ExposureValue used in the frame on which statistics have > been generated on Same here. > >> + * >> + * \var AgcMeanLuminance::Params::constraintModeIndex >> + * \brief The index of the constraint mode to use >> + * >> + * \var AgcMeanLuminance::Params::exposureModeIndex >> + * \brief The index of the exposure mode to use >> + */ >> + >> /** >> * \brief Calculate the new exposure value and split it between exposure time >> * and gain >> - * \param[in] constraintModeIndex The index of the current constraint mode >> - * \param[in] exposureModeIndex The index of the current exposure mode >> - * \param[in] yHist A Histogram from the ISP statistics to use in constraining >> - * the calculated gain >> - * \param[in] effectiveExposureValue The EV applied to the frame from which the >> - * statistics in use derive >> - * \param[in] traits The traits object implementing the necessary functions >> + * \param[in] params The set of parameters for the calculation > > for the exposure value calculation > ? Done. > >> * >> * Calculate a new exposure value to try to obtain the target. The calculated >> * exposure value is filtered to prevent rapid changes from frame to frame, and >> @@ -667,20 +681,16 @@ utils::Duration AgcMeanLuminance::filterExposure(utils::Duration exposureValue) >> * gain >> */ >> std::tuple<utils::Duration, double, double, double> >> -AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex, >> - uint32_t exposureModeIndex, >> - const Histogram &yHist, >> - utils::Duration effectiveExposureValue, >> - const Traits &traits) >> +AgcMeanLuminance::calculateNewEv(const Params ¶ms) >> { >> /* >> * The pipeline handler should validate that we have received an allowed >> * value for AeExposureMode. >> */ >> ExposureModeHelper &exposureModeHelper = >> - exposureModeHelpers_.at(exposureModeIndex); >> + exposureModeHelpers_.at(params.exposureModeIndex); >> >> - if (effectiveExposureValue == 0s) { >> + if (params.effectiveExposureValue == 0s) { >> LOG(AgcMeanLuminance, Error) >> << "Effective exposure value is 0. This is a bug in AGC " >> "and must be fixed for proper operation."; >> @@ -692,8 +702,8 @@ AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex, >> return exposureModeHelper.splitExposure(10ms); >> } >> >> - double gain = estimateInitialGain(traits); >> - gain = constraintClampGain(constraintModeIndex, yHist, gain); >> + double gain = estimateInitialGain(params.traits); >> + gain = constraintClampGain(params.constraintModeIndex, params.yHist, gain); >> >> /* >> * We don't check whether we're already close to the target, because >> @@ -702,7 +712,7 @@ AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex, >> * pass through the splitExposure() function. >> */ >> >> - utils::Duration newExposureValue = effectiveExposureValue * gain; >> + utils::Duration newExposureValue = params.effectiveExposureValue * gain; >> >> /* >> * We filter the exposure value to make sure changes are not too jarring >> diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h >> index d5425cd20b..eb9d6fec2b 100644 >> --- a/src/ipa/libipa/agc_mean_luminance.h >> +++ b/src/ipa/libipa/agc_mean_luminance.h >> @@ -79,10 +79,16 @@ public: >> return controls_; >> } >> >> + struct Params { >> + const Traits &traits; >> + const Histogram &yHist; >> + utils::Duration effectiveExposureValue; >> + uint32_t constraintModeIndex; >> + uint32_t exposureModeIndex; >> + }; >> + >> std::tuple<utils::Duration, double, double, double> >> - calculateNewEv(uint32_t constraintModeIndex, uint32_t exposureModeIndex, >> - const Histogram &yHist, utils::Duration effectiveExposureValue, >> - const Traits &traits); >> + calculateNewEv(const Params ¶ms); >> >> double effectiveYTarget() const; >> >> diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp >> index 29b7c66159..fb97b87c6e 100644 >> --- a/src/ipa/mali-c55/algorithms/agc.cpp >> +++ b/src/ipa/mali-c55/algorithms/agc.cpp >> @@ -331,14 +331,16 @@ void Agc::process(IPAContext &context, >> double analogueGain = frameContext.agc.sensorGain; >> utils::Duration currentShutter = exposure * configuration.sensor.lineDuration; >> utils::Duration effectiveExposureValue = currentShutter * analogueGain; >> - AgcTraits agcTraits(statistics_); >> >> utils::Duration shutterTime; >> double aGain, qGain, dGain; >> - std::tie(shutterTime, aGain, qGain, dGain) = >> - agc_.calculateNewEv(activeState.agc.constraintMode, >> - activeState.agc.exposureMode, statistics_.yHist, >> - effectiveExposureValue, agcTraits); >> + std::tie(shutterTime, aGain, qGain, dGain) = agc_.calculateNewEv({ >> + .traits = AgcTraits(statistics_), >> + .yHist = statistics_.yHist, >> + .effectiveExposureValue = effectiveExposureValue, >> + .constraintModeIndex = activeState.agc.constraintMode, >> + .exposureModeIndex = activeState.agc.exposureMode, >> + }); >> >> LOG(MaliC55Agc, Debug) >> << "Divided up shutter, analogue gain and digital gain are " >> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp >> index 129f5be2a5..8b4bcbd71b 100644 >> --- a/src/ipa/rkisp1/algorithms/agc.cpp >> +++ b/src/ipa/rkisp1/algorithms/agc.cpp >> @@ -716,20 +716,22 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, >> /* The lower 4 bits are fractional and meant to be discarded. */ >> Histogram hist({ params->hist.hist_bins, context.hw.numHistogramBins }, >> [](uint32_t x) { return x >> 4; }); >> - AgcTraits agcTraits{ >> - { params->ae.exp_mean, context.hw.numAeCells }, >> - meteringModes_.at(frameContext.agc.meteringMode), >> - }; >> >> agc_.setExposureCompensation(pow(2.0, frameContext.agc.exposureValue)); >> agc_.setLux(frameContext.lux.lux); >> >> utils::Duration newExposureTime; >> double aGain, qGain, dGain; >> - std::tie(newExposureTime, aGain, qGain, dGain) = >> - agc_.calculateNewEv(frameContext.agc.constraintMode, >> - frameContext.agc.exposureMode, >> - hist, effectiveExposureValue, agcTraits); >> + std::tie(newExposureTime, aGain, qGain, dGain) = agc_.calculateNewEv({ >> + .traits = AgcTraits{ >> + { params->ae.exp_mean, context.hw.numAeCells }, >> + meteringModes_.at(frameContext.agc.meteringMode), >> + }, >> + .yHist = hist, >> + .effectiveExposureValue = effectiveExposureValue, >> + .constraintModeIndex = frameContext.agc.constraintMode, >> + .exposureModeIndex = frameContext.agc.exposureMode, >> + }); >> >> LOG(RkISP1Agc, Debug) >> << "Divided up exposure time, analogue gain, quantization gain" > > Minors apart > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > Thanks > j > >> -- >> 2.55.0 >>
diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp index a919418d69..ac5b6e7513 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -236,22 +236,24 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, * frameContext.sensor.exposure; double analogueGain = frameContext.sensor.gain; utils::Duration effectiveExposureValue = exposureTime * analogueGain; - AgcTraits agcTraits{ - rgbTriples_, - {{ - context.activeState.awb.gains.red, - context.activeState.awb.gains.blue, - context.activeState.awb.gains.green, - }}, - bdsGrid_, - }; utils::Duration newExposureTime; double aGain, qGain, dGain; - std::tie(newExposureTime, aGain, qGain, dGain) = - agc_.calculateNewEv(context.activeState.agc.constraintMode, - context.activeState.agc.exposureMode, hist, - effectiveExposureValue, agcTraits); + std::tie(newExposureTime, aGain, qGain, dGain) = agc_.calculateNewEv({ + .traits = AgcTraits{ + rgbTriples_, + {{ + context.activeState.awb.gains.red, + context.activeState.awb.gains.blue, + context.activeState.awb.gains.green, + }}, + bdsGrid_, + }, + .yHist = hist, + .effectiveExposureValue = effectiveExposureValue, + .constraintModeIndex = context.activeState.agc.constraintMode, + .exposureModeIndex = context.activeState.agc.exposureMode, + }); LOG(IPU3Agc, Debug) << "Divided up exposure time, analogue gain and digital gain are " diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp index 31636159a2..083d67eb70 100644 --- a/src/ipa/libipa/agc_mean_luminance.cpp +++ b/src/ipa/libipa/agc_mean_luminance.cpp @@ -648,16 +648,30 @@ utils::Duration AgcMeanLuminance::filterExposure(utils::Duration exposureValue) return filteredExposure_; } +/** + * \struct AgcMeanLuminance::Params + * \brief Collection of parameters for the mean luminance AGC algorithm + * + * \var AgcMeanLuminance::Params::traits + * \brief The traits object implementing the necessary functions + * + * \var AgcMeanLuminance::Params::yHist + * \brief A histogram from the ISP statistics to use in constraining the calculated gain + * + * \var AgcMeanLuminance::Params::effectiveExposureValue + * \brief The EV applied to the frame from which the statistics in use derive + * + * \var AgcMeanLuminance::Params::constraintModeIndex + * \brief The index of the constraint mode to use + * + * \var AgcMeanLuminance::Params::exposureModeIndex + * \brief The index of the exposure mode to use + */ + /** * \brief Calculate the new exposure value and split it between exposure time * and gain - * \param[in] constraintModeIndex The index of the current constraint mode - * \param[in] exposureModeIndex The index of the current exposure mode - * \param[in] yHist A Histogram from the ISP statistics to use in constraining - * the calculated gain - * \param[in] effectiveExposureValue The EV applied to the frame from which the - * statistics in use derive - * \param[in] traits The traits object implementing the necessary functions + * \param[in] params The set of parameters for the calculation * * Calculate a new exposure value to try to obtain the target. The calculated * exposure value is filtered to prevent rapid changes from frame to frame, and @@ -667,20 +681,16 @@ utils::Duration AgcMeanLuminance::filterExposure(utils::Duration exposureValue) * gain */ std::tuple<utils::Duration, double, double, double> -AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex, - uint32_t exposureModeIndex, - const Histogram &yHist, - utils::Duration effectiveExposureValue, - const Traits &traits) +AgcMeanLuminance::calculateNewEv(const Params ¶ms) { /* * The pipeline handler should validate that we have received an allowed * value for AeExposureMode. */ ExposureModeHelper &exposureModeHelper = - exposureModeHelpers_.at(exposureModeIndex); + exposureModeHelpers_.at(params.exposureModeIndex); - if (effectiveExposureValue == 0s) { + if (params.effectiveExposureValue == 0s) { LOG(AgcMeanLuminance, Error) << "Effective exposure value is 0. This is a bug in AGC " "and must be fixed for proper operation."; @@ -692,8 +702,8 @@ AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex, return exposureModeHelper.splitExposure(10ms); } - double gain = estimateInitialGain(traits); - gain = constraintClampGain(constraintModeIndex, yHist, gain); + double gain = estimateInitialGain(params.traits); + gain = constraintClampGain(params.constraintModeIndex, params.yHist, gain); /* * We don't check whether we're already close to the target, because @@ -702,7 +712,7 @@ AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex, * pass through the splitExposure() function. */ - utils::Duration newExposureValue = effectiveExposureValue * gain; + utils::Duration newExposureValue = params.effectiveExposureValue * gain; /* * We filter the exposure value to make sure changes are not too jarring diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h index d5425cd20b..eb9d6fec2b 100644 --- a/src/ipa/libipa/agc_mean_luminance.h +++ b/src/ipa/libipa/agc_mean_luminance.h @@ -79,10 +79,16 @@ public: return controls_; } + struct Params { + const Traits &traits; + const Histogram &yHist; + utils::Duration effectiveExposureValue; + uint32_t constraintModeIndex; + uint32_t exposureModeIndex; + }; + std::tuple<utils::Duration, double, double, double> - calculateNewEv(uint32_t constraintModeIndex, uint32_t exposureModeIndex, - const Histogram &yHist, utils::Duration effectiveExposureValue, - const Traits &traits); + calculateNewEv(const Params ¶ms); double effectiveYTarget() const; diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp index 29b7c66159..fb97b87c6e 100644 --- a/src/ipa/mali-c55/algorithms/agc.cpp +++ b/src/ipa/mali-c55/algorithms/agc.cpp @@ -331,14 +331,16 @@ void Agc::process(IPAContext &context, double analogueGain = frameContext.agc.sensorGain; utils::Duration currentShutter = exposure * configuration.sensor.lineDuration; utils::Duration effectiveExposureValue = currentShutter * analogueGain; - AgcTraits agcTraits(statistics_); utils::Duration shutterTime; double aGain, qGain, dGain; - std::tie(shutterTime, aGain, qGain, dGain) = - agc_.calculateNewEv(activeState.agc.constraintMode, - activeState.agc.exposureMode, statistics_.yHist, - effectiveExposureValue, agcTraits); + std::tie(shutterTime, aGain, qGain, dGain) = agc_.calculateNewEv({ + .traits = AgcTraits(statistics_), + .yHist = statistics_.yHist, + .effectiveExposureValue = effectiveExposureValue, + .constraintModeIndex = activeState.agc.constraintMode, + .exposureModeIndex = activeState.agc.exposureMode, + }); LOG(MaliC55Agc, Debug) << "Divided up shutter, analogue gain and digital gain are " diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp index 129f5be2a5..8b4bcbd71b 100644 --- a/src/ipa/rkisp1/algorithms/agc.cpp +++ b/src/ipa/rkisp1/algorithms/agc.cpp @@ -716,20 +716,22 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, /* The lower 4 bits are fractional and meant to be discarded. */ Histogram hist({ params->hist.hist_bins, context.hw.numHistogramBins }, [](uint32_t x) { return x >> 4; }); - AgcTraits agcTraits{ - { params->ae.exp_mean, context.hw.numAeCells }, - meteringModes_.at(frameContext.agc.meteringMode), - }; agc_.setExposureCompensation(pow(2.0, frameContext.agc.exposureValue)); agc_.setLux(frameContext.lux.lux); utils::Duration newExposureTime; double aGain, qGain, dGain; - std::tie(newExposureTime, aGain, qGain, dGain) = - agc_.calculateNewEv(frameContext.agc.constraintMode, - frameContext.agc.exposureMode, - hist, effectiveExposureValue, agcTraits); + std::tie(newExposureTime, aGain, qGain, dGain) = agc_.calculateNewEv({ + .traits = AgcTraits{ + { params->ae.exp_mean, context.hw.numAeCells }, + meteringModes_.at(frameContext.agc.meteringMode), + }, + .yHist = hist, + .effectiveExposureValue = effectiveExposureValue, + .constraintModeIndex = frameContext.agc.constraintMode, + .exposureModeIndex = frameContext.agc.exposureMode, + }); LOG(RkISP1Agc, Debug) << "Divided up exposure time, analogue gain, quantization gain"
Add a new type that contains all parameters for the `calculateNewEv()` function. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- src/ipa/ipu3/algorithms/agc.cpp | 28 +++++++++-------- src/ipa/libipa/agc_mean_luminance.cpp | 44 ++++++++++++++++----------- src/ipa/libipa/agc_mean_luminance.h | 12 ++++++-- src/ipa/mali-c55/algorithms/agc.cpp | 12 +++++--- src/ipa/rkisp1/algorithms/agc.cpp | 18 ++++++----- 5 files changed, 68 insertions(+), 46 deletions(-)