[libcamera-devel] [PATCH 02/13] ipa: ipu3: set frameContext before controls

[Jean-Michel Hautbois]

Hi Kieran, Laurent,

On 15/10/2021 00:00, Laurent Pinchart wrote:
> On Thu, Oct 14, 2021 at 11:26:22AM +0100, Kieran Bingham wrote:
>> Quoting Jean-Michel Hautbois (2021-10-13 16:41:14)
>>
>> Missing some context here. Is this correct? If not put something that's
>> a bit more correct please ;-)
>>
>> """
>> The AGC frame context needs to be initialised correctly for the first
>> iteration.
>>
>> Set the gain and exposure appropriately to the current values known to
>> the IPA.
>> """
>>

Oh my, I missed this commit message, sorry :-(.

>>
>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>
>>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
>>> ---
>>>  src/ipa/ipu3/ipu3.cpp | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
>>> index 6d9bbf39..e54fbab5 100644
>>> --- a/src/ipa/ipu3/ipu3.cpp
>>> +++ b/src/ipa/ipu3/ipu3.cpp
>>> @@ -415,6 +415,9 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo)
>>>                         return ret;
>>>         }
>>>  
>>> +       context_.frameContext.agc.gain = camHelper_->gain(gain_);
>>> +       context_.frameContext.agc.exposure = exposure_;
> 
> Why is this special, why don't we set the rest of the context here, or,
> possibly better, set this in the AGC algorithm ? The exposure_ and gain_
> members of IPU3IPA seem a bit out of place. Maybe they're removed later
> in this series ?
> 

Indeed, they should move to AGC, and later we will not pass exposure and
gain this way, but with a duration value and a double to represent the
shutter speed and the analogue gain. I wanted it in the series but did
not come to something satisfying yet.

>>> +
>>>         return 0;
>>>  }
>>>  
>