[libcamera-devel] [libcamera-devel 2/5] ipa: libipa: Add init() function to the Algorithm class

[Laurent Pinchart]

Hi Florian,

Thank you for the patch.

By the way, you don't need to add "libcamera-devel" manually to the
subject line, the [libcamera-devel] tag is added by the mailing list.

On Mon, May 23, 2022 at 11:24:32AM +0200, Florian Sylvestre via libcamera-devel wrote:
> Add the init() function that will be called during algorithm initialization
> to provide each algorithm the list of algorithms tuning data.
> Each algorithm will be responsible to grab their corresponding parameters.
> 
> Signed-off-by: Florian Sylvestre <fsylvestre at baylibre.com>
> ---
>  src/ipa/libipa/algorithm.cpp | 15 +++++++++++++++
>  src/ipa/libipa/algorithm.h   | 10 +++++++++-
>  2 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp
> index 398d5372..269a4beb 100644
> --- a/src/ipa/libipa/algorithm.cpp
> +++ b/src/ipa/libipa/algorithm.cpp
> @@ -29,6 +29,21 @@ namespace ipa {
>   * to manage algorithms regardless of their specific type.
>   */
>  
> +/**
> + * \fn Algorithm::init()
> + * \brief Configure the Algorithm with default parameters
> + * \param[in] context The shared IPA context
> + * \param[in] params The initial parameters used to tune algorithms
> + *
> + * This function is called once before the camera is running to get default
> + * algorithm parameters.

This may be understood as meaning it's called once every time the camera
is started. I'd make this a bit clearer:

 * This function is called once, when the IPA module is initialized, to
 * initialize the algorithm. The \a params YamlObject contains IPA module
 * parameters, typically tuning data for all algorithms. The Algorithm is
 * responsible for reading the parameters relevant to its configuration.

> + *
> + * Algorithms are responsible to read the parameters given and extract their
> + * parameter configuration.
> + *
> + * \return 0 if successful, an error code otherwise
> + */
> +
>  /**
>   * \fn Algorithm::configure()
>   * \brief Configure the Algorithm given an IPAConfigInfo
> diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h
> index 766aee5d..f5be1caf 100644
> --- a/src/ipa/libipa/algorithm.h
> +++ b/src/ipa/libipa/algorithm.h
> @@ -10,12 +10,20 @@ namespace libcamera {
>  
>  namespace ipa {
>  
> -template<typename Context, typename Config, typename Params, typename Stats>
> +template<typename Context, typename TuningData,

As we're trying to push for YAML usage everywhere through libcamera, I'm
tempted to drop the additional template argument and use YamlObject
unconditionally in init() instead of TuningData.

Does anyone have an opinion on this ?

> +	 typename Config, typename Params, typename Stats>
> +
>  class Algorithm
>  {
>  public:
>  	virtual ~Algorithm() {}
>  
> +	virtual int init([[maybe_unused]] Context &context,
> +			 [[maybe_unused]] const TuningData *params)
> +	{
> +		return 0;
> +	}
> +
>  	virtual int configure([[maybe_unused]] Context &context,
>  			      [[maybe_unused]] const Config &configInfo)
>  	{

-- 
Regards,

Laurent Pinchart