[libcamera-devel] [PATCH v2 02/14] ipu3: ipa: Report effective sensor controls with statistics to IPA

[Jean-Michel Hautbois]

On 10/11/2021 22:53, Kieran Bingham wrote:
> Quoting Jean-Michel Hautbois (2021-11-10 19:58:49)
>> From: Han-Lin Chen <hanlinchen at chromium.org>
>>
>> The Intel close sourced IPA requires the effective controls applied to
>> the sensor when the statistics are generated. Report effective sensor controls
>> with the statistics to IPA.
>>
>> Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
>> [Jean-Michel: Reword s/stastistics/statistics and move reset after IPA
>> start]
>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
>> ---
>>   src/libcamera/pipeline/ipu3/frames.h | 4 ++++
>>   src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +++++++++
>>   2 files changed, 13 insertions(+)
>>
>> diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h
>> index 3ef7e445..614701e2 100644
>> --- a/src/libcamera/pipeline/ipu3/frames.h
>> +++ b/src/libcamera/pipeline/ipu3/frames.h
>> @@ -14,6 +14,8 @@
>>   
>>   #include <libcamera/base/signal.h>
>>   
>> +#include <libcamera/controls.h>
>> +
>>   namespace libcamera {
>>   
>>   class FrameBuffer;
>> @@ -34,6 +36,8 @@ public:
>>                  FrameBuffer *paramBuffer;
>>                  FrameBuffer *statBuffer;
>>   
>> +               ControlList effectiveSensorControls;
>> +
>>                  bool paramDequeued;
>>                  bool metadataProcessed;
>>          };
>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
>> index 8816efc5..3fcfa777 100644
>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
>> @@ -767,6 +767,12 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis
>>          if (ret)
>>                  goto error;
>>   
>> +       /*
>> +        * Reset the delayed controls with the gain and exposure values set by
>> +        * the IPA.
> 
> This seems to imply that resetting it is setting some values. Is that
> accurate?
> 
> Looking at DelayedControls::reset() - it reads the controls from the
> device. Has the IPA already 'set' some controls before this point (not
> through the DelayedControls...)
> 

Not yet, except HFLIP and VFLIP controls, which are not useful for us in 
this ? Maybe some later ? I must say this is what RPi is doing too :-).

> Other than that,
> 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
>> +        */
>> +       data->delayedCtrls_->reset();
>> +
>>          /*
>>           * Start the ImgU video devices, buffers will be queued to the
>>           * ImgU output and viewfinder when requests will be queued.
>> @@ -1363,6 +1369,8 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
>>          request->metadata().set(controls::SensorTimestamp,
>>                                  buffer->metadata().timestamp);
>>   
>> +       info->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence);
>> +
>>          if (request->findBuffer(&rawStream_))
>>                  pipe()->completeBuffer(request, buffer);
>>   
>> @@ -1419,6 +1427,7 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
>>          ev.frame = info->id;
>>          ev.bufferId = info->statBuffer->cookie();
>>          ev.frameTimestamp = request->metadata().get(controls::SensorTimestamp);
>> +       ev.sensorControls = info->effectiveSensorControls;
>>          ipa_->processEvent(ev);
>>   }
>>   
>> -- 
>> 2.32.0
>>