[{"id":26737,"web_url":"https://patchwork.libcamera.org/comment/26737/","msgid":"<20230324153959.2gvwhsvaw26wqa6h@uno.localdomain>","date":"2023-03-24T15:39:59","subject":"Re: [libcamera-devel] [PATCH v5 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 24, 2023 at 03:29:04PM +0100, 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 <dse@thaumatec.com>\n> ---\n>  src/ipa/rkisp1/algorithms/af.cpp      | 117 ++++++++++++++++++++++++++\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, 179 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..aca127d2\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/af.cpp\n> @@ -0,0 +1,117 @@\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\n> + * (libcamera::ipa::algorithms::AfHillClimbing).\n\nfits on the previous line\n\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      ^ rogue space\n\nThanks :)\n\n> + */\n> +\n> +LOG_DEFINE_CATEGORY(RkISP1Af)\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::init\n> + */\n> +int Af::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)\n> +{\n> +\twaitFramesLens_ = tuningData[\"wait-frames-lens\"].get<uint32_t>(1);\n> +\n> +\tLOG(RkISP1Af, Debug) << \"waitFramesLens_: \" << waitFramesLens_;\n> +\n> +\treturn af.init(tuningData);\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::configure\n> + */\n> +int Af::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)\n> +{\n> +\tconst auto &lensConfig = context.configuration.lens;\n> +\n> +\taf.configure(lensConfig.minFocusPosition, lensConfig.maxFocusPosition,\n> +\t\t     configInfo.outputSize);\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 = lensConfig.maxFocusPosition + 1;\n> +\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.setFramesToSkip(waitFramesLens_);\n\nI wonder if this wouldn't be better names simply \"skipFrames()\"\n\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 <linux/rkisp1-config.h>\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\nI envision one day where this can be made\n        ipa::algorithms::Af *af;\n\nand the actual algorithm instantiated a run-time based on a\nconfiguration option. This is a good start though\n\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 401c098f..5c5f80c7 100644\n> --- a/src/ipa/rkisp1/ipa_context.cpp\n> +++ b/src/ipa/rkisp1/ipa_context.cpp\n> @@ -134,6 +134,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 bfb6e1b7..2c2eec3b 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -58,6 +58,11 @@ struct IPASessionConfiguration {\n>  };\n>\n>  struct IPAActiveState {\n> +\tstruct {\n> +\t\tint32_t lensPosition;\n> +\t\tbool applyLensCtrls;\n> +\t} af;\n> +\n\nReviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n\nThanks\n  j\n\n>  \tstruct {\n>  \t\tstruct {\n>  \t\t\tuint32_t exposure;\n> --\n> 2.39.2\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 50890C0F2A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 24 Mar 2023 15:40:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8717361ECF;\n\tFri, 24 Mar 2023 16:40:04 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 261BF603AA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 24 Mar 2023 16:40:03 +0100 (CET)","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 A491FA58;\n\tFri, 24 Mar 2023 16:40:02 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679672404;\n\tbh=5+i11CROXUO6RIyZw1LteloOEBHeSUm4IPM+X9BKIF8=;\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=hLnFzbuthVbEWZRkpIqE6FMu0k9CXWAGQVab1SBiNlOAdf48dWT/RAU1pn5aEapTu\n\thPVBiPEjcnIfCUlNebGsLGuuc1Un6TkqDTBEq4ibWV2AeR1t/7rrnpMt+7kzf714VB\n\txok9fcPtYv72O6od7iXmLvP2mqmNlc3YNvEbk4W861KFKghqpIWQ+XAJuIUKTdlG85\n\tMk9MyKhD1UwEklTM7z1xle6lhfRfmLLq1X2nG1yB4/tz/7Hqn5PA1rchmXCJG2gKQ5\n\tfjr2vpQ1ruAC84MuqRx9P/4HJCSk6P4+Cp8ti0IlBvg0SwK6yTov1XR6Wj+/PTpksJ\n\t/1DQHyko2sPdQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1679672402;\n\tbh=5+i11CROXUO6RIyZw1LteloOEBHeSUm4IPM+X9BKIF8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WRycUmAt0IqRmZECjxtp4gw6+svTfsoQzGUMoOzs33p0vxr1KHj6ISh4Cu/pOUK0m\n\t/F4zBqglUi2N6nIlFpA952PL4fTdncEf0HwU2E5cpJyJH57WsP8Zjx7t5LErmxvbRV\n\tYsLrNZMJdKwypFQoHsGKhqigxyTYG6P5waK7QgO4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"WRycUmAt\"; dkim-atps=neutral","Date":"Fri, 24 Mar 2023 16:39:59 +0100","To":"Daniel Semkowicz <dse@thaumatec.com>","Message-ID":"<20230324153959.2gvwhsvaw26wqa6h@uno.localdomain>","References":"<20230324142908.64224-1-dse@thaumatec.com>\n\t<20230324142908.64224-7-dse@thaumatec.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230324142908.64224-7-dse@thaumatec.com>","Subject":"Re: [libcamera-devel] [PATCH v5 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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"jacopo.mondi@ideasonboard.com, libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]