[libcamera-devel] [PATCH v4 05/32] ipa: libipa: Provide a common base for frame contexts

[Kieran Bingham]

Quoting Laurent Pinchart (2022-09-08 02:41:33)
> From: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> Provide a common FrameContext as a base for IPA modules to inherit from.
> 
> This will allow having a common set of parameters for every frame
> context managed by the FCQueue implementation.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

Missing SoB.

> ---
> Changes since v3:
> 
> - Rename structure from IPAFrameContext to FrameContext
> - Move changes to IPA modules to separate patches
> - Make the FrameContext::frame member private
> ---
>  src/ipa/libipa/fc_queue.cpp | 18 ++++++++++++++++++
>  src/ipa/libipa/fc_queue.h   | 10 ++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp
> index b81d497e255a..8f61c85131c9 100644
> --- a/src/ipa/libipa/fc_queue.cpp
> +++ b/src/ipa/libipa/fc_queue.cpp
> @@ -20,6 +20,24 @@ namespace ipa {
>   * \brief Queue to access per-frame Context
>   */
>  
> +/**
> + * \struct FrameContext
> + * \brief Context for a frame
> + *
> + * The frame context stores data specific to a single frame processed by the
> + * IPA. Each frame processed by the IPA has a context associated with it,
> + * accessible through the Frame Context Queue.
> + *
> + * Fields in the frame context should reflect values and controls associated
> + * with the specific frame as requested by the application, and as configured by
> + * the hardware. Fields can be read by algorithms to determine if they should
> + * update any specific action for this frame, and finally to update the metadata
> + * control lists when the frame is fully completed.
> + *
> + * \var FrameContext::frame
> + * \brief The frame number
> + */
> +
>  /**
>   * \class FCQueue
>   * \brief A support class for queueing FrameContext instances through the IPA
> diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h
> index 0f3af0f3a189..d8a5b8297a70 100644
> --- a/src/ipa/libipa/fc_queue.h
> +++ b/src/ipa/libipa/fc_queue.h
> @@ -8,6 +8,7 @@
>  #pragma once
>  
>  #include <algorithm>
> +#include <stdint.h>
>  #include <vector>
>  
>  #include <libcamera/base/log.h>
> @@ -18,6 +19,15 @@ LOG_DECLARE_CATEGORY(FCQueue)
>  
>  namespace ipa {
>  
> +template<typename FrameContext>
> +class FCQueue;
> +
> +struct FrameContext {
> +private:
> +       template<typename T> friend class FCQueue;
> +       uint32_t frame;

Seems fine to me.

I'm still not sure what else is going to end up in here. There's a
chance that we might need to store the incoming Request ControlList (or
a copy of) ... so I still think there's sufficient requirement for the
common base type.


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> +};
> +
>  template<typename FrameContext>
>  class FCQueue
>  {
> -- 
> Regards,
> 
> Laurent Pinchart
>