[{"id":25876,"web_url":"https://patchwork.libcamera.org/comment/25876/","msgid":"","date":"2022-11-23T13:32:29","subject":"Re: [libcamera-devel] [PATCH v1 1/5] ipa: raspberrypi: Generalise\n\tstatistics","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 this patch.\n\nOn Tue, 22 Nov 2022 at 11:22, Naushir Patuck via libcamera-devel\n wrote:\n>\n> At present, the controller algorithms access the bcm2835_isp_stats structure,\n> which is hardware specific. It would be desirable to abstract out the statistics\n> structure to remove hardware specific headers from the algorithms source files.\n>\n> Define a new templated RegionStats class that encompasses region based\n> statistics generated by the ISP. For the VC4 ISP, this can be used to hold\n> RGB sums and focus FoM values.\n>\n> Define a new Statistics structure that holds all the VC4 ISP statistics output.\n> This includes AGC hostograms, AGC/AWB region sums and focus FoM regions.\n>\n> Signed-off-by: Naushir Patuck \n\nAll looks good to me.\n\nReviewed-by: David Plowman \n\nThanks!\nDavid\n\n> ---\n> src/ipa/raspberrypi/controller/region_stats.h | 109 ++++++++++++++++++\n> src/ipa/raspberrypi/statistics.h | 70 +++++++++++\n> 2 files changed, 179 insertions(+)\n> create mode 100644 src/ipa/raspberrypi/controller/region_stats.h\n> create mode 100644 src/ipa/raspberrypi/statistics.h\n>\n> diff --git a/src/ipa/raspberrypi/controller/region_stats.h b/src/ipa/raspberrypi/controller/region_stats.h\n> new file mode 100644\n> index 000000000000..8a3453617654\n> --- /dev/null\n> +++ b/src/ipa/raspberrypi/controller/region_stats.h\n> @@ -0,0 +1,109 @@\n> +/* SPDX-License-Identifier: BSD-2-Clause */\n> +/*\n> + * Copyright (C) 2022, Raspberry Pi Ltd\n> + *\n> + * region_stats.h - Raspberry Pi region based statistics container\n> + */\n> +#pragma once\n> +\n> +#include \n> +#include \n> +#include \n> +\n> +namespace RPiController {\n> +\n> +template\n> +class RegionStats\n> +{\n> +public:\n> + RegionStats()\n> + : numRows_(0), numCols_(0), numFloating_(0)\n> + {\n> + }\n> +\n> + void init(unsigned int numRows, unsigned int numCols = 1, unsigned int numFloating = 0)\n> + {\n> + numRows_ = numRows;\n> + numCols_ = numCols;\n> + numFloating_ = numFloating;\n> + regions_.resize(numRows_ * numCols_ + numFloating_);\n> + }\n> +\n> + unsigned int numRegions() const\n> + {\n> + return numRows_ * numCols_;\n> + }\n> +\n> + unsigned int numFloatingRegions() const\n> + {\n> + return numFloating_;\n> + }\n> +\n> + void set(unsigned int index, const T &value, uint32_t counted, uint32_t uncounted)\n> + {\n> + if (index >= numRegions())\n> + return;\n> + set_(index, value, counted, uncounted);\n> + }\n> +\n> + void set(unsigned int row, unsigned int col, const T &value, uint32_t counted, uint32_t uncounted)\n> + {\n> + set(row * numCols_ + col, value, counted, uncounted);\n> + }\n> +\n> + void setFloating(unsigned int index, const T &value, uint32_t counted, uint32_t uncounted)\n> + {\n> + if (index >= numFloatingRegions())\n> + return;\n> + set(numRegions() + index, value, counted, uncounted);\n> + }\n> +\n> + const T &get(unsigned int index, uint32_t &counted, uint32_t &uncounted) const\n> + {\n> + counted = uncounted = 0;\n> + if (index >= numRegions())\n> + return default_;\n> + return get_(index, counted, uncounted);\n> + }\n> +\n> + const T &get(unsigned int row, unsigned int col, uint32_t &counted, uint32_t &uncounted) const\n> + {\n> + return get(row * numCols_ + col, counted, uncounted);\n> + }\n> +\n> + const T &getFloating(unsigned int index, uint32_t &counted, uint32_t &uncounted) const\n> + {\n> + counted = uncounted = 0;\n> + if (index >= numFloatingRegions())\n> + return default_;\n> + return get_(numRegions() + index, counted, uncounted);\n> + }\n> +\n> +private:\n> + void set_(unsigned int index, const T &value, uint32_t counted, uint32_t uncounted)\n> + {\n> + regions_[index] = { value, counted, uncounted };\n> + }\n> +\n> + const T &get_(unsigned int index, uint32_t &counted, uint32_t &uncounted) const\n> + {\n> + counted = regions_[index].counted;\n> + uncounted = regions_[index].uncounted;\n> + return regions_[index].value;\n> + }\n> +\n> + unsigned int numRows_;\n> + unsigned int numCols_;\n> + unsigned int numFloating_;\n> +\n> + struct Region {\n> + T value;\n> + uint32_t counted;\n> + uint32_t uncounted;\n> + };\n> +\n> + std::vector regions_;\n> + T default_;\n> +};\n> +\n> +} /* namespace RPiController */\n> diff --git a/src/ipa/raspberrypi/statistics.h b/src/ipa/raspberrypi/statistics.h\n> new file mode 100644\n> index 000000000000..a762bf3d41aa\n> --- /dev/null\n> +++ b/src/ipa/raspberrypi/statistics.h\n> @@ -0,0 +1,70 @@\n> +/* SPDX-License-Identifier: BSD-2-Clause */\n> +/*\n> + * Copyright (C) 2022, Raspberry Pi Ltd\n> + *\n> + * statistics.h - Raspberry Pi generic statistics structure\n> + */\n> +#pragma once\n> +\n> +#include \n> +#include \n> +#include \n> +\n> +#include \"histogram.h\"\n> +#include \"region_stats.h\"\n> +\n> +namespace RPiController {\n> +\n> +struct RgbySums {\n> + RgbySums(uint64_t _rSum = 0, uint64_t _gSum = 0, uint64_t _bSum = 0, uint64_t _ySum = 0)\n> + : rSum(_rSum), gSum(_gSum), bSum(_bSum), ySum(_ySum)\n> + {\n> + }\n> + uint64_t rSum;\n> + uint64_t gSum;\n> + uint64_t bSum;\n> + uint64_t ySum;\n> +};\n> +\n> +using RgbyRegions = RegionStats;\n> +using FocusRegions = RegionStats;\n> +\n> +struct Statistics {\n> + /*\n> + * Positioning of the AGC statistics gathering in the pipeline:\n> + * Pre-WB correction or post-WB correction.\n> + * Assume this is post-LSC.\n> + */\n> + enum class AgcStatsPos { PreWb, PostWb };\n> + const AgcStatsPos agcStatsPos;\n> +\n> + /*\n> + * Positioning of the AWB/ALSC statistics gathering in the pipeline:\n> + * Pre-LSC or post-LSC.\n> + */\n> + enum class ColourStatsPos { PreLsc, PostLsc };\n> + const ColourStatsPos colourStatsPos;\n> +\n> + Statistics(AgcStatsPos a, ColourStatsPos c)\n> + : agcStatsPos(a), colourStatsPos(c)\n> + {\n> + }\n> +\n> + /* Histogram statistics. Not all histograms may be populated! */\n> + Histogram rHist;\n> + Histogram gHist;\n> + Histogram bHist;\n> + Histogram yHist;\n> +\n> + /* Row sums for flicker avoidance. */\n> + std::vector rowSums;\n> +\n> + /* Region based colour sums. */\n> + RgbyRegions agcRegions;\n> + RgbyRegions awbRegions;\n> +\n> + /* Region based focus FoM. */\n> + FocusRegions focusRegions;\n> +};\n> +\n> +} /* namespace RPiController */\n> --\n> 2.25.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 54283BE08B\n\tfor ;\n\tWed, 23 Nov 2022 13:32:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C618463313;\n\tWed, 23 Nov 2022 14:32:48 +0100 (CET)","from mail-pg1-x52e.google.com (mail-pg1-x52e.google.com\n\t[IPv6:2607:f8b0:4864:20::52e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ABECB63311\n\tfor ;\n\tWed, 23 Nov 2022 14:32:41 +0100 (CET)","by mail-pg1-x52e.google.com with SMTP id s196so16793163pgs.3\n\tfor ;\n\tWed, 23 Nov 2022 05:32:41 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669210368;\n\tbh=mP4S6eO7NOgDoo2WGNPRrKcSTTgnIH5aCQrkwWy0ROc=;\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=DSVg6guNHrcxHY5Qy8HOS4DbI0r5VJ9wMVeXRkdAMuWVJXCjpPPnevSnE0S+qjdJr\n\tppqBC/AoM5pjyLM3CZK5auUgRpjlH5tfcm4pg7L5NI65tT2VQOrlJddEq+TgGxBduw\n\tAecW0iP/JbfWaVYDboYHIFyFDK9dfigB98qpDzReh5+FS8K8ITRQ0T/DHIMJYQmNi5\n\t/kNd36vYcQO7iH1NaJjgzC+9y+jW2zV9nEx2QeSN5X2uK4nGApecZwtqM1VSvRc4EP\n\tYimSbRZrpU9WMr6w5KLZLgqPjlJEboEHKPZsyqt459mkntvHoNniJEQfGSpmuUE0qj\n\tlI7iK1R+yUWdg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\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=+zDWJoI+pGaVMp4j8OcCI8msZzLzneREGMe88XtfrpQ=;\n\tb=gdnzxfnqFBvtqddHHLaKOtXsxW8Sy5WYQDhKbxDYyITlQolSZvWXj+EuxTSJi6aNud\n\tPtBjnE+3rE2mS1EjhB5zke82jGxMCoTJedR86LCZiulZyvQUYIypAIjuB7BvELwLMHpI\n\t37bIHGmas2SbNvj5MV8JvqBoXTgcxCWaQUtHqkHdn3yM1ONw5RgL5X4QlUZK9e69vXMr\n\t4mN8dvV9CF1loKMCW08RJeCvmIDlHxM7a32SO3QiwWjLXqSnK4rSWulkyuK3bzkyw+8a\n\tg20R+lSNlBeApsRfpCrjV5Jn9bstWC67T5iA6R5+mzRKKJeoW3ggCAUGSjPOsvMBXaOp\n\tp9kg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"gdnzxfnq\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\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=+zDWJoI+pGaVMp4j8OcCI8msZzLzneREGMe88XtfrpQ=;\n\tb=vOhXgfLlgpyxI5M4GYqTKOlyzSM938K3iP55wWE/GUjKmzZONttVHW7tgZXU057laR\n\tS2lYTHkbPayFtqePn2CSzHm/nnZn4q/y/GnDI0IacdFLutkwiuqXmPkwaSixyTAeBihT\n\tc6Q/Kt/+Jh6KCV1JtoyXzgAJMVN/q1Tf9cvf0N6U8/JCIIoxvIxQGUMlWYqvS9acn/W6\n\tEiUq28Vmq54fPd+ZJKo6Jg+lA24isOkL4Xo07TkMvKl+8HhI3afvdW+l6FxW60Dh1/fl\n\tfMoDbpfPBe94YWJdiiLi63ME4P+1DI5uafkXLIZI8eneVpklcGk0s3gZJHXDf3fmWjNm\n\tO/IA==","X-Gm-Message-State":"ANoB5pkIMqTkKstApRBN45mpRrdLFzIr0wGRwCiWwPFFlu9vDyqmjwyP\n\tF81y5c6dfYu/w7gqOB5VlSW/Weds9K4ppcZrwHkm7IS5ZRI=","X-Google-Smtp-Source":"AA0mqf5vtoDFhzkt9dBkJ/jEDPS10NiUtWFnTv9zj5ktTOkUTwgQ6la88omnCjW6su8A7BEZjw03vXuiq7eQIRuFItY=","X-Received":"by 2002:a62:cfc1:0:b0:573:20a7:4 with SMTP id\n\tb184-20020a62cfc1000000b0057320a70004mr8987244pfg.78.1669210360134; \n\tWed, 23 Nov 2022 05:32:40 -0800 (PST)","MIME-Version":"1.0","References":"<20221122112224.31691-1-naush@raspberrypi.com>\n\t<20221122112224.31691-2-naush@raspberrypi.com>","In-Reply-To":"<20221122112224.31691-2-naush@raspberrypi.com>","Date":"Wed, 23 Nov 2022 13:32:29 +0000","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v1 1/5] ipa: raspberrypi: Generalise\n\tstatistics","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\" "}}]