[{"id":26397,"web_url":"https://patchwork.libcamera.org/comment/26397/","msgid":"<167555025898.2778051.8149209482432418233@Monstersaurus>","date":"2023-02-04T22:37:38","subject":"Re: [libcamera-devel] [PATCH v3 5/8] ipa: rkisp1: Add AF algorithm\n\tbasing on common AfHillClimbing class","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Daniel,\n\nThank you for progressing AF support on RKISP1, and in particular\nrebasing all this after all the rework to the algorithm designs.\n\nQuoting Daniel Semkowicz via libcamera-devel (2023-01-19 08:41:09)\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 | 101 ++++++++++++++++++++++++++\n> src/ipa/rkisp1/algorithms/af.h | 42 +++++++++++\n> src/ipa/rkisp1/algorithms/meson.build | 1 +\n> 3 files changed, 144 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..c2a321cd\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/af.cpp\n> @@ -0,0 +1,101 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2022, 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::ipa::rkisp1::algorithms {\n> +\n> +/**\n> + * \\class Af\n> + * \\brief AF control algorithm\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> + waitFramesLens_ = tuningData[\"wait-frames-lens\"].get(1);\n\nI am weary that perhaps many of the other tuning file parameters may not\nbe well documented, but I'd like to fix that.\n\nWould it be possible to prepare some documentation on the accepted\ntuning parameters handled by this algorithm, perhaps collected as a\nsingle documentation block at the top of the file please?\n\n\n> +\n> + LOG(RkISP1Af, Debug) << \"waitFramesLens_: \" << waitFramesLens_;\n> +\n> + return initBase(tuningData);\n\nI guess we may need some common documentation for the bases\nimplementation here too.\n\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::configure\n> + */\n> +int Af::configure([[maybe_unused]] IPAContext &context,\n> + [[maybe_unused]] const IPACameraSensorInfo &configInfo)\n> +{\n> + return 0;\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::queueRequest\n> + */\n> +void Af::queueRequest([[maybe_unused]] IPAContext &context,\n> + const uint32_t frame,\n> + [[maybe_unused]] IPAFrameContext &frameContext,\n> + const ControlList &controls)\n> +{\n> + queueRequestBase(frame, controls);\n> +}\n> +\n> +/**\n> + * \\copydoc libcamera::ipa::Algorithm::prepare\n> + */\n> +void Af::prepare([[maybe_unused]] IPAContext &context,\n> + [[maybe_unused]] const uint32_t frame,\n> + [[maybe_unused]] IPAFrameContext &frameContext,\n> + [[maybe_unused]] rkisp1_params_cfg *params)\n> +{\n> +}\n\nI think algorithm methods are optional. If they're not needed they don't\nneed to be defined in the header or the class.\n\nThe base/virtual implementation in libipa/algorithm.h will cover it so\nwe don't need to duplicate here with a no-op implementation.\n\nSame for Af::configure()?\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) << \"lensPosition: \" << context.activeState.af.lensPosition\n> + << \", Sharpness: \" << sharpness\n> + << \", Luminance: \" << luminance;\n> +\n> + uint32_t lensPosition = processAutofocus(sharpness);\n> +\n> + if (lensPosition != context.activeState.af.lensPosition) {\n> + context.activeState.af.lensPosition = lensPosition;\n> + context.activeState.af.applyLensCtrls = true;\n> + setFramesToSkip(waitFramesLens_);\n> + }\n> +}\n> +\n> +void Af::setMeteringMode([[maybe_unused]] controls::AfMeteringEnum metering)\n> +{\n> + LOG(RkISP1Af, Error) << __FUNCTION__ << \" not implemented!\";\n> +}\n> +\n> +void Af::setWindows([[maybe_unused]] Span windows)\n> +{\n> + LOG(RkISP1Af, Error) << __FUNCTION__ << \" not implemented!\";\n> +}\n> +\n> +REGISTER_IPA_ALGORITHM(Af, \"Af\")\n> +\n> +} /* namespace libcamera::ipa::rkisp1::algorithms */\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..882be952\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/algorithms/af.h\n> @@ -0,0 +1,42 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2022, 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::ipa::rkisp1::algorithms {\n> +\n> +class Af : public ipa::common::algorithms::AfHillClimbing, public Algorithm\n> +{\n> +public:\n> + Af() = default;\n> + ~Af() = default;\n> +\n> + int init(IPAContext &context, const YamlObject &tuningData) override;\n> + int configure(IPAContext &context, const IPACameraSensorInfo &configInfo) override;\n> + void queueRequest(IPAContext &context, const uint32_t frame,\n> + IPAFrameContext &frameContext, const ControlList &controls) override;\n> + void prepare(IPAContext &context, const uint32_t frame,\n> + IPAFrameContext &frameContext, rkisp1_params_cfg *params) override;\n> + void process(IPAContext &context, const uint32_t frame,\n> + IPAFrameContext &frameContext, const rkisp1_stat_buffer *stats,\n> + ControlList &metadata) override;\n> +\n> +private:\n> + void setMeteringMode(controls::AfMeteringEnum metering) final;\n> + void setWindows(Span windows) final;\n\nThese two methods are private and not called in this patch. Maybe just\nadd them when they are needed rather than adding unused stubs that get\nupdated later.\n\n\n\n> +\n> + /* Wait number of frames after changing lens position */\n> + uint32_t waitFramesLens_;\n> +};\n> +\n> +} /* namespace libcamera::ipa::rkisp1::algorithms */\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> -- \n> 2.39.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8A8DDBEFBE\n\tfor ;\n\tSat, 4 Feb 2023 22:37:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D6102625E4;\n\tSat, 4 Feb 2023 23:37:43 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9FE5F625DD\n\tfor ;\n\tSat, 4 Feb 2023 23:37:41 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F04A81026;\n\tSat, 4 Feb 2023 23:37:40 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1675550263;\n\tbh=C+XIT13+CEv4x8kRo9pZS5PW4aWH1sz2mF7tFGmUN+s=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=lg6K6mjyjrxpVemUrZDOuDi+yD6Gk7vTA4Uq1HClnE+V+FYAtHTXoWXQhUleUjscU\n\teZsUIR+u0LILFycH5ZLY6n7We1YkOWqOXPp3A5fHyMNdRHWdT3GcJHphu4SCU/oMsv\n\tSvUKZZHLnkP8yj6tjMMCONZWhKJjGzCPEw0fIAw3U/85H5cf0R7HxtmHOACCiStFpf\n\t3b4HJvVc1akzFIDeXPWxSzcW2GJBsXEhFKz7QaHplyy1biWCcoB61RZOZ+hLfk8ISG\n\tUpCaTgxeXkHnIzlkZE83H7GYsGmDiBqyV2lE6nfGGe9nTp9uf2YQzGcmSABftEHG8I\n\tluzXdVZXYi8QQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1675550261;\n\tbh=C+XIT13+CEv4x8kRo9pZS5PW4aWH1sz2mF7tFGmUN+s=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=XtybHYf/0i1YvbWPap/6yLByrQ7WzW+wCvC6lWJlGgixQYP5yLtffvhNdUPoT91IZ\n\tIiDx3mV/WUzTNaqhk8tE3vvmjYvwcVFH+q2alqfMWAs1D8HUNsPixDzW6SRrVYD5M9\n\tlbtzPrTsFEogdXRZ8lUJIvtf7z8jEZu1NBuv1+ag="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"XtybHYf/\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230119084112.20564-6-dse@thaumatec.com>","References":"<20230119084112.20564-1-dse@thaumatec.com>\n\t<20230119084112.20564-6-dse@thaumatec.com>","To":"Daniel Semkowicz , libcamera-devel@lists.libcamera.org","Date":"Sat, 04 Feb 2023 22:37:38 +0000","Message-ID":"<167555025898.2778051.8149209482432418233@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v3 5/8] ipa: rkisp1: Add AF algorithm\n\tbasing on common AfHillClimbing class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]