[libcamera-devel] [PATCH v3 03/14] ipa: ipu3: Use sensor controls to update frameContext
Jean-Michel Hautbois
jeanmichel.hautbois at ideasonboard.com
Thu Nov 11 14:51:19 CET 2021
On 11/11/2021 14:13, Kieran Bingham wrote:
> Quoting Jean-Michel Hautbois (2021-11-11 11:05:54)
>> The pipeline handler populates the new sensorControls ControlList, to
>> have the effective exposure and gain values for the current frame. This
>> is done when a statistics buffer is received.
>>
>> Make those values the frameContext::sensor values for the frame when the
>> EventStatReady event is received.
>>
>> AGC also needs to use frameContext.sensor as its input values and
>> frameContext.agc as its output values. Modify computeExposure by passing
>> it the frameContext instead of individual exposure and gain values.
>>
>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
>> ---
>> src/ipa/ipu3/algorithms/agc.cpp | 19 ++++++++++---------
>> src/ipa/ipu3/algorithms/agc.h | 2 +-
>> src/ipa/ipu3/ipa_context.cpp | 11 +++++++++++
>> src/ipa/ipu3/ipa_context.h | 5 +++++
>> src/ipa/ipu3/ipu3.cpp | 4 ++++
>> 5 files changed, 31 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
>> index b5d736c1..825b35d4 100644
>> --- a/src/ipa/ipu3/algorithms/agc.cpp
>> +++ b/src/ipa/ipu3/algorithms/agc.cpp
>> @@ -169,10 +169,9 @@ void Agc::filterExposure()
>>
>> /**
>> * \brief Estimate the new exposure and gain values
>> - * \param[inout] exposure The exposure value reference as a number of lines
>> - * \param[inout] gain The gain reference to be updated
>> + * \param[inout] frameContext The shared IPA frame Context
>> */
>> -void Agc::computeExposure(uint32_t &exposure, double &analogueGain)
>> +void Agc::computeExposure(IPAFrameContext &frameContext)
>> {
>> /* Algorithm initialization should wait for first valid frames */
>> /* \todo - have a number of frames given by DelayedControls ?
>> @@ -189,6 +188,10 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain)
>> return;
>> }
>>
>> + /* Get the effective exposure and gain applied on the sensor. */
>> + uint32_t &exposure = frameContext.sensor.exposure;
>> + double &analogueGain = frameContext.sensor.gain;
>
> I don't think these need to be references now, but it doesn't
> particularly matter.
>
> Perhaps removing the & would make it clear that the
> frameContext.sensor.exposure and frameContext.sensor.gain isn't going to
> be modified in this function (which the & could otherwise imply).
>
>
>> +
>> /* Estimate the gain needed to have the proportion wanted */
>> double evGain = kEvGainTarget * knumHistogramBins / iqMean_;
>>
>> @@ -233,8 +236,9 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain)
>> << shutterTime << " and "
>> << stepGain;
>>
>> - exposure = shutterTime / lineDuration_;
>> - analogueGain = stepGain;
>> + /* Update the estimated exposure and gain. */
>> + frameContext.agc.exposure = shutterTime / lineDuration_;
>> + frameContext.agc.gain = stepGain;
>>
>> /*
>> * Update the exposure value for the next process call.
>> @@ -257,11 +261,8 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain)
>> */
>> void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
>> {
>> - /* Get the latest exposure and gain applied */
>> - uint32_t &exposure = context.frameContext.agc.exposure;
>> - double &analogueGain = context.frameContext.agc.gain;
>> measureBrightness(stats, context.configuration.grid.bdsGrid);
>> - computeExposure(exposure, analogueGain);
>> + computeExposure(context.frameContext);
>
> That's a lot cleaner...
>
>> frameCount_++;
>> }
>>
>> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
>> index 69e0b831..f0db25ee 100644
>> --- a/src/ipa/ipu3/algorithms/agc.h
>> +++ b/src/ipa/ipu3/algorithms/agc.h
>> @@ -34,7 +34,7 @@ private:
>> void measureBrightness(const ipu3_uapi_stats_3a *stats,
>> const ipu3_uapi_grid_config &grid);
>> void filterExposure();
>> - void computeExposure(uint32_t &exposure, double &gain);
>> + void computeExposure(IPAFrameContext &frameContext);
>>
>> uint64_t frameCount_;
>> uint64_t lastFrame_;
>> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
>> index 2355a9c7..a7ff957d 100644
>> --- a/src/ipa/ipu3/ipa_context.cpp
>> +++ b/src/ipa/ipu3/ipa_context.cpp
>> @@ -119,6 +119,17 @@ namespace libcamera::ipa::ipu3 {
>> * \brief White balance gain for B channel
>> */
>>
>> +/**
>> + * \var IPAFrameContext::sensor
>> + * \brief Effective sensor values
>> + *
>> + * \var IPAFrameContext::sensor.exposure
>> + * \brief Exposure time expressed as a number of lines
>> + *
>> + * \var IPAFrameContext::sensor.gain
>> + * \brief Analogue gain multiplier
>> + */
>> +
>> /**
>> * \var IPAFrameContext::toneMapping
>> * \brief Context for ToneMapping and Gamma control
>> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
>> index 1e46c61a..a5a19800 100644
>> --- a/src/ipa/ipu3/ipa_context.h
>> +++ b/src/ipa/ipu3/ipa_context.h
>> @@ -47,6 +47,11 @@ struct IPAFrameContext {
>> } gains;
>> } awb;
>>
>> + struct {
>> + uint32_t exposure;
>> + double gain;
>> + } sensor;
>> +
>> struct {
>> double gamma;
>> struct ipu3_uapi_gamma_corr_lut gammaCorrection;
>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
>> index bcc3863b..dee002a5 100644
>> --- a/src/ipa/ipu3/ipu3.cpp
>> +++ b/src/ipa/ipu3/ipu3.cpp
>> @@ -549,6 +549,10 @@ void IPAIPU3::processEvent(const IPU3Event &event)
>> const ipu3_uapi_stats_3a *stats =
>> reinterpret_cast<ipu3_uapi_stats_3a *>(mem.data());
>>
>> + /* \todo move those into processControls ? */
>
> I still haven't understood why we want to move these into
> processControls.
>
> I think that is supposed to deal with processing controls from the
> Request?
>
That is more a question, I will remove it as a comment for the series,
and if it appears later we need to move those then it will answer the
question :-).
> For everything else
>
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
>> + context_.frameContext.sensor.exposure = event.sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
>> + context_.frameContext.sensor.gain = camHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
>> +
>> parseStatistics(event.frame, event.frameTimestamp, stats);
>> break;
>> }
>> --
>> 2.32.0
>>
More information about the libcamera-devel
mailing list