Message ID | 20250815102945.1602071-5-stefan.klug@ideasonboard.com |
---|---|
State | New |
Headers | show |
Series |
|
Related | show |
Quoting Stefan Klug (2025-08-15 19:29:24) > In ExposureModeHelper::splitExposure() the quantization of exposure time > and gain is not taken into account. This can lead to visible flicker > when the quantization steps are too big. As a preparation to fixing > that, add a function to set the sensor line length and the current > sensor mode helper and extend the clampXXX functions to return the > quantization error. > > By default the exposure time quantization is assumed to be 1us and gain > is assumed to not be quantized at all. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> > > --- > > Changes in v3: > - Collected tags > - Renamed lineLength to lineDuration > - Remove the "no functional changes" sentence, as the returned > exposureTime/gain are now quantized, which happened outside of this > class before. So that is actually a functional change. > --- > src/ipa/libipa/exposure_mode_helper.cpp | 49 ++++++++++++++++++++----- > src/ipa/libipa/exposure_mode_helper.h | 10 ++++- > 2 files changed, 48 insertions(+), 11 deletions(-) > > diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp > index 0c1e99e31a47..b18c30a6291c 100644 > --- a/src/ipa/libipa/exposure_mode_helper.cpp > +++ b/src/ipa/libipa/exposure_mode_helper.cpp > @@ -70,18 +70,37 @@ namespace ipa { > * the runtime limits set through setLimits() instead. > */ > ExposureModeHelper::ExposureModeHelper(const Span<std::pair<utils::Duration, double>> stages) > + : lineDuration_(1us), minExposureTime_(0us), maxExposureTime_(0us), > + minGain_(0), maxGain_(0), sensorHelper_(nullptr) > { > - minExposureTime_ = 0us; > - maxExposureTime_ = 0us; > - minGain_ = 0; > - maxGain_ = 0; > - > for (const auto &[s, g] : stages) { > exposureTimes_.push_back(s); > gains_.push_back(g); > } > } > > +/** > + * \brief Configure sensor details > + * \param[in] lineDuration The current line length of the sensor > + * \param[in] sensorHelper The sensor helper > + * > + * This function sets the line length and sensor helper. These are used in > + * splitExposure() to take the quantization of the exposure and gain into > + * account. > + * > + * When this is not called, is is assumed that exposure is in micro second s/this is not/this has not been/ s/is is/it is/ > + * granularity and gain has no quantization at all. > + * > + * The caller has to ensure that sensorHelper is valid for the lifetime of > + * ExposureModeHelper or the next call to configure. "or the next call to configure" is a bit confusing. Do you mean that the sensorHelper must be valid until the next configure() call or configure() must be called again if ExposureModeHelper dies? Paul > + */ > +void ExposureModeHelper::configure(utils::Duration lineDuration, > + const CameraSensorHelper *sensorHelper) > +{ > + lineDuration_ = lineDuration; > + sensorHelper_ = sensorHelper; > +} > + > /** > * \brief Set the exposure time and gain limits > * \param[in] minExposureTime The minimum exposure time supported > @@ -108,14 +127,26 @@ void ExposureModeHelper::setLimits(utils::Duration minExposureTime, > maxGain_ = maxGain; > } > > -utils::Duration ExposureModeHelper::clampExposureTime(utils::Duration exposureTime) const > +utils::Duration ExposureModeHelper::clampExposureTime(utils::Duration exposureTime, > + double *quantizationGain) const > { > - return std::clamp(exposureTime, minExposureTime_, maxExposureTime_); > + utils::Duration clamped; > + utils::Duration exp; > + > + clamped = std::clamp(exposureTime, minExposureTime_, maxExposureTime_); > + exp = static_cast<long>(clamped / lineDuration_) * lineDuration_; > + if (quantizationGain) > + *quantizationGain = clamped / exp; > + > + return exp; > } > > -double ExposureModeHelper::clampGain(double gain) const > +double ExposureModeHelper::clampGain(double gain, double *quantizationGain) const > { > - return std::clamp(gain, minGain_, maxGain_); > + double clamped = std::clamp(gain, minGain_, maxGain_); > + if (!sensorHelper_) > + return clamped; > + return sensorHelper_->quantizeGain(clamped, quantizationGain); > } > > /** > diff --git a/src/ipa/libipa/exposure_mode_helper.h b/src/ipa/libipa/exposure_mode_helper.h > index c5be1b6703a8..ac7e8da95c6c 100644 > --- a/src/ipa/libipa/exposure_mode_helper.h > +++ b/src/ipa/libipa/exposure_mode_helper.h > @@ -14,6 +14,8 @@ > #include <libcamera/base/span.h> > #include <libcamera/base/utils.h> > > +#include "camera_sensor_helper.h" > + > namespace libcamera { > > namespace ipa { > @@ -24,6 +26,7 @@ public: > ExposureModeHelper(const Span<std::pair<utils::Duration, double>> stages); > ~ExposureModeHelper() = default; > > + void configure(utils::Duration lineLength, const CameraSensorHelper *sensorHelper); > void setLimits(utils::Duration minExposureTime, utils::Duration maxExposureTime, > double minGain, double maxGain); > > @@ -36,16 +39,19 @@ public: > double maxGain() const { return maxGain_; } > > private: > - utils::Duration clampExposureTime(utils::Duration exposureTime) const; > - double clampGain(double gain) const; > + utils::Duration clampExposureTime(utils::Duration exposureTime, > + double *quantizationGain = nullptr) const; > + double clampGain(double gain, double *quantizationGain = nullptr) const; > > std::vector<utils::Duration> exposureTimes_; > std::vector<double> gains_; > > + utils::Duration lineDuration_; > utils::Duration minExposureTime_; > utils::Duration maxExposureTime_; > double minGain_; > double maxGain_; > + const CameraSensorHelper *sensorHelper_; > }; > > } /* namespace ipa */ > -- > 2.48.1 >
diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp index 0c1e99e31a47..b18c30a6291c 100644 --- a/src/ipa/libipa/exposure_mode_helper.cpp +++ b/src/ipa/libipa/exposure_mode_helper.cpp @@ -70,18 +70,37 @@ namespace ipa { * the runtime limits set through setLimits() instead. */ ExposureModeHelper::ExposureModeHelper(const Span<std::pair<utils::Duration, double>> stages) + : lineDuration_(1us), minExposureTime_(0us), maxExposureTime_(0us), + minGain_(0), maxGain_(0), sensorHelper_(nullptr) { - minExposureTime_ = 0us; - maxExposureTime_ = 0us; - minGain_ = 0; - maxGain_ = 0; - for (const auto &[s, g] : stages) { exposureTimes_.push_back(s); gains_.push_back(g); } } +/** + * \brief Configure sensor details + * \param[in] lineDuration The current line length of the sensor + * \param[in] sensorHelper The sensor helper + * + * This function sets the line length and sensor helper. These are used in + * splitExposure() to take the quantization of the exposure and gain into + * account. + * + * When this is not called, is is assumed that exposure is in micro second + * granularity and gain has no quantization at all. + * + * The caller has to ensure that sensorHelper is valid for the lifetime of + * ExposureModeHelper or the next call to configure. + */ +void ExposureModeHelper::configure(utils::Duration lineDuration, + const CameraSensorHelper *sensorHelper) +{ + lineDuration_ = lineDuration; + sensorHelper_ = sensorHelper; +} + /** * \brief Set the exposure time and gain limits * \param[in] minExposureTime The minimum exposure time supported @@ -108,14 +127,26 @@ void ExposureModeHelper::setLimits(utils::Duration minExposureTime, maxGain_ = maxGain; } -utils::Duration ExposureModeHelper::clampExposureTime(utils::Duration exposureTime) const +utils::Duration ExposureModeHelper::clampExposureTime(utils::Duration exposureTime, + double *quantizationGain) const { - return std::clamp(exposureTime, minExposureTime_, maxExposureTime_); + utils::Duration clamped; + utils::Duration exp; + + clamped = std::clamp(exposureTime, minExposureTime_, maxExposureTime_); + exp = static_cast<long>(clamped / lineDuration_) * lineDuration_; + if (quantizationGain) + *quantizationGain = clamped / exp; + + return exp; } -double ExposureModeHelper::clampGain(double gain) const +double ExposureModeHelper::clampGain(double gain, double *quantizationGain) const { - return std::clamp(gain, minGain_, maxGain_); + double clamped = std::clamp(gain, minGain_, maxGain_); + if (!sensorHelper_) + return clamped; + return sensorHelper_->quantizeGain(clamped, quantizationGain); } /** diff --git a/src/ipa/libipa/exposure_mode_helper.h b/src/ipa/libipa/exposure_mode_helper.h index c5be1b6703a8..ac7e8da95c6c 100644 --- a/src/ipa/libipa/exposure_mode_helper.h +++ b/src/ipa/libipa/exposure_mode_helper.h @@ -14,6 +14,8 @@ #include <libcamera/base/span.h> #include <libcamera/base/utils.h> +#include "camera_sensor_helper.h" + namespace libcamera { namespace ipa { @@ -24,6 +26,7 @@ public: ExposureModeHelper(const Span<std::pair<utils::Duration, double>> stages); ~ExposureModeHelper() = default; + void configure(utils::Duration lineLength, const CameraSensorHelper *sensorHelper); void setLimits(utils::Duration minExposureTime, utils::Duration maxExposureTime, double minGain, double maxGain); @@ -36,16 +39,19 @@ public: double maxGain() const { return maxGain_; } private: - utils::Duration clampExposureTime(utils::Duration exposureTime) const; - double clampGain(double gain) const; + utils::Duration clampExposureTime(utils::Duration exposureTime, + double *quantizationGain = nullptr) const; + double clampGain(double gain, double *quantizationGain = nullptr) const; std::vector<utils::Duration> exposureTimes_; std::vector<double> gains_; + utils::Duration lineDuration_; utils::Duration minExposureTime_; utils::Duration maxExposureTime_; double minGain_; double maxGain_; + const CameraSensorHelper *sensorHelper_; }; } /* namespace ipa */