[{"id":15686,"web_url":"https://patchwork.libcamera.org/comment/15686/","msgid":"","date":"2021-03-15T01:24:32","subject":"Re: [libcamera-devel] [RFC PATCH v2 1/3] WIP: ipa: Add a common\n\tinterface for algorithm objects","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, Feb 23, 2021 at 05:40:39PM +0100, Jean-Michel Hautbois wrote:\n> In order to instanciate and use algorithms (AWB, AGC, etc.)\n> there is a need for a common class to define mandatory methods.\n> \n> Instead of reinventing the wheel, reuse what Raspberry Pi has done and\n> adapt to the minimum requirements expected.\n> \n> Signed-off-by: Jean-Michel Hautbois \n> ---\n> src/ipa/libipa/algorithm.cpp | 25 +++++++++++++++++++++++++\n> src/ipa/libipa/algorithm.h | 29 +++++++++++++++++++++++++++++\n> src/ipa/libipa/meson.build | 4 +++-\n> 3 files changed, 57 insertions(+), 1 deletion(-)\n> create mode 100644 src/ipa/libipa/algorithm.cpp\n> create mode 100644 src/ipa/libipa/algorithm.h\n> \n> diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp\n> new file mode 100644\n> index 00000000..24cd0ae2\n> --- /dev/null\n> +++ b/src/ipa/libipa/algorithm.cpp\n> @@ -0,0 +1,25 @@\n> +/* SPDX-License-Identifier: BSD-2-Clause */\n> +/*\n> + * Copyright (C) 2019, Raspberry Pi (Trading) Limited\n> + *\n> + * algorithm.cpp - ISP control algorithms\n> + */\n> +\n> +#include \"algorithm.h\"\n> +\n> +/**\n> + * \\file algorithm.h\n> + * \\brief Algorithm common interface\n> + */\n> +\n> +namespace libcamera {\n> +\n\n/**\n * \\brief The IPA namespace\n *\n * The IPA namespace groups all types specific to IPA modules. It serves as the\n * top-level namespace for the IPA library libipa, and also contains\n * module-specific namespaces for IPA modules.\n */\n\n> +namespace ipa {\n\n/**\n * \\class Algorithm\n * \\brief The base class for all IPA algorithms\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 * to manage algorithms regardless of their specific type.\n */\n\n> +\n> +void Algorithm::initialise() {}\n> +\n> +void Algorithm::process() {}\n> +\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..56cd8172\n> --- /dev/null\n> +++ b/src/ipa/libipa/algorithm.h\n> @@ -0,0 +1,29 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Raspberry Pi (Trading) Limited\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> +class Algorithm\n> +{\n> +public:\n> +\tAlgorithm()\n> +\t{\n> +\t}\n\nThere's no need to provide a default constructor when no other\nconstructors are defined, the compiler will implicitly declare and\ndefine a default constructor.\n\n> +\tvirtual ~Algorithm() = default;\n\nI'd move this to the .cpp file, to match initialise() and process(). You\ncan write\n\nAlgorithm::~Algorithm() = default;\n\nin the .cpp files.\n\n> +\tvirtual void initialise();\n> +\tvirtual void process();\n\nThese two functions are currently unused, you can drop them.\n\nReviewed-by: Laurent Pinchart \n\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 b29ef0f4..585d02a3 100644\n> --- a/src/ipa/libipa/meson.build\n> +++ b/src/ipa/libipa/meson.build\n> @@ -1,13 +1,15 @@\n> # SPDX-License-Identifier: CC0-1.0\n> \n> libipa_headers = files([\n> + 'algorithm.h',\n> ])\n> \n> libipa_sources = files([\n> + 'algorithm.cpp',\n> ])\n> \n> libipa_includes = include_directories('..')\n> \n> -libipa = static_library('ipa', libipa_sources,\n> +libipa = static_library('ipa', [libipa_sources, libipa_headers],\n> include_directories : ipa_includes,\n> dependencies : libcamera_dep)","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 E196BBD80C\n\tfor ;\n\tMon, 15 Mar 2021 01:25:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3ECB96084E;\n\tMon, 15 Mar 2021 02:25:10 +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 A8F35602E0\n\tfor ;\n\tMon, 15 Mar 2021 02:25:08 +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 133D487A;\n\tMon, 15 Mar 2021 02:25:08 +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=\"SZl/Z5kO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615771508;\n\tbh=MjEBFy+S+ej2FVd8NRDZ/mAbUNeLe5V5sLFtfqqSWTI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=SZl/Z5kOO8BU5FarGGdfR9SgHs5/J8lBCaSlnTLEr4/LJW8dQVUkU64LZAqFXzSM9\n\tAD5ZBXCEJiP6GtojAjkDWOQbhrqHQkcaSmHMfW3GYIlqsz190vMv8PfZoWy74jciN7\n\tCk0viyB9hX67u88HkelpZmpJ5KLGLAcyGCluOJic=","Date":"Mon, 15 Mar 2021 03:24:32 +0200","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20210223164041.49932-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20210223164041.49932-2-jeanmichel.hautbois@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210223164041.49932-2-jeanmichel.hautbois@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH v2 1/3] WIP: ipa: Add a common\n\tinterface for algorithm objects","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15723,"web_url":"https://patchwork.libcamera.org/comment/15723/","msgid":"","date":"2021-03-16T16:02:35","subject":"Re: [libcamera-devel] [RFC PATCH v2 1/3] WIP: ipa: Add a common\n\tinterface for algorithm objects","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 15/03/2021 02:24, Laurent Pinchart wrote:\n> Hi Jean-Michel,\n> \n> Thank you for the patch.\n> \n> On Tue, Feb 23, 2021 at 05:40:39PM +0100, Jean-Michel Hautbois wrote:\n>> In order to instanciate and use algorithms (AWB, AGC, etc.)\n>> there is a need for a common class to define mandatory methods.\n>>\n>> Instead of reinventing the wheel, reuse what Raspberry Pi has done and\n>> adapt to the minimum requirements expected.\n>>\n>> Signed-off-by: Jean-Michel Hautbois \n>> ---\n>> src/ipa/libipa/algorithm.cpp | 25 +++++++++++++++++++++++++\n>> src/ipa/libipa/algorithm.h | 29 +++++++++++++++++++++++++++++\n>> src/ipa/libipa/meson.build | 4 +++-\n>> 3 files changed, 57 insertions(+), 1 deletion(-)\n>> create mode 100644 src/ipa/libipa/algorithm.cpp\n>> create mode 100644 src/ipa/libipa/algorithm.h\n>>\n>> diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp\n>> new file mode 100644\n>> index 00000000..24cd0ae2\n>> --- /dev/null\n>> +++ b/src/ipa/libipa/algorithm.cpp\n>> @@ -0,0 +1,25 @@\n>> +/* SPDX-License-Identifier: BSD-2-Clause */\n>> +/*\n>> + * Copyright (C) 2019, Raspberry Pi (Trading) Limited\n>> + *\n>> + * algorithm.cpp - ISP control algorithms\n>> + */\n>> +\n>> +#include \"algorithm.h\"\n>> +\n>> +/**\n>> + * \\file algorithm.h\n>> + * \\brief Algorithm common interface\n>> + */\n>> +\n>> +namespace libcamera {\n>> +\n> \n> /**\n> * \\brief The IPA namespace\n> *\n> * The IPA namespace groups all types specific to IPA modules. It serves as the\n> * top-level namespace for the IPA library libipa, and also contains\n> * module-specific namespaces for IPA modules.\n> */\n> \n>> +namespace ipa {\n> \n> /**\n> * \\class Algorithm\n> * \\brief The base class for all IPA algorithms\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> * to manage algorithms regardless of their specific type.\n> */\n> \n>> +\n>> +void Algorithm::initialise() {}\n>> +\n>> +void Algorithm::process() {}\n>> +\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..56cd8172\n>> --- /dev/null\n>> +++ b/src/ipa/libipa/algorithm.h\n>> @@ -0,0 +1,29 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2019, Raspberry Pi (Trading) Limited\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>> +class Algorithm\n>> +{\n>> +public:\n>> +\tAlgorithm()\n>> +\t{\n>> +\t}\n> \n> There's no need to provide a default constructor when no other\n> constructors are defined, the compiler will implicitly declare and\n> define a default constructor.\n> \n>> +\tvirtual ~Algorithm() = default;\n> \n> I'd move this to the .cpp file, to match initialise() and process(). You\n> can write\n> \n> Algorithm::~Algorithm() = default;\n> \n> in the .cpp files.\n> \n>> +\tvirtual void initialise();\n>> +\tvirtual void process();\n> \n> These two functions are currently unused, you can drop them.\n\nIt means class Algorithm would be totally empty, so if I move the\ndestructor I get :\n../src/ipa/libipa/algorithm.cpp:35:12: error: definition of implicitly\ndeclared destructor\nAlgorithm::~Algorithm() = default;\n\n> Reviewed-by: Laurent Pinchart \n> \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 b29ef0f4..585d02a3 100644\n>> --- a/src/ipa/libipa/meson.build\n>> +++ b/src/ipa/libipa/meson.build\n>> @@ -1,13 +1,15 @@\n>> # SPDX-License-Identifier: CC0-1.0\n>> \n>> libipa_headers = files([\n>> + 'algorithm.h',\n>> ])\n>> \n>> libipa_sources = files([\n>> + 'algorithm.cpp',\n>> ])\n>> \n>> libipa_includes = include_directories('..')\n>> \n>> -libipa = static_library('ipa', libipa_sources,\n>> +libipa = static_library('ipa', [libipa_sources, libipa_headers],\n>> include_directories : ipa_includes,\n>> dependencies : libcamera_dep)\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 E65E6C32E1\n\tfor ;\n\tTue, 16 Mar 2021 16:02:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5287568D4E;\n\tTue, 16 Mar 2021 17:02:37 +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 2628368D48\n\tfor ;\n\tTue, 16 Mar 2021 17:02:36 +0100 (CET)","from [IPv6:2a01:e0a:169:7140:f840:1816:ba68:a009] (unknown\n\t[IPv6:2a01:e0a:169:7140:f840:1816:ba68:a009])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A0BE18C8;\n\tTue, 16 Mar 2021 17:02: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=\"dJ3Od23c\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615910555;\n\tbh=YKkghCDcgAKvqGxBXOCfSx15FeXH/yBghowzqGyxprY=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=dJ3Od23c+a+mq5FKGsXOIMDl3+9//zrVzMzO74xheW26+5HYFsv7PS3cqJcDQ5zOk\n\tJGMduRN3JxpVLzOLMF6u2ffZRqkb6DNn4ZoOoP+18i+OtAfEX+PR4rtBHe+aUXGGq+\n\t2pDNy69Dp2QsiT6uV4xpu/+5tAppz+15whA/fJK4=","To":"Laurent Pinchart ,\n\tJean-Michel Hautbois ","References":"<20210223164041.49932-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20210223164041.49932-2-jeanmichel.hautbois@ideasonboard.com>\n\t","From":"Jean-Michel Hautbois ","Message-ID":"","Date":"Tue, 16 Mar 2021 17:02:35 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.7.1","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [RFC PATCH v2 1/3] WIP: ipa: Add a common\n\tinterface for algorithm objects","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15724,"web_url":"https://patchwork.libcamera.org/comment/15724/","msgid":"<8c3b5acd-0cb8-13e0-2987-78f066ab20de@ideasonboard.com>","date":"2021-03-16T16:23:12","subject":"Re: [libcamera-devel] [RFC PATCH v2 1/3] WIP: ipa: Add a common\n\tinterface for algorithm objects","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi JM,\n\nOn 16/03/2021 17:02, Jean-Michel Hautbois wrote:\n> Hi Laurent,\n> \n> On 15/03/2021 02:24, Laurent Pinchart wrote:\n>> Hi Jean-Michel,\n>>\n>> Thank you for the patch.\n>>\n>> On Tue, Feb 23, 2021 at 05:40:39PM +0100, Jean-Michel Hautbois wrote:\n>>> In order to instanciate and use algorithms (AWB, AGC, etc.)\n>>> there is a need for a common class to define mandatory methods.\n>>>\n>>> Instead of reinventing the wheel, reuse what Raspberry Pi has done and\n>>> adapt to the minimum requirements expected.\n>>>\n>>> Signed-off-by: Jean-Michel Hautbois \n>>> ---\n>>> src/ipa/libipa/algorithm.cpp | 25 +++++++++++++++++++++++++\n>>> src/ipa/libipa/algorithm.h | 29 +++++++++++++++++++++++++++++\n>>> src/ipa/libipa/meson.build | 4 +++-\n>>> 3 files changed, 57 insertions(+), 1 deletion(-)\n>>> create mode 100644 src/ipa/libipa/algorithm.cpp\n>>> create mode 100644 src/ipa/libipa/algorithm.h\n>>>\n>>> diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp\n>>> new file mode 100644\n>>> index 00000000..24cd0ae2\n>>> --- /dev/null\n>>> +++ b/src/ipa/libipa/algorithm.cpp\n>>> @@ -0,0 +1,25 @@\n>>> +/* SPDX-License-Identifier: BSD-2-Clause */\n>>> +/*\n>>> + * Copyright (C) 2019, Raspberry Pi (Trading) Limited\n>>> + *\n>>> + * algorithm.cpp - ISP control algorithms\n>>> + */\n>>> +\n>>> +#include \"algorithm.h\"\n>>> +\n>>> +/**\n>>> + * \\file algorithm.h\n>>> + * \\brief Algorithm common interface\n>>> + */\n>>> +\n>>> +namespace libcamera {\n>>> +\n>>\n>> /**\n>> * \\brief The IPA namespace\n>> *\n>> * The IPA namespace groups all types specific to IPA modules. It serves as the\n>> * top-level namespace for the IPA library libipa, and also contains\n>> * module-specific namespaces for IPA modules.\n>> */\n>>\n>>> +namespace ipa {\n>>\n>> /**\n>> * \\class Algorithm\n>> * \\brief The base class for all IPA algorithms\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>> * to manage algorithms regardless of their specific type.\n>> */\n>>\n>>> +\n>>> +void Algorithm::initialise() {}\n>>> +\n>>> +void Algorithm::process() {}\n>>> +\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..56cd8172\n>>> --- /dev/null\n>>> +++ b/src/ipa/libipa/algorithm.h\n>>> @@ -0,0 +1,29 @@\n>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>> +/*\n>>> + * Copyright (C) 2019, Raspberry Pi (Trading) Limited\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>>> +class Algorithm\n>>> +{\n>>> +public:\n>>> +\tAlgorithm()\n>>> +\t{\n>>> +\t}\n>>\n>> There's no need to provide a default constructor when no other\n>> constructors are defined, the compiler will implicitly declare and\n>> define a default constructor.\n>>\n>>> +\tvirtual ~Algorithm() = default;\n>>\n>> I'd move this to the .cpp file, to match initialise() and process(). You\n>> can write\n>>\n>> Algorithm::~Algorithm() = default;\n>>\n>> in the .cpp files.\n>>\n>>> +\tvirtual void initialise();\n>>> +\tvirtual void process();\n>>\n>> These two functions are currently unused, you can drop them.\n> \n> It means class Algorithm would be totally empty, so if I move the\n> destructor I get :\n> ../src/ipa/libipa/algorithm.cpp:35:12: error: definition of implicitly\n> declared destructor\n> Algorithm::~Algorithm() = default;\n\nWell, no you keep the declaration in Algorithm class like:\nvirtual ~Algorithm();\n\nBut you declare it in cpp file :-).\n\n>> Reviewed-by: Laurent Pinchart \nThanks !\n\n>>\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 b29ef0f4..585d02a3 100644\n>>> --- a/src/ipa/libipa/meson.build\n>>> +++ b/src/ipa/libipa/meson.build\n>>> @@ -1,13 +1,15 @@\n>>> # SPDX-License-Identifier: CC0-1.0\n>>> \n>>> libipa_headers = files([\n>>> + 'algorithm.h',\n>>> ])\n>>> \n>>> libipa_sources = files([\n>>> + 'algorithm.cpp',\n>>> ])\n>>> \n>>> libipa_includes = include_directories('..')\n>>> \n>>> -libipa = static_library('ipa', libipa_sources,\n>>> +libipa = static_library('ipa', [libipa_sources, libipa_headers],\n>>> include_directories : ipa_includes,\n>>> dependencies : libcamera_dep)\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 0F878BD80C\n\tfor ;\n\tTue, 16 Mar 2021 16:23:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 84EAE68D49;\n\tTue, 16 Mar 2021 17:23:14 +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 CE98368D48\n\tfor ;\n\tTue, 16 Mar 2021 17:23:12 +0100 (CET)","from [IPv6:2a01:e0a:169:7140:f840:1816:ba68:a009] (unknown\n\t[IPv6:2a01:e0a:169:7140:f840:1816:ba68:a009])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 56E9E8C8;\n\tTue, 16 Mar 2021 17:23:12 +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=\"E7LL94Vk\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615911792;\n\tbh=xHu4+kANxCm+ms4oM7/Nq12/llje+wadLw/Ok99rzjA=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=E7LL94VkQFoS27i3vODU6wQT57HmhteF7MGl4+HEmljqJzDZF3TR2NchfgBguUHOx\n\tByZjMJiT+znRu27EhBOFXJNUEdslx8ji6gscVAmWxlK1cK6Ue+Ou7QOBJaUBDkkuVv\n\tcLKpf5b1ompMonqIbNLVdfd0zNIjmugCco21fjCk=","To":"Jean-Michel Hautbois ,\n\tLaurent Pinchart ","References":"<20210223164041.49932-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20210223164041.49932-2-jeanmichel.hautbois@ideasonboard.com>\n\t\n\t","From":"Jean-Michel Hautbois ","Message-ID":"<8c3b5acd-0cb8-13e0-2987-78f066ab20de@ideasonboard.com>","Date":"Tue, 16 Mar 2021 17:23:12 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.7.1","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [RFC PATCH v2 1/3] WIP: ipa: Add a common\n\tinterface for algorithm objects","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15725,"web_url":"https://patchwork.libcamera.org/comment/15725/","msgid":"","date":"2021-03-16T16:33:43","subject":"Re: [libcamera-devel] [RFC PATCH v2 1/3] WIP: ipa: Add a common\n\tinterface for algorithm objects","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jean-Michel,\n\nOn Tue, Mar 16, 2021 at 05:02:35PM +0100, Jean-Michel Hautbois wrote:\n> On 15/03/2021 02:24, Laurent Pinchart wrote:\n> > On Tue, Feb 23, 2021 at 05:40:39PM +0100, Jean-Michel Hautbois wrote:\n> >> In order to instanciate and use algorithms (AWB, AGC, etc.)\n> >> there is a need for a common class to define mandatory methods.\n> >>\n> >> Instead of reinventing the wheel, reuse what Raspberry Pi has done and\n> >> adapt to the minimum requirements expected.\n> >>\n> >> Signed-off-by: Jean-Michel Hautbois \n> >> ---\n> >> src/ipa/libipa/algorithm.cpp | 25 +++++++++++++++++++++++++\n> >> src/ipa/libipa/algorithm.h | 29 +++++++++++++++++++++++++++++\n> >> src/ipa/libipa/meson.build | 4 +++-\n> >> 3 files changed, 57 insertions(+), 1 deletion(-)\n> >> create mode 100644 src/ipa/libipa/algorithm.cpp\n> >> create mode 100644 src/ipa/libipa/algorithm.h\n> >>\n> >> diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp\n> >> new file mode 100644\n> >> index 00000000..24cd0ae2\n> >> --- /dev/null\n> >> +++ b/src/ipa/libipa/algorithm.cpp\n> >> @@ -0,0 +1,25 @@\n> >> +/* SPDX-License-Identifier: BSD-2-Clause */\n> >> +/*\n> >> + * Copyright (C) 2019, Raspberry Pi (Trading) Limited\n> >> + *\n> >> + * algorithm.cpp - ISP control algorithms\n> >> + */\n> >> +\n> >> +#include \"algorithm.h\"\n> >> +\n> >> +/**\n> >> + * \\file algorithm.h\n> >> + * \\brief Algorithm common interface\n> >> + */\n> >> +\n> >> +namespace libcamera {\n> >> +\n> > \n> > /**\n> > * \\brief The IPA namespace\n> > *\n> > * The IPA namespace groups all types specific to IPA modules. It serves as the\n> > * top-level namespace for the IPA library libipa, and also contains\n> > * module-specific namespaces for IPA modules.\n> > */\n> > \n> >> +namespace ipa {\n> > \n> > /**\n> > * \\class Algorithm\n> > * \\brief The base class for all IPA algorithms\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> > * to manage algorithms regardless of their specific type.\n> > */\n> > \n> >> +\n> >> +void Algorithm::initialise() {}\n> >> +\n> >> +void Algorithm::process() {}\n> >> +\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..56cd8172\n> >> --- /dev/null\n> >> +++ b/src/ipa/libipa/algorithm.h\n> >> @@ -0,0 +1,29 @@\n> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) 2019, Raspberry Pi (Trading) Limited\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> >> +class Algorithm\n> >> +{\n> >> +public:\n> >> +\tAlgorithm()\n> >> +\t{\n> >> +\t}\n> > \n> > There's no need to provide a default constructor when no other\n> > constructors are defined, the compiler will implicitly declare and\n> > define a default constructor.\n> > \n> >> +\tvirtual ~Algorithm() = default;\n> > \n> > I'd move this to the .cpp file, to match initialise() and process(). You\n> > can write\n> > \n> > Algorithm::~Algorithm() = default;\n> > \n> > in the .cpp files.\n> > \n> >> +\tvirtual void initialise();\n> >> +\tvirtual void process();\n> > \n> > These two functions are currently unused, you can drop them.\n> \n> It means class Algorithm would be totally empty, so if I move the\n> destructor I get :\n> ../src/ipa/libipa/algorithm.cpp:35:12: error: definition of implicitly\n> declared destructor\n> Algorithm::~Algorithm() = default;\n\nYou need to keep the declaration of the virtual destructor in the class\ndefinition, it's only its definition that is moved to the .cpp file.\n\n> > Reviewed-by: Laurent Pinchart \n> > \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 b29ef0f4..585d02a3 100644\n> >> --- a/src/ipa/libipa/meson.build\n> >> +++ b/src/ipa/libipa/meson.build\n> >> @@ -1,13 +1,15 @@\n> >> # SPDX-License-Identifier: CC0-1.0\n> >> \n> >> libipa_headers = files([\n> >> + 'algorithm.h',\n> >> ])\n> >> \n> >> libipa_sources = files([\n> >> + 'algorithm.cpp',\n> >> ])\n> >> \n> >> libipa_includes = include_directories('..')\n> >> \n> >> -libipa = static_library('ipa', libipa_sources,\n> >> +libipa = static_library('ipa', [libipa_sources, libipa_headers],\n> >> include_directories : ipa_includes,\n> >> dependencies : libcamera_dep)\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 E91D6C32E1\n\tfor ;\n\tTue, 16 Mar 2021 16:34:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 66A3468D49;\n\tTue, 16 Mar 2021 17:34:22 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 87A4668D48\n\tfor ;\n\tTue, 16 Mar 2021 17:34:20 +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 01E5F8C8;\n\tTue, 16 Mar 2021 17:34:19 +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=\"H/eRX/0P\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615912460;\n\tbh=GVFHqznEhAnOgYqZRsG8IwtRVVXsh6bTZd7xxRshktM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=H/eRX/0PohB6GyKRCB4YCLJI5oJl24Zvi1GRsoR00b5YhVXQVZ9uhygJ38i5zHt7h\n\tccv/KMnIi9RuLgpNoFuHCxiFDwYLsQRjsczz5aH9uVvhleXoFDxUm1t58HHil/KeSj\n\tmRlcu6iBngPWiykOMK2tweXKwAVAeQE3iyZ/BLrk=","Date":"Tue, 16 Mar 2021 18:33:43 +0200","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20210223164041.49932-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20210223164041.49932-2-jeanmichel.hautbois@ideasonboard.com>\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 1/3] WIP: ipa: Add a common\n\tinterface for algorithm objects","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]