[PATCH 1/4] libipa: FrameContext: Move init() to FrameContext

[Stefan Klug]

Hi Jacopo,

Thank you for the patch. 

On Wed, Oct 16, 2024 at 07:03:42PM +0200, Jacopo Mondi wrote:
> The FCtQueue structure initializes a new FrameContext using its init()
> function. In case of request underrun, where a FrameContext is
> initialized without application's controls being supplied, the
> FrameContext needs to be initialized to default values.
> 
> In order to allow the single IPAs to initialize a FrameContext to
> the desired default values, move the init() function to the FrameContext
> structure, which each IPA derives to a per-IPA type.
> 
> In this way each IPA can override the FrameContext::init() function
> and initialize the FrameContext to the desired default values.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Looks good to me.

Reviewed-by: Stefan Klug <stefan.klug at ideasonboard.com> 

Best regards,
Stefan

> ---
>  src/ipa/libipa/fc_queue.cpp    | 10 ++++++++++
>  src/ipa/libipa/fc_queue.h      | 16 ++++++++--------
>  src/ipa/rkisp1/ipa_context.cpp |  5 +++++
>  src/ipa/rkisp1/ipa_context.h   |  2 ++
>  4 files changed, 25 insertions(+), 8 deletions(-)
> 
> diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp
> index 0365e9197748..fa2454fd5706 100644
> --- a/src/ipa/libipa/fc_queue.cpp
> +++ b/src/ipa/libipa/fc_queue.cpp
> @@ -38,6 +38,16 @@ namespace ipa {
>   * \brief The frame number
>   */
>  
> +/**
> + * \fn FrameContext::init()
> + * \brief Initialize a frame context
> + * \param[in] frameNum The frame number to assign to this FrameContext
> + *
> + * This function initializes a frame context by assigning it a frame number.
> + * The single IPA modules are expected to override this function to initialize
> + * their derived FrameContext implementation to their desired default values.
> + */
> +
>  /**
>   * \class FCQueue
>   * \brief A support class for managing FrameContext instances in IPA modules
> diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h
> index 24d9e82b727d..b1e8bc1485d4 100644
> --- a/src/ipa/libipa/fc_queue.h
> +++ b/src/ipa/libipa/fc_queue.h
> @@ -22,6 +22,12 @@ template<typename FrameContext>
>  class FCQueue;
>  
>  struct FrameContext {
> +protected:
> +	virtual void init(const uint32_t frameNum)
> +	{
> +		frame = frameNum;
> +	}
> +
>  private:
>  	template<typename T> friend class FCQueue;
>  	uint32_t frame;
> @@ -61,7 +67,7 @@ public:
>  			LOG(FCQueue, Warning)
>  				<< "Frame " << frame << " already initialised";
>  		else
> -			init(frameContext, frame);
> +			frameContext.init(frame);
>  
>  		return frameContext;
>  	}
> @@ -98,18 +104,12 @@ public:
>  		LOG(FCQueue, Warning)
>  			<< "Obtained an uninitialised FrameContext for " << frame;
>  
> -		init(frameContext, frame);
> +		frameContext.init(frame);
>  
>  		return frameContext;
>  	}
>  
>  private:
> -	void init(FrameContext &frameContext, const uint32_t frame)
> -	{
> -		frameContext = {};
> -		frameContext.frame = frame;
> -	}
> -
>  	std::vector<FrameContext> contexts_;
>  };
>  
> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> index 14d0c02a2b32..4e4fe5f4ae96 100644
> --- a/src/ipa/rkisp1/ipa_context.cpp
> +++ b/src/ipa/rkisp1/ipa_context.cpp
> @@ -417,6 +417,11 @@ namespace libcamera::ipa::rkisp1 {
>   * \brief Analogue gain multiplier
>   */
>  
> +void IPAFrameContext::init(const uint32_t frameNum)
> +{
> +	FrameContext::init(frameNum);
> +}
> +
>  /**
>   * \struct IPAContext
>   * \brief Global IPA context data shared between all algorithms
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index e274d9b01e1c..51e04bc30627 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -177,6 +177,8 @@ struct IPAFrameContext : public FrameContext {
>  	struct {
>  		Matrix<float, 3, 3> ccm;
>  	} ccm;
> +
> +	void init(const uint32_t frame) override;
>  };
>  
>  struct IPAContext {
> -- 
> 2.47.0
>