[libcamera-devel] [PATCH v2 3/3] ipa: ipu3: agc: Introduce lineDuration in IPASessionConfiguration
Jean-Michel Hautbois
jeanmichel.hautbois at ideasonboard.com
Tue Feb 8 18:09:59 CET 2022
Hi Kieran,
On 07/02/2022 16:34, Kieran Bingham wrote:
> Hi JM,
>
> Quoting Jean-Michel Hautbois (2022-02-07 13:59:37)
>> Instead of having a local cached value for line duration, store it in
>> the IPASessionConfiguration::agc structure.
>
> Looking at my previous comments on this patch in
> https://patchwork.libcamera.org/patch/14778/, I was suggesting that I
> believed this line duration should be in the sensor specific structure,
> not the AGC specific structure.
>
> I'm sorry my comment then was not clear, perhaps I should have made it
> more direct.
>
> Is the lineDuration specific to the AGC algorithm in any way that I have
> not seen or understood?
>
> In rkisp1/ipa_context.h, you have:
>
> struct IPASessionConfiguration {
> ...
> struct {
> utils::Duration lineDuration;
> } sensor;
> ...
> };
>
>
>> While at it, configure the default analogue gain and shutter speed to
>> controlled fixed values.
>>
>> The latter is set to be 10ms as it will in most cases be close to the
>> one needed, making the AGC faster to converge.
>>
>> 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 +--
>> src/ipa/ipu3/ipa_context.cpp | 3 +++
>> src/ipa/ipu3/ipa_context.h | 1 +
>> 4 files changed, 20 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
>> index a929085c..137c36cf 100644
>> --- a/src/ipa/ipu3/algorithms/agc.cpp
>> +++ b/src/ipa/ipu3/algorithms/agc.cpp
>> @@ -71,7 +71,7 @@ static constexpr uint32_t kNumStartupFrames = 10;
>> static constexpr double kRelativeLuminanceTarget = 0.16;
>>
>> Agc::Agc()
>> - : frameCount_(0), lineDuration_(0s), minShutterSpeed_(0s),
>> + : frameCount_(0), minShutterSpeed_(0s),
>> maxShutterSpeed_(0s), filteredExposure_(0s)
>> {
>> }
>> @@ -85,11 +85,14 @@ Agc::Agc()
>> */
>> int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>> {
>> - stride_ = context.configuration.grid.stride;
>> + IPASessionConfiguration &configuration = context.configuration;
>> + IPAFrameContext &frameContext = context.frameContext;
>> +
>> + stride_ = configuration.grid.stride;
>>
>> /* \todo use the IPAContext to provide the limits */
>> - lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s
>> - / configInfo.sensorInfo.pixelRate;
>> + configuration.agc.lineDuration = configInfo.sensorInfo.lineLength * 1.0s
>> + / configInfo.sensorInfo.pixelRate;
>
> And in RKISP1 this is calculated during the IPA configure, before it
> calls configure on the algorithms. Should we do that here to align them?
>
I noticed that lineDuration_ is cached and used in updateControls()
which is called in configure()... and in init() (so, before it is set ?)...
We might have an issue there ?
>
>>
>> minShutterSpeed_ = context.configuration.agc.minShutterSpeed;
>> maxShutterSpeed_ = std::min(context.configuration.agc.maxShutterSpeed,
>> @@ -99,8 +102,8 @@ int Agc::configure(IPAContext &context, const IPAConfigInfo &configInfo)
>> maxAnalogueGain_ = std::min(context.configuration.agc.maxAnalogueGain, kMaxAnalogueGain);
>>
>> /* Configure the default exposure and gain. */
>> - context.frameContext.agc.gain = minAnalogueGain_;
>> - context.frameContext.agc.exposure = minShutterSpeed_ / lineDuration_;
>> + frameContext.agc.gain = std::max(minAnalogueGain_, kMinAnalogueGain);
>> + frameContext.agc.exposure = 10ms / configuration.agc.lineDuration;
>>
>> return 0;
>> }
>> @@ -182,9 +185,11 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue)
>> * \param[in] yGain The gain calculated based on the relative luminance target
>> * \param[in] iqMeanGain The gain calculated based on the relative luminance target
>> */
>> -void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,
>> +void Agc::computeExposure(IPAContext &context, double yGain,
>> double iqMeanGain)
>> {
>> + const IPASessionConfiguration &configuration = context.configuration;
>> + IPAFrameContext &frameContext = context.frameContext;
>> /* Get the effective exposure and gain applied on the sensor. */
>> uint32_t exposure = frameContext.sensor.exposure;
>> double analogueGain = frameContext.sensor.gain;
>> @@ -200,7 +205,7 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,
>> /* extracted from Rpi::Agc::computeTargetExposure */
>>
>> /* Calculate the shutter time in seconds */
>> - utils::Duration currentShutter = exposure * lineDuration_;
>> + utils::Duration currentShutter = exposure * configuration.agc.lineDuration;
>>
>> /*
>> * Update the exposure value for the next computation using the values
>> @@ -247,7 +252,7 @@ void Agc::computeExposure(IPAFrameContext &frameContext, double yGain,
>> << stepGain;
>>
>> /* Update the estimated exposure and gain. */
>> - frameContext.agc.exposure = shutterTime / lineDuration_;
>> + frameContext.agc.exposure = shutterTime / configuration.agc.lineDuration;
>> frameContext.agc.gain = stepGain;
>> }
>>
>> @@ -354,7 +359,7 @@ void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
>> break;
>> }
>>
>> - computeExposure(context.frameContext, yGain, iqMeanGain);
>> + computeExposure(context, yGain, iqMeanGain);
>> frameCount_++;
>> }
>>
>> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
>> index 84bfe045..ad705605 100644
>> --- a/src/ipa/ipu3/algorithms/agc.h
>> +++ b/src/ipa/ipu3/algorithms/agc.h
>> @@ -34,7 +34,7 @@ private:
>> double measureBrightness(const ipu3_uapi_stats_3a *stats,
>> const ipu3_uapi_grid_config &grid) const;
>> utils::Duration filterExposure(utils::Duration currentExposure);
>> - void computeExposure(IPAFrameContext &frameContext, double yGain,
>> + void computeExposure(IPAContext &context, double yGain,
>> double iqMeanGain);
>> double estimateLuminance(IPAFrameContext &frameContext,
>> const ipu3_uapi_grid_config &grid,
>> @@ -43,7 +43,6 @@ private:
>>
>> uint64_t frameCount_;
>>
>> - utils::Duration lineDuration_;
>> utils::Duration minShutterSpeed_;
>> utils::Duration maxShutterSpeed_;
>>
>> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
>> index 86794ac1..ace9c66f 100644
>> --- a/src/ipa/ipu3/ipa_context.cpp
>> +++ b/src/ipa/ipu3/ipa_context.cpp
>> @@ -84,6 +84,9 @@ namespace libcamera::ipa::ipu3 {
>> *
>> * \var IPASessionConfiguration::agc.maxAnalogueGain
>> * \brief Maximum analogue gain supported with the configured sensor
>> + *
>> + * \var IPASessionConfiguration::agc.lineDuration
>> + * \brief Line duration in microseconds
>> */
>>
>> /**
>> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
>> index c6dc0814..7696fd14 100644
>> --- a/src/ipa/ipu3/ipa_context.h
>> +++ b/src/ipa/ipu3/ipa_context.h
>> @@ -30,6 +30,7 @@ struct IPASessionConfiguration {
>> utils::Duration maxShutterSpeed;
>> double minAnalogueGain;
>> double maxAnalogueGain;
>> + utils::Duration lineDuration;
>> } agc;
>> };
>>
>> --
>> 2.32.0
>>
More information about the libcamera-devel
mailing list