[libcamera-devel] [PATCH 1/4] ipa: libipa: algorithm: Add queueRequest() to the Algorithm class

[Laurent Pinchart]

Hi Florian,

Thank you for the patch.

On Mon, Jul 04, 2022 at 05:23:15PM +0200, Florian Sylvestre via libcamera-devel wrote:
> Add queueRequest() function to the Algorithm class. The queueRequest() function
> provides controls values coming from the application to each algorithm.
> Each algorithm is responsible for retrieving the controls associated to them.
> 
> Signed-off-by: Florian Sylvestre <fsylvestre at baylibre.com>
> ---
>  src/ipa/ipu3/module.h        |  2 +-
>  src/ipa/libipa/algorithm.cpp | 13 +++++++++++++
>  src/ipa/libipa/algorithm.h   |  6 ++++++
>  src/ipa/libipa/module.cpp    |  5 +++++
>  src/ipa/libipa/module.h      |  3 ++-
>  src/ipa/rkisp1/module.h      |  2 +-
>  6 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/src/ipa/ipu3/module.h b/src/ipa/ipu3/module.h
> index d94fc459..5c2179e0 100644
> --- a/src/ipa/ipu3/module.h
> +++ b/src/ipa/ipu3/module.h
> @@ -20,7 +20,7 @@ namespace libcamera {
>  namespace ipa::ipu3 {
>  
>  using Module = ipa::Module<IPAContext, IPAFrameContext, IPAConfigInfo,
> -			   ipu3_uapi_params, ipu3_uapi_stats_3a>;
> +			   ipu3_uapi_params, ipu3_uapi_stats_3a, ControlList>;
>  
>  } /* namespace ipa::ipu3 */
>  
> diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp
> index 8549fe3f..1fb811ef 100644
> --- a/src/ipa/libipa/algorithm.cpp
> +++ b/src/ipa/libipa/algorithm.cpp
> @@ -81,6 +81,19 @@ namespace ipa {
>   * includes setting fields and flags that enable those processing blocks.
>   */
>  
> +/**
> + * \fn Algorithm::queueRequest()
> + * \brief Provide control values to the algorithm
> + * \param[in] context The shared IPA context
> + * \param[in] frame The frame number to aplly the control values

s/aplly/apply/

> + * \param[in] controls The list of user controls
> + *
> + * This function provides controls values coming from the application to the
> + * algorithm. A frame number is provided to indicate the concerned frame.
> + * Each algorithm is responsible for retrieving the controls associated to
> + * them.

Let's also mention when this is called:

 * This function is called for each request queued to the camera. It provides
 * the controls stored in the request to the algorithm. The \a frame number
 * identifies the corresponding frame.
 *
 * Algorithms shall read the applicable controls and store their value for later
 * use during frame processing.

> + */
> +
>  /**
>   * \fn Algorithm::process()
>   * \brief Process ISP statistics, and run algorithm operations
> diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h
> index 2a8871d8..aa846625 100644
> --- a/src/ipa/libipa/algorithm.h
> +++ b/src/ipa/libipa/algorithm.h
> @@ -40,6 +40,12 @@ public:
>  	{
>  	}
>  
> +	virtual void queueRequest([[maybe_unused]] typename Module::Context &context,
> +				  [[maybe_unused]] const uint32_t frame,
> +				  [[maybe_unused]] const typename Module::ControlList &controls)
> +	{
> +	}
> +
>  	virtual void process([[maybe_unused]] typename Module::Context &context,
>  			     [[maybe_unused]] typename Module::FrameContext *frameContext,
>  			     [[maybe_unused]] const typename Module::Stats *stats)
> diff --git a/src/ipa/libipa/module.cpp b/src/ipa/libipa/module.cpp
> index 77352104..e87a52fc 100644
> --- a/src/ipa/libipa/module.cpp
> +++ b/src/ipa/libipa/module.cpp
> @@ -77,6 +77,11 @@ namespace ipa {
>   * \brief The type of the IPA statistics and ISP results
>   */
>  
> +/**
> + * \typedef Module::ControlList
> + * \brief The type of the ISP runtime controls list
> + */
> +
>  /**
>   * \fn Module::algorithms()
>   * \brief Retrieve the list of instantiated algorithms
> diff --git a/src/ipa/libipa/module.h b/src/ipa/libipa/module.h
> index 4149a353..81d3bf7f 100644
> --- a/src/ipa/libipa/module.h
> +++ b/src/ipa/libipa/module.h
> @@ -26,7 +26,7 @@ LOG_DECLARE_CATEGORY(IPAModuleAlgo)
>  namespace ipa {
>  
>  template<typename _Context, typename _FrameContext, typename _Config,
> -	 typename _Params, typename _Stats>
> +	 typename _Params, typename _Stats, typename _ControlList>

I don't expect usage of a different container than
libcamera::ControlList for controls, so this can be hardcoded. No need
to change the template parameters to the Module class, just use
ControlList directly in queueRequest() instead of Module::ControlList.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

>  class Module : public Loggable
>  {
>  public:
> @@ -35,6 +35,7 @@ public:
>  	using Config = _Config;
>  	using Params = _Params;
>  	using Stats = _Stats;
> +	using ControlList = _ControlList;
>  
>  	virtual ~Module() {}
>  
> diff --git a/src/ipa/rkisp1/module.h b/src/ipa/rkisp1/module.h
> index 89f83208..08381a08 100644
> --- a/src/ipa/rkisp1/module.h
> +++ b/src/ipa/rkisp1/module.h
> @@ -20,7 +20,7 @@ namespace libcamera {
>  namespace ipa::rkisp1 {
>  
>  using Module = ipa::Module<IPAContext, IPAFrameContext, IPACameraSensorInfo,
> -			   rkisp1_params_cfg, rkisp1_stat_buffer>;
> +			   rkisp1_params_cfg, rkisp1_stat_buffer, ControlList>;
>  
>  } /* namespace ipa::rkisp1 */
>  

-- 
Regards,

Laurent Pinchart