[libcamera-devel] [PATCH 0/4] ipa: ipu3: af: Temporarily suspend AWB and AE during AF scanning

[Jean-Michel Hautbois]

Hello Hans,

On 23/03/2022 14:30, Hans de Goede wrote:
> Hi,
> 
> On 3/22/22 15:01, Jean-Michel Hautbois via libcamera-devel wrote:
>> Hi Kate !
>>
>> On 22/03/2022 03:41, Kate Hsuan via libcamera-devel wrote:
>>> The floating results of AWB and AE significantly impact the AF
>>> performance and the AF scene variance estimation. If the exposure value
>>> and color settings are changed by the algorithm, the AF scene variance will
>>> be affected and the algorithm may find an incorrect local maximum value.
>>> Consequently, a wrong lens position is determined and we could only get
>>> a blurred image.
>>
>> I agree with you, at least on one assumption: AWB can have an impact on AF. AE is more debatable I think :-).
> 
> Interesting I would actually expect the importance to be the otherway
> around. If AE is way off, so way overexposed or much to dark, there
> is not much on which the focus algorithm can actually focus. Where as
> if an image is e.g. to "green" I would not expect that to make a lot
> of difference for the contrast of the image.

Well, indeed, if your scene is too dark or too bright, you will have big 
issues. But in case you are flickering around a somehow not that bad 
value, then shouldn't really change the edges. On the contrary if you 
change only the red channel for instance, you may experience weird 
things... I am not sure that I am clear here :-). Maybe should we 
conduct experiments extracting the filters output in various conditions 
for a given scene ?

> 
>> I think we should consider something a bit different from what you did, if you don't mind, as I don't think AWB should know anything out of its own scope, same for AGC, same for AF, same for... you get it :-).
>> This should be taken into account at the IPA level, the controller, which could set a state for an algorithm.
> 
> I agree, but I think we want this on other IPU-s too, so maybe we need to do this
> at an even higher level, assuming that is possible (I still need
> to familiarize myself with libcamera more) ?
> 
>> I think that, while autofocus is starting, or in coarse search mode, we should not change the awb gains. When is gets into the fine search mode, then AWB could be started as it should not interfere anymore. So, I think AF could have a 'status' field in its frameContext, and the algorithms (AWB, AGC, probably AF) should be "pausable". David sent a patch lately for RPi [1] and this is something we should consider.
>>
>> That beeing said, and I hope I am clear in my explanations, before calling the process() call for the algorithms, the IPU3IPU could set the status of AWB and AGC based on the AF status. I would start AF after AGC is stable enough (could be based on a number of frames for a first try) and I would enable AWB while AGC is starting.
>>
>> Once AGC is stable, AF is entering the coarse search, and in this phase AWB is paused. Once it enters the fine search, we can resume AWB.
> 
> Agreed I think we also need some sort of AGC "done" bit because as
> said I don't think doing auto-focus before we at least have the
> exposure setup somewhat correct makes a lot of sense.
> 
>> I think it should be enough, as we are not zooming, so illumination is not changing a lot with focus change AFAIK.
> 
> Regards,
> 
> Hans
> 
> 
> 
>>
>> I don't really have time to test it so if you want/can I would greatly appreciate :-).
>>
>> Thanks !
>> JM
>>
>> [1]: https://patchwork.libcamera.org/patch/15343/
>>
>>>
>>> This patch proposed an AWB and AE suspension mechanism while AF is
>>> searching for the focus. A lock flag is used to identify that AWB and
>>> AE should be suspended or not. AGC and AE algorithm test the flag and
>>> determine that they have to perform the algorithm or suspend itself.
>>>
>>> Kate Hsuan (4):
>>>     ipa: ipu3: af: Introduce AWB and AE lock when AF scanning
>>>     ipa: ipu3: af: AE and AWE lock request flow when performing AF
>>>     ipa: ipu3: awb: AWB lock for AF scan
>>>     ipa: ipu3: agc: AE lock for AF scanning
>>>
>>>    src/ipa/ipu3/algorithms/af.cpp  | 25 +++++++++++++++++++++++++
>>>    src/ipa/ipu3/algorithms/af.h    |  2 ++
>>>    src/ipa/ipu3/algorithms/agc.cpp | 14 ++++++++++++++
>>>    src/ipa/ipu3/algorithms/agc.h   |  1 +
>>>    src/ipa/ipu3/algorithms/awb.cpp | 13 +++++++++++++
>>>    src/ipa/ipu3/algorithms/awb.h   |  1 +
>>>    src/ipa/ipu3/ipa_context.h      |  1 +
>>>    7 files changed, 57 insertions(+)
>>>
>>
>