[libcamera-devel] [PATCH v1 4/7] ipa: ipu3: use process method for all algorithms

[Kieran Bingham]

Hi JM,

On 28/06/2021 21:22, Jean-Michel Hautbois wrote:
> The main goal will be to have the same process() prototype for all
> algorithms, each of them would then grab the values needed using a
> metadata exchange way (for instance, current analogue gain, shutter time,
> or red/blue gains calculated in the AWB algorithm from AGC, etc.).

We need to document how algorithms should operate, what is the expected
flow and interactions and how does the IPA function overall.

As part of introducing this new method - I would expect to see updates
to the documentation as to how the new API change is expected to be
used, but we don't have /any/ documentation yet.

The goal is to define this in a way that describes how any algorithm
which is used by libipa

Ideally, the documentation should describe the design of how algorithms
are called and expected to operate.

We have a libipa/algorithm.h which is where I would probably expect to
see this documentation.

Which also infers that this new ->process() call should be a member of
the base algorithm class?

But - we haven't been able to actually use the base class due to these
methods directly specifying / referencing IPU3 specific parameters.


That means that we can't have generic documentation for algorithms in
libipa - but we should have something for the IPU3 directly in that case.

Ideally, then this documentation can be moved or used as part of the
generic libipa abstractions.



> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> ---
>  src/ipa/ipu3/ipu3.cpp     | 2 +-
>  src/ipa/ipu3/ipu3_awb.cpp | 5 +++++
>  src/ipa/ipu3/ipu3_awb.h   | 3 ++-
>  3 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index f43f8620..4466391a 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -296,7 +296,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>  	agcAlgo_->process(stats, exposure_, gain);
>  	gain_ = camHelper_->gainCode(gain);
>  
> -	awbAlgo_->calculateWBGains(stats);
> +	awbAlgo_->process(stats);
>  
>  	if (agcAlgo_->updateControls())
>  		setControls(frame);
> diff --git a/src/ipa/ipu3/ipu3_awb.cpp b/src/ipa/ipu3/ipu3_awb.cpp
> index 9b409c8f..a94935c5 100644
> --- a/src/ipa/ipu3/ipu3_awb.cpp
> +++ b/src/ipa/ipu3/ipu3_awb.cpp
> @@ -351,6 +351,11 @@ void IPU3Awb::calculateWBGains(const ipu3_uapi_stats_3a *stats)
>  	}
>  }
>  
> +void IPU3Awb::process(const ipu3_uapi_stats_3a *stats)
> +{
> +	calculateWBGains(stats);
> +}
> +
>  void IPU3Awb::updateWbParameters(ipu3_uapi_params &params, double agcGamma)
>  {
>  	/*
> diff --git a/src/ipa/ipu3/ipu3_awb.h b/src/ipa/ipu3/ipu3_awb.h
> index f4100f4a..795e32e3 100644
> --- a/src/ipa/ipu3/ipu3_awb.h
> +++ b/src/ipa/ipu3/ipu3_awb.h
> @@ -33,7 +33,7 @@ public:
>  	~IPU3Awb();
>  
>  	void initialise(ipu3_uapi_params &params, const Size &bdsOutputSize, struct ipu3_uapi_grid_config &bdsGrid);
> -	void calculateWBGains(const ipu3_uapi_stats_3a *stats);
> +	void process(const ipu3_uapi_stats_3a *stats);
>  	void updateWbParameters(ipu3_uapi_params &params, double agcGamma);
>  
>  private:
> @@ -42,6 +42,7 @@ private:
>  	void clearAwbStats();
>  	void awbGreyWorld();
>  	uint32_t estimateCCT(double red, double green, double blue);
> +	void calculateWBGains(const ipu3_uapi_stats_3a *stats);
>  
>  	struct ipu3_uapi_grid_config awbGrid_;
>  
>