[libcamera-devel] [PATCH v2 14/14] ipa: ipu3: Pass the AnalogueGain control

[Kieran Bingham]

Quoting Jean-Michel Hautbois (2021-11-10 19:59:01)
> We can set the controls::AnalogueGain metadata now that AGC is updating
> it correctly.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

Eeek. I think I need to revoke this (temporarily? :D) - I think it might be wrong.

> ---
>  src/ipa/ipu3/ipu3.cpp | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 9220fea5..e69382f9 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -637,6 +637,8 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>  
>         ctrls.set(controls::ExposureTime, context_.frameContext.agc.exposure * lineDuration_.count());
>  
> +       ctrls.set(controls::AnalogueGain, context_.frameContext.agc.gain);
> +

I fear that the exposure and gain are now being reported /after running
the algorithms.

That means this metadata is going to report the 'desired' gain/exposure
that will be found in a frame about 2 or 3 frames from now.

While I think it /should/ be reporting what was given / determined to be
the 'input' exposure and gain that comes in.

This makes me go back to the earlier patch thinking that when the
exposure and gain are stored in 3/14 - they should be stored separately
as 'input' parameters.

Same questions now apply to the ExposureTime added from
context_.frameContext.agc.exposure in [13/14]

>         /*
>          * \todo We should be able to add 'anything' (with a Control) in here to
>          * get information to say.
> -- 
> 2.32.0
>