[libcamera-devel] [PATCH v2 06/11] ipa: libipa: Introduce Algorithm class template
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Nov 23 11:02:03 CET 2021
Hi Jean-Michel,
Thank you for the patch.
On Tue, Nov 23, 2021 at 10:14:46AM +0100, Jean-Michel Hautbois wrote:
> The algorithms are using the same function names with specialized
> parameters. Instead of duplicating code, introduce a libipa Algorithm
> class which implements a base class with template parameters in libipa,
> and use it in each IPA.
>
> As we now won't need an algorithm class for each IPA, move the
> documentation to libipa, and make it agnostic of the IPA used.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> ---
> src/ipa/ipu3/algorithms/algorithm.h | 12 ++----
> src/ipa/ipu3/algorithms/meson.build | 1 -
> .../{ipu3/algorithms => libipa}/algorithm.cpp | 35 ++++++----------
> src/ipa/libipa/algorithm.h | 41 +++++++++++++++++++
> src/ipa/libipa/meson.build | 1 +
> src/ipa/rkisp1/algorithms/algorithm.h | 28 +++++++++++++
> src/ipa/rkisp1/algorithms/meson.build | 4 ++
> src/ipa/rkisp1/meson.build | 4 ++
> src/ipa/rkisp1/rkisp1.cpp | 5 ++-
Shouldn't this have been split in two, with the introduction of
src/ipa/libipa/algorithm.h and porting of IPU3 first, and then the
rkisp1 additions ?
> 9 files changed, 97 insertions(+), 34 deletions(-)
> rename src/ipa/{ipu3/algorithms => libipa}/algorithm.cpp (74%)
> create mode 100644 src/ipa/libipa/algorithm.h
> create mode 100644 src/ipa/rkisp1/algorithms/algorithm.h
> create mode 100644 src/ipa/rkisp1/algorithms/meson.build
>
> diff --git a/src/ipa/ipu3/algorithms/algorithm.h b/src/ipa/ipu3/algorithms/algorithm.h
> index 43f5d8b0..3c0ce461 100644
> --- a/src/ipa/ipu3/algorithms/algorithm.h
> +++ b/src/ipa/ipu3/algorithms/algorithm.h
> @@ -9,21 +9,15 @@
>
> #include <libcamera/ipa/ipu3_ipa_interface.h>
>
> +#include <libipa/algorithm.h>
> +
> #include "ipa_context.h"
>
> namespace libcamera {
>
> namespace ipa::ipu3 {
>
> -class Algorithm
> -{
> -public:
> - virtual ~Algorithm() {}
> -
> - virtual int configure(IPAContext &context, const IPAConfigInfo &configInfo);
> - virtual void prepare(IPAContext &context, ipu3_uapi_params *params);
> - virtual void process(IPAContext &context, const ipu3_uapi_stats_3a *stats);
> -};
> +using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPAConfigInfo, ipu3_uapi_params, ipu3_uapi_stats_3a>;
>
> } /* namespace ipa::ipu3 */
>
> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build
> index 3ec42f72..4db6ae1d 100644
> --- a/src/ipa/ipu3/algorithms/meson.build
> +++ b/src/ipa/ipu3/algorithms/meson.build
> @@ -2,7 +2,6 @@
>
> ipu3_ipa_algorithms = files([
> 'agc.cpp',
> - 'algorithm.cpp',
> 'awb.cpp',
> 'blc.cpp',
> 'tone_mapping.cpp',
> diff --git a/src/ipa/ipu3/algorithms/algorithm.cpp b/src/ipa/libipa/algorithm.cpp
> similarity index 74%
> rename from src/ipa/ipu3/algorithms/algorithm.cpp
> rename to src/ipa/libipa/algorithm.cpp
> index 3e7e3018..888ffb57 100644
> --- a/src/ipa/ipu3/algorithms/algorithm.cpp
> +++ b/src/ipa/libipa/algorithm.cpp
> @@ -2,7 +2,7 @@
> /*
> * Copyright (C) 2021, Ideas On Board
> *
> - * algorithm.cpp - IPU3 control algorithm interface
> + * algorithm.cpp - IPA control algorithm interface
> */
>
> #include "algorithm.h"
> @@ -14,11 +14,11 @@
>
> namespace libcamera {
>
> -namespace ipa::ipu3 {
> +namespace ipa {
>
> /**
> * \class Algorithm
> - * \brief The base class for all IPU3 algorithms
> + * \brief The base class for all IPA algorithms
The template parameters need to be documented with \tparam.
> *
> * The Algorithm class defines a standard interface for IPA algorithms. By
> * abstracting algorithms, it makes possible the implementation of generic code
> @@ -26,6 +26,7 @@ namespace ipa::ipu3 {
> */
>
> /**
> + * \fn Algorithm::configure()
> * \brief Configure the Algorithm given an IPAConfigInfo
> * \param[in] context The shared IPA context
> * \param[in] configInfo The IPA configuration data, received from the pipeline
> @@ -39,37 +40,29 @@ namespace ipa::ipu3 {
> *
> * \return 0 if successful, an error code otherwise
> */
> -int Algorithm::configure([[maybe_unused]] IPAContext &context,
> - [[maybe_unused]] const IPAConfigInfo &configInfo)
> -{
> - return 0;
> -}
>
> /**
> + * \fn Algorithm::prepare()
> * \brief Fill the \a params buffer with ISP processing parameters for a frame
> * \param[in] context The shared IPA context
> - * \param[out] params The IPU3 specific parameters.
> + * \param[out] params The ISP specific parameters.
> *
> * This function is called for every frame when the camera is running before it
> - * is processed by the ImgU to prepare the ImgU processing parameters for that
> - * frame.
> + * is processed by the ISP.
* is processed by the ISP to prepare the ISP processing parameters for that
* frame.
> *
> * Algorithms shall fill in the parameter structure fields appropriately to
> - * configure the ImgU processing blocks that they are responsible for. This
> + * configure the ISP processing blocks that they are responsible for. This
> * includes setting fields and flags that enable those processing blocks.
> */
> -void Algorithm::prepare([[maybe_unused]] IPAContext &context,
> - [[maybe_unused]] ipu3_uapi_params *params)
> -{
> -}
>
> /**
> + * \fn Algorithm::process()
> * \brief Process ISP statistics, and run algorithm operations
> * \param[in] context The shared IPA context
> - * \param[in] stats The IPU3 statistics and ISP results
> + * \param[in] stats The IPA statistics and ISP results
> *
> * This function is called while camera is running for every frame processed by
> - * the ImgU, to process statistics generated from that frame by the ImgU.
> + * the ISP, to process statistics generated from that frame by the ISP.
> * Algorithms shall use this data to run calculations and update their state
> * accordingly.
> *
> @@ -91,11 +84,7 @@ void Algorithm::prepare([[maybe_unused]] IPAContext &context,
> * Care shall be taken to ensure the ordering of access to the information
> * such that the algorithms use up to date state as required.
> */
> -void Algorithm::process([[maybe_unused]] IPAContext &context,
> - [[maybe_unused]] const ipu3_uapi_stats_3a *stats)
> -{
> -}
>
> -} /* namespace ipa::ipu3 */
> +} /* namespace ipa */
>
> } /* namespace libcamera */
> diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h
> new file mode 100644
> index 00000000..74dc49c4
> --- /dev/null
> +++ b/src/ipa/libipa/algorithm.h
> @@ -0,0 +1,41 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Ideas On Board
> + *
> + * algorithm.h - ISP control algorithm interface
> + */
> +#ifndef __LIBCAMERA_IPA_LIBIPA_ALGORITHM_H__
> +#define __LIBCAMERA_IPA_LIBIPA_ALGORITHM_H__
> +
> +namespace libcamera {
> +
> +namespace ipa {
> +
> +template<typename Context, typename IPAConfigInfo, typename Params, typename Stats>
s/IPAConfigInfo/Config/
> +class Algorithm
> +{
> +public:
> + virtual ~Algorithm() {}
> +
> + virtual int configure([[maybe_unused]] Context &context,
> + [[maybe_unused]] const IPAConfigInfo &configInfo)
> + {
> + return 0;
> + }
> +
> + virtual void prepare([[maybe_unused]] Context &context,
> + [[maybe_unused]] Params *params)
> + {
> + }
> +
> + virtual void process([[maybe_unused]] Context &context,
> + [[maybe_unused]] const Stats *stats)
> + {
> + }
> +};
> +
> +} /* namespace ipa */
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_IPA_LIBIPA_ALGORITHM_H__ */
> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build
> index 4d073a03..161cc5a1 100644
> --- a/src/ipa/libipa/meson.build
> +++ b/src/ipa/libipa/meson.build
> @@ -1,6 +1,7 @@
> # SPDX-License-Identifier: CC0-1.0
>
> libipa_headers = files([
> + 'algorithm.h',
> 'camera_sensor_helper.h',
> 'histogram.h'
> ])
> diff --git a/src/ipa/rkisp1/algorithms/algorithm.h b/src/ipa/rkisp1/algorithms/algorithm.h
> new file mode 100644
> index 00000000..dfa58727
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/algorithm.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Ideas On Board
> + *
> + * algorithm.h - RkISP1 control algorithm interface
> + */
> +#ifndef __LIBCAMERA_IPA_RKISP1_ALGORITHM_H__
> +#define __LIBCAMERA_IPA_RKISP1_ALGORITHM_H__
> +
> +#include <linux/rkisp1-config.h>
> +
> +#include <libcamera/ipa/rkisp1_ipa_interface.h>
> +
> +#include <libipa/algorithm.h>
> +
> +#include "ipa_context.h"
> +
> +namespace libcamera {
> +
> +namespace ipa::rkisp1 {
> +
> +using Algorithm = libcamera::ipa::Algorithm<IPAContext, IPACameraSensorInfo, rkisp1_params_cfg, rkisp1_stat_buffer>;
> +
> +} /* namespace ipa::rkisp1 */
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_IPA_RKISP1_ALGORITHM_H__ */
> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build
> new file mode 100644
> index 00000000..1c6c59cf
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/meson.build
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: CC0-1.0
> +
> +rkisp1_ipa_algorithms = files([
> +])
> diff --git a/src/ipa/rkisp1/meson.build b/src/ipa/rkisp1/meson.build
> index 3683c922..8c822fbb 100644
> --- a/src/ipa/rkisp1/meson.build
> +++ b/src/ipa/rkisp1/meson.build
> @@ -1,5 +1,7 @@
> # SPDX-License-Identifier: CC0-1.0
>
> +subdir('algorithms')
> +
> ipa_name = 'ipa_rkisp1'
>
> rkisp1_ipa_sources = files([
> @@ -7,6 +9,8 @@ rkisp1_ipa_sources = files([
> 'rkisp1.cpp',
> ])
>
> +rkisp1_ipa_sources += rkisp1_ipa_algorithms
> +
> mod = shared_module(ipa_name,
> [rkisp1_ipa_sources, libcamera_generated_ipa_headers],
> name_prefix : '',
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 34c3f9a2..0c54d8ec 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -25,7 +25,7 @@
>
> #include <libcamera/internal/mapped_framebuffer.h>
>
> -#include "ipa_context.h"
> +#include "algorithms/algorithm.h"
> #include "libipa/camera_sensor_helper.h"
>
> namespace libcamera {
> @@ -82,6 +82,9 @@ private:
>
> /* Local parameter storage */
> struct IPAContext context_;
> +
> + /* Maintain the algorithms used by the IPA */
> + std::list<std::unique_ptr<ipa::rkisp1::Algorithm>> algorithms_;
> };
>
> int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list