[{"id":26798,"web_url":"https://patchwork.libcamera.org/comment/26798/","msgid":"<20230330183314.f6zdkjexwbz4n3jk@uno.localdomain>","date":"2023-03-30T18:33:14","subject":"Re: [libcamera-devel] [PATCH v2 10/10] ipa: raspberrypi: Remove the\n\tfocus reporting algorithm","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Mon, Mar 27, 2023 at 01:20:30PM +0100, Naushir Patuck via libcamera-devel wrote:\n> Update the IPA focus reporting code to use the generalised statistics\n> Region structure.\n>\n> Remove the focus reporting algorithm as the functionality is duplicated\n> this bit of IPA code.  Remove focus_status.h as it is no longer needed.\n\nis duplicated \"by\" ?\n\n>\n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> ---\n>  src/ipa/raspberrypi/controller/focus_status.h | 20 --------\n>  src/ipa/raspberrypi/controller/rpi/af.h       |  1 -\n>  src/ipa/raspberrypi/controller/rpi/focus.cpp  | 49 -------------------\n>  src/ipa/raspberrypi/data/imx290.json          |  3 --\n>  src/ipa/raspberrypi/data/imx296.json          |  3 --\n>  src/ipa/raspberrypi/data/imx296_mono.json     |  3 --\n>  src/ipa/raspberrypi/data/imx477.json          |  3 --\n>  src/ipa/raspberrypi/data/imx477_noir.json     |  3 --\n>  .../raspberrypi/data/imx477_scientific.json   |  3 --\n>  src/ipa/raspberrypi/data/imx477_v1.json       |  3 --\n>  src/ipa/raspberrypi/data/imx708.json          |  3 --\n>  src/ipa/raspberrypi/data/imx708_noir.json     |  3 --\n>  src/ipa/raspberrypi/data/imx708_wide.json     |  3 --\n>  .../raspberrypi/data/imx708_wide_noir.json    |  3 --\n>  src/ipa/raspberrypi/meson.build               |  1 -\n>  src/ipa/raspberrypi/raspberrypi.cpp           | 16 ++++--\n>  16 files changed, 11 insertions(+), 109 deletions(-)\n>  delete mode 100644 src/ipa/raspberrypi/controller/focus_status.h\n>  delete mode 100644 src/ipa/raspberrypi/controller/rpi/focus.cpp\n>\n> diff --git a/src/ipa/raspberrypi/controller/focus_status.h b/src/ipa/raspberrypi/controller/focus_status.h\n> deleted file mode 100644\n> index 8b74e59840c1..000000000000\n> --- a/src/ipa/raspberrypi/controller/focus_status.h\n> +++ /dev/null\n> @@ -1,20 +0,0 @@\n> -/* SPDX-License-Identifier: BSD-2-Clause */\n> -/*\n> - * Copyright (C) 2020, Raspberry Pi Ltd\n> - *\n> - * focus_status.h - focus measurement status\n> - */\n> -#pragma once\n> -\n> -#include <linux/bcm2835-isp.h>\n> -\n> -/*\n> - * The focus algorithm should post the following structure into the image's\n> - * \"focus.status\" metadata. Recall that it's only reporting focus (contrast)\n> - * measurements, it's not driving any kind of auto-focus algorithm!\n> - */\n> -\n> -struct FocusStatus {\n> -\tunsigned int num;\n> -\tuint32_t focusMeasures[FOCUS_REGIONS];\n> -};\n> diff --git a/src/ipa/raspberrypi/controller/rpi/af.h b/src/ipa/raspberrypi/controller/rpi/af.h\n> index b479feb88c39..6d2bae671a22 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/af.h\n> +++ b/src/ipa/raspberrypi/controller/rpi/af.h\n> @@ -28,7 +28,6 @@\n>   * \"nuisance\" scans. During each interval where PDAF is not working, only\n>   * ONE scan will be performed; CAF cannot track objects using CDAF alone.\n>   *\n> - * This algorithm is unrelated to \"rpi.focus\" which merely reports CDAF FoM.\n>   */\n>\n>  namespace RPiController {\n> diff --git a/src/ipa/raspberrypi/controller/rpi/focus.cpp b/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> deleted file mode 100644\n> index ea3cc00e42c3..000000000000\n> --- a/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> +++ /dev/null\n> @@ -1,49 +0,0 @@\n> -/* SPDX-License-Identifier: BSD-2-Clause */\n> -/*\n> - * Copyright (C) 2020, Raspberry Pi Ltd\n> - *\n> - * focus.cpp - focus algorithm\n> - */\n> -#include <stdint.h>\n> -\n> -#include <libcamera/base/log.h>\n> -\n> -#include \"../focus_status.h\"\n> -#include \"focus.h\"\n> -\n> -using namespace RPiController;\n> -using namespace libcamera;\n> -\n> -LOG_DEFINE_CATEGORY(RPiFocus)\n> -\n> -#define NAME \"rpi.focus\"\n> -\n> -Focus::Focus(Controller *controller)\n> -\t: Algorithm(controller)\n> -{\n> -}\n> -\n> -char const *Focus::name() const\n> -{\n> -\treturn NAME;\n> -}\n> -\n> -void Focus::process(StatisticsPtr &stats, Metadata *imageMetadata)\n> -{\n> -\tFocusStatus status;\n> -\tfor (unsigned int i = 0; i < stats->focusRegions.numRegions(); i++)\n> -\t\tstatus.focusMeasures[i] = stats->focusRegions.get(i).val;\n> -\tstatus.num = stats->focusRegions.numRegions();\n> -\timageMetadata->set(\"focus.status\", status);\n> -\n> -\tLOG(RPiFocus, Debug)\n> -\t\t<< \"Focus contrast measure: \"\n> -\t\t<< (status.focusMeasures[5] + status.focusMeasures[6]) / 10;\n> -}\n> -\n> -/* Register algorithm with the system. */\n> -static Algorithm *create(Controller *controller)\n> -{\n> -\treturn new Focus(controller);\n> -}\n> -static RegisterAlgorithm reg(NAME, &create);\n> diff --git a/src/ipa/raspberrypi/data/imx290.json b/src/ipa/raspberrypi/data/imx290.json\n> index bfb9c6093d26..ace68d0ebf1c 100644\n> --- a/src/ipa/raspberrypi/data/imx290.json\n> +++ b/src/ipa/raspberrypi/data/imx290.json\n> @@ -195,9 +195,6 @@\n>                      }\n>                  ]\n>              }\n> -        },\n> -        {\n> -            \"rpi.focus\": { }\n>          }\n>      ]\n>  }\n> \\ No newline at end of file\n> diff --git a/src/ipa/raspberrypi/data/imx296.json b/src/ipa/raspberrypi/data/imx296.json\n> index 346f5b658957..ae8722c46a85 100644\n> --- a/src/ipa/raspberrypi/data/imx296.json\n> +++ b/src/ipa/raspberrypi/data/imx296.json\n> @@ -532,9 +532,6 @@\n>                  \"strength\": 1.0,\n>                  \"limit\": 0.18\n>              }\n> -        },\n> -        {\n> -            \"rpi.focus\": { }\n>          }\n>      ]\n>  }\n> diff --git a/src/ipa/raspberrypi/data/imx296_mono.json b/src/ipa/raspberrypi/data/imx296_mono.json\n> index e9fa30c6a08d..30965b4b8d61 100644\n> --- a/src/ipa/raspberrypi/data/imx296_mono.json\n> +++ b/src/ipa/raspberrypi/data/imx296_mono.json\n> @@ -228,9 +228,6 @@\n>                  \"strength\": 1.0,\n>                  \"limit\": 0.18\n>              }\n> -        },\n> -        {\n> -            \"rpi.focus\": { }\n>          }\n>      ]\n>  }\n> diff --git a/src/ipa/raspberrypi/data/imx477.json b/src/ipa/raspberrypi/data/imx477.json\n> index bfc0774ffdfc..daffc268ff43 100644\n> --- a/src/ipa/raspberrypi/data/imx477.json\n> +++ b/src/ipa/raspberrypi/data/imx477.json\n> @@ -513,9 +513,6 @@\n>          },\n>          {\n>              \"rpi.sharpen\": { }\n> -        },\n> -        {\n> -            \"rpi.focus\": { }\n>          }\n>      ]\n>  }\n> \\ No newline at end of file\n> diff --git a/src/ipa/raspberrypi/data/imx477_noir.json b/src/ipa/raspberrypi/data/imx477_noir.json\n> index dadd97bc3d78..52d7f072d0ce 100644\n> --- a/src/ipa/raspberrypi/data/imx477_noir.json\n> +++ b/src/ipa/raspberrypi/data/imx477_noir.json\n> @@ -424,9 +424,6 @@\n>          },\n>          {\n>              \"rpi.sharpen\": { }\n> -        },\n> -        {\n> -            \"rpi.focus\": { }\n>          }\n>      ]\n>  }\n> \\ No newline at end of file\n> diff --git a/src/ipa/raspberrypi/data/imx477_scientific.json b/src/ipa/raspberrypi/data/imx477_scientific.json\n> index 17c4ed0a5e74..26c692fdbab9 100644\n> --- a/src/ipa/raspberrypi/data/imx477_scientific.json\n> +++ b/src/ipa/raspberrypi/data/imx477_scientific.json\n> @@ -474,9 +474,6 @@\n>          },\n>          {\n>              \"rpi.sharpen\": { }\n> -        },\n> -        {\n> -            \"rpi.focus\": { }\n>          }\n>      ]\n>  }\n> \\ No newline at end of file\n> diff --git a/src/ipa/raspberrypi/data/imx477_v1.json b/src/ipa/raspberrypi/data/imx477_v1.json\n> index 5bcaac67069b..d64020091efa 100644\n> --- a/src/ipa/raspberrypi/data/imx477_v1.json\n> +++ b/src/ipa/raspberrypi/data/imx477_v1.json\n> @@ -511,9 +511,6 @@\n>          },\n>          {\n>              \"rpi.sharpen\": { }\n> -        },\n> -        {\n> -            \"rpi.focus\": { }\n>          }\n>      ]\n>  }\n> \\ No newline at end of file\n> diff --git a/src/ipa/raspberrypi/data/imx708.json b/src/ipa/raspberrypi/data/imx708.json\n> index c38b2d4cf256..b9830a3bf692 100644\n> --- a/src/ipa/raspberrypi/data/imx708.json\n> +++ b/src/ipa/raspberrypi/data/imx708.json\n> @@ -512,9 +512,6 @@\n>          {\n>              \"rpi.sharpen\": { }\n>          },\n> -        {\n> -            \"rpi.focus\": { }\n> -        },\n>          {\n>              \"rpi.af\":\n>              {\n> diff --git a/src/ipa/raspberrypi/data/imx708_noir.json b/src/ipa/raspberrypi/data/imx708_noir.json\n> index 082274e34c53..075f70355cec 100644\n> --- a/src/ipa/raspberrypi/data/imx708_noir.json\n> +++ b/src/ipa/raspberrypi/data/imx708_noir.json\n> @@ -512,9 +512,6 @@\n>          {\n>              \"rpi.sharpen\": { }\n>          },\n> -        {\n> -            \"rpi.focus\": { }\n> -        },\n>          {\n>              \"rpi.af\":\n>              {\n> diff --git a/src/ipa/raspberrypi/data/imx708_wide.json b/src/ipa/raspberrypi/data/imx708_wide.json\n> index cdc61436995d..b772efee3b96 100644\n> --- a/src/ipa/raspberrypi/data/imx708_wide.json\n> +++ b/src/ipa/raspberrypi/data/imx708_wide.json\n> @@ -403,9 +403,6 @@\n>          {\n>              \"rpi.sharpen\": { }\n>          },\n> -        {\n> -            \"rpi.focus\": { }\n> -        },\n>          {\n>              \"rpi.af\":\n>              {\n> diff --git a/src/ipa/raspberrypi/data/imx708_wide_noir.json b/src/ipa/raspberrypi/data/imx708_wide_noir.json\n> index 8a7f59910833..c5f6b53dca7a 100644\n> --- a/src/ipa/raspberrypi/data/imx708_wide_noir.json\n> +++ b/src/ipa/raspberrypi/data/imx708_wide_noir.json\n> @@ -403,9 +403,6 @@\n>          {\n>              \"rpi.sharpen\": { }\n>          },\n> -        {\n> -            \"rpi.focus\": { }\n> -        },\n>          {\n>              \"rpi.af\":\n>              {\n> diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build\n> index 9230e17bca22..de78cbd80f9c 100644\n> --- a/src/ipa/raspberrypi/meson.build\n> +++ b/src/ipa/raspberrypi/meson.build\n> @@ -33,7 +33,6 @@ rpi_ipa_sources = files([\n>      'controller/rpi/awb.cpp',\n>      'controller/rpi/sharpen.cpp',\n>      'controller/rpi/black_level.cpp',\n> -    'controller/rpi/focus.cpp',\n>      'controller/rpi/geq.cpp',\n>      'controller/rpi/noise.cpp',\n>      'controller/rpi/lux.cpp',\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index d813b1b7ca61..c7b9b7007ce9 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -50,7 +50,6 @@\n>  #include \"denoise_algorithm.h\"\n>  #include \"denoise_status.h\"\n>  #include \"dpc_status.h\"\n> -#include \"focus_status.h\"\n>  #include \"geq_status.h\"\n>  #include \"lux_status.h\"\n>  #include \"metadata.h\"\n> @@ -640,14 +639,18 @@ void IPARPi::reportMetadata(unsigned int ipaContext)\n>  \t\t\t\t\t static_cast<int32_t>(blackLevelStatus->blackLevelG),\n>  \t\t\t\t\t static_cast<int32_t>(blackLevelStatus->blackLevelB) });\n>\n> -\tFocusStatus *focusStatus = rpiMetadata.getLocked<FocusStatus>(\"focus.status\");\n> -\tif (focusStatus && focusStatus->num == 12) {\n> +\tRPiController::FocusRegions *focusStatus =\n> +\t\trpiMetadata.getLocked<RPiController::FocusRegions>(\"focus.status\");\n> +\tif (focusStatus) {\n> +\t\tlibcamera::Size size = focusStatus->size();\n>  \t\t/*\n>  \t\t * We get a 4x3 grid of regions by default. Calculate the average\n>  \t\t * FoM over the central two positions to give an overall scene FoM.\n>  \t\t * This can change later if it is not deemed suitable.\n>  \t\t */\n> -\t\tint32_t focusFoM = (focusStatus->focusMeasures[5] + focusStatus->focusMeasures[6]) / 2;\n> +\t\tASSERT(size == Size(4, 3));\n> +\t\tint32_t focusFoM = (focusStatus->get({ 1, 1 }).val +\n> +\t\t\t\t    focusStatus->get({ 2, 1 }).val) / 2;\n>  \t\tlibcameraMetadata_.set(controls::FocusFoM, focusFoM);\n>  \t}\n>\n> @@ -1426,7 +1429,6 @@ RPiController::StatisticsPtr IPARPi::fillStatistics(bcm2835_isp_stats *stats) co\n>  \t\tstatistics->focusRegions.set(i, { stats->focus_stats[i].contrast_val[1][1] / 1000,\n>  \t\t\t\t\t\t  stats->focus_stats[i].contrast_val_num[1][1],\n>  \t\t\t\t\t\t  stats->focus_stats[i].contrast_val_num[1][0] });\n> -\n>  \treturn statistics;\n>  }\n>\n> @@ -1443,6 +1445,10 @@ void IPARPi::processStats(unsigned int bufferId, unsigned int ipaContext)\n>  \tSpan<uint8_t> mem = it->second.planes()[0];\n>  \tbcm2835_isp_stats *stats = reinterpret_cast<bcm2835_isp_stats *>(mem.data());\n>  \tRPiController::StatisticsPtr statistics = fillStatistics(stats);\n> +\n> +\t/* Save the focus stats in the metadata structure to report out later. */\n> +\trpiMetadata_[ipaContext].set(\"focus.status\", statistics->focusRegions);\n> +\n\nCan't the AF algorithm fill the focus.status in its process() call ?\n\n>  \thelper_->process(statistics, rpiMetadata);\n>  \tcontroller_.process(statistics, &rpiMetadata);\n>\n> --\n> 2.34.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 839C6C0F2A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 30 Mar 2023 18:33:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DE05B626E2;\n\tThu, 30 Mar 2023 20:33:18 +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 E4E1861EC7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 30 Mar 2023 20:33:17 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6F6226E1;\n\tThu, 30 Mar 2023 20:33:17 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1680201198;\n\tbh=NBW2a0Mp7cA5RlFyOuNld5DiqAzAqpuO+skdXIzMkjk=;\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=pONB72Ao8dVf3TrHHpCTicw5CVFN0bAjNB+YygZ9dnBDiOz0P8TkzLXI66ZDW5ZO/\n\tScgTmS2n4u3/yC9vzbJEi0CDtEebeydFSy90gzDgMdU15+KxQg6NW0J05RXVqGvM+P\n\tntS7wRsVlbeMICgLuDCbVjg14om9O/Nn83kjliogVgaDJ09lDANNYch1SvL9FZKUJ5\n\tSUB/46ufri0CVA5tOUWFNjOMImFmsext2e7ya4v6PM3IOr08PhXEAe0sxhNmHg6igs\n\tu5PWaXLoqfVXSlhJYQ97Izn+llslZiAOMjBjgJMCro7JLsm1jWAbuPcqjfAd+gnomt\n\tyiEqvxnRtysoA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1680201197;\n\tbh=NBW2a0Mp7cA5RlFyOuNld5DiqAzAqpuO+skdXIzMkjk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=X97ceq7StyrXJB8WLKgQfDJO6hnkx0flsiM+mHiQbbG3E2QoI5jOQsM8vwhECejle\n\ttNybNTys0sStELf4nd7c3V5CIRApgsNk+3SorVlhI7lkYLlAtRFjP4kdI8ZFBp8Y6G\n\tLt1MLPN7LbwvLU1ZaJ+gzUBKuWSabXos176Oteb0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"X97ceq7S\"; dkim-atps=neutral","Date":"Thu, 30 Mar 2023 20:33:14 +0200","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<20230330183314.f6zdkjexwbz4n3jk@uno.localdomain>","References":"<20230327122030.11756-1-naush@raspberrypi.com>\n\t<20230327122030.11756-11-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230327122030.11756-11-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v2 10/10] ipa: raspberrypi: Remove the\n\tfocus reporting algorithm","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":26802,"web_url":"https://patchwork.libcamera.org/comment/26802/","msgid":"<CAEmqJPqKDhj+C4TQCu3Byp2P7MB6aVDpVRJnaTcn55QXOjMcYQ@mail.gmail.com>","date":"2023-03-31T07:25:31","subject":"Re: [libcamera-devel] [PATCH v2 10/10] ipa: raspberrypi: Remove the\n\tfocus reporting algorithm","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\nThanks for the review!\n\nOn Thu, 30 Mar 2023 at 19:33, Jacopo Mondi\n<jacopo.mondi@ideasonboard.com> wrote:\n>\n> Hi Naush\n>\n> On Mon, Mar 27, 2023 at 01:20:30PM +0100, Naushir Patuck via libcamera-devel wrote:\n> > Update the IPA focus reporting code to use the generalised statistics\n> > Region structure.\n> >\n> > Remove the focus reporting algorithm as the functionality is duplicated\n> > this bit of IPA code.  Remove focus_status.h as it is no longer needed.\n>\n> is duplicated \"by\" ?\n\nAck.\n\n>\n> >\n> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > ---\n> >  src/ipa/raspberrypi/controller/focus_status.h | 20 --------\n> >  src/ipa/raspberrypi/controller/rpi/af.h       |  1 -\n> >  src/ipa/raspberrypi/controller/rpi/focus.cpp  | 49 -------------------\n> >  src/ipa/raspberrypi/data/imx290.json          |  3 --\n> >  src/ipa/raspberrypi/data/imx296.json          |  3 --\n> >  src/ipa/raspberrypi/data/imx296_mono.json     |  3 --\n> >  src/ipa/raspberrypi/data/imx477.json          |  3 --\n> >  src/ipa/raspberrypi/data/imx477_noir.json     |  3 --\n> >  .../raspberrypi/data/imx477_scientific.json   |  3 --\n> >  src/ipa/raspberrypi/data/imx477_v1.json       |  3 --\n> >  src/ipa/raspberrypi/data/imx708.json          |  3 --\n> >  src/ipa/raspberrypi/data/imx708_noir.json     |  3 --\n> >  src/ipa/raspberrypi/data/imx708_wide.json     |  3 --\n> >  .../raspberrypi/data/imx708_wide_noir.json    |  3 --\n> >  src/ipa/raspberrypi/meson.build               |  1 -\n> >  src/ipa/raspberrypi/raspberrypi.cpp           | 16 ++++--\n> >  16 files changed, 11 insertions(+), 109 deletions(-)\n> >  delete mode 100644 src/ipa/raspberrypi/controller/focus_status.h\n> >  delete mode 100644 src/ipa/raspberrypi/controller/rpi/focus.cpp\n> >\n> > diff --git a/src/ipa/raspberrypi/controller/focus_status.h b/src/ipa/raspberrypi/controller/focus_status.h\n> > deleted file mode 100644\n> > index 8b74e59840c1..000000000000\n> > --- a/src/ipa/raspberrypi/controller/focus_status.h\n> > +++ /dev/null\n> > @@ -1,20 +0,0 @@\n> > -/* SPDX-License-Identifier: BSD-2-Clause */\n> > -/*\n> > - * Copyright (C) 2020, Raspberry Pi Ltd\n> > - *\n> > - * focus_status.h - focus measurement status\n> > - */\n> > -#pragma once\n> > -\n> > -#include <linux/bcm2835-isp.h>\n> > -\n> > -/*\n> > - * The focus algorithm should post the following structure into the image's\n> > - * \"focus.status\" metadata. Recall that it's only reporting focus (contrast)\n> > - * measurements, it's not driving any kind of auto-focus algorithm!\n> > - */\n> > -\n> > -struct FocusStatus {\n> > -     unsigned int num;\n> > -     uint32_t focusMeasures[FOCUS_REGIONS];\n> > -};\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/af.h b/src/ipa/raspberrypi/controller/rpi/af.h\n> > index b479feb88c39..6d2bae671a22 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/af.h\n> > +++ b/src/ipa/raspberrypi/controller/rpi/af.h\n> > @@ -28,7 +28,6 @@\n> >   * \"nuisance\" scans. During each interval where PDAF is not working, only\n> >   * ONE scan will be performed; CAF cannot track objects using CDAF alone.\n> >   *\n> > - * This algorithm is unrelated to \"rpi.focus\" which merely reports CDAF FoM.\n> >   */\n> >\n> >  namespace RPiController {\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/focus.cpp b/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> > deleted file mode 100644\n> > index ea3cc00e42c3..000000000000\n> > --- a/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> > +++ /dev/null\n> > @@ -1,49 +0,0 @@\n> > -/* SPDX-License-Identifier: BSD-2-Clause */\n> > -/*\n> > - * Copyright (C) 2020, Raspberry Pi Ltd\n> > - *\n> > - * focus.cpp - focus algorithm\n> > - */\n> > -#include <stdint.h>\n> > -\n> > -#include <libcamera/base/log.h>\n> > -\n> > -#include \"../focus_status.h\"\n> > -#include \"focus.h\"\n> > -\n> > -using namespace RPiController;\n> > -using namespace libcamera;\n> > -\n> > -LOG_DEFINE_CATEGORY(RPiFocus)\n> > -\n> > -#define NAME \"rpi.focus\"\n> > -\n> > -Focus::Focus(Controller *controller)\n> > -     : Algorithm(controller)\n> > -{\n> > -}\n> > -\n> > -char const *Focus::name() const\n> > -{\n> > -     return NAME;\n> > -}\n> > -\n> > -void Focus::process(StatisticsPtr &stats, Metadata *imageMetadata)\n> > -{\n> > -     FocusStatus status;\n> > -     for (unsigned int i = 0; i < stats->focusRegions.numRegions(); i++)\n> > -             status.focusMeasures[i] = stats->focusRegions.get(i).val;\n> > -     status.num = stats->focusRegions.numRegions();\n> > -     imageMetadata->set(\"focus.status\", status);\n> > -\n> > -     LOG(RPiFocus, Debug)\n> > -             << \"Focus contrast measure: \"\n> > -             << (status.focusMeasures[5] + status.focusMeasures[6]) / 10;\n> > -}\n> > -\n> > -/* Register algorithm with the system. */\n> > -static Algorithm *create(Controller *controller)\n> > -{\n> > -     return new Focus(controller);\n> > -}\n> > -static RegisterAlgorithm reg(NAME, &create);\n> > diff --git a/src/ipa/raspberrypi/data/imx290.json b/src/ipa/raspberrypi/data/imx290.json\n> > index bfb9c6093d26..ace68d0ebf1c 100644\n> > --- a/src/ipa/raspberrypi/data/imx290.json\n> > +++ b/src/ipa/raspberrypi/data/imx290.json\n> > @@ -195,9 +195,6 @@\n> >                      }\n> >                  ]\n> >              }\n> > -        },\n> > -        {\n> > -            \"rpi.focus\": { }\n> >          }\n> >      ]\n> >  }\n> > \\ No newline at end of file\n> > diff --git a/src/ipa/raspberrypi/data/imx296.json b/src/ipa/raspberrypi/data/imx296.json\n> > index 346f5b658957..ae8722c46a85 100644\n> > --- a/src/ipa/raspberrypi/data/imx296.json\n> > +++ b/src/ipa/raspberrypi/data/imx296.json\n> > @@ -532,9 +532,6 @@\n> >                  \"strength\": 1.0,\n> >                  \"limit\": 0.18\n> >              }\n> > -        },\n> > -        {\n> > -            \"rpi.focus\": { }\n> >          }\n> >      ]\n> >  }\n> > diff --git a/src/ipa/raspberrypi/data/imx296_mono.json b/src/ipa/raspberrypi/data/imx296_mono.json\n> > index e9fa30c6a08d..30965b4b8d61 100644\n> > --- a/src/ipa/raspberrypi/data/imx296_mono.json\n> > +++ b/src/ipa/raspberrypi/data/imx296_mono.json\n> > @@ -228,9 +228,6 @@\n> >                  \"strength\": 1.0,\n> >                  \"limit\": 0.18\n> >              }\n> > -        },\n> > -        {\n> > -            \"rpi.focus\": { }\n> >          }\n> >      ]\n> >  }\n> > diff --git a/src/ipa/raspberrypi/data/imx477.json b/src/ipa/raspberrypi/data/imx477.json\n> > index bfc0774ffdfc..daffc268ff43 100644\n> > --- a/src/ipa/raspberrypi/data/imx477.json\n> > +++ b/src/ipa/raspberrypi/data/imx477.json\n> > @@ -513,9 +513,6 @@\n> >          },\n> >          {\n> >              \"rpi.sharpen\": { }\n> > -        },\n> > -        {\n> > -            \"rpi.focus\": { }\n> >          }\n> >      ]\n> >  }\n> > \\ No newline at end of file\n> > diff --git a/src/ipa/raspberrypi/data/imx477_noir.json b/src/ipa/raspberrypi/data/imx477_noir.json\n> > index dadd97bc3d78..52d7f072d0ce 100644\n> > --- a/src/ipa/raspberrypi/data/imx477_noir.json\n> > +++ b/src/ipa/raspberrypi/data/imx477_noir.json\n> > @@ -424,9 +424,6 @@\n> >          },\n> >          {\n> >              \"rpi.sharpen\": { }\n> > -        },\n> > -        {\n> > -            \"rpi.focus\": { }\n> >          }\n> >      ]\n> >  }\n> > \\ No newline at end of file\n> > diff --git a/src/ipa/raspberrypi/data/imx477_scientific.json b/src/ipa/raspberrypi/data/imx477_scientific.json\n> > index 17c4ed0a5e74..26c692fdbab9 100644\n> > --- a/src/ipa/raspberrypi/data/imx477_scientific.json\n> > +++ b/src/ipa/raspberrypi/data/imx477_scientific.json\n> > @@ -474,9 +474,6 @@\n> >          },\n> >          {\n> >              \"rpi.sharpen\": { }\n> > -        },\n> > -        {\n> > -            \"rpi.focus\": { }\n> >          }\n> >      ]\n> >  }\n> > \\ No newline at end of file\n> > diff --git a/src/ipa/raspberrypi/data/imx477_v1.json b/src/ipa/raspberrypi/data/imx477_v1.json\n> > index 5bcaac67069b..d64020091efa 100644\n> > --- a/src/ipa/raspberrypi/data/imx477_v1.json\n> > +++ b/src/ipa/raspberrypi/data/imx477_v1.json\n> > @@ -511,9 +511,6 @@\n> >          },\n> >          {\n> >              \"rpi.sharpen\": { }\n> > -        },\n> > -        {\n> > -            \"rpi.focus\": { }\n> >          }\n> >      ]\n> >  }\n> > \\ No newline at end of file\n> > diff --git a/src/ipa/raspberrypi/data/imx708.json b/src/ipa/raspberrypi/data/imx708.json\n> > index c38b2d4cf256..b9830a3bf692 100644\n> > --- a/src/ipa/raspberrypi/data/imx708.json\n> > +++ b/src/ipa/raspberrypi/data/imx708.json\n> > @@ -512,9 +512,6 @@\n> >          {\n> >              \"rpi.sharpen\": { }\n> >          },\n> > -        {\n> > -            \"rpi.focus\": { }\n> > -        },\n> >          {\n> >              \"rpi.af\":\n> >              {\n> > diff --git a/src/ipa/raspberrypi/data/imx708_noir.json b/src/ipa/raspberrypi/data/imx708_noir.json\n> > index 082274e34c53..075f70355cec 100644\n> > --- a/src/ipa/raspberrypi/data/imx708_noir.json\n> > +++ b/src/ipa/raspberrypi/data/imx708_noir.json\n> > @@ -512,9 +512,6 @@\n> >          {\n> >              \"rpi.sharpen\": { }\n> >          },\n> > -        {\n> > -            \"rpi.focus\": { }\n> > -        },\n> >          {\n> >              \"rpi.af\":\n> >              {\n> > diff --git a/src/ipa/raspberrypi/data/imx708_wide.json b/src/ipa/raspberrypi/data/imx708_wide.json\n> > index cdc61436995d..b772efee3b96 100644\n> > --- a/src/ipa/raspberrypi/data/imx708_wide.json\n> > +++ b/src/ipa/raspberrypi/data/imx708_wide.json\n> > @@ -403,9 +403,6 @@\n> >          {\n> >              \"rpi.sharpen\": { }\n> >          },\n> > -        {\n> > -            \"rpi.focus\": { }\n> > -        },\n> >          {\n> >              \"rpi.af\":\n> >              {\n> > diff --git a/src/ipa/raspberrypi/data/imx708_wide_noir.json b/src/ipa/raspberrypi/data/imx708_wide_noir.json\n> > index 8a7f59910833..c5f6b53dca7a 100644\n> > --- a/src/ipa/raspberrypi/data/imx708_wide_noir.json\n> > +++ b/src/ipa/raspberrypi/data/imx708_wide_noir.json\n> > @@ -403,9 +403,6 @@\n> >          {\n> >              \"rpi.sharpen\": { }\n> >          },\n> > -        {\n> > -            \"rpi.focus\": { }\n> > -        },\n> >          {\n> >              \"rpi.af\":\n> >              {\n> > diff --git a/src/ipa/raspberrypi/meson.build b/src/ipa/raspberrypi/meson.build\n> > index 9230e17bca22..de78cbd80f9c 100644\n> > --- a/src/ipa/raspberrypi/meson.build\n> > +++ b/src/ipa/raspberrypi/meson.build\n> > @@ -33,7 +33,6 @@ rpi_ipa_sources = files([\n> >      'controller/rpi/awb.cpp',\n> >      'controller/rpi/sharpen.cpp',\n> >      'controller/rpi/black_level.cpp',\n> > -    'controller/rpi/focus.cpp',\n> >      'controller/rpi/geq.cpp',\n> >      'controller/rpi/noise.cpp',\n> >      'controller/rpi/lux.cpp',\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index d813b1b7ca61..c7b9b7007ce9 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -50,7 +50,6 @@\n> >  #include \"denoise_algorithm.h\"\n> >  #include \"denoise_status.h\"\n> >  #include \"dpc_status.h\"\n> > -#include \"focus_status.h\"\n> >  #include \"geq_status.h\"\n> >  #include \"lux_status.h\"\n> >  #include \"metadata.h\"\n> > @@ -640,14 +639,18 @@ void IPARPi::reportMetadata(unsigned int ipaContext)\n> >                                        static_cast<int32_t>(blackLevelStatus->blackLevelG),\n> >                                        static_cast<int32_t>(blackLevelStatus->blackLevelB) });\n> >\n> > -     FocusStatus *focusStatus = rpiMetadata.getLocked<FocusStatus>(\"focus.status\");\n> > -     if (focusStatus && focusStatus->num == 12) {\n> > +     RPiController::FocusRegions *focusStatus =\n> > +             rpiMetadata.getLocked<RPiController::FocusRegions>(\"focus.status\");\n> > +     if (focusStatus) {\n> > +             libcamera::Size size = focusStatus->size();\n> >               /*\n> >                * We get a 4x3 grid of regions by default. Calculate the average\n> >                * FoM over the central two positions to give an overall scene FoM.\n> >                * This can change later if it is not deemed suitable.\n> >                */\n> > -             int32_t focusFoM = (focusStatus->focusMeasures[5] + focusStatus->focusMeasures[6]) / 2;\n> > +             ASSERT(size == Size(4, 3));\n> > +             int32_t focusFoM = (focusStatus->get({ 1, 1 }).val +\n> > +                                 focusStatus->get({ 2, 1 }).val) / 2;\n> >               libcameraMetadata_.set(controls::FocusFoM, focusFoM);\n> >       }\n> >\n> > @@ -1426,7 +1429,6 @@ RPiController::StatisticsPtr IPARPi::fillStatistics(bcm2835_isp_stats *stats) co\n> >               statistics->focusRegions.set(i, { stats->focus_stats[i].contrast_val[1][1] / 1000,\n> >                                                 stats->focus_stats[i].contrast_val_num[1][1],\n> >                                                 stats->focus_stats[i].contrast_val_num[1][0] });\n> > -\n> >       return statistics;\n> >  }\n> >\n> > @@ -1443,6 +1445,10 @@ void IPARPi::processStats(unsigned int bufferId, unsigned int ipaContext)\n> >       Span<uint8_t> mem = it->second.planes()[0];\n> >       bcm2835_isp_stats *stats = reinterpret_cast<bcm2835_isp_stats *>(mem.data());\n> >       RPiController::StatisticsPtr statistics = fillStatistics(stats);\n> > +\n> > +     /* Save the focus stats in the metadata structure to report out later. */\n> > +     rpiMetadata_[ipaContext].set(\"focus.status\", statistics->focusRegions);\n> > +\n>\n> Can't the AF algorithm fill the focus.status in its process() call ?\n\nThat would be the ideal solution, but we cannot rely on the autofocus algorithm\nbeing available all the time.  For example, the IMX477 does not enable af since\nit only has a mechanical focus adjustment, but we do want focus statistics\noutput to help users with the manual focus adjustment.\n\nRegards,\nNaush\n\n\n>\n> >       helper_->process(statistics, rpiMetadata);\n> >       controller_.process(statistics, &rpiMetadata);\n> >\n> > --\n> > 2.34.1\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 61107C326B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 31 Mar 2023 07:25:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6E8C062724;\n\tFri, 31 Mar 2023 09:25:37 +0200 (CEST)","from mail-yb1-xb30.google.com (mail-yb1-xb30.google.com\n\t[IPv6:2607:f8b0:4864:20::b30])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C841C61ECB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 31 Mar 2023 09:25:34 +0200 (CEST)","by mail-yb1-xb30.google.com with SMTP id f188so7955711ybb.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 31 Mar 2023 00:25:34 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1680247537;\n\tbh=rkFqWWGoiirjhFQC3hZ0h9t+zGIcvX2e9nJzc10+1EA=;\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=UiBd8iUK33K4lCw0dTcLi9TM87Cb+0fTEWTLL3eTq5+y/Ru8c0vn7KNYgf1kZsSO+\n\tVdm+69xlY01Wa25JdBbY8SlykK7juZDmCA3zwzKHK64nIwKHeP+JJFGC12/200Gdjp\n\tQQ1a+N06nFE+d6CxKuNzG5JLRC8PaZM1kWeOvihq8eELNc1aUvnaYP3PZ73RvsHxgZ\n\tOI6y9PodL0pf1WSrnyZxL/tVcMoOK+ua8OPbLa9bt1GlNl70fWBEgjC9/eTXQjmuT8\n\ttolIUa//7iuXC8r9fZwglI6lzH4YK98YkaYt87cJ9LmpIijxQNxehol5Oyydt+vZ+P\n\tPu5krtsLK5u9A==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1680247533;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=Najn8cb1cMHPhs9CiibIxE1Hy2+hfFQ4FW975emZYtE=;\n\tb=MkUcL1YJCkcpXOytjtucfP8eZIxOVNlSHW/M4zlIza1xlbDAa4WR9eWqN3KoFNKfU8\n\tUaXek3Aw5tzD/O/D/n82MTrOykqf/kRCk++h5kPQSiARYlTTgfd6CoR9nzx2wOl3Tl6t\n\tAeocbChFWNWIpQkyik54f4vR4ky6QridTOEwkI6ghcOZPwUOnbryq0CQ9l83BFnrwrfy\n\tBD6jmFp2I+1qrFy28Br3nMGT/DwUt3ue4/GTh44t6GsP49e84+fKqrFDoRV5yiLKKkwp\n\tYclP3+68e21YduuKzSpAVpUoQ8kGo92j4ZtZnTUnAiuNqG70GgsPk1iDNMZAv4qv8Qoq\n\tPWmA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"MkUcL1YJ\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112; t=1680247533;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=Najn8cb1cMHPhs9CiibIxE1Hy2+hfFQ4FW975emZYtE=;\n\tb=Ewrs7toOvgbwMjnTn6ToAODxvdArBsOjyw8cfEAWraCnKYsol/nOLc88vDvN5w0a7n\n\ttYg1j7k6qSE/6XTq4UyqlSoWnamNtctlFKRsyNvJMJ17W2hrzuLl0z+rDn2MrzoArB5R\n\tp7jc0no7dM/f9FjdLXrNfR/SyfH49MwT/fm9wZGs5uXKLOc6wA8W57+dEkWHe4wWaRqK\n\twdxGjgFwvAuWBKcgk3jzdmu5WD9nWn1ssAXG1bNowv2PDBWyWpiIKWu8p1ZG2jtH0W8F\n\tb7fpWVkxuMb67jkhbEYmk35dJNsXN4AMw1VTBxJccWbYlSbWbplh8uTIRUD1gkgcvzaA\n\t5b2A==","X-Gm-Message-State":"AAQBX9cK2l8x4ryPhQ0Io5pMIxdEzkoh0eg1gCeHnpwbdi2dSQAvLlj/\n\txvl2henpM6p8cJvKzGFiu9tiK6kqH4WmsoK1nxTSaZN5JryWirOiWB8JUg==","X-Google-Smtp-Source":"AKy350bWGjqm/OQaNpiKUOb+nl0zUdXJ9B8OkDBCqG+x1vgzLIWNOx9omQPUtgMGDtvP3yrrsOxlp+FKu27lBM+zfPc=","X-Received":"by 2002:a05:6902:a8e:b0:b7c:1144:a729 with SMTP id\n\tcd14-20020a0569020a8e00b00b7c1144a729mr8653053ybb.9.1680247533399;\n\tFri, 31 Mar 2023 00:25:33 -0700 (PDT)","MIME-Version":"1.0","References":"<20230327122030.11756-1-naush@raspberrypi.com>\n\t<20230327122030.11756-11-naush@raspberrypi.com>\n\t<20230330183314.f6zdkjexwbz4n3jk@uno.localdomain>","In-Reply-To":"<20230330183314.f6zdkjexwbz4n3jk@uno.localdomain>","Date":"Fri, 31 Mar 2023 08:25:31 +0100","Message-ID":"<CAEmqJPqKDhj+C4TQCu3Byp2P7MB6aVDpVRJnaTcn55QXOjMcYQ@mail.gmail.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 10/10] ipa: raspberrypi: Remove the\n\tfocus reporting algorithm","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Naushir Patuck via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]