[{"id":26681,"web_url":"https://patchwork.libcamera.org/comment/26681/","msgid":"<20230320200219.3waxbayqusxk2wvz@uno.localdomain>","date":"2023-03-20T20:02:19","subject":"Re: [libcamera-devel] [PATCH v4 03/10] ipa: Add base class defining\n\tAF algorithm interface","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Daniel\n\nOn Tue, Mar 14, 2023 at 03:48:27PM +0100, Daniel Semkowicz wrote:\n> Define common interface with basic methods that should be supported\n> by the Auto Focus algorithms. Interface methods match the AF controls\n\nDefine common interface with pure virtual methods that should be\nimplemented by the Auto Focus algorithm implementations.\n\n> that can be set in the frame request.\n> Common implementation of controls parsing is provided, so the AF\n> algorithms deriving from this interface should be able to reuse it.\n>\n> Signed-off-by: Daniel Semkowicz \n> ---\n> src/ipa/libipa/algorithms/af.cpp | 155 ++++++++++++++++++++++++++\n> src/ipa/libipa/algorithms/af.h | 41 +++++++\n> src/ipa/libipa/algorithms/meson.build | 9 ++\n> src/ipa/libipa/meson.build | 6 +\n> 4 files changed, 211 insertions(+)\n> create mode 100644 src/ipa/libipa/algorithms/af.cpp\n> create mode 100644 src/ipa/libipa/algorithms/af.h\n> create mode 100644 src/ipa/libipa/algorithms/meson.build\n>\n> diff --git a/src/ipa/libipa/algorithms/af.cpp b/src/ipa/libipa/algorithms/af.cpp\n> new file mode 100644\n> index 00000000..2052080f\n> --- /dev/null\n> +++ b/src/ipa/libipa/algorithms/af.cpp\n> @@ -0,0 +1,155 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Theobroma Systems\n> + *\n> + * af.cpp - Autofocus control algorithm interface\n> + */\n> +\n> +#include \"af.h\"\n> +\n> +/**\n> + * \\file af.h\n> + * \\brief AF algorithm common interface\n> + */\n> +\n> +namespace libcamera::ipa::common::algorithms {\n\nNit: other algrithms implementations have\n\nnamespace libcamera {\n\nnamespace ipa::...\n\n}\n\n}\n\nAlso, we have namespaces as ipa::ipu3::algorithms and\nipa::rkisp1::algorithms... Should this be just ipa::algorithms ?\n\n> +\n> +/**\n> + * \\class Af\n> + * \\brief Common interface for auto-focus algorithms\n> + *\n> + * The Af class defines a standard interface for IPA auto focus algorithms.\n> + */\n> +\n> +/**\n> + * \\brief Provide control values to the algorithm\n> + * \\param[in] controls The list of user controls\n> + *\n> + * This method should be called in the libcamera::ipa::Algorithm::queueRequest()\n> + * method of the platform layer.\n\nIt probably make sense not using \\copydoc of Algorithm::queueRequest() as\nthe function signature is different like you've done..\n\n> + */\n> +void Af::queueRequest(const ControlList &controls)\n> +{\n> +\tusing namespace controls;\n> +\n> +\tfor (auto const &[id, value] : controls) {\n> +\t\tswitch (id) {\n\nNot a big fan of switching on controls' numeric ids, but in this case\nI guess it's effective...\n\n> +\t\tcase AF_MODE: {\n> +\t\t\tsetMode(static_cast(value.get()));\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase AF_RANGE: {\n> +\t\t\tsetRange(static_cast(value.get()));\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase AF_SPEED: {\n> +\t\t\tsetSpeed(static_cast(value.get()));\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase AF_METERING: {\n> +\t\t\tsetMeteringMode(static_cast(value.get()));\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase AF_WINDOWS: {\n> +\t\t\tsetWindows(value.get>());\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase AF_TRIGGER: {\n> +\t\t\tsetTrigger(static_cast(value.get()));\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase AF_PAUSE: {\n> +\t\t\tsetPause(static_cast(value.get()));\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase LENS_POSITION: {\n> +\t\t\tsetLensPosition(value.get());\n\nMmmm I wonder if the algorithm's state machine (in example: you can't\nset LensPosition if running in a mode !Manual) could be implemented in\nthis base class. However this is not a blocker for this patch\n\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tdefault:\n\nShould we error here ?\n\n> +\t\t\tbreak;\n> +\t\t}\n> +\t}\n> +}\n> +\n> +/**\n> + * \\fn Af::setMode()\n> + * \\copybrief libcamera::controls::AfMode\n\nit's a bit weird to read a function documentation as:\n\"Control to set the mode of the AF (autofocus) algorithm.\"\n\nI'm all for referring to the controls (maybe with the \\sa anchor)\nbut I'm afraid the functions need a bit of documentation of their own\n\nWe can help with that\n\n> + * \\param[in] mode AF mode\n> + *\n> + * \\copydetails libcamera::controls::AfMode\n> + */\n> +\n> +/**\n> + * \\fn Af::setRange()\n> + * \\copybrief libcamera::controls::AfRange\n> + * \\param[in] range AF range\n> + *\n> + * \\copydetails libcamera::controls::AfRange\n> + */\n> +\n> +/**\n> + * \\fn Af::setSpeed()\n> + * \\copybrief libcamera::controls::AfSpeed\n> + * \\param[in] speed Lens move speed\n> + *\n> +* \\copydetails libcamera::controls::AfSpeed\n> + */\n> +\n> +/**\n> + * \\fn Af::setMeteringMode()\n> + * \\copybrief libcamera::controls::AfMetering\n> + * \\param[in] metering AF metering mode\n> + *\n> + * \\copydetails libcamera::controls::AfMetering\n> + */\n> +\n> +/**\n> + * \\fn Af::setWindows()\n> + * \\copybrief libcamera::controls::AfWindows\n> + * \\param[in] windows AF windows\n> + *\n> + * \\copydetails libcamera::controls::AfWindows\n> + */\n> +\n> +/**\n> + * \\fn Af::setTrigger()\n> + * \\copybrief libcamera::controls::AfTrigger\n> + * \\param[in] trigger Trigger mode\n> + *\n> + * \\copydetails libcamera::controls::AfTrigger\n> + */\n> +\n> +/**\n> + * \\fn Af::setPause()\n> + * \\copybrief libcamera::controls::AfPause\n> + * \\param[in] pause Pause mode\n> + *\n> + * \\copydetails libcamera::controls::AfPause\n> + */\n> +\n> +/**\n> + * \\fn Af::setLensPosition()\n> + * \\copybrief libcamera::controls::LensPosition\n> + * \\param[in] lensPosition Lens position\n> + *\n> + * \\copydetails libcamera::controls::LensPosition\n> + */\n> +\n> +/**\n> + * \\fn Af::state()\n> + * \\copybrief libcamera::controls::AfState\n> + * \\return AF state\n> + *\n> + * \\copydetails libcamera::controls::AfState\n> + */\n> +\n> +/**\n> + * \\fn Af::pauseState()\n> + * \\copybrief libcamera::controls::AfPauseState\n> + * \\return AF pause state\n> + *\n> + * \\copydetails libcamera::controls::AfPauseState\n> + */\n> +\n> +} /* namespace libcamera::ipa::common::algorithms */\n> diff --git a/src/ipa/libipa/algorithms/af.h b/src/ipa/libipa/algorithms/af.h\n> new file mode 100644\n> index 00000000..1428902c\n> --- /dev/null\n> +++ b/src/ipa/libipa/algorithms/af.h\n> @@ -0,0 +1,41 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Theobroma Systems\n> + *\n> + * af.h - Autofocus control algorithm interface\n> + */\n> +#pragma once\n> +\n> +#include \n> +\n> +namespace libcamera::ipa::common::algorithms {\n> +\n> +class Af\n> +{\n> +public:\n> +\tvirtual ~Af() = default;\n> +\n> +\tvoid queueRequest(const ControlList &controls);\n> +\n> +\tvirtual void setMode(controls::AfModeEnum mode) = 0;\n> +\n> +\tvirtual void setRange(controls::AfRangeEnum range) = 0;\n> +\n> +\tvirtual void setSpeed(controls::AfSpeedEnum speed) = 0;\n> +\n> +\tvirtual void setMeteringMode(controls::AfMeteringEnum metering) = 0;\n> +\n> +\tvirtual void setWindows(Span windows) = 0;\n> +\n> +\tvirtual void setTrigger(controls::AfTriggerEnum trigger) = 0;\n> +\n> +\tvirtual void setPause(controls::AfPauseEnum pause) = 0;\n> +\n> +\tvirtual void setLensPosition(float lensPosition) = 0;\n> +\n> +\tvirtual controls::AfStateEnum state() = 0;\n> +\n> +\tvirtual controls::AfPauseStateEnum pauseState() = 0;\n> +};\n> +\n> +} /* namespace libcamera::ipa::common::algorithms */\n> diff --git a/src/ipa/libipa/algorithms/meson.build b/src/ipa/libipa/algorithms/meson.build\n> new file mode 100644\n> index 00000000..7602976c\n> --- /dev/null\n> +++ b/src/ipa/libipa/algorithms/meson.build\n> @@ -0,0 +1,9 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +\n> +common_ipa_algorithms_headers = files([\n\nI would s/common_ipa_/libipa_/\n\n> + 'af.h',\n> +])\n> +\n> +common_ipa_algorithms_sources = files([\n> + 'af.cpp',\n> +])\n> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n> index 016b8e0e..0cfc551a 100644\n> --- a/src/ipa/libipa/meson.build\n> +++ b/src/ipa/libipa/meson.build\n> @@ -1,5 +1,7 @@\n> # SPDX-License-Identifier: CC0-1.0\n>\n> +subdir('algorithms')\n> +\n> libipa_headers = files([\n> 'algorithm.h',\n> 'camera_sensor_helper.h',\n> @@ -8,6 +10,8 @@ libipa_headers = files([\n> 'module.h',\n> ])\n>\n> +libipa_headers += common_ipa_algorithms_headers\n> +\n> libipa_sources = files([\n> 'algorithm.cpp',\n> 'camera_sensor_helper.cpp',\n> @@ -16,6 +20,8 @@ libipa_sources = files([\n> 'module.cpp',\n> ])\n>\n> +libipa_sources += common_ipa_algorithms_sources\n> +\n\nMostly minors though! We can fix on top if a new version is not\nrequired\n\n> libipa_includes = include_directories('..')\n>\n> libipa = static_library('ipa', [libipa_sources, libipa_headers],\n> --\n> 2.39.2\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 42D7EC0F1B\n\tfor ;\n\tMon, 20 Mar 2023 20:02:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5789D626E3;\n\tMon, 20 Mar 2023 21:02: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 4165961ED4\n\tfor ;\n\tMon, 20 Mar 2023 21:02:23 +0100 (CET)","from ideasonboard.com (host-87-18-61-243.retail.telecomitalia.it\n\t[87.18.61.243])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6BEDEA25;\n\tMon, 20 Mar 2023 21:02:22 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679342544;\n\tbh=5d2djDOoV5Sxw6KetzCAfjSA8RB4NZfGWdPor70KORQ=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=eD4mITKNsMvoCmL7hPpykzc3LPCvYelMiSXEPoRaU9EHHPr/yl8m0BL762OFfEQFv\n\tWViMP//I+Td4ZIYrf3PsuK3yvSrApnaJ+BuZUk9M2353XLPNhKXLwz7mNClPgVlQiE\n\tjah/UpU4LW2w68qr6p1PrARAHWTn915/BydBlCpGdXi9hOpleF5UHalqtxZeEF/cw6\n\tv7Y7x/eQnHRU8ULKqb0tq0HNo5FpBQ34w3eSvVIvHWmf850MfbY9VBGAj0Gvy+R3hl\n\t6KXZnFNv6GLrz1mcgbfyqh44oQqSMvMkgXLfvI6oHAWYFoZyonl6ja0aazw8uHGHzs\n\t82Gg24lnjyJ2Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1679342542;\n\tbh=5d2djDOoV5Sxw6KetzCAfjSA8RB4NZfGWdPor70KORQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LQyHWEufwjOtty1aQnQvVmt2/96N6RGogr0JXGe8lkuJaQXmHo37vTsOuggqi864V\n\tnGo/9S1Jv7DJjVhoCUh4rMUdYLkVR5JqleUEY00w4nGXhuXrAd4rTp+2zhOfpf1N0J\n\tjRY5F9o699axOIvhfsJ9SnmvKsElCPB8bgxyHSLI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"LQyHWEuf\"; dkim-atps=neutral","Date":"Mon, 20 Mar 2023 21:02:19 +0100","To":"Daniel Semkowicz ","Message-ID":"<20230320200219.3waxbayqusxk2wvz@uno.localdomain>","References":"<20230314144834.85193-1-dse@thaumatec.com>\n\t<20230314144834.85193-4-dse@thaumatec.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230314144834.85193-4-dse@thaumatec.com>","Subject":"Re: [libcamera-devel] [PATCH v4 03/10] ipa: Add base class defining\n\tAF algorithm interface","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"jacopo.mondi@ideasonboard.com, libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26690,"web_url":"https://patchwork.libcamera.org/comment/26690/","msgid":"","date":"2023-03-21T07:09:12","subject":"Re: [libcamera-devel] [PATCH v4 03/10] ipa: Add base class defining\n\tAF algorithm interface","submitter":{"id":126,"url":"https://patchwork.libcamera.org/api/people/126/","name":"Daniel Semkowicz","email":"dse@thaumatec.com"},"content":"Hi Jacopo,\n\nOn Mon, Mar 20, 2023 at 9:02 PM Jacopo Mondi\n wrote:\n>\n> Hi Daniel\n>\n> On Tue, Mar 14, 2023 at 03:48:27PM +0100, Daniel Semkowicz wrote:\n> > Define common interface with basic methods that should be supported\n> > by the Auto Focus algorithms. Interface methods match the AF controls\n>\n> Define common interface with pure virtual methods that should be\n> implemented by the Auto Focus algorithm implementations.\n>\n> > that can be set in the frame request.\n> > Common implementation of controls parsing is provided, so the AF\n> > algorithms deriving from this interface should be able to reuse it.\n> >\n> > Signed-off-by: Daniel Semkowicz \n> > ---\n> > src/ipa/libipa/algorithms/af.cpp | 155 ++++++++++++++++++++++++++\n> > src/ipa/libipa/algorithms/af.h | 41 +++++++\n> > src/ipa/libipa/algorithms/meson.build | 9 ++\n> > src/ipa/libipa/meson.build | 6 +\n> > 4 files changed, 211 insertions(+)\n> > create mode 100644 src/ipa/libipa/algorithms/af.cpp\n> > create mode 100644 src/ipa/libipa/algorithms/af.h\n> > create mode 100644 src/ipa/libipa/algorithms/meson.build\n> >\n> > diff --git a/src/ipa/libipa/algorithms/af.cpp b/src/ipa/libipa/algorithms/af.cpp\n> > new file mode 100644\n> > index 00000000..2052080f\n> > --- /dev/null\n> > +++ b/src/ipa/libipa/algorithms/af.cpp\n> > @@ -0,0 +1,155 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2023, Theobroma Systems\n> > + *\n> > + * af.cpp - Autofocus control algorithm interface\n> > + */\n> > +\n> > +#include \"af.h\"\n> > +\n> > +/**\n> > + * \\file af.h\n> > + * \\brief AF algorithm common interface\n> > + */\n> > +\n> > +namespace libcamera::ipa::common::algorithms {\n>\n> Nit: other algrithms implementations have\n>\n> namespace libcamera {\n>\n> namespace ipa::...\n>\n> }\n>\n> }\n>\n> Also, we have namespaces as ipa::ipu3::algorithms and\n> ipa::rkisp1::algorithms... Should this be just ipa::algorithms ?\n>\n\nSure, I will fix that.\n\n> > +\n> > +/**\n> > + * \\class Af\n> > + * \\brief Common interface for auto-focus algorithms\n> > + *\n> > + * The Af class defines a standard interface for IPA auto focus algorithms.\n> > + */\n> > +\n> > +/**\n> > + * \\brief Provide control values to the algorithm\n> > + * \\param[in] controls The list of user controls\n> > + *\n> > + * This method should be called in the libcamera::ipa::Algorithm::queueRequest()\n> > + * method of the platform layer.\n>\n> It probably make sense not using \\copydoc of Algorithm::queueRequest() as\n> the function signature is different like you've done..\n\nYou are right. I will document the parameters separately. Do you think adding\na reference as \"see also\" makes sense here, or the difference is too big?\n\n>\n> > + */\n> > +void Af::queueRequest(const ControlList &controls)\n> > +{\n> > + using namespace controls;\n> > +\n> > + for (auto const &[id, value] : controls) {\n> > + switch (id) {\n>\n> Not a big fan of switching on controls' numeric ids, but in this case\n> I guess it's effective...\n>\n> > + case AF_MODE: {\n> > + setMode(static_cast(value.get()));\n> > + break;\n> > + }\n> > + case AF_RANGE: {\n> > + setRange(static_cast(value.get()));\n> > + break;\n> > + }\n> > + case AF_SPEED: {\n> > + setSpeed(static_cast(value.get()));\n> > + break;\n> > + }\n> > + case AF_METERING: {\n> > + setMeteringMode(static_cast(value.get()));\n> > + break;\n> > + }\n> > + case AF_WINDOWS: {\n> > + setWindows(value.get>());\n> > + break;\n> > + }\n> > + case AF_TRIGGER: {\n> > + setTrigger(static_cast(value.get()));\n> > + break;\n> > + }\n> > + case AF_PAUSE: {\n> > + setPause(static_cast(value.get()));\n> > + break;\n> > + }\n> > + case LENS_POSITION: {\n> > + setLensPosition(value.get());\n>\n> Mmmm I wonder if the algorithm's state machine (in example: you can't\n> set LensPosition if running in a mode !Manual) could be implemented in\n> this base class. However this is not a blocker for this patch\n\nYes, I had a similar idea, but as this is the first common algorithm\nimplementation, I am not completely sure which parts will be always\nreusable. Since it serves as the Interface for other algorithms, it was\nsafer to leave the details to the \"upper layer\". With additional AF\nalgorithms using this interface, it should be easier to move\nthe common part here.\n\n>\n> > + break;\n> > + }\n> > + default:\n>\n> Should we error here ?\n\nWe can't, because controls are not filtered and there can be controls\nof other algorithms in the list.\n\n>\n> > + break;\n> > + }\n> > + }\n> > +}\n> > +\n> > +/**\n> > + * \\fn Af::setMode()\n> > + * \\copybrief libcamera::controls::AfMode\n>\n> it's a bit weird to read a function documentation as:\n> \"Control to set the mode of the AF (autofocus) algorithm.\"\n>\n> I'm all for referring to the controls (maybe with the \\sa anchor)\n> but I'm afraid the functions need a bit of documentation of their own\n>\n> We can help with that\n\nSo I need to merge the previous version of the functions documentation\nand the current one :D\n\n>\n> > + * \\param[in] mode AF mode\n> > + *\n> > + * \\copydetails libcamera::controls::AfMode\n> > + */\n> > +\n> > +/**\n> > + * \\fn Af::setRange()\n> > + * \\copybrief libcamera::controls::AfRange\n> > + * \\param[in] range AF range\n> > + *\n> > + * \\copydetails libcamera::controls::AfRange\n> > + */\n> > +\n> > +/**\n> > + * \\fn Af::setSpeed()\n> > + * \\copybrief libcamera::controls::AfSpeed\n> > + * \\param[in] speed Lens move speed\n> > + *\n> > +* \\copydetails libcamera::controls::AfSpeed\n> > + */\n> > +\n> > +/**\n> > + * \\fn Af::setMeteringMode()\n> > + * \\copybrief libcamera::controls::AfMetering\n> > + * \\param[in] metering AF metering mode\n> > + *\n> > + * \\copydetails libcamera::controls::AfMetering\n> > + */\n> > +\n> > +/**\n> > + * \\fn Af::setWindows()\n> > + * \\copybrief libcamera::controls::AfWindows\n> > + * \\param[in] windows AF windows\n> > + *\n> > + * \\copydetails libcamera::controls::AfWindows\n> > + */\n> > +\n> > +/**\n> > + * \\fn Af::setTrigger()\n> > + * \\copybrief libcamera::controls::AfTrigger\n> > + * \\param[in] trigger Trigger mode\n> > + *\n> > + * \\copydetails libcamera::controls::AfTrigger\n> > + */\n> > +\n> > +/**\n> > + * \\fn Af::setPause()\n> > + * \\copybrief libcamera::controls::AfPause\n> > + * \\param[in] pause Pause mode\n> > + *\n> > + * \\copydetails libcamera::controls::AfPause\n> > + */\n> > +\n> > +/**\n> > + * \\fn Af::setLensPosition()\n> > + * \\copybrief libcamera::controls::LensPosition\n> > + * \\param[in] lensPosition Lens position\n> > + *\n> > + * \\copydetails libcamera::controls::LensPosition\n> > + */\n> > +\n> > +/**\n> > + * \\fn Af::state()\n> > + * \\copybrief libcamera::controls::AfState\n> > + * \\return AF state\n> > + *\n> > + * \\copydetails libcamera::controls::AfState\n> > + */\n> > +\n> > +/**\n> > + * \\fn Af::pauseState()\n> > + * \\copybrief libcamera::controls::AfPauseState\n> > + * \\return AF pause state\n> > + *\n> > + * \\copydetails libcamera::controls::AfPauseState\n> > + */\n> > +\n> > +} /* namespace libcamera::ipa::common::algorithms */\n> > diff --git a/src/ipa/libipa/algorithms/af.h b/src/ipa/libipa/algorithms/af.h\n> > new file mode 100644\n> > index 00000000..1428902c\n> > --- /dev/null\n> > +++ b/src/ipa/libipa/algorithms/af.h\n> > @@ -0,0 +1,41 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2023, Theobroma Systems\n> > + *\n> > + * af.h - Autofocus control algorithm interface\n> > + */\n> > +#pragma once\n> > +\n> > +#include \n> > +\n> > +namespace libcamera::ipa::common::algorithms {\n> > +\n> > +class Af\n> > +{\n> > +public:\n> > + virtual ~Af() = default;\n> > +\n> > + void queueRequest(const ControlList &controls);\n> > +\n> > + virtual void setMode(controls::AfModeEnum mode) = 0;\n> > +\n> > + virtual void setRange(controls::AfRangeEnum range) = 0;\n> > +\n> > + virtual void setSpeed(controls::AfSpeedEnum speed) = 0;\n> > +\n> > + virtual void setMeteringMode(controls::AfMeteringEnum metering) = 0;\n> > +\n> > + virtual void setWindows(Span windows) = 0;\n> > +\n> > + virtual void setTrigger(controls::AfTriggerEnum trigger) = 0;\n> > +\n> > + virtual void setPause(controls::AfPauseEnum pause) = 0;\n> > +\n> > + virtual void setLensPosition(float lensPosition) = 0;\n> > +\n> > + virtual controls::AfStateEnum state() = 0;\n> > +\n> > + virtual controls::AfPauseStateEnum pauseState() = 0;\n> > +};\n> > +\n> > +} /* namespace libcamera::ipa::common::algorithms */\n> > diff --git a/src/ipa/libipa/algorithms/meson.build b/src/ipa/libipa/algorithms/meson.build\n> > new file mode 100644\n> > index 00000000..7602976c\n> > --- /dev/null\n> > +++ b/src/ipa/libipa/algorithms/meson.build\n> > @@ -0,0 +1,9 @@\n> > +# SPDX-License-Identifier: CC0-1.0\n> > +\n> > +common_ipa_algorithms_headers = files([\n>\n> I would s/common_ipa_/libipa_/\n>\n> > + 'af.h',\n> > +])\n> > +\n> > +common_ipa_algorithms_sources = files([\n> > + 'af.cpp',\n> > +])\n> > diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n> > index 016b8e0e..0cfc551a 100644\n> > --- a/src/ipa/libipa/meson.build\n> > +++ b/src/ipa/libipa/meson.build\n> > @@ -1,5 +1,7 @@\n> > # SPDX-License-Identifier: CC0-1.0\n> >\n> > +subdir('algorithms')\n> > +\n> > libipa_headers = files([\n> > 'algorithm.h',\n> > 'camera_sensor_helper.h',\n> > @@ -8,6 +10,8 @@ libipa_headers = files([\n> > 'module.h',\n> > ])\n> >\n> > +libipa_headers += common_ipa_algorithms_headers\n> > +\n> > libipa_sources = files([\n> > 'algorithm.cpp',\n> > 'camera_sensor_helper.cpp',\n> > @@ -16,6 +20,8 @@ libipa_sources = files([\n> > 'module.cpp',\n> > ])\n> >\n> > +libipa_sources += common_ipa_algorithms_sources\n> > +\n>\n> Mostly minors though! We can fix on top if a new version is not\n> required\n>\n> > libipa_includes = include_directories('..')\n> >\n> > libipa = static_library('ipa', [libipa_sources, libipa_headers],\n> > --\n> > 2.39.2\n> >\n\nBest regards\nDaniel","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 64F16C0F2A\n\tfor ;\n\tTue, 21 Mar 2023 07:09:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 94ACC626E3;\n\tTue, 21 Mar 2023 08:09:25 +0100 (CET)","from mail-ed1-x52a.google.com (mail-ed1-x52a.google.com\n\t[IPv6:2a00:1450:4864:20::52a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1443161ECE\n\tfor ;\n\tTue, 21 Mar 2023 08:09:24 +0100 (CET)","by mail-ed1-x52a.google.com with SMTP id ek18so55872081edb.6\n\tfor ;\n\tTue, 21 Mar 2023 00:09:24 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679382565;\n\tbh=vnQcyOGxW4/MCBXIYzFi53qkaGR2i63b0ICgKWRs7v4=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=1Cpou6polG5GxZP+KUocUsC6UZyQeo2aTM1lzch8ZVzcpPlTSPEyItl4jugkChx07\n\t9rR5+KzPgdhhBs2GF1E74ADMhwc901e4LeXyfPZZOxB9HFsBu6ZhjJMkboJCkU4BZw\n\tcu2t7xAA6kp6cHNiTroUmAedUaPoa5tWQEOI5cTxqPuGhzMvwQIqGcV06riotHAUCx\n\tmFiTJmdxfUkZNZLni7V1U0VcEuzVh2d/hrBiuIkho2imwn6ALT89rscIjVa+gnbgS2\n\tzt3NJw0ZLOo8DQ8N9ZtsbhptT1fhk+3T6xlQ6ZvPWWcODvcmw28izfYzr/4Jry2LW0\n\tMJAFSm8aDmtYw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=thaumatec-com.20210112.gappssmtp.com; s=20210112; t=1679382563;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=SL9qTszscqRn7aVM7dR99qf4Q9bXXLP90j0JA4OLHcE=;\n\tb=lWEp6rF4cuC3uVQYlzY3LT0sKHkQc6RN2svw6eWw4TYQLNpeUKAZ13oAPA38d21I1U\n\tIQwbCA0zckNT4voDjKLy8tcUy+s6pbKCXCkLO8kzttmM3Jpu9BKhayAxAf9+tLBPbZMq\n\tXSIe8DRsRAICU8/2ng4IShNUzQFvF1+yeo7b/O05c3aZhAD1aBvTXvKsa5MLjl6aQZAG\n\tNrIUQY9KKPUkLFKR/x8Wa7cMlPwg1KaMf70bhPrbsxm7//kwVBlHZo2PmJD2JtkXbwY3\n\tf4MYEWwDc6/C1EMA7zgEDrV8P4IsQfN9/U7tHKHnLfcbPubh9rsAi/sVHhoJwRoXRn0d\n\tSW1A=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=thaumatec-com.20210112.gappssmtp.com\n\theader.i=@thaumatec-com.20210112.gappssmtp.com header.b=\"lWEp6rF4\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112; t=1679382563;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=SL9qTszscqRn7aVM7dR99qf4Q9bXXLP90j0JA4OLHcE=;\n\tb=u1jehLiCmdX6vj6vBgIlJcUr//8zMRaKui+g8KpiWvwuzw+GyomCXFXrvAqoghKZ81\n\tzvp8anaKCtF1+GJAfAUxN8t2rvDvA4A6koAk+E6cBJw3FJJm97SOAp0q67ub5+eV3YZf\n\tnHnRHkidHz46/8MofiE/tJQepQ/dStflKRfpB+egZGIAbyWzpa6fLEzpLMI7USl1/Opn\n\tx2TN0VitnmXu6xCH/YQLIR3E0nfkx+U2Ue8mMKzwXnU56ZJU/c2dPH6H2qVRn9WIGVFB\n\tmig+zeJ33mZYILHrIheVwYMoeABKgag69/C9Keo0qOarhNQoJdURNGve5IISj2UaOcTX\n\tp0OQ==","X-Gm-Message-State":"AO0yUKWqqjNuDj7JM9UJOG9VehFoK76fHjVngjAaLOOq+6aHF3wsnHKB\n\toRnKBg6o0ocK6dWUCFuOsS1AgKSy3S6mMyPm+fCG/cOQTfMN/73/rFA=","X-Google-Smtp-Source":"AK7set89NFIFmufYzTfi3/CTa56rQjlk+FHVBDCzjbBtKUtTuD/yHfuBD2bXzodkDh3qWr5zaGhU5FYuN69U/XBU0M8=","X-Received":"by 2002:a17:906:f9d1:b0:924:efbb:8a8b with SMTP id\n\tlj17-20020a170906f9d100b00924efbb8a8bmr801310ejb.6.1679382563466;\n\tTue, 21 Mar 2023 00:09:23 -0700 (PDT)","MIME-Version":"1.0","References":"<20230314144834.85193-1-dse@thaumatec.com>\n\t<20230314144834.85193-4-dse@thaumatec.com>\n\t<20230320200219.3waxbayqusxk2wvz@uno.localdomain>","In-Reply-To":"<20230320200219.3waxbayqusxk2wvz@uno.localdomain>","Date":"Tue, 21 Mar 2023 08:09:12 +0100","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH v4 03/10] ipa: Add base class defining\n\tAF algorithm interface","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","From":"Daniel Semkowicz via libcamera-devel\n\t","Reply-To":"Daniel Semkowicz ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26694,"web_url":"https://patchwork.libcamera.org/comment/26694/","msgid":"<20230321080733.m66zuntkxqix6pdd@uno.localdomain>","date":"2023-03-21T08:07:33","subject":"Re: [libcamera-devel] [PATCH v4 03/10] ipa: Add base class defining\n\tAF algorithm interface","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Daniel\n\nOn Tue, Mar 21, 2023 at 08:09:12AM +0100, Daniel Semkowicz wrote:\n> Hi Jacopo,\n>\n> On Mon, Mar 20, 2023 at 9:02 PM Jacopo Mondi\n> wrote:\n> >\n> > Hi Daniel\n> >\n> > On Tue, Mar 14, 2023 at 03:48:27PM +0100, Daniel Semkowicz wrote:\n> > > Define common interface with basic methods that should be supported\n> > > by the Auto Focus algorithms. Interface methods match the AF controls\n> >\n> > Define common interface with pure virtual methods that should be\n> > implemented by the Auto Focus algorithm implementations.\n> >\n> > > that can be set in the frame request.\n> > > Common implementation of controls parsing is provided, so the AF\n> > > algorithms deriving from this interface should be able to reuse it.\n> > >\n> > > Signed-off-by: Daniel Semkowicz \n> > > ---\n> > > src/ipa/libipa/algorithms/af.cpp | 155 ++++++++++++++++++++++++++\n> > > src/ipa/libipa/algorithms/af.h | 41 +++++++\n> > > src/ipa/libipa/algorithms/meson.build | 9 ++\n> > > src/ipa/libipa/meson.build | 6 +\n> > > 4 files changed, 211 insertions(+)\n> > > create mode 100644 src/ipa/libipa/algorithms/af.cpp\n> > > create mode 100644 src/ipa/libipa/algorithms/af.h\n> > > create mode 100644 src/ipa/libipa/algorithms/meson.build\n> > >\n> > > diff --git a/src/ipa/libipa/algorithms/af.cpp b/src/ipa/libipa/algorithms/af.cpp\n> > > new file mode 100644\n> > > index 00000000..2052080f\n> > > --- /dev/null\n> > > +++ b/src/ipa/libipa/algorithms/af.cpp\n> > > @@ -0,0 +1,155 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2023, Theobroma Systems\n> > > + *\n> > > + * af.cpp - Autofocus control algorithm interface\n> > > + */\n> > > +\n> > > +#include \"af.h\"\n> > > +\n> > > +/**\n> > > + * \\file af.h\n> > > + * \\brief AF algorithm common interface\n> > > + */\n> > > +\n> > > +namespace libcamera::ipa::common::algorithms {\n> >\n> > Nit: other algrithms implementations have\n> >\n> > namespace libcamera {\n> >\n> > namespace ipa::...\n> >\n> > }\n> >\n> > }\n> >\n> > Also, we have namespaces as ipa::ipu3::algorithms and\n> > ipa::rkisp1::algorithms... Should this be just ipa::algorithms ?\n> >\n>\n> Sure, I will fix that.\n>\n> > > +\n> > > +/**\n> > > + * \\class Af\n> > > + * \\brief Common interface for auto-focus algorithms\n> > > + *\n> > > + * The Af class defines a standard interface for IPA auto focus algorithms.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\brief Provide control values to the algorithm\n> > > + * \\param[in] controls The list of user controls\n> > > + *\n> > > + * This method should be called in the libcamera::ipa::Algorithm::queueRequest()\n> > > + * method of the platform layer.\n> >\n> > It probably make sense not using \\copydoc of Algorithm::queueRequest() as\n> > the function signature is different like you've done..\n>\n> You are right. I will document the parameters separately. Do you think adding\n> a reference as \"see also\" makes sense here, or the difference is too big?\n>\n\nSorry, my comment was not clear. I was just saying you've done the\nright thing not using \\copydoc :)\n\nI think this documentation block is fine as it is\n\n> >\n> > > + */\n> > > +void Af::queueRequest(const ControlList &controls)\n> > > +{\n> > > + using namespace controls;\n> > > +\n> > > + for (auto const &[id, value] : controls) {\n> > > + switch (id) {\n> >\n> > Not a big fan of switching on controls' numeric ids, but in this case\n> > I guess it's effective...\n> >\n> > > + case AF_MODE: {\n> > > + setMode(static_cast(value.get()));\n> > > + break;\n> > > + }\n> > > + case AF_RANGE: {\n> > > + setRange(static_cast(value.get()));\n> > > + break;\n> > > + }\n> > > + case AF_SPEED: {\n> > > + setSpeed(static_cast(value.get()));\n> > > + break;\n> > > + }\n> > > + case AF_METERING: {\n> > > + setMeteringMode(static_cast(value.get()));\n> > > + break;\n> > > + }\n> > > + case AF_WINDOWS: {\n> > > + setWindows(value.get>());\n> > > + break;\n> > > + }\n> > > + case AF_TRIGGER: {\n> > > + setTrigger(static_cast(value.get()));\n> > > + break;\n> > > + }\n> > > + case AF_PAUSE: {\n> > > + setPause(static_cast(value.get()));\n> > > + break;\n> > > + }\n> > > + case LENS_POSITION: {\n> > > + setLensPosition(value.get());\n> >\n> > Mmmm I wonder if the algorithm's state machine (in example: you can't\n> > set LensPosition if running in a mode !Manual) could be implemented in\n> > this base class. However this is not a blocker for this patch\n>\n> Yes, I had a similar idea, but as this is the first common algorithm\n> implementation, I am not completely sure which parts will be always\n> reusable. Since it serves as the Interface for other algorithms, it was\n> safer to leave the details to the \"upper layer\". With additional AF\n> algorithms using this interface, it should be easier to move\n> the common part here.\n>\n\nMakes sense!\n\n> >\n> > > + break;\n> > > + }\n> > > + default:\n> >\n> > Should we error here ?\n>\n> We can't, because controls are not filtered and there can be controls\n> of other algorithms in the list.\n>\n\nRight...\n\n> >\n> > > + break;\n> > > + }\n> > > + }\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\fn Af::setMode()\n> > > + * \\copybrief libcamera::controls::AfMode\n> >\n> > it's a bit weird to read a function documentation as:\n> > \"Control to set the mode of the AF (autofocus) algorithm.\"\n> >\n> > I'm all for referring to the controls (maybe with the \\sa anchor)\n> > but I'm afraid the functions need a bit of documentation of their own\n> >\n> > We can help with that\n>\n> So I need to merge the previous version of the functions documentation\n> and the current one :D\n>\n\nLet me know how we can help\n\n> >\n> > > + * \\param[in] mode AF mode\n> > > + *\n> > > + * \\copydetails libcamera::controls::AfMode\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn Af::setRange()\n> > > + * \\copybrief libcamera::controls::AfRange\n> > > + * \\param[in] range AF range\n> > > + *\n> > > + * \\copydetails libcamera::controls::AfRange\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn Af::setSpeed()\n> > > + * \\copybrief libcamera::controls::AfSpeed\n> > > + * \\param[in] speed Lens move speed\n> > > + *\n> > > +* \\copydetails libcamera::controls::AfSpeed\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn Af::setMeteringMode()\n> > > + * \\copybrief libcamera::controls::AfMetering\n> > > + * \\param[in] metering AF metering mode\n> > > + *\n> > > + * \\copydetails libcamera::controls::AfMetering\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn Af::setWindows()\n> > > + * \\copybrief libcamera::controls::AfWindows\n> > > + * \\param[in] windows AF windows\n> > > + *\n> > > + * \\copydetails libcamera::controls::AfWindows\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn Af::setTrigger()\n> > > + * \\copybrief libcamera::controls::AfTrigger\n> > > + * \\param[in] trigger Trigger mode\n> > > + *\n> > > + * \\copydetails libcamera::controls::AfTrigger\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn Af::setPause()\n> > > + * \\copybrief libcamera::controls::AfPause\n> > > + * \\param[in] pause Pause mode\n> > > + *\n> > > + * \\copydetails libcamera::controls::AfPause\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn Af::setLensPosition()\n> > > + * \\copybrief libcamera::controls::LensPosition\n> > > + * \\param[in] lensPosition Lens position\n> > > + *\n> > > + * \\copydetails libcamera::controls::LensPosition\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn Af::state()\n> > > + * \\copybrief libcamera::controls::AfState\n> > > + * \\return AF state\n> > > + *\n> > > + * \\copydetails libcamera::controls::AfState\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn Af::pauseState()\n> > > + * \\copybrief libcamera::controls::AfPauseState\n> > > + * \\return AF pause state\n> > > + *\n> > > + * \\copydetails libcamera::controls::AfPauseState\n> > > + */\n> > > +\n> > > +} /* namespace libcamera::ipa::common::algorithms */\n> > > diff --git a/src/ipa/libipa/algorithms/af.h b/src/ipa/libipa/algorithms/af.h\n> > > new file mode 100644\n> > > index 00000000..1428902c\n> > > --- /dev/null\n> > > +++ b/src/ipa/libipa/algorithms/af.h\n> > > @@ -0,0 +1,41 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2023, Theobroma Systems\n> > > + *\n> > > + * af.h - Autofocus control algorithm interface\n> > > + */\n> > > +#pragma once\n> > > +\n> > > +#include \n> > > +\n> > > +namespace libcamera::ipa::common::algorithms {\n> > > +\n> > > +class Af\n> > > +{\n> > > +public:\n> > > + virtual ~Af() = default;\n> > > +\n> > > + void queueRequest(const ControlList &controls);\n> > > +\n> > > + virtual void setMode(controls::AfModeEnum mode) = 0;\n> > > +\n> > > + virtual void setRange(controls::AfRangeEnum range) = 0;\n> > > +\n> > > + virtual void setSpeed(controls::AfSpeedEnum speed) = 0;\n> > > +\n> > > + virtual void setMeteringMode(controls::AfMeteringEnum metering) = 0;\n> > > +\n> > > + virtual void setWindows(Span windows) = 0;\n> > > +\n> > > + virtual void setTrigger(controls::AfTriggerEnum trigger) = 0;\n> > > +\n> > > + virtual void setPause(controls::AfPauseEnum pause) = 0;\n> > > +\n> > > + virtual void setLensPosition(float lensPosition) = 0;\n> > > +\n> > > + virtual controls::AfStateEnum state() = 0;\n> > > +\n> > > + virtual controls::AfPauseStateEnum pauseState() = 0;\n> > > +};\n> > > +\n> > > +} /* namespace libcamera::ipa::common::algorithms */\n> > > diff --git a/src/ipa/libipa/algorithms/meson.build b/src/ipa/libipa/algorithms/meson.build\n> > > new file mode 100644\n> > > index 00000000..7602976c\n> > > --- /dev/null\n> > > +++ b/src/ipa/libipa/algorithms/meson.build\n> > > @@ -0,0 +1,9 @@\n> > > +# SPDX-License-Identifier: CC0-1.0\n> > > +\n> > > +common_ipa_algorithms_headers = files([\n> >\n> > I would s/common_ipa_/libipa_/\n> >\n> > > + 'af.h',\n> > > +])\n> > > +\n> > > +common_ipa_algorithms_sources = files([\n> > > + 'af.cpp',\n> > > +])\n> > > diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n> > > index 016b8e0e..0cfc551a 100644\n> > > --- a/src/ipa/libipa/meson.build\n> > > +++ b/src/ipa/libipa/meson.build\n> > > @@ -1,5 +1,7 @@\n> > > # SPDX-License-Identifier: CC0-1.0\n> > >\n> > > +subdir('algorithms')\n> > > +\n> > > libipa_headers = files([\n> > > 'algorithm.h',\n> > > 'camera_sensor_helper.h',\n> > > @@ -8,6 +10,8 @@ libipa_headers = files([\n> > > 'module.h',\n> > > ])\n> > >\n> > > +libipa_headers += common_ipa_algorithms_headers\n> > > +\n> > > libipa_sources = files([\n> > > 'algorithm.cpp',\n> > > 'camera_sensor_helper.cpp',\n> > > @@ -16,6 +20,8 @@ libipa_sources = files([\n> > > 'module.cpp',\n> > > ])\n> > >\n> > > +libipa_sources += common_ipa_algorithms_sources\n> > > +\n> >\n> > Mostly minors though! We can fix on top if a new version is not\n> > required\n> >\n> > > libipa_includes = include_directories('..')\n> > >\n> > > libipa = static_library('ipa', [libipa_sources, libipa_headers],\n> > > --\n> > > 2.39.2\n> > >\n>\n> Best regards\n> Daniel","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 CF39DC0F1B\n\tfor ;\n\tTue, 21 Mar 2023 08:07:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1507A626E5;\n\tTue, 21 Mar 2023 09:07:39 +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 41F17626DA\n\tfor ;\n\tTue, 21 Mar 2023 09:07:37 +0100 (CET)","from ideasonboard.com (host-87-18-61-243.retail.telecomitalia.it\n\t[87.18.61.243])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 54CBE496;\n\tTue, 21 Mar 2023 09:07:36 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679386059;\n\tbh=c3hW5h4GGCQrDkrThHa32XcT98yLHHvuwHg4qITkR+Q=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=cKvnNHvdhesOE/6n4GmwZGITo1mFEjnJfAqRvdBwZH2HU5EdqvJjMkZffi+BmYjW3\n\tZ6MTeRAEuvSb0JrhOGfmZN+P+pgHqS7nA4xNCsAkTYEICPNI2CltL+IUy3bj+Xo3vS\n\tngiaxmQw/Fz/M/UUBCYGsP4XZQU7ta7nolMQ9afYDXqFRThjHVeWpR5mWdfhuxap+9\n\tdmbXqfleCBkX45dCUxqZ/JujrJazsWK5YYSrOLhV5eDPTIwUywYeGhMF01IOYHsVjE\n\tMGeisGk6Jn+l6ytaYwHT6CbOyx7kM+4qkovimDPiDivLcNDBUglMCQNulsrJa6eNyB\n\t0mV7RrHpBuDIg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1679386056;\n\tbh=c3hW5h4GGCQrDkrThHa32XcT98yLHHvuwHg4qITkR+Q=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=a02D38RiiPN4NIaH/QDiA9yQYpKzlHca4YP1dCmF59T2VSwr9iQfxybRJLraGRLiY\n\tZtNWqmE5+XddNpiSPllB5d+HhRXC5oVkHfMrhwrMnidLv2MaYoyvkVidRdxlYKk1/I\n\tW3ovYLR+0BJchgLGdg4GubqU/4UbvksIVu63bn34="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"a02D38Ri\"; dkim-atps=neutral","Date":"Tue, 21 Mar 2023 09:07:33 +0100","To":"Daniel Semkowicz ","Message-ID":"<20230321080733.m66zuntkxqix6pdd@uno.localdomain>","References":"<20230314144834.85193-1-dse@thaumatec.com>\n\t<20230314144834.85193-4-dse@thaumatec.com>\n\t<20230320200219.3waxbayqusxk2wvz@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v4 03/10] ipa: Add base class defining\n\tAF algorithm interface","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]