[{"id":22417,"web_url":"https://patchwork.libcamera.org/comment/22417/","msgid":"<164807936828.1103026.919370156988060363@Monstersaurus>","date":"2022-03-23T23:49:28","subject":"Re: [libcamera-devel] [RFC PATCH v2 2/4] ipa: raspberrypi:\n\tIntroduce an autofocus algorithm","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jean-Michel Hautbois via libcamera-devel (2022-03-23 16:01:43)\n> Now that the ancillary links are plumbed and we can set the lens\n> position, implement a contrast-based algorithm for RPi. This algorithm\n> is adapted from the one proposed for the IPU3 IPA.\n> \n> It is currently taking all the regions and tries to make the focus on\n> the global scene in a first attempt.\n> \n> Signed-off-by: Jean-Michel Hautbois \n> ---\n> .../raspberrypi/controller/af_algorithm.hpp | 20 ++\n> src/ipa/raspberrypi/controller/af_status.h | 31 +++\n> src/ipa/raspberrypi/controller/focus_status.h | 3 +\n> src/ipa/raspberrypi/controller/iob/af.cpp | 231 ++++++++++++++++++\n> src/ipa/raspberrypi/controller/iob/af.h | 55 +++++\n> src/ipa/raspberrypi/meson.build | 1 +\n> 6 files changed, 341 insertions(+)\n> create mode 100644 src/ipa/raspberrypi/controller/af_algorithm.hpp\n> create mode 100644 src/ipa/raspberrypi/controller/af_status.h\n> create mode 100644 src/ipa/raspberrypi/controller/iob/af.cpp\n> create mode 100644 src/ipa/raspberrypi/controller/iob/af.h\n> \n> diff --git a/src/ipa/raspberrypi/controller/af_algorithm.hpp b/src/ipa/raspberrypi/controller/af_algorithm.hpp\n> new file mode 100644\n> index 00000000..553a37e1\n> --- /dev/null\n> +++ b/src/ipa/raspberrypi/controller/af_algorithm.hpp\n> @@ -0,0 +1,20 @@\n> +/* SPDX-License-Identifier: BSD-2-Clause */\n> +/*\n> + * Copyright (C) 2019, Raspberry Pi (Trading) Limited\n> + *\n> + * af_algorithm.hpp - autofocus control algorithm interface\n> + */\n> +#pragma once\n> +\n> +#include \"algorithm.hpp\"\n> +\n> +namespace RPiController {\n> +\n> +class AfAlgorithm : public Algorithm\n> +{\n> +public:\n> + AfAlgorithm(Controller *controller) : Algorithm(controller) {}\n> + // An af algorithm must provide the following:\n> +};\n> +\n> +} // namespace RPiController\n> diff --git a/src/ipa/raspberrypi/controller/af_status.h b/src/ipa/raspberrypi/controller/af_status.h\n> new file mode 100644\n> index 00000000..835e1e2f\n> --- /dev/null\n> +++ b/src/ipa/raspberrypi/controller/af_status.h\n> @@ -0,0 +1,31 @@\n> +/* SPDX-License-Identifier: BSD-2-Clause */\n> +/*\n> + * Copyright (C) 2020, Raspberry Pi (Trading) Limited\n> + * Copyright (C) 2022, Ideas On Board\n> + *\n> + * af_status.h - autofocus measurement status\n> + */\n> +#pragma once\n> +\n> +#include \n> +\n> +/*\n> + * The focus algorithm should post the following structure into the image's\n> + * \"af.status\" metadata.\n> + */\n> +\n> +#ifdef __cplusplus\n> +extern \"C\" {\n> +#endif\n\nI don't understand why this extern C is required?\n\n> +\n> +struct AfStatus {\n> + unsigned int num;\n> + uint32_t focus_measures[FOCUS_REGIONS];\n> + bool stable;\n> + uint32_t focus;\n> + double maxVariance;\n> +};\n> +\n> +#ifdef __cplusplus\n> +}\n> +#endif\n> diff --git a/src/ipa/raspberrypi/controller/focus_status.h b/src/ipa/raspberrypi/controller/focus_status.h\n> index ace2fe2c..8122df4b 100644\n> --- a/src/ipa/raspberrypi/controller/focus_status.h\n> +++ b/src/ipa/raspberrypi/controller/focus_status.h\n> @@ -19,6 +19,9 @@ extern \"C\" {\n> struct FocusStatus {\n> unsigned int num;\n> uint32_t focus_measures[FOCUS_REGIONS];\n> + bool stable;\n\nI assume there will be different states for this later, depending on\nwhat needs to get reported back in metadata, but that can be handled\nwhen the AF controls get plumbed.\n\n\n> + uint32_t focus;\n> + double maxVariance;\n> };\n> \n> #ifdef __cplusplus\n> diff --git a/src/ipa/raspberrypi/controller/iob/af.cpp b/src/ipa/raspberrypi/controller/iob/af.cpp\n> new file mode 100644\n> index 00000000..dc5258ba\n> --- /dev/null\n> +++ b/src/ipa/raspberrypi/controller/iob/af.cpp\n> @@ -0,0 +1,231 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Red Hat\n> + * Copyright (C) 2022, Ideas On Board\n> + *\n> + * af.cpp - automatic contrast-based focus algorithm\n> + */\n> +#include \n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \"af.h\"\n> +\n> +using namespace RPiController;\n> +using namespace libcamera;\n> +\n> +LOG_DEFINE_CATEGORY(IoBAf)\n> +\n> +#define NAME \"iob.af\"\n> +\n> +/*\n> + * Maximum focus steps of the VCM control\n> + * \\todo should be obtained from the VCM driver\n\nI think we're in a position where we should be doing this now.\n\n> + */\n> +static constexpr uint32_t kMaxFocusSteps = 1023;\n\npi@earth:~ $ v4l2-ctl -d /dev/v4l-subdev1 -l\n\nCamera Controls\n\n focus_absolute 0x009a090a (int) : min=0 max=4095 step=1 default=0 value=1017\n\nFrom an Arducam IMX519 with an AK7375.\n\n\n> +\n> +/* Minimum focus step for searching appropriate focus */\n> +static constexpr uint32_t kCoarseSearchStep = 30;\n> +static constexpr uint32_t kFineSearchStep = 1;\n\nThe course step is probably going to have to be relative to either the\nFineSearchStep or a division of the max by how long we want to take to\nsearch?\n\n\n> +\n> +/* Max ratio of variance change, 0.0 < kMaxChange < 1.0 */\n> +static constexpr double kMaxChange = 0.5;\n> +\n> +/* The numbers of frame to be ignored, before performing focus scan. */\n> +static constexpr uint32_t kIgnoreFrame = 10;\n\nWhy is this? Is that to let the AE/AGC settle?\n\nIs there already a means in the RPi IPA to 'know' when that has settled\n?\n\n\n> +\n> +/* Fine scan range 0 < kFineRange < 1 */\n> +static constexpr double kFineRange = 0.05;\n> +\n> +Af::Af(Controller *controller)\n> + : AfAlgorithm(controller), focus_(0), bestFocus_(0), ignoreCounter_(0),\n> + currentVariance_(0.0), previousVariance_(0.0), maxStep_(0),\n> + coarseCompleted_(false), fineCompleted_(false)\n> +{\n> +}\n> +\n> +char const *Af::Name() const\n> +{\n> + return NAME;\n> +}\n> +\n> +void Af::Initialise()\n> +{\n> + status_.focus = 0.0;\n> + status_.maxVariance = 0.0;\n> + status_.stable = false;\n> +}\n> +\n> +void Af::Prepare(Metadata *image_metadata)\n> +{\n> + image_metadata->Set(\"af.status\", status_);\n> +}\n> +\n> +double Af::estimateVariance()\n> +{\n> + unsigned int i;\n> + double mean;\n> + uint64_t total = 0;\n> + double var_sum = 0.0;\n> +\n> + /* Compute the mean value. */\n> + for (i = 0; i < FOCUS_REGIONS; i++)\n> + total += status_.focus_measures[i];\n> + mean = total / FOCUS_REGIONS;\n> +\n> + /* Compute the sum of the squared variance. */\n> + for (i = 0; i < FOCUS_REGIONS; i++)\n> + var_sum += std::pow(status_.focus_measures[i] - mean, 2);\n> +\n> + return var_sum / FOCUS_REGIONS;\n> +}\n> +\n> +bool Af::afNeedIgnoreFrame()\n> +{\n> + if (ignoreCounter_ == 0)\n> + return false;\n> + else\n> + ignoreCounter_--;\n> + return true;\n> +}\n> +\n> +void Af::afCoarseScan()\n> +{\n> + if (coarseCompleted_)\n> + return;\n> +\n> + if (afNeedIgnoreFrame())\n> + return;\n> +\n> + if (afScan(kCoarseSearchStep)) {\n> + coarseCompleted_ = true;\n> + status_.maxVariance = 0;\n> + focus_ = status_.focus - (status_.focus * kFineRange);\n> + status_.focus = focus_;\n> + previousVariance_ = 0;\n> + maxStep_ = std::clamp(focus_ + static_cast((focus_ * kFineRange)),\n> + 0U, kMaxFocusSteps);\n> + }\n> +}\n> +\n> +void Af::afFineScan()\n> +{\n> + if (!coarseCompleted_)\n> + return;\n> +\n> + if (afNeedIgnoreFrame())\n> + return;\n> +\n> + if (afScan(kFineSearchStep)) {\n> + status_.stable = true;\n> + fineCompleted_ = true;\n> + }\n> +}\n> +\n> +bool Af::afScan(uint32_t minSteps)\n> +{\n> + if (focus_ > maxStep_) {\n> + /* If the max step is reached, move lens to the position. */\n> + status_.focus = bestFocus_;\n> + return true;\n> + } else {\n> + /*\n> + * Find the maximum of the variance by estimating its\n> + * derivative. If the direction changes, it means we have passed\n> + * a maximum one step before.\n> + */\n> + if ((currentVariance_ - status_.maxVariance) >=\n> + -(status_.maxVariance * 0.1)) {\n> + /*\n> + * Positive and zero derivative:\n> + * The variance is still increasing. The focus could be\n> + * increased for the next comparison. Also, the max\n> + * variance and previous focus value are updated.\n> + */\n> + bestFocus_ = focus_;\n> + focus_ += minSteps;\n> + status_.focus = focus_;\n> + status_.maxVariance = currentVariance_;\n> + } else {\n> + /*\n> + * Negative derivative:\n> + * The variance starts to decrease which means the maximum\n> + * variance is found. Set focus step to previous good one\n> + * then return immediately.\n> + */\n> + status_.focus = bestFocus_;\n> + return true;\n> + }\n> + }\n> +\n> + previousVariance_ = currentVariance_;\n> + LOG(IoBAf, Debug) << \" Previous step is \"\n> + << bestFocus_\n> + << \" Current step is \"\n> + << focus_;\n> + return false;\n> +}\n\nThis 'hill climb' stops at the first hill. Is it ever possible or\nexpected that there could be more than one hill? (I.e. could there be\nanother bigger hill after a small peak?)\n\nI wonder if the over all algorithm would need to sweep, and store the\nvalues in a table - to then choose the best position - but that's quite\na rework from the current model - so it's just an open question for now,\nI suspect keeping this as is for now is enough to get started, but it's\nprobably a research topic.\n\n\n> +\n> +void Af::afReset()\n> +{\n> + if (afNeedIgnoreFrame())\n> + return;\n> +\n> + status_.maxVariance = 0;\n> + status_.focus = 0;\n> + focus_ = 0;\n> + status_.stable = false;\n> + ignoreCounter_ = kIgnoreFrame;\n> + previousVariance_ = 0.0;\n> + coarseCompleted_ = false;\n> + fineCompleted_ = false;\n> + maxStep_ = kMaxFocusSteps;\n> +}\n> +\n> +bool Af::afIsOutOfFocus()\n> +{\n> + const uint32_t diff_var = std::abs(currentVariance_ -\n> + status_.maxVariance);\n> + const double var_ratio = diff_var / status_.maxVariance;\n> + LOG(IoBAf, Debug) << \"Variance change rate: \"\n> + << var_ratio\n> + << \" Current VCM step: \"\n> + << status_.focus;\n> + if (var_ratio > kMaxChange)\n> + return true;\n> + else\n> + return false;\n> +}\n> +\n> +void Af::Process(StatisticsPtr &stats, Metadata *image_metadata)\n> +{\n> + unsigned int i;\n> + image_metadata->Get(\"af.status\", status_);\n> +\n> + /* Use the second filter results only, and cache those. */\n> + for (i = 0; i < FOCUS_REGIONS; i++)\n> + status_.focus_measures[i] = stats->focus_stats[i].contrast_val[1][1]\n> + / stats->focus_stats[i].contrast_val_num[1][1];\n> + status_.num = i;\n> +\n> + currentVariance_ = estimateVariance();\n> +\n> + if (!status_.stable) {\n> + afCoarseScan();\n> + afFineScan();\n> + } else {\n> + if (afIsOutOfFocus())\n> + afReset();\n> + else\n> + ignoreCounter_ = kIgnoreFrame;\n> + }\n> +}\n> +\n> +/* Register algorithm with the system. */\n> +static Algorithm *Create(Controller *controller)\n> +{\n> + return new Af(controller);\n> +}\n> +static RegisterAlgorithm reg(NAME, &Create);\n> diff --git a/src/ipa/raspberrypi/controller/iob/af.h b/src/ipa/raspberrypi/controller/iob/af.h\n> new file mode 100644\n> index 00000000..45c9711f\n> --- /dev/null\n> +++ b/src/ipa/raspberrypi/controller/iob/af.h\n> @@ -0,0 +1,55 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Red Hat\n> + * Copyright (C) 2022, Ideas On Board\n> + *\n> + * af.h - automatic contrast-based focus algorithm\n> + */\n> +#pragma once\n> +\n> +#include \"../af_algorithm.hpp\"\n> +#include \"../af_status.h\"\n> +#include \"../metadata.hpp\"\n> +\n> +namespace RPiController {\n> +\n> +class Af : public AfAlgorithm\n> +{\n> +public:\n> + Af(Controller *controller);\n> + char const *Name() const override;\n> + void Initialise() override;\n> + void Prepare(Metadata *image_metadata) override;\n> + void Process(StatisticsPtr &stats, Metadata *image_metadata) override;\n> +private:\n> + double estimateVariance();\n> + bool afNeedIgnoreFrame();\n> + void afCoarseScan();\n> + void afFineScan();\n> + bool afScan(uint32_t minSteps);\n> + void afReset();\n> + bool afIsOutOfFocus();\n> +\n> + AfStatus status_;\n> +\n> + /* VCM step configuration. It is the current setting of the VCM step. */\n> + uint32_t focus_;\n> + /* The best VCM step. It is a local optimum VCM step during scanning. */\n> + uint32_t bestFocus_;\n> +\n> + /* The frames ignored before starting measuring. */\n> + uint32_t ignoreCounter_;\n> +\n> + /* Current AF statistic variance. */\n> + double currentVariance_;\n> + /* It is used to determine the derivative during scanning */\n> + double previousVariance_;\n> + /* The designated maximum range of focus scanning. */\n> + uint32_t maxStep_;\n> + /* If the coarse scan completes, it is set to true. */\n> + bool coarseCompleted_;\n> + /* If the fine scan completes, it is set to true. */\n> + bool fineCompleted_;\n> +};\n> +\n> +} /* namespace RPiController */\n> diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build\n> index 32897e07..37068ecc 100644\n> --- a/src/ipa/raspberrypi/meson.build\n> +++ b/src/ipa/raspberrypi/meson.build\n> @@ -28,6 +28,7 @@ rpi_ipa_sources = files([\n> 'controller/controller.cpp',\n> 'controller/histogram.cpp',\n> 'controller/algorithm.cpp',\n> + 'controller/iob/af.cpp',\n> 'controller/rpi/alsc.cpp',\n> 'controller/rpi/awb.cpp',\n> 'controller/rpi/sharpen.cpp',\n> -- \n> 2.32.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 A68EBBD80A\n\tfor ;\n\tWed, 23 Mar 2022 23:49:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1904D604DA;\n\tThu, 24 Mar 2022 00:49:33 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4AE34604C5\n\tfor ;\n\tThu, 24 Mar 2022 00:49:31 +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 D5D9E12E9;\n\tThu, 24 Mar 2022 00:49:30 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648079373;\n\tbh=D6sQ55LMAkEj1q363s13zEqjESm/D6u0rXwJL6ZfuaY=;\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=J87Wy5udCEfa5tnsuTBQisFtutodMn4g9Vbg3Wujzqyht3i52iPmBbzIEhMUas2yN\n\tO22IzHzsnIUwz/HRpKLLOxe/EOdguS2yrpLounJAKmhzPwsCpU8lUSpeh64L+mqE6u\n\trZ0ZuxM/UXpxDNuKHBhS6Mm16pXHvxzmA5J3xCpa5M/qAc0r3epV555uMQfi73RXyn\n\tM524JkOAEoOtxidro2CVmkkGRaTkejU+aiz0GFySBJKrjSZaV5ce3h3YMI40vTK9Z5\n\txFeWCAQPunUYXUPlzdElgPjOHZBFuFwW8KaRth1kIuatLHo0EzlqFtvvn++9CcVI5d\n\tTk9qwmnCuNofQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1648079370;\n\tbh=D6sQ55LMAkEj1q363s13zEqjESm/D6u0rXwJL6ZfuaY=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=RRjIoUyF6ULAx6AUrg182Nxn3VhrK2dhW+pbxKVZMm1yIJiTl8Z+zjmzbJ3rj+0Y+\n\t+kkT+Ll9FkmLpvVVCt/ygO6u38dckHPNrKA+5A2S/VKDrJNVimzJYc5T9OdJ9kGa66\n\t501gNx8pX4SofHP4lex8qDXw7Lf4cpeoL64YRF28="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"RRjIoUyF\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220323160145.90606-3-jeanmichel.hautbois@ideasonboard.com>","References":"<20220323160145.90606-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20220323160145.90606-3-jeanmichel.hautbois@ideasonboard.com>","To":"Jean-Michel Hautbois ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 23 Mar 2022 23:49:28 +0000","Message-ID":"<164807936828.1103026.919370156988060363@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [RFC PATCH v2 2/4] ipa: raspberrypi:\n\tIntroduce an autofocus algorithm","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\" "}},{"id":22424,"web_url":"https://patchwork.libcamera.org/comment/22424/","msgid":"","date":"2022-03-24T09:54:21","subject":"Re: [libcamera-devel] [RFC PATCH v2 2/4] ipa: raspberrypi:\n\tIntroduce an autofocus algorithm","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Kieran,\n\nOn 24/03/2022 00:49, Kieran Bingham wrote:\n> Quoting Jean-Michel Hautbois via libcamera-devel (2022-03-23 16:01:43)\n>> Now that the ancillary links are plumbed and we can set the lens\n>> position, implement a contrast-based algorithm for RPi. This algorithm\n>> is adapted from the one proposed for the IPU3 IPA.\n>>\n>> It is currently taking all the regions and tries to make the focus on\n>> the global scene in a first attempt.\n>>\n>> Signed-off-by: Jean-Michel Hautbois \n>> ---\n>> .../raspberrypi/controller/af_algorithm.hpp | 20 ++\n>> src/ipa/raspberrypi/controller/af_status.h | 31 +++\n>> src/ipa/raspberrypi/controller/focus_status.h | 3 +\n>> src/ipa/raspberrypi/controller/iob/af.cpp | 231 ++++++++++++++++++\n>> src/ipa/raspberrypi/controller/iob/af.h | 55 +++++\n>> src/ipa/raspberrypi/meson.build | 1 +\n>> 6 files changed, 341 insertions(+)\n>> create mode 100644 src/ipa/raspberrypi/controller/af_algorithm.hpp\n>> create mode 100644 src/ipa/raspberrypi/controller/af_status.h\n>> create mode 100644 src/ipa/raspberrypi/controller/iob/af.cpp\n>> create mode 100644 src/ipa/raspberrypi/controller/iob/af.h\n>>\n>> diff --git a/src/ipa/raspberrypi/controller/af_algorithm.hpp b/src/ipa/raspberrypi/controller/af_algorithm.hpp\n>> new file mode 100644\n>> index 00000000..553a37e1\n>> --- /dev/null\n>> +++ b/src/ipa/raspberrypi/controller/af_algorithm.hpp\n>> @@ -0,0 +1,20 @@\n>> +/* SPDX-License-Identifier: BSD-2-Clause */\n>> +/*\n>> + * Copyright (C) 2019, Raspberry Pi (Trading) Limited\n>> + *\n>> + * af_algorithm.hpp - autofocus control algorithm interface\n>> + */\n>> +#pragma once\n>> +\n>> +#include \"algorithm.hpp\"\n>> +\n>> +namespace RPiController {\n>> +\n>> +class AfAlgorithm : public Algorithm\n>> +{\n>> +public:\n>> + AfAlgorithm(Controller *controller) : Algorithm(controller) {}\n>> + // An af algorithm must provide the following:\n>> +};\n>> +\n>> +} // namespace RPiController\n>> diff --git a/src/ipa/raspberrypi/controller/af_status.h b/src/ipa/raspberrypi/controller/af_status.h\n>> new file mode 100644\n>> index 00000000..835e1e2f\n>> --- /dev/null\n>> +++ b/src/ipa/raspberrypi/controller/af_status.h\n>> @@ -0,0 +1,31 @@\n>> +/* SPDX-License-Identifier: BSD-2-Clause */\n>> +/*\n>> + * Copyright (C) 2020, Raspberry Pi (Trading) Limited\n>> + * Copyright (C) 2022, Ideas On Board\n>> + *\n>> + * af_status.h - autofocus measurement status\n>> + */\n>> +#pragma once\n>> +\n>> +#include \n>> +\n>> +/*\n>> + * The focus algorithm should post the following structure into the image's\n>> + * \"af.status\" metadata.\n>> + */\n>> +\n>> +#ifdef __cplusplus\n>> +extern \"C\" {\n>> +#endif\n> \n> I don't understand why this extern C is required?\n\nTo be honest: it comes from a copy/paste of focus_state.hpp and I did \nnot think about it at all. All the _status files have this extern \"C\", I \nsuppose there is a reason, David ? Naushir ?\n\n> \n>> +\n>> +struct AfStatus {\n>> + unsigned int num;\n>> + uint32_t focus_measures[FOCUS_REGIONS];\n>> + bool stable;\n>> + uint32_t focus;\n>> + double maxVariance;\n>> +};\n>> +\n>> +#ifdef __cplusplus\n>> +}\n>> +#endif\n>> diff --git a/src/ipa/raspberrypi/controller/focus_status.h b/src/ipa/raspberrypi/controller/focus_status.h\n>> index ace2fe2c..8122df4b 100644\n>> --- a/src/ipa/raspberrypi/controller/focus_status.h\n>> +++ b/src/ipa/raspberrypi/controller/focus_status.h\n>> @@ -19,6 +19,9 @@ extern \"C\" {\n>> struct FocusStatus {\n>> unsigned int num;\n>> uint32_t focus_measures[FOCUS_REGIONS];\n>> + bool stable;\n> \n> I assume there will be different states for this later, depending on\n> what needs to get reported back in metadata, but that can be handled\n> when the AF controls get plumbed.\n\nIt is really to introduce it, I did not change the structure, as I think \nit should be done at the same time as the controls introduction.\n\n> \n> \n>> + uint32_t focus;\n>> + double maxVariance;\n>> };\n>> \n>> #ifdef __cplusplus\n>> diff --git a/src/ipa/raspberrypi/controller/iob/af.cpp b/src/ipa/raspberrypi/controller/iob/af.cpp\n>> new file mode 100644\n>> index 00000000..dc5258ba\n>> --- /dev/null\n>> +++ b/src/ipa/raspberrypi/controller/iob/af.cpp\n>> @@ -0,0 +1,231 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2021, Red Hat\n>> + * Copyright (C) 2022, Ideas On Board\n>> + *\n>> + * af.cpp - automatic contrast-based focus algorithm\n>> + */\n>> +#include \n>> +\n>> +#include \n>> +\n>> +#include \n>> +\n>> +#include \"af.h\"\n>> +\n>> +using namespace RPiController;\n>> +using namespace libcamera;\n>> +\n>> +LOG_DEFINE_CATEGORY(IoBAf)\n>> +\n>> +#define NAME \"iob.af\"\n>> +\n>> +/*\n>> + * Maximum focus steps of the VCM control\n>> + * \\todo should be obtained from the VCM driver\n> \n> I think we're in a position where we should be doing this now.\n\nYes :-).\n\n> \n>> + */\n>> +static constexpr uint32_t kMaxFocusSteps = 1023;\n> \n> pi@earth:~ $ v4l2-ctl -d /dev/v4l-subdev1 -l\n> \n> Camera Controls\n> \n> focus_absolute 0x009a090a (int) : min=0 max=4095 step=1 default=0 value=1017\n> \n> From an Arducam IMX519 with an AK7375.\n> \n> \n>> +\n>> +/* Minimum focus step for searching appropriate focus */\n>> +static constexpr uint32_t kCoarseSearchStep = 30;\n>> +static constexpr uint32_t kFineSearchStep = 1;\n> \n> The course step is probably going to have to be relative to either the\n> FineSearchStep or a division of the max by how long we want to take to\n> search?\n\nAgain, I think it should be done with the AF controls ?\n\n> \n> \n>> +\n>> +/* Max ratio of variance change, 0.0 < kMaxChange < 1.0 */\n>> +static constexpr double kMaxChange = 0.5;\n>> +\n>> +/* The numbers of frame to be ignored, before performing focus scan. */\n>> +static constexpr uint32_t kIgnoreFrame = 10;\n> \n> Why is this? Is that to let the AE/AGC settle?\n> \n> Is there already a means in the RPi IPA to 'know' when that has settled\n> ?\n\nIndeed, there is a mistrustCount_ which is used in \nIPARPI::signalStatReady() so I think there is not need to add another \ndrop here... :-).\n\n> \n> \n>> +\n>> +/* Fine scan range 0 < kFineRange < 1 */\n>> +static constexpr double kFineRange = 0.05;\n>> +\n>> +Af::Af(Controller *controller)\n>> + : AfAlgorithm(controller), focus_(0), bestFocus_(0), ignoreCounter_(0),\n>> + currentVariance_(0.0), previousVariance_(0.0), maxStep_(0),\n>> + coarseCompleted_(false), fineCompleted_(false)\n>> +{\n>> +}\n>> +\n>> +char const *Af::Name() const\n>> +{\n>> + return NAME;\n>> +}\n>> +\n>> +void Af::Initialise()\n>> +{\n>> + status_.focus = 0.0;\n>> + status_.maxVariance = 0.0;\n>> + status_.stable = false;\n>> +}\n>> +\n>> +void Af::Prepare(Metadata *image_metadata)\n>> +{\n>> + image_metadata->Set(\"af.status\", status_);\n>> +}\n>> +\n>> +double Af::estimateVariance()\n>> +{\n>> + unsigned int i;\n>> + double mean;\n>> + uint64_t total = 0;\n>> + double var_sum = 0.0;\n>> +\n>> + /* Compute the mean value. */\n>> + for (i = 0; i < FOCUS_REGIONS; i++)\n>> + total += status_.focus_measures[i];\n>> + mean = total / FOCUS_REGIONS;\n>> +\n>> + /* Compute the sum of the squared variance. */\n>> + for (i = 0; i < FOCUS_REGIONS; i++)\n>> + var_sum += std::pow(status_.focus_measures[i] - mean, 2);\n>> +\n>> + return var_sum / FOCUS_REGIONS;\n>> +}\n>> +\n>> +bool Af::afNeedIgnoreFrame()\n>> +{\n>> + if (ignoreCounter_ == 0)\n>> + return false;\n>> + else\n>> + ignoreCounter_--;\n>> + return true;\n>> +}\n>> +\n>> +void Af::afCoarseScan()\n>> +{\n>> + if (coarseCompleted_)\n>> + return;\n>> +\n>> + if (afNeedIgnoreFrame())\n>> + return;\n>> +\n>> + if (afScan(kCoarseSearchStep)) {\n>> + coarseCompleted_ = true;\n>> + status_.maxVariance = 0;\n>> + focus_ = status_.focus - (status_.focus * kFineRange);\n>> + status_.focus = focus_;\n>> + previousVariance_ = 0;\n>> + maxStep_ = std::clamp(focus_ + static_cast((focus_ * kFineRange)),\n>> + 0U, kMaxFocusSteps);\n>> + }\n>> +}\n>> +\n>> +void Af::afFineScan()\n>> +{\n>> + if (!coarseCompleted_)\n>> + return;\n>> +\n>> + if (afNeedIgnoreFrame())\n>> + return;\n>> +\n>> + if (afScan(kFineSearchStep)) {\n>> + status_.stable = true;\n>> + fineCompleted_ = true;\n>> + }\n>> +}\n>> +\n>> +bool Af::afScan(uint32_t minSteps)\n>> +{\n>> + if (focus_ > maxStep_) {\n>> + /* If the max step is reached, move lens to the position. */\n>> + status_.focus = bestFocus_;\n>> + return true;\n>> + } else {\n>> + /*\n>> + * Find the maximum of the variance by estimating its\n>> + * derivative. If the direction changes, it means we have passed\n>> + * a maximum one step before.\n>> + */\n>> + if ((currentVariance_ - status_.maxVariance) >=\n>> + -(status_.maxVariance * 0.1)) {\n>> + /*\n>> + * Positive and zero derivative:\n>> + * The variance is still increasing. The focus could be\n>> + * increased for the next comparison. Also, the max\n>> + * variance and previous focus value are updated.\n>> + */\n>> + bestFocus_ = focus_;\n>> + focus_ += minSteps;\n>> + status_.focus = focus_;\n>> + status_.maxVariance = currentVariance_;\n>> + } else {\n>> + /*\n>> + * Negative derivative:\n>> + * The variance starts to decrease which means the maximum\n>> + * variance is found. Set focus step to previous good one\n>> + * then return immediately.\n>> + */\n>> + status_.focus = bestFocus_;\n>> + return true;\n>> + }\n>> + }\n>> +\n>> + previousVariance_ = currentVariance_;\n>> + LOG(IoBAf, Debug) << \" Previous step is \"\n>> + << bestFocus_\n>> + << \" Current step is \"\n>> + << focus_;\n>> + return false;\n>> +}\n> \n> This 'hill climb' stops at the first hill. Is it ever possible or\n> expected that there could be more than one hill? (I.e. could there be\n> another bigger hill after a small peak?)\n\nTheretically we could have a local maximum lower than global maximum, yes.\n\n> \n> I wonder if the over all algorithm would need to sweep, and store the\n> values in a table - to then choose the best position - but that's quite\n> a rework from the current model - so it's just an open question for now,\n> I suspect keeping this as is for now is enough to get started, but it's\n> probably a research topic.\n\nConsidering a vcm with 4096 positions, and a step of 30, it would take \n~137 frames to sweep. So, something like 5 seconds at 30fps. We could \nalso say we want it to sweep for one second, and calculate \nkCoarseSearchStep based on this, but I suspect it could pass the maximum \nwithout noticing it...\n\n/*\n\nvariance\n ^\n | max max'\n | | | | | | | | |\n | | | | | | | | |\n | | | | | | | | |\n | | | | | | | | |\n | | | v | | | | |\n | | | xx | | | | |\n | | | xx x | xxxx| | | |\n | | |xx x|xx |xx | | |\n | | |x | | x | | |\n | | | | | x | | |\n | | x| | | xx | | xx|\n | | xx| | | x | | xx |\n | | x | | | x | | xx |\n | x|xxx | | | xx| | x |\n | xx| | | | x| |xx |\n | x | | | | |x |x |\n | xxx | | | | |xxxxxx| |\n |xxx | | | | | | |\n-+------+------+------+------+------+------+------+--> position\n | step\n */\n\nHere, we sampled a maximum (max') which is not the real one (max).\nI suppose we have room for improvements, it is a first algorithm.\n\n> \n>> +\n>> +void Af::afReset()\n>> +{\n>> + if (afNeedIgnoreFrame())\n>> + return;\n>> +\n>> + status_.maxVariance = 0;\n>> + status_.focus = 0;\n>> + focus_ = 0;\n>> + status_.stable = false;\n>> + ignoreCounter_ = kIgnoreFrame;\n>> + previousVariance_ = 0.0;\n>> + coarseCompleted_ = false;\n>> + fineCompleted_ = false;\n>> + maxStep_ = kMaxFocusSteps;\n>> +}\n>> +\n>> +bool Af::afIsOutOfFocus()\n>> +{\n>> + const uint32_t diff_var = std::abs(currentVariance_ -\n>> + status_.maxVariance);\n>> + const double var_ratio = diff_var / status_.maxVariance;\n>> + LOG(IoBAf, Debug) << \"Variance change rate: \"\n>> + << var_ratio\n>> + << \" Current VCM step: \"\n>> + << status_.focus;\n>> + if (var_ratio > kMaxChange)\n>> + return true;\n>> + else\n>> + return false;\n>> +}\n>> +\n>> +void Af::Process(StatisticsPtr &stats, Metadata *image_metadata)\n>> +{\n>> + unsigned int i;\n>> + image_metadata->Get(\"af.status\", status_);\n>> +\n>> + /* Use the second filter results only, and cache those. */\n>> + for (i = 0; i < FOCUS_REGIONS; i++)\n>> + status_.focus_measures[i] = stats->focus_stats[i].contrast_val[1][1]\n>> + / stats->focus_stats[i].contrast_val_num[1][1];\n>> + status_.num = i;\n>> +\n>> + currentVariance_ = estimateVariance();\n>> +\n>> + if (!status_.stable) {\n>> + afCoarseScan();\n>> + afFineScan();\n>> + } else {\n>> + if (afIsOutOfFocus())\n>> + afReset();\n>> + else\n>> + ignoreCounter_ = kIgnoreFrame;\n>> + }\n>> +}\n>> +\n>> +/* Register algorithm with the system. */\n>> +static Algorithm *Create(Controller *controller)\n>> +{\n>> + return new Af(controller);\n>> +}\n>> +static RegisterAlgorithm reg(NAME, &Create);\n>> diff --git a/src/ipa/raspberrypi/controller/iob/af.h b/src/ipa/raspberrypi/controller/iob/af.h\n>> new file mode 100644\n>> index 00000000..45c9711f\n>> --- /dev/null\n>> +++ b/src/ipa/raspberrypi/controller/iob/af.h\n>> @@ -0,0 +1,55 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2021, Red Hat\n>> + * Copyright (C) 2022, Ideas On Board\n>> + *\n>> + * af.h - automatic contrast-based focus algorithm\n>> + */\n>> +#pragma once\n>> +\n>> +#include \"../af_algorithm.hpp\"\n>> +#include \"../af_status.h\"\n>> +#include \"../metadata.hpp\"\n>> +\n>> +namespace RPiController {\n>> +\n>> +class Af : public AfAlgorithm\n>> +{\n>> +public:\n>> + Af(Controller *controller);\n>> + char const *Name() const override;\n>> + void Initialise() override;\n>> + void Prepare(Metadata *image_metadata) override;\n>> + void Process(StatisticsPtr &stats, Metadata *image_metadata) override;\n>> +private:\n>> + double estimateVariance();\n>> + bool afNeedIgnoreFrame();\n>> + void afCoarseScan();\n>> + void afFineScan();\n>> + bool afScan(uint32_t minSteps);\n>> + void afReset();\n>> + bool afIsOutOfFocus();\n>> +\n>> + AfStatus status_;\n>> +\n>> + /* VCM step configuration. It is the current setting of the VCM step. */\n>> + uint32_t focus_;\n>> + /* The best VCM step. It is a local optimum VCM step during scanning. */\n>> + uint32_t bestFocus_;\n>> +\n>> + /* The frames ignored before starting measuring. */\n>> + uint32_t ignoreCounter_;\n>> +\n>> + /* Current AF statistic variance. */\n>> + double currentVariance_;\n>> + /* It is used to determine the derivative during scanning */\n>> + double previousVariance_;\n>> + /* The designated maximum range of focus scanning. */\n>> + uint32_t maxStep_;\n>> + /* If the coarse scan completes, it is set to true. */\n>> + bool coarseCompleted_;\n>> + /* If the fine scan completes, it is set to true. */\n>> + bool fineCompleted_;\n>> +};\n>> +\n>> +} /* namespace RPiController */\n>> diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build\n>> index 32897e07..37068ecc 100644\n>> --- a/src/ipa/raspberrypi/meson.build\n>> +++ b/src/ipa/raspberrypi/meson.build\n>> @@ -28,6 +28,7 @@ rpi_ipa_sources = files([\n>> 'controller/controller.cpp',\n>> 'controller/histogram.cpp',\n>> 'controller/algorithm.cpp',\n>> + 'controller/iob/af.cpp',\n>> 'controller/rpi/alsc.cpp',\n>> 'controller/rpi/awb.cpp',\n>> 'controller/rpi/sharpen.cpp',\n>> -- \n>> 2.32.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 28E88BD80A\n\tfor ;\n\tThu, 24 Mar 2022 09:54:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 70FE5604D5;\n\tThu, 24 Mar 2022 10:54:26 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CB8E0601F5\n\tfor ;\n\tThu, 24 Mar 2022 10:54:24 +0100 (CET)","from [IPV6:2a01:e0a:169:7140:62c2:96e1:1c82:440d] (unknown\n\t[IPv6:2a01:e0a:169:7140:62c2:96e1:1c82:440d])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 57B8C1844;\n\tThu, 24 Mar 2022 10:54:24 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648115666;\n\tbh=StZ77M0EECcfld9/pSLvoJV90eN1w3ILzSSog1RFmuY=;\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:\n\tFrom;\n\tb=Lz7cxqT4jJfA8ilUJRS6ES9hqIbWAJkFltURNQm7zKcAvkafVJZgzgwIwaKaPRA47\n\t5PIu7ssz1bklZrsbcMhYyTYhhTpyRNlUQwenNUwQoGWuUcwirFi94nTB5YEHg5ehpi\n\tvCuA+QO4JiFinjDRpUvMha2BCinvGRw+O9m6wXLoQ7IVeWtRvpqXR8unazL62Ct9jY\n\tQ2cPOOmbJEQyN2HfDR/R3r9Q3JzP8qxSTu/JIOkM9UKeTcZPuqRr78VQDKd2aSrceK\n\tLiwIh8YnPyRHs0cMzqiE65etEKJVRLv+hnQyg50AnDCI+s8X6J6tRqXc9yuVCrfu+d\n\tlUag0yRu05BvA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1648115664;\n\tbh=StZ77M0EECcfld9/pSLvoJV90eN1w3ILzSSog1RFmuY=;\n\th=Date:Subject:To:References:Cc:From:In-Reply-To:From;\n\tb=IEm8WOoliJuS0PSARHme/bw8GVxYEjQHL2tQpUmQmo3p/JNiJkK1vY3GzGONYx940\n\tp0XDwQ1wovJqxDMMIwdMg3d3GsQRSFG8hKmp24c0EzcClzGii7kClrEvsesXx5J+Ad\n\tDLjCZTXgm0irg0bNI/WOEMaiLBw2GdAfknsc96DE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"IEm8WOol\"; dkim-atps=neutral","Message-ID":"","Date":"Thu, 24 Mar 2022 10:54:21 +0100","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.7.0","Content-Language":"en-US","To":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220323160145.90606-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20220323160145.90606-3-jeanmichel.hautbois@ideasonboard.com>\n\t<164807936828.1103026.919370156988060363@Monstersaurus>","In-Reply-To":"<164807936828.1103026.919370156988060363@Monstersaurus>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [RFC PATCH v2 2/4] ipa: raspberrypi:\n\tIntroduce an autofocus algorithm","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":"Jean-Michel Hautbois via libcamera-devel\n\t","Reply-To":"Jean-Michel Hautbois ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22426,"web_url":"https://patchwork.libcamera.org/comment/22426/","msgid":"","date":"2022-03-24T10:15:56","subject":"Re: [libcamera-devel] [RFC PATCH v2 2/4] ipa: raspberrypi:\n\tIntroduce an autofocus algorithm","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jean-Michel,\n\nThank you for your work.\n\nOn Wed, 23 Mar 2022 at 16:01, Jean-Michel Hautbois via libcamera-devel <\nlibcamera-devel@lists.libcamera.org> wrote:\n\n> Now that the ancillary links are plumbed and we can set the lens\n> position, implement a contrast-based algorithm for RPi. This algorithm\n> is adapted from the one proposed for the IPU3 IPA.\n>\n> It is currently taking all the regions and tries to make the focus on\n> the global scene in a first attempt.\n>\n> Signed-off-by: Jean-Michel Hautbois \n> ---\n> .../raspberrypi/controller/af_algorithm.hpp | 20 ++\n> src/ipa/raspberrypi/controller/af_status.h | 31 +++\n> src/ipa/raspberrypi/controller/focus_status.h | 3 +\n> src/ipa/raspberrypi/controller/iob/af.cpp | 231 ++++++++++++++++++\n> src/ipa/raspberrypi/controller/iob/af.h | 55 +++++\n> src/ipa/raspberrypi/meson.build | 1 +\n> 6 files changed, 341 insertions(+)\n> create mode 100644 src/ipa/raspberrypi/controller/af_algorithm.hpp\n> create mode 100644 src/ipa/raspberrypi/controller/af_status.h\n> create mode 100644 src/ipa/raspberrypi/controller/iob/af.cpp\n> create mode 100644 src/ipa/raspberrypi/controller/iob/af.h\n>\n> diff --git a/src/ipa/raspberrypi/controller/af_algorithm.hpp\n> b/src/ipa/raspberrypi/controller/af_algorithm.hpp\n> new file mode 100644\n> index 00000000..553a37e1\n> --- /dev/null\n> +++ b/src/ipa/raspberrypi/controller/af_algorithm.hpp\n> @@ -0,0 +1,20 @@\n> +/* SPDX-License-Identifier: BSD-2-Clause */\n> +/*\n> + * Copyright (C) 2019, Raspberry Pi (Trading) Limited\n>\n\n2022 :-)\n\n\n> + *\n> + * af_algorithm.hpp - autofocus control algorithm interface\n> + */\n> +#pragma once\n> +\n> +#include \"algorithm.hpp\"\n> +\n> +namespace RPiController {\n> +\n> +class AfAlgorithm : public Algorithm\n> +{\n> +public:\n> + AfAlgorithm(Controller *controller) : Algorithm(controller) {}\n> + // An af algorithm must provide the following:\n> +};\n> +\n> +} // namespace RPiController\n> diff --git a/src/ipa/raspberrypi/controller/af_status.h\n> b/src/ipa/raspberrypi/controller/af_status.h\n> new file mode 100644\n> index 00000000..835e1e2f\n> --- /dev/null\n> +++ b/src/ipa/raspberrypi/controller/af_status.h\n> @@ -0,0 +1,31 @@\n> +/* SPDX-License-Identifier: BSD-2-Clause */\n> +/*\n> + * Copyright (C) 2020, Raspberry Pi (Trading) Limited\n>\n\n2022\n\n\n> + * Copyright (C) 2022, Ideas On Board\n> + *\n> + * af_status.h - autofocus measurement status\n> + */\n> +#pragma once\n> +\n> +#include \n>\n\nThis header should not be included here.\n\n\n> +\n> +/*\n> + * The focus algorithm should post the following structure into the\n> image's\n> + * \"af.status\" metadata.\n> + */\n> +\n> +#ifdef __cplusplus\n> +extern \"C\" {\n> +#endif\n>\n\nYou can remove the extern \"C\", this is a throwback from the time this file\nwould be\nlinked with a C library.\n\n\n> +\n> +struct AfStatus {\n> + unsigned int num;\n> + uint32_t focus_measures[FOCUS_REGIONS];\n> + bool stable;\n> + uint32_t focus;\n> + double maxVariance;\n> +};\n>\n\nI wonder if all these are actually needed in AfStatus? The idea of the\nController metadata is to provide the IPA with all that it needs to set the\nlens control.\nmaxVariance is probably not needed by the IPA, and num/focus_measure is\npassed out through FocusStatus, see below.\n\nThe bool stable is ok for now, but I expect it needs to change to an enum\nshowing focussing states very soon.\n\n\n> +\n> +#ifdef __cplusplus\n> +}\n> +#endif\n> diff --git a/src/ipa/raspberrypi/controller/focus_status.h\n> b/src/ipa/raspberrypi/controller/focus_status.h\n> index ace2fe2c..8122df4b 100644\n> --- a/src/ipa/raspberrypi/controller/focus_status.h\n> +++ b/src/ipa/raspberrypi/controller/focus_status.h\n> @@ -19,6 +19,9 @@ extern \"C\" {\n> struct FocusStatus {\n> unsigned int num;\n> uint32_t focus_measures[FOCUS_REGIONS];\n> + bool stable;\n> + uint32_t focus;\n> + double maxVariance;\n>\n\nI don't think we want to include these additions in FocusStatus. This\nstruct is only used to report the focus FoM values back to the application.\nFor focus control, you can use the AfStatus struct above.\n\n\n> };\n>\n> #ifdef __cplusplus\n> diff --git a/src/ipa/raspberrypi/controller/iob/af.cpp\n> b/src/ipa/raspberrypi/controller/iob/af.cpp\n> new file mode 100644\n> index 00000000..dc5258ba\n> --- /dev/null\n> +++ b/src/ipa/raspberrypi/controller/iob/af.cpp\n> @@ -0,0 +1,231 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Red Hat\n> + * Copyright (C) 2022, Ideas On Board\n> + *\n> + * af.cpp - automatic contrast-based focus algorithm\n> + */\n> +#include \n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \"af.h\"\n> +\n> +using namespace RPiController;\n> +using namespace libcamera;\n> +\n> +LOG_DEFINE_CATEGORY(IoBAf)\n> +\n> +#define NAME \"iob.af\"\n> +\n> +/*\n> + * Maximum focus steps of the VCM control\n> + * \\todo should be obtained from the VCM driver\n> + */\n> +static constexpr uint32_t kMaxFocusSteps = 1023;\n> +\n> +/* Minimum focus step for searching appropriate focus */\n> +static constexpr uint32_t kCoarseSearchStep = 30;\n> +static constexpr uint32_t kFineSearchStep = 1;\n> +\n> +/* Max ratio of variance change, 0.0 < kMaxChange < 1.0 */\n> +static constexpr double kMaxChange = 0.5;\n> +\n> +/* The numbers of frame to be ignored, before performing focus scan. */\n> +static constexpr uint32_t kIgnoreFrame = 10;\n> +\n> +/* Fine scan range 0 < kFineRange < 1 */\n> +static constexpr double kFineRange = 0.05;\n> +\n> +Af::Af(Controller *controller)\n> + : AfAlgorithm(controller), focus_(0), bestFocus_(0),\n> ignoreCounter_(0),\n> + currentVariance_(0.0), previousVariance_(0.0), maxStep_(0),\n> + coarseCompleted_(false), fineCompleted_(false)\n> +{\n> +}\n> +\n> +char const *Af::Name() const\n> +{\n> + return NAME;\n> +}\n> +\n> +void Af::Initialise()\n> +{\n> + status_.focus = 0.0;\n> + status_.maxVariance = 0.0;\n> + status_.stable = false;\n> +}\n> +\n> +void Af::Prepare(Metadata *image_metadata)\n> +{\n> + image_metadata->Set(\"af.status\", status_);\n> +}\n> +\n> +double Af::estimateVariance()\n> +{\n> + unsigned int i;\n> + double mean;\n> + uint64_t total = 0;\n> + double var_sum = 0.0;\n> +\n> + /* Compute the mean value. */\n> + for (i = 0; i < FOCUS_REGIONS; i++)\n> + total += status_.focus_measures[i];\n> + mean = total / FOCUS_REGIONS;\n> +\n> + /* Compute the sum of the squared variance. */\n> + for (i = 0; i < FOCUS_REGIONS; i++)\n> + var_sum += std::pow(status_.focus_measures[i] - mean, 2);\n> +\n> + return var_sum / FOCUS_REGIONS;\n> +}\n> +\n> +bool Af::afNeedIgnoreFrame()\n> +{\n> + if (ignoreCounter_ == 0)\n> + return false;\n> + else\n> + ignoreCounter_--;\n> + return true;\n> +}\n> +\n> +void Af::afCoarseScan()\n> +{\n> + if (coarseCompleted_)\n> + return;\n> +\n> + if (afNeedIgnoreFrame())\n> + return;\n> +\n> + if (afScan(kCoarseSearchStep)) {\n> + coarseCompleted_ = true;\n> + status_.maxVariance = 0;\n> + focus_ = status_.focus - (status_.focus * kFineRange);\n> + status_.focus = focus_;\n> + previousVariance_ = 0;\n> + maxStep_ = std::clamp(focus_ +\n> static_cast((focus_ * kFineRange)),\n> + 0U, kMaxFocusSteps);\n> + }\n> +}\n> +\n> +void Af::afFineScan()\n> +{\n> + if (!coarseCompleted_)\n> + return;\n> +\n> + if (afNeedIgnoreFrame())\n> + return;\n> +\n> + if (afScan(kFineSearchStep)) {\n> + status_.stable = true;\n> + fineCompleted_ = true;\n> + }\n> +}\n> +\n> +bool Af::afScan(uint32_t minSteps)\n> +{\n> + if (focus_ > maxStep_) {\n> + /* If the max step is reached, move lens to the position.\n> */\n> + status_.focus = bestFocus_;\n> + return true;\n> + } else {\n> + /*\n> + * Find the maximum of the variance by estimating its\n> + * derivative. If the direction changes, it means we have\n> passed\n> + * a maximum one step before.\n> + */\n> + if ((currentVariance_ - status_.maxVariance) >=\n> + -(status_.maxVariance * 0.1)) {\n> + /*\n> + * Positive and zero derivative:\n> + * The variance is still increasing. The focus\n> could be\n> + * increased for the next comparison. Also, the max\n> + * variance and previous focus value are updated.\n> + */\n> + bestFocus_ = focus_;\n> + focus_ += minSteps;\n> + status_.focus = focus_;\n> + status_.maxVariance = currentVariance_;\n> + } else {\n> + /*\n> + * Negative derivative:\n> + * The variance starts to decrease which means the\n> maximum\n> + * variance is found. Set focus step to previous\n> good one\n> + * then return immediately.\n> + */\n> + status_.focus = bestFocus_;\n> + return true;\n> + }\n> + }\n> +\n> + previousVariance_ = currentVariance_;\n> + LOG(IoBAf, Debug) << \" Previous step is \"\n> + << bestFocus_\n> + << \" Current step is \"\n> + << focus_;\n> + return false;\n> +}\n> +\n> +void Af::afReset()\n> +{\n> + if (afNeedIgnoreFrame())\n> + return;\n> +\n> + status_.maxVariance = 0;\n> + status_.focus = 0;\n> + focus_ = 0;\n> + status_.stable = false;\n> + ignoreCounter_ = kIgnoreFrame;\n> + previousVariance_ = 0.0;\n> + coarseCompleted_ = false;\n> + fineCompleted_ = false;\n> + maxStep_ = kMaxFocusSteps;\n> +}\n> +\n> +bool Af::afIsOutOfFocus()\n> +{\n> + const uint32_t diff_var = std::abs(currentVariance_ -\n> + status_.maxVariance);\n> + const double var_ratio = diff_var / status_.maxVariance;\n> + LOG(IoBAf, Debug) << \"Variance change rate: \"\n> + << var_ratio\n> + << \" Current VCM step: \"\n> + << status_.focus;\n> + if (var_ratio > kMaxChange)\n> + return true;\n> + else\n> + return false;\n> +}\n> +\n> +void Af::Process(StatisticsPtr &stats, Metadata *image_metadata)\n> +{\n> + unsigned int i;\n> + image_metadata->Get(\"af.status\", status_);\n>\n\nI'm a bit unsure of this one. Perhaps I don't understand the code well\nenough, but do you need to do this? \"af.status\" gets set in Af::Prepare()\nfrom status_, then here we set status_ from \"af.status\". Nothing will have\nchanged status_ between the two calls, so maybe this is unnecessary?\n\n\n> +\n> + /* Use the second filter results only, and cache those. */\n> + for (i = 0; i < FOCUS_REGIONS; i++)\n> + status_.focus_measures[i] =\n> stats->focus_stats[i].contrast_val[1][1]\n> + /\n> stats->focus_stats[i].contrast_val_num[1][1];\n> + status_.num = i;\n> +\n> + currentVariance_ = estimateVariance();\n> +\n> + if (!status_.stable) {\n> + afCoarseScan();\n> + afFineScan();\n> + } else {\n> + if (afIsOutOfFocus())\n> + afReset();\n> + else\n> + ignoreCounter_ = kIgnoreFrame;\n> + }\n> +}\n> +\n> +/* Register algorithm with the system. */\n> +static Algorithm *Create(Controller *controller)\n> +{\n> + return new Af(controller);\n> +}\n> +static RegisterAlgorithm reg(NAME, &Create);\n> diff --git a/src/ipa/raspberrypi/controller/iob/af.h\n> b/src/ipa/raspberrypi/controller/iob/af.h\n> new file mode 100644\n> index 00000000..45c9711f\n> --- /dev/null\n> +++ b/src/ipa/raspberrypi/controller/iob/af.h\n> @@ -0,0 +1,55 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Red Hat\n> + * Copyright (C) 2022, Ideas On Board\n> + *\n> + * af.h - automatic contrast-based focus algorithm\n> + */\n> +#pragma once\n> +\n> +#include \"../af_algorithm.hpp\"\n> +#include \"../af_status.h\"\n> +#include \"../metadata.hpp\"\n> +\n> +namespace RPiController {\n> +\n> +class Af : public AfAlgorithm\n>\n\nIt may be practical to put class Af in a separate (iob?) namespace to avoid\nsymbol clashes. Once RPi have an algorithm implementation, we will have to\nenclose our class Af in a rpi namespace or equivalent. What do folks think?\n\nRegards,\nNaush\n\n\n> +{\n> +public:\n> + Af(Controller *controller);\n> + char const *Name() const override;\n> + void Initialise() override;\n> + void Prepare(Metadata *image_metadata) override;\n> + void Process(StatisticsPtr &stats, Metadata *image_metadata)\n> override;\n> +private:\n> + double estimateVariance();\n> + bool afNeedIgnoreFrame();\n> + void afCoarseScan();\n> + void afFineScan();\n> + bool afScan(uint32_t minSteps);\n> + void afReset();\n> + bool afIsOutOfFocus();\n> +\n> + AfStatus status_;\n> +\n> + /* VCM step configuration. It is the current setting of the VCM\n> step. */\n> + uint32_t focus_;\n> + /* The best VCM step. It is a local optimum VCM step during\n> scanning. */\n> + uint32_t bestFocus_;\n> +\n> + /* The frames ignored before starting measuring. */\n> + uint32_t ignoreCounter_;\n> +\n> + /* Current AF statistic variance. */\n> + double currentVariance_;\n> + /* It is used to determine the derivative during scanning */\n> + double previousVariance_;\n> + /* The designated maximum range of focus scanning. */\n> + uint32_t maxStep_;\n> + /* If the coarse scan completes, it is set to true. */\n> + bool coarseCompleted_;\n> + /* If the fine scan completes, it is set to true. */\n> + bool fineCompleted_;\n> +};\n> +\n> +} /* namespace RPiController */\n> diff --git a/src/ipa/raspberrypi/meson.build\n> b/src/ipa/raspberrypi/meson.build\n> index 32897e07..37068ecc 100644\n> --- a/src/ipa/raspberrypi/meson.build\n> +++ b/src/ipa/raspberrypi/meson.build\n> @@ -28,6 +28,7 @@ rpi_ipa_sources = files([\n> 'controller/controller.cpp',\n> 'controller/histogram.cpp',\n> 'controller/algorithm.cpp',\n> + 'controller/iob/af.cpp',\n> 'controller/rpi/alsc.cpp',\n> 'controller/rpi/awb.cpp',\n> 'controller/rpi/sharpen.cpp',\n> --\n> 2.32.0\n>\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 5D03FBD80A\n\tfor ;\n\tThu, 24 Mar 2022 10:16:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9B6F2604DB;\n\tThu, 24 Mar 2022 11:16:15 +0100 (CET)","from mail-lf1-x133.google.com (mail-lf1-x133.google.com\n\t[IPv6:2a00:1450:4864:20::133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B0508601F5\n\tfor ;\n\tThu, 24 Mar 2022 11:16:14 +0100 (CET)","by mail-lf1-x133.google.com with SMTP id 5so7219631lfp.1\n\tfor ;\n\tThu, 24 Mar 2022 03:16:14 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648116975;\n\tbh=xuqr+2zTl7bXafsHfsEjeczBsKJhkC/Uk9+kPwSUND0=;\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=j/5HrS/nr9yRbZ+Tevb0UH6vCwES2NAWRi4dFe33v/J/g8eqjoZmtguL2njam2ZRJ\n\t4kDcJkw11ooaTV5pv8C4biAKH3BymH7z+5A5Tg9LlvuIWdtwIbM1d9VVrlGD/HmSyc\n\tSAxX8C6zFadee1AULs9YZ+jPc6cgnemwjsDL559lLMvfU5nxZqkVp95AR9TvtRzrJa\n\tin6CQxCkNlmVrvWWLXxuNZq9DaG9J9v/ogwE9e/Ufm6RSVGPJCo5e1gzvLxmvYAnEU\n\tJOkgYVDfCoXO9DrCyoOKTNjHsjcHgp00GZMmgjMJ+kWhcje1y0+gtMJ1x29a9nulix\n\tDljFp8sHVEc5Q==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=CKk9T2RC/dTV3JVSYU8G+gawV8CxQ44zK6razvPQiDs=;\n\tb=iIQoNaLKd15L/tWL0zuO5CNLvE3c65n1tN2es9zQQIVsoy42BucYyjV9pa1l0h5Ghl\n\tCHGB6BNTePdC9/WJgEa0Jyzea51Io5VeGGa1auamg4VZidBJmZhz62tdm2kBaFkx+9Lj\n\tLpgQqYuNBJsKAD5NVlIIFiWwoLDV6fkax24pFP/wX/YCg2Xmp9Ie8BdsNgUSqbalALCB\n\tPFfWesjBzCh2CabaqEBeM0TBKKN3gtAsvYWx7h3dI3CQldtaeCRob/+tw2gf96/o2Saj\n\twQYSF0zjfVXR+zEZ3YD2PfgGWg0R5x4D0vneJi7a1rK8MSUu6acpmFJ5vwZOnRrzICi8\n\tRE8g=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"iIQoNaLK\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=CKk9T2RC/dTV3JVSYU8G+gawV8CxQ44zK6razvPQiDs=;\n\tb=l9J4Q9f6REnHHup+V4fVf5WNvhVYzoQZcqcksUQdtVgNqO9tzEmBXF1Jja8uKPcf2D\n\t5z+3Ahp7IgXkkvPyq32hGxz9JGNAj7ALwI7aUJ+UEk250rl/iac5f7M810APewqLaGoy\n\tv6YVmWkKvbFas7+K6wHPCmFNm6D3MZmk7raz4Ll8Of4BwTBU5hfZojtVLqfV85YXrTZm\n\tzNBRKOXNlwIdhkfxRL20cFg6Hz41wucFB+LknIYpxzroOMOhXtpU7oclktU2NaRCMIua\n\tzzciTd2MbzZT6q09rD2Eq42iOGLQ4jeJbZ9g/fDv5c2NonstDaGVqj+QU28d7kIaG+JB\n\tpWuw==","X-Gm-Message-State":"AOAM533YJfYAFuyQT6k7zdNIWwS9XM4WevWTKSNfrMKi5kS7i9B9rrwx\n\tR2nujeL2sFLIkmU5y2qOT9znYRNMzW1XmgQtyP0+9hHPFGsUNQ==","X-Google-Smtp-Source":"ABdhPJyDwCV0m4LwbAnkEb/BOUR6jpgHje7XKXaTAaZ2Ke+/a9QVXZqtGE7apu7itIML1mUQx6Awh45GBqCxRc5v6o0=","X-Received":"by 2002:ac2:4203:0:b0:448:8053:d402 with SMTP id\n\ty3-20020ac24203000000b004488053d402mr3199539lfh.687.1648116972733;\n\tThu, 24 Mar 2022 03:16:12 -0700 (PDT)","MIME-Version":"1.0","References":"<20220323160145.90606-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20220323160145.90606-3-jeanmichel.hautbois@ideasonboard.com>","In-Reply-To":"<20220323160145.90606-3-jeanmichel.hautbois@ideasonboard.com>","Date":"Thu, 24 Mar 2022 10:15:56 +0000","Message-ID":"","To":"Jean-Michel Hautbois ","Content-Type":"multipart/alternative; boundary=\"000000000000b55a8705daf422f7\"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 2/4] ipa: raspberrypi:\n\tIntroduce an autofocus algorithm","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":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22450,"web_url":"https://patchwork.libcamera.org/comment/22450/","msgid":"","date":"2022-03-25T11:15:21","subject":"Re: [libcamera-devel] [RFC PATCH v2 2/4] ipa: raspberrypi:\n\tIntroduce an autofocus algorithm","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Jean-Michel\n\nThanks for this patch, great to see progress on the AF question!\n\nOn Wed, 23 Mar 2022 at 16:01, Jean-Michel Hautbois via libcamera-devel\n wrote:\n>\n> Now that the ancillary links are plumbed and we can set the lens\n> position, implement a contrast-based algorithm for RPi. This algorithm\n> is adapted from the one proposed for the IPU3 IPA.\n>\n> It is currently taking all the regions and tries to make the focus on\n> the global scene in a first attempt.\n>\n> Signed-off-by: Jean-Michel Hautbois \n> ---\n> .../raspberrypi/controller/af_algorithm.hpp | 20 ++\n> src/ipa/raspberrypi/controller/af_status.h | 31 +++\n> src/ipa/raspberrypi/controller/focus_status.h | 3 +\n> src/ipa/raspberrypi/controller/iob/af.cpp | 231 ++++++++++++++++++\n> src/ipa/raspberrypi/controller/iob/af.h | 55 +++++\n> src/ipa/raspberrypi/meson.build | 1 +\n> 6 files changed, 341 insertions(+)\n> create mode 100644 src/ipa/raspberrypi/controller/af_algorithm.hpp\n> create mode 100644 src/ipa/raspberrypi/controller/af_status.h\n> create mode 100644 src/ipa/raspberrypi/controller/iob/af.cpp\n> create mode 100644 src/ipa/raspberrypi/controller/iob/af.h\n>\n> diff --git a/src/ipa/raspberrypi/controller/af_algorithm.hpp b/src/ipa/raspberrypi/controller/af_algorithm.hpp\n> new file mode 100644\n> index 00000000..553a37e1\n> --- /dev/null\n> +++ b/src/ipa/raspberrypi/controller/af_algorithm.hpp\n> @@ -0,0 +1,20 @@\n> +/* SPDX-License-Identifier: BSD-2-Clause */\n> +/*\n> + * Copyright (C) 2019, Raspberry Pi (Trading) Limited\n> + *\n> + * af_algorithm.hpp - autofocus control algorithm interface\n> + */\n> +#pragma once\n> +\n> +#include \"algorithm.hpp\"\n> +\n> +namespace RPiController {\n> +\n> +class AfAlgorithm : public Algorithm\n> +{\n> +public:\n> + AfAlgorithm(Controller *controller) : Algorithm(controller) {}\n> + // An af algorithm must provide the following:\n\nYes, we have a bit of a chicken-or-egg thing going on here at the\nmoment, don't we? Should we try and make this look a bit more like an\ninterface that implements the AF Controls proposal (even though that's\nnot agreed yet). Or do we commit this first on the understanding that\nwe'll come back and patch it all up later? I don't think I really\nmind, I guess it would just be good to be clear on the plan!\n\nFor the record, the kinds of methods we'd want in here would include\nthings like:\n\nSetMode(AfMode mode); // set the AF mode (manual, auto, continuous)\nTrigger(); // start a cycle (in auto mode)\nCancel(); // cancel a cycle (in auto mode)\nSetWindows(...); // set AF windows\nSetRange(....) ; // set AF range\netc.\n\n> +};\n> +\n> +} // namespace RPiController\n> diff --git a/src/ipa/raspberrypi/controller/af_status.h b/src/ipa/raspberrypi/controller/af_status.h\n> new file mode 100644\n> index 00000000..835e1e2f\n> --- /dev/null\n> +++ b/src/ipa/raspberrypi/controller/af_status.h\n> @@ -0,0 +1,31 @@\n> +/* SPDX-License-Identifier: BSD-2-Clause */\n> +/*\n> + * Copyright (C) 2020, Raspberry Pi (Trading) Limited\n> + * Copyright (C) 2022, Ideas On Board\n> + *\n> + * af_status.h - autofocus measurement status\n> + */\n> +#pragma once\n> +\n> +#include \n> +\n> +/*\n> + * The focus algorithm should post the following structure into the image's\n> + * \"af.status\" metadata.\n> + */\n> +\n> +#ifdef __cplusplus\n> +extern \"C\" {\n> +#endif\n> +\n> +struct AfStatus {\n> + unsigned int num;\n> + uint32_t focus_measures[FOCUS_REGIONS];\n> + bool stable;\n> + uint32_t focus;\n> + double maxVariance;\n> +};\n\nExactly what to include here is an interesting question. The basic\nprinciples are that we should include:\n\n* Anything the pipeline handler needs in order to control hardware or\nother parts of the system. In this case that would probably mean we\npass out \"the next lens position to ask for\".\n\n* Anything that we need to fill in the libcamera metadata. So this\nwould include the AfState (which is mostly just\nscanning/focused/failed).\n\n* We also like to pass out any settings that have been applied. This\nis so that you know for sure whether the algorithm has seen the\nsettings you asked for. In this case it might include the mode, the AF\nwindows, the range, the speed etc. We do tend to do this even when\nthere's no libcamera metadata for the information - we take the view\nthat it's a thing we might need to consider adding at some point in\nthe future.\n\n* Anything else that might reasonably be thought to be \"useful\". This\nis obviously where things become more debatable!\n\nThen the final principle would be that we mostly try to exclude other\nthings because (a) they might be algorithm specific and (b) it just\nmakes for more stuff to get wrong unless you really need them.\n\nLooking specifically at the above:\n\nfocus_measures: I'm not convinced that these are necessary here,\nparticularly when they're in the focus algorithm metadata already.\n\nstable: I guess this is a focused/not-focused kind of flag? Maybe a\nscanning/focused/failed enum would be better, and look a bit more like\na future \"full\" AF algorithm?\n\nfocus: is that the next lens position to ask for?\n\nmaxVariance: that's feeling a bit implementation specific to me. The\nquestion is always: what might the pipeline handler do with this?\n\n> +\n> +#ifdef __cplusplus\n> +}\n> +#endif\n> diff --git a/src/ipa/raspberrypi/controller/focus_status.h b/src/ipa/raspberrypi/controller/focus_status.h\n> index ace2fe2c..8122df4b 100644\n> --- a/src/ipa/raspberrypi/controller/focus_status.h\n> +++ b/src/ipa/raspberrypi/controller/focus_status.h\n> @@ -19,6 +19,9 @@ extern \"C\" {\n> struct FocusStatus {\n> unsigned int num;\n> uint32_t focus_measures[FOCUS_REGIONS];\n> + bool stable;\n> + uint32_t focus;\n> + double maxVariance;\n\nI'm guessing these should have been removed again when the AfStatus was added?\n\n> };\n>\n> #ifdef __cplusplus\n> diff --git a/src/ipa/raspberrypi/controller/iob/af.cpp b/src/ipa/raspberrypi/controller/iob/af.cpp\n> new file mode 100644\n> index 00000000..dc5258ba\n> --- /dev/null\n> +++ b/src/ipa/raspberrypi/controller/iob/af.cpp\n> @@ -0,0 +1,231 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Red Hat\n> + * Copyright (C) 2022, Ideas On Board\n> + *\n> + * af.cpp - automatic contrast-based focus algorithm\n> + */\n> +#include \n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \"af.h\"\n> +\n> +using namespace RPiController;\n> +using namespace libcamera;\n> +\n> +LOG_DEFINE_CATEGORY(IoBAf)\n> +\n> +#define NAME \"iob.af\"\n> +\n> +/*\n> + * Maximum focus steps of the VCM control\n> + * \\todo should be obtained from the VCM driver\n> + */\n> +static constexpr uint32_t kMaxFocusSteps = 1023;\n> +\n> +/* Minimum focus step for searching appropriate focus */\n> +static constexpr uint32_t kCoarseSearchStep = 30;\n> +static constexpr uint32_t kFineSearchStep = 1;\n> +\n> +/* Max ratio of variance change, 0.0 < kMaxChange < 1.0 */\n> +static constexpr double kMaxChange = 0.5;\n> +\n> +/* The numbers of frame to be ignored, before performing focus scan. */\n> +static constexpr uint32_t kIgnoreFrame = 10;\n> +\n> +/* Fine scan range 0 < kFineRange < 1 */\n> +static constexpr double kFineRange = 0.05;\n> +\n> +Af::Af(Controller *controller)\n> + : AfAlgorithm(controller), focus_(0), bestFocus_(0), ignoreCounter_(0),\n> + currentVariance_(0.0), previousVariance_(0.0), maxStep_(0),\n> + coarseCompleted_(false), fineCompleted_(false)\n> +{\n> +}\n> +\n> +char const *Af::Name() const\n> +{\n> + return NAME;\n> +}\n> +\n> +void Af::Initialise()\n> +{\n> + status_.focus = 0.0;\n> + status_.maxVariance = 0.0;\n> + status_.stable = false;\n> +}\n> +\n> +void Af::Prepare(Metadata *image_metadata)\n> +{\n> + image_metadata->Set(\"af.status\", status_);\n> +}\n> +\n> +double Af::estimateVariance()\n> +{\n> + unsigned int i;\n> + double mean;\n> + uint64_t total = 0;\n> + double var_sum = 0.0;\n> +\n> + /* Compute the mean value. */\n> + for (i = 0; i < FOCUS_REGIONS; i++)\n> + total += status_.focus_measures[i];\n> + mean = total / FOCUS_REGIONS;\n> +\n> + /* Compute the sum of the squared variance. */\n> + for (i = 0; i < FOCUS_REGIONS; i++)\n> + var_sum += std::pow(status_.focus_measures[i] - mean, 2);\n> +\n> + return var_sum / FOCUS_REGIONS;\n> +}\n\nI'm interested in the choice of variance across the regions of the\nper-pixel FoM for measuring the \"goodness\" of focus, rather than just\nthe per-pixel FoM. But maybe this kind of question can be left until\nthe algorithm is running and then we can examine the focus measure\ncurves and see what works best.\n\n> +\n> +bool Af::afNeedIgnoreFrame()\n> +{\n> + if (ignoreCounter_ == 0)\n> + return false;\n> + else\n> + ignoreCounter_--;\n> + return true;\n> +}\n> +\n> +void Af::afCoarseScan()\n> +{\n> + if (coarseCompleted_)\n> + return;\n> +\n> + if (afNeedIgnoreFrame())\n> + return;\n> +\n> + if (afScan(kCoarseSearchStep)) {\n> + coarseCompleted_ = true;\n> + status_.maxVariance = 0;\n> + focus_ = status_.focus - (status_.focus * kFineRange);\n> + status_.focus = focus_;\n> + previousVariance_ = 0;\n> + maxStep_ = std::clamp(focus_ + static_cast((focus_ * kFineRange)),\n> + 0U, kMaxFocusSteps);\n> + }\n> +}\n> +\n> +void Af::afFineScan()\n> +{\n> + if (!coarseCompleted_)\n> + return;\n> +\n> + if (afNeedIgnoreFrame())\n> + return;\n> +\n> + if (afScan(kFineSearchStep)) {\n> + status_.stable = true;\n> + fineCompleted_ = true;\n> + }\n> +}\n> +\n> +bool Af::afScan(uint32_t minSteps)\n> +{\n> + if (focus_ > maxStep_) {\n> + /* If the max step is reached, move lens to the position. */\n> + status_.focus = bestFocus_;\n> + return true;\n> + } else {\n> + /*\n> + * Find the maximum of the variance by estimating its\n> + * derivative. If the direction changes, it means we have passed\n> + * a maximum one step before.\n> + */\n> + if ((currentVariance_ - status_.maxVariance) >=\n> + -(status_.maxVariance * 0.1)) {\n> + /*\n> + * Positive and zero derivative:\n> + * The variance is still increasing. The focus could be\n> + * increased for the next comparison. Also, the max\n> + * variance and previous focus value are updated.\n> + */\n> + bestFocus_ = focus_;\n> + focus_ += minSteps;\n> + status_.focus = focus_;\n> + status_.maxVariance = currentVariance_;\n> + } else {\n> + /*\n> + * Negative derivative:\n> + * The variance starts to decrease which means the maximum\n> + * variance is found. Set focus step to previous good one\n> + * then return immediately.\n> + */\n> + status_.focus = bestFocus_;\n> + return true;\n> + }\n> + }\n> +\n> + previousVariance_ = currentVariance_;\n> + LOG(IoBAf, Debug) << \" Previous step is \"\n> + << bestFocus_\n> + << \" Current step is \"\n> + << focus_;\n> + return false;\n> +}\n> +\n> +void Af::afReset()\n> +{\n> + if (afNeedIgnoreFrame())\n> + return;\n> +\n> + status_.maxVariance = 0;\n> + status_.focus = 0;\n> + focus_ = 0;\n> + status_.stable = false;\n> + ignoreCounter_ = kIgnoreFrame;\n> + previousVariance_ = 0.0;\n> + coarseCompleted_ = false;\n> + fineCompleted_ = false;\n> + maxStep_ = kMaxFocusSteps;\n> +}\n> +\n> +bool Af::afIsOutOfFocus()\n> +{\n> + const uint32_t diff_var = std::abs(currentVariance_ -\n> + status_.maxVariance);\n> + const double var_ratio = diff_var / status_.maxVariance;\n> + LOG(IoBAf, Debug) << \"Variance change rate: \"\n> + << var_ratio\n> + << \" Current VCM step: \"\n> + << status_.focus;\n> + if (var_ratio > kMaxChange)\n> + return true;\n> + else\n> + return false;\n> +}\n> +\n> +void Af::Process(StatisticsPtr &stats, Metadata *image_metadata)\n> +{\n> + unsigned int i;\n> + image_metadata->Get(\"af.status\", status_);\n> +\n> + /* Use the second filter results only, and cache those. */\n> + for (i = 0; i < FOCUS_REGIONS; i++)\n> + status_.focus_measures[i] = stats->focus_stats[i].contrast_val[1][1]\n> + / stats->focus_stats[i].contrast_val_num[1][1];\n> + status_.num = i;\n> +\n> + currentVariance_ = estimateVariance();\n> +\n> + if (!status_.stable) {\n> + afCoarseScan();\n> + afFineScan();\n> + } else {\n> + if (afIsOutOfFocus())\n> + afReset();\n> + else\n> + ignoreCounter_ = kIgnoreFrame;\n> + }\n> +}\n> +\n> +/* Register algorithm with the system. */\n> +static Algorithm *Create(Controller *controller)\n> +{\n> + return new Af(controller);\n> +}\n> +static RegisterAlgorithm reg(NAME, &Create);\n> diff --git a/src/ipa/raspberrypi/controller/iob/af.h b/src/ipa/raspberrypi/controller/iob/af.h\n> new file mode 100644\n> index 00000000..45c9711f\n> --- /dev/null\n> +++ b/src/ipa/raspberrypi/controller/iob/af.h\n> @@ -0,0 +1,55 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Red Hat\n> + * Copyright (C) 2022, Ideas On Board\n> + *\n> + * af.h - automatic contrast-based focus algorithm\n> + */\n> +#pragma once\n> +\n> +#include \"../af_algorithm.hpp\"\n> +#include \"../af_status.h\"\n> +#include \"../metadata.hpp\"\n> +\n> +namespace RPiController {\n\nI think the namespace thing has been commented on elsewhere.\n\n> +\n> +class Af : public AfAlgorithm\n> +{\n> +public:\n> + Af(Controller *controller);\n> + char const *Name() const override;\n> + void Initialise() override;\n> + void Prepare(Metadata *image_metadata) override;\n> + void Process(StatisticsPtr &stats, Metadata *image_metadata) override;\n> +private:\n> + double estimateVariance();\n> + bool afNeedIgnoreFrame();\n> + void afCoarseScan();\n> + void afFineScan();\n> + bool afScan(uint32_t minSteps);\n> + void afReset();\n> + bool afIsOutOfFocus();\n> +\n> + AfStatus status_;\n> +\n> + /* VCM step configuration. It is the current setting of the VCM step. */\n> + uint32_t focus_;\n> + /* The best VCM step. It is a local optimum VCM step during scanning. */\n> + uint32_t bestFocus_;\n> +\n> + /* The frames ignored before starting measuring. */\n> + uint32_t ignoreCounter_;\n> +\n> + /* Current AF statistic variance. */\n> + double currentVariance_;\n> + /* It is used to determine the derivative during scanning */\n> + double previousVariance_;\n> + /* The designated maximum range of focus scanning. */\n> + uint32_t maxStep_;\n> + /* If the coarse scan completes, it is set to true. */\n> + bool coarseCompleted_;\n> + /* If the fine scan completes, it is set to true. */\n> + bool fineCompleted_;\n> +};\n> +\n> +} /* namespace RPiController */\n> diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build\n> index 32897e07..37068ecc 100644\n> --- a/src/ipa/raspberrypi/meson.build\n> +++ b/src/ipa/raspberrypi/meson.build\n> @@ -28,6 +28,7 @@ rpi_ipa_sources = files([\n> 'controller/controller.cpp',\n> 'controller/histogram.cpp',\n> 'controller/algorithm.cpp',\n> + 'controller/iob/af.cpp',\n> 'controller/rpi/alsc.cpp',\n> 'controller/rpi/awb.cpp',\n> 'controller/rpi/sharpen.cpp',\n> --\n> 2.32.0\n>\n\nOne other thing I'd like to understand better is how an application\nknows what lens positions to ask for in \"manual mode\". The most common\nuse case, I think, would be to set the lens to \"hyperfocal position\"\nwhen the system starts, or after capturing an image following an AF\ncycle.\n\nHow would we know what lens position corresponds to hyperfocal? Would\nit be the default value of the V4L2 focus position control? The catch\nhere is that the lens actuator cannot know - in fact it can't even\nknow the valid usable range for any given type of module.\n\nDoes anyone have any thoughts on this?\n\nThanks again for all this work!\n\nBest regards\nDavid","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 E1C18C0F1B\n\tfor ;\n\tFri, 25 Mar 2022 11:15:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4B7DB604D4;\n\tFri, 25 Mar 2022 12:15:34 +0100 (CET)","from mail-wm1-x330.google.com (mail-wm1-x330.google.com\n\t[IPv6:2a00:1450:4864:20::330])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 02350601F8\n\tfor ;\n\tFri, 25 Mar 2022 12:15:32 +0100 (CET)","by mail-wm1-x330.google.com with SMTP id\n\tl9-20020a05600c4f0900b0038ccd1b8642so2474862wmq.0\n\tfor ;\n\tFri, 25 Mar 2022 04:15:32 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648206934;\n\tbh=cQlUxj4ibrElVZXBJufIKyZ9R+nYV4JC+OznzskE40I=;\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=i4rIP8axh9u9TjupSSG6stISSLlAdOKv+ndca0xiIye+dI3ecqX8BTjNoUzhir2LK\n\tlNTOG7mphgsZAbNJGGKr+CbIWqdZkrPldhHQJkLobB4WR7NwMU/FIceelX/JNRIEBk\n\t7fBWe+F0tx/82s0YvYB7eFCuqFumSlOx/kpwllIOEVS+vIaOeZ1Q2bo0m3pws10gdX\n\t7SGWR3bcVyw+JhZeK76lPlNGnIVZiwasEvAEMlbjVarWI794dkslG9xKqVCRBBfJlo\n\tH/265EdXY0S/6JILtnVdlNXVRFOIyBr4nuHqled9T7jJYxeSJkERD1PyZ34Cpu2gTv\n\t2hJr/IIeZgnmw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=XiUIOl+ZyjzbHmeCtQQPmkmvHwggJGB1UDKZm/t38l0=;\n\tb=NeAikY+7ESSMWN4aV/mBAS+T1gAzrMgTRY61wpNyjgwsAsTfGgo7pVT87Ck2dyxLjB\n\tKvwyupRWhcfuOFXtbzCkZXdm2Y0Fgt5euUNRW1tughlQpiJ9mLMzexS/gRYK9KZmAWz6\n\tmW/PYZCp80Y7lGNUUhIAKZFvqf38N9cNnBJVBYnnUDPEcWvMv4T7c+sZvniVVLqPXq0W\n\tOBXVRRxIfjs4nkmlNGm3xvR+7GUXfVPlaG1yDPAz9huaFo3GQbQTrI5OGSq9MK7FC/q3\n\tIzb7AHIaI12rs0F/iA0aRG/qsfDRqUmUR7iUomCgnCOoZz2A+M2SBIzCjF9/0LY6mTyx\n\tnOKA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"NeAikY+7\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=XiUIOl+ZyjzbHmeCtQQPmkmvHwggJGB1UDKZm/t38l0=;\n\tb=d3SiWrz9DerTOkbr0liWmaNUs9xcgZY9LADw7ibaYuopJrRXg9bNYtuY9rSOAT65xu\n\tK6qciDmIoAPuUZ2IPkZ1Xs4y9oHz0E1gGlg99zr4T4FkHBl1Px65lRhXKQk6A/vkA2/Z\n\tHoVAQKfKupirCpW6pTUGYylg0J4CWbTaM1DbNqHrSPKN8LabUShvchyn/+ZoOysCzK7U\n\t/kSbYXFsVfkWKGToizgZ5GailPVH1pO+GNQzG92JXQ07cZ9kmg9rhtxNDyhhidkdgQ9P\n\tJeQKAnMTUbGtgNO0hq7ycNlhyNmIdKDJKZeLHYQYsg/NoE2qIcTFjdv7Kk9GIwGM780Y\n\tQCkQ==","X-Gm-Message-State":"AOAM530Jy4IRJ0GDy4Vo9FUFlD0ceXih1QMAlLZhijsDCOWX2YZSijPi\n\ty4IVhzRHqkFsZcm3+dVxmkINPWT767nloPLrYUIlhWBopkk=","X-Google-Smtp-Source":"ABdhPJzrbxzVaF9V9E+8sJtp2fS3xk8i33L4qMcjzbzMFHxV3fH/xs0yDEuVpXkZlbQF2fJoeTKy8BAo+kgKC12oxVw=","X-Received":"by 2002:a7b:c452:0:b0:38c:9146:569 with SMTP id\n\tl18-20020a7bc452000000b0038c91460569mr18336988wmi.201.1648206932406;\n\tFri, 25 Mar 2022 04:15:32 -0700 (PDT)","MIME-Version":"1.0","References":"<20220323160145.90606-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20220323160145.90606-3-jeanmichel.hautbois@ideasonboard.com>","In-Reply-To":"<20220323160145.90606-3-jeanmichel.hautbois@ideasonboard.com>","Date":"Fri, 25 Mar 2022 11:15:21 +0000","Message-ID":"","To":"Jean-Michel Hautbois ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 2/4] ipa: raspberrypi:\n\tIntroduce an autofocus algorithm","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":"David Plowman via libcamera-devel ","Reply-To":"David Plowman ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22467,"web_url":"https://patchwork.libcamera.org/comment/22467/","msgid":"","date":"2022-03-27T21:35:12","subject":"Re: [libcamera-devel] [RFC PATCH v2 2/4] ipa: raspberrypi:\n\tIntroduce an autofocus algorithm","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Mar 24, 2022 at 10:54:21AM +0100, Jean-Michel Hautbois via libcamera-devel wrote:\n> On 24/03/2022 00:49, Kieran Bingham wrote:\n> > Quoting Jean-Michel Hautbois via libcamera-devel (2022-03-23 16:01:43)\n> >> Now that the ancillary links are plumbed and we can set the lens\n> >> position, implement a contrast-based algorithm for RPi. This algorithm\n> >> is adapted from the one proposed for the IPU3 IPA.\n> >>\n> >> It is currently taking all the regions and tries to make the focus on\n> >> the global scene in a first attempt.\n> >>\n> >> Signed-off-by: Jean-Michel Hautbois \n> >> ---\n> >> .../raspberrypi/controller/af_algorithm.hpp | 20 ++\n> >> src/ipa/raspberrypi/controller/af_status.h | 31 +++\n> >> src/ipa/raspberrypi/controller/focus_status.h | 3 +\n> >> src/ipa/raspberrypi/controller/iob/af.cpp | 231 ++++++++++++++++++\n> >> src/ipa/raspberrypi/controller/iob/af.h | 55 +++++\n> >> src/ipa/raspberrypi/meson.build | 1 +\n> >> 6 files changed, 341 insertions(+)\n> >> create mode 100644 src/ipa/raspberrypi/controller/af_algorithm.hpp\n> >> create mode 100644 src/ipa/raspberrypi/controller/af_status.h\n> >> create mode 100644 src/ipa/raspberrypi/controller/iob/af.cpp\n> >> create mode 100644 src/ipa/raspberrypi/controller/iob/af.h\n> >>\n> >> diff --git a/src/ipa/raspberrypi/controller/af_algorithm.hpp b/src/ipa/raspberrypi/controller/af_algorithm.hpp\n> >> new file mode 100644\n> >> index 00000000..553a37e1\n> >> --- /dev/null\n> >> +++ b/src/ipa/raspberrypi/controller/af_algorithm.hpp\n> >> @@ -0,0 +1,20 @@\n> >> +/* SPDX-License-Identifier: BSD-2-Clause */\n> >> +/*\n> >> + * Copyright (C) 2019, Raspberry Pi (Trading) Limited\n> >> + *\n> >> + * af_algorithm.hpp - autofocus control algorithm interface\n> >> + */\n> >> +#pragma once\n> >> +\n> >> +#include \"algorithm.hpp\"\n> >> +\n> >> +namespace RPiController {\n> >> +\n> >> +class AfAlgorithm : public Algorithm\n> >> +{\n> >> +public:\n> >> + AfAlgorithm(Controller *controller) : Algorithm(controller) {}\n> >> + // An af algorithm must provide the following:\n> >> +};\n> >> +\n> >> +} // namespace RPiController\n> >> diff --git a/src/ipa/raspberrypi/controller/af_status.h b/src/ipa/raspberrypi/controller/af_status.h\n> >> new file mode 100644\n> >> index 00000000..835e1e2f\n> >> --- /dev/null\n> >> +++ b/src/ipa/raspberrypi/controller/af_status.h\n> >> @@ -0,0 +1,31 @@\n> >> +/* SPDX-License-Identifier: BSD-2-Clause */\n> >> +/*\n> >> + * Copyright (C) 2020, Raspberry Pi (Trading) Limited\n> >> + * Copyright (C) 2022, Ideas On Board\n> >> + *\n> >> + * af_status.h - autofocus measurement status\n> >> + */\n> >> +#pragma once\n> >> +\n> >> +#include \n> >> +\n> >> +/*\n> >> + * The focus algorithm should post the following structure into the image's\n> >> + * \"af.status\" metadata.\n> >> + */\n> >> +\n> >> +#ifdef __cplusplus\n> >> +extern \"C\" {\n> >> +#endif\n> > \n> > I don't understand why this extern C is required?\n> \n> To be honest: it comes from a copy/paste of focus_state.hpp and I did \n> not think about it at all. All the _status files have this extern \"C\", I \n> suppose there is a reason, David ? Naushir ?\n> \n> >> +\n> >> +struct AfStatus {\n> >> + unsigned int num;\n> >> + uint32_t focus_measures[FOCUS_REGIONS];\n> >> + bool stable;\n> >> + uint32_t focus;\n> >> + double maxVariance;\n> >> +};\n> >> +\n> >> +#ifdef __cplusplus\n> >> +}\n> >> +#endif\n> >> diff --git a/src/ipa/raspberrypi/controller/focus_status.h b/src/ipa/raspberrypi/controller/focus_status.h\n> >> index ace2fe2c..8122df4b 100644\n> >> --- a/src/ipa/raspberrypi/controller/focus_status.h\n> >> +++ b/src/ipa/raspberrypi/controller/focus_status.h\n> >> @@ -19,6 +19,9 @@ extern \"C\" {\n> >> struct FocusStatus {\n> >> unsigned int num;\n> >> uint32_t focus_measures[FOCUS_REGIONS];\n> >> + bool stable;\n> > \n> > I assume there will be different states for this later, depending on\n> > what needs to get reported back in metadata, but that can be handled\n> > when the AF controls get plumbed.\n> \n> It is really to introduce it, I did not change the structure, as I think \n> it should be done at the same time as the controls introduction.\n> \n> >> + uint32_t focus;\n> >> + double maxVariance;\n> >> };\n> >> \n> >> #ifdef __cplusplus\n> >> diff --git a/src/ipa/raspberrypi/controller/iob/af.cpp b/src/ipa/raspberrypi/controller/iob/af.cpp\n> >> new file mode 100644\n> >> index 00000000..dc5258ba\n> >> --- /dev/null\n> >> +++ b/src/ipa/raspberrypi/controller/iob/af.cpp\n> >> @@ -0,0 +1,231 @@\n> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) 2021, Red Hat\n> >> + * Copyright (C) 2022, Ideas On Board\n> >> + *\n> >> + * af.cpp - automatic contrast-based focus algorithm\n> >> + */\n> >> +#include \n> >> +\n> >> +#include \n> >> +\n> >> +#include \n> >> +\n> >> +#include \"af.h\"\n> >> +\n> >> +using namespace RPiController;\n> >> +using namespace libcamera;\n> >> +\n> >> +LOG_DEFINE_CATEGORY(IoBAf)\n> >> +\n> >> +#define NAME \"iob.af\"\n> >> +\n> >> +/*\n> >> + * Maximum focus steps of the VCM control\n> >> + * \\todo should be obtained from the VCM driver\n> > \n> > I think we're in a position where we should be doing this now.\n> \n> Yes :-).\n> \n> >> + */\n> >> +static constexpr uint32_t kMaxFocusSteps = 1023;\n> > \n> > pi@earth:~ $ v4l2-ctl -d /dev/v4l-subdev1 -l\n> > \n> > Camera Controls\n> > \n> > focus_absolute 0x009a090a (int) : min=0 max=4095 step=1 default=0 value=1017\n> > \n> > From an Arducam IMX519 with an AK7375.\n\nNote that the value gives the absolute maximum supported by the VCM\ncontroller driver. It most likely won't match the maximum value that a\nparticular camera module should use. This should come from the tuning\ndata.\n\n> >> +\n> >> +/* Minimum focus step for searching appropriate focus */\n> >> +static constexpr uint32_t kCoarseSearchStep = 30;\n> >> +static constexpr uint32_t kFineSearchStep = 1;\n> > \n> > The course step is probably going to have to be relative to either the\n> > FineSearchStep or a division of the max by how long we want to take to\n> > search?\n> \n> Again, I think it should be done with the AF controls ?\n\nNot sure to follow you here.\n\n> >> +\n> >> +/* Max ratio of variance change, 0.0 < kMaxChange < 1.0 */\n> >> +static constexpr double kMaxChange = 0.5;\n> >> +\n> >> +/* The numbers of frame to be ignored, before performing focus scan. */\n> >> +static constexpr uint32_t kIgnoreFrame = 10;\n> > \n> > Why is this? Is that to let the AE/AGC settle?\n> > \n> > Is there already a means in the RPi IPA to 'know' when that has settled\n> > ?\n> \n> Indeed, there is a mistrustCount_ which is used in \n> IPARPI::signalStatReady() so I think there is not need to add another \n> drop here... :-).\n> \n> >> +\n> >> +/* Fine scan range 0 < kFineRange < 1 */\n> >> +static constexpr double kFineRange = 0.05;\n> >> +\n> >> +Af::Af(Controller *controller)\n> >> + : AfAlgorithm(controller), focus_(0), bestFocus_(0), ignoreCounter_(0),\n> >> + currentVariance_(0.0), previousVariance_(0.0), maxStep_(0),\n> >> + coarseCompleted_(false), fineCompleted_(false)\n> >> +{\n> >> +}\n> >> +\n> >> +char const *Af::Name() const\n> >> +{\n> >> + return NAME;\n> >> +}\n> >> +\n> >> +void Af::Initialise()\n> >> +{\n> >> + status_.focus = 0.0;\n> >> + status_.maxVariance = 0.0;\n> >> + status_.stable = false;\n> >> +}\n> >> +\n> >> +void Af::Prepare(Metadata *image_metadata)\n> >> +{\n> >> + image_metadata->Set(\"af.status\", status_);\n> >> +}\n> >> +\n> >> +double Af::estimateVariance()\n> >> +{\n> >> + unsigned int i;\n> >> + double mean;\n> >> + uint64_t total = 0;\n> >> + double var_sum = 0.0;\n> >> +\n> >> + /* Compute the mean value. */\n> >> + for (i = 0; i < FOCUS_REGIONS; i++)\n> >> + total += status_.focus_measures[i];\n> >> + mean = total / FOCUS_REGIONS;\n> >> +\n> >> + /* Compute the sum of the squared variance. */\n> >> + for (i = 0; i < FOCUS_REGIONS; i++)\n> >> + var_sum += std::pow(status_.focus_measures[i] - mean, 2);\n> >> +\n> >> + return var_sum / FOCUS_REGIONS;\n> >> +}\n> >> +\n> >> +bool Af::afNeedIgnoreFrame()\n> >> +{\n> >> + if (ignoreCounter_ == 0)\n> >> + return false;\n> >> + else\n> >> + ignoreCounter_--;\n> >> + return true;\n> >> +}\n> >> +\n> >> +void Af::afCoarseScan()\n> >> +{\n> >> + if (coarseCompleted_)\n> >> + return;\n> >> +\n> >> + if (afNeedIgnoreFrame())\n> >> + return;\n> >> +\n> >> + if (afScan(kCoarseSearchStep)) {\n> >> + coarseCompleted_ = true;\n> >> + status_.maxVariance = 0;\n> >> + focus_ = status_.focus - (status_.focus * kFineRange);\n> >> + status_.focus = focus_;\n> >> + previousVariance_ = 0;\n> >> + maxStep_ = std::clamp(focus_ + static_cast((focus_ * kFineRange)),\n> >> + 0U, kMaxFocusSteps);\n> >> + }\n> >> +}\n> >> +\n> >> +void Af::afFineScan()\n> >> +{\n> >> + if (!coarseCompleted_)\n> >> + return;\n> >> +\n> >> + if (afNeedIgnoreFrame())\n> >> + return;\n> >> +\n> >> + if (afScan(kFineSearchStep)) {\n> >> + status_.stable = true;\n> >> + fineCompleted_ = true;\n> >> + }\n> >> +}\n> >> +\n> >> +bool Af::afScan(uint32_t minSteps)\n> >> +{\n> >> + if (focus_ > maxStep_) {\n> >> + /* If the max step is reached, move lens to the position. */\n> >> + status_.focus = bestFocus_;\n> >> + return true;\n> >> + } else {\n> >> + /*\n> >> + * Find the maximum of the variance by estimating its\n> >> + * derivative. If the direction changes, it means we have passed\n> >> + * a maximum one step before.\n> >> + */\n> >> + if ((currentVariance_ - status_.maxVariance) >=\n> >> + -(status_.maxVariance * 0.1)) {\n> >> + /*\n> >> + * Positive and zero derivative:\n> >> + * The variance is still increasing. The focus could be\n> >> + * increased for the next comparison. Also, the max\n> >> + * variance and previous focus value are updated.\n> >> + */\n> >> + bestFocus_ = focus_;\n> >> + focus_ += minSteps;\n> >> + status_.focus = focus_;\n> >> + status_.maxVariance = currentVariance_;\n> >> + } else {\n> >> + /*\n> >> + * Negative derivative:\n> >> + * The variance starts to decrease which means the maximum\n> >> + * variance is found. Set focus step to previous good one\n> >> + * then return immediately.\n> >> + */\n> >> + status_.focus = bestFocus_;\n> >> + return true;\n> >> + }\n> >> + }\n> >> +\n> >> + previousVariance_ = currentVariance_;\n> >> + LOG(IoBAf, Debug) << \" Previous step is \"\n> >> + << bestFocus_\n> >> + << \" Current step is \"\n> >> + << focus_;\n> >> + return false;\n> >> +}\n> > \n> > This 'hill climb' stops at the first hill. Is it ever possible or\n> > expected that there could be more than one hill? (I.e. could there be\n> > another bigger hill after a small peak?)\n> \n> Theretically we could have a local maximum lower than global maximum, yes.\n> \n> > I wonder if the over all algorithm would need to sweep, and store the\n> > values in a table - to then choose the best position - but that's quite\n> > a rework from the current model - so it's just an open question for now,\n> > I suspect keeping this as is for now is enough to get started, but it's\n> > probably a research topic.\n> \n> Considering a vcm with 4096 positions, and a step of 30, it would take \n> ~137 frames to sweep. So, something like 5 seconds at 30fps. We could \n> also say we want it to sweep for one second, and calculate \n> kCoarseSearchStep based on this, but I suspect it could pass the maximum \n> without noticing it...\n> \n> /*\n> \n> variance\n> ^\n> | max max'\n> | | | | | | | | |\n> | | | | | | | | |\n> | | | | | | | | |\n> | | | | | | | | |\n> | | | v | | | | |\n> | | | xx | | | | |\n> | | | xx x | xxxx| | | |\n> | | |xx x|xx |xx | | |\n> | | |x | | x | | |\n> | | | | | x | | |\n> | | x| | | xx | | xx|\n> | | xx| | | x | | xx |\n> | | x | | | x | | xx |\n> | x|xxx | | | xx| | x |\n> | xx| | | | x| |xx |\n> | x | | | | |x |x |\n> | xxx | | | | |xxxxxx| |\n> |xxx | | | | | | |\n> -+------+------+------+------+------+------+------+--> position\n> | step\n> */\n> \n> Here, we sampled a maximum (max') which is not the real one (max).\n> I suppose we have room for improvements, it is a first algorithm.\n> \n> >> +\n> >> +void Af::afReset()\n> >> +{\n> >> + if (afNeedIgnoreFrame())\n> >> + return;\n> >> +\n> >> + status_.maxVariance = 0;\n> >> + status_.focus = 0;\n> >> + focus_ = 0;\n> >> + status_.stable = false;\n> >> + ignoreCounter_ = kIgnoreFrame;\n> >> + previousVariance_ = 0.0;\n> >> + coarseCompleted_ = false;\n> >> + fineCompleted_ = false;\n> >> + maxStep_ = kMaxFocusSteps;\n> >> +}\n> >> +\n> >> +bool Af::afIsOutOfFocus()\n> >> +{\n> >> + const uint32_t diff_var = std::abs(currentVariance_ -\n> >> + status_.maxVariance);\n> >> + const double var_ratio = diff_var / status_.maxVariance;\n> >> + LOG(IoBAf, Debug) << \"Variance change rate: \"\n> >> + << var_ratio\n> >> + << \" Current VCM step: \"\n> >> + << status_.focus;\n> >> + if (var_ratio > kMaxChange)\n> >> + return true;\n> >> + else\n> >> + return false;\n> >> +}\n> >> +\n> >> +void Af::Process(StatisticsPtr &stats, Metadata *image_metadata)\n> >> +{\n> >> + unsigned int i;\n> >> + image_metadata->Get(\"af.status\", status_);\n> >> +\n> >> + /* Use the second filter results only, and cache those. */\n> >> + for (i = 0; i < FOCUS_REGIONS; i++)\n> >> + status_.focus_measures[i] = stats->focus_stats[i].contrast_val[1][1]\n> >> + / stats->focus_stats[i].contrast_val_num[1][1];\n> >> + status_.num = i;\n> >> +\n> >> + currentVariance_ = estimateVariance();\n> >> +\n> >> + if (!status_.stable) {\n> >> + afCoarseScan();\n> >> + afFineScan();\n> >> + } else {\n> >> + if (afIsOutOfFocus())\n> >> + afReset();\n> >> + else\n> >> + ignoreCounter_ = kIgnoreFrame;\n> >> + }\n> >> +}\n> >> +\n> >> +/* Register algorithm with the system. */\n> >> +static Algorithm *Create(Controller *controller)\n> >> +{\n> >> + return new Af(controller);\n> >> +}\n> >> +static RegisterAlgorithm reg(NAME, &Create);\n> >> diff --git a/src/ipa/raspberrypi/controller/iob/af.h b/src/ipa/raspberrypi/controller/iob/af.h\n> >> new file mode 100644\n> >> index 00000000..45c9711f\n> >> --- /dev/null\n> >> +++ b/src/ipa/raspberrypi/controller/iob/af.h\n> >> @@ -0,0 +1,55 @@\n> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) 2021, Red Hat\n> >> + * Copyright (C) 2022, Ideas On Board\n> >> + *\n> >> + * af.h - automatic contrast-based focus algorithm\n> >> + */\n> >> +#pragma once\n> >> +\n> >> +#include \"../af_algorithm.hpp\"\n> >> +#include \"../af_status.h\"\n> >> +#include \"../metadata.hpp\"\n> >> +\n> >> +namespace RPiController {\n> >> +\n> >> +class Af : public AfAlgorithm\n> >> +{\n> >> +public:\n> >> + Af(Controller *controller);\n> >> + char const *Name() const override;\n> >> + void Initialise() override;\n> >> + void Prepare(Metadata *image_metadata) override;\n> >> + void Process(StatisticsPtr &stats, Metadata *image_metadata) override;\n> >> +private:\n> >> + double estimateVariance();\n> >> + bool afNeedIgnoreFrame();\n> >> + void afCoarseScan();\n> >> + void afFineScan();\n> >> + bool afScan(uint32_t minSteps);\n> >> + void afReset();\n> >> + bool afIsOutOfFocus();\n> >> +\n> >> + AfStatus status_;\n> >> +\n> >> + /* VCM step configuration. It is the current setting of the VCM step. */\n> >> + uint32_t focus_;\n> >> + /* The best VCM step. It is a local optimum VCM step during scanning. */\n> >> + uint32_t bestFocus_;\n> >> +\n> >> + /* The frames ignored before starting measuring. */\n> >> + uint32_t ignoreCounter_;\n> >> +\n> >> + /* Current AF statistic variance. */\n> >> + double currentVariance_;\n> >> + /* It is used to determine the derivative during scanning */\n> >> + double previousVariance_;\n> >> + /* The designated maximum range of focus scanning. */\n> >> + uint32_t maxStep_;\n> >> + /* If the coarse scan completes, it is set to true. */\n> >> + bool coarseCompleted_;\n> >> + /* If the fine scan completes, it is set to true. */\n> >> + bool fineCompleted_;\n> >> +};\n> >> +\n> >> +} /* namespace RPiController */\n> >> diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build\n> >> index 32897e07..37068ecc 100644\n> >> --- a/src/ipa/raspberrypi/meson.build\n> >> +++ b/src/ipa/raspberrypi/meson.build\n> >> @@ -28,6 +28,7 @@ rpi_ipa_sources = files([\n> >> 'controller/controller.cpp',\n> >> 'controller/histogram.cpp',\n> >> 'controller/algorithm.cpp',\n> >> + 'controller/iob/af.cpp',\n> >> 'controller/rpi/alsc.cpp',\n> >> 'controller/rpi/awb.cpp',\n> >> 'controller/rpi/sharpen.cpp',","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 0A0FBC3256\n\tfor ;\n\tSun, 27 Mar 2022 21:35:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5D55C65631;\n\tSun, 27 Mar 2022 23:35:16 +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 F386B600AB\n\tfor ;\n\tSun, 27 Mar 2022 23:35:14 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 594722F7;\n\tSun, 27 Mar 2022 23:35:14 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648416916;\n\tbh=kL9cAH3HnsexGBS916ERrrV+T8B78nFC1mfON/YOLoA=;\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=jkefuwrjwxgeVzUR2JJbB4NEV5kJf3aRXXFsIxI/qSASdTt+ihiOXSEjR8XRe8RbK\n\t6GBu6qfWXZkwT7Y0BjdU93VBznEDA7oTwQcfLHc7//NJAToI29tlqgqIb3oOIJKG0R\n\tttkB92B8KJMk3NVVYr0x1TtMf5hc0XjmeeRXzETP2ZJspNa83PXHl7tw+JbM0OGTEt\n\tR/17NSn7umxuSbU5zsQv08ViSIrKLPQ9Q6vfpecn+W8Xj3ep+Xb+rt+Gldi/N3JttD\n\tOj0Eu0VA/r4CoHFj5SsdFR0y4lUMDnSMPwCom08xFza4tSRZk5C1ssHxxL4S3ukyBN\n\tpHKw8JucxMdsA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1648416914;\n\tbh=kL9cAH3HnsexGBS916ERrrV+T8B78nFC1mfON/YOLoA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rq4ZIBMvosPYwaeM29XFTNmMrW9c+2QSnFW5BdbRyb+sz23Pj+116Q8WXOgHahGSL\n\tWMbzIo1Wy5jpL67HlWskuCkMcoEUIH6CKl+9e21Ptxt28CUbkS8gIY7w+8wKNpyddZ\n\tsmKfCvcjuphuC46wlCNwKUULDppTCp14Ksf1lwQM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"rq4ZIBMv\"; dkim-atps=neutral","Date":"Mon, 28 Mar 2022 00:35:12 +0300","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20220323160145.90606-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20220323160145.90606-3-jeanmichel.hautbois@ideasonboard.com>\n\t<164807936828.1103026.919370156988060363@Monstersaurus>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 2/4] ipa: raspberrypi:\n\tIntroduce an autofocus algorithm","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":22468,"web_url":"https://patchwork.libcamera.org/comment/22468/","msgid":"","date":"2022-03-27T21:39:51","subject":"Re: [libcamera-devel] [RFC PATCH v2 2/4] ipa: raspberrypi:\n\tIntroduce an autofocus algorithm","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Mar 24, 2022 at 10:15:56AM +0000, Naushir Patuck via libcamera-devel wrote:\n> On Wed, 23 Mar 2022 at 16:01, Jean-Michel Hautbois via libcamera-devel wrote:\n> > Now that the ancillary links are plumbed and we can set the lens\n> > position, implement a contrast-based algorithm for RPi. This algorithm\n> > is adapted from the one proposed for the IPU3 IPA.\n> >\n> > It is currently taking all the regions and tries to make the focus on\n> > the global scene in a first attempt.\n> >\n> > Signed-off-by: Jean-Michel Hautbois \n> > ---\n> > .../raspberrypi/controller/af_algorithm.hpp | 20 ++\n> > src/ipa/raspberrypi/controller/af_status.h | 31 +++\n> > src/ipa/raspberrypi/controller/focus_status.h | 3 +\n> > src/ipa/raspberrypi/controller/iob/af.cpp | 231 ++++++++++++++++++\n> > src/ipa/raspberrypi/controller/iob/af.h | 55 +++++\n> > src/ipa/raspberrypi/meson.build | 1 +\n> > 6 files changed, 341 insertions(+)\n> > create mode 100644 src/ipa/raspberrypi/controller/af_algorithm.hpp\n> > create mode 100644 src/ipa/raspberrypi/controller/af_status.h\n> > create mode 100644 src/ipa/raspberrypi/controller/iob/af.cpp\n> > create mode 100644 src/ipa/raspberrypi/controller/iob/af.h\n> >\n> > diff --git a/src/ipa/raspberrypi/controller/af_algorithm.hpp\n> > b/src/ipa/raspberrypi/controller/af_algorithm.hpp\n> > new file mode 100644\n> > index 00000000..553a37e1\n> > --- /dev/null\n> > +++ b/src/ipa/raspberrypi/controller/af_algorithm.hpp\n> > @@ -0,0 +1,20 @@\n> > +/* SPDX-License-Identifier: BSD-2-Clause */\n> > +/*\n> > + * Copyright (C) 2019, Raspberry Pi (Trading) Limited\n> \n> 2022 :-)\n> \n> \n> > + *\n> > + * af_algorithm.hpp - autofocus control algorithm interface\n> > + */\n> > +#pragma once\n> > +\n> > +#include \"algorithm.hpp\"\n> > +\n> > +namespace RPiController {\n> > +\n> > +class AfAlgorithm : public Algorithm\n> > +{\n> > +public:\n> > + AfAlgorithm(Controller *controller) : Algorithm(controller) {}\n> > + // An af algorithm must provide the following:\n> > +};\n> > +\n> > +} // namespace RPiController\n> > diff --git a/src/ipa/raspberrypi/controller/af_status.h\n> > b/src/ipa/raspberrypi/controller/af_status.h\n> > new file mode 100644\n> > index 00000000..835e1e2f\n> > --- /dev/null\n> > +++ b/src/ipa/raspberrypi/controller/af_status.h\n> > @@ -0,0 +1,31 @@\n> > +/* SPDX-License-Identifier: BSD-2-Clause */\n> > +/*\n> > + * Copyright (C) 2020, Raspberry Pi (Trading) Limited\n> \n> 2022\n> \n> > + * Copyright (C) 2022, Ideas On Board\n> > + *\n> > + * af_status.h - autofocus measurement status\n> > + */\n> > +#pragma once\n> > +\n> > +#include \n> \n> This header should not be included here.\n> \n> > +\n> > +/*\n> > + * The focus algorithm should post the following structure into the image's\n> > + * \"af.status\" metadata.\n> > + */\n> > +\n> > +#ifdef __cplusplus\n> > +extern \"C\" {\n> > +#endif\n> >\n> \n> You can remove the extern \"C\", this is a throwback from the time this file would be\n> linked with a C library.\n\nLet's avoid future cargo-cult. Jean-Michel, could you include a patch in\nthe next version that drops unneeded extern C statement in the Raspberry\nPi IPA ?\n\n> > +\n> > +struct AfStatus {\n> > + unsigned int num;\n> > + uint32_t focus_measures[FOCUS_REGIONS];\n> > + bool stable;\n> > + uint32_t focus;\n> > + double maxVariance;\n> > +};\n> >\n> \n> I wonder if all these are actually needed in AfStatus? The idea of the\n> Controller metadata is to provide the IPA with all that it needs to set the\n> lens control.\n> maxVariance is probably not needed by the IPA, and num/focus_measure is\n> passed out through FocusStatus, see below.\n> \n> The bool stable is ok for now, but I expect it needs to change to an enum\n> showing focussing states very soon.\n> \n> > +\n> > +#ifdef __cplusplus\n> > +}\n> > +#endif\n> > diff --git a/src/ipa/raspberrypi/controller/focus_status.h\n> > b/src/ipa/raspberrypi/controller/focus_status.h\n> > index ace2fe2c..8122df4b 100644\n> > --- a/src/ipa/raspberrypi/controller/focus_status.h\n> > +++ b/src/ipa/raspberrypi/controller/focus_status.h\n> > @@ -19,6 +19,9 @@ extern \"C\" {\n> > struct FocusStatus {\n> > unsigned int num;\n> > uint32_t focus_measures[FOCUS_REGIONS];\n> > + bool stable;\n> > + uint32_t focus;\n> > + double maxVariance;\n> >\n> \n> I don't think we want to include these additions in FocusStatus. This\n> struct is only used to report the focus FoM values back to the application.\n> For focus control, you can use the AfStatus struct above.\n> \n> > };\n> >\n> > #ifdef __cplusplus\n> > diff --git a/src/ipa/raspberrypi/controller/iob/af.cpp b/src/ipa/raspberrypi/controller/iob/af.cpp\n> > new file mode 100644\n> > index 00000000..dc5258ba\n> > --- /dev/null\n> > +++ b/src/ipa/raspberrypi/controller/iob/af.cpp\n> > @@ -0,0 +1,231 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2021, Red Hat\n> > + * Copyright (C) 2022, Ideas On Board\n> > + *\n> > + * af.cpp - automatic contrast-based focus algorithm\n> > + */\n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \"af.h\"\n> > +\n> > +using namespace RPiController;\n> > +using namespace libcamera;\n> > +\n> > +LOG_DEFINE_CATEGORY(IoBAf)\n> > +\n> > +#define NAME \"iob.af\"\n> > +\n> > +/*\n> > + * Maximum focus steps of the VCM control\n> > + * \\todo should be obtained from the VCM driver\n> > + */\n> > +static constexpr uint32_t kMaxFocusSteps = 1023;\n> > +\n> > +/* Minimum focus step for searching appropriate focus */\n> > +static constexpr uint32_t kCoarseSearchStep = 30;\n> > +static constexpr uint32_t kFineSearchStep = 1;\n> > +\n> > +/* Max ratio of variance change, 0.0 < kMaxChange < 1.0 */\n> > +static constexpr double kMaxChange = 0.5;\n> > +\n> > +/* The numbers of frame to be ignored, before performing focus scan. */\n> > +static constexpr uint32_t kIgnoreFrame = 10;\n> > +\n> > +/* Fine scan range 0 < kFineRange < 1 */\n> > +static constexpr double kFineRange = 0.05;\n> > +\n> > +Af::Af(Controller *controller)\n> > + : AfAlgorithm(controller), focus_(0), bestFocus_(0), ignoreCounter_(0),\n> > + currentVariance_(0.0), previousVariance_(0.0), maxStep_(0),\n> > + coarseCompleted_(false), fineCompleted_(false)\n> > +{\n> > +}\n> > +\n> > +char const *Af::Name() const\n> > +{\n> > + return NAME;\n> > +}\n> > +\n> > +void Af::Initialise()\n> > +{\n> > + status_.focus = 0.0;\n> > + status_.maxVariance = 0.0;\n> > + status_.stable = false;\n> > +}\n> > +\n> > +void Af::Prepare(Metadata *image_metadata)\n> > +{\n> > + image_metadata->Set(\"af.status\", status_);\n> > +}\n> > +\n> > +double Af::estimateVariance()\n> > +{\n> > + unsigned int i;\n> > + double mean;\n> > + uint64_t total = 0;\n> > + double var_sum = 0.0;\n> > +\n> > + /* Compute the mean value. */\n> > + for (i = 0; i < FOCUS_REGIONS; i++)\n> > + total += status_.focus_measures[i];\n> > + mean = total / FOCUS_REGIONS;\n> > +\n> > + /* Compute the sum of the squared variance. */\n> > + for (i = 0; i < FOCUS_REGIONS; i++)\n> > + var_sum += std::pow(status_.focus_measures[i] - mean, 2);\n> > +\n> > + return var_sum / FOCUS_REGIONS;\n> > +}\n> > +\n> > +bool Af::afNeedIgnoreFrame()\n> > +{\n> > + if (ignoreCounter_ == 0)\n> > + return false;\n> > + else\n> > + ignoreCounter_--;\n> > + return true;\n> > +}\n> > +\n> > +void Af::afCoarseScan()\n> > +{\n> > + if (coarseCompleted_)\n> > + return;\n> > +\n> > + if (afNeedIgnoreFrame())\n> > + return;\n> > +\n> > + if (afScan(kCoarseSearchStep)) {\n> > + coarseCompleted_ = true;\n> > + status_.maxVariance = 0;\n> > + focus_ = status_.focus - (status_.focus * kFineRange);\n> > + status_.focus = focus_;\n> > + previousVariance_ = 0;\n> > + maxStep_ = std::clamp(focus_ + static_cast((focus_ * kFineRange)),\n> > + 0U, kMaxFocusSteps);\n> > + }\n> > +}\n> > +\n> > +void Af::afFineScan()\n> > +{\n> > + if (!coarseCompleted_)\n> > + return;\n> > +\n> > + if (afNeedIgnoreFrame())\n> > + return;\n> > +\n> > + if (afScan(kFineSearchStep)) {\n> > + status_.stable = true;\n> > + fineCompleted_ = true;\n> > + }\n> > +}\n> > +\n> > +bool Af::afScan(uint32_t minSteps)\n> > +{\n> > + if (focus_ > maxStep_) {\n> > + /* If the max step is reached, move lens to the position. */\n> > + status_.focus = bestFocus_;\n> > + return true;\n> > + } else {\n> > + /*\n> > + * Find the maximum of the variance by estimating its\n> > + * derivative. If the direction changes, it means we have passed\n> > + * a maximum one step before.\n> > + */\n> > + if ((currentVariance_ - status_.maxVariance) >=\n> > + -(status_.maxVariance * 0.1)) {\n> > + /*\n> > + * Positive and zero derivative:\n> > + * The variance is still increasing. The focus could be\n> > + * increased for the next comparison. Also, the max\n> > + * variance and previous focus value are updated.\n> > + */\n> > + bestFocus_ = focus_;\n> > + focus_ += minSteps;\n> > + status_.focus = focus_;\n> > + status_.maxVariance = currentVariance_;\n> > + } else {\n> > + /*\n> > + * Negative derivative:\n> > + * The variance starts to decrease which means the maximum\n> > + * variance is found. Set focus step to previous good one\n> > + * then return immediately.\n> > + */\n> > + status_.focus = bestFocus_;\n> > + return true;\n> > + }\n> > + }\n> > +\n> > + previousVariance_ = currentVariance_;\n> > + LOG(IoBAf, Debug) << \" Previous step is \"\n> > + << bestFocus_\n> > + << \" Current step is \"\n> > + << focus_;\n> > + return false;\n> > +}\n> > +\n> > +void Af::afReset()\n> > +{\n> > + if (afNeedIgnoreFrame())\n> > + return;\n> > +\n> > + status_.maxVariance = 0;\n> > + status_.focus = 0;\n> > + focus_ = 0;\n> > + status_.stable = false;\n> > + ignoreCounter_ = kIgnoreFrame;\n> > + previousVariance_ = 0.0;\n> > + coarseCompleted_ = false;\n> > + fineCompleted_ = false;\n> > + maxStep_ = kMaxFocusSteps;\n> > +}\n> > +\n> > +bool Af::afIsOutOfFocus()\n> > +{\n> > + const uint32_t diff_var = std::abs(currentVariance_ -\n> > + status_.maxVariance);\n> > + const double var_ratio = diff_var / status_.maxVariance;\n> > + LOG(IoBAf, Debug) << \"Variance change rate: \"\n> > + << var_ratio\n> > + << \" Current VCM step: \"\n> > + << status_.focus;\n> > + if (var_ratio > kMaxChange)\n> > + return true;\n> > + else\n> > + return false;\n> > +}\n> > +\n> > +void Af::Process(StatisticsPtr &stats, Metadata *image_metadata)\n> > +{\n> > + unsigned int i;\n> > + image_metadata->Get(\"af.status\", status_);\n> \n> I'm a bit unsure of this one. Perhaps I don't understand the code well\n> enough, but do you need to do this? \"af.status\" gets set in Af::Prepare()\n> from status_, then here we set status_ from \"af.status\". Nothing will have\n> changed status_ between the two calls, so maybe this is unnecessary?\n> \n> > +\n> > + /* Use the second filter results only, and cache those. */\n> > + for (i = 0; i < FOCUS_REGIONS; i++)\n> > + status_.focus_measures[i] = stats->focus_stats[i].contrast_val[1][1]\n> > + / stats->focus_stats[i].contrast_val_num[1][1];\n> > + status_.num = i;\n> > +\n> > + currentVariance_ = estimateVariance();\n> > +\n> > + if (!status_.stable) {\n> > + afCoarseScan();\n> > + afFineScan();\n> > + } else {\n> > + if (afIsOutOfFocus())\n> > + afReset();\n> > + else\n> > + ignoreCounter_ = kIgnoreFrame;\n> > + }\n> > +}\n> > +\n> > +/* Register algorithm with the system. */\n> > +static Algorithm *Create(Controller *controller)\n> > +{\n> > + return new Af(controller);\n> > +}\n> > +static RegisterAlgorithm reg(NAME, &Create);\n> > diff --git a/src/ipa/raspberrypi/controller/iob/af.h b/src/ipa/raspberrypi/controller/iob/af.h\n> > new file mode 100644\n> > index 00000000..45c9711f\n> > --- /dev/null\n> > +++ b/src/ipa/raspberrypi/controller/iob/af.h\n> > @@ -0,0 +1,55 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2021, Red Hat\n> > + * Copyright (C) 2022, Ideas On Board\n> > + *\n> > + * af.h - automatic contrast-based focus algorithm\n> > + */\n> > +#pragma once\n> > +\n> > +#include \"../af_algorithm.hpp\"\n> > +#include \"../af_status.h\"\n> > +#include \"../metadata.hpp\"\n> > +\n> > +namespace RPiController {\n> > +\n> > +class Af : public AfAlgorithm\n> \n> It may be practical to put class Af in a separate (iob?) namespace to avoid\n> symbol clashes. Once RPi have an algorithm implementation, we will have to\n> enclose our class Af in a rpi namespace or equivalent. What do folks think?\n\nAgreed. I suspect we'll either drop this implementation at that point\nthough.\n\n> > +{\n> > +public:\n> > + Af(Controller *controller);\n> > + char const *Name() const override;\n> > + void Initialise() override;\n> > + void Prepare(Metadata *image_metadata) override;\n> > + void Process(StatisticsPtr &stats, Metadata *image_metadata) override;\n> > +private:\n> > + double estimateVariance();\n> > + bool afNeedIgnoreFrame();\n> > + void afCoarseScan();\n> > + void afFineScan();\n> > + bool afScan(uint32_t minSteps);\n> > + void afReset();\n> > + bool afIsOutOfFocus();\n> > +\n> > + AfStatus status_;\n> > +\n> > + /* VCM step configuration. It is the current setting of the VCM step. */\n> > + uint32_t focus_;\n> > + /* The best VCM step. It is a local optimum VCM step during scanning. */\n> > + uint32_t bestFocus_;\n> > +\n> > + /* The frames ignored before starting measuring. */\n> > + uint32_t ignoreCounter_;\n> > +\n> > + /* Current AF statistic variance. */\n> > + double currentVariance_;\n> > + /* It is used to determine the derivative during scanning */\n> > + double previousVariance_;\n> > + /* The designated maximum range of focus scanning. */\n> > + uint32_t maxStep_;\n> > + /* If the coarse scan completes, it is set to true. */\n> > + bool coarseCompleted_;\n> > + /* If the fine scan completes, it is set to true. */\n> > + bool fineCompleted_;\n> > +};\n> > +\n> > +} /* namespace RPiController */\n> > diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build\n> > index 32897e07..37068ecc 100644\n> > --- a/src/ipa/raspberrypi/meson.build\n> > +++ b/src/ipa/raspberrypi/meson.build\n> > @@ -28,6 +28,7 @@ rpi_ipa_sources = files([\n> > 'controller/controller.cpp',\n> > 'controller/histogram.cpp',\n> > 'controller/algorithm.cpp',\n> > + 'controller/iob/af.cpp',\n> > 'controller/rpi/alsc.cpp',\n> > 'controller/rpi/awb.cpp',\n> > 'controller/rpi/sharpen.cpp',","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 0E37CC0F1B\n\tfor ;\n\tSun, 27 Mar 2022 21:39:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6701365631;\n\tSun, 27 Mar 2022 23:39:54 +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 6FD9C600AB\n\tfor ;\n\tSun, 27 Mar 2022 23:39:53 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DB3802F7;\n\tSun, 27 Mar 2022 23:39:52 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648417194;\n\tbh=0U92AI8iuvmcanOr1wsGFHm6WORd8glus/y6qfe9jSo=;\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=U/XzCa+/NDk4Qb5V1Od6KJro8SawX1QuTOpLYOf+mUzstgAf5/+lMl0cSayIjeTzV\n\tbyxppHES8CawTmVt51sBBaQDBN26IOAyg3iXLUAqYfkyJKXY7VsLNHD66tT4fZU549\n\t0UdV4hOQiHW058504DqassRWf7sG9a++XCMBmKwkSIMfUK83q7nqUyUk35AlmWJzkh\n\tOjvTYy0FUSkmtFP3Iyuu8wY3PdHt97NbcKFHy26YBIgeW+Wid6vleqtb99MVeG28YR\n\tlBszj4F1cq7GBnv/zfYUkegNQ1jliZ3ANWUfG/Oap7FJyNLjTA4rx3Avy5zBwWFeHk\n\tXIWU4SLxYYuzg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1648417193;\n\tbh=0U92AI8iuvmcanOr1wsGFHm6WORd8glus/y6qfe9jSo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=uYHOwClTc4GmEeBtC7rvOUn5kK7+qZWz90iBGf9TU5eI6XbSKBkBdl5ohLlho5nH1\n\tR+N40meT2vfC3AlTcLWNgiDJtAH4B3Mc4dxrXE8QJLc1dk+HBmOaEwb9OnRfJLoXHa\n\t8gBUJrgQ31rTaRZ7hJD9LJ5N00hcH1+/VuXXsgvw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"uYHOwClT\"; dkim-atps=neutral","Date":"Mon, 28 Mar 2022 00:39:51 +0300","To":"Naushir Patuck ","Message-ID":"","References":"<20220323160145.90606-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20220323160145.90606-3-jeanmichel.hautbois@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 2/4] ipa: raspberrypi:\n\tIntroduce an autofocus algorithm","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22469,"web_url":"https://patchwork.libcamera.org/comment/22469/","msgid":"","date":"2022-03-27T21:56:21","subject":"Re: [libcamera-devel] [RFC PATCH v2 2/4] ipa: raspberrypi:\n\tIntroduce an autofocus algorithm","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\n(CC'ing Kate for a question below)\n\nOn Fri, Mar 25, 2022 at 11:15:21AM +0000, David Plowman via libcamera-devel wrote:\n> Hi Jean-Michel\n> \n> Thanks for this patch, great to see progress on the AF question!\n> \n> On Wed, 23 Mar 2022 at 16:01, Jean-Michel Hautbois wrote:\n> >\n> > Now that the ancillary links are plumbed and we can set the lens\n> > position, implement a contrast-based algorithm for RPi. This algorithm\n> > is adapted from the one proposed for the IPU3 IPA.\n> >\n> > It is currently taking all the regions and tries to make the focus on\n> > the global scene in a first attempt.\n> >\n> > Signed-off-by: Jean-Michel Hautbois \n> > ---\n> > .../raspberrypi/controller/af_algorithm.hpp | 20 ++\n> > src/ipa/raspberrypi/controller/af_status.h | 31 +++\n> > src/ipa/raspberrypi/controller/focus_status.h | 3 +\n> > src/ipa/raspberrypi/controller/iob/af.cpp | 231 ++++++++++++++++++\n> > src/ipa/raspberrypi/controller/iob/af.h | 55 +++++\n> > src/ipa/raspberrypi/meson.build | 1 +\n> > 6 files changed, 341 insertions(+)\n> > create mode 100644 src/ipa/raspberrypi/controller/af_algorithm.hpp\n> > create mode 100644 src/ipa/raspberrypi/controller/af_status.h\n> > create mode 100644 src/ipa/raspberrypi/controller/iob/af.cpp\n> > create mode 100644 src/ipa/raspberrypi/controller/iob/af.h\n> >\n> > diff --git a/src/ipa/raspberrypi/controller/af_algorithm.hpp b/src/ipa/raspberrypi/controller/af_algorithm.hpp\n> > new file mode 100644\n> > index 00000000..553a37e1\n> > --- /dev/null\n> > +++ b/src/ipa/raspberrypi/controller/af_algorithm.hpp\n> > @@ -0,0 +1,20 @@\n> > +/* SPDX-License-Identifier: BSD-2-Clause */\n> > +/*\n> > + * Copyright (C) 2019, Raspberry Pi (Trading) Limited\n> > + *\n> > + * af_algorithm.hpp - autofocus control algorithm interface\n> > + */\n> > +#pragma once\n> > +\n> > +#include \"algorithm.hpp\"\n> > +\n> > +namespace RPiController {\n> > +\n> > +class AfAlgorithm : public Algorithm\n> > +{\n> > +public:\n> > + AfAlgorithm(Controller *controller) : Algorithm(controller) {}\n> > + // An af algorithm must provide the following:\n> \n> Yes, we have a bit of a chicken-or-egg thing going on here at the\n> moment, don't we? Should we try and make this look a bit more like an\n> interface that implements the AF Controls proposal (even though that's\n> not agreed yet). Or do we commit this first on the understanding that\n> we'll come back and patch it all up later? I don't think I really\n> mind, I guess it would just be good to be clear on the plan!\n> \n> For the record, the kinds of methods we'd want in here would include\n> things like:\n> \n> SetMode(AfMode mode); // set the AF mode (manual, auto, continuous)\n> Trigger(); // start a cycle (in auto mode)\n> Cancel(); // cancel a cycle (in auto mode)\n> SetWindows(...); // set AF windows\n> SetRange(....) ; // set AF range\n> etc.\n> \n> > +};\n> > +\n> > +} // namespace RPiController\n> > diff --git a/src/ipa/raspberrypi/controller/af_status.h b/src/ipa/raspberrypi/controller/af_status.h\n> > new file mode 100644\n> > index 00000000..835e1e2f\n> > --- /dev/null\n> > +++ b/src/ipa/raspberrypi/controller/af_status.h\n> > @@ -0,0 +1,31 @@\n> > +/* SPDX-License-Identifier: BSD-2-Clause */\n> > +/*\n> > + * Copyright (C) 2020, Raspberry Pi (Trading) Limited\n> > + * Copyright (C) 2022, Ideas On Board\n> > + *\n> > + * af_status.h - autofocus measurement status\n> > + */\n> > +#pragma once\n> > +\n> > +#include \n> > +\n> > +/*\n> > + * The focus algorithm should post the following structure into the image's\n> > + * \"af.status\" metadata.\n> > + */\n> > +\n> > +#ifdef __cplusplus\n> > +extern \"C\" {\n> > +#endif\n> > +\n> > +struct AfStatus {\n> > + unsigned int num;\n> > + uint32_t focus_measures[FOCUS_REGIONS];\n> > + bool stable;\n> > + uint32_t focus;\n> > + double maxVariance;\n> > +};\n> \n> Exactly what to include here is an interesting question. The basic\n> principles are that we should include:\n> \n> * Anything the pipeline handler needs in order to control hardware or\n> other parts of the system. In this case that would probably mean we\n> pass out \"the next lens position to ask for\".\n> \n> * Anything that we need to fill in the libcamera metadata. So this\n> would include the AfState (which is mostly just\n> scanning/focused/failed).\n> \n> * We also like to pass out any settings that have been applied. This\n> is so that you know for sure whether the algorithm has seen the\n> settings you asked for. In this case it might include the mode, the AF\n> windows, the range, the speed etc. We do tend to do this even when\n> there's no libcamera metadata for the information - we take the view\n> that it's a thing we might need to consider adding at some point in\n> the future.\n> \n> * Anything else that might reasonably be thought to be \"useful\". This\n> is obviously where things become more debatable!\n> \n> Then the final principle would be that we mostly try to exclude other\n> things because (a) they might be algorithm specific and (b) it just\n> makes for more stuff to get wrong unless you really need them.\n> \n> Looking specifically at the above:\n> \n> focus_measures: I'm not convinced that these are necessary here,\n> particularly when they're in the focus algorithm metadata already.\n> \n> stable: I guess this is a focused/not-focused kind of flag? Maybe a\n> scanning/focused/failed enum would be better, and look a bit more like\n> a future \"full\" AF algorithm?\n> \n> focus: is that the next lens position to ask for?\n> \n> maxVariance: that's feeling a bit implementation specific to me. The\n> question is always: what might the pipeline handler do with this?\n> \n> > +\n> > +#ifdef __cplusplus\n> > +}\n> > +#endif\n> > diff --git a/src/ipa/raspberrypi/controller/focus_status.h b/src/ipa/raspberrypi/controller/focus_status.h\n> > index ace2fe2c..8122df4b 100644\n> > --- a/src/ipa/raspberrypi/controller/focus_status.h\n> > +++ b/src/ipa/raspberrypi/controller/focus_status.h\n> > @@ -19,6 +19,9 @@ extern \"C\" {\n> > struct FocusStatus {\n> > unsigned int num;\n> > uint32_t focus_measures[FOCUS_REGIONS];\n> > + bool stable;\n> > + uint32_t focus;\n> > + double maxVariance;\n> \n> I'm guessing these should have been removed again when the AfStatus was added?\n> \n> > };\n> >\n> > #ifdef __cplusplus\n> > diff --git a/src/ipa/raspberrypi/controller/iob/af.cpp b/src/ipa/raspberrypi/controller/iob/af.cpp\n> > new file mode 100644\n> > index 00000000..dc5258ba\n> > --- /dev/null\n> > +++ b/src/ipa/raspberrypi/controller/iob/af.cpp\n> > @@ -0,0 +1,231 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2021, Red Hat\n> > + * Copyright (C) 2022, Ideas On Board\n> > + *\n> > + * af.cpp - automatic contrast-based focus algorithm\n> > + */\n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \"af.h\"\n> > +\n> > +using namespace RPiController;\n> > +using namespace libcamera;\n> > +\n> > +LOG_DEFINE_CATEGORY(IoBAf)\n> > +\n> > +#define NAME \"iob.af\"\n> > +\n> > +/*\n> > + * Maximum focus steps of the VCM control\n> > + * \\todo should be obtained from the VCM driver\n> > + */\n> > +static constexpr uint32_t kMaxFocusSteps = 1023;\n> > +\n> > +/* Minimum focus step for searching appropriate focus */\n> > +static constexpr uint32_t kCoarseSearchStep = 30;\n> > +static constexpr uint32_t kFineSearchStep = 1;\n> > +\n> > +/* Max ratio of variance change, 0.0 < kMaxChange < 1.0 */\n> > +static constexpr double kMaxChange = 0.5;\n> > +\n> > +/* The numbers of frame to be ignored, before performing focus scan. */\n> > +static constexpr uint32_t kIgnoreFrame = 10;\n> > +\n> > +/* Fine scan range 0 < kFineRange < 1 */\n> > +static constexpr double kFineRange = 0.05;\n> > +\n> > +Af::Af(Controller *controller)\n> > + : AfAlgorithm(controller), focus_(0), bestFocus_(0), ignoreCounter_(0),\n> > + currentVariance_(0.0), previousVariance_(0.0), maxStep_(0),\n> > + coarseCompleted_(false), fineCompleted_(false)\n> > +{\n> > +}\n> > +\n> > +char const *Af::Name() const\n> > +{\n> > + return NAME;\n> > +}\n> > +\n> > +void Af::Initialise()\n> > +{\n> > + status_.focus = 0.0;\n> > + status_.maxVariance = 0.0;\n> > + status_.stable = false;\n> > +}\n> > +\n> > +void Af::Prepare(Metadata *image_metadata)\n> > +{\n> > + image_metadata->Set(\"af.status\", status_);\n> > +}\n> > +\n> > +double Af::estimateVariance()\n> > +{\n> > + unsigned int i;\n> > + double mean;\n> > + uint64_t total = 0;\n> > + double var_sum = 0.0;\n> > +\n> > + /* Compute the mean value. */\n> > + for (i = 0; i < FOCUS_REGIONS; i++)\n> > + total += status_.focus_measures[i];\n> > + mean = total / FOCUS_REGIONS;\n> > +\n> > + /* Compute the sum of the squared variance. */\n> > + for (i = 0; i < FOCUS_REGIONS; i++)\n> > + var_sum += std::pow(status_.focus_measures[i] - mean, 2);\n> > +\n> > + return var_sum / FOCUS_REGIONS;\n> > +}\n> \n> I'm interested in the choice of variance across the regions of the\n> per-pixel FoM for measuring the \"goodness\" of focus, rather than just\n> the per-pixel FoM. But maybe this kind of question can be left until\n> the algorithm is running and then we can examine the focus measure\n> curves and see what works best.\n\nI had the exact same question :-) This has been copied from IPU3 code\nsubmitted by Kate, she may have some insight. The filter used by the\nIPU3 for focus measurement is configurable (using a 3x11 kernel), and\nI don't know what filter the values we use correspond to. It is likely\ndifferent than what the Raspberry Pi uses, so using the variance there\nmay make sense. With statistics that accumulate absolute values of a\nsecond-order derivative, I don't see what added value using the variance\nbrings.\n\nLet's assume a scene with objects in two different focal planes,\nforeground and background, with more objects (= covering a larger\nportion of the image) on the foreground plane. Let's assume the FoM\nvalue calculated by the hardware filter is the same for any object that\nis in focus (= foreground object when we focus on the foreground plane\nand background object when we focus on the background plane), and the\nsame for any object that is not in focus (= foreground object when we\nfocus on the background plane and background object when we focus on the\nforeground plane). The variance will be the same for the foreground and\nbackground focus positions, while the FoM will be higher for foreground\nfocus. Using the variance seems counterproductive.\n\n> > +\n> > +bool Af::afNeedIgnoreFrame()\n> > +{\n> > + if (ignoreCounter_ == 0)\n> > + return false;\n> > + else\n> > + ignoreCounter_--;\n> > + return true;\n> > +}\n> > +\n> > +void Af::afCoarseScan()\n> > +{\n> > + if (coarseCompleted_)\n> > + return;\n> > +\n> > + if (afNeedIgnoreFrame())\n> > + return;\n> > +\n> > + if (afScan(kCoarseSearchStep)) {\n> > + coarseCompleted_ = true;\n> > + status_.maxVariance = 0;\n> > + focus_ = status_.focus - (status_.focus * kFineRange);\n> > + status_.focus = focus_;\n> > + previousVariance_ = 0;\n> > + maxStep_ = std::clamp(focus_ + static_cast((focus_ * kFineRange)),\n> > + 0U, kMaxFocusSteps);\n> > + }\n> > +}\n> > +\n> > +void Af::afFineScan()\n> > +{\n> > + if (!coarseCompleted_)\n> > + return;\n> > +\n> > + if (afNeedIgnoreFrame())\n> > + return;\n> > +\n> > + if (afScan(kFineSearchStep)) {\n> > + status_.stable = true;\n> > + fineCompleted_ = true;\n> > + }\n> > +}\n> > +\n> > +bool Af::afScan(uint32_t minSteps)\n> > +{\n> > + if (focus_ > maxStep_) {\n> > + /* If the max step is reached, move lens to the position. */\n> > + status_.focus = bestFocus_;\n> > + return true;\n> > + } else {\n> > + /*\n> > + * Find the maximum of the variance by estimating its\n> > + * derivative. If the direction changes, it means we have passed\n> > + * a maximum one step before.\n> > + */\n> > + if ((currentVariance_ - status_.maxVariance) >=\n> > + -(status_.maxVariance * 0.1)) {\n> > + /*\n> > + * Positive and zero derivative:\n> > + * The variance is still increasing. The focus could be\n> > + * increased for the next comparison. Also, the max\n> > + * variance and previous focus value are updated.\n> > + */\n> > + bestFocus_ = focus_;\n> > + focus_ += minSteps;\n> > + status_.focus = focus_;\n> > + status_.maxVariance = currentVariance_;\n> > + } else {\n> > + /*\n> > + * Negative derivative:\n> > + * The variance starts to decrease which means the maximum\n> > + * variance is found. Set focus step to previous good one\n> > + * then return immediately.\n> > + */\n> > + status_.focus = bestFocus_;\n> > + return true;\n> > + }\n> > + }\n> > +\n> > + previousVariance_ = currentVariance_;\n> > + LOG(IoBAf, Debug) << \" Previous step is \"\n> > + << bestFocus_\n> > + << \" Current step is \"\n> > + << focus_;\n> > + return false;\n> > +}\n> > +\n> > +void Af::afReset()\n> > +{\n> > + if (afNeedIgnoreFrame())\n> > + return;\n> > +\n> > + status_.maxVariance = 0;\n> > + status_.focus = 0;\n> > + focus_ = 0;\n> > + status_.stable = false;\n> > + ignoreCounter_ = kIgnoreFrame;\n> > + previousVariance_ = 0.0;\n> > + coarseCompleted_ = false;\n> > + fineCompleted_ = false;\n> > + maxStep_ = kMaxFocusSteps;\n> > +}\n> > +\n> > +bool Af::afIsOutOfFocus()\n> > +{\n> > + const uint32_t diff_var = std::abs(currentVariance_ -\n> > + status_.maxVariance);\n> > + const double var_ratio = diff_var / status_.maxVariance;\n> > + LOG(IoBAf, Debug) << \"Variance change rate: \"\n> > + << var_ratio\n> > + << \" Current VCM step: \"\n> > + << status_.focus;\n> > + if (var_ratio > kMaxChange)\n> > + return true;\n> > + else\n> > + return false;\n> > +}\n> > +\n> > +void Af::Process(StatisticsPtr &stats, Metadata *image_metadata)\n> > +{\n> > + unsigned int i;\n> > + image_metadata->Get(\"af.status\", status_);\n> > +\n> > + /* Use the second filter results only, and cache those. */\n> > + for (i = 0; i < FOCUS_REGIONS; i++)\n> > + status_.focus_measures[i] = stats->focus_stats[i].contrast_val[1][1]\n> > + / stats->focus_stats[i].contrast_val_num[1][1];\n> > + status_.num = i;\n> > +\n> > + currentVariance_ = estimateVariance();\n> > +\n> > + if (!status_.stable) {\n> > + afCoarseScan();\n> > + afFineScan();\n> > + } else {\n> > + if (afIsOutOfFocus())\n> > + afReset();\n> > + else\n> > + ignoreCounter_ = kIgnoreFrame;\n> > + }\n> > +}\n> > +\n> > +/* Register algorithm with the system. */\n> > +static Algorithm *Create(Controller *controller)\n> > +{\n> > + return new Af(controller);\n> > +}\n> > +static RegisterAlgorithm reg(NAME, &Create);\n> > diff --git a/src/ipa/raspberrypi/controller/iob/af.h b/src/ipa/raspberrypi/controller/iob/af.h\n> > new file mode 100644\n> > index 00000000..45c9711f\n> > --- /dev/null\n> > +++ b/src/ipa/raspberrypi/controller/iob/af.h\n> > @@ -0,0 +1,55 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2021, Red Hat\n> > + * Copyright (C) 2022, Ideas On Board\n> > + *\n> > + * af.h - automatic contrast-based focus algorithm\n> > + */\n> > +#pragma once\n> > +\n> > +#include \"../af_algorithm.hpp\"\n> > +#include \"../af_status.h\"\n> > +#include \"../metadata.hpp\"\n> > +\n> > +namespace RPiController {\n> \n> I think the namespace thing has been commented on elsewhere.\n> \n> > +\n> > +class Af : public AfAlgorithm\n> > +{\n> > +public:\n> > + Af(Controller *controller);\n> > + char const *Name() const override;\n> > + void Initialise() override;\n> > + void Prepare(Metadata *image_metadata) override;\n> > + void Process(StatisticsPtr &stats, Metadata *image_metadata) override;\n> > +private:\n> > + double estimateVariance();\n> > + bool afNeedIgnoreFrame();\n> > + void afCoarseScan();\n> > + void afFineScan();\n> > + bool afScan(uint32_t minSteps);\n> > + void afReset();\n> > + bool afIsOutOfFocus();\n> > +\n> > + AfStatus status_;\n> > +\n> > + /* VCM step configuration. It is the current setting of the VCM step. */\n> > + uint32_t focus_;\n> > + /* The best VCM step. It is a local optimum VCM step during scanning. */\n> > + uint32_t bestFocus_;\n> > +\n> > + /* The frames ignored before starting measuring. */\n> > + uint32_t ignoreCounter_;\n> > +\n> > + /* Current AF statistic variance. */\n> > + double currentVariance_;\n> > + /* It is used to determine the derivative during scanning */\n> > + double previousVariance_;\n> > + /* The designated maximum range of focus scanning. */\n> > + uint32_t maxStep_;\n> > + /* If the coarse scan completes, it is set to true. */\n> > + bool coarseCompleted_;\n> > + /* If the fine scan completes, it is set to true. */\n> > + bool fineCompleted_;\n> > +};\n> > +\n> > +} /* namespace RPiController */\n> > diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build\n> > index 32897e07..37068ecc 100644\n> > --- a/src/ipa/raspberrypi/meson.build\n> > +++ b/src/ipa/raspberrypi/meson.build\n> > @@ -28,6 +28,7 @@ rpi_ipa_sources = files([\n> > 'controller/controller.cpp',\n> > 'controller/histogram.cpp',\n> > 'controller/algorithm.cpp',\n> > + 'controller/iob/af.cpp',\n> > 'controller/rpi/alsc.cpp',\n> > 'controller/rpi/awb.cpp',\n> > 'controller/rpi/sharpen.cpp',\n> \n> One other thing I'd like to understand better is how an application\n> knows what lens positions to ask for in \"manual mode\". The most common\n> use case, I think, would be to set the lens to \"hyperfocal position\"\n> when the system starts, or after capturing an image following an AF\n> cycle.\n> \n> How would we know what lens position corresponds to hyperfocal? Would\n> it be the default value of the V4L2 focus position control? The catch\n> here is that the lens actuator cannot know - in fact it can't even\n> know the valid usable range for any given type of module.\n> \n> Does anyone have any thoughts on this?\n\nAs it's module-dependent, I think this will need to come from tuning\ndata. An interesting question will be how to measure it.","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 F3719C3256\n\tfor ;\n\tSun, 27 Mar 2022 21:56:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 47C4365633;\n\tSun, 27 Mar 2022 23:56:26 +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 2A671600AB\n\tfor ;\n\tSun, 27 Mar 2022 23:56:24 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 870712F7;\n\tSun, 27 Mar 2022 23:56:23 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648418186;\n\tbh=Lg4e1yLWhqkwjk9Q9XNAKnM7Zaw1C9BZenudG1R++Eg=;\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=Of8G0iT5FVyTdpkZLRtqA9XkqfGecMR71gknsO7dK1KmV7Bl7fWpyoRXJnhIOVqln\n\tMp3nJ+N/hRp1/kApU7jO8FcwkY9AXk7SdZeornYuo406JT2WyoDyYxlcvPfrqOGJ9b\n\tuFSFoU4AoYUKZ4REHJKWGenS2XoKArgzmEAxnbfZgy681z0khyi0Fa1DMpwgZiQ1Om\n\tgv8+byWf7o/Yy/UN/mheg1mcjlA5OqnO8srN5wd8H96qRwwIZ0W+D5sYxQjGJGJcQG\n\tEJqnpU8gdCbPeDR04tyCPfZ1SeV8xIGJ5nT3O43WLQvQFZw2UTy1zoQPMS26oEh6nU\n\tMko5hwUmPvGTw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1648418183;\n\tbh=Lg4e1yLWhqkwjk9Q9XNAKnM7Zaw1C9BZenudG1R++Eg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=tUMas/FJbAJkTLlyd2cj4lIQftAJ9qm4iUHfvcqTTRUCToV0A4dszA8uzpP0+8AYb\n\tQd6SGMveH+CwSaKO4520kbuYD3JlL4O2Q7J2crralswrtFTypG4dbf6yeaU2duWi0B\n\tIrpf2ZiCUBiMfhsUJG7pdc622vtVM751kKA2PMbU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"tUMas/FJ\"; dkim-atps=neutral","Date":"Mon, 28 Mar 2022 00:56:21 +0300","To":"David Plowman ","Message-ID":"","References":"<20220323160145.90606-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20220323160145.90606-3-jeanmichel.hautbois@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 2/4] ipa: raspberrypi:\n\tIntroduce an autofocus algorithm","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]