[libcamera-devel] [PATCH v2 13/13] ipa: ipu3: Use sensor limits for analogue gain
Jean-Michel Hautbois
jeanmichel.hautbois at ideasonboard.com
Thu Oct 21 07:45:44 CEST 2021
Hi Laurent,
On 21/10/2021 04:39, Laurent Pinchart wrote:
> Hi Jean-Michel,
>
> Thank you for the patch.
>
> On Wed, Oct 20, 2021 at 05:46:07PM +0200, Jean-Michel Hautbois wrote:
>> Instead of using constants for the analogue gains limits, use the
>> minimum and maximum from the configured sensor.
>>
>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
>> ---
>> src/ipa/ipu3/algorithms/agc.cpp | 25 +++++++++++++------------
>> src/ipa/ipu3/algorithms/agc.h | 3 +++
>> 2 files changed, 16 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
>> index fc37eca4..690ad7e6 100644
>> --- a/src/ipa/ipu3/algorithms/agc.cpp
>> +++ b/src/ipa/ipu3/algorithms/agc.cpp
>> @@ -30,10 +30,9 @@ static constexpr uint32_t kInitialFrameMinAECount = 4;
>> /* Number of frames to wait between new gain/exposure estimations */
>> static constexpr uint32_t kFrameSkipCount = 6;
>>
>> -/* Maximum analogue gain value
>> - * \todo grab it from a camera helper */
>> -static constexpr double kMinGain = 1.0;
>> -static constexpr double kMaxGain = 8.0;
>> +/* Limits for analogue gain values */
>> +static constexpr double kMinAnalogueGain = 1.0;
>> +static constexpr double kMaxAnalogueGain = 16.0;
>
> Should we keep 8.0, or is there a reason to allow gains up to 16.0 ? It
> seems like it will add lots of noise.
It is a clamp value, there is only one sensor I tested which can do more
than 8.0 which is the ov5670 (I believe because of a broken driver). We
could keep 8.0 indeed.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
>>
>> /* Histogram constants */
>> static constexpr uint32_t knumHistogramBins = 256;
>> @@ -53,15 +52,17 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>> lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s
>> / configInfo.sensorInfo.pixelRate;
>>
>> - /* Configure the default exposure and gain */
>> - context.frameContext.agc.gain =
>> - context.configuration.agc.minAnalogueGain;
>> - context.frameContext.agc.exposure = minExposureLines_;
>> -
>> /* \todo replace the exposure in lines storage with time based ones */
>> minExposureLines_ = context.configuration.agc.minShutterSpeed / lineDuration_;
>> maxExposureLines_ = context.configuration.agc.maxShutterSpeed / lineDuration_;
>>
>> + minAnalogueGain_ = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain);
>> + maxAnalogueGain_ = std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain);
>> +
>> + /* Configure the default exposure and gain */
>> + context.frameContext.agc.gain = minAnalogueGain_;
>> + context.frameContext.agc.exposure = minExposureLines_;
>> +
>> prevExposureValue_ = context.frameContext.agc.gain
>> * context.frameContext.agc.exposure
>> * lineDuration_;
>> @@ -139,7 +140,7 @@ void Agc::lockExposureGain(uint32_t &exposure, double &analogueGain)
>> utils::Duration minShutterSpeed = minExposureLines_ * lineDuration_;
>> utils::Duration maxShutterSpeed = maxExposureLines_ * lineDuration_;
>>
>> - utils::Duration maxTotalExposure = maxShutterSpeed * kMaxGain;
>> + utils::Duration maxTotalExposure = maxShutterSpeed * maxAnalogueGain_;
>> currentExposure_ = std::min(currentExposure_, maxTotalExposure);
>> LOG(IPU3Agc, Debug) << "Target total exposure " << currentExposure_
>> << ", maximum is " << maxTotalExposure;
>> @@ -154,10 +155,10 @@ void Agc::lockExposureGain(uint32_t &exposure, double &analogueGain)
>> * Push the shutter time up to the maximum first, and only then
>> * increase the gain.
>> */
>> - shutterTime = std::clamp<utils::Duration>(exposureValue / kMinGain,
>> + shutterTime = std::clamp<utils::Duration>(exposureValue / minAnalogueGain_,
>> minShutterSpeed, maxShutterSpeed);
>> double stepGain = std::clamp(exposureValue / shutterTime,
>> - kMinGain, kMaxGain);
>> + minAnalogueGain_, maxAnalogueGain_);
>> LOG(IPU3Agc, Debug) << "Divided up shutter and gain are "
>> << shutterTime << " and "
>> << stepGain;
>> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
>> index ad133b98..1840205b 100644
>> --- a/src/ipa/ipu3/algorithms/agc.h
>> +++ b/src/ipa/ipu3/algorithms/agc.h
>> @@ -45,6 +45,9 @@ private:
>> uint32_t minExposureLines_;
>> uint32_t maxExposureLines_;
>>
>> + double minAnalogueGain_;
>> + double maxAnalogueGain_;
>> +
>> utils::Duration filteredExposure_;
>> utils::Duration currentExposure_;
>> utils::Duration prevExposureValue_;
>
More information about the libcamera-devel
mailing list