| Message ID | 20260723154327.1357866-16-barnabas.pocze@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Barnabás On Thu, Jul 23, 2026 at 05:42:58PM +0200, Barnabás Pőcze wrote: > There is no reason not to have this member function as `const`. Change the > map lookup to use `at()` just like it is done for `calculateNewEv()`, with > the same requirement that validation is the job of the caller. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/ipa/libipa/agc_mean_luminance.cpp | 4 ++-- > src/ipa/libipa/agc_mean_luminance.h | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp > index 083d67eb70..e8679c66be 100644 > --- a/src/ipa/libipa/agc_mean_luminance.cpp > +++ b/src/ipa/libipa/agc_mean_luminance.cpp > @@ -539,7 +539,7 @@ double AgcMeanLuminance::estimateInitialGain(const Traits &traits) const > */ > double AgcMeanLuminance::constraintClampGain(uint32_t constraintModeIndex, > const Histogram &hist, > - double gain) > + double gain) const > { > auto applyConstraint = [this, &gain, &hist](const AgcConstraint &constraint) { > double lux = lux_; > @@ -569,7 +569,7 @@ double AgcMeanLuminance::constraintClampGain(uint32_t constraintModeIndex, > } > }; > > - std::vector<AgcConstraint> &constraints = constraintModes_[constraintModeIndex]; > + const std::vector<AgcConstraint> &constraints = constraintModes_.at(constraintModeIndex); Is it worth using find() to avoid an exception ? We should be rather sure that constraintModeIndex is in the map, so I'm not sure Either way Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > std::for_each(constraints.begin(), constraints.end(), applyConstraint); > > std::for_each(additionalConstraints_.begin(), additionalConstraints_.end(), applyConstraint); > diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h > index eb9d6fec2b..9b82d97ab6 100644 > --- a/src/ipa/libipa/agc_mean_luminance.h > +++ b/src/ipa/libipa/agc_mean_luminance.h > @@ -105,7 +105,7 @@ private: > double estimateInitialGain(const Traits &traits) const; > double constraintClampGain(uint32_t constraintModeIndex, > const Histogram &hist, > - double gain); > + double gain) const; > utils::Duration filterExposure(utils::Duration exposureValue); > > utils::Duration filteredExposure_; > -- > 2.55.0 >
2026. 07. 24. 14:32 keltezéssel, Jacopo Mondi írta: > Hi Barnabás > > On Thu, Jul 23, 2026 at 05:42:58PM +0200, Barnabás Pőcze wrote: >> There is no reason not to have this member function as `const`. Change the >> map lookup to use `at()` just like it is done for `calculateNewEv()`, with >> the same requirement that validation is the job of the caller. >> >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> --- >> src/ipa/libipa/agc_mean_luminance.cpp | 4 ++-- >> src/ipa/libipa/agc_mean_luminance.h | 2 +- >> 2 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp >> index 083d67eb70..e8679c66be 100644 >> --- a/src/ipa/libipa/agc_mean_luminance.cpp >> +++ b/src/ipa/libipa/agc_mean_luminance.cpp >> @@ -539,7 +539,7 @@ double AgcMeanLuminance::estimateInitialGain(const Traits &traits) const >> */ >> double AgcMeanLuminance::constraintClampGain(uint32_t constraintModeIndex, >> const Histogram &hist, >> - double gain) >> + double gain) const >> { >> auto applyConstraint = [this, &gain, &hist](const AgcConstraint &constraint) { >> double lux = lux_; >> @@ -569,7 +569,7 @@ double AgcMeanLuminance::constraintClampGain(uint32_t constraintModeIndex, >> } >> }; >> >> - std::vector<AgcConstraint> &constraints = constraintModes_[constraintModeIndex]; >> + const std::vector<AgcConstraint> &constraints = constraintModes_.at(constraintModeIndex); > > Is it worth using find() to avoid an exception ? We should be rather > sure that constraintModeIndex is in the map, so I'm not sure The `exposureModeHelpers_` lookup also uses `.at()`, so I decided to do the same here as well. > > Either way > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > >> std::for_each(constraints.begin(), constraints.end(), applyConstraint); >> >> std::for_each(additionalConstraints_.begin(), additionalConstraints_.end(), applyConstraint); >> diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h >> index eb9d6fec2b..9b82d97ab6 100644 >> --- a/src/ipa/libipa/agc_mean_luminance.h >> +++ b/src/ipa/libipa/agc_mean_luminance.h >> @@ -105,7 +105,7 @@ private: >> double estimateInitialGain(const Traits &traits) const; >> double constraintClampGain(uint32_t constraintModeIndex, >> const Histogram &hist, >> - double gain); >> + double gain) const; >> utils::Duration filterExposure(utils::Duration exposureValue); >> >> utils::Duration filteredExposure_; >> -- >> 2.55.0 >>
diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp index 083d67eb70..e8679c66be 100644 --- a/src/ipa/libipa/agc_mean_luminance.cpp +++ b/src/ipa/libipa/agc_mean_luminance.cpp @@ -539,7 +539,7 @@ double AgcMeanLuminance::estimateInitialGain(const Traits &traits) const */ double AgcMeanLuminance::constraintClampGain(uint32_t constraintModeIndex, const Histogram &hist, - double gain) + double gain) const { auto applyConstraint = [this, &gain, &hist](const AgcConstraint &constraint) { double lux = lux_; @@ -569,7 +569,7 @@ double AgcMeanLuminance::constraintClampGain(uint32_t constraintModeIndex, } }; - std::vector<AgcConstraint> &constraints = constraintModes_[constraintModeIndex]; + const std::vector<AgcConstraint> &constraints = constraintModes_.at(constraintModeIndex); std::for_each(constraints.begin(), constraints.end(), applyConstraint); std::for_each(additionalConstraints_.begin(), additionalConstraints_.end(), applyConstraint); diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h index eb9d6fec2b..9b82d97ab6 100644 --- a/src/ipa/libipa/agc_mean_luminance.h +++ b/src/ipa/libipa/agc_mean_luminance.h @@ -105,7 +105,7 @@ private: double estimateInitialGain(const Traits &traits) const; double constraintClampGain(uint32_t constraintModeIndex, const Histogram &hist, - double gain); + double gain) const; utils::Duration filterExposure(utils::Duration exposureValue); utils::Duration filteredExposure_;
There is no reason not to have this member function as `const`. Change the map lookup to use `at()` just like it is done for `calculateNewEv()`, with the same requirement that validation is the job of the caller. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- src/ipa/libipa/agc_mean_luminance.cpp | 4 ++-- src/ipa/libipa/agc_mean_luminance.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)