[{"id":21140,"web_url":"https://patchwork.libcamera.org/comment/21140/","msgid":"","date":"2021-11-23T15:09:01","subject":"Re: [libcamera-devel] [PATCH v3 07/11] ipa: rkisp1: Use the\n\tAlgorithm class","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 04:04:19PM +0100, Jean-Michel Hautbois wrote:\n> Now that libipa offers a templated class for Algorithm, use it in\n> RkISP1.\n> \n> Signed-off-by: Jean-Michel Hautbois \n> ---\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> 4 files changed, 40 insertions(+), 1 deletion(-)\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/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 32E86BDB13\n\tfor ;\n\tTue, 23 Nov 2021 15:09:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A85B460230;\n\tTue, 23 Nov 2021 16:09:24 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BBFD760121\n\tfor ;\n\tTue, 23 Nov 2021 16:09:23 +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 3F68C993;\n\tTue, 23 Nov 2021 16:09:23 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"jA+q2KBy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637680163;\n\tbh=eguw0O2X66RemUnVCD49y5o4e98pN2yXkd09aBr+Ky8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jA+q2KBysAwrjisxgih1DZpJDmtVTb3dHPJfAl8n/ZMRYs1b+LG5D3iAWEYL07oOm\n\tt0vCtqm3jJPtONuXnwVfaix6KvqmB5/8wsUbP3IJGnHLGhUiXQCrdQtxPUHDBjKk8a\n\tPvbFElayQRNOjVMnFXrWkbuAltWFRhDaOPMF4VuI=","Date":"Tue, 23 Nov 2021 17:09:01 +0200","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20211123150423.125524-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211123150423.125524-8-jeanmichel.hautbois@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211123150423.125524-8-jeanmichel.hautbois@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 07/11] ipa: rkisp1: Use the\n\tAlgorithm class","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":21141,"web_url":"https://patchwork.libcamera.org/comment/21141/","msgid":"<926c95b0-4fca-aa48-ad5c-4d4f164a90e2@ideasonboard.com>","date":"2021-11-23T15:09:51","subject":"Re: [libcamera-devel] [PATCH v3 07/11] ipa: rkisp1: Use the\n\tAlgorithm class","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"No comment ? :-)\n\nOn 23/11/2021 16:09, Laurent Pinchart wrote:\n> Hi Jean-Michel,\n> \n> Thank you for the patch.\n> \n> On Tue, Nov 23, 2021 at 04:04:19PM +0100, Jean-Michel Hautbois wrote:\n>> Now that libipa offers a templated class for Algorithm, use it in\n>> RkISP1.\n>>\n>> Signed-off-by: Jean-Michel Hautbois \n>> ---\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>> 4 files changed, 40 insertions(+), 1 deletion(-)\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/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 6AC39BDB13\n\tfor ;\n\tTue, 23 Nov 2021 15:09:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 244E960230;\n\tTue, 23 Nov 2021 16:09:56 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D46A860121\n\tfor ;\n\tTue, 23 Nov 2021 16:09:54 +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 9B3A0993;\n\tTue, 23 Nov 2021 16:09:54 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"wAD2BTfI\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637680194;\n\tbh=fp5E+okIOCz3aEn+1dxuhex8JkUK1zC+6TLxa014pnY=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=wAD2BTfI7EPduFsiLtEB0wWL6MDDcIPh4NXyCb6KzCHBZdjREbcQYwca1T34SZuFE\n\t9H5HWCZ7S0JuAF+bQlChdIwx9kfqcTmdjbgnE6++9AvITgCSfV6xHznXHxBtWDyNRm\n\tMKyBu3ZnObEPVSZw4oaewvo5Zn/rZmzK1g9L/YF0=","Message-ID":"<926c95b0-4fca-aa48-ad5c-4d4f164a90e2@ideasonboard.com>","Date":"Tue, 23 Nov 2021 16:09:51 +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":"<20211123150423.125524-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211123150423.125524-8-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 v3 07/11] ipa: rkisp1: Use the\n\tAlgorithm class","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":21143,"web_url":"https://patchwork.libcamera.org/comment/21143/","msgid":"","date":"2021-11-23T15:13:48","subject":"Re: [libcamera-devel] [PATCH v3 07/11] ipa: rkisp1: Use the\n\tAlgorithm class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Nov 23, 2021 at 04:09:51PM +0100, Jean-Michel Hautbois wrote:\n> No comment ? :-)\n\nDo I always need to comment ? :-)\n\nReviewed-by: Laurent Pinchart \n\n> On 23/11/2021 16:09, Laurent Pinchart wrote:\n> > Hi Jean-Michel,\n> > \n> > Thank you for the patch.\n> > \n> > On Tue, Nov 23, 2021 at 04:04:19PM +0100, Jean-Michel Hautbois wrote:\n> >> Now that libipa offers a templated class for Algorithm, use it in\n> >> RkISP1.\n> >>\n> >> Signed-off-by: Jean-Michel Hautbois \n> >> ---\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> >> 4 files changed, 40 insertions(+), 1 deletion(-)\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/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 B63B3BF415\n\tfor ;\n\tTue, 23 Nov 2021 15:14:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6A2766038A;\n\tTue, 23 Nov 2021 16:14:12 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2D15360121\n\tfor ;\n\tTue, 23 Nov 2021 16:14:11 +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 BC84C993;\n\tTue, 23 Nov 2021 16:14:10 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"N2cVVWOu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637680451;\n\tbh=xWaGnPLoBaCpTs90PPqWxrVsaSbArrL2QUv+ZQ4ehf4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=N2cVVWOutIXJQz7SpOTv4XobjYSJ8Z3MIoDlX+1vab8GKPLiJ8eMM/3w7OI8+edw0\n\tXCwLJOGZ/scCntixWI1CXNFZa+2mKTknC0qQCFKeMgwtIZXij0bSAHbuLbpZnt/SRJ\n\t/0uNn8ini9TChAGBhvgIsFK0oZ6RnqkkPYXWvvrQ=","Date":"Tue, 23 Nov 2021 17:13:48 +0200","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20211123150423.125524-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211123150423.125524-8-jeanmichel.hautbois@ideasonboard.com>\n\t\n\t<926c95b0-4fca-aa48-ad5c-4d4f164a90e2@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<926c95b0-4fca-aa48-ad5c-4d4f164a90e2@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 07/11] ipa: rkisp1: Use the\n\tAlgorithm class","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":21150,"web_url":"https://patchwork.libcamera.org/comment/21150/","msgid":"<163768168871.3059017.17176016291539554234@Monstersaurus>","date":"2021-11-23T15:34:48","subject":"Re: [libcamera-devel] [PATCH v3 07/11] ipa: rkisp1: Use the\n\tAlgorithm class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jean-Michel Hautbois (2021-11-23 15:04:19)\n> Now that libipa offers a templated class for Algorithm, use it in\n> RkISP1.\n> \n> Signed-off-by: Jean-Michel Hautbois \n> ---\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> 4 files changed, 40 insertions(+), 1 deletion(-)\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/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\nWe're under ipa/rkisp1/algorithms/algorithm.h\nSo with the current style, that might need an extra _ALGORITHM_\n\nBut it also feels a bit redundant to have\n..._ALGORITHM_ALGORITHM_H__\n\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\nIs this intentionally removed?\nI suspect it should be put after the libipa/camera_sensor_helper.h...\n\nWhich means libipa/camera_sensor_helper.h should probably have been put\nbefore ipa_context.h in the patch that added that...\n\nWith the minors resolved:\n\n\nReviewed-by: Kieran Bingham \n\n> +#include \"algorithms/algorithm.h\"\n> #include \"libipa/camera_sensor_helper.h\"\n> \n> namespace libcamera {\n> @@ -82,6 +82,9 @@ private:\n> \n> /* Local parameter storage */\n> struct IPAContext context_;\n> +\n> + /* Maintain the algorithms used by the IPA */\n> + std::list> algorithms_;\n> };\n> \n> int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)\n> -- \n> 2.32.0\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 62405BDB13\n\tfor ;\n\tTue, 23 Nov 2021 15:34:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BBBC060230;\n\tTue, 23 Nov 2021 16:34:52 +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 25D6460121\n\tfor ;\n\tTue, 23 Nov 2021 16:34:52 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B766E93;\n\tTue, 23 Nov 2021 16:34:51 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"N4dsuHzB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637681691;\n\tbh=gVfT5Fbu+hMuupt562eWP8VuB4NdwlnLd79oaBow2dA=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=N4dsuHzBb4/LdjzozgFZ2kwcCGPjuq84MeApSjEvgzXo2lZAMkLMTNqNJjGUUC7VP\n\tr12Q8+3VE0R9QwHbFKxLKpAFkLjfmRsboYGPQiRJksWpZ+27m0vYi+Qt7o8uS4Gvu5\n\tn94unerYoklOb8R7AcrmIv3BqMHia0VD2fwQvZ6E=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211123150423.125524-8-jeanmichel.hautbois@ideasonboard.com>","References":"<20211123150423.125524-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211123150423.125524-8-jeanmichel.hautbois@ideasonboard.com>","From":"Kieran Bingham ","To":"Jean-Michel Hautbois ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 23 Nov 2021 15:34:48 +0000","Message-ID":"<163768168871.3059017.17176016291539554234@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v3 07/11] ipa: rkisp1: Use the\n\tAlgorithm class","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21151,"web_url":"https://patchwork.libcamera.org/comment/21151/","msgid":"<7e56871f-7054-1f35-c231-4280c7d1b4f7@ideasonboard.com>","date":"2021-11-23T15:37:41","subject":"Re: [libcamera-devel] [PATCH v3 07/11] ipa: rkisp1: Use the\n\tAlgorithm class","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"On 23/11/2021 16:34, Kieran Bingham wrote:\n> Quoting Jean-Michel Hautbois (2021-11-23 15:04:19)\n>> Now that libipa offers a templated class for Algorithm, use it in\n>> RkISP1.\n>>\n>> Signed-off-by: Jean-Michel Hautbois \n>> ---\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>> 4 files changed, 40 insertions(+), 1 deletion(-)\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/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> \n> We're under ipa/rkisp1/algorithms/algorithm.h\n> So with the current style, that might need an extra _ALGORITHM_\n> \n> But it also feels a bit redundant to have\n> ..._ALGORITHM_ALGORITHM_H__\n\nI really dislike this :-) ! And in IPU3 we have \n__LIBCAMERA_IPA_IPU3_ALGORITHM_H__\n\n> \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> \n> Is this intentionally removed?\n\nYes, because it is now included by algorithm.h wchich need it to define \nits template parameter Context.\n\n> I suspect it should be put after the libipa/camera_sensor_helper.h...\n> \n> Which means libipa/camera_sensor_helper.h should probably have been put\n> before ipa_context.h in the patch that added that...\n> \n> With the minors resolved:\n> \n> \n> Reviewed-by: Kieran Bingham \n> \n>> +#include \"algorithms/algorithm.h\"\n>> #include \"libipa/camera_sensor_helper.h\"\n>> \n>> namespace libcamera {\n>> @@ -82,6 +82,9 @@ private:\n>> \n>> /* Local parameter storage */\n>> struct IPAContext context_;\n>> +\n>> + /* Maintain the algorithms used by the IPA */\n>> + std::list> algorithms_;\n>> };\n>> \n>> int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)\n>> -- \n>> 2.32.0\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 2F509BDB13\n\tfor ;\n\tTue, 23 Nov 2021 15:37:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6933A60230;\n\tTue, 23 Nov 2021 16:37:46 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B89CD60121\n\tfor ;\n\tTue, 23 Nov 2021 16:37:44 +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 6681D93;\n\tTue, 23 Nov 2021 16:37:44 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"kzlRirgD\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637681864;\n\tbh=WkLwnnv5YCNoyUD37/Wwl5Z+UUnAMb1XiD77fDsCZIs=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=kzlRirgDj8syXdODQ7BJeHlP0UqDeFCQRyR2/0uPsUZ8gDp2nE26QqP4GjgLTfy2M\n\tKdNPLOux0X9wtAI18jFB+ut4lnRn1V8XwpxbkE1EAV8mn3rUAxI01o+cUTOq8KeOZD\n\tPtZDdWCOe3LbKKrbDDu2rJNW6S+vnBy0ajUmtb5I=","Message-ID":"<7e56871f-7054-1f35-c231-4280c7d1b4f7@ideasonboard.com>","Date":"Tue, 23 Nov 2021 16:37:41 +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":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20211123150423.125524-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211123150423.125524-8-jeanmichel.hautbois@ideasonboard.com>\n\t<163768168871.3059017.17176016291539554234@Monstersaurus>","From":"Jean-Michel Hautbois ","In-Reply-To":"<163768168871.3059017.17176016291539554234@Monstersaurus>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v3 07/11] ipa: rkisp1: Use the\n\tAlgorithm class","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21154,"web_url":"https://patchwork.libcamera.org/comment/21154/","msgid":"<163768419863.3059017.11646705442431593480@Monstersaurus>","date":"2021-11-23T16:16:38","subject":"Re: [libcamera-devel] [PATCH v3 07/11] ipa: rkisp1: Use the\n\tAlgorithm class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jean-Michel Hautbois (2021-11-23 15:37:41)\n> \n> \n> On 23/11/2021 16:34, Kieran Bingham wrote:\n> > Quoting Jean-Michel Hautbois (2021-11-23 15:04:19)\n> >> Now that libipa offers a templated class for Algorithm, use it in\n> >> RkISP1.\n> >>\n> >> Signed-off-by: Jean-Michel Hautbois \n> >> ---\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> >> 4 files changed, 40 insertions(+), 1 deletion(-)\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/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> > \n> > We're under ipa/rkisp1/algorithms/algorithm.h\n> > So with the current style, that might need an extra _ALGORITHM_\n> > \n> > But it also feels a bit redundant to have\n> > ..._ALGORITHM_ALGORITHM_H__\n> \n> I really dislike this :-) ! And in IPU3 we have \n> __LIBCAMERA_IPA_IPU3_ALGORITHM_H__\n\nYes, me too - so lets leave it as it is.\n\n\n> \n> > \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> > \n> > Is this intentionally removed?\n> \n> Yes, because it is now included by algorithm.h wchich need it to define \n> its template parameter Context.\n> \n\nI'd be tempted to say it should still be specified here, as we shouldn't\nrely on other component headers to provide the headers we need.\n\nBut equally we can't anticipate the algorithm.h to remove the context.h\nas that's fundamental property of the algorithms.\n\nIWYU [0] would complain, but either way, it won't bother me so much,\nand my tag is already there.\n\n[0] https://include-what-you-use.org/\n\n> > I suspect it should be put after the libipa/camera_sensor_helper.h...\n> > \n> > Which means libipa/camera_sensor_helper.h should probably have been put\n> > before ipa_context.h in the patch that added that...\n> > \n> > With the minors resolved:\n> > \n> > \n> > Reviewed-by: Kieran Bingham \n> > \n> >> +#include \"algorithms/algorithm.h\"\n> >> #include \"libipa/camera_sensor_helper.h\"\n> >> \n> >> namespace libcamera {\n> >> @@ -82,6 +82,9 @@ private:\n> >> \n> >> /* Local parameter storage */\n> >> struct IPAContext context_;\n> >> +\n> >> + /* Maintain the algorithms used by the IPA */\n> >> + std::list> algorithms_;\n> >> };\n> >> \n> >> int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)\n> >> -- \n> >> 2.32.0\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 D1368BF415\n\tfor ;\n\tTue, 23 Nov 2021 16:16:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 262E760230;\n\tTue, 23 Nov 2021 17:16:42 +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 5C22C60121\n\tfor ;\n\tTue, 23 Nov 2021 17:16:41 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E9968A1B;\n\tTue, 23 Nov 2021 17:16:40 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"AGoV07Ch\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637684201;\n\tbh=TFqGym8pHk413nxMYtxHeUqTaaJPUONH2B0KumpuYg8=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=AGoV07ChGyvctAQeV+66ikrLTadLAGFUwXaJwtmZQFtfALisJeZYrs/eaS9eiHNu6\n\tzADMRJKrBxfOtsLpTiJkL8W+S0+cbLhoSLqeL+T5N8FbicnspKVE0KVNNTF6E49GU3\n\tbqmjtJfNn/T7KrphvT8xVDu4iO80lfAwg1O0SHAc=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<7e56871f-7054-1f35-c231-4280c7d1b4f7@ideasonboard.com>","References":"<20211123150423.125524-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211123150423.125524-8-jeanmichel.hautbois@ideasonboard.com>\n\t<163768168871.3059017.17176016291539554234@Monstersaurus>\n\t<7e56871f-7054-1f35-c231-4280c7d1b4f7@ideasonboard.com>","From":"Kieran Bingham ","To":"Jean-Michel Hautbois ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 23 Nov 2021 16:16:38 +0000","Message-ID":"<163768419863.3059017.11646705442431593480@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v3 07/11] ipa: rkisp1: Use the\n\tAlgorithm class","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21160,"web_url":"https://patchwork.libcamera.org/comment/21160/","msgid":"","date":"2021-11-23T16:59:29","subject":"Re: [libcamera-devel] [PATCH v3 07/11] ipa: rkisp1: Use the\n\tAlgorithm class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Nov 23, 2021 at 03:34:48PM +0000, Kieran Bingham wrote:\n> Quoting Jean-Michel Hautbois (2021-11-23 15:04:19)\n> > Now that libipa offers a templated class for Algorithm, use it in\n> > RkISP1.\n> > \n> > Signed-off-by: Jean-Michel Hautbois \n> > ---\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> > 4 files changed, 40 insertions(+), 1 deletion(-)\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/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> \n> We're under ipa/rkisp1/algorithms/algorithm.h\n> So with the current style, that might need an extra _ALGORITHM_\n> \n> But it also feels a bit redundant to have\n> ..._ALGORITHM_ALGORITHM_H__\n\nCan I volunteer someone to switch all our headers to \"#pragma once\" ?\n\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> \n> Is this intentionally removed?\n> I suspect it should be put after the libipa/camera_sensor_helper.h...\n> \n> Which means libipa/camera_sensor_helper.h should probably have been put\n> before ipa_context.h in the patch that added that...\n> \n> With the minors resolved:\n> \n> \n> Reviewed-by: Kieran Bingham \n> \n> > +#include \"algorithms/algorithm.h\"\n> > #include \"libipa/camera_sensor_helper.h\"\n> > \n> > namespace libcamera {\n> > @@ -82,6 +82,9 @@ private:\n> > \n> > /* Local parameter storage */\n> > struct IPAContext context_;\n> > +\n> > + /* Maintain the algorithms used by the IPA */\n> > + std::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 2996DBDB13\n\tfor ;\n\tTue, 23 Nov 2021 16:59:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CF79760230;\n\tTue, 23 Nov 2021 17:59:53 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5226960121\n\tfor ;\n\tTue, 23 Nov 2021 17:59:52 +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 E3772A1B;\n\tTue, 23 Nov 2021 17:59:51 +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=\"aM31GZh+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637686792;\n\tbh=s6tH+3iRSpQwhHfxh7ADFAx9N5NeFhC162VvzS+Koxg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=aM31GZh+31ScPcZmAPRgX3T2yREtIgqX7lazM2WZQ8IU+ZHMGJu0HqJuLlYzVIFA5\n\tLXPVLSUZxf+PXtWQTqwXGDGun3HaxJ4G4Cmc6a1NTpHeG6GwQE2qO/hTMlpFwCXddV\n\tPR/5SimcdW1V/QEDuzc0vWYJ3nV+fGEXERzY8pWw=","Date":"Tue, 23 Nov 2021 18:59:29 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20211123150423.125524-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211123150423.125524-8-jeanmichel.hautbois@ideasonboard.com>\n\t<163768168871.3059017.17176016291539554234@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<163768168871.3059017.17176016291539554234@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v3 07/11] ipa: rkisp1: Use the\n\tAlgorithm class","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":21162,"web_url":"https://patchwork.libcamera.org/comment/21162/","msgid":"<163768773371.3059017.3704032407790386786@Monstersaurus>","date":"2021-11-23T17:15:33","subject":"Re: [libcamera-devel] [PATCH v3 07/11] ipa: rkisp1: Use the\n\tAlgorithm class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2021-11-23 16:59:29)\n> On Tue, Nov 23, 2021 at 03:34:48PM +0000, Kieran Bingham wrote:\n> > Quoting Jean-Michel Hautbois (2021-11-23 15:04:19)\n> > > Now that libipa offers a templated class for Algorithm, use it in\n> > > RkISP1.\n> > > \n> > > Signed-off-by: Jean-Michel Hautbois \n> > > ---\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> > > 4 files changed, 40 insertions(+), 1 deletion(-)\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/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> > \n> > We're under ipa/rkisp1/algorithms/algorithm.h\n> > So with the current style, that might need an extra _ALGORITHM_\n> > \n> > But it also feels a bit redundant to have\n> > ..._ALGORITHM_ALGORITHM_H__\n> \n> Can I volunteer someone to switch all our headers to \"#pragma once\" ?\n\nI'd love to ... Can we do that? I thought I'd brought that up before and\nfor some reason we couldn't ...\n\n\nGet ready for a patch storm...\n\nOk - so ... not straight away ...\n\n\n> \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> > \n> > Is this intentionally removed?\n> > I suspect it should be put after the libipa/camera_sensor_helper.h...\n> > \n> > Which means libipa/camera_sensor_helper.h should probably have been put\n> > before ipa_context.h in the patch that added that...\n> > \n> > With the minors resolved:\n> > \n> > \n> > Reviewed-by: Kieran Bingham \n> > \n> > > +#include \"algorithms/algorithm.h\"\n> > > #include \"libipa/camera_sensor_helper.h\"\n> > > \n> > > namespace libcamera {\n> > > @@ -82,6 +82,9 @@ private:\n> > > \n> > > /* Local parameter storage */\n> > > struct IPAContext context_;\n> > > +\n> > > + /* Maintain the algorithms used by the IPA */\n> > > + std::list> algorithms_;\n> > > };\n> > > \n> > > int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 1C393BDB13\n\tfor ;\n\tTue, 23 Nov 2021 17:15:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4BD0B60230;\n\tTue, 23 Nov 2021 18:15:38 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5457B60121\n\tfor ;\n\tTue, 23 Nov 2021 18:15:36 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DED79A1B;\n\tTue, 23 Nov 2021 18:15:35 +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=\"R2QflEkr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637687736;\n\tbh=SYQIK3DhXpLcR02qej+SUBVo/cbjtFiGi8fVst1bdWY=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=R2QflEkrHqqRDNanCjHfS3ATwTYMk7uNxZPnqEVMgcCP9tWJyN8fIeUI+jV5V+SvQ\n\tCdbBvpVBCkFuFp+rQi1kFjplTpaXeHDWQU+o6pY/XI+f1pTW4IAUO7eFAhP35NuALG\n\tLJ0C8J+fhSzwtZ1oDZqu1TYuFFHQ4Dbay61EP1tE=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20211123150423.125524-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211123150423.125524-8-jeanmichel.hautbois@ideasonboard.com>\n\t<163768168871.3059017.17176016291539554234@Monstersaurus>\n\t","From":"Kieran Bingham ","To":"Laurent Pinchart ","Date":"Tue, 23 Nov 2021 17:15:33 +0000","Message-ID":"<163768773371.3059017.3704032407790386786@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v3 07/11] ipa: rkisp1: Use the\n\tAlgorithm class","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":21163,"web_url":"https://patchwork.libcamera.org/comment/21163/","msgid":"","date":"2021-11-23T17:31:49","subject":"Re: [libcamera-devel] [PATCH v3 07/11] ipa: rkisp1: Use the\n\tAlgorithm class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Nov 23, 2021 at 05:15:33PM +0000, Kieran Bingham wrote:\n> Quoting Laurent Pinchart (2021-11-23 16:59:29)\n> > On Tue, Nov 23, 2021 at 03:34:48PM +0000, Kieran Bingham wrote:\n> > > Quoting Jean-Michel Hautbois (2021-11-23 15:04:19)\n> > > > Now that libipa offers a templated class for Algorithm, use it in\n> > > > RkISP1.\n> > > > \n> > > > Signed-off-by: Jean-Michel Hautbois \n> > > > ---\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> > > > 4 files changed, 40 insertions(+), 1 deletion(-)\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/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> > > \n> > > We're under ipa/rkisp1/algorithms/algorithm.h\n> > > So with the current style, that might need an extra _ALGORITHM_\n> > > \n> > > But it also feels a bit redundant to have\n> > > ..._ALGORITHM_ALGORITHM_H__\n> > \n> > Can I volunteer someone to switch all our headers to \"#pragma once\" ?\n> \n> I'd love to ... Can we do that? I thought I'd brought that up before and\n> for some reason we couldn't ...\n\nI've inquired about potential drawbacks before, and haven't found any\nsignificant one, so I think we're good to go.\n\n> Get ready for a patch storm...\n> \n> Ok - so ... not straight away ...\n\n:-)\n\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> > > \n> > > Is this intentionally removed?\n> > > I suspect it should be put after the libipa/camera_sensor_helper.h...\n> > > \n> > > Which means libipa/camera_sensor_helper.h should probably have been put\n> > > before ipa_context.h in the patch that added that...\n> > > \n> > > With the minors resolved:\n> > > \n> > > \n> > > Reviewed-by: Kieran Bingham \n> > > \n> > > > +#include \"algorithms/algorithm.h\"\n> > > > #include \"libipa/camera_sensor_helper.h\"\n> > > > \n> > > > namespace libcamera {\n> > > > @@ -82,6 +82,9 @@ private:\n> > > > \n> > > > /* Local parameter storage */\n> > > > struct IPAContext context_;\n> > > > +\n> > > > + /* Maintain the algorithms used by the IPA */\n> > > > + std::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 2C42EBF415\n\tfor ;\n\tTue, 23 Nov 2021 17:32:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5D36560230;\n\tTue, 23 Nov 2021 18:32:13 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1C2FC60121\n\tfor ;\n\tTue, 23 Nov 2021 18:32:12 +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 87F48A1B;\n\tTue, 23 Nov 2021 18:32:11 +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=\"jpTgExbs\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637688731;\n\tbh=+db0Qay/bazs21llJxt3LaOeK6Wk4BKCX2jJJYRz/7A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jpTgExbs8quB9LArbrfvvbMDZGeG6cf3fADSs0n/4YcVYWEbfgTPK9GLGVM9Cql8p\n\tjujguFCMOuTCsNnipDzo4f/DQ211VKpT8TKG0yTM8yC92o7APhzGclESVBpMtkIR8A\n\t4Fuu+LOGD4VadDGjPRknsqYRrFmk4hHS4ZpGBdX0=","Date":"Tue, 23 Nov 2021 19:31:49 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20211123150423.125524-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211123150423.125524-8-jeanmichel.hautbois@ideasonboard.com>\n\t<163768168871.3059017.17176016291539554234@Monstersaurus>\n\t\n\t<163768773371.3059017.3704032407790386786@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<163768773371.3059017.3704032407790386786@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v3 07/11] ipa: rkisp1: Use the\n\tAlgorithm class","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\" "}}]