| Message ID | 20251028-exposure-limits-v2-6-a8b5a318323e@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
On Tue, Oct 28, 2025 at 10:31:52AM +0100, Jacopo Mondi wrote: > The maximum exposure time the AGC algorithm can achieve is, by > definition, limited by the maximum programmed frame duration. > > When initializing the AGC algorithm, use the frame duration and the > sensor's exposure margin to calculate the maximum exposure, unless > the IPA has asked for a fixed exposure by setting min == max. > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > --- > src/ipa/libipa/exposure_mode_helper.cpp | 25 +++++++++++++++++++++++-- > 1 file changed, 23 insertions(+), 2 deletions(-) > > diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp > index edb8f04b245f01119d0d0b0917d10f98c7172dc0..8f40290959e24294ead008c7228e2f67ffc5adf8 100644 > --- a/src/ipa/libipa/exposure_mode_helper.cpp > +++ b/src/ipa/libipa/exposure_mode_helper.cpp > @@ -106,12 +106,33 @@ void ExposureModeHelper::configure(utils::Duration lineDuration, > const CameraSensorHelper *sensorHelper) > { > lineDuration_ = lineDuration; > - minExposureTime_ = minExposureTime; > - maxExposureTime_ = maxExposureTime; > maxFrameDuration_ = maxFrameDuration; > minGain_ = minGain; > maxGain_ = maxGain; > sensorHelper_ = sensorHelper; > + > + minExposureTime_ = minExposureTime; > + > + /* > + * Compute the maximum exposure time. > + * > + * If maxExposureTime is equal to minExposureTime then we use them > + * to fix the exposure time. > + * > + * Otherwise, if the exposure can range between a min and max, use the > + * maxFrameDuration minus the margin as upper limit for exposure > + * (capped to the provided max exposure). > + */ > + auto margin = sensorHelper_->exposureMargin(); > + if (!margin.has_value()) { > + LOG(ExposureModeHelper, Warning) > + << "Exposure margin not known. Default to 4"; > + margin = { 4 }; > + } > + > + maxExposureTime_ = minExposureTime != maxExposureTime > + ? maxFrameDuration - margin.value() * lineDuration > + : minExposureTime; I'd reorganize this to call exposureMargin() only when minExposureTime != maxExposureTime. What happens if maxFrameDuration is smaller than the margin ? Or if subtracting the margin makes max < min ? And even more importantly, shouldn't the maxFrameDuration - margin only *clamp* maxExposureTime, not replace it ? > } > > /**
Hi Laurent On Sun, Nov 02, 2025 at 10:34:16PM +0200, Laurent Pinchart wrote: > On Tue, Oct 28, 2025 at 10:31:52AM +0100, Jacopo Mondi wrote: > > The maximum exposure time the AGC algorithm can achieve is, by > > definition, limited by the maximum programmed frame duration. > > > > When initializing the AGC algorithm, use the frame duration and the > > sensor's exposure margin to calculate the maximum exposure, unless > > the IPA has asked for a fixed exposure by setting min == max. > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > --- > > src/ipa/libipa/exposure_mode_helper.cpp | 25 +++++++++++++++++++++++-- > > 1 file changed, 23 insertions(+), 2 deletions(-) > > > > diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp > > index edb8f04b245f01119d0d0b0917d10f98c7172dc0..8f40290959e24294ead008c7228e2f67ffc5adf8 100644 > > --- a/src/ipa/libipa/exposure_mode_helper.cpp > > +++ b/src/ipa/libipa/exposure_mode_helper.cpp > > @@ -106,12 +106,33 @@ void ExposureModeHelper::configure(utils::Duration lineDuration, > > const CameraSensorHelper *sensorHelper) > > { > > lineDuration_ = lineDuration; > > - minExposureTime_ = minExposureTime; > > - maxExposureTime_ = maxExposureTime; > > maxFrameDuration_ = maxFrameDuration; > > minGain_ = minGain; > > maxGain_ = maxGain; > > sensorHelper_ = sensorHelper; > > + > > + minExposureTime_ = minExposureTime; > > + > > + /* > > + * Compute the maximum exposure time. > > + * > > + * If maxExposureTime is equal to minExposureTime then we use them > > + * to fix the exposure time. > > + * > > + * Otherwise, if the exposure can range between a min and max, use the > > + * maxFrameDuration minus the margin as upper limit for exposure > > + * (capped to the provided max exposure). > > + */ > > + auto margin = sensorHelper_->exposureMargin(); > > + if (!margin.has_value()) { > > + LOG(ExposureModeHelper, Warning) > > + << "Exposure margin not known. Default to 4"; > > + margin = { 4 }; > > + } > > + > > + maxExposureTime_ = minExposureTime != maxExposureTime > > + ? maxFrameDuration - margin.value() * lineDuration > > + : minExposureTime; > > I'd reorganize this to call exposureMargin() only when minExposureTime > != maxExposureTime. Sure. I'll re-work this function anyway, I'll keep this in mind. > > What happens if maxFrameDuration is smaller than the margin ? Or if How can this happen ? The frame duration is at least the visible frame height, isn't it ? And the max frame duration is (height + max vblank). Can this be smaller than the margin ? > subtracting the margin makes max < min ? And even more importantly, are you talking about max and min exposure right ? Again, this seems very unlikely, the min exposure is usually in same order of magnitude of the margin (from 4 to 75 lines in the sensors we currently support). To have maxExposure < minExposure we should have that the min_exposure + margin > (height + min_vblank). This seems quite unlikely. I can however make sure that maxExposure = max(maxExposure, minExposure) > shouldn't the maxFrameDuration - margin only *clamp* maxExposureTime, > not replace it ? mmm, I'm not sure why... The maximum achievable exposure follows the max frame duration, doesn't it ? Applications can't set an exposure range but a single ExposureTime, so it's not like we're overwriting a user setting. Have I missed something ? > > > } > > > > /** > > -- > Regards, > > Laurent Pinchart
diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp index edb8f04b245f01119d0d0b0917d10f98c7172dc0..8f40290959e24294ead008c7228e2f67ffc5adf8 100644 --- a/src/ipa/libipa/exposure_mode_helper.cpp +++ b/src/ipa/libipa/exposure_mode_helper.cpp @@ -106,12 +106,33 @@ void ExposureModeHelper::configure(utils::Duration lineDuration, const CameraSensorHelper *sensorHelper) { lineDuration_ = lineDuration; - minExposureTime_ = minExposureTime; - maxExposureTime_ = maxExposureTime; maxFrameDuration_ = maxFrameDuration; minGain_ = minGain; maxGain_ = maxGain; sensorHelper_ = sensorHelper; + + minExposureTime_ = minExposureTime; + + /* + * Compute the maximum exposure time. + * + * If maxExposureTime is equal to minExposureTime then we use them + * to fix the exposure time. + * + * Otherwise, if the exposure can range between a min and max, use the + * maxFrameDuration minus the margin as upper limit for exposure + * (capped to the provided max exposure). + */ + auto margin = sensorHelper_->exposureMargin(); + if (!margin.has_value()) { + LOG(ExposureModeHelper, Warning) + << "Exposure margin not known. Default to 4"; + margin = { 4 }; + } + + maxExposureTime_ = minExposureTime != maxExposureTime + ? maxFrameDuration - margin.value() * lineDuration + : minExposureTime; } /**
The maximum exposure time the AGC algorithm can achieve is, by definition, limited by the maximum programmed frame duration. When initializing the AGC algorithm, use the frame duration and the sensor's exposure margin to calculate the maximum exposure, unless the IPA has asked for a fixed exposure by setting min == max. Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> --- src/ipa/libipa/exposure_mode_helper.cpp | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-)