[{"id":22379,"web_url":"https://patchwork.libcamera.org/comment/22379/","msgid":"","date":"2022-03-23T01:19:08","subject":"Re: [libcamera-devel] [PATCH 1/3] ipa: ipu3: af: Move constants to\n\timplementation","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Tue, Mar 15, 2022 at 05:53:43PM +0000, Kieran Bingham via libcamera-devel wrote:\n> A selection of constants are imported from ChromiumOS.\n> \n> Move these out of the header, and simplify their documentation. Further\n> more, add a direct reference to the location they were obtained from.\n> \n> Signed-off-by: Kieran Bingham \n> ---\n> src/ipa/ipu3/algorithms/af.cpp | 73 ++++++++++------------------------\n> src/ipa/ipu3/algorithms/af.h | 11 -----\n> 2 files changed, 22 insertions(+), 62 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp\n> index 0170a3728892..3cedf8cbd687 100644\n> --- a/src/ipa/ipu3/algorithms/af.cpp\n> +++ b/src/ipa/ipu3/algorithms/af.cpp\n> @@ -29,59 +29,30 @@\n> * \\file af.h\n> */\n> \n> -/**\n> - * \\var kAfMinGridWidth\n> - * \\brief the minimum width of AF grid.\n> - * The minimum grid horizontal dimensions.\n> - */\n> -\n> -/**\n> - * \\var kAfMinGridHeight\n> - * \\brief the minimum height of AF grid.\n> - * The minimum grid vertical dimensions.\n> - */\n> -\n> -/**\n> - * \\var kAfMaxGridWidth\n> - * \\brief the maximum width of AF grid.\n> - * The maximum grid horizontal dimensions.\n> - */\n> -\n> -/**\n> - * \\var kAfMaxGridHeight\n> - * \\brief The maximum height of AF grid.\n> - * The maximum grid vertical dimensions.\n> - */\n> -\n> -/**\n> - * \\var kAfMinGridBlockWidth\n> - * \\brief The minimum block size of the width.\n> - * The minimum value of Log2 of the width of the grid cell.\n> - */\n> -\n> -/**\n> - * \\var kAfMinGridBlockHeight\n> - * \\brief The minimum block size of the height.\n> - * The minimum value of Log2 of the height of the grid cell.\n> - */\n> -\n> -/**\n> - * \\def kAfMaxGridBlockWidth\n> - * \\brief The maximum block size of the width.\n> - * The maximum value of Log2 of the width of the grid cell.\n> - */\n> -\n> -/**\n> - * \\var kAfMaxGridBlockHeight\n> - * \\brief The maximum block size of the height.\n> - * The maximum value of Log2 of the height of the grid cell.\n> +/*\n> + * Static variables from ChromiumOS Intel Camera HAL and ia_imaging library:\n> + * - https://chromium.googlesource.com/chromiumos/platform/arc-camera/+/master/hal/intel/psl/ipu3/statsConverter/ipu3-stats.h\n> + * - https://chromium.googlesource.com/chromiumos/platform/camera/+/refs/heads/main/hal/intel/ipu3/include/ia_imaging/af_public.h\n> */\n> \n> -/**\n> - * \\var kAfDefaultHeightPerSlice\n> - * \\brief The default number of blocks in vertical axis per slice.\n> - * The number of blocks in vertical axis per slice.\n> - */\n> +/** The minimum horizontal grid dimensions. */\n\nShould it be s/dimensions/dimension/ ? Same below.\n\nReviewed-by: Laurent Pinchart \n\n> +static constexpr uint8_t kAfMinGridWidth = 16;\n> +/** The minimum vertical grid dimensions. */\n> +static constexpr uint8_t kAfMinGridHeight = 16;\n> +/** The maximum horizontal grid dimensions. */\n> +static constexpr uint8_t kAfMaxGridWidth = 32;\n> +/** The maximum vertical grid dimensions. */\n> +static constexpr uint8_t kAfMaxGridHeight = 24;\n> +/** The minimum value of Log2 of the width of the grid cell. */\n> +static constexpr uint16_t kAfMinGridBlockWidth = 4;\n> +/** The minimum value of Log2 of the height of the grid cell. */\n> +static constexpr uint16_t kAfMinGridBlockHeight = 3;\n> +/** The maximum value of Log2 of the width of the grid cell. */\n> +static constexpr uint16_t kAfMaxGridBlockWidth = 6;\n> +/** The maximum value of Log2 of the height of the grid cell. */\n> +static constexpr uint16_t kAfMaxGridBlockHeight = 6;\n> +/** The number of blocks in vertical axis per slice. */\n> +static constexpr uint16_t kAfDefaultHeightPerSlice = 2;\n> \n> namespace libcamera {\n> \n> diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h\n> index 13c7e0e877fd..906f2843dd49 100644\n> --- a/src/ipa/ipu3/algorithms/af.h\n> +++ b/src/ipa/ipu3/algorithms/af.h\n> @@ -15,17 +15,6 @@\n> \n> #include \"algorithm.h\"\n> \n> -/* Static variables from repo of chromium */\n> -static constexpr uint8_t kAfMinGridWidth = 16;\n> -static constexpr uint8_t kAfMinGridHeight = 16;\n> -static constexpr uint8_t kAfMaxGridWidth = 32;\n> -static constexpr uint8_t kAfMaxGridHeight = 24;\n> -static constexpr uint16_t kAfMinGridBlockWidth = 4;\n> -static constexpr uint16_t kAfMinGridBlockHeight = 3;\n> -static constexpr uint16_t kAfMaxGridBlockWidth = 6;\n> -static constexpr uint16_t kAfMaxGridBlockHeight = 6;\n> -static constexpr uint16_t kAfDefaultHeightPerSlice = 2;\n> -\n> namespace libcamera {\n> \n> namespace ipa::ipu3::algorithms {","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 4DD90BD80A\n\tfor ;\n\tWed, 23 Mar 2022 01:19:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 84751604E6;\n\tWed, 23 Mar 2022 02:19:30 +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 D5605604C6\n\tfor ;\n\tWed, 23 Mar 2022 02:19:28 +0100 (CET)","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 A369C9DE;\n\tWed, 23 Mar 2022 02:19:27 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1647998370;\n\tbh=7TKuiJLmraWTBdKGHCyEObLC28QJF+QpESQwVjAkuGU=;\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=S5obfDNqUsZVDZj7GzooHNZqFb/0+0lTeMw+i9BaZID0J92ofo8qV6dU8lN6DsdV8\n\t1Vfx7MnH8m41Nmt+WVsQAQAfLHOIbSxU91yT2lJwpBT4y+1z2Q783KJg1Hz7yas6MW\n\ts7rWDQyy9rI6zMJaSnKJFmVpFs92JX8CiiAmmgVIhVclmKvytQ9lQLV242viIyrsGt\n\tQ5HOIJUne88rh9EBUm4jnMTLRtzmxMyFIiDVByVsr/S9Zcr+EgLowgCRNimKEKxbLu\n\tdU3B/BCeNeaYWRF8Kwyz7roQ7kMdUH22tTNberY1bf2WQA+SM7jH7AfN41wVRca1ys\n\tPl15ThXU/+lSQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1647998368;\n\tbh=7TKuiJLmraWTBdKGHCyEObLC28QJF+QpESQwVjAkuGU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=vXHjgBDMLflzClCVNR2RH3PIMVZ0wThuERxvrSsznKGN/8C/F550kQlVu1k2NsZYt\n\tvCnB9qfL6JZnPZJySp1J2XHuPoofO8rm5kUEqOfxKdJbNt2i3KB8ZVWsgWWE3qRtsk\n\tIazDPVbIab7BBXDZ68tzfcv0VdVemaPTHCJFgc5Q="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"vXHjgBDM\"; dkim-atps=neutral","Date":"Wed, 23 Mar 2022 03:19:08 +0200","To":"Kieran Bingham ","Message-ID":"","References":"<20220315175345.479951-1-kieran.bingham@ideasonboard.com>\n\t<20220315175345.479951-2-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220315175345.479951-2-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/3] ipa: ipu3: af: Move constants to\n\timplementation","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\" "}}]