| Message ID | 20250331144352.736700-9-stefan.klug@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi all,
I made a mistake in last-minute-variable-sorting and forgot to compile
:-/. Will be added in v2.
This patch needs a fixup:
diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
index a498b646bdd5..f6cb7144b31a 100644
--- a/src/ipa/libipa/agc_mean_luminance.cpp
+++ b/src/ipa/libipa/agc_mean_luminance.cpp
@@ -143,7 +143,7 @@ static constexpr double kMaxRelativeLuminanceTarget = 0.95;
*/
AgcMeanLuminance::AgcMeanLuminance()
- : frameCount_(0), filteredExposure_(0s), relativeLuminanceTarget_(0), exposureCompensation_(1.0)
+ : exposureCompensation_(1.0), frameCount_(0), filteredExposure_(0s), relativeLuminanceTarget_(0)
{
}
Best regards,
Stefan
On Mon, Mar 31, 2025 at 04:43:47PM +0200, Stefan Klug wrote:
> Exposure compensation allows to over- or under-expose an
> image by a given value (typically provided in EV). Add support
> for that in agc_mean_luminance.
>
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
> src/ipa/libipa/agc_mean_luminance.cpp | 23 +++++++++++++++++++++--
> src/ipa/libipa/agc_mean_luminance.h | 6 ++++++
> 2 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
> index 9154f083a510..a498b646bdd5 100644
> --- a/src/ipa/libipa/agc_mean_luminance.cpp
> +++ b/src/ipa/libipa/agc_mean_luminance.cpp
> @@ -44,6 +44,15 @@ static constexpr uint32_t kNumStartupFrames = 10;
> */
> static constexpr double kDefaultRelativeLuminanceTarget = 0.16;
>
> +/*
> + * Maximum relative luminance target
> + *
> + * This value limits the relative luminance target after applying the exposure
> + * compensation. Targeting a value above this limit results in saturation
> + * and the inability to regulate properly.
> + */
> +static constexpr double kMaxRelativeLuminanceTarget = 0.95;
> +
> /**
> * \struct AgcMeanLuminance::AgcConstraint
> * \brief The boundaries and target for an AeConstraintMode constraint
> @@ -134,7 +143,7 @@ static constexpr double kDefaultRelativeLuminanceTarget = 0.16;
> */
>
> AgcMeanLuminance::AgcMeanLuminance()
> - : frameCount_(0), filteredExposure_(0s), relativeLuminanceTarget_(0)
> + : frameCount_(0), filteredExposure_(0s), relativeLuminanceTarget_(0), exposureCompensation_(1.0)
> {
> }
>
> @@ -369,6 +378,15 @@ int AgcMeanLuminance::parseTuningData(const YamlObject &tuningData)
> return parseExposureModes(tuningData);
> }
>
> +/**
> + * \fn AgcMeanLuminance::setExposureCompensation()
> + * \brief Set the exposure compensation value
> + * \param[in] gain The exposure compensation gain
> + *
> + * This function sets the exposure compensation value to be used in the
> + * AGC calculations. It is expressed as gain instead of EV.
> + */
> +
> /**
> * \brief Set the ExposureModeHelper limits for this class
> * \param[in] minExposureTime Minimum exposure time to allow
> @@ -425,7 +443,8 @@ void AgcMeanLuminance::setLimits(utils::Duration minExposureTime,
> */
> double AgcMeanLuminance::estimateInitialGain() const
> {
> - double yTarget = relativeLuminanceTarget_;
> + double yTarget = std::min(relativeLuminanceTarget_ * exposureCompensation_,
> + kMaxRelativeLuminanceTarget);
> double yGain = 1.0;
>
> /*
> diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h
> index c41391cb0b73..cad7ef845487 100644
> --- a/src/ipa/libipa/agc_mean_luminance.h
> +++ b/src/ipa/libipa/agc_mean_luminance.h
> @@ -44,6 +44,11 @@ public:
>
> int parseTuningData(const YamlObject &tuningData);
>
> + void setExposureCompensation(double gain)
> + {
> + exposureCompensation_ = gain;
> + }
> +
> void setLimits(utils::Duration minExposureTime, utils::Duration maxExposureTime,
> double minGain, double maxGain);
>
> @@ -84,6 +89,7 @@ private:
> double gain);
> utils::Duration filterExposure(utils::Duration exposureValue);
>
> + double exposureCompensation_;
> uint64_t frameCount_;
> utils::Duration filteredExposure_;
> double relativeLuminanceTarget_;
> --
> 2.43.0
>
Hi Stefan On 31/03/2025 15:43, Stefan Klug wrote: > Exposure compensation allows to over- or under-expose an > image by a given value (typically provided in EV). Add support Given you're implementing it as gain instead, I would either drop "typically provided in EV" or explain that you're not doing so. > for that in agc_mean_luminance. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > --- > src/ipa/libipa/agc_mean_luminance.cpp | 23 +++++++++++++++++++++-- > src/ipa/libipa/agc_mean_luminance.h | 6 ++++++ > 2 files changed, 27 insertions(+), 2 deletions(-) > > diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp > index 9154f083a510..a498b646bdd5 100644 > --- a/src/ipa/libipa/agc_mean_luminance.cpp > +++ b/src/ipa/libipa/agc_mean_luminance.cpp > @@ -44,6 +44,15 @@ static constexpr uint32_t kNumStartupFrames = 10; > */ > static constexpr double kDefaultRelativeLuminanceTarget = 0.16; > > +/* > + * Maximum relative luminance target > + * > + * This value limits the relative luminance target after applying the exposure > + * compensation. Targeting a value above this limit results in saturation > + * and the inability to regulate properly. > + */ > +static constexpr double kMaxRelativeLuminanceTarget = 0.95; > + > /** > * \struct AgcMeanLuminance::AgcConstraint > * \brief The boundaries and target for an AeConstraintMode constraint > @@ -134,7 +143,7 @@ static constexpr double kDefaultRelativeLuminanceTarget = 0.16; > */ > > AgcMeanLuminance::AgcMeanLuminance() > - : frameCount_(0), filteredExposure_(0s), relativeLuminanceTarget_(0) > + : frameCount_(0), filteredExposure_(0s), relativeLuminanceTarget_(0), exposureCompensation_(1.0) > { > } > > @@ -369,6 +378,15 @@ int AgcMeanLuminance::parseTuningData(const YamlObject &tuningData) > return parseExposureModes(tuningData); > } > > +/** > + * \fn AgcMeanLuminance::setExposureCompensation() > + * \brief Set the exposure compensation value > + * \param[in] gain The exposure compensation gain > + * > + * This function sets the exposure compensation value to be used in the > + * AGC calculations. It is expressed as gain instead of EV. > + */ > + > /** > * \brief Set the ExposureModeHelper limits for this class > * \param[in] minExposureTime Minimum exposure time to allow > @@ -425,7 +443,8 @@ void AgcMeanLuminance::setLimits(utils::Duration minExposureTime, > */ > double AgcMeanLuminance::estimateInitialGain() const > { > - double yTarget = relativeLuminanceTarget_; > + double yTarget = std::min(relativeLuminanceTarget_ * exposureCompensation_, > + kMaxRelativeLuminanceTarget); The capping is really a separate change to the one described in the commit message...I would personally split them into separate commits, but failing that it ought to be described in the commit message. Either way (and if you split, keep the tag for both): Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> > double yGain = 1.0; > > /* > diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h > index c41391cb0b73..cad7ef845487 100644 > --- a/src/ipa/libipa/agc_mean_luminance.h > +++ b/src/ipa/libipa/agc_mean_luminance.h > @@ -44,6 +44,11 @@ public: > > int parseTuningData(const YamlObject &tuningData); > > + void setExposureCompensation(double gain) > + { > + exposureCompensation_ = gain; > + } > + > void setLimits(utils::Duration minExposureTime, utils::Duration maxExposureTime, > double minGain, double maxGain); > > @@ -84,6 +89,7 @@ private: > double gain); > utils::Duration filterExposure(utils::Duration exposureValue); > > + double exposureCompensation_; > uint64_t frameCount_; > utils::Duration filteredExposure_; > double relativeLuminanceTarget_;
diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp index 9154f083a510..a498b646bdd5 100644 --- a/src/ipa/libipa/agc_mean_luminance.cpp +++ b/src/ipa/libipa/agc_mean_luminance.cpp @@ -44,6 +44,15 @@ static constexpr uint32_t kNumStartupFrames = 10; */ static constexpr double kDefaultRelativeLuminanceTarget = 0.16; +/* + * Maximum relative luminance target + * + * This value limits the relative luminance target after applying the exposure + * compensation. Targeting a value above this limit results in saturation + * and the inability to regulate properly. + */ +static constexpr double kMaxRelativeLuminanceTarget = 0.95; + /** * \struct AgcMeanLuminance::AgcConstraint * \brief The boundaries and target for an AeConstraintMode constraint @@ -134,7 +143,7 @@ static constexpr double kDefaultRelativeLuminanceTarget = 0.16; */ AgcMeanLuminance::AgcMeanLuminance() - : frameCount_(0), filteredExposure_(0s), relativeLuminanceTarget_(0) + : frameCount_(0), filteredExposure_(0s), relativeLuminanceTarget_(0), exposureCompensation_(1.0) { } @@ -369,6 +378,15 @@ int AgcMeanLuminance::parseTuningData(const YamlObject &tuningData) return parseExposureModes(tuningData); } +/** + * \fn AgcMeanLuminance::setExposureCompensation() + * \brief Set the exposure compensation value + * \param[in] gain The exposure compensation gain + * + * This function sets the exposure compensation value to be used in the + * AGC calculations. It is expressed as gain instead of EV. + */ + /** * \brief Set the ExposureModeHelper limits for this class * \param[in] minExposureTime Minimum exposure time to allow @@ -425,7 +443,8 @@ void AgcMeanLuminance::setLimits(utils::Duration minExposureTime, */ double AgcMeanLuminance::estimateInitialGain() const { - double yTarget = relativeLuminanceTarget_; + double yTarget = std::min(relativeLuminanceTarget_ * exposureCompensation_, + kMaxRelativeLuminanceTarget); double yGain = 1.0; /* diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h index c41391cb0b73..cad7ef845487 100644 --- a/src/ipa/libipa/agc_mean_luminance.h +++ b/src/ipa/libipa/agc_mean_luminance.h @@ -44,6 +44,11 @@ public: int parseTuningData(const YamlObject &tuningData); + void setExposureCompensation(double gain) + { + exposureCompensation_ = gain; + } + void setLimits(utils::Duration minExposureTime, utils::Duration maxExposureTime, double minGain, double maxGain); @@ -84,6 +89,7 @@ private: double gain); utils::Duration filterExposure(utils::Duration exposureValue); + double exposureCompensation_; uint64_t frameCount_; utils::Duration filteredExposure_; double relativeLuminanceTarget_;
Exposure compensation allows to over- or under-expose an image by a given value (typically provided in EV). Add support for that in agc_mean_luminance. Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- src/ipa/libipa/agc_mean_luminance.cpp | 23 +++++++++++++++++++++-- src/ipa/libipa/agc_mean_luminance.h | 6 ++++++ 2 files changed, 27 insertions(+), 2 deletions(-)