[libcamera-devel] [PATCH v4 16/32] ipa: rkisp1: Convert to use the FCQueue

[Jacopo Mondi]

Hi Laurent

On Thu, Sep 08, 2022 at 04:41:44AM +0300, Laurent Pinchart via libcamera-devel wrote:
> From: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> Establish a queue of FrameContexts using the new FCQueue and use it to
> supply the FrameContext to the algorithms.
>
> The algorithms on the RKISP1 do not use this yet themselves, but are
> able to do so after the introduction of this patch.
>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

> ---
>  src/ipa/rkisp1/ipa_context.cpp |  6 +++---
>  src/ipa/rkisp1/ipa_context.h   |  2 ++
>  src/ipa/rkisp1/rkisp1.cpp      | 34 ++++++++++++++++++++++++----------
>  3 files changed, 29 insertions(+), 13 deletions(-)
>
> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> index d18f4996aad2..e9846742ee4f 100644
> --- a/src/ipa/rkisp1/ipa_context.cpp
> +++ b/src/ipa/rkisp1/ipa_context.cpp
> @@ -205,8 +205,7 @@ namespace libcamera::ipa::rkisp1 {
>   * \struct IPAFrameContext
>   * \brief Per-frame context for algorithms
>   *
> - * This structure is currently unused and will be replaced by a real per-frame
> - * context.
> + * \todo Populate the frame context for all algorithms
>   */
>
>  /**
> @@ -219,7 +218,8 @@ namespace libcamera::ipa::rkisp1 {
>   * \var IPAContext::activeState
>   * \brief The IPA active state, storing the latest state for all algorithms
>   *
> - * \todo Introduce per-frame contexts
> + * \var IPAContext::frameContexts
> + * \brief Ring buffer of per-frame contexts
>   */
>
>  } /* namespace libcamera::ipa::rkisp1 */
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index f6b3e6eb951c..f6aaefffed52 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -97,6 +97,8 @@ struct IPAFrameContext : public FrameContext {
>  struct IPAContext {
>  	IPASessionConfiguration configuration;
>  	IPAActiveState activeState;
> +
> +	FCQueue<IPAFrameContext> frameContexts;
>  };
>
>  } /* namespace ipa::rkisp1 */
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 24d5b9647838..c5ed0bb21f67 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -40,13 +40,18 @@ using namespace std::literals::chrono_literals;
>
>  namespace ipa::rkisp1 {
>
> +/* Maximum number of frame contexts to be held */
> +static constexpr uint32_t kMaxFrameContexts = 16;
> +
>  class IPARkISP1 : public IPARkISP1Interface, public Module
>  {
>  public:
> +	IPARkISP1();
> +
>  	int init(const IPASettings &settings, unsigned int hwRevision,
>  		 ControlInfoMap *ipaControls) override;
>  	int start() override;
> -	void stop() override {}
> +	void stop() override;
>
>  	int configure(const IPACameraSensorInfo &info,
>  		      const std::map<uint32_t, IPAStream> &streamConfig,
> @@ -100,6 +105,11 @@ const ControlInfoMap::Map rkisp1Controls{
>
>  } /* namespace */
>
> +IPARkISP1::IPARkISP1()
> +	: context_({ {}, {}, { kMaxFrameContexts } })
> +{
> +}
> +
>  std::string IPARkISP1::logPrefix() const
>  {
>  	return "rkisp1";
> @@ -186,6 +196,11 @@ int IPARkISP1::start()
>  	return 0;
>  }
>
> +void IPARkISP1::stop()
> +{
> +	context_.frameContexts.clear();
> +}
> +
>  /**
>   * \todo The RkISP1 pipeline currently provides an empty IPACameraSensorInfo
>   * if the connected sensor does not provide enough information to properly
> @@ -223,8 +238,10 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,
>  		<< "Exposure: " << minExposure << "-" << maxExposure
>  		<< " Gain: " << minGain << "-" << maxGain;
>
> -	/* Clean context at configuration */
> -	context_ = {};
> +	/* Clear the IPA context before the streaming session. */
> +	context_.configuration = {};
> +	context_.activeState = {};
> +	context_.frameContexts.clear();
>
>  	/* Set the hardware revision for the algorithms. */
>  	context_.configuration.hw.revision = hwRevision_;
> @@ -287,8 +304,7 @@ void IPARkISP1::unmapBuffers(const std::vector<unsigned int> &ids)
>
>  void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls)
>  {
> -	/* \todo Obtain the frame context to pass to process from the FCQueue */
> -	IPAFrameContext frameContext;
> +	IPAFrameContext &frameContext = context_.frameContexts.init(frame);
>
>  	for (auto const &algo : algorithms())
>  		algo->queueRequest(context_, frame, frameContext, controls);
> @@ -296,8 +312,7 @@ void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls)
>
>  void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)
>  {
> -	/* \todo Obtain the frame context to pass to process from the FCQueue */
> -	IPAFrameContext frameContext;
> +	IPAFrameContext &frameContext = context_.frameContexts.get(frame);
>
>  	rkisp1_params_cfg *params =
>  		reinterpret_cast<rkisp1_params_cfg *>(
> @@ -316,6 +331,8 @@ void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)
>  void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId,
>  				   const ControlList &sensorControls)
>  {
> +	IPAFrameContext &frameContext = context_.frameContexts.get(frame);
> +
>  	const rkisp1_stat_buffer *stats =
>  		reinterpret_cast<rkisp1_stat_buffer *>(
>  			mappedBuffers_.at(bufferId).planes()[0].data());
> @@ -327,9 +344,6 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId
>
>  	unsigned int aeState = 0;
>
> -	/* \todo Obtain the frame context to pass to process from the FCQueue */
> -	IPAFrameContext frameContext;
> -
>  	for (auto const &algo : algorithms())
>  		algo->process(context_, frame, frameContext, stats);
>
> --
> Regards,
>
> Laurent Pinchart
>