[{"id":21114,"web_url":"https://patchwork.libcamera.org/comment/21114/","msgid":"","date":"2021-11-23T10:02:03","subject":"Re: [libcamera-devel] [PATCH v2 06/11] ipa: libipa: Introduce\n\tAlgorithm class template","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jean-Michel,\n\nThank you for the patch.\n\nOn Tue, Nov 23, 2021 at 10:14:46AM +0100, Jean-Michel Hautbois wrote:\n> The algorithms are using the same function names with specialized\n> parameters. Instead of duplicating code, introduce a libipa Algorithm\n> class which implements a base class with template parameters in libipa,\n> and use it in each IPA.\n> \n> As we now won't need an algorithm class for each IPA, move the\n> documentation to libipa, and make it agnostic of the IPA used.\n> \n> Signed-off-by: Laurent Pinchart \n> Signed-off-by: Jean-Michel Hautbois \n> ---\n> src/ipa/ipu3/algorithms/algorithm.h | 12 ++----\n> src/ipa/ipu3/algorithms/meson.build | 1 -\n> .../{ipu3/algorithms => libipa}/algorithm.cpp | 35 ++++++----------\n> src/ipa/libipa/algorithm.h | 41 +++++++++++++++++++\n> src/ipa/libipa/meson.build | 1 +\n> src/ipa/rkisp1/algorithms/algorithm.h | 28 +++++++++++++\n> src/ipa/rkisp1/algorithms/meson.build | 4 ++\n> src/ipa/rkisp1/meson.build | 4 ++\n> src/ipa/rkisp1/rkisp1.cpp | 5 ++-\n\nShouldn't this have been split in two, with the introduction of\nsrc/ipa/libipa/algorithm.h and porting of IPU3 first, and then the\nrkisp1 additions ?\n\n> 9 files changed, 97 insertions(+), 34 deletions(-)\n> rename src/ipa/{ipu3/algorithms => libipa}/algorithm.cpp (74%)\n> create mode 100644 src/ipa/libipa/algorithm.h\n> create mode 100644 src/ipa/rkisp1/algorithms/algorithm.h\n> create mode 100644 src/ipa/rkisp1/algorithms/meson.build\n> \n> diff --git a/src/ipa/ipu3/algorithms/algorithm.h b/src/ipa/ipu3/algorithms/algorithm.h\n> index 43f5d8b0..3c0ce461 100644\n> --- a/src/ipa/ipu3/algorithms/algorithm.h\n> +++ b/src/ipa/ipu3/algorithms/algorithm.h\n> @@ -9,21 +9,15 @@\n> \n> #include \n> \n> +#include \n> +\n> #include \"ipa_context.h\"\n> \n> namespace libcamera {\n> \n> namespace ipa::ipu3 {\n> \n> -class Algorithm\n> -{\n> -public:\n> -\tvirtual ~Algorithm() {}\n> -\n> -\tvirtual int configure(IPAContext &context, const IPAConfigInfo &configInfo);\n> -\tvirtual void prepare(IPAContext &context, ipu3_uapi_params *params);\n> -\tvirtual void process(IPAContext &context, const ipu3_uapi_stats_3a *stats);\n> -};\n> +using Algorithm = libcamera::ipa::Algorithm;\n> \n> } /* namespace ipa::ipu3 */\n> \n> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build\n> index 3ec42f72..4db6ae1d 100644\n> --- a/src/ipa/ipu3/algorithms/meson.build\n> +++ b/src/ipa/ipu3/algorithms/meson.build\n> @@ -2,7 +2,6 @@\n> \n> ipu3_ipa_algorithms = files([\n> 'agc.cpp',\n> - 'algorithm.cpp',\n> 'awb.cpp',\n> 'blc.cpp',\n> 'tone_mapping.cpp',\n> diff --git a/src/ipa/ipu3/algorithms/algorithm.cpp b/src/ipa/libipa/algorithm.cpp\n> similarity index 74%\n> rename from src/ipa/ipu3/algorithms/algorithm.cpp\n> rename to src/ipa/libipa/algorithm.cpp\n> index 3e7e3018..888ffb57 100644\n> --- a/src/ipa/ipu3/algorithms/algorithm.cpp\n> +++ b/src/ipa/libipa/algorithm.cpp\n> @@ -2,7 +2,7 @@\n> /*\n> * Copyright (C) 2021, Ideas On Board\n> *\n> - * algorithm.cpp - IPU3 control algorithm interface\n> + * algorithm.cpp - IPA control algorithm interface\n> */\n> \n> #include \"algorithm.h\"\n> @@ -14,11 +14,11 @@\n> \n> namespace libcamera {\n> \n> -namespace ipa::ipu3 {\n> +namespace ipa {\n> \n> /**\n> * \\class Algorithm\n> - * \\brief The base class for all IPU3 algorithms\n> + * \\brief The base class for all IPA algorithms\n\nThe template parameters need to be documented with \\tparam.\n\n> *\n> * The Algorithm class defines a standard interface for IPA algorithms. By\n> * abstracting algorithms, it makes possible the implementation of generic code\n> @@ -26,6 +26,7 @@ namespace ipa::ipu3 {\n> */\n> \n> /**\n> + * \\fn Algorithm::configure()\n> * \\brief Configure the Algorithm given an IPAConfigInfo\n> * \\param[in] context The shared IPA context\n> * \\param[in] configInfo The IPA configuration data, received from the pipeline\n> @@ -39,37 +40,29 @@ namespace ipa::ipu3 {\n> *\n> * \\return 0 if successful, an error code otherwise\n> */\n> -int Algorithm::configure([[maybe_unused]] IPAContext &context,\n> -\t\t\t [[maybe_unused]] const IPAConfigInfo &configInfo)\n> -{\n> -\treturn 0;\n> -}\n> \n> /**\n> + * \\fn Algorithm::prepare()\n> * \\brief Fill the \\a params buffer with ISP processing parameters for a frame\n> * \\param[in] context The shared IPA context\n> - * \\param[out] params The IPU3 specific parameters.\n> + * \\param[out] params The ISP specific parameters.\n> *\n> * This function is called for every frame when the camera is running before it\n> - * is processed by the ImgU to prepare the ImgU processing parameters for that\n> - * frame.\n> + * is processed by the ISP.\n\n * is processed by the ISP to prepare the ISP processing parameters for that\n * frame.\n\n> *\n> * Algorithms shall fill in the parameter structure fields appropriately to\n> - * configure the ImgU processing blocks that they are responsible for. This\n> + * configure the ISP processing blocks that they are responsible for. This\n> * includes setting fields and flags that enable those processing blocks.\n> */\n> -void Algorithm::prepare([[maybe_unused]] IPAContext &context,\n> -\t\t\t[[maybe_unused]] ipu3_uapi_params *params)\n> -{\n> -}\n> \n> /**\n> + * \\fn Algorithm::process()\n> * \\brief Process ISP statistics, and run algorithm operations\n> * \\param[in] context The shared IPA context\n> - * \\param[in] stats The IPU3 statistics and ISP results\n> + * \\param[in] stats The IPA statistics and ISP results\n> *\n> * This function is called while camera is running for every frame processed by\n> - * the ImgU, to process statistics generated from that frame by the ImgU.\n> + * the ISP, to process statistics generated from that frame by the ISP.\n> * Algorithms shall use this data to run calculations and update their state\n> * accordingly.\n> *\n> @@ -91,11 +84,7 @@ void Algorithm::prepare([[maybe_unused]] IPAContext &context,\n> * Care shall be taken to ensure the ordering of access to the information\n> * such that the algorithms use up to date state as required.\n> */\n> -void Algorithm::process([[maybe_unused]] IPAContext &context,\n> -\t\t\t[[maybe_unused]] const ipu3_uapi_stats_3a *stats)\n> -{\n> -}\n> \n> -} /* namespace ipa::ipu3 */\n> +} /* namespace ipa */\n> \n> } /* namespace libcamera */\n> diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h\n> new file mode 100644\n> index 00000000..74dc49c4\n> --- /dev/null\n> +++ b/src/ipa/libipa/algorithm.h\n> @@ -0,0 +1,41 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Ideas On Board\n> + *\n> + * algorithm.h - ISP control algorithm interface\n> + */\n> +#ifndef __LIBCAMERA_IPA_LIBIPA_ALGORITHM_H__\n> +#define __LIBCAMERA_IPA_LIBIPA_ALGORITHM_H__\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa {\n> +\n> +template\n\ns/IPAConfigInfo/Config/\n\n> +class Algorithm\n> +{\n> +public:\n> +\tvirtual ~Algorithm() {}\n> +\n> +\tvirtual int configure([[maybe_unused]] Context &context,\n> +\t\t\t [[maybe_unused]] const IPAConfigInfo &configInfo)\n> +\t{\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tvirtual void prepare([[maybe_unused]] Context &context,\n> +\t\t\t [[maybe_unused]] Params *params)\n> +\t{\n> +\t}\n> +\n> +\tvirtual void process([[maybe_unused]] Context &context,\n> +\t\t\t [[maybe_unused]] const Stats *stats)\n> +\t{\n> +\t}\n> +};\n> +\n> +} /* namespace ipa */\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_IPA_LIBIPA_ALGORITHM_H__ */\n> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n> index 4d073a03..161cc5a1 100644\n> --- a/src/ipa/libipa/meson.build\n> +++ b/src/ipa/libipa/meson.build\n> @@ -1,6 +1,7 @@\n> # SPDX-License-Identifier: CC0-1.0\n> \n> libipa_headers = files([\n> + 'algorithm.h',\n> 'camera_sensor_helper.h',\n> 'histogram.h'\n> ])\n> diff --git a/src/ipa/rkisp1/algorithms/algorithm.h b/src/ipa/rkisp1/algorithms/algorithm.h\n> new file mode 100644\n> index 00000000..dfa58727\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/algorithm.h\n> @@ -0,0 +1,28 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Ideas On Board\n> + *\n> + * algorithm.h - RkISP1 control algorithm interface\n> + */\n> +#ifndef __LIBCAMERA_IPA_RKISP1_ALGORITHM_H__\n> +#define __LIBCAMERA_IPA_RKISP1_ALGORITHM_H__\n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \"ipa_context.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::rkisp1 {\n> +\n> +using Algorithm = libcamera::ipa::Algorithm;\n> +\n> +} /* namespace ipa::rkisp1 */\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_IPA_RKISP1_ALGORITHM_H__ */\n> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n> new file mode 100644\n> index 00000000..1c6c59cf\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/meson.build\n> @@ -0,0 +1,4 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +\n> +rkisp1_ipa_algorithms = files([\n> +])\n> diff --git a/src/ipa/rkisp1/meson.build b/src/ipa/rkisp1/meson.build\n> index 3683c922..8c822fbb 100644\n> --- a/src/ipa/rkisp1/meson.build\n> +++ b/src/ipa/rkisp1/meson.build\n> @@ -1,5 +1,7 @@\n> # SPDX-License-Identifier: CC0-1.0\n> \n> +subdir('algorithms')\n> +\n> ipa_name = 'ipa_rkisp1'\n> \n> rkisp1_ipa_sources = files([\n> @@ -7,6 +9,8 @@ rkisp1_ipa_sources = files([\n> 'rkisp1.cpp',\n> ])\n> \n> +rkisp1_ipa_sources += rkisp1_ipa_algorithms\n> +\n> mod = shared_module(ipa_name,\n> [rkisp1_ipa_sources, libcamera_generated_ipa_headers],\n> name_prefix : '',\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 34c3f9a2..0c54d8ec 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -25,7 +25,7 @@\n> \n> #include \n> \n> -#include \"ipa_context.h\"\n> +#include \"algorithms/algorithm.h\"\n> #include \"libipa/camera_sensor_helper.h\"\n> \n> namespace libcamera {\n> @@ -82,6 +82,9 @@ private:\n> \n> \t/* Local parameter storage */\n> \tstruct IPAContext context_;\n> +\n> +\t/* Maintain the algorithms used by the IPA */\n> +\tstd::list> algorithms_;\n> };\n> \n> int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7E293BF415\n\tfor ;\n\tTue, 23 Nov 2021 10:02:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A95416038C;\n\tTue, 23 Nov 2021 11:02:27 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7CAD26036F\n\tfor ;\n\tTue, 23 Nov 2021 11:02:26 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E8609A1B;\n\tTue, 23 Nov 2021 11:02:25 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"udEfEE+T\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637661746;\n\tbh=VadCsqGGEAhR2tQFiFE4NyjYPWfrNyJshIMxjMw4Jm8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=udEfEE+Tg3ZnGc++kuPSTa5HlskGLJV/IeSbrmM3s8rxt1GFr/iJKIyvuzu7ZFaP8\n\t8KRTqOw7aguj8yhf+2omgyUrM+0rPRSDQWUKhShDuT9zs43z8jyJtLA0t0aDAyHPoB\n\tR3eW37A37l3oSXxz2XnVrYudqNPQE4uywKU4SxQM=","Date":"Tue, 23 Nov 2021 12:02:03 +0200","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20211123091451.67404-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211123091451.67404-7-jeanmichel.hautbois@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211123091451.67404-7-jeanmichel.hautbois@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 06/11] ipa: libipa: Introduce\n\tAlgorithm class template","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21128,"web_url":"https://patchwork.libcamera.org/comment/21128/","msgid":"<300b59a4-2389-2daa-0c65-5b5a337c935f@ideasonboard.com>","date":"2021-11-23T13:28:32","subject":"Re: [libcamera-devel] [PATCH v2 06/11] ipa: libipa: Introduce\n\tAlgorithm class template","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 23/11/2021 11:02, Laurent Pinchart wrote:\n> Hi Jean-Michel,\n> \n> Thank you for the patch.\n> \n> On Tue, Nov 23, 2021 at 10:14:46AM +0100, Jean-Michel Hautbois wrote:\n>> The algorithms are using the same function names with specialized\n>> parameters. Instead of duplicating code, introduce a libipa Algorithm\n>> class which implements a base class with template parameters in libipa,\n>> and use it in each IPA.\n>>\n>> As we now won't need an algorithm class for each IPA, move the\n>> documentation to libipa, and make it agnostic of the IPA used.\n>>\n>> Signed-off-by: Laurent Pinchart \n>> Signed-off-by: Jean-Michel Hautbois \n>> ---\n>> src/ipa/ipu3/algorithms/algorithm.h | 12 ++----\n>> src/ipa/ipu3/algorithms/meson.build | 1 -\n>> .../{ipu3/algorithms => libipa}/algorithm.cpp | 35 ++++++----------\n>> src/ipa/libipa/algorithm.h | 41 +++++++++++++++++++\n>> src/ipa/libipa/meson.build | 1 +\n>> src/ipa/rkisp1/algorithms/algorithm.h | 28 +++++++++++++\n>> src/ipa/rkisp1/algorithms/meson.build | 4 ++\n>> src/ipa/rkisp1/meson.build | 4 ++\n>> src/ipa/rkisp1/rkisp1.cpp | 5 ++-\n> \n> Shouldn't this have been split in two, with the introduction of\n> src/ipa/libipa/algorithm.h and porting of IPU3 first, and then the\n> rkisp1 additions ?\n\nOK, I will split it in two :-).\n\n> \n>> 9 files changed, 97 insertions(+), 34 deletions(-)\n>> rename src/ipa/{ipu3/algorithms => libipa}/algorithm.cpp (74%)\n>> create mode 100644 src/ipa/libipa/algorithm.h\n>> create mode 100644 src/ipa/rkisp1/algorithms/algorithm.h\n>> create mode 100644 src/ipa/rkisp1/algorithms/meson.build\n>>\n>> diff --git a/src/ipa/ipu3/algorithms/algorithm.h b/src/ipa/ipu3/algorithms/algorithm.h\n>> index 43f5d8b0..3c0ce461 100644\n>> --- a/src/ipa/ipu3/algorithms/algorithm.h\n>> +++ b/src/ipa/ipu3/algorithms/algorithm.h\n>> @@ -9,21 +9,15 @@\n>> \n>> #include \n>> \n>> +#include \n>> +\n>> #include \"ipa_context.h\"\n>> \n>> namespace libcamera {\n>> \n>> namespace ipa::ipu3 {\n>> \n>> -class Algorithm\n>> -{\n>> -public:\n>> -\tvirtual ~Algorithm() {}\n>> -\n>> -\tvirtual int configure(IPAContext &context, const IPAConfigInfo &configInfo);\n>> -\tvirtual void prepare(IPAContext &context, ipu3_uapi_params *params);\n>> -\tvirtual void process(IPAContext &context, const ipu3_uapi_stats_3a *stats);\n>> -};\n>> +using Algorithm = libcamera::ipa::Algorithm;\n>> \n>> } /* namespace ipa::ipu3 */\n>> \n>> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build\n>> index 3ec42f72..4db6ae1d 100644\n>> --- a/src/ipa/ipu3/algorithms/meson.build\n>> +++ b/src/ipa/ipu3/algorithms/meson.build\n>> @@ -2,7 +2,6 @@\n>> \n>> ipu3_ipa_algorithms = files([\n>> 'agc.cpp',\n>> - 'algorithm.cpp',\n>> 'awb.cpp',\n>> 'blc.cpp',\n>> 'tone_mapping.cpp',\n>> diff --git a/src/ipa/ipu3/algorithms/algorithm.cpp b/src/ipa/libipa/algorithm.cpp\n>> similarity index 74%\n>> rename from src/ipa/ipu3/algorithms/algorithm.cpp\n>> rename to src/ipa/libipa/algorithm.cpp\n>> index 3e7e3018..888ffb57 100644\n>> --- a/src/ipa/ipu3/algorithms/algorithm.cpp\n>> +++ b/src/ipa/libipa/algorithm.cpp\n>> @@ -2,7 +2,7 @@\n>> /*\n>> * Copyright (C) 2021, Ideas On Board\n>> *\n>> - * algorithm.cpp - IPU3 control algorithm interface\n>> + * algorithm.cpp - IPA control algorithm interface\n>> */\n>> \n>> #include \"algorithm.h\"\n>> @@ -14,11 +14,11 @@\n>> \n>> namespace libcamera {\n>> \n>> -namespace ipa::ipu3 {\n>> +namespace ipa {\n>> \n>> /**\n>> * \\class Algorithm\n>> - * \\brief The base class for all IPU3 algorithms\n>> + * \\brief The base class for all IPA algorithms\n> \n> The template parameters need to be documented with \\tparam.\n> \n\nI haven't been warned by Doxygen, I will change that (did not know there \nis a specific template parameters keyword ;-)).\n\n>> *\n>> * The Algorithm class defines a standard interface for IPA algorithms. By\n>> * abstracting algorithms, it makes possible the implementation of generic code\n>> @@ -26,6 +26,7 @@ namespace ipa::ipu3 {\n>> */\n>> \n>> /**\n>> + * \\fn Algorithm::configure()\n>> * \\brief Configure the Algorithm given an IPAConfigInfo\n>> * \\param[in] context The shared IPA context\n>> * \\param[in] configInfo The IPA configuration data, received from the pipeline\n>> @@ -39,37 +40,29 @@ namespace ipa::ipu3 {\n>> *\n>> * \\return 0 if successful, an error code otherwise\n>> */\n>> -int Algorithm::configure([[maybe_unused]] IPAContext &context,\n>> -\t\t\t [[maybe_unused]] const IPAConfigInfo &configInfo)\n>> -{\n>> -\treturn 0;\n>> -}\n>> \n>> /**\n>> + * \\fn Algorithm::prepare()\n>> * \\brief Fill the \\a params buffer with ISP processing parameters for a frame\n>> * \\param[in] context The shared IPA context\n>> - * \\param[out] params The IPU3 specific parameters.\n>> + * \\param[out] params The ISP specific parameters.\n>> *\n>> * This function is called for every frame when the camera is running before it\n>> - * is processed by the ImgU to prepare the ImgU processing parameters for that\n>> - * frame.\n>> + * is processed by the ISP.\n> \n> * is processed by the ISP to prepare the ISP processing parameters for that\n> * frame.\n> \n>> *\n>> * Algorithms shall fill in the parameter structure fields appropriately to\n>> - * configure the ImgU processing blocks that they are responsible for. This\n>> + * configure the ISP processing blocks that they are responsible for. This\n>> * includes setting fields and flags that enable those processing blocks.\n>> */\n>> -void Algorithm::prepare([[maybe_unused]] IPAContext &context,\n>> -\t\t\t[[maybe_unused]] ipu3_uapi_params *params)\n>> -{\n>> -}\n>> \n>> /**\n>> + * \\fn Algorithm::process()\n>> * \\brief Process ISP statistics, and run algorithm operations\n>> * \\param[in] context The shared IPA context\n>> - * \\param[in] stats The IPU3 statistics and ISP results\n>> + * \\param[in] stats The IPA statistics and ISP results\n>> *\n>> * This function is called while camera is running for every frame processed by\n>> - * the ImgU, to process statistics generated from that frame by the ImgU.\n>> + * the ISP, to process statistics generated from that frame by the ISP.\n>> * Algorithms shall use this data to run calculations and update their state\n>> * accordingly.\n>> *\n>> @@ -91,11 +84,7 @@ void Algorithm::prepare([[maybe_unused]] IPAContext &context,\n>> * Care shall be taken to ensure the ordering of access to the information\n>> * such that the algorithms use up to date state as required.\n>> */\n>> -void Algorithm::process([[maybe_unused]] IPAContext &context,\n>> -\t\t\t[[maybe_unused]] const ipu3_uapi_stats_3a *stats)\n>> -{\n>> -}\n>> \n>> -} /* namespace ipa::ipu3 */\n>> +} /* namespace ipa */\n>> \n>> } /* namespace libcamera */\n>> diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h\n>> new file mode 100644\n>> index 00000000..74dc49c4\n>> --- /dev/null\n>> +++ b/src/ipa/libipa/algorithm.h\n>> @@ -0,0 +1,41 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2021, Ideas On Board\n>> + *\n>> + * algorithm.h - ISP control algorithm interface\n>> + */\n>> +#ifndef __LIBCAMERA_IPA_LIBIPA_ALGORITHM_H__\n>> +#define __LIBCAMERA_IPA_LIBIPA_ALGORITHM_H__\n>> +\n>> +namespace libcamera {\n>> +\n>> +namespace ipa {\n>> +\n>> +template\n> \n> s/IPAConfigInfo/Config/\n> \n>> +class Algorithm\n>> +{\n>> +public:\n>> +\tvirtual ~Algorithm() {}\n>> +\n>> +\tvirtual int configure([[maybe_unused]] Context &context,\n>> +\t\t\t [[maybe_unused]] const IPAConfigInfo &configInfo)\n>> +\t{\n>> +\t\treturn 0;\n>> +\t}\n>> +\n>> +\tvirtual void prepare([[maybe_unused]] Context &context,\n>> +\t\t\t [[maybe_unused]] Params *params)\n>> +\t{\n>> +\t}\n>> +\n>> +\tvirtual void process([[maybe_unused]] Context &context,\n>> +\t\t\t [[maybe_unused]] const Stats *stats)\n>> +\t{\n>> +\t}\n>> +};\n>> +\n>> +} /* namespace ipa */\n>> +\n>> +} /* namespace libcamera */\n>> +\n>> +#endif /* __LIBCAMERA_IPA_LIBIPA_ALGORITHM_H__ */\n>> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n>> index 4d073a03..161cc5a1 100644\n>> --- a/src/ipa/libipa/meson.build\n>> +++ b/src/ipa/libipa/meson.build\n>> @@ -1,6 +1,7 @@\n>> # SPDX-License-Identifier: CC0-1.0\n>> \n>> libipa_headers = files([\n>> + 'algorithm.h',\n>> 'camera_sensor_helper.h',\n>> 'histogram.h'\n>> ])\n>> diff --git a/src/ipa/rkisp1/algorithms/algorithm.h b/src/ipa/rkisp1/algorithms/algorithm.h\n>> new file mode 100644\n>> index 00000000..dfa58727\n>> --- /dev/null\n>> +++ b/src/ipa/rkisp1/algorithms/algorithm.h\n>> @@ -0,0 +1,28 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2021, Ideas On Board\n>> + *\n>> + * algorithm.h - RkISP1 control algorithm interface\n>> + */\n>> +#ifndef __LIBCAMERA_IPA_RKISP1_ALGORITHM_H__\n>> +#define __LIBCAMERA_IPA_RKISP1_ALGORITHM_H__\n>> +\n>> +#include \n>> +\n>> +#include \n>> +\n>> +#include \n>> +\n>> +#include \"ipa_context.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +namespace ipa::rkisp1 {\n>> +\n>> +using Algorithm = libcamera::ipa::Algorithm;\n>> +\n>> +} /* namespace ipa::rkisp1 */\n>> +\n>> +} /* namespace libcamera */\n>> +\n>> +#endif /* __LIBCAMERA_IPA_RKISP1_ALGORITHM_H__ */\n>> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n>> new file mode 100644\n>> index 00000000..1c6c59cf\n>> --- /dev/null\n>> +++ b/src/ipa/rkisp1/algorithms/meson.build\n>> @@ -0,0 +1,4 @@\n>> +# SPDX-License-Identifier: CC0-1.0\n>> +\n>> +rkisp1_ipa_algorithms = files([\n>> +])\n>> diff --git a/src/ipa/rkisp1/meson.build b/src/ipa/rkisp1/meson.build\n>> index 3683c922..8c822fbb 100644\n>> --- a/src/ipa/rkisp1/meson.build\n>> +++ b/src/ipa/rkisp1/meson.build\n>> @@ -1,5 +1,7 @@\n>> # SPDX-License-Identifier: CC0-1.0\n>> \n>> +subdir('algorithms')\n>> +\n>> ipa_name = 'ipa_rkisp1'\n>> \n>> rkisp1_ipa_sources = files([\n>> @@ -7,6 +9,8 @@ rkisp1_ipa_sources = files([\n>> 'rkisp1.cpp',\n>> ])\n>> \n>> +rkisp1_ipa_sources += rkisp1_ipa_algorithms\n>> +\n>> mod = shared_module(ipa_name,\n>> [rkisp1_ipa_sources, libcamera_generated_ipa_headers],\n>> name_prefix : '',\n>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n>> index 34c3f9a2..0c54d8ec 100644\n>> --- a/src/ipa/rkisp1/rkisp1.cpp\n>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n>> @@ -25,7 +25,7 @@\n>> \n>> #include \n>> \n>> -#include \"ipa_context.h\"\n>> +#include \"algorithms/algorithm.h\"\n>> #include \"libipa/camera_sensor_helper.h\"\n>> \n>> namespace libcamera {\n>> @@ -82,6 +82,9 @@ private:\n>> \n>> \t/* Local parameter storage */\n>> \tstruct IPAContext context_;\n>> +\n>> +\t/* Maintain the algorithms used by the IPA */\n>> +\tstd::list> algorithms_;\n>> };\n>> \n>> int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 674ABBDB13\n\tfor ;\n\tTue, 23 Nov 2021 13:28:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BBC6360121;\n\tTue, 23 Nov 2021 14:28:36 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2896460121\n\tfor ;\n\tTue, 23 Nov 2021 14:28:35 +0100 (CET)","from [IPV6:2a01:e0a:169:7140:3c3b:9149:b:8aa9] (unknown\n\t[IPv6:2a01:e0a:169:7140:3c3b:9149:b:8aa9])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B9B00993;\n\tTue, 23 Nov 2021 14:28:34 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"E/BQQv9c\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637674114;\n\tbh=Inlvsfsf6Hi4wnFKZ4BeTZ81bQLoKjIPxfhgxpTy/is=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=E/BQQv9czVPgNB1UJ+0ofXIkBgbpwIolKsFlhTnZDPo56iUrEk/70jfTw07zPKQvs\n\tcOjg4/QOlIUBji3/DtAYFYJ6IwEW3vKDHjeWV30MS2vpvYWROEKxiaOov61rLl4sVm\n\tr29oEk5eIxDJBQlrwnvAFwbjS7HFPt/xC6ClBqM4=","Message-ID":"<300b59a4-2389-2daa-0c65-5b5a337c935f@ideasonboard.com>","Date":"Tue, 23 Nov 2021 14:28:32 +0100","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.3.1","Content-Language":"en-US","To":"Laurent Pinchart ","References":"<20211123091451.67404-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211123091451.67404-7-jeanmichel.hautbois@ideasonboard.com>\n\t","From":"Jean-Michel Hautbois ","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v2 06/11] ipa: libipa: Introduce\n\tAlgorithm class template","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]