[libcamera-devel,v4,2/9] ipa: ipu3: Introduce a Context structure

13399 diff mbox series

An increasing amount of data and information needs to be shared between
the components that build up to implement image processing algorithms.

Create a context structure which will allow us to work towards calling
algorithms in a modular way, and sharing information between the modules.

The IPA context is a global context set at configure time (IPAConfiguration)
and a per-frame context (IPAFrameContext) used while streaming.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
---
 src/ipa/ipu3/ipa_context.h | 32 ++++++++++++++++++++++++++
 src/ipa/ipu3/ipu3.cpp      | 46 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+)
 create mode 100644 src/ipa/ipu3/ipa_context.h

On 19/08/2021 15:57, Jean-Michel Hautbois wrote:
> An increasing amount of data and information needs to be shared between
> the components that build up to implement image processing algorithms.
> 
> Create a context structure which will allow us to work towards calling
> algorithms in a modular way, and sharing information between the modules.
> 
> The IPA context is a global context set at configure time (IPAConfiguration)

This is now (IPASessionConfiguration)

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


> and a per-frame context (IPAFrameContext) used while streaming.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> ---
>  src/ipa/ipu3/ipa_context.h | 32 ++++++++++++++++++++++++++
>  src/ipa/ipu3/ipu3.cpp      | 46 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 78 insertions(+)
>  create mode 100644 src/ipa/ipu3/ipa_context.h
> 
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> new file mode 100644
> index 00000000..2706d3ca
> --- /dev/null
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * ipu3_ipa_context.h - IPU3 IPA Context
> + *
> + */
> +#ifndef __LIBCAMERA_IPU3_IPA_CONTEXT_H__
> +#define __LIBCAMERA_IPU3_IPA_CONTEXT_H__
> +
> +#include <linux/intel-ipu3.h>
> +
> +namespace libcamera {
> +
> +namespace ipa::ipu3 {
> +
> +struct IPASessionConfiguration {
> +};
> +
> +struct IPAFrameContext {
> +};
> +
> +struct IPAContext {
> +	IPASessionConfiguration configuration;
> +	IPAFrameContext frameContext;
> +};
> +
> +} /* namespace ipa::ipu3 */
> +
> +} /* namespace libcamera*/
> +
> +#endif /* __LIBCAMERA_IPU3_IPA_CONTEXT_H__ */
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index c34fa460..a4359d3e 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -36,6 +36,52 @@
>  static constexpr uint32_t kMaxCellWidthPerSet = 160;
>  static constexpr uint32_t kMaxCellHeightPerSet = 56;
>  
> +/**
> + * \file ipa_context.h
> + * \brief Context and state information shared between the algorithms
> + */
> +
> +/**
> + * \struct IPASessionConfiguration
> + * \brief Session configuration for the IPA module
> + *
> + * The session configuration contains all IPA configuration parameters that
> + * remain constant during the capture session, from IPA module start to stop.
> + * It is typically set during the configure() operation of the IPA module, but
> + * may also be updated in the start() operation.
> + */
> +
> +/**
> + * \struct IPAFrameContext
> + * \brief Per-frame context for algorithms
> + *
> + * 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 IPAContext structure.
> + *
> + * \todo Detail how to access contexts for a particular frame
> + *
> + * Each of the fields in the frame context belongs to either a specific
> + * algorithm, or to the top-level IPA module. A field may be read by any
> + * algorithm, but should only be written by its owner.
> + */
> +
> +/**
> + * \struct IPAContext
> + * \brief Global IPA context data shared between all algorithms
> + *
> + * \var IPAContext::configuration
> + * \brief The IPA session configuration, immutable during the session
> + *
> + * \var IPAContext::frameContext
> + * \brief The frame context for the frame being processed
> + *
> + * \todo While the frame context is supposed to be per-frame, this
> + * single frame context stores data related to both the current frame
> + * and the previous frames, with fields being updated as the algorithms
> + * are run. This needs to be turned into real per-frame data storage.
> + */
> +
>  namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(IPAIPU3)
>

Hi Jean-Michel,

Thank you for the patch.

On Thu, Aug 19, 2021 at 04:57:52PM +0200, Jean-Michel Hautbois wrote:
> An increasing amount of data and information needs to be shared between
> the components that build up to implement image processing algorithms.
> 
> Create a context structure which will allow us to work towards calling
> algorithms in a modular way, and sharing information between the modules.
> 
> The IPA context is a global context set at configure time (IPAConfiguration)

As Kieran mentioned, this is now called IPASessionConfiguration.

> and a per-frame context (IPAFrameContext) used while streaming.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> ---
>  src/ipa/ipu3/ipa_context.h | 32 ++++++++++++++++++++++++++
>  src/ipa/ipu3/ipu3.cpp      | 46 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 78 insertions(+)
>  create mode 100644 src/ipa/ipu3/ipa_context.h
> 
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> new file mode 100644
> index 00000000..2706d3ca
> --- /dev/null
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * ipu3_ipa_context.h - IPU3 IPA Context
> + *
> + */
> +#ifndef __LIBCAMERA_IPU3_IPA_CONTEXT_H__
> +#define __LIBCAMERA_IPU3_IPA_CONTEXT_H__
> +
> +#include <linux/intel-ipu3.h>
> +
> +namespace libcamera {
> +
> +namespace ipa::ipu3 {
> +
> +struct IPASessionConfiguration {
> +};
> +
> +struct IPAFrameContext {
> +};
> +
> +struct IPAContext {
> +	IPASessionConfiguration configuration;
> +	IPAFrameContext frameContext;
> +};
> +
> +} /* namespace ipa::ipu3 */
> +
> +} /* namespace libcamera*/
> +
> +#endif /* __LIBCAMERA_IPU3_IPA_CONTEXT_H__ */
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index c34fa460..a4359d3e 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -36,6 +36,52 @@
>  static constexpr uint32_t kMaxCellWidthPerSet = 160;
>  static constexpr uint32_t kMaxCellHeightPerSet = 56;

I'd move the documentation about those two lines.

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

>  
> +/**
> + * \file ipa_context.h
> + * \brief Context and state information shared between the algorithms
> + */
> +
> +/**
> + * \struct IPASessionConfiguration
> + * \brief Session configuration for the IPA module
> + *
> + * The session configuration contains all IPA configuration parameters that
> + * remain constant during the capture session, from IPA module start to stop.
> + * It is typically set during the configure() operation of the IPA module, but
> + * may also be updated in the start() operation.
> + */
> +
> +/**
> + * \struct IPAFrameContext
> + * \brief Per-frame context for algorithms
> + *
> + * 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 IPAContext structure.
> + *
> + * \todo Detail how to access contexts for a particular frame
> + *
> + * Each of the fields in the frame context belongs to either a specific
> + * algorithm, or to the top-level IPA module. A field may be read by any
> + * algorithm, but should only be written by its owner.
> + */
> +
> +/**
> + * \struct IPAContext
> + * \brief Global IPA context data shared between all algorithms
> + *
> + * \var IPAContext::configuration
> + * \brief The IPA session configuration, immutable during the session
> + *
> + * \var IPAContext::frameContext
> + * \brief The frame context for the frame being processed
> + *
> + * \todo While the frame context is supposed to be per-frame, this
> + * single frame context stores data related to both the current frame
> + * and the previous frames, with fields being updated as the algorithms
> + * are run. This needs to be turned into real per-frame data storage.
> + */
> +
>  namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(IPAIPU3)