[libcamera-devel] [PATCH v2 03/14] ipa: ipu3: Use sensor controls to update frameContext

[Jean-Michel Hautbois]

Hi Kieran,

On 10/11/2021 22:50, Kieran Bingham wrote:
> Hi JM,
> 
> Quoting Jean-Michel Hautbois (2021-11-10 19:58:50)
>> 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::agc values for the frame when the
>> EventStatReady event is received.
>>
>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
>> ---
>>   src/ipa/ipu3/ipu3.cpp | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
>> index bcc3863b..1111f200 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 ? */
>> +               context_.frameContext.agc.exposure = event.sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
>> +               context_.frameContext.agc.gain = camHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
> 
> My only worry with this reading it now, is that we are storing the
> exposure and gain of the frame that that is associated with the
> statistics in what I had thought was a structure that maps the 'output'
> of the algorithms.

I am not sure it is only the output, it is a frame context, input and 
output.

> 
> You've explored this a bit more now - so you probably have a much better
> idea than me. This is ultimately an 'input' to the algorithms. Do we
> need to distinguish between the 'input' and 'output' ? (I.e. do we need
> to have both values kept so that the algorithm can determine how much it
> is changing? Or is that not a problem because it's all handled
> internally anyway....

We update the value for the input of the algorithms, and they will 
update those values. It is debatable though: should we have two 
frameContexts, one before and one after the algorithm for instance ? I 
can't see a real value to add this though.

> 
>> +
>>                  parseStatistics(event.frame, event.frameTimestamp, stats);
>>                  break;
>>          }
>> -- 
>> 2.32.0
>>