[{"id":26814,"web_url":"https://patchwork.libcamera.org/comment/26814/","msgid":"<20230403092512.wujctvvkhz5k6s5e@uno.localdomain>","date":"2023-04-03T09:25:12","subject":"Re: [libcamera-devel] [PATCH v6 06/10] ipa: rkisp1: Add AF\n\talgorithm based on AfHillClimbing","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Daniel\n\nOn Fri, Mar 31, 2023 at 10:19:26AM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> Add contrast based auto focus implementation for rkisp1 platform using\n> the common AfHillClimbing algorithm.\n>\n> Rockchip ISP AF block allows calculation of sharpness and luminance\n> in up to three user defined windows. If no windows are set, there are\n> some default settings applied for the first window and exposed through\n> the driver. For each frame, use the sharpness value calculated for this\n> default window and feed the hill climbing algorithm with them. Then set\n> the lens position to value calculated by the algorithm.\n>\n> Signed-off-by: Daniel Semkowicz \n> ---\n> src/ipa/rkisp1/algorithms/af.cpp | 121 ++++++++++++++++++++++++++\n> src/ipa/rkisp1/algorithms/af.h | 43 +++++++++\n> src/ipa/rkisp1/algorithms/meson.build | 1 +\n> src/ipa/rkisp1/ipa_context.cpp | 13 +++\n> src/ipa/rkisp1/ipa_context.h | 5 ++\n> 5 files changed, 183 insertions(+)\n> create mode 100644 src/ipa/rkisp1/algorithms/af.cpp\n> create mode 100644 src/ipa/rkisp1/algorithms/af.h\n>\n> diff --git a/src/ipa/rkisp1/algorithms/af.cpp b/src/ipa/rkisp1/algorithms/af.cpp\n> new file mode 100644\n> index 00000000..fde924d4\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/af.cpp\n> @@ -0,0 +1,121 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Theobroma Systems\n> + *\n> + * af.cpp - RkISP1 AF hill climbing based control algorithm\n> + */\n> +\n> +#include \"af.h\"\n> +\n> +/**\n> + * \\file af.h\n> + */\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::rkisp1::algorithms {\n> +\n> +/**\n> + * \\class Af\n> + * \\brief AF contrast based hill climbing control algorithm for RkISP platforms\n> + *\n> + * Auto focus algorithm for RkISP platforms, based on the common hill climbing\n> + * auto focus implementation (libcamera::ipa::algorithms::AfHillClimbing).\n> + *\n> + * This is the platform layer of the algorithm.\n> + *\n> + * Tuning file parameters:\n> + * - **wait-frames-lens:** Number of frames that should be skipped when lens\n> + * position is changed. Lens movement takes some time and statistics measured\n> + * during the lens movement are unstable. Currently there is no way to know\n> + * when lens movement finished and this is a workaround for this. Wait a fixed\n> + * amount of time on each movement. This parameter should be set according\n> + * to the worst case - the number of frames it takes to move lens between\n> + * limit positions.\n> + * .\n> + * \\sa libcamera::ipa::algorithms::AfHillClimbing for additional tuning\n> + * parameters.\n> + *\n> + * \\todo Model the lens delay as number of frames required for the lens position\n> + * to stabilize in the CameraLens class.\n> + */\n> +\n> +LOG_DEFINE_CATEGORY(RkISP1Af)\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::init\n> + */\n> +int Af::init(IPAContext &context, const YamlObject &tuningData)\n> +{\n> +\tconst auto &lensConfig = context.configuration.lens;\n> +\tif (!lensConfig) {\n> +\t\tLOG(RkISP1Af, Error) << \"Lens not found\";\n> +\t\treturn 1;\n\nAs we discussed in v5, I think it's sane to fail at init() time\nif the user has instantiated the Af algorithm in config file, but the\nplatform does not expose a movable lens.\n\nMaybe a return -EINVAL instead of just 1, but that's a minor I can fix\nwhen applying.\n\nLooking at the diff with v5, my tag still stands\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> +\t}\n> +\n> +\twaitFramesLens_ = tuningData[\"wait-frames-lens\"].get(1);\n> +\n> +\tLOG(RkISP1Af, Debug) << \"waitFramesLens_: \" << waitFramesLens_;\n> +\n> +\treturn af.init(lensConfig->minFocusPosition,\n> +\t\t lensConfig->maxFocusPosition, tuningData);\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::configure\n> + */\n> +int Af::configure(IPAContext &context,\n> +\t\t const IPACameraSensorInfo &configInfo)\n> +{\n> +\t/*\n> +\t * Lens position is unknown at the startup, so initialize\n> +\t * the current position to something out of range.\n> +\t */\n> +\tcontext.activeState.af.lensPosition =\n> +\t\tcontext.configuration.lens->maxFocusPosition + 1;\n> +\n> +\taf.configure(configInfo.outputSize);\n> +\treturn 0;\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> + */\n> +void Af::queueRequest([[maybe_unused]] IPAContext &context,\n> +\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n> +\t\t const ControlList &controls)\n> +{\n> +\taf.queueRequest(controls);\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::process\n> + */\n> +void Af::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> +\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n> +\t\t const rkisp1_stat_buffer *stats,\n> +\t\t [[maybe_unused]] ControlList &metadata)\n> +{\n> +\tuint32_t sharpness = stats->params.af.window[0].sum;\n> +\tuint32_t luminance = stats->params.af.window[0].lum;\n> +\n> +\tLOG(RkISP1Af, Debug)\n> +\t\t<< \"lensPosition: \" << context.activeState.af.lensPosition\n> +\t\t<< \", Sharpness: \" << sharpness\n> +\t\t<< \", Luminance: \" << luminance;\n> +\n> +\tint32_t newLensPosition = af.process(sharpness);\n> +\n> +\tif (newLensPosition != context.activeState.af.lensPosition) {\n> +\t\tcontext.activeState.af.lensPosition = newLensPosition;\n> +\t\tcontext.activeState.af.applyLensCtrls = true;\n> +\t\taf.skipFrames(waitFramesLens_);\n> +\t}\n> +}\n> +\n> +REGISTER_IPA_ALGORITHM(Af, \"Af\")\n> +\n> +} /* namespace ipa::rkisp1::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/rkisp1/algorithms/af.h b/src/ipa/rkisp1/algorithms/af.h\n> new file mode 100644\n> index 00000000..3ba66d38\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/af.h\n> @@ -0,0 +1,43 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Theobroma Systems\n> + *\n> + * af.h - RkISP1 AF hill climbing based control algorithm\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +\n> +#include \"libipa/algorithms/af_hill_climbing.h\"\n> +\n> +#include \"algorithm.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::rkisp1::algorithms {\n> +\n> +class Af : public Algorithm\n> +{\n> +public:\n> +\tint init(IPAContext &context, const YamlObject &tuningData) override;\n> +\tint configure(IPAContext &context,\n> +\t\t const IPACameraSensorInfo &configInfo) override;\n> +\tvoid queueRequest(IPAContext &context, uint32_t frame,\n> +\t\t\t IPAFrameContext &frameContext,\n> +\t\t\t const ControlList &controls) override;\n> +\tvoid process(IPAContext &context, uint32_t frame,\n> +\t\t IPAFrameContext &frameContext,\n> +\t\t const rkisp1_stat_buffer *stats,\n> +\t\t ControlList &metadata) override;\n> +\n> +private:\n> +\tipa::algorithms::AfHillClimbing af;\n> +\n> +\t/* Wait number of frames after changing lens position */\n> +\tuint32_t waitFramesLens_ = 0;\n> +};\n> +\n> +} /* namespace ipa::rkisp1::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n> index 93a48329..ab7e44f3 100644\n> --- a/src/ipa/rkisp1/algorithms/meson.build\n> +++ b/src/ipa/rkisp1/algorithms/meson.build\n> @@ -1,6 +1,7 @@\n> # SPDX-License-Identifier: CC0-1.0\n>\n> rkisp1_ipa_algorithms = files([\n> + 'af.cpp',\n> 'agc.cpp',\n> 'awb.cpp',\n> 'blc.cpp',\n> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> index aea99299..d80a7b1b 100644\n> --- a/src/ipa/rkisp1/ipa_context.cpp\n> +++ b/src/ipa/rkisp1/ipa_context.cpp\n> @@ -139,6 +139,19 @@ namespace libcamera::ipa::rkisp1 {\n> * member may be read by any algorithm, but shall only be written by its owner.\n> */\n>\n> +/**\n> + * \\var IPAActiveState::af\n> + * \\brief State for the Automatic Focus Control algorithm\n> + *\n> + * \\var IPAActiveState::af.lensPosition\n> + * \\brief Lens position calculated by the AF algorithm\n> + *\n> + * \\var IPAActiveState::af.applyLensCtrls\n> + * \\brief Whether the lens position should be applied\n> + *\n> + * If true, IPA should send new controls to the PH to set new lens position.\n> + */\n> +\n> /**\n> * \\var IPAActiveState::agc\n> * \\brief State for the Automatic Gain Control algorithm\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 65b3fbfe..3dcc5aa0 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -62,6 +62,11 @@ struct IPASessionConfiguration {\n> };\n>\n> struct IPAActiveState {\n> +\tstruct {\n> +\t\tint32_t lensPosition;\n> +\t\tbool applyLensCtrls;\n> +\t} af;\n> +\n> \tstruct {\n> \t\tstruct {\n> \t\t\tuint32_t exposure;\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 57D2FC0F2A\n\tfor ;\n\tMon, 3 Apr 2023 09:25:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A952F62724;\n\tMon, 3 Apr 2023 11:25:17 +0200 (CEST)","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 A341761EC6\n\tfor ;\n\tMon, 3 Apr 2023 11:25:16 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 26EB16CF;\n\tMon, 3 Apr 2023 11:25:16 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1680513917;\n\tbh=MkH5XlmReaO7/WkmcL2FQhX7a88uKBc15DSJrOfeezs=;\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=K8TzhCxoIGhwDv85OVL5suLUjihJ383FrHYK6hJXIcTVNzUesHfr83BoqrcDjRg6v\n\tYZupxIewrtMUnU6jTvRs9chUWNiX/c9jeZ89hzyvQs3HJFAGqqgq1Vit/N1YANke1g\n\twQHU56EU+xSYflG6NshUnwbfJEYhAGUD/d9iGXZvFKKF3g19XOYzaHa2FvxvsERVLY\n\tTxSAYEb5xY9jSLhKa8uX+BqrnrK1J6jbzIduITWMrUoZ03R0CM8xSkv//lawAnt2P6\n\tNXX3Z3Tw7ait6PhYr3ssiQQl1LFYqiLgwz7CgmG3zxzUY2hISGQyXNZkfocswWH6GV\n\tz9a1UCDSPo6mQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1680513916;\n\tbh=MkH5XlmReaO7/WkmcL2FQhX7a88uKBc15DSJrOfeezs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Ba3L51K++VBo8E4TNFT03gDbADByhuGO4Zx32rfyXd9TTYlS4VAhkZlKXypIB2Twk\n\tot6jAj0jpG4n93doO8UemKxfWLyfdj4cooN1D4lirWrnQc80QVPF7zT8BSFtEE4I1Q\n\tKXAsCVnG2WBphX2OfVwOfXmSIV9meFuM6jhPRTLo="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Ba3L51K+\"; dkim-atps=neutral","Date":"Mon, 3 Apr 2023 11:25:12 +0200","To":"Daniel Semkowicz ","Message-ID":"<20230403092512.wujctvvkhz5k6s5e@uno.localdomain>","References":"<20230331081930.19289-1-dse@thaumatec.com>\n\t<20230331081930.19289-7-dse@thaumatec.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230331081930.19289-7-dse@thaumatec.com>","Subject":"Re: [libcamera-devel] [PATCH v6 06/10] ipa: rkisp1: Add AF\n\talgorithm based on AfHillClimbing","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":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26828,"web_url":"https://patchwork.libcamera.org/comment/26828/","msgid":"","date":"2023-04-04T06:09:06","subject":"Re: [libcamera-devel] [PATCH v6 06/10] ipa: rkisp1: Add AF\n\talgorithm based on AfHillClimbing","submitter":{"id":126,"url":"https://patchwork.libcamera.org/api/people/126/","name":"Daniel Semkowicz","email":"dse@thaumatec.com"},"content":"Hi Jacopo,\n\nOn Mon, Apr 3, 2023 at 11:25 AM Jacopo Mondi\n wrote:\n>\n> Hi Daniel\n>\n> On Fri, Mar 31, 2023 at 10:19:26AM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> > Add contrast based auto focus implementation for rkisp1 platform using\n> > the common AfHillClimbing algorithm.\n> >\n> > Rockchip ISP AF block allows calculation of sharpness and luminance\n> > in up to three user defined windows. If no windows are set, there are\n> > some default settings applied for the first window and exposed through\n> > the driver. For each frame, use the sharpness value calculated for this\n> > default window and feed the hill climbing algorithm with them. Then set\n> > the lens position to value calculated by the algorithm.\n> >\n> > Signed-off-by: Daniel Semkowicz \n> > ---\n> > src/ipa/rkisp1/algorithms/af.cpp | 121 ++++++++++++++++++++++++++\n> > src/ipa/rkisp1/algorithms/af.h | 43 +++++++++\n> > src/ipa/rkisp1/algorithms/meson.build | 1 +\n> > src/ipa/rkisp1/ipa_context.cpp | 13 +++\n> > src/ipa/rkisp1/ipa_context.h | 5 ++\n> > 5 files changed, 183 insertions(+)\n> > create mode 100644 src/ipa/rkisp1/algorithms/af.cpp\n> > create mode 100644 src/ipa/rkisp1/algorithms/af.h\n> >\n> > diff --git a/src/ipa/rkisp1/algorithms/af.cpp b/src/ipa/rkisp1/algorithms/af.cpp\n> > new file mode 100644\n> > index 00000000..fde924d4\n> > --- /dev/null\n> > +++ b/src/ipa/rkisp1/algorithms/af.cpp\n> > @@ -0,0 +1,121 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2023, Theobroma Systems\n> > + *\n> > + * af.cpp - RkISP1 AF hill climbing based control algorithm\n> > + */\n> > +\n> > +#include \"af.h\"\n> > +\n> > +/**\n> > + * \\file af.h\n> > + */\n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace ipa::rkisp1::algorithms {\n> > +\n> > +/**\n> > + * \\class Af\n> > + * \\brief AF contrast based hill climbing control algorithm for RkISP platforms\n> > + *\n> > + * Auto focus algorithm for RkISP platforms, based on the common hill climbing\n> > + * auto focus implementation (libcamera::ipa::algorithms::AfHillClimbing).\n> > + *\n> > + * This is the platform layer of the algorithm.\n> > + *\n> > + * Tuning file parameters:\n> > + * - **wait-frames-lens:** Number of frames that should be skipped when lens\n> > + * position is changed. Lens movement takes some time and statistics measured\n> > + * during the lens movement are unstable. Currently there is no way to know\n> > + * when lens movement finished and this is a workaround for this. Wait a fixed\n> > + * amount of time on each movement. This parameter should be set according\n> > + * to the worst case - the number of frames it takes to move lens between\n> > + * limit positions.\n> > + * .\n> > + * \\sa libcamera::ipa::algorithms::AfHillClimbing for additional tuning\n> > + * parameters.\n> > + *\n> > + * \\todo Model the lens delay as number of frames required for the lens position\n> > + * to stabilize in the CameraLens class.\n> > + */\n> > +\n> > +LOG_DEFINE_CATEGORY(RkISP1Af)\n> > +\n> > +/**\n> > + * \\copydoc libcamera::ipa::Algorithm::init\n> > + */\n> > +int Af::init(IPAContext &context, const YamlObject &tuningData)\n> > +{\n> > + const auto &lensConfig = context.configuration.lens;\n> > + if (!lensConfig) {\n> > + LOG(RkISP1Af, Error) << \"Lens not found\";\n> > + return 1;\n>\n> As we discussed in v5, I think it's sane to fail at init() time\n> if the user has instantiated the Af algorithm in config file, but the\n> platform does not expose a movable lens.\n>\n> Maybe a return -EINVAL instead of just 1, but that's a minor I can fix\n> when applying.\n\nAh, yes the -EINVAL is a better choice than \"1\" :)\n\n>\n> Looking at the diff with v5, my tag still stands\n> Reviewed-by: Jacopo Mondi \n>\n> Thanks\n> j\n>\n> > + }\n> > +\n> > + waitFramesLens_ = tuningData[\"wait-frames-lens\"].get(1);\n> > +\n> > + LOG(RkISP1Af, Debug) << \"waitFramesLens_: \" << waitFramesLens_;\n> > +\n> > + return af.init(lensConfig->minFocusPosition,\n> > + lensConfig->maxFocusPosition, tuningData);\n> > +}\n> > +\n> > +/**\n> > + * \\copydoc libcamera::ipa::Algorithm::configure\n> > + */\n> > +int Af::configure(IPAContext &context,\n> > + const IPACameraSensorInfo &configInfo)\n> > +{\n> > + /*\n> > + * Lens position is unknown at the startup, so initialize\n> > + * the current position to something out of range.\n> > + */\n> > + context.activeState.af.lensPosition =\n> > + context.configuration.lens->maxFocusPosition + 1;\n> > +\n> > + af.configure(configInfo.outputSize);\n> > + return 0;\n> > +}\n> > +\n> > +/**\n> > + * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> > + */\n> > +void Af::queueRequest([[maybe_unused]] IPAContext &context,\n> > + [[maybe_unused]] const uint32_t frame,\n> > + [[maybe_unused]] IPAFrameContext &frameContext,\n> > + const ControlList &controls)\n> > +{\n> > + af.queueRequest(controls);\n> > +}\n> > +\n> > +/**\n> > + * \\copydoc libcamera::ipa::Algorithm::process\n> > + */\n> > +void Af::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> > + [[maybe_unused]] IPAFrameContext &frameContext,\n> > + const rkisp1_stat_buffer *stats,\n> > + [[maybe_unused]] ControlList &metadata)\n> > +{\n> > + uint32_t sharpness = stats->params.af.window[0].sum;\n> > + uint32_t luminance = stats->params.af.window[0].lum;\n> > +\n> > + LOG(RkISP1Af, Debug)\n> > + << \"lensPosition: \" << context.activeState.af.lensPosition\n> > + << \", Sharpness: \" << sharpness\n> > + << \", Luminance: \" << luminance;\n> > +\n> > + int32_t newLensPosition = af.process(sharpness);\n> > +\n> > + if (newLensPosition != context.activeState.af.lensPosition) {\n> > + context.activeState.af.lensPosition = newLensPosition;\n> > + context.activeState.af.applyLensCtrls = true;\n> > + af.skipFrames(waitFramesLens_);\n> > + }\n> > +}\n> > +\n> > +REGISTER_IPA_ALGORITHM(Af, \"Af\")\n> > +\n> > +} /* namespace ipa::rkisp1::algorithms */\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/ipa/rkisp1/algorithms/af.h b/src/ipa/rkisp1/algorithms/af.h\n> > new file mode 100644\n> > index 00000000..3ba66d38\n> > --- /dev/null\n> > +++ b/src/ipa/rkisp1/algorithms/af.h\n> > @@ -0,0 +1,43 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2023, Theobroma Systems\n> > + *\n> > + * af.h - RkISP1 AF hill climbing based control algorithm\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \n> > +\n> > +#include \"libipa/algorithms/af_hill_climbing.h\"\n> > +\n> > +#include \"algorithm.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace ipa::rkisp1::algorithms {\n> > +\n> > +class Af : public Algorithm\n> > +{\n> > +public:\n> > + int init(IPAContext &context, const YamlObject &tuningData) override;\n> > + int configure(IPAContext &context,\n> > + const IPACameraSensorInfo &configInfo) override;\n> > + void queueRequest(IPAContext &context, uint32_t frame,\n> > + IPAFrameContext &frameContext,\n> > + const ControlList &controls) override;\n> > + void process(IPAContext &context, uint32_t frame,\n> > + IPAFrameContext &frameContext,\n> > + const rkisp1_stat_buffer *stats,\n> > + ControlList &metadata) override;\n> > +\n> > +private:\n> > + ipa::algorithms::AfHillClimbing af;\n> > +\n> > + /* Wait number of frames after changing lens position */\n> > + uint32_t waitFramesLens_ = 0;\n> > +};\n> > +\n> > +} /* namespace ipa::rkisp1::algorithms */\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n> > index 93a48329..ab7e44f3 100644\n> > --- a/src/ipa/rkisp1/algorithms/meson.build\n> > +++ b/src/ipa/rkisp1/algorithms/meson.build\n> > @@ -1,6 +1,7 @@\n> > # SPDX-License-Identifier: CC0-1.0\n> >\n> > rkisp1_ipa_algorithms = files([\n> > + 'af.cpp',\n> > 'agc.cpp',\n> > 'awb.cpp',\n> > 'blc.cpp',\n> > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> > index aea99299..d80a7b1b 100644\n> > --- a/src/ipa/rkisp1/ipa_context.cpp\n> > +++ b/src/ipa/rkisp1/ipa_context.cpp\n> > @@ -139,6 +139,19 @@ namespace libcamera::ipa::rkisp1 {\n> > * member may be read by any algorithm, but shall only be written by its owner.\n> > */\n> >\n> > +/**\n> > + * \\var IPAActiveState::af\n> > + * \\brief State for the Automatic Focus Control algorithm\n> > + *\n> > + * \\var IPAActiveState::af.lensPosition\n> > + * \\brief Lens position calculated by the AF algorithm\n> > + *\n> > + * \\var IPAActiveState::af.applyLensCtrls\n> > + * \\brief Whether the lens position should be applied\n> > + *\n> > + * If true, IPA should send new controls to the PH to set new lens position.\n> > + */\n> > +\n> > /**\n> > * \\var IPAActiveState::agc\n> > * \\brief State for the Automatic Gain Control algorithm\n> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > index 65b3fbfe..3dcc5aa0 100644\n> > --- a/src/ipa/rkisp1/ipa_context.h\n> > +++ b/src/ipa/rkisp1/ipa_context.h\n> > @@ -62,6 +62,11 @@ struct IPASessionConfiguration {\n> > };\n> >\n> > struct IPAActiveState {\n> > + struct {\n> > + int32_t lensPosition;\n> > + bool applyLensCtrls;\n> > + } af;\n> > +\n> > struct {\n> > struct {\n> > uint32_t exposure;\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 61D80C0F2A\n\tfor ;\n\tTue, 4 Apr 2023 06:09:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C151862720;\n\tTue, 4 Apr 2023 08:09:18 +0200 (CEST)","from mail-ed1-x52d.google.com (mail-ed1-x52d.google.com\n\t[IPv6:2a00:1450:4864:20::52d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 73C2B6271C\n\tfor ;\n\tTue, 4 Apr 2023 08:09:17 +0200 (CEST)","by mail-ed1-x52d.google.com with SMTP id w9so126105211edc.3\n\tfor ;\n\tMon, 03 Apr 2023 23:09:17 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1680588558;\n\tbh=WC8OT4OzjyMQzZDrDrV+321+vrgZOA41yC54OAqM5JY=;\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=fURKVudEHsaS58mqmRLy5OxyZW849spqv87P/lh1T+65G/WUO9vA86Jd1SCcCfm4o\n\tiqn51y4sgLPaBGpGZpWwYpvopy/jd02d5RdIRoBi64XXMRx7gmjmRmb+IHxSX5AHrh\n\tsHPeDwLMGfEJ3ybrx38Gbrhq/GaIKKY0r/qEXotO8pfTik64hgFK9uNh6Kp5zJW886\n\tcv6OGMwjw7T92vPTtDnQo9XkY/jCSs51qx5uuFg3mv1eG73AEwc3bvUDon7Uq+Xcpv\n\tjwsli2aeRAdQfrJzj4Zj+LfnjhJzJkJeaImSINx0f2pA1rQakgFQaQYZlX4BSPbP3Z\n\tQ5CFTCebTOz5Q==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=thaumatec-com.20210112.gappssmtp.com; s=20210112; t=1680588557;\n\tx=1683180557; \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=61vWBfK/HYY46PqpOE9ERa08MIX9eN+9SLvp2tFzVbs=;\n\tb=XBVdSLohTP6r4QR8QkpOoNxTZoPWotvuu3d2xd+ELRPkWBQsp9r1GdE3VpJB4Sz2q+\n\trxgBWShmTvGL3sCR0aW881Z9hwjK2GvJF8fT0bOYv9gCD/o4jdzhglf61uxY2Kg9BuGT\n\tpdxns3SJDadXbYtkE/PK4Z7uXO+fOPSyj71lCgpLcjrJrF/pBrCX6tVmX6wryg5p8tEl\n\tsrvRB2VoBQcv1kNlXuk+I+qCGPBpM9P/efuXLJq/KH3rNyT3UyDUhQycGkvd2wbohQcG\n\tLx74l9YoUyLQnaOrk1+wUJ6IEEpjYv0QAjnu+u8DA+MYfxpTIe3flOAPnnRVThBKvsoU\n\tB4mA=="],"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=\"XBVdSLoh\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112; t=1680588557; x=1683180557;\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=61vWBfK/HYY46PqpOE9ERa08MIX9eN+9SLvp2tFzVbs=;\n\tb=n0QFARmiAjJ8zsaz649HzuTDDceTv4qX5YM4n8mPEyzdgmIiol82RI3FllPFqTNQKL\n\tqrdI7r6mhdKUez7XWzCD7gNRCU0/jgz0tGdqeLD42VVB5mswREirGLoyIRxR9K32xbey\n\ttJ9cKaXdOiS0uj3rd83su2FJjn5UTbOBYxNey8SXMPPdHcvx1tfrw77TT7znkwseQI8R\n\t3i3qU8kZr1mRYXvaVJ8X3S10WOH301AbDKuuS2r8PqPKB/2n19AUVOYIys3Zxa5oDA1U\n\twiNnOwPP8x5jzHRqQz8yfrmuY1pbjMKj6lscQjhT5rDIznvEYNqnhzW7LAuq7zefEPNX\n\tX+oQ==","X-Gm-Message-State":"AAQBX9f8KhsKuzwHuJyOfa3MPmGDa8p3SHQEKutALJVB8JkYRQ71pAB9\n\tQmIfHEGf1qvJ0b6NB0Chi0vwiXKY9COcDhD7s4Xyvg==","X-Google-Smtp-Source":"AKy350YBS6JB2G5vUsPfMU18I1POPKJ3I9341zEoOsx1L37MKeQDeQPHKiBLDKiWWoSqx8w6w0n6JQWOCpdyDWUw2l0=","X-Received":"by 2002:a50:9f89:0:b0:4fb:c8e3:1adb with SMTP id\n\tc9-20020a509f89000000b004fbc8e31adbmr781406edf.3.1680588556925;\n\tMon, 03 Apr 2023 23:09:16 -0700 (PDT)","MIME-Version":"1.0","References":"<20230331081930.19289-1-dse@thaumatec.com>\n\t<20230331081930.19289-7-dse@thaumatec.com>\n\t<20230403092512.wujctvvkhz5k6s5e@uno.localdomain>","In-Reply-To":"<20230403092512.wujctvvkhz5k6s5e@uno.localdomain>","Date":"Tue, 4 Apr 2023 08:09:06 +0200","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH v6 06/10] ipa: rkisp1: Add AF\n\talgorithm based on AfHillClimbing","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":27144,"web_url":"https://patchwork.libcamera.org/comment/27144/","msgid":"","date":"2023-05-26T09:19:02","subject":"Re: [libcamera-devel] [PATCH v6 06/10] ipa: rkisp1: Add AF\n\talgorithm based on AfHillClimbing","submitter":{"id":126,"url":"https://patchwork.libcamera.org/api/people/126/","name":"Daniel Semkowicz","email":"dse@thaumatec.com"},"content":"Hi Laurent,\n\nOn Wed, Apr 26, 2023 at 6:04 AM Laurent Pinchart\n wrote:\n>\n> On Wed, Apr 26, 2023 at 06:46:58AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > Hi Daniel,\n> >\n> > Thank you for the patch.\n> >\n> > On Fri, Mar 31, 2023 at 10:19:26AM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> > > Add contrast based auto focus implementation for rkisp1 platform using\n> > > the common AfHillClimbing algorithm.\n> > >\n> > > Rockchip ISP AF block allows calculation of sharpness and luminance\n> > > in up to three user defined windows. If no windows are set, there are\n> > > some default settings applied for the first window and exposed through\n> > > the driver. For each frame, use the sharpness value calculated for this\n> > > default window and feed the hill climbing algorithm with them. Then set\n> > > the lens position to value calculated by the algorithm.\n> > >\n> > > Signed-off-by: Daniel Semkowicz \n> > > ---\n> > > src/ipa/rkisp1/algorithms/af.cpp | 121 ++++++++++++++++++++++++++\n> > > src/ipa/rkisp1/algorithms/af.h | 43 +++++++++\n> > > src/ipa/rkisp1/algorithms/meson.build | 1 +\n> > > src/ipa/rkisp1/ipa_context.cpp | 13 +++\n> > > src/ipa/rkisp1/ipa_context.h | 5 ++\n> > > 5 files changed, 183 insertions(+)\n> > > create mode 100644 src/ipa/rkisp1/algorithms/af.cpp\n> > > create mode 100644 src/ipa/rkisp1/algorithms/af.h\n> > >\n> > > diff --git a/src/ipa/rkisp1/algorithms/af.cpp b/src/ipa/rkisp1/algorithms/af.cpp\n> > > new file mode 100644\n> > > index 00000000..fde924d4\n> > > --- /dev/null\n> > > +++ b/src/ipa/rkisp1/algorithms/af.cpp\n> > > @@ -0,0 +1,121 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2023, Theobroma Systems\n> > > + *\n> > > + * af.cpp - RkISP1 AF hill climbing based control algorithm\n> >\n> > s/climbing based/climbing-based/\n> >\n> > > + */\n> > > +\n> > > +#include \"af.h\"\n> > > +\n> > > +/**\n> > > + * \\file af.h\n> >\n> > Missing \\brief\n> >\n> > > + */\n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +namespace ipa::rkisp1::algorithms {\n> > > +\n> > > +/**\n> > > + * \\class Af\n> > > + * \\brief AF contrast based hill climbing control algorithm for RkISP platforms\n> >\n> > s/contrast based/contrast-based/\n> >\n> > > + *\n> > > + * Auto focus algorithm for RkISP platforms, based on the common hill climbing\n> > > + * auto focus implementation (libcamera::ipa::algorithms::AfHillClimbing).\n> > > + *\n> > > + * This is the platform layer of the algorithm.\n> > > + *\n> > > + * Tuning file parameters:\n> > > + * - **wait-frames-lens:** Number of frames that should be skipped when lens\n> >\n> > rkisp1 tuning files use camelCase for parameter names.\n\nI will correct it.\n\n> >\n> > > + * position is changed. Lens movement takes some time and statistics measured\n> > > + * during the lens movement are unstable. Currently there is no way to know\n> > > + * when lens movement finished and this is a workaround for this. Wait a fixed\n> > > + * amount of time on each movement. This parameter should be set according\n> > > + * to the worst case - the number of frames it takes to move lens between\n> > > + * limit positions.\n> > > + * .\n> >\n> > Stray period ?\n\nThe same as in the 04/10, dot is used to explicitly mark the end of\nthe list in the Doxygen.\n\n> >\n> > > + * \\sa libcamera::ipa::algorithms::AfHillClimbing for additional tuning\n> > > + * parameters.\n> > > + *\n> > > + * \\todo Model the lens delay as number of frames required for the lens position\n> > > + * to stabilize in the CameraLens class.\n> >\n> > More than that, we should take the lens movement into account in the\n> > algorithm to avoid skipping frames at all.\n\nYes, I agree. We should only base on the information if the lens\nmovement has finished instead of skipping frames in the AF. I will\nimprove that note.\n\n> >\n> > > + */\n> > > +\n> > > +LOG_DEFINE_CATEGORY(RkISP1Af)\n> > > +\n> > > +/**\n> > > + * \\copydoc libcamera::ipa::Algorithm::init\n> > > + */\n> > > +int Af::init(IPAContext &context, const YamlObject &tuningData)\n> > > +{\n> > > + const auto &lensConfig = context.configuration.lens;\n> > > + if (!lensConfig) {\n> > > + LOG(RkISP1Af, Error) << \"Lens not found\";\n> > > + return 1;\n> > > + }\n> > > +\n> > > + waitFramesLens_ = tuningData[\"wait-frames-lens\"].get(1);\n> > > +\n> > > + LOG(RkISP1Af, Debug) << \"waitFramesLens_: \" << waitFramesLens_;\n> > > +\n> > > + return af.init(lensConfig->minFocusPosition,\n> > > + lensConfig->maxFocusPosition, tuningData);\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\copydoc libcamera::ipa::Algorithm::configure\n> > > + */\n> > > +int Af::configure(IPAContext &context,\n> > > + const IPACameraSensorInfo &configInfo)\n> > > +{\n> > > + /*\n> > > + * Lens position is unknown at the startup, so initialize\n> > > + * the current position to something out of range.\n> > > + */\n> > > + context.activeState.af.lensPosition =\n> > > + context.configuration.lens->maxFocusPosition + 1;\n> > > +\n> > > + af.configure(configInfo.outputSize);\n> > > + return 0;\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> > > + */\n> > > +void Af::queueRequest([[maybe_unused]] IPAContext &context,\n> > > + [[maybe_unused]] const uint32_t frame,\n> > > + [[maybe_unused]] IPAFrameContext &frameContext,\n> > > + const ControlList &controls)\n> > > +{\n> > > + af.queueRequest(controls);\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\copydoc libcamera::ipa::Algorithm::process\n> > > + */\n> > > +void Af::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> > > + [[maybe_unused]] IPAFrameContext &frameContext,\n> > > + const rkisp1_stat_buffer *stats,\n> > > + [[maybe_unused]] ControlList &metadata)\n> > > +{\n> > > + uint32_t sharpness = stats->params.af.window[0].sum;\n> > > + uint32_t luminance = stats->params.af.window[0].lum;\n> > > +\n> > > + LOG(RkISP1Af, Debug)\n> > > + << \"lensPosition: \" << context.activeState.af.lensPosition\n> > > + << \", Sharpness: \" << sharpness\n> >\n> > Maybe contrast, as everything in this series talks about contrast-based\n> > AF ?\n\nLog category for this debug message is \"RkISP1Af\". Isn't it easier\nfor debugging, if We stick with the rkisp naming in the platform related\ncode?\n\n> >\n> > > + << \", Luminance: \" << luminance;\n> >\n> > The luminance isn't used, so you could drop it.\n>\n> Or better, you should use it :-) The luminance is meant to normalize the\n> sharpness values.\n\nCan We introduce the normalization later as an improvement?\n\n>\n> > > +\n> > > + int32_t newLensPosition = af.process(sharpness);\n> > > +\n> > > + if (newLensPosition != context.activeState.af.lensPosition) {\n> > > + context.activeState.af.lensPosition = newLensPosition;\n> > > + context.activeState.af.applyLensCtrls = true;\n> > > + af.skipFrames(waitFramesLens_);\n> >\n> > I'm tempted to implement frame skipping in this class instead of the\n> > ipa::algorithms::AfHillClimbing class. It would be simpler, we could\n> > just skip calling process(). On the other hand, I suppose\n> > ipa::algorithms::AfHillClimbing will need to change when we'll improve\n> > the mechanism that takes lens movement into account, and it's hard to\n> > predict how the interface will change.\n> >\n\nSo maybe it would be simpler to use the current approach and improve\nit later when needed, when the interface become clear.\n\n> > > + }\n> > > +}\n> >\n> > The IPA module should also report the lens position in metadata.\n> >\n> > > +\n> > > +REGISTER_IPA_ALGORITHM(Af, \"Af\")\n> > > +\n> > > +} /* namespace ipa::rkisp1::algorithms */\n> > > +\n> > > +} /* namespace libcamera */\n> > > diff --git a/src/ipa/rkisp1/algorithms/af.h b/src/ipa/rkisp1/algorithms/af.h\n> > > new file mode 100644\n> > > index 00000000..3ba66d38\n> > > --- /dev/null\n> > > +++ b/src/ipa/rkisp1/algorithms/af.h\n> > > @@ -0,0 +1,43 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2023, Theobroma Systems\n> > > + *\n> > > + * af.h - RkISP1 AF hill climbing based control algorithm\n> >\n> > s/climbing based/climbing-based/\n> >\n> > > + */\n> > > +\n> > > +#pragma once\n> > > +\n> > > +#include \n> > > +\n> > > +#include \"libipa/algorithms/af_hill_climbing.h\"\n> > > +\n> > > +#include \"algorithm.h\"\n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +namespace ipa::rkisp1::algorithms {\n> > > +\n> > > +class Af : public Algorithm\n> > > +{\n> > > +public:\n> > > + int init(IPAContext &context, const YamlObject &tuningData) override;\n> > > + int configure(IPAContext &context,\n> > > + const IPACameraSensorInfo &configInfo) override;\n> > > + void queueRequest(IPAContext &context, uint32_t frame,\n> > > + IPAFrameContext &frameContext,\n> > > + const ControlList &controls) override;\n> > > + void process(IPAContext &context, uint32_t frame,\n> > > + IPAFrameContext &frameContext,\n> > > + const rkisp1_stat_buffer *stats,\n> > > + ControlList &metadata) override;\n> > > +\n> > > +private:\n> > > + ipa::algorithms::AfHillClimbing af;\n> >\n> > s/af/af_/\n> >\n> > > +\n> > > + /* Wait number of frames after changing lens position */\n> > > + uint32_t waitFramesLens_ = 0;\n> > > +};\n> > > +\n> > > +} /* namespace ipa::rkisp1::algorithms */\n> > > +\n> > > +} /* namespace libcamera */\n> > > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n> > > index 93a48329..ab7e44f3 100644\n> > > --- a/src/ipa/rkisp1/algorithms/meson.build\n> > > +++ b/src/ipa/rkisp1/algorithms/meson.build\n> > > @@ -1,6 +1,7 @@\n> > > # SPDX-License-Identifier: CC0-1.0\n> > >\n> > > rkisp1_ipa_algorithms = files([\n> > > + 'af.cpp',\n> > > 'agc.cpp',\n> > > 'awb.cpp',\n> > > 'blc.cpp',\n> > > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> > > index aea99299..d80a7b1b 100644\n> > > --- a/src/ipa/rkisp1/ipa_context.cpp\n> > > +++ b/src/ipa/rkisp1/ipa_context.cpp\n> > > @@ -139,6 +139,19 @@ namespace libcamera::ipa::rkisp1 {\n> > > * member may be read by any algorithm, but shall only be written by its owner.\n> > > */\n> > >\n> > > +/**\n> > > + * \\var IPAActiveState::af\n> > > + * \\brief State for the Automatic Focus Control algorithm\n> > > + *\n> > > + * \\var IPAActiveState::af.lensPosition\n> > > + * \\brief Lens position calculated by the AF algorithm\n> > > + *\n> > > + * \\var IPAActiveState::af.applyLensCtrls\n> > > + * \\brief Whether the lens position should be applied\n> > > + *\n> > > + * If true, IPA should send new controls to the PH to set new lens position.\n> > > + */\n> > > +\n> > > /**\n> > > * \\var IPAActiveState::agc\n> > > * \\brief State for the Automatic Gain Control algorithm\n> > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > > index 65b3fbfe..3dcc5aa0 100644\n> > > --- a/src/ipa/rkisp1/ipa_context.h\n> > > +++ b/src/ipa/rkisp1/ipa_context.h\n> > > @@ -62,6 +62,11 @@ struct IPASessionConfiguration {\n> > > };\n> > >\n> > > struct IPAActiveState {\n> > > + struct {\n> > > + int32_t lensPosition;\n> > > + bool applyLensCtrls;\n> >\n> > I suppose applyLens will be used later in the series.\n\nShould I move it to the later patches?\n\n> >\n> > > + } af;\n> > > +\n> > > struct {\n> > > struct {\n> > > uint32_t exposure;\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 68923C31E9\n\tfor ;\n\tFri, 26 May 2023 09:19:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AED1B626FA;\n\tFri, 26 May 2023 11:19:14 +0200 (CEST)","from mail-ej1-x62c.google.com (mail-ej1-x62c.google.com\n\t[IPv6:2a00:1450:4864:20::62c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A3C8061EA9\n\tfor ;\n\tFri, 26 May 2023 11:19:13 +0200 (CEST)","by mail-ej1-x62c.google.com with SMTP id\n\ta640c23a62f3a-96f7bf29550so73011566b.3\n\tfor ;\n\tFri, 26 May 2023 02:19:13 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685092754;\n\tbh=l1V3466ZWRwhkEaOUUVq8V3KKVdVTmPUZ1dviA2vgpg=;\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=ig+q3TpF4Zo6uJZoUlA7ZwJDmH6jP1ZFemHbSJWv9EgaizpI2/kSjUhztj5cS7lhG\n\tWtCq2HitWLdeU89Gr3wJCSlMJMC5ANyCp7HgfgbjoZtM0w8yiUceawFxWn2iX1dxa5\n\tO9zY2EeMZNTC+wmADJLOP9DPqzUOQ613yXy1+h+Jo5XYNEtQ5Ey5oii/rd4mdonXAV\n\thQcf/+amiwjCWZrG0BayPdjHnAXoqzoRGXnYSgFwIEmM/MpwHaSHyue2liadxVLHPb\n\tWKSw1NAGEyGrG/qtHHszQBrSXRnZ6mCOZUa64hiATyiXUmDx/Q0ewE+uSOGrtaFrvX\n\t9RQKLtL9PO6Wg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=thaumatec-com.20221208.gappssmtp.com; s=20221208; t=1685092753;\n\tx=1687684753; \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=yRLEcyKW0LxkHlzusttKsKMCip7icVrY2aFeu4uBsus=;\n\tb=Kkh+av2WUbuYLHD9XPWWAniIGTgW8CCI3SWNRL3bLasaBK1FTvs69acxZR7jlU8ITg\n\t0rVZmUtJfLfc+dFu2wimn7WJs6S9CZ4VS5fXHPJgs+ryjDQ5y+IjgX91nbN7X46FNTzD\n\t4Ue/jFAbPB/kk9cbzAyag3qaHgfvG2eCe62LM1yBFFSFlnMajb87lgnOr5Vco4zFjpUb\n\ti8QSEDM6vRTcO8NmA31URj7MkPc5C9wGJZZ0Mfzvq3hR/cgwTPhD4Y7t9/hPhxP6g53b\n\tqcUdoxg+p2lBcVHEbU3g1JNF8zm8uwEw2JinuAbKZHGhj6secJmDuVucviqGCLvPpBz0\n\tqITg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=thaumatec-com.20221208.gappssmtp.com\n\theader.i=@thaumatec-com.20221208.gappssmtp.com header.b=\"Kkh+av2W\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1685092753; x=1687684753;\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=yRLEcyKW0LxkHlzusttKsKMCip7icVrY2aFeu4uBsus=;\n\tb=JlBNWEsxWk2M/mgWkEDnZ+z3RcDh+nnnoFhRfDezqWxGt+jlmf8R97eCnaBBOHwGS0\n\tqZYfizl341w6pUu4D1G7ZHXkKFN2GeXqYMqXFNAjuCUae3nE9iXOhSrQagL6NSMyTbF2\n\tE5wTftVssRtq9RgfLynl9pSn6/w2o4T2KlgTXv2/oCc/t9Dya7pqajH6zxgzM40IDjL/\n\tQbeDftPwNkU1D8xOZmSEKfFQ2zl3OTAo9GM7o7SpSWqqmX1xQ0vUyOuQrX8a+e0QVjm5\n\tH41zdMRRLzMuC5WOC1esj9nGBqqG361G/EnsXqXqpWOWPw+fdGKQxi8AepewrlTnWhmf\n\tO0og==","X-Gm-Message-State":"AC+VfDzIcEdU8QGwkyP27BoIPCOOTdGJ3+O9DbP+2VMWhNKHMlGN/gDl\n\tB1YxcQ+jFOHJ+D1kMOX5QB0enRBbOtti8rygW/vypQ==","X-Google-Smtp-Source":"ACHHUZ5VlHSyQNcDxaUMH4EAcPM7Io+lRXhHgrRWRfOToYF4Yfhlmv83poiglOWaXf+9vt46ryBEDdFfstqDDI8GsLQ=","X-Received":"by 2002:a17:907:a40a:b0:966:4e84:d82d with SMTP id\n\tsg10-20020a170907a40a00b009664e84d82dmr1486114ejc.3.1685092753021;\n\tFri, 26 May 2023 02:19:13 -0700 (PDT)","MIME-Version":"1.0","References":"<20230331081930.19289-1-dse@thaumatec.com>\n\t<20230331081930.19289-7-dse@thaumatec.com>\n\t<20230426034658.GG1667@pendragon.ideasonboard.com>\n\t<20230426040417.GI1667@pendragon.ideasonboard.com>","In-Reply-To":"<20230426040417.GI1667@pendragon.ideasonboard.com>","Date":"Fri, 26 May 2023 11:19:02 +0200","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH v6 06/10] ipa: rkisp1: Add AF\n\talgorithm based on AfHillClimbing","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":27200,"web_url":"https://patchwork.libcamera.org/comment/27200/","msgid":"<20230531175337.GH24749@pendragon.ideasonboard.com>","date":"2023-05-31T17:53:37","subject":"Re: [libcamera-devel] [PATCH v6 06/10] ipa: rkisp1: Add AF\n\talgorithm based on AfHillClimbing","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Daniel,\n\nOn Fri, May 26, 2023 at 11:19:02AM +0200, Daniel Semkowicz wrote:\n> On Wed, Apr 26, 2023 at 6:04 AM Laurent Pinchart wrote:\n> > On Wed, Apr 26, 2023 at 06:46:58AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > > On Fri, Mar 31, 2023 at 10:19:26AM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> > > > Add contrast based auto focus implementation for rkisp1 platform using\n> > > > the common AfHillClimbing algorithm.\n> > > >\n> > > > Rockchip ISP AF block allows calculation of sharpness and luminance\n> > > > in up to three user defined windows. If no windows are set, there are\n> > > > some default settings applied for the first window and exposed through\n> > > > the driver. For each frame, use the sharpness value calculated for this\n> > > > default window and feed the hill climbing algorithm with them. Then set\n> > > > the lens position to value calculated by the algorithm.\n> > > >\n> > > > Signed-off-by: Daniel Semkowicz \n> > > > ---\n> > > > src/ipa/rkisp1/algorithms/af.cpp | 121 ++++++++++++++++++++++++++\n> > > > src/ipa/rkisp1/algorithms/af.h | 43 +++++++++\n> > > > src/ipa/rkisp1/algorithms/meson.build | 1 +\n> > > > src/ipa/rkisp1/ipa_context.cpp | 13 +++\n> > > > src/ipa/rkisp1/ipa_context.h | 5 ++\n> > > > 5 files changed, 183 insertions(+)\n> > > > create mode 100644 src/ipa/rkisp1/algorithms/af.cpp\n> > > > create mode 100644 src/ipa/rkisp1/algorithms/af.h\n> > > >\n> > > > diff --git a/src/ipa/rkisp1/algorithms/af.cpp b/src/ipa/rkisp1/algorithms/af.cpp\n> > > > new file mode 100644\n> > > > index 00000000..fde924d4\n> > > > --- /dev/null\n> > > > +++ b/src/ipa/rkisp1/algorithms/af.cpp\n> > > > @@ -0,0 +1,121 @@\n> > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > > +/*\n> > > > + * Copyright (C) 2023, Theobroma Systems\n> > > > + *\n> > > > + * af.cpp - RkISP1 AF hill climbing based control algorithm\n> > >\n> > > s/climbing based/climbing-based/\n> > >\n> > > > + */\n> > > > +\n> > > > +#include \"af.h\"\n> > > > +\n> > > > +/**\n> > > > + * \\file af.h\n> > >\n> > > Missing \\brief\n> > >\n> > > > + */\n> > > > +\n> > > > +namespace libcamera {\n> > > > +\n> > > > +namespace ipa::rkisp1::algorithms {\n> > > > +\n> > > > +/**\n> > > > + * \\class Af\n> > > > + * \\brief AF contrast based hill climbing control algorithm for RkISP platforms\n> > >\n> > > s/contrast based/contrast-based/\n> > >\n> > > > + *\n> > > > + * Auto focus algorithm for RkISP platforms, based on the common hill climbing\n> > > > + * auto focus implementation (libcamera::ipa::algorithms::AfHillClimbing).\n> > > > + *\n> > > > + * This is the platform layer of the algorithm.\n> > > > + *\n> > > > + * Tuning file parameters:\n> > > > + * - **wait-frames-lens:** Number of frames that should be skipped when lens\n> > >\n> > > rkisp1 tuning files use camelCase for parameter names.\n> \n> I will correct it.\n> \n> > > > + * position is changed. Lens movement takes some time and statistics measured\n> > > > + * during the lens movement are unstable. Currently there is no way to know\n> > > > + * when lens movement finished and this is a workaround for this. Wait a fixed\n> > > > + * amount of time on each movement. This parameter should be set according\n> > > > + * to the worst case - the number of frames it takes to move lens between\n> > > > + * limit positions.\n> > > > + * .\n> > >\n> > > Stray period ?\n> \n> The same as in the 04/10, dot is used to explicitly mark the end of\n> the list in the Doxygen.\n> \n> > > > + * \\sa libcamera::ipa::algorithms::AfHillClimbing for additional tuning\n> > > > + * parameters.\n> > > > + *\n> > > > + * \\todo Model the lens delay as number of frames required for the lens position\n> > > > + * to stabilize in the CameraLens class.\n> > >\n> > > More than that, we should take the lens movement into account in the\n> > > algorithm to avoid skipping frames at all.\n> \n> Yes, I agree. We should only base on the information if the lens\n> movement has finished instead of skipping frames in the AF. I will\n> improve that note.\n> \n> > > > + */\n> > > > +\n> > > > +LOG_DEFINE_CATEGORY(RkISP1Af)\n> > > > +\n> > > > +/**\n> > > > + * \\copydoc libcamera::ipa::Algorithm::init\n> > > > + */\n> > > > +int Af::init(IPAContext &context, const YamlObject &tuningData)\n> > > > +{\n> > > > + const auto &lensConfig = context.configuration.lens;\n> > > > + if (!lensConfig) {\n> > > > + LOG(RkISP1Af, Error) << \"Lens not found\";\n> > > > + return 1;\n> > > > + }\n> > > > +\n> > > > + waitFramesLens_ = tuningData[\"wait-frames-lens\"].get(1);\n> > > > +\n> > > > + LOG(RkISP1Af, Debug) << \"waitFramesLens_: \" << waitFramesLens_;\n> > > > +\n> > > > + return af.init(lensConfig->minFocusPosition,\n> > > > + lensConfig->maxFocusPosition, tuningData);\n> > > > +}\n> > > > +\n> > > > +/**\n> > > > + * \\copydoc libcamera::ipa::Algorithm::configure\n> > > > + */\n> > > > +int Af::configure(IPAContext &context,\n> > > > + const IPACameraSensorInfo &configInfo)\n> > > > +{\n> > > > + /*\n> > > > + * Lens position is unknown at the startup, so initialize\n> > > > + * the current position to something out of range.\n> > > > + */\n> > > > + context.activeState.af.lensPosition =\n> > > > + context.configuration.lens->maxFocusPosition + 1;\n> > > > +\n> > > > + af.configure(configInfo.outputSize);\n> > > > + return 0;\n> > > > +}\n> > > > +\n> > > > +/**\n> > > > + * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> > > > + */\n> > > > +void Af::queueRequest([[maybe_unused]] IPAContext &context,\n> > > > + [[maybe_unused]] const uint32_t frame,\n> > > > + [[maybe_unused]] IPAFrameContext &frameContext,\n> > > > + const ControlList &controls)\n> > > > +{\n> > > > + af.queueRequest(controls);\n> > > > +}\n> > > > +\n> > > > +/**\n> > > > + * \\copydoc libcamera::ipa::Algorithm::process\n> > > > + */\n> > > > +void Af::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> > > > + [[maybe_unused]] IPAFrameContext &frameContext,\n> > > > + const rkisp1_stat_buffer *stats,\n> > > > + [[maybe_unused]] ControlList &metadata)\n> > > > +{\n> > > > + uint32_t sharpness = stats->params.af.window[0].sum;\n> > > > + uint32_t luminance = stats->params.af.window[0].lum;\n> > > > +\n> > > > + LOG(RkISP1Af, Debug)\n> > > > + << \"lensPosition: \" << context.activeState.af.lensPosition\n> > > > + << \", Sharpness: \" << sharpness\n> > >\n> > > Maybe contrast, as everything in this series talks about contrast-based\n> > > AF ?\n> \n> Log category for this debug message is \"RkISP1Af\". Isn't it easier\n> for debugging, if We stick with the rkisp naming in the platform related\n> code?\n\nI was talking about \"Sharpness\", not the category :-) But the ISP\ndocumentation talks about sharpness, so I'm fine with that term.\n\n> > > > + << \", Luminance: \" << luminance;\n> > >\n> > > The luminance isn't used, so you could drop it.\n> >\n> > Or better, you should use it :-) The luminance is meant to normalize the\n> > sharpness values.\n> \n> Can We introduce the normalization later as an improvement?\n\nI've explained in the review of the next patch how to handle the shift\ncalculation. The normalization should be equally simple (in terms of\ncode at least).\n\nThe idea is that the sharpness is proportional to the square of the\npixel values, so higher illumination will result in higher sharpness. To\ncompensate for the illumination, you can simply divide the sharpness by\nthe square of the mean illumination over the window:\n\n- Multiply the illumination computed by the hardware by the shift factor\n to make it independent of the window size.\n- Divide the resulting value by the window size in pixels, to obtain a\n mean illumination in the [0.0, 1.0] range.\n- Divide the sharpness by the square of the mean illumination. Make sure\n the mean illumination value is > 0 to avoid divisions by 0.\n\nIt would be nice if you could do so already, as it should just be a few\nlines of code.\n\n> > > > +\n> > > > + int32_t newLensPosition = af.process(sharpness);\n> > > > +\n> > > > + if (newLensPosition != context.activeState.af.lensPosition) {\n> > > > + context.activeState.af.lensPosition = newLensPosition;\n> > > > + context.activeState.af.applyLensCtrls = true;\n> > > > + af.skipFrames(waitFramesLens_);\n> > >\n> > > I'm tempted to implement frame skipping in this class instead of the\n> > > ipa::algorithms::AfHillClimbing class. It would be simpler, we could\n> > > just skip calling process(). On the other hand, I suppose\n> > > ipa::algorithms::AfHillClimbing will need to change when we'll improve\n> > > the mechanism that takes lens movement into account, and it's hard to\n> > > predict how the interface will change.\n> \n> So maybe it would be simpler to use the current approach and improve\n> it later when needed, when the interface become clear.\n\nOK. Let's keep frame skipping where it is, but I'd like frame skipping\nto cover the lens movement only if possible, not the other delays. That\nmay be lots of yak shaving though, so this could be done on top too.\nWould you agree to give it a try once this series gets merged ? :-)\n\n> > > > + }\n> > > > +}\n> > >\n> > > The IPA module should also report the lens position in metadata.\n> > >\n> > > > +\n> > > > +REGISTER_IPA_ALGORITHM(Af, \"Af\")\n> > > > +\n> > > > +} /* namespace ipa::rkisp1::algorithms */\n> > > > +\n> > > > +} /* namespace libcamera */\n> > > > diff --git a/src/ipa/rkisp1/algorithms/af.h b/src/ipa/rkisp1/algorithms/af.h\n> > > > new file mode 100644\n> > > > index 00000000..3ba66d38\n> > > > --- /dev/null\n> > > > +++ b/src/ipa/rkisp1/algorithms/af.h\n> > > > @@ -0,0 +1,43 @@\n> > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > > +/*\n> > > > + * Copyright (C) 2023, Theobroma Systems\n> > > > + *\n> > > > + * af.h - RkISP1 AF hill climbing based control algorithm\n> > >\n> > > s/climbing based/climbing-based/\n> > >\n> > > > + */\n> > > > +\n> > > > +#pragma once\n> > > > +\n> > > > +#include \n> > > > +\n> > > > +#include \"libipa/algorithms/af_hill_climbing.h\"\n> > > > +\n> > > > +#include \"algorithm.h\"\n> > > > +\n> > > > +namespace libcamera {\n> > > > +\n> > > > +namespace ipa::rkisp1::algorithms {\n> > > > +\n> > > > +class Af : public Algorithm\n> > > > +{\n> > > > +public:\n> > > > + int init(IPAContext &context, const YamlObject &tuningData) override;\n> > > > + int configure(IPAContext &context,\n> > > > + const IPACameraSensorInfo &configInfo) override;\n> > > > + void queueRequest(IPAContext &context, uint32_t frame,\n> > > > + IPAFrameContext &frameContext,\n> > > > + const ControlList &controls) override;\n> > > > + void process(IPAContext &context, uint32_t frame,\n> > > > + IPAFrameContext &frameContext,\n> > > > + const rkisp1_stat_buffer *stats,\n> > > > + ControlList &metadata) override;\n> > > > +\n> > > > +private:\n> > > > + ipa::algorithms::AfHillClimbing af;\n> > >\n> > > s/af/af_/\n> > >\n> > > > +\n> > > > + /* Wait number of frames after changing lens position */\n> > > > + uint32_t waitFramesLens_ = 0;\n> > > > +};\n> > > > +\n> > > > +} /* namespace ipa::rkisp1::algorithms */\n> > > > +\n> > > > +} /* namespace libcamera */\n> > > > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n> > > > index 93a48329..ab7e44f3 100644\n> > > > --- a/src/ipa/rkisp1/algorithms/meson.build\n> > > > +++ b/src/ipa/rkisp1/algorithms/meson.build\n> > > > @@ -1,6 +1,7 @@\n> > > > # SPDX-License-Identifier: CC0-1.0\n> > > >\n> > > > rkisp1_ipa_algorithms = files([\n> > > > + 'af.cpp',\n> > > > 'agc.cpp',\n> > > > 'awb.cpp',\n> > > > 'blc.cpp',\n> > > > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> > > > index aea99299..d80a7b1b 100644\n> > > > --- a/src/ipa/rkisp1/ipa_context.cpp\n> > > > +++ b/src/ipa/rkisp1/ipa_context.cpp\n> > > > @@ -139,6 +139,19 @@ namespace libcamera::ipa::rkisp1 {\n> > > > * member may be read by any algorithm, but shall only be written by its owner.\n> > > > */\n> > > >\n> > > > +/**\n> > > > + * \\var IPAActiveState::af\n> > > > + * \\brief State for the Automatic Focus Control algorithm\n> > > > + *\n> > > > + * \\var IPAActiveState::af.lensPosition\n> > > > + * \\brief Lens position calculated by the AF algorithm\n> > > > + *\n> > > > + * \\var IPAActiveState::af.applyLensCtrls\n> > > > + * \\brief Whether the lens position should be applied\n> > > > + *\n> > > > + * If true, IPA should send new controls to the PH to set new lens position.\n> > > > + */\n> > > > +\n> > > > /**\n> > > > * \\var IPAActiveState::agc\n> > > > * \\brief State for the Automatic Gain Control algorithm\n> > > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > > > index 65b3fbfe..3dcc5aa0 100644\n> > > > --- a/src/ipa/rkisp1/ipa_context.h\n> > > > +++ b/src/ipa/rkisp1/ipa_context.h\n> > > > @@ -62,6 +62,11 @@ struct IPASessionConfiguration {\n> > > > };\n> > > >\n> > > > struct IPAActiveState {\n> > > > + struct {\n> > > > + int32_t lensPosition;\n> > > > + bool applyLensCtrls;\n> > >\n> > > I suppose applyLens will be used later in the series.\n> \n> Should I move it to the later patches?\n\nNo, it's OK, it was a note for myself.\n\n> > >\n> > > > + } af;\n> > > > +\n> > > > struct {\n> > > > struct {\n> > > > uint32_t exposure;","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 76124C31E9\n\tfor ;\n\tWed, 31 May 2023 17:53:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BEF0A626FA;\n\tWed, 31 May 2023 19:53:40 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 14896626F8\n\tfor ;\n\tWed, 31 May 2023 19:53:39 +0200 (CEST)","from pendragon.ideasonboard.com (om126205251136.34.openmobile.ne.jp\n\t[126.205.251.136])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AF7DD12F;\n\tWed, 31 May 2023 19:53:16 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685555620;\n\tbh=RKjqiEtEtEVtbWy+aPZUI9D/DVypX38vCyj+Z4oqLh4=;\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=i4RujAYGISrZX1WPARI85aUaSvt3i9288CoCECSYiRCAJSRVy0/5GVzeT454Ro7/X\n\tTUCMnkZZtihj7L61iAucaSGaYQu0COr5P8bN4CXIaquv1sACjLc2gwEljWwL3r2hMK\n\tBAkkQal84GC2In8imhEP0EEZ4Tq0R4rO6dmQpMfzr3SFxTAOUD/V+Nbi76Hp8uzAqR\n\tUTG12/pWZg2FTbgBxTrl9tCEqEQ9TuIHptJSGN4nJCsaMctEgG/CsTnG1elIf95EgC\n\tMr4CO40ocZSRRRJRGGYR5EWzkkJgbIG7mC8nog03BDIfLCdMHVNwSdI3cFDsunllLZ\n\te9nPvM7QRU0mw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1685555597;\n\tbh=RKjqiEtEtEVtbWy+aPZUI9D/DVypX38vCyj+Z4oqLh4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=keqTOESn8nj14sm1w1Y5i+7+/pF5rWiZo95iyEI8SFjxBkUT5VRx0K+1gd/duOSVd\n\t7G4gRGbYpC/4/Q1AGBpd12cl/bhkB5dqMGWaTnKyfaXSQvefkxnJVgmoM1xwepToZO\n\tusbcVr7uXPTEZRqcuZ7UEzIBXlWaNBjTNwKARAtg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"keqTOESn\"; dkim-atps=neutral","Date":"Wed, 31 May 2023 20:53:37 +0300","To":"Daniel Semkowicz ","Message-ID":"<20230531175337.GH24749@pendragon.ideasonboard.com>","References":"<20230331081930.19289-1-dse@thaumatec.com>\n\t<20230331081930.19289-7-dse@thaumatec.com>\n\t<20230426034658.GG1667@pendragon.ideasonboard.com>\n\t<20230426040417.GI1667@pendragon.ideasonboard.com>\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 v6 06/10] ipa: rkisp1: Add AF\n\talgorithm based on AfHillClimbing","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":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27340,"web_url":"https://patchwork.libcamera.org/comment/27340/","msgid":"","date":"2023-06-14T05:39:14","subject":"Re: [libcamera-devel] [PATCH v6 06/10] ipa: rkisp1: Add AF\n\talgorithm based on AfHillClimbing","submitter":{"id":126,"url":"https://patchwork.libcamera.org/api/people/126/","name":"Daniel Semkowicz","email":"dse@thaumatec.com"},"content":"Hi Laurent,\n\nOn Wed, May 31, 2023 at 7:53 PM Laurent Pinchart\n wrote:\n>\n> Hi Daniel,\n>\n> On Fri, May 26, 2023 at 11:19:02AM +0200, Daniel Semkowicz wrote:\n> > On Wed, Apr 26, 2023 at 6:04 AM Laurent Pinchart wrote:\n> > > On Wed, Apr 26, 2023 at 06:46:58AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > > > On Fri, Mar 31, 2023 at 10:19:26AM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> > > > > Add contrast based auto focus implementation for rkisp1 platform using\n> > > > > the common AfHillClimbing algorithm.\n> > > > >\n> > > > > Rockchip ISP AF block allows calculation of sharpness and luminance\n> > > > > in up to three user defined windows. If no windows are set, there are\n> > > > > some default settings applied for the first window and exposed through\n> > > > > the driver. For each frame, use the sharpness value calculated for this\n> > > > > default window and feed the hill climbing algorithm with them. Then set\n> > > > > the lens position to value calculated by the algorithm.\n> > > > >\n> > > > > Signed-off-by: Daniel Semkowicz \n> > > > > ---\n> > > > > src/ipa/rkisp1/algorithms/af.cpp | 121 ++++++++++++++++++++++++++\n> > > > > src/ipa/rkisp1/algorithms/af.h | 43 +++++++++\n> > > > > src/ipa/rkisp1/algorithms/meson.build | 1 +\n> > > > > src/ipa/rkisp1/ipa_context.cpp | 13 +++\n> > > > > src/ipa/rkisp1/ipa_context.h | 5 ++\n> > > > > 5 files changed, 183 insertions(+)\n> > > > > create mode 100644 src/ipa/rkisp1/algorithms/af.cpp\n> > > > > create mode 100644 src/ipa/rkisp1/algorithms/af.h\n> > > > >\n> > > > > diff --git a/src/ipa/rkisp1/algorithms/af.cpp b/src/ipa/rkisp1/algorithms/af.cpp\n> > > > > new file mode 100644\n> > > > > index 00000000..fde924d4\n> > > > > --- /dev/null\n> > > > > +++ b/src/ipa/rkisp1/algorithms/af.cpp\n> > > > > @@ -0,0 +1,121 @@\n> > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > > > +/*\n> > > > > + * Copyright (C) 2023, Theobroma Systems\n> > > > > + *\n> > > > > + * af.cpp - RkISP1 AF hill climbing based control algorithm\n> > > >\n> > > > s/climbing based/climbing-based/\n> > > >\n> > > > > + */\n> > > > > +\n> > > > > +#include \"af.h\"\n> > > > > +\n> > > > > +/**\n> > > > > + * \\file af.h\n> > > >\n> > > > Missing \\brief\n> > > >\n> > > > > + */\n> > > > > +\n> > > > > +namespace libcamera {\n> > > > > +\n> > > > > +namespace ipa::rkisp1::algorithms {\n> > > > > +\n> > > > > +/**\n> > > > > + * \\class Af\n> > > > > + * \\brief AF contrast based hill climbing control algorithm for RkISP platforms\n> > > >\n> > > > s/contrast based/contrast-based/\n> > > >\n> > > > > + *\n> > > > > + * Auto focus algorithm for RkISP platforms, based on the common hill climbing\n> > > > > + * auto focus implementation (libcamera::ipa::algorithms::AfHillClimbing).\n> > > > > + *\n> > > > > + * This is the platform layer of the algorithm.\n> > > > > + *\n> > > > > + * Tuning file parameters:\n> > > > > + * - **wait-frames-lens:** Number of frames that should be skipped when lens\n> > > >\n> > > > rkisp1 tuning files use camelCase for parameter names.\n> >\n> > I will correct it.\n> >\n> > > > > + * position is changed. Lens movement takes some time and statistics measured\n> > > > > + * during the lens movement are unstable. Currently there is no way to know\n> > > > > + * when lens movement finished and this is a workaround for this. Wait a fixed\n> > > > > + * amount of time on each movement. This parameter should be set according\n> > > > > + * to the worst case - the number of frames it takes to move lens between\n> > > > > + * limit positions.\n> > > > > + * .\n> > > >\n> > > > Stray period ?\n> >\n> > The same as in the 04/10, dot is used to explicitly mark the end of\n> > the list in the Doxygen.\n> >\n> > > > > + * \\sa libcamera::ipa::algorithms::AfHillClimbing for additional tuning\n> > > > > + * parameters.\n> > > > > + *\n> > > > > + * \\todo Model the lens delay as number of frames required for the lens position\n> > > > > + * to stabilize in the CameraLens class.\n> > > >\n> > > > More than that, we should take the lens movement into account in the\n> > > > algorithm to avoid skipping frames at all.\n> >\n> > Yes, I agree. We should only base on the information if the lens\n> > movement has finished instead of skipping frames in the AF. I will\n> > improve that note.\n> >\n> > > > > + */\n> > > > > +\n> > > > > +LOG_DEFINE_CATEGORY(RkISP1Af)\n> > > > > +\n> > > > > +/**\n> > > > > + * \\copydoc libcamera::ipa::Algorithm::init\n> > > > > + */\n> > > > > +int Af::init(IPAContext &context, const YamlObject &tuningData)\n> > > > > +{\n> > > > > + const auto &lensConfig = context.configuration.lens;\n> > > > > + if (!lensConfig) {\n> > > > > + LOG(RkISP1Af, Error) << \"Lens not found\";\n> > > > > + return 1;\n> > > > > + }\n> > > > > +\n> > > > > + waitFramesLens_ = tuningData[\"wait-frames-lens\"].get(1);\n> > > > > +\n> > > > > + LOG(RkISP1Af, Debug) << \"waitFramesLens_: \" << waitFramesLens_;\n> > > > > +\n> > > > > + return af.init(lensConfig->minFocusPosition,\n> > > > > + lensConfig->maxFocusPosition, tuningData);\n> > > > > +}\n> > > > > +\n> > > > > +/**\n> > > > > + * \\copydoc libcamera::ipa::Algorithm::configure\n> > > > > + */\n> > > > > +int Af::configure(IPAContext &context,\n> > > > > + const IPACameraSensorInfo &configInfo)\n> > > > > +{\n> > > > > + /*\n> > > > > + * Lens position is unknown at the startup, so initialize\n> > > > > + * the current position to something out of range.\n> > > > > + */\n> > > > > + context.activeState.af.lensPosition =\n> > > > > + context.configuration.lens->maxFocusPosition + 1;\n> > > > > +\n> > > > > + af.configure(configInfo.outputSize);\n> > > > > + return 0;\n> > > > > +}\n> > > > > +\n> > > > > +/**\n> > > > > + * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> > > > > + */\n> > > > > +void Af::queueRequest([[maybe_unused]] IPAContext &context,\n> > > > > + [[maybe_unused]] const uint32_t frame,\n> > > > > + [[maybe_unused]] IPAFrameContext &frameContext,\n> > > > > + const ControlList &controls)\n> > > > > +{\n> > > > > + af.queueRequest(controls);\n> > > > > +}\n> > > > > +\n> > > > > +/**\n> > > > > + * \\copydoc libcamera::ipa::Algorithm::process\n> > > > > + */\n> > > > > +void Af::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n> > > > > + [[maybe_unused]] IPAFrameContext &frameContext,\n> > > > > + const rkisp1_stat_buffer *stats,\n> > > > > + [[maybe_unused]] ControlList &metadata)\n> > > > > +{\n> > > > > + uint32_t sharpness = stats->params.af.window[0].sum;\n> > > > > + uint32_t luminance = stats->params.af.window[0].lum;\n> > > > > +\n> > > > > + LOG(RkISP1Af, Debug)\n> > > > > + << \"lensPosition: \" << context.activeState.af.lensPosition\n> > > > > + << \", Sharpness: \" << sharpness\n> > > >\n> > > > Maybe contrast, as everything in this series talks about contrast-based\n> > > > AF ?\n> >\n> > Log category for this debug message is \"RkISP1Af\". Isn't it easier\n> > for debugging, if We stick with the rkisp naming in the platform related\n> > code?\n>\n> I was talking about \"Sharpness\", not the category :-) But the ISP\n> documentation talks about sharpness, so I'm fine with that term.\n>\n> > > > > + << \", Luminance: \" << luminance;\n> > > >\n> > > > The luminance isn't used, so you could drop it.\n> > >\n> > > Or better, you should use it :-) The luminance is meant to normalize the\n> > > sharpness values.\n> >\n> > Can We introduce the normalization later as an improvement?\n>\n> I've explained in the review of the next patch how to handle the shift\n> calculation. The normalization should be equally simple (in terms of\n> code at least).\n>\n> The idea is that the sharpness is proportional to the square of the\n> pixel values, so higher illumination will result in higher sharpness. To\n> compensate for the illumination, you can simply divide the sharpness by\n> the square of the mean illumination over the window:\n>\n> - Multiply the illumination computed by the hardware by the shift factor\n> to make it independent of the window size.\n> - Divide the resulting value by the window size in pixels, to obtain a\n> mean illumination in the [0.0, 1.0] range.\n> - Divide the sharpness by the square of the mean illumination. Make sure\n> the mean illumination value is > 0 to avoid divisions by 0.\n>\n> It would be nice if you could do so already, as it should just be a few\n> lines of code.\n\nOk, I will do some tests with that.\n\n>\n> > > > > +\n> > > > > + int32_t newLensPosition = af.process(sharpness);\n> > > > > +\n> > > > > + if (newLensPosition != context.activeState.af.lensPosition) {\n> > > > > + context.activeState.af.lensPosition = newLensPosition;\n> > > > > + context.activeState.af.applyLensCtrls = true;\n> > > > > + af.skipFrames(waitFramesLens_);\n> > > >\n> > > > I'm tempted to implement frame skipping in this class instead of the\n> > > > ipa::algorithms::AfHillClimbing class. It would be simpler, we could\n> > > > just skip calling process(). On the other hand, I suppose\n> > > > ipa::algorithms::AfHillClimbing will need to change when we'll improve\n> > > > the mechanism that takes lens movement into account, and it's hard to\n> > > > predict how the interface will change.\n> >\n> > So maybe it would be simpler to use the current approach and improve\n> > it later when needed, when the interface become clear.\n>\n> OK. Let's keep frame skipping where it is, but I'd like frame skipping\n> to cover the lens movement only if possible, not the other delays. That\n> may be lots of yak shaving though, so this could be done on top too.\n> Would you agree to give it a try once this series gets merged ? :-)\n\nI think the discussions in other patches in this series already impacted the\ndelays handling, so I will probably need to change it already in the next\nversion.\n\n>\n> > > > > + }\n> > > > > +}\n> > > >\n> > > > The IPA module should also report the lens position in metadata.\n> > > >\n> > > > > +\n> > > > > +REGISTER_IPA_ALGORITHM(Af, \"Af\")\n> > > > > +\n> > > > > +} /* namespace ipa::rkisp1::algorithms */\n> > > > > +\n> > > > > +} /* namespace libcamera */\n> > > > > diff --git a/src/ipa/rkisp1/algorithms/af.h b/src/ipa/rkisp1/algorithms/af.h\n> > > > > new file mode 100644\n> > > > > index 00000000..3ba66d38\n> > > > > --- /dev/null\n> > > > > +++ b/src/ipa/rkisp1/algorithms/af.h\n> > > > > @@ -0,0 +1,43 @@\n> > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > > > +/*\n> > > > > + * Copyright (C) 2023, Theobroma Systems\n> > > > > + *\n> > > > > + * af.h - RkISP1 AF hill climbing based control algorithm\n> > > >\n> > > > s/climbing based/climbing-based/\n> > > >\n> > > > > + */\n> > > > > +\n> > > > > +#pragma once\n> > > > > +\n> > > > > +#include \n> > > > > +\n> > > > > +#include \"libipa/algorithms/af_hill_climbing.h\"\n> > > > > +\n> > > > > +#include \"algorithm.h\"\n> > > > > +\n> > > > > +namespace libcamera {\n> > > > > +\n> > > > > +namespace ipa::rkisp1::algorithms {\n> > > > > +\n> > > > > +class Af : public Algorithm\n> > > > > +{\n> > > > > +public:\n> > > > > + int init(IPAContext &context, const YamlObject &tuningData) override;\n> > > > > + int configure(IPAContext &context,\n> > > > > + const IPACameraSensorInfo &configInfo) override;\n> > > > > + void queueRequest(IPAContext &context, uint32_t frame,\n> > > > > + IPAFrameContext &frameContext,\n> > > > > + const ControlList &controls) override;\n> > > > > + void process(IPAContext &context, uint32_t frame,\n> > > > > + IPAFrameContext &frameContext,\n> > > > > + const rkisp1_stat_buffer *stats,\n> > > > > + ControlList &metadata) override;\n> > > > > +\n> > > > > +private:\n> > > > > + ipa::algorithms::AfHillClimbing af;\n> > > >\n> > > > s/af/af_/\n> > > >\n> > > > > +\n> > > > > + /* Wait number of frames after changing lens position */\n> > > > > + uint32_t waitFramesLens_ = 0;\n> > > > > +};\n> > > > > +\n> > > > > +} /* namespace ipa::rkisp1::algorithms */\n> > > > > +\n> > > > > +} /* namespace libcamera */\n> > > > > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build\n> > > > > index 93a48329..ab7e44f3 100644\n> > > > > --- a/src/ipa/rkisp1/algorithms/meson.build\n> > > > > +++ b/src/ipa/rkisp1/algorithms/meson.build\n> > > > > @@ -1,6 +1,7 @@\n> > > > > # SPDX-License-Identifier: CC0-1.0\n> > > > >\n> > > > > rkisp1_ipa_algorithms = files([\n> > > > > + 'af.cpp',\n> > > > > 'agc.cpp',\n> > > > > 'awb.cpp',\n> > > > > 'blc.cpp',\n> > > > > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> > > > > index aea99299..d80a7b1b 100644\n> > > > > --- a/src/ipa/rkisp1/ipa_context.cpp\n> > > > > +++ b/src/ipa/rkisp1/ipa_context.cpp\n> > > > > @@ -139,6 +139,19 @@ namespace libcamera::ipa::rkisp1 {\n> > > > > * member may be read by any algorithm, but shall only be written by its owner.\n> > > > > */\n> > > > >\n> > > > > +/**\n> > > > > + * \\var IPAActiveState::af\n> > > > > + * \\brief State for the Automatic Focus Control algorithm\n> > > > > + *\n> > > > > + * \\var IPAActiveState::af.lensPosition\n> > > > > + * \\brief Lens position calculated by the AF algorithm\n> > > > > + *\n> > > > > + * \\var IPAActiveState::af.applyLensCtrls\n> > > > > + * \\brief Whether the lens position should be applied\n> > > > > + *\n> > > > > + * If true, IPA should send new controls to the PH to set new lens position.\n> > > > > + */\n> > > > > +\n> > > > > /**\n> > > > > * \\var IPAActiveState::agc\n> > > > > * \\brief State for the Automatic Gain Control algorithm\n> > > > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > > > > index 65b3fbfe..3dcc5aa0 100644\n> > > > > --- a/src/ipa/rkisp1/ipa_context.h\n> > > > > +++ b/src/ipa/rkisp1/ipa_context.h\n> > > > > @@ -62,6 +62,11 @@ struct IPASessionConfiguration {\n> > > > > };\n> > > > >\n> > > > > struct IPAActiveState {\n> > > > > + struct {\n> > > > > + int32_t lensPosition;\n> > > > > + bool applyLensCtrls;\n> > > >\n> > > > I suppose applyLens will be used later in the series.\n> >\n> > Should I move it to the later patches?\n>\n> No, it's OK, it was a note for myself.\n>\n> > > >\n> > > > > + } af;\n> > > > > +\n> > > > > struct {\n> > > > > struct {\n> > > > > uint32_t exposure;\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 936CCC31E9\n\tfor ;\n\tWed, 14 Jun 2023 05:39:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DEBCE6289E;\n\tWed, 14 Jun 2023 07:39:30 +0200 (CEST)","from mail-pg1-x52b.google.com (mail-pg1-x52b.google.com\n\t[IPv6:2607:f8b0:4864:20::52b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9028A614FE\n\tfor ;\n\tWed, 14 Jun 2023 07:39:29 +0200 (CEST)","by mail-pg1-x52b.google.com with SMTP id\n\t41be03b00d2f7-54f73f09765so2070203a12.1\n\tfor ;\n\tTue, 13 Jun 2023 22:39:29 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686721170;\n\tbh=NnyvcJoW6hP+Exj69+BfuYkqsEbwapW6GtgQn362ovI=;\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=MpJuuV/jm+2nNDjGXJzYC6QcNCiUUwgpwLZieZvG8DeJKY7vIJXTFwQT2T5SVDHxH\n\tWfcaEEksuwwV8IueVzTZAxHnDHDK2KlwQq+01vCpUv5IZvoGigDMK5wa4n4o7C7/EA\n\tqGs5/Ov/ky5XCT7PlShU2uYXYeT7j0r/TrR3pHOatZQlsTXyGwHFyb3eCzKZ/Qr2Je\n\tRmpRSZJJH1s8B3P80uZoMeenpRTTPZZY6MEeVRsDCyR4vetklyz/EvL5t5nYqvmx4P\n\tZyY/p1biJfHlzy2kuAglaETl5A0CB7mpvemdrsk0z1FDJhoACeY0jcWGYpiwsMGNhd\n\t5FAg0jiKO9qkA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=thaumatec-com.20221208.gappssmtp.com; s=20221208; t=1686721168;\n\tx=1689313168; \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=3wkSpzBcuuslXWhm0CRKyYT5wlsbPp1c7XrlGMAQKAY=;\n\tb=C0N5SXAQVRQapNVLNOGTYbCCykjEWYYmSbLOOMdyK/GlsKs9l6nnO5+t3fj7sEZ6ni\n\tKPMvQvvcxnmMhnv8DAjiI5h0mWRtuYTyFGyuckoVbbja7fmakeYJqKqiZYmiI6rAjA8r\n\tkmVVzshWVFnulcGUpz4+C45k580XCy7y6rbiPQmyNe8BqODIfiK6HYWPfpNFw6U/EGsD\n\thLh2TCDY+f2mR589hNUIx/DtKZdkpXC+d6cFSxl36KS8bcnUN+mrMXUEF7ipDSSumbPL\n\tbAfLPC5D6BUeHE5Cy13Q5GgHoPWcxPQwb7dZDbOjHmf9qoCnRzeUpq9BObVLKCuZ9MrR\n\t+K+Q=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=thaumatec-com.20221208.gappssmtp.com\n\theader.i=@thaumatec-com.20221208.gappssmtp.com header.b=\"C0N5SXAQ\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1686721168; x=1689313168;\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=3wkSpzBcuuslXWhm0CRKyYT5wlsbPp1c7XrlGMAQKAY=;\n\tb=ONvRAlSYthN918a3+ScGUtrxw+jIv6fvOYAeWR68ZEVUPz9IlhIA6EcZM3LcUFiD6Y\n\tCo9qfZUAMJMWPizmZdph+rvaaqIio9lnrlt2LYNaV+CUSSYYV9DTbTk7cQ8Ie51zfj+L\n\tTGkmTdLy0KH3S780zGRBrsCIBiVkcx/CzhWXdPZIcIZxzZRJFZv3sG3yWVTqJNqwVOrc\n\tun+O9sqnS4OP0rUOeNClRWv3GXhBb+qqU1pq+0p+jHb/VsfiC6Y1p6Zz0+AWhCTzI2rW\n\twzZsNPeZagwq0kXX2mXPQQyvZqRQai8SUSVnOIJD8UGjSRfNLlDf5yRQPcvxAD/JkeH4\n\t4oig==","X-Gm-Message-State":"AC+VfDwx7v8kRD7dJAUTltCURdVc4NHaN7xF8Y7UPs3ciSjQbVGs5eLT\n\tD1L8wBJ90HadYUv3zMhAVsVQWI0qd75TL0LcvpgUQg==","X-Google-Smtp-Source":"ACHHUZ7BDyDingUJcDyq3cc2taj6IdJLpvJ+zsGtbWsQoHHnZHgMomnkpR/sn27o3DEc5x+GS/xi0goxK20s3AbEt3k=","X-Received":"by 2002:a17:90a:de98:b0:25c:cbc:af5a with SMTP id\n\tn24-20020a17090ade9800b0025c0cbcaf5amr485469pjv.21.1686721167438;\n\tTue, 13 Jun 2023 22:39:27 -0700 (PDT)","MIME-Version":"1.0","References":"<20230331081930.19289-1-dse@thaumatec.com>\n\t<20230331081930.19289-7-dse@thaumatec.com>\n\t<20230426034658.GG1667@pendragon.ideasonboard.com>\n\t<20230426040417.GI1667@pendragon.ideasonboard.com>\n\t\n\t<20230531175337.GH24749@pendragon.ideasonboard.com>","In-Reply-To":"<20230531175337.GH24749@pendragon.ideasonboard.com>","Date":"Wed, 14 Jun 2023 07:39:14 +0200","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH v6 06/10] ipa: rkisp1: Add AF\n\talgorithm based on AfHillClimbing","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\" "}}]