[{"id":26711,"web_url":"https://patchwork.libcamera.org/comment/26711/","msgid":"","date":"2023-03-22T14:10:45","subject":"Re: [libcamera-devel] [PATCH v1 08/10] ipa: raspberrypi: Generalise\n\tthe focus algorithm","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for the patch.\n\nMostly I guess I'm OK with this, I find I'm just wondering a little\nbit whether there's any point in the \"rpi.focus\" algorithm at all, and\nwhether we should just set the FocusFoM metadata unconditionally?\n\nI suppose one thing it used to enable was for folks running headless\n(or without X Windows) to set RPiFocus debug and get numbers printed\nout to the console. Are we happy that we have other ways to do that\nnow?\n\nDavid\n\nOn Wed, 22 Mar 2023 at 13:06, Naushir Patuck via libcamera-devel\n wrote:\n>\n> Generalise the focus reporting algorithm code by removing any hard-coded\n> assumptions about the target hardware platform. Instead, the algorithm\n> code uses the generalised statistics structure to determine focus region\n> sizes.\n>\n> Remove focus_status.h as it is no longer needed with this change.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> src/ipa/raspberrypi/controller/focus_status.h | 20 -------------------\n> src/ipa/raspberrypi/controller/rpi/focus.cpp | 12 ++---------\n> src/ipa/raspberrypi/raspberrypi.cpp | 11 ++++++----\n> 3 files changed, 9 insertions(+), 34 deletions(-)\n> delete mode 100644 src/ipa/raspberrypi/controller/focus_status.h\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 \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/focus.cpp b/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> index ea3cc00e42c3..683d460ad6f7 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> @@ -8,7 +8,6 @@\n>\n> #include \n>\n> -#include \"../focus_status.h\"\n> #include \"focus.h\"\n>\n> using namespace RPiController;\n> @@ -30,15 +29,8 @@ char const *Focus::name() const\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> + /* Pass the stats directly to the IPA to report out to the application */\n> + imageMetadata->set(\"focus.status\", stats->focusRegions);\n> }\n>\n> /* Register algorithm with the system. */\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index b6e5465ca32a..181512de74ad 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(blackLevelStatus->blackLevelG),\n> static_cast(blackLevelStatus->blackLevelB) });\n>\n> - FocusStatus *focusStatus = rpiMetadata.getLocked(\"focus.status\");\n> - if (focusStatus && focusStatus->num == 12) {\n> + RPiController::FocusRegions *focusStatus =\n> + rpiMetadata.getLocked(\"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> --\n> 2.34.1\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 25307C0F1B\n\tfor ;\n\tWed, 22 Mar 2023 14:11:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6C923626DA;\n\tWed, 22 Mar 2023 15:10:59 +0100 (CET)","from mail-oo1-xc2a.google.com (mail-oo1-xc2a.google.com\n\t[IPv6:2607:f8b0:4864:20::c2a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 799F361ECE\n\tfor ;\n\tWed, 22 Mar 2023 15:10:57 +0100 (CET)","by mail-oo1-xc2a.google.com with SMTP id\n\ta23-20020a4ad5d7000000b005250867d3d9so3004574oot.10\n\tfor ;\n\tWed, 22 Mar 2023 07:10:57 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679494259;\n\tbh=bcwVqVEjSISUcEv9PVTPEgcd7PG41fnmIJkaoU2169g=;\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=GTuY8T73GRJphU/XlnFSiZ3aKBzoidwVUQa7pWDTnlSghlZ3EOt6yRL9+2knpiMb6\n\tg/SI50uFFd9sQuArdxtX0ZmmBNivREWjJvWcujo46lDJr6xDlHS062rq204ovu2lN7\n\t2X4bERlRR5MQIy3G6dY/53qAeXE3id454wIm9C/QhiwYDc6MjgkdtGxzv38ltkEZpi\n\teTP0QGA91c2tuw1AjCCI9jFuuTrTlaMrHkwr2F5cx294+pA3vhYp68E/QXLGSbIA7M\n\tnuXBs8K3kyE0BQpNpKNBIBTb3t10bgoMHvIT0zng+LLgEZX0GX/5Udi8FdI5ui956e\n\t7od/qJHakA0SQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1679494256;\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=gWgBD/6THXUq9F64Lyp2lbPGVYjkGNGA31QBtd/4uFw=;\n\tb=tkGY68A4LLmIU2WgTRHTp4HK+n9oNVs32r+lz45uL738JEFFa040vnI3uxXpdbgtzx\n\tDM4l3n0WIdbI5oq9FRnrEdADaKObRvLjOL83wO2DVSwOnYADoHWlZKLObBXYFgI4cQMP\n\tKaE02v0C8ekRH8GKaYAQKW6GYs60PgizBxcsmgQAhTfUaHrKvIt/by5hmhfe2tjar6Pt\n\tcBPTZxKH6l9YzXUCMBGhKO3twLbWEaP7Kmg1yLG6QVCUbyxUl1r9Z4It4YQRzoqbuxGr\n\tTczMIAcQRS/QcQlgSlmI7bbeBmrnC1bk7XznEIkeLxNhdPS9FCy1DuGFBEVJxKannPcX\n\tB4bg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"tkGY68A4\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112; t=1679494256;\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=gWgBD/6THXUq9F64Lyp2lbPGVYjkGNGA31QBtd/4uFw=;\n\tb=DLgGdtWc+MhnVrAg8ZjotgLtXsN0JvW6cn3lkuMipn/ZUEzjKpxQPfO/1xEfwagLaR\n\tJJI6dW7hghBGhrvqBVRRVKrpPFjt5+tNWdBSm4paRkBKwB/Wazkp5fCE81iRSzPJSdB4\n\tCqX0NcvNoDhpZe/nfqHSboIOPhNJHJjqYrZ6JbYkO5Cw4cW02dTwAJt/v6Pomd1tc+3n\n\tLe0eQ+7K6JMtygiFKMmVaGgsqgotsBitHCxd+TlG1/abRHhQPSZ69RvY4fWd9EDuTu3y\n\tfKUiImdDanAnROFAyWHcezYn/g5DznSHL58v3FzwdOWxjJm+eppFMEmv8IMgPi2bUg6h\n\tCeKw==","X-Gm-Message-State":"AO0yUKUEC0/eY4IocaJzgkUf/mXTxUyDZXX9ipXINTIpIbEyj83JNVod\n\t4qTW57lqxYDzPK/j1MDp3l5rgQtzp16mwOPdSu20yhvBuyUSDKqgivM=","X-Google-Smtp-Source":"AK7set+YC7w2fxr8W0cX/qJLtCo/W6RiNWmzCcqNJZ9/qJe9wI6d5u8And0Tl+hwMn/YLXAG8kCMKlQjQgsvzEqqe1Q=","X-Received":"by 2002:a4a:e285:0:b0:525:2b3b:7453 with SMTP id\n\tk5-20020a4ae285000000b005252b3b7453mr989251oot.0.1679494256039;\n\tWed, 22 Mar 2023 07:10:56 -0700 (PDT)","MIME-Version":"1.0","References":"<20230322130612.5208-1-naush@raspberrypi.com>\n\t<20230322130612.5208-9-naush@raspberrypi.com>","In-Reply-To":"<20230322130612.5208-9-naush@raspberrypi.com>","Date":"Wed, 22 Mar 2023 14:10:45 +0000","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v1 08/10] ipa: raspberrypi: Generalise\n\tthe focus 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@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26712,"web_url":"https://patchwork.libcamera.org/comment/26712/","msgid":"","date":"2023-03-22T14:39:09","subject":"Re: [libcamera-devel] [PATCH v1 08/10] ipa: raspberrypi: Generalise\n\tthe focus algorithm","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nThanks for the feedback.\n\nOn Wed, 22 Mar 2023 at 14:10, David Plowman \nwrote:\n\n> Hi Naush\n>\n> Thanks for the patch.\n>\n> Mostly I guess I'm OK with this, I find I'm just wondering a little\n> bit whether there's any point in the \"rpi.focus\" algorithm at all, and\n> whether we should just set the FocusFoM metadata unconditionally?\n>\n> I suppose one thing it used to enable was for folks running headless\n> (or without X Windows) to set RPiFocus debug and get numbers printed\n> out to the console. Are we happy that we have other ways to do that\n> now?\n>\n\nI do agree that rpi.focus is a bit redundant, and we ought to\nunconditionally\nadd the focus stats into the metadata. If folks are using debug log\nmessages to\nget the numbers out, we should encourage them to pull it out of the\nmetadata in\ntheir apps. Conveniently, libcamera-apps had a recent change to output\nstuff\nlike to the console without preview :-)\n\nRegards,\nNaush\n\n\n>\n> David\n>\n> On Wed, 22 Mar 2023 at 13:06, Naushir Patuck via libcamera-devel\n> wrote:\n> >\n> > Generalise the focus reporting algorithm code by removing any hard-coded\n> > assumptions about the target hardware platform. Instead, the algorithm\n> > code uses the generalised statistics structure to determine focus region\n> > sizes.\n> >\n> > Remove focus_status.h as it is no longer needed with this change.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > src/ipa/raspberrypi/controller/focus_status.h | 20 -------------------\n> > src/ipa/raspberrypi/controller/rpi/focus.cpp | 12 ++---------\n> > src/ipa/raspberrypi/raspberrypi.cpp | 11 ++++++----\n> > 3 files changed, 9 insertions(+), 34 deletions(-)\n> > delete mode 100644 src/ipa/raspberrypi/controller/focus_status.h\n> >\n> > diff --git a/src/ipa/raspberrypi/controller/focus_status.h\n> 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 \n> > -\n> > -/*\n> > - * The focus algorithm should post the following structure into the\n> image's\n> > - * \"focus.status\" metadata. Recall that it's only reporting focus\n> (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/focus.cpp\n> b/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> > index ea3cc00e42c3..683d460ad6f7 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> > @@ -8,7 +8,6 @@\n> >\n> > #include \n> >\n> > -#include \"../focus_status.h\"\n> > #include \"focus.h\"\n> >\n> > using namespace RPiController;\n> > @@ -30,15 +29,8 @@ char const *Focus::name() const\n> >\n> > void Focus::process(StatisticsPtr &stats, Metadata *imageMetadata)\n> > {\n> > - FocusStatus status;\n> > - for (unsigned int i = 0; i < stats->focusRegions.numRegions();\n> 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]) /\n> 10;\n> > + /* Pass the stats directly to the IPA to report out to the\n> application */\n> > + imageMetadata->set(\"focus.status\", stats->focusRegions);\n> > }\n> >\n> > /* Register algorithm with the system. */\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index b6e5465ca32a..181512de74ad 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\n> ipaContext)\n> >\n> static_cast(blackLevelStatus->blackLevelG),\n> >\n> static_cast(blackLevelStatus->blackLevelB) });\n> >\n> > - FocusStatus *focusStatus =\n> rpiMetadata.getLocked(\"focus.status\");\n> > - if (focusStatus && focusStatus->num == 12) {\n> > + RPiController::FocusRegions *focusStatus =\n> > +\n> rpiMetadata.getLocked(\"focus.status\");\n> > + if (focusStatus) {\n> > + libcamera::Size size = focusStatus->size();\n> > /*\n> > * We get a 4x3 grid of regions by default. Calculate\n> the average\n> > * FoM over the central two positions to give an overall\n> scene FoM.\n> > * This can change later if it is not deemed suitable.\n> > */\n> > - int32_t focusFoM = (focusStatus->focusMeasures[5] +\n> 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> > --\n> > 2.34.1\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 D7511C0F2A\n\tfor ;\n\tWed, 22 Mar 2023 14:39:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EDCB8626E5;\n\tWed, 22 Mar 2023 15:39:17 +0100 (CET)","from mail-yb1-xb31.google.com (mail-yb1-xb31.google.com\n\t[IPv6:2607:f8b0:4864:20::b31])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DDFD361ECE\n\tfor ;\n\tWed, 22 Mar 2023 15:39:15 +0100 (CET)","by mail-yb1-xb31.google.com with SMTP id z83so21263230ybb.2\n\tfor ;\n\tWed, 22 Mar 2023 07:39:15 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679495958;\n\tbh=XmBwYjcPH44jvf7LiUEpbCl2tGRrOvYVEzS6OCKCGdM=;\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=3VBdOCHC2iA3RtuY39hy6XEeNXNDLVtkhlhj3fqgpIQ1jagGNu79rcFGxNVY5sgvA\n\tZE2oyNQg8t00zc/M20cWV1xY71jXZmTaFX+64BHo7hAR7jDj7ZL61ltwTlgo1dh8rN\n\tkfcbCO/Scoyp92t3IVQse2xWiKC/H2FBsgfTPNiAdmJRyVUDvP5IEDKQYV4AwUxM/5\n\tPqZO2hRRRp6Y3p2AAEf93QzEfEB/RdjAlKKvfjXbRV8/6S8xK86auCGLEBmJzJy3xY\n\tBjeY5zEHIsvteLZbiFT/ZdGIXzEpTOxGSN84r2gW5v+g0s4LuUfrmwgyCkB3ysp7gC\n\tWRk/4qxR5AUhQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1679495954;\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=O2PrgzKIPBaicOTZpiPAa71eMWyJwyXNt2L/bp0duyU=;\n\tb=F5S9KTjkXU3CcJYobmQ4lYfsSXnablC5JteGXwt4I2ssfd88JlgfiD3ZlyXfNL55QB\n\t2AzccAsHAZly81C0uuN6bFk3sfX0yC0FQR5lXWizi3Tp3IN8j6is1FpRSlQqOB5Co+mR\n\t2bls087GxuewoXsgrh3ieAUj9FhTnB86PDI8k0yvXdLI7Hk/Wo4SVE1yBmO4rIh+s6HQ\n\t7BFKAPfnQ2CVqj214wM6fPnSWJ6dzxHtG4CQOTCmRXIbvCPOIag0KarsKGfRiQkwTOK+\n\tKdpT3xXaDOeOGqiafy563rdRHBxqp+7B4o7mDo6ty7eFQLmhGtf14tLVg01VHRZ1Qzjq\n\ttqkw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"F5S9KTjk\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112; t=1679495954;\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=O2PrgzKIPBaicOTZpiPAa71eMWyJwyXNt2L/bp0duyU=;\n\tb=OSoNrVp1wCkZ7cErlk/TbIhFAiaDFfBPstr1GCw9vgX1SJ3IIoRkvPDETYm8GtAtrZ\n\t8eoGbwEFswx6TOsCRIudUZUXD59n95E5MqUhiEoSUyu3g+cpOXv7itD3fpgZqhdwZ7pd\n\t54FX6K6BzLK+FQ5uVtLx0VQSp+ez8OJ3EK8YnY9XSN7wleVmpEREH82wxaWskIJjeUuj\n\t0T3szX+vDDV4DD6+f8LWYHARGwdprVQmC5XBJN3nEFwFFr2hlJ41xtd4hNe1CXrUfIqZ\n\ti3IXFGiKfJxJkCZGI/O1+5cCiDpDdbZktAXs+x6Go54fplGXwXJMWYqHc1kdgjcMgU0e\n\tt+lA==","X-Gm-Message-State":"AAQBX9fl44/kd8/94oTbCcerCg82euR6stmlToe/Nr/jqTflsCI3trYR\n\tVhfAKSem+BCXVz+GUmmu7u5zgk9Tj+zzicF9gw6CIvliSmySd72qA/o4Ww==","X-Google-Smtp-Source":"AKy350ZcNAKRxzzrju+ag2enGPKuwQjSy/h5qwJ6w7wfEXT09LFR1qbQgDSJmtDFvp2QQHP35Z/PsXX8hIfQFiXsPkM=","X-Received":"by 2002:a05:6902:168d:b0:94a:ebba:cba6 with SMTP id\n\tbx13-20020a056902168d00b0094aebbacba6mr6542ybb.9.1679495954632;\n\tWed, 22 Mar 2023 07:39:14 -0700 (PDT)","MIME-Version":"1.0","References":"<20230322130612.5208-1-naush@raspberrypi.com>\n\t<20230322130612.5208-9-naush@raspberrypi.com>\n\t","In-Reply-To":"","Date":"Wed, 22 Mar 2023 14:39:09 +0000","Message-ID":"","To":"David Plowman ","Content-Type":"multipart/alternative; boundary=\"000000000000c767fc05f77e1fd2\"","Subject":"Re: [libcamera-devel] [PATCH v1 08/10] ipa: raspberrypi: Generalise\n\tthe focus 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@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26726,"web_url":"https://patchwork.libcamera.org/comment/26726/","msgid":"<20230324092006.7rhjpip2sua4ri5p@uno.localdomain>","date":"2023-03-24T09:20:06","subject":"Re: [libcamera-devel] [PATCH v1 08/10] ipa: raspberrypi: Generalise\n\tthe focus 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 Wed, Mar 22, 2023 at 01:06:10PM +0000, Naushir Patuck via libcamera-devel wrote:\n> Generalise the focus reporting algorithm code by removing any hard-coded\n> assumptions about the target hardware platform. Instead, the algorithm\n> code uses the generalised statistics structure to determine focus region\n> sizes.\n>\n> Remove focus_status.h as it is no longer needed with this change.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> src/ipa/raspberrypi/controller/focus_status.h | 20 -------------------\n> src/ipa/raspberrypi/controller/rpi/focus.cpp | 12 ++---------\n> src/ipa/raspberrypi/raspberrypi.cpp | 11 ++++++----\n> 3 files changed, 9 insertions(+), 34 deletions(-)\n> delete mode 100644 src/ipa/raspberrypi/controller/focus_status.h\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 \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/focus.cpp b/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> index ea3cc00e42c3..683d460ad6f7 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/focus.cpp\n> @@ -8,7 +8,6 @@\n>\n> #include \n>\n> -#include \"../focus_status.h\"\n> #include \"focus.h\"\n>\n> using namespace RPiController;\n> @@ -30,15 +29,8 @@ char const *Focus::name() const\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> +\t/* Pass the stats directly to the IPA to report out to the application */\n> +\timageMetadata->set(\"focus.status\", stats->focusRegions);\n> }\n>\n> /* Register algorithm with the system. */\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index b6e5465ca32a..181512de74ad 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(blackLevelStatus->blackLevelG),\n> \t\t\t\t\t static_cast(blackLevelStatus->blackLevelB) });\n>\n> -\tFocusStatus *focusStatus = rpiMetadata.getLocked(\"focus.status\");\n> -\tif (focusStatus && focusStatus->num == 12) {\n> +\tRPiController::FocusRegions *focusStatus =\n> +\t\trpiMetadata.getLocked(\"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\nNot a 100% sure I understand this one, but\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> \t\tlibcameraMetadata_.set(controls::FocusFoM, focusFoM);\n> \t}\n>\n> --\n> 2.34.1\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 A71B9C0F1B\n\tfor ;\n\tFri, 24 Mar 2023 09:20:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 04267626E2;\n\tFri, 24 Mar 2023 10:20:11 +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 F31AB626D7\n\tfor ;\n\tFri, 24 Mar 2023 10:20:09 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 645F5A49;\n\tFri, 24 Mar 2023 10:20:09 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679649611;\n\tbh=KmCbervTJ0ei6yBctTviNw04TMN5w50UUk80Jtjsb3s=;\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=RmHw4F2yhokWx6MUF6D5evMBmCxYFfBXhbGL+9Kax1l+cP+JQGxqvb5eSklCLqJSG\n\tc9ttbGQUyjXe6+4U0jN/5bReda2hgRoj4DBP/ErouiS7w9wg399n0lL5cMAJmCCoLo\n\tHH2CaE7INk1YR35s62CWzqPk3jU8HXlKPorFpijQs9Qs7z9Zx2gftCHNHV8Il8DxAM\n\tYmlGrYGMVxKTUm1fN7DtCh6gDkbX27/TSusbpyeupR4XGqA57nlOnOsbH3xnfdodZd\n\t0MblaFIt58mnwiVn0l2N6q+Cd9MPKb1AnE3NPBPNh7vQhIs7XmRvD/Rq6YuP+N3C4V\n\tcQeIxR3yIDaug==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1679649609;\n\tbh=KmCbervTJ0ei6yBctTviNw04TMN5w50UUk80Jtjsb3s=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=bQiJdCvj2bbDJVTrNhwhyJXudIGbR7hvryUQChGkepXWVeGq64rXHrbeNHsYOxyqy\n\tFeG5LDocyCvDRNdXoGgrzcSotA3pkNDofpOIZE5OVnw2QLFoizPEHMYgxuVeelkHJ7\n\tNQG+ESLWaTkt1LZm5nYGAo018q2Q2PJw92N634so="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"bQiJdCvj\"; dkim-atps=neutral","Date":"Fri, 24 Mar 2023 10:20:06 +0100","To":"Naushir Patuck ","Message-ID":"<20230324092006.7rhjpip2sua4ri5p@uno.localdomain>","References":"<20230322130612.5208-1-naush@raspberrypi.com>\n\t<20230322130612.5208-9-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230322130612.5208-9-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v1 08/10] ipa: raspberrypi: Generalise\n\tthe focus 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":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]