| Message ID | 20211013154125.133419-7-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Quoting Jean-Michel Hautbois (2021-10-13 16:41:18) > The exposure limits are set for one sensor until now, in a number of > lines. We should not use those, but exposure limits in a time unit. > > Introduce default limits which with a default minimum of 20ms. This > value should be given by the IPAIPU3. Use cached values expressed as a > number of lines in the configure() call. > > Adapt the process() call accordingly. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/agc.cpp | 30 +++++++++++++++++------------- > src/ipa/ipu3/algorithms/agc.h | 3 ++- > 2 files changed, 19 insertions(+), 14 deletions(-) > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > index 7c2d4201..2dd5c5c1 100644 > --- a/src/ipa/ipu3/algorithms/agc.cpp > +++ b/src/ipa/ipu3/algorithms/agc.cpp > @@ -40,8 +40,8 @@ static constexpr uint32_t kMinGain = kMinISO / 100; > static constexpr uint32_t kMaxGain = kMaxISO / 100; > > /* \todo use calculated value based on sensor */ > -static constexpr uint32_t kMinExposure = 1; > -static constexpr uint32_t kMaxExposure = 1976; > +static constexpr libcamera::utils::Duration kMinShutterSpeed = 100us; > +static constexpr libcamera::utils::Duration kMaxShutterSpeed = 20ms; I guess these limits impose restrictions on long exposure support somehow? But we don't need to worry about that for now. > > /* Histogram constants */ > static constexpr uint32_t knumHistogramBins = 256; > @@ -49,8 +49,8 @@ static constexpr double kEvGainTarget = 0.5; > > Agc::Agc() > : frameCount_(0), lastFrame_(0), iqMean_(0.0), lineDuration_(0s), > - maxExposureTime_(0s), filteredExposure_(0s), filteredExposureNoDg_(0s), > - currentExposure_(0s), currentExposureNoDg_(0s) > + minExposureLines_(0), maxExposureLines_(0), filteredExposure_(0s), > + filteredExposureNoDg_(0s), currentExposure_(0s), currentExposureNoDg_(0s) > { > } > > @@ -60,7 +60,9 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo) > > lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s > / configInfo.sensorInfo.pixelRate; > - maxExposureTime_ = kMaxExposure * lineDuration_; > + > + minExposureLines_ = kMinShutterSpeed / lineDuration_; > + maxExposureLines_ = kMaxShutterSpeed / lineDuration_; > > return 0; > } > @@ -140,28 +142,30 @@ void Agc::lockExposureGain(uint32_t &exposure, double &gain) > double newGain = kEvGainTarget * knumHistogramBins / iqMean_; > > /* extracted from Rpi::Agc::computeTargetExposure */ > - libcamera::utils::Duration currentShutter = exposure * lineDuration_; > + Duration currentShutter = exposure * lineDuration_; > currentExposureNoDg_ = currentShutter * gain; > LOG(IPU3Agc, Debug) << "Actual total exposure " << currentExposureNoDg_ > << " Shutter speed " << currentShutter > << " Gain " << gain; > + > currentExposure_ = currentExposureNoDg_ * newGain; > - libcamera::utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain; > + Duration maxTotalExposure = kMaxShutterSpeed * kMaxGain; > currentExposure_ = std::min(currentExposure_, maxTotalExposure); > - LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_; > + LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_ > + << " maximum is " << maxTotalExposure; > > /* \todo: estimate if we need to desaturate */ > filterExposure(); > > - libcamera::utils::Duration newExposure = 0.0s; > - if (currentShutter < maxExposureTime_) { > - exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / currentExposureNoDg_), kMinExposure, kMaxExposure); > + Duration newExposure = 0.0s; > + if (currentShutter < kMaxShutterSpeed) { > + exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / currentExposureNoDg_), minExposureLines_, maxExposureLines_); > newExposure = currentExposure_ / exposure; > gain = std::clamp(static_cast<uint32_t>(gain * currentExposure_ / newExposure), kMinGain, kMaxGain); > - } else if (currentShutter >= maxExposureTime_) { > + } else if (currentShutter >= kMaxShutterSpeed) { > gain = std::clamp(static_cast<uint32_t>(gain * currentExposure_ / currentExposureNoDg_), kMinGain, kMaxGain); > newExposure = currentExposure_ / gain; > - exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / newExposure), kMinExposure, kMaxExposure); > + exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / newExposure), minExposureLines_, maxExposureLines_); > } > LOG(IPU3Agc, Debug) << "Adjust exposure " << exposure * lineDuration_ << " and gain " << gain; > } > diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h > index cd4d4855..7605ab39 100644 > --- a/src/ipa/ipu3/algorithms/agc.h > +++ b/src/ipa/ipu3/algorithms/agc.h > @@ -44,7 +44,8 @@ private: > double iqMean_; > > Duration lineDuration_; > - Duration maxExposureTime_; > + uint32_t minExposureLines_; > + uint32_t maxExposureLines_; > > Duration filteredExposure_; > Duration filteredExposureNoDg_; > -- > 2.30.2 >
Hi Jean-Michel, (CC'ing David for a question below) Thank you for the patch. On Wed, Oct 13, 2021 at 05:41:18PM +0200, Jean-Michel Hautbois wrote: > The exposure limits are set for one sensor until now, in a number of > lines. We should not use those, but exposure limits in a time unit. > > Introduce default limits which with a default minimum of 20ms. This That's a default maximum, right ? Isn't 20ms a bit low ? > value should be given by the IPAIPU3. Use cached values expressed as a > number of lines in the configure() call. > > Adapt the process() call accordingly. > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/agc.cpp | 30 +++++++++++++++++------------- > src/ipa/ipu3/algorithms/agc.h | 3 ++- > 2 files changed, 19 insertions(+), 14 deletions(-) > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > index 7c2d4201..2dd5c5c1 100644 > --- a/src/ipa/ipu3/algorithms/agc.cpp > +++ b/src/ipa/ipu3/algorithms/agc.cpp > @@ -40,8 +40,8 @@ static constexpr uint32_t kMinGain = kMinISO / 100; > static constexpr uint32_t kMaxGain = kMaxISO / 100; > > /* \todo use calculated value based on sensor */ > -static constexpr uint32_t kMinExposure = 1; > -static constexpr uint32_t kMaxExposure = 1976; > +static constexpr libcamera::utils::Duration kMinShutterSpeed = 100us; > +static constexpr libcamera::utils::Duration kMaxShutterSpeed = 20ms; I'd like to rename shutter speed to integration time (in a separate patch). What do you think ? "exposure time" may also be a candidate, but it is often shortened to "exposure" which is then easily confused with the photographic exposure. > /* Histogram constants */ > static constexpr uint32_t knumHistogramBins = 256; > @@ -49,8 +49,8 @@ static constexpr double kEvGainTarget = 0.5; > > Agc::Agc() > : frameCount_(0), lastFrame_(0), iqMean_(0.0), lineDuration_(0s), > - maxExposureTime_(0s), filteredExposure_(0s), filteredExposureNoDg_(0s), > - currentExposure_(0s), currentExposureNoDg_(0s) > + minExposureLines_(0), maxExposureLines_(0), filteredExposure_(0s), > + filteredExposureNoDg_(0s), currentExposure_(0s), currentExposureNoDg_(0s) > { > } > > @@ -60,7 +60,9 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo) > > lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s > / configInfo.sensorInfo.pixelRate; > - maxExposureTime_ = kMaxExposure * lineDuration_; Removing the maxExposureTime_ member variable is fine as it's currently a constant, but that won't always be the case. Shouldn't we keep it to prepare for calculation of the maximum exposure time from the sensor's limits ? > + > + minExposureLines_ = kMinShutterSpeed / lineDuration_; > + maxExposureLines_ = kMaxShutterSpeed / lineDuration_; I wonder if this goes in the right direction. Wouldn't it be better for the algorithm to operate on a time, with conversion to/from lines being handled outside (in ipu3.cpp, or possibly even in the pipeline handler) ? David, what's your opinion on this ? > > return 0; > } > @@ -140,28 +142,30 @@ void Agc::lockExposureGain(uint32_t &exposure, double &gain) > double newGain = kEvGainTarget * knumHistogramBins / iqMean_; > > /* extracted from Rpi::Agc::computeTargetExposure */ > - libcamera::utils::Duration currentShutter = exposure * lineDuration_; > + Duration currentShutter = exposure * lineDuration_; This compiles because there's a "using utils::Duration" in agc.h. This is harmful, I'll send a patch to drop it, so you will need utils::duration here (the libcamera:: prefix can be dropped as we're in the libcamera namespace). > currentExposureNoDg_ = currentShutter * gain; > LOG(IPU3Agc, Debug) << "Actual total exposure " << currentExposureNoDg_ > << " Shutter speed " << currentShutter > << " Gain " << gain; > + > currentExposure_ = currentExposureNoDg_ * newGain; > - libcamera::utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain; > + Duration maxTotalExposure = kMaxShutterSpeed * kMaxGain; > currentExposure_ = std::min(currentExposure_, maxTotalExposure); > - LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_; > + LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_ > + << " maximum is " << maxTotalExposure; << ", maximum is " << maxTotalExposure; > > /* \todo: estimate if we need to desaturate */ > filterExposure(); > > - libcamera::utils::Duration newExposure = 0.0s; > - if (currentShutter < maxExposureTime_) { > - exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / currentExposureNoDg_), kMinExposure, kMaxExposure); > + Duration newExposure = 0.0s; > + if (currentShutter < kMaxShutterSpeed) { > + exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / currentExposureNoDg_), minExposureLines_, maxExposureLines_); This is a very good occasion to wrap lines: exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / currentExposureNoDg_), minExposureLines_, maxExposureLines_); > newExposure = currentExposure_ / exposure; > gain = std::clamp(static_cast<uint32_t>(gain * currentExposure_ / newExposure), kMinGain, kMaxGain); > - } else if (currentShutter >= maxExposureTime_) { > + } else if (currentShutter >= kMaxShutterSpeed) { > gain = std::clamp(static_cast<uint32_t>(gain * currentExposure_ / currentExposureNoDg_), kMinGain, kMaxGain); > newExposure = currentExposure_ / gain; > - exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / newExposure), kMinExposure, kMaxExposure); > + exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / newExposure), minExposureLines_, maxExposureLines_); Same here. > } > LOG(IPU3Agc, Debug) << "Adjust exposure " << exposure * lineDuration_ << " and gain " << gain; > } > diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h > index cd4d4855..7605ab39 100644 > --- a/src/ipa/ipu3/algorithms/agc.h > +++ b/src/ipa/ipu3/algorithms/agc.h > @@ -44,7 +44,8 @@ private: > double iqMean_; > > Duration lineDuration_; > - Duration maxExposureTime_; > + uint32_t minExposureLines_; > + uint32_t maxExposureLines_; > > Duration filteredExposure_; > Duration filteredExposureNoDg_;
Hi Laurent, On 15/10/2021 02:34, Laurent Pinchart wrote: > Hi Jean-Michel, > > (CC'ing David for a question below) > > Thank you for the patch. > > On Wed, Oct 13, 2021 at 05:41:18PM +0200, Jean-Michel Hautbois wrote: >> The exposure limits are set for one sensor until now, in a number of >> lines. We should not use those, but exposure limits in a time unit. >> >> Introduce default limits which with a default minimum of 20ms. This > > That's a default maximum, right ? > > Isn't 20ms a bit low ? > It is not meant to stay like that, the value should come from IPAIPU3 based on what the sensor can do, maybe with a local maximum of the maximum if we want (60ms sounds better). It also means we need to change exposure and VBLANK in the setControls() call. For that, we need to pass a full exposure time and not only the exposure in a number of lines. >> value should be given by the IPAIPU3. Use cached values expressed as a >> number of lines in the configure() call. >> >> Adapt the process() call accordingly. >> >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> >> --- >> src/ipa/ipu3/algorithms/agc.cpp | 30 +++++++++++++++++------------- >> src/ipa/ipu3/algorithms/agc.h | 3 ++- >> 2 files changed, 19 insertions(+), 14 deletions(-) >> >> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp >> index 7c2d4201..2dd5c5c1 100644 >> --- a/src/ipa/ipu3/algorithms/agc.cpp >> +++ b/src/ipa/ipu3/algorithms/agc.cpp >> @@ -40,8 +40,8 @@ static constexpr uint32_t kMinGain = kMinISO / 100; >> static constexpr uint32_t kMaxGain = kMaxISO / 100; >> >> /* \todo use calculated value based on sensor */ >> -static constexpr uint32_t kMinExposure = 1; >> -static constexpr uint32_t kMaxExposure = 1976; >> +static constexpr libcamera::utils::Duration kMinShutterSpeed = 100us; >> +static constexpr libcamera::utils::Duration kMaxShutterSpeed = 20ms; > > I'd like to rename shutter speed to integration time (in a separate > patch). What do you think ? "exposure time" may also be a candidate, but > it is often shortened to "exposure" which is then easily confused with > the photographic exposure. > I hesitated, between the three. I think "integration time" is digital photography term, while shutter speed is more general, but maybe David and others have a different feeling ? >> /* Histogram constants */ >> static constexpr uint32_t knumHistogramBins = 256; >> @@ -49,8 +49,8 @@ static constexpr double kEvGainTarget = 0.5; >> >> Agc::Agc() >> : frameCount_(0), lastFrame_(0), iqMean_(0.0), lineDuration_(0s), >> - maxExposureTime_(0s), filteredExposure_(0s), filteredExposureNoDg_(0s), >> - currentExposure_(0s), currentExposureNoDg_(0s) >> + minExposureLines_(0), maxExposureLines_(0), filteredExposure_(0s), >> + filteredExposureNoDg_(0s), currentExposure_(0s), currentExposureNoDg_(0s) >> { >> } >> >> @@ -60,7 +60,9 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo) >> >> lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s >> / configInfo.sensorInfo.pixelRate; >> - maxExposureTime_ = kMaxExposure * lineDuration_; > > Removing the maxExposureTime_ member variable is fine as it's currently > a constant, but that won't always be the case. Shouldn't we keep it to > prepare for calculation of the maximum exposure time from the sensor's > limits ? > >> + >> + minExposureLines_ = kMinShutterSpeed / lineDuration_; >> + maxExposureLines_ = kMaxShutterSpeed / lineDuration_; > > I wonder if this goes in the right direction. Wouldn't it be better for > the algorithm to operate on a time, with conversion to/from lines being > handled outside (in ipu3.cpp, or possibly even in the pipeline handler) > ? David, what's your opinion on this ? Yes, we should do that. I could keep the previous maxExposureTime, and use local variables in lockExposureGain() for the moment... > >> >> return 0; >> } >> @@ -140,28 +142,30 @@ void Agc::lockExposureGain(uint32_t &exposure, double &gain) >> double newGain = kEvGainTarget * knumHistogramBins / iqMean_; >> >> /* extracted from Rpi::Agc::computeTargetExposure */ >> - libcamera::utils::Duration currentShutter = exposure * lineDuration_; >> + Duration currentShutter = exposure * lineDuration_; > > This compiles because there's a "using utils::Duration" in agc.h. This > is harmful, I'll send a patch to drop it, so you will need > utils::duration here (the libcamera:: prefix can be dropped as we're in > the libcamera namespace). OK, thanks. > >> currentExposureNoDg_ = currentShutter * gain; >> LOG(IPU3Agc, Debug) << "Actual total exposure " << currentExposureNoDg_ >> << " Shutter speed " << currentShutter >> << " Gain " << gain; >> + >> currentExposure_ = currentExposureNoDg_ * newGain; >> - libcamera::utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain; >> + Duration maxTotalExposure = kMaxShutterSpeed * kMaxGain; >> currentExposure_ = std::min(currentExposure_, maxTotalExposure); >> - LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_; >> + LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_ >> + << " maximum is " << maxTotalExposure; > > << ", maximum is " << maxTotalExposure; > >> >> /* \todo: estimate if we need to desaturate */ >> filterExposure(); >> >> - libcamera::utils::Duration newExposure = 0.0s; >> - if (currentShutter < maxExposureTime_) { >> - exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / currentExposureNoDg_), kMinExposure, kMaxExposure); >> + Duration newExposure = 0.0s; >> + if (currentShutter < kMaxShutterSpeed) { >> + exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / currentExposureNoDg_), minExposureLines_, maxExposureLines_); > > This is a very good occasion to wrap lines: > > exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / currentExposureNoDg_), > minExposureLines_, maxExposureLines_); > >> newExposure = currentExposure_ / exposure; >> gain = std::clamp(static_cast<uint32_t>(gain * currentExposure_ / newExposure), kMinGain, kMaxGain); >> - } else if (currentShutter >= maxExposureTime_) { >> + } else if (currentShutter >= kMaxShutterSpeed) { >> gain = std::clamp(static_cast<uint32_t>(gain * currentExposure_ / currentExposureNoDg_), kMinGain, kMaxGain); >> newExposure = currentExposure_ / gain; >> - exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / newExposure), kMinExposure, kMaxExposure); >> + exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / newExposure), minExposureLines_, maxExposureLines_); > > Same here. > >> } >> LOG(IPU3Agc, Debug) << "Adjust exposure " << exposure * lineDuration_ << " and gain " << gain; >> } >> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h >> index cd4d4855..7605ab39 100644 >> --- a/src/ipa/ipu3/algorithms/agc.h >> +++ b/src/ipa/ipu3/algorithms/agc.h >> @@ -44,7 +44,8 @@ private: >> double iqMean_; >> >> Duration lineDuration_; >> - Duration maxExposureTime_; >> + uint32_t minExposureLines_; >> + uint32_t maxExposureLines_; >> >> Duration filteredExposure_; >> Duration filteredExposureNoDg_; >
diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp index 7c2d4201..2dd5c5c1 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -40,8 +40,8 @@ static constexpr uint32_t kMinGain = kMinISO / 100; static constexpr uint32_t kMaxGain = kMaxISO / 100; /* \todo use calculated value based on sensor */ -static constexpr uint32_t kMinExposure = 1; -static constexpr uint32_t kMaxExposure = 1976; +static constexpr libcamera::utils::Duration kMinShutterSpeed = 100us; +static constexpr libcamera::utils::Duration kMaxShutterSpeed = 20ms; /* Histogram constants */ static constexpr uint32_t knumHistogramBins = 256; @@ -49,8 +49,8 @@ static constexpr double kEvGainTarget = 0.5; Agc::Agc() : frameCount_(0), lastFrame_(0), iqMean_(0.0), lineDuration_(0s), - maxExposureTime_(0s), filteredExposure_(0s), filteredExposureNoDg_(0s), - currentExposure_(0s), currentExposureNoDg_(0s) + minExposureLines_(0), maxExposureLines_(0), filteredExposure_(0s), + filteredExposureNoDg_(0s), currentExposure_(0s), currentExposureNoDg_(0s) { } @@ -60,7 +60,9 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo) lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s / configInfo.sensorInfo.pixelRate; - maxExposureTime_ = kMaxExposure * lineDuration_; + + minExposureLines_ = kMinShutterSpeed / lineDuration_; + maxExposureLines_ = kMaxShutterSpeed / lineDuration_; return 0; } @@ -140,28 +142,30 @@ void Agc::lockExposureGain(uint32_t &exposure, double &gain) double newGain = kEvGainTarget * knumHistogramBins / iqMean_; /* extracted from Rpi::Agc::computeTargetExposure */ - libcamera::utils::Duration currentShutter = exposure * lineDuration_; + Duration currentShutter = exposure * lineDuration_; currentExposureNoDg_ = currentShutter * gain; LOG(IPU3Agc, Debug) << "Actual total exposure " << currentExposureNoDg_ << " Shutter speed " << currentShutter << " Gain " << gain; + currentExposure_ = currentExposureNoDg_ * newGain; - libcamera::utils::Duration maxTotalExposure = maxExposureTime_ * kMaxGain; + Duration maxTotalExposure = kMaxShutterSpeed * kMaxGain; currentExposure_ = std::min(currentExposure_, maxTotalExposure); - LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_; + LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_ + << " maximum is " << maxTotalExposure; /* \todo: estimate if we need to desaturate */ filterExposure(); - libcamera::utils::Duration newExposure = 0.0s; - if (currentShutter < maxExposureTime_) { - exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / currentExposureNoDg_), kMinExposure, kMaxExposure); + Duration newExposure = 0.0s; + if (currentShutter < kMaxShutterSpeed) { + exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / currentExposureNoDg_), minExposureLines_, maxExposureLines_); newExposure = currentExposure_ / exposure; gain = std::clamp(static_cast<uint32_t>(gain * currentExposure_ / newExposure), kMinGain, kMaxGain); - } else if (currentShutter >= maxExposureTime_) { + } else if (currentShutter >= kMaxShutterSpeed) { gain = std::clamp(static_cast<uint32_t>(gain * currentExposure_ / currentExposureNoDg_), kMinGain, kMaxGain); newExposure = currentExposure_ / gain; - exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / newExposure), kMinExposure, kMaxExposure); + exposure = std::clamp(static_cast<uint32_t>(exposure * currentExposure_ / newExposure), minExposureLines_, maxExposureLines_); } LOG(IPU3Agc, Debug) << "Adjust exposure " << exposure * lineDuration_ << " and gain " << gain; } diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h index cd4d4855..7605ab39 100644 --- a/src/ipa/ipu3/algorithms/agc.h +++ b/src/ipa/ipu3/algorithms/agc.h @@ -44,7 +44,8 @@ private: double iqMean_; Duration lineDuration_; - Duration maxExposureTime_; + uint32_t minExposureLines_; + uint32_t maxExposureLines_; Duration filteredExposure_; Duration filteredExposureNoDg_;
The exposure limits are set for one sensor until now, in a number of lines. We should not use those, but exposure limits in a time unit. Introduce default limits which with a default minimum of 20ms. This value should be given by the IPAIPU3. Use cached values expressed as a number of lines in the configure() call. Adapt the process() call accordingly. Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> --- src/ipa/ipu3/algorithms/agc.cpp | 30 +++++++++++++++++------------- src/ipa/ipu3/algorithms/agc.h | 3 ++- 2 files changed, 19 insertions(+), 14 deletions(-)