[libcamera-devel] [PATCH v4 01/12] ipa: libipa: Introduce a Module class template

paul.elder at ideasonboard.com paul.elder at ideasonboard.com
Thu Jun 23 10:55:00 CEST 2022


Hi Laurent,

On Mon, Jun 20, 2022 at 04:42:54AM +0300, Laurent Pinchart wrote:
> libipa defines an abstract Algorithm class template that is specialized
> by IPA modules. IPA modules then instantiate and manage algorithms
> internally, without help from libipa. With ongoing work on tuning data
> support for the RkISP1, and future similar work for the IPU3, more code
> duplication for algorithms management is expected.
> 
> To address this and share code between multiple IPA modules, introduce a
> new Module class template that will define and manage top-level concepts
> for the IPA module.
> 
> The Module class template needs to be specialized with the same types as
> the Algorithm class. To avoid manual specialization of both classes,
> store the types in the Module class, and replace the template arguments
> of the Algorithm class with a single Module argument from which the
> other types are retrieved.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>

> ---
>  src/ipa/ipu3/algorithms/algorithm.h   |  8 +--
>  src/ipa/ipu3/module.h                 | 27 ++++++++++
>  src/ipa/libipa/algorithm.cpp          | 17 ++++---
>  src/ipa/libipa/algorithm.h            | 17 +++----
>  src/ipa/libipa/meson.build            |  5 +-
>  src/ipa/libipa/module.cpp             | 73 +++++++++++++++++++++++++++
>  src/ipa/libipa/module.h               | 30 +++++++++++
>  src/ipa/rkisp1/algorithms/algorithm.h | 10 +---
>  src/ipa/rkisp1/module.h               | 27 ++++++++++
>  9 files changed, 183 insertions(+), 31 deletions(-)
>  create mode 100644 src/ipa/ipu3/module.h
>  create mode 100644 src/ipa/libipa/module.cpp
>  create mode 100644 src/ipa/libipa/module.h
>  create mode 100644 src/ipa/rkisp1/module.h
> 
> diff --git a/src/ipa/ipu3/algorithms/algorithm.h b/src/ipa/ipu3/algorithms/algorithm.h
> index 234b2bd77f72..ae134a9404fe 100644
> --- a/src/ipa/ipu3/algorithms/algorithm.h
> +++ b/src/ipa/ipu3/algorithms/algorithm.h
> @@ -7,19 +7,15 @@
>  
>  #pragma once
>  
> -#include <libcamera/ipa/ipu3_ipa_interface.h>
> -
>  #include <libipa/algorithm.h>
>  
> -#include "ipa_context.h"
> +#include "module.h"
>  
>  namespace libcamera {
>  
>  namespace ipa::ipu3 {
>  
> -using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAFrameContext,
> -					    IPAConfigInfo, ipu3_uapi_params,
> -					    ipu3_uapi_stats_3a>;
> +using Algorithm = libcamera::ipa::Algorithm<Module>;
>  
>  } /* namespace ipa::ipu3 */
>  
> diff --git a/src/ipa/ipu3/module.h b/src/ipa/ipu3/module.h
> new file mode 100644
> index 000000000000..d94fc4594871
> --- /dev/null
> +++ b/src/ipa/ipu3/module.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2022, Ideas On Board
> + *
> + * module.h - IPU3 IPA Module
> + */
> +
> +#pragma once
> +
> +#include <linux/intel-ipu3.h>
> +
> +#include <libcamera/ipa/ipu3_ipa_interface.h>
> +
> +#include <libipa/module.h>
> +
> +#include "ipa_context.h"
> +
> +namespace libcamera {
> +
> +namespace ipa::ipu3 {
> +
> +using Module = ipa::Module<IPAContext, IPAFrameContext, IPAConfigInfo,
> +			   ipu3_uapi_params, ipu3_uapi_stats_3a>;
> +
> +} /* namespace ipa::ipu3 */
> +
> +} /* namespace libcamera*/
> diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp
> index cce2ed62986d..2df91e5d8fed 100644
> --- a/src/ipa/libipa/algorithm.cpp
> +++ b/src/ipa/libipa/algorithm.cpp
> @@ -19,14 +19,17 @@ namespace ipa {
>  /**
>   * \class Algorithm
>   * \brief The base class for all IPA algorithms
> - * \tparam Context The type of shared IPA context
> - * \tparam Config The type of the IPA configuration data
> - * \tparam Params The type of the ISP specific parameters
> - * \tparam Stats The type of the IPA statistics and ISP results
> + * \tparam Module The IPA module type for this class of algorithms
>   *
> - * The Algorithm class defines a standard interface for IPA algorithms. By
> - * abstracting algorithms, it makes possible the implementation of generic code
> - * to manage algorithms regardless of their specific type.
> + * The Algorithm class defines a standard interface for IPA algorithms
> + * compatible with the \a Module. By abstracting algorithms, it makes possible
> + * the implementation of generic code to manage algorithms regardless of their
> + * specific type.
> + *
> + * To specialize the Algorithm class template, an IPA module shall specialize
> + * the Module class template with module-specific context and configuration
> + * types, and pass the specialized Module class as the \a Module template
> + * argument.
>   */
>  
>  /**
> diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h
> index 032a05b56635..fd2ffcfbc900 100644
> --- a/src/ipa/libipa/algorithm.h
> +++ b/src/ipa/libipa/algorithm.h
> @@ -10,27 +10,26 @@ namespace libcamera {
>  
>  namespace ipa {
>  
> -template<typename Context, typename FrameContext, typename Config,
> -	 typename Params, typename Stats>
> +template<typename Module>
>  class Algorithm
>  {
>  public:
>  	virtual ~Algorithm() {}
>  
> -	virtual int configure([[maybe_unused]] Context &context,
> -			      [[maybe_unused]] const Config &configInfo)
> +	virtual int configure([[maybe_unused]] typename Module::Context &context,
> +			      [[maybe_unused]] const typename Module::Config &configInfo)
>  	{
>  		return 0;
>  	}
>  
> -	virtual void prepare([[maybe_unused]] Context &context,
> -			     [[maybe_unused]] Params *params)
> +	virtual void prepare([[maybe_unused]] typename Module::Context &context,
> +			     [[maybe_unused]] typename Module::Params *params)
>  	{
>  	}
>  
> -	virtual void process([[maybe_unused]] Context &context,
> -			     [[maybe_unused]] FrameContext *frameContext,
> -			     [[maybe_unused]] const Stats *stats)
> +	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/meson.build b/src/ipa/libipa/meson.build
> index 161cc5a1f0d0..465cf7d6c4a7 100644
> --- a/src/ipa/libipa/meson.build
> +++ b/src/ipa/libipa/meson.build
> @@ -3,13 +3,16 @@
>  libipa_headers = files([
>      'algorithm.h',
>      'camera_sensor_helper.h',
> -    'histogram.h'
> +    'histogram.h',
> +    'module.h',
>  ])
>  
>  libipa_sources = files([
> +    'algorithm.cpp',
>      'camera_sensor_helper.cpp',
>      'histogram.cpp',
>      'libipa.cpp',
> +    'module.cpp',
>  ])
>  
>  libipa_includes = include_directories('..')
> diff --git a/src/ipa/libipa/module.cpp b/src/ipa/libipa/module.cpp
> new file mode 100644
> index 000000000000..5a6f49a80e6d
> --- /dev/null
> +++ b/src/ipa/libipa/module.cpp
> @@ -0,0 +1,73 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2022, Ideas On Board
> + *
> + * module.cpp - IPA Module
> + */
> +
> +#include "module.h"
> +
> +/**
> + * \file module.h
> + * \brief IPA Module common interface
> + */
> +
> +namespace libcamera {
> +
> +namespace ipa {
> +
> +/**
> + * \class Module
> + * \brief The base class for all IPA modules
> + * \tparam Context The type of the shared IPA context
> + * \tparam FrameContext The type of the frame context
> + * \tparam Config The type of the IPA configuration data
> + * \tparam Params The type of the ISP specific parameters
> + * \tparam Stats The type of the IPA statistics and ISP results
> + *
> + * The Module class template defines a standard internal interface between IPA
> + * modules and libipa.
> + *
> + * While IPA modules are platform-specific, many of their internal functions are
> + * conceptually similar, even if they take different types of platform-specifc
> + * parameters. For instance, IPA modules could share code that instantiates,
> + * initializes and run algorithms if it wasn't for the fact that the the format
> + * of ISP parameters or statistics passed to the related functions is
> + * device-dependent.
> + *
> + * To enable a shared implementation of those common tasks in libipa, the Module
> + * class template defines a standard internal interface between IPA modules and
> + * libipa. The template parameters specify the types of module-dependent data.
> + * IPA modules shall create a specialization of the Module class template in
> + * their namespace, and use it to specialize other classes of libipa, such as
> + * the Algorithm class.
> + */
> +
> +/**
> + * \typedef Module::Context
> + * \brief The type of the shared IPA context
> + */
> +
> +/**
> + * \typedef Module::FrameContext
> + * \brief The type of the frame context
> + */
> +
> +/**
> + * \typedef Module::Config
> + * \brief The type of the IPA configuration data
> + */
> +
> +/**
> + * \typedef Module::Params
> + * \brief The type of the ISP specific parameters
> + */
> +
> +/**
> + * \typedef Module::Stats
> + * \brief The type of the IPA statistics and ISP results
> + */
> +
> +} /* namespace ipa */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/libipa/module.h b/src/ipa/libipa/module.h
> new file mode 100644
> index 000000000000..c4d778120408
> --- /dev/null
> +++ b/src/ipa/libipa/module.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2022, Ideas On Board
> + *
> + * module.h - IPA module
> + */
> +
> +#pragma once
> +
> +namespace libcamera {
> +
> +namespace ipa {
> +
> +template<typename _Context, typename _FrameContext, typename _Config,
> +	 typename _Params, typename _Stats>
> +class Module
> +{
> +public:
> +	using Context = _Context;
> +	using FrameContext = _FrameContext;
> +	using Config = _Config;
> +	using Params = _Params;
> +	using Stats = _Stats;
> +
> +	virtual ~Module() {}
> +};
> +
> +} /* namespace ipa */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/rkisp1/algorithms/algorithm.h b/src/ipa/rkisp1/algorithms/algorithm.h
> index 68e3a44e19b4..c3212cff76fe 100644
> --- a/src/ipa/rkisp1/algorithms/algorithm.h
> +++ b/src/ipa/rkisp1/algorithms/algorithm.h
> @@ -7,21 +7,15 @@
>  
>  #pragma once
>  
> -#include <linux/rkisp1-config.h>
> -
> -#include <libcamera/ipa/rkisp1_ipa_interface.h>
> -
>  #include <libipa/algorithm.h>
>  
> -#include "ipa_context.h"
> +#include "module.h"
>  
>  namespace libcamera {
>  
>  namespace ipa::rkisp1 {
>  
> -using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAFrameContext,
> -					    IPACameraSensorInfo, rkisp1_params_cfg,
> -					    rkisp1_stat_buffer>;
> +using Algorithm = libcamera::ipa::Algorithm<Module>;
>  
>  } /* namespace ipa::rkisp1 */
>  
> diff --git a/src/ipa/rkisp1/module.h b/src/ipa/rkisp1/module.h
> new file mode 100644
> index 000000000000..89f83208a75c
> --- /dev/null
> +++ b/src/ipa/rkisp1/module.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2022, Ideas On Board
> + *
> + * module.h - RkISP1 IPA Module
> + */
> +
> +#pragma once
> +
> +#include <linux/rkisp1-config.h>
> +
> +#include <libcamera/ipa/rkisp1_ipa_interface.h>
> +
> +#include <libipa/module.h>
> +
> +#include "ipa_context.h"
> +
> +namespace libcamera {
> +
> +namespace ipa::rkisp1 {
> +
> +using Module = ipa::Module<IPAContext, IPAFrameContext, IPACameraSensorInfo,
> +			   rkisp1_params_cfg, rkisp1_stat_buffer>;
> +
> +} /* namespace ipa::rkisp1 */
> +
> +} /* namespace libcamera*/


More information about the libcamera-devel mailing list