From patchwork Tue Sep 14 16:37:05 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jean-Michel Hautbois X-Patchwork-Id: 13850 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id AACDCBDC71 for ; Tue, 14 Sep 2021 16:37:16 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 2FB7E6918A; Tue, 14 Sep 2021 18:37:15 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="TNP9FbiU"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id BE61860249 for ; Tue, 14 Sep 2021 18:37:13 +0200 (CEST) Received: from tatooine.ideasonboard.com (unknown [IPv6:2a01:e0a:169:7140:d0a7:2575:a724:b30a]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 47DEB499; Tue, 14 Sep 2021 18:37:13 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1631637433; bh=7iFuYX5Jd9uTHPDAF6PKzCxzUB+2rJANmHloS45ic+M=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=TNP9FbiU3r0ucCR+wNkP6v4+8nnuQ+ftllD2yKnd16OUtxCqT2yPbtaa48OpovAfR gVhGpcn8KrTVQ9ttEXa1aWFWu3E9rq9GfHMhuYVfxdgEwxm6uPC3O6YVGLdVIvtCce ql51BWABgdkUAXXSIUYhX5tCmkoXStHIGcIgclkM= From: Jean-Michel Hautbois To: libcamera-devel@lists.libcamera.org Date: Tue, 14 Sep 2021 18:37:05 +0200 Message-Id: <20210914163709.85751-2-jeanmichel.hautbois@ideasonboard.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210914163709.85751-1-jeanmichel.hautbois@ideasonboard.com> References: <20210914163709.85751-1-jeanmichel.hautbois@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v8 1/5] ipa: ipu3: Move the AWB stats structures X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The structure Ipu3AwbCell describes the AWB stats layout on the kernel side. We will need it to be used by the AGC algorithm to be introduced later, so let's make it visible from ipa::ipu3::algorithms and not only for the AWB class. The IspStatsRegion will be needed by AGC too, so let's move it in the same namespace too. Signed-off-by: Jean-Michel Hautbois Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- src/ipa/ipu3/algorithms/awb.h | 37 ++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h index a16dd68d..cc848060 100644 --- a/src/ipa/ipu3/algorithms/awb.h +++ b/src/ipa/ipu3/algorithms/awb.h @@ -23,6 +23,24 @@ namespace ipa::ipu3::algorithms { static constexpr uint32_t kAwbStatsSizeX = 16; static constexpr uint32_t kAwbStatsSizeY = 12; +/* \todo Move the cell layout into intel-ipu3.h kernel header */ +struct Ipu3AwbCell { + unsigned char greenRedAvg; + unsigned char redAvg; + unsigned char blueAvg; + unsigned char greenBlueAvg; + unsigned char satRatio; + unsigned char padding[3]; +} __attribute__((packed)); + +struct IspStatsRegion { + unsigned int counted; + unsigned int uncounted; + unsigned long long rSum; + unsigned long long gSum; + unsigned long long bSum; +}; + class Awb : public Algorithm { public: @@ -32,16 +50,7 @@ public: void prepare(IPAContext &context, ipu3_uapi_params *params) override; void process(IPAContext &context, const ipu3_uapi_stats_3a *stats) override; - struct Ipu3AwbCell { - unsigned char greenRedAvg; - unsigned char redAvg; - unsigned char blueAvg; - unsigned char greenBlueAvg; - unsigned char satRatio; - unsigned char padding[3]; - } __attribute__((packed)); - - /* \todo Make these three structs available to all the ISPs ? */ + /* \todo Make these structs available to all the ISPs ? */ struct RGB { RGB(double _R = 0, double _G = 0, double _B = 0) : R(_R), G(_G), B(_B) @@ -55,14 +64,6 @@ public: } }; - struct IspStatsRegion { - unsigned int counted; - unsigned int uncounted; - unsigned long long rSum; - unsigned long long gSum; - unsigned long long bSum; - }; - struct AwbStatus { double temperatureK; double redGain; From patchwork Tue Sep 14 16:37:06 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jean-Michel Hautbois X-Patchwork-Id: 13851 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 60A51BDC71 for ; Tue, 14 Sep 2021 16:37:17 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 82FFE69194; Tue, 14 Sep 2021 18:37:15 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="W/JBLMC9"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id F159F60132 for ; Tue, 14 Sep 2021 18:37:13 +0200 (CEST) Received: from tatooine.ideasonboard.com (unknown [IPv6:2a01:e0a:169:7140:d0a7:2575:a724:b30a]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 83703B2B; Tue, 14 Sep 2021 18:37:13 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1631637433; bh=Aw4KxdC8a77fU/I47il0Yqo1rGcZWltOQiJHUg3eahI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=W/JBLMC90LagCiGDY+G7VTDP0MUuMs/oqXCNyokEgw5HzALn1iM0rXvfN3vMFesSj TUEFF4n65DngOVv3BTbgv9zT2/sMrrRt/9a3ia3a9hzHzrEu2xnIc266RAWLF/gvTZ k/CCqH/7z5viH/ufkH79P7RYIsao0fLzckEoviZg= From: Jean-Michel Hautbois To: libcamera-devel@lists.libcamera.org Date: Tue, 14 Sep 2021 18:37:06 +0200 Message-Id: <20210914163709.85751-3-jeanmichel.hautbois@ideasonboard.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210914163709.85751-1-jeanmichel.hautbois@ideasonboard.com> References: <20210914163709.85751-1-jeanmichel.hautbois@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v8 2/5] ipa: ipu3: Rename IspStatsRegion to Accumulator X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The IspStatsRegion structure was introduced as an attempt to prepare for a generic AWB algorithm structure. The structure name by itself is not explicit and it is too optimistic to try and make a generic one for now. Its role is to accumulate the pixels in a given zone. Rename it to accumulator, and remove the uncounted field at the same time. It is always possible to know how many pixels are not relevant for the algorithm by calculating total-counted. The uncounted field was only declared and not used. Amend the documentation accordingly. Signed-off-by: Jean-Michel Hautbois Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- src/ipa/ipu3/algorithms/awb.cpp | 77 +++++++++++++++++++++++++-------- src/ipa/ipu3/algorithms/awb.h | 5 +-- 2 files changed, 61 insertions(+), 21 deletions(-) diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp index e05647c9..d780a085 100644 --- a/src/ipa/ipu3/algorithms/awb.cpp +++ b/src/ipa/ipu3/algorithms/awb.cpp @@ -21,26 +21,68 @@ static constexpr uint32_t kMinZonesCounted = 16; static constexpr uint32_t kMinGreenLevelInZone = 32; /** - * \struct IspStatsRegion - * \brief RGB statistics for a given region + * \struct Accumulator + * \brief RGB statistics for a given zone * - * The IspStatsRegion structure is intended to abstract the ISP specific - * statistics and use an agnostic algorithm to compute AWB. + * - Cells are defined in Pixels + * - Zones are defined in Cells * - * \var IspStatsRegion::counted - * \brief Number of pixels used to calculate the sums + * 81 cells + * /───────────── 1296 pixels ───────────\ + * 16 zones + * 16 + * ┌────┬────┬────┬────┬────┬─ ──────┬────┐ \ + * │Cell│ │ │ │ │ | │ │ │ + * 16 │ px │ │ │ │ │ | │ │ │ + * ├────┼────┼────┼────┼────┼─ ──────┼────┤ │ + * │ │ │ │ │ │ | │ │ + * │ │ │ │ │ │ | │ │ 7 + * │ ── │ ── │ ── │ ── │ ── │ ── ── ─┤ ── │ 1 2 4 + * │ │ │ │ │ │ | │ │ 2 0 5 * - * \var IspStatsRegion::uncounted - * \brief Remaining number of pixels in the region + * │ │ │ │ │ │ | │ │ z p c + * ├────┼────┼────┼────┼────┼─ ──────┼────┤ o i e + * │ │ │ │ │ │ | │ │ n x l + * │ │ | │ │ e e l + * ├─── ───┼─ ──────┼────┤ s l s + * │ │ | │ │ s + * │ │ | │ │ + * ├─── Zone of Cells ───┼─ ──────┼────┤ │ + * │ (5 x 4) │ | │ │ │ + * │ │ | │ │ │ + * ├── ───┼─ ──────┼────┤ │ + * │ │ │ | │ │ │ + * │ │ │ │ │ │ | │ │ │ + * └────┴────┴────┴────┴────┴─ ──────┴────┘ / * - * \var IspStatsRegion::rSum - * \brief Sum of the red values in the region + * The algorithm works with a fixed number of zones \a kAwbStatsSizeX x + * \a kAwbStatsSizeY. For example, a frame of 1296x720 is divided into 81x45 + * cells of [16x16] pixels. In the case of \a kAwbStatsSizeX=16 and + * \a kAwbStatsSizeY=12 the zones are made of [5x4] cells. The cells are + * left-aligned and calculated by IPAIPU3::calculateBdsGrid(). * - * \var IspStatsRegion::gSum - * \brief Sum of the green values in the region + * Each statistics cell represents the average value of the pixels in that cell + * split by colour components. * - * \var IspStatsRegion::bSum - * \brief Sum of the blue values in the region + * The Accumulator structure stores the sum of the average of each cell in a + * zone of the image, as well as the number of cells which were unsaturated and + * therefore included in the average. + * \todo move this description and structure into a common header + * + * Cells which are saturated beyond the threshold defined in + * ipu3_uapi_awb_config_s are not included in the average. + * + * \var Accumulator::counted + * \brief Number of unsaturated cells used to calculate the sums + * + * \var Accumulator::rSum + * \brief Sum of the average red values of each unsaturated cell in the zone + * + * \var Accumulator::gSum + * \brief Sum of the average green values of each unsaturated cell in the zone + * + * \var Accumulator::bSum + * \brief Sum of the average blue values of each unsaturated cell in the zone */ /** @@ -157,7 +199,7 @@ uint32_t Awb::estimateCCT(double red, double green, double blue) return 449 * n * n * n + 3525 * n * n + 6823.3 * n + 5520.33; } -/* Generate an RGB vector with the average values for each region */ +/* Generate an RGB vector with the average values for each zone */ void Awb::generateZones(std::vector &zones) { for (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) { @@ -174,7 +216,7 @@ void Awb::generateZones(std::vector &zones) } } -/* Translate the IPU3 statistics into the default statistics region array */ +/* Translate the IPU3 statistics into the default statistics zone array */ void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats, const ipu3_uapi_grid_config &grid) { @@ -215,7 +257,6 @@ void Awb::clearAwbStats() awbStats_[i].rSum = 0; awbStats_[i].gSum = 0; awbStats_[i].counted = 0; - awbStats_[i].uncounted = 0; } } @@ -304,7 +345,7 @@ void Awb::prepare(IPAContext &context, ipu3_uapi_params *params) /* * Optical center is column start (respectively row start) of the - * region of interest minus its X center (respectively Y center). + * cell of interest minus its X center (respectively Y center). * * For the moment use BDS as a first approximation, but it should * be calculated based on Shading (SHD) parameters. diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h index cc848060..ac8ccc84 100644 --- a/src/ipa/ipu3/algorithms/awb.h +++ b/src/ipa/ipu3/algorithms/awb.h @@ -33,9 +33,8 @@ struct Ipu3AwbCell { unsigned char padding[3]; } __attribute__((packed)); -struct IspStatsRegion { +struct Accumulator { unsigned int counted; - unsigned int uncounted; unsigned long long rSum; unsigned long long gSum; unsigned long long bSum; @@ -82,7 +81,7 @@ private: uint32_t estimateCCT(double red, double green, double blue); std::vector zones_; - IspStatsRegion awbStats_[kAwbStatsSizeX * kAwbStatsSizeY]; + Accumulator awbStats_[kAwbStatsSizeX * kAwbStatsSizeY]; AwbStatus asyncResults_; }; From patchwork Tue Sep 14 16:37:07 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jean-Michel Hautbois X-Patchwork-Id: 13852 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 8538EBDC71 for ; Tue, 14 Sep 2021 16:37:18 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 49E3D69198; Tue, 14 Sep 2021 18:37:18 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="uo66ccKL"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 19AAB60249 for ; Tue, 14 Sep 2021 18:37:14 +0200 (CEST) Received: from tatooine.ideasonboard.com (unknown [IPv6:2a01:e0a:169:7140:d0a7:2575:a724:b30a]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BE0ED2A5; Tue, 14 Sep 2021 18:37:13 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1631637433; bh=h7LU+pzmjJNo6QGA2NemyFreZybReJjSpq3dRpvEcAE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=uo66ccKLcwQsGHbtThFF6XyRFuQE3eaENZ5d6+JPHzX63iGbRHthaltYZJd201RWN vUFgNOFo/G0MYxY6tr7xaBFkcIFBASkXFfe879WqcrGxOZPvRL6WFzNLHJvTwvBtP5 BVM3UvFFGorp7zolu9eOvJQ7MFFm8B/IpArO4KSM= From: Jean-Michel Hautbois To: libcamera-devel@lists.libcamera.org Date: Tue, 14 Sep 2021 18:37:07 +0200 Message-Id: <20210914163709.85751-4-jeanmichel.hautbois@ideasonboard.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210914163709.85751-1-jeanmichel.hautbois@ideasonboard.com> References: <20210914163709.85751-1-jeanmichel.hautbois@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v8 3/5] ipa: ipu3: Change Accumulator structure layout X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The pixel component sums for the Accumulator are inconsistent with other similar structures such as the IPAFrameContext::awb::gains. Group the red, green, and blue sums together in a struct and store them as uint64_t to reduce potential architectural differences. Signed-off-by: Jean-Michel Hautbois Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- src/ipa/ipu3/algorithms/awb.cpp | 24 ++++++++++++------------ src/ipa/ipu3/algorithms/awb.h | 8 +++++--- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp index d780a085..d78ae4f4 100644 --- a/src/ipa/ipu3/algorithms/awb.cpp +++ b/src/ipa/ipu3/algorithms/awb.cpp @@ -75,13 +75,13 @@ static constexpr uint32_t kMinGreenLevelInZone = 32; * \var Accumulator::counted * \brief Number of unsaturated cells used to calculate the sums * - * \var Accumulator::rSum + * \var Accumulator::sum::red * \brief Sum of the average red values of each unsaturated cell in the zone * - * \var Accumulator::gSum + * \var Accumulator::sum::green * \brief Sum of the average green values of each unsaturated cell in the zone * - * \var Accumulator::bSum + * \var Accumulator::sum::blue * \brief Sum of the average blue values of each unsaturated cell in the zone */ @@ -206,10 +206,10 @@ void Awb::generateZones(std::vector &zones) RGB zone; double counted = awbStats_[i].counted; if (counted >= kMinZonesCounted) { - zone.G = awbStats_[i].gSum / counted; + zone.G = awbStats_[i].sum.green / counted; if (zone.G >= kMinGreenLevelInZone) { - zone.R = awbStats_[i].rSum / counted; - zone.B = awbStats_[i].bSum / counted; + zone.R = awbStats_[i].sum.red / counted; + zone.B = awbStats_[i].sum.blue / counted; zones.push_back(zone); } } @@ -242,9 +242,9 @@ void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats, /* The cell is not saturated, use the current cell */ awbStats_[awbRegionPosition].counted++; uint32_t greenValue = currentCell->greenRedAvg + currentCell->greenBlueAvg; - awbStats_[awbRegionPosition].gSum += greenValue / 2; - awbStats_[awbRegionPosition].rSum += currentCell->redAvg; - awbStats_[awbRegionPosition].bSum += currentCell->blueAvg; + awbStats_[awbRegionPosition].sum.green += greenValue / 2; + awbStats_[awbRegionPosition].sum.red += currentCell->redAvg; + awbStats_[awbRegionPosition].sum.blue += currentCell->blueAvg; } } } @@ -253,9 +253,9 @@ void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats, void Awb::clearAwbStats() { for (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) { - awbStats_[i].bSum = 0; - awbStats_[i].rSum = 0; - awbStats_[i].gSum = 0; + awbStats_[i].sum.blue = 0; + awbStats_[i].sum.red = 0; + awbStats_[i].sum.green = 0; awbStats_[i].counted = 0; } } diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h index ac8ccc84..3385ebe7 100644 --- a/src/ipa/ipu3/algorithms/awb.h +++ b/src/ipa/ipu3/algorithms/awb.h @@ -35,9 +35,11 @@ struct Ipu3AwbCell { struct Accumulator { unsigned int counted; - unsigned long long rSum; - unsigned long long gSum; - unsigned long long bSum; + struct { + uint64_t red; + uint64_t green; + uint64_t blue; + } sum; }; class Awb : public Algorithm From patchwork Tue Sep 14 16:37:08 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jean-Michel Hautbois X-Patchwork-Id: 13853 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id EA984BDC71 for ; Tue, 14 Sep 2021 16:37:18 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id AE7FB69195; Tue, 14 Sep 2021 18:37:18 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="GVCgPsdk"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 4D0EA69187 for ; Tue, 14 Sep 2021 18:37:14 +0200 (CEST) Received: from tatooine.ideasonboard.com (unknown [IPv6:2a01:e0a:169:7140:d0a7:2575:a724:b30a]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 01FFE499; Tue, 14 Sep 2021 18:37:13 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1631637434; bh=4BxLt+cgpqkV6Mc6MIdLNyI53xFrhDsdk9onnuBBels=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=GVCgPsdkm7MXUcOeCedgXQBsgB2soJ3Vlg2k+7HLC32ZfmbLxJW1bQSUDngQVxdIl faWPL8ALmPHLDEzOnEaWASMFpY+GaQbxYloLX4WqRKCZXqAKBekdBd+NGMhuGqG6PU UGmBx0HDUMi4Zsgfy9fWYnW4IYpnbuOck+RiFbnI= From: Jean-Michel Hautbois To: libcamera-devel@lists.libcamera.org Date: Tue, 14 Sep 2021 18:37:08 +0200 Message-Id: <20210914163709.85751-5-jeanmichel.hautbois@ideasonboard.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210914163709.85751-1-jeanmichel.hautbois@ideasonboard.com> References: <20210914163709.85751-1-jeanmichel.hautbois@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v8 4/5] ipa: ipu3: Make the naming consistent X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The variables mix the terms cell, region and zone. It can confuse the reader, and make the algorithm more difficult to follow. Rename the local variables consistent with their definitions: - Cells are defined in Pixels - Zones are defined in Cells There is no "region" as such, so replace it with the correct term. While reading again the code for this renaming, the Agc::processBrightness() function was really too difficult to follow, and mixed pixels, cells, region of interest. The history is that AGC was meant to be done in one particular region of the input frame (the center, or anything else) but there is no need for that, and there is no easy way to define this region of interest right now. It will be better to introduce metering instead. While we are renaming and making things more consistent, let's improve Agc at the same time: - Remove the aeRegion reference, as start_x and y_start are always 0 - Use only the grid passed to the function, and not the one in the stats - rename i and j accordingly, to follow the Awb way - remove the unused variables (count, kCellSize, etc.) Signed-off-by: Jean-Michel Hautbois --- src/ipa/ipu3/algorithms/agc.cpp | 44 +++++++++------------------------ src/ipa/ipu3/algorithms/awb.cpp | 26 +++++++++---------- 2 files changed, 25 insertions(+), 45 deletions(-) diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp index 5ff50f4a..52e8015e 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -6,6 +6,7 @@ */ #include "agc.h" +#include "awb.h" #include #include @@ -47,9 +48,6 @@ static constexpr uint32_t kMaxExposure = 1976; static constexpr uint32_t knumHistogramBins = 256; static constexpr double kEvGainTarget = 0.5; -/* A cell is 8 bytes and contains averages for RGB values and saturation ratio */ -static constexpr uint8_t kCellSize = 8; - Agc::Agc() : frameCount_(0), lastFrame_(0), iqMean_(0.0), lineDuration_(0s), maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s), @@ -70,37 +68,19 @@ int Agc::configure([[maybe_unused]] IPAContext &context, void Agc::processBrightness(const ipu3_uapi_stats_3a *stats, const ipu3_uapi_grid_config &grid) { - const struct ipu3_uapi_grid_config statsAeGrid = stats->stats_4a_config.awb_config.grid; - Rectangle aeRegion = { statsAeGrid.x_start, - statsAeGrid.y_start, - static_cast(statsAeGrid.x_end - statsAeGrid.x_start) + 1, - static_cast(statsAeGrid.y_end - statsAeGrid.y_start) + 1 }; - Point topleft = aeRegion.topLeft(); - int topleftX = topleft.x >> grid.block_width_log2; - int topleftY = topleft.y >> grid.block_height_log2; - - /* Align to the grid cell width and height */ - uint32_t startX = topleftX << grid.block_width_log2; - uint32_t startY = topleftY * grid.width << grid.block_width_log2; - uint32_t endX = (startX + (aeRegion.size().width >> grid.block_width_log2)) << grid.block_width_log2; - uint32_t i, j; - uint32_t count = 0; - uint32_t hist[knumHistogramBins] = { 0 }; - for (j = topleftY; - j < topleftY + (aeRegion.size().height >> grid.block_height_log2); - j++) { - for (i = startX + startY; i < endX + startY; i += kCellSize) { - /* - * The grid width (and maybe height) is not reliable. - * We observed a bit shift which makes the value 160 to be 32 in the stats grid. - * Use the one passed at init time. - */ - if (stats->awb_raw_buffer.meta_data[i + 4 + j * grid.width] == 0) { - uint8_t Gr = stats->awb_raw_buffer.meta_data[i + 0 + j * grid.width]; - uint8_t Gb = stats->awb_raw_buffer.meta_data[i + 3 + j * grid.width]; + + for (unsigned int cellY = 0; cellY < grid.height; cellY++) { + for (unsigned int cellX = 0; cellX < grid.width; cellX++) { + uint32_t cellPosition = (cellY * grid.width + cellX) + * sizeof(Ipu3AwbCell); + + /* Cast the initial IPU3 structure to simplify the reading */ + Ipu3AwbCell *currentCell = reinterpret_cast(const_cast(&stats->awb_raw_buffer.meta_data[cellPosition])); + if (currentCell->satRatio == 0) { + uint8_t Gr = currentCell->greenRedAvg; + uint8_t Gb = currentCell->greenBlueAvg; hist[(Gr + Gb) / 2]++; - count++; } } } diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp index d78ae4f4..b96a6ebf 100644 --- a/src/ipa/ipu3/algorithms/awb.cpp +++ b/src/ipa/ipu3/algorithms/awb.cpp @@ -220,31 +220,31 @@ void Awb::generateZones(std::vector &zones) void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats, const ipu3_uapi_grid_config &grid) { - uint32_t regionWidth = round(grid.width / static_cast(kAwbStatsSizeX)); - uint32_t regionHeight = round(grid.height / static_cast(kAwbStatsSizeY)); + uint32_t cellsPerZoneX = round(grid.width / static_cast(kAwbStatsSizeX)); + uint32_t cellsPerZoneY = round(grid.height / static_cast(kAwbStatsSizeY)); /* * Generate a (kAwbStatsSizeX x kAwbStatsSizeY) array from the IPU3 grid which is * (grid.width x grid.height). */ - for (unsigned int j = 0; j < kAwbStatsSizeY * regionHeight; j++) { - for (unsigned int i = 0; i < kAwbStatsSizeX * regionWidth; i++) { - uint32_t cellPosition = j * grid.width + i; - uint32_t cellX = (cellPosition / regionWidth) % kAwbStatsSizeX; - uint32_t cellY = ((cellPosition / grid.width) / regionHeight) % kAwbStatsSizeY; + for (unsigned int cellY = 0; cellY < kAwbStatsSizeY * cellsPerZoneY; cellY++) { + for (unsigned int cellX = 0; cellX < kAwbStatsSizeX * cellsPerZoneX; cellX++) { + uint32_t cellPosition = (cellY * grid.width + cellX) + * sizeof(Ipu3AwbCell); + uint32_t zoneX = cellX / cellsPerZoneX; + uint32_t zoneY = cellY / cellsPerZoneY; - uint32_t awbRegionPosition = cellY * kAwbStatsSizeX + cellX; - cellPosition *= 8; + uint32_t awbZonePosition = zoneY * kAwbStatsSizeX + zoneX; /* Cast the initial IPU3 structure to simplify the reading */ Ipu3AwbCell *currentCell = reinterpret_cast(const_cast(&stats->awb_raw_buffer.meta_data[cellPosition])); if (currentCell->satRatio == 0) { /* The cell is not saturated, use the current cell */ - awbStats_[awbRegionPosition].counted++; + awbStats_[awbZonePosition].counted++; uint32_t greenValue = currentCell->greenRedAvg + currentCell->greenBlueAvg; - awbStats_[awbRegionPosition].sum.green += greenValue / 2; - awbStats_[awbRegionPosition].sum.red += currentCell->redAvg; - awbStats_[awbRegionPosition].sum.blue += currentCell->blueAvg; + awbStats_[awbZonePosition].sum.green += greenValue / 2; + awbStats_[awbZonePosition].sum.red += currentCell->redAvg; + awbStats_[awbZonePosition].sum.blue += currentCell->blueAvg; } } } From patchwork Tue Sep 14 16:37:09 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jean-Michel Hautbois X-Patchwork-Id: 13854 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 7D333BDC71 for ; Tue, 14 Sep 2021 16:37:19 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3DB4669190; Tue, 14 Sep 2021 18:37:19 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="KqQEGNJA"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9226A6918C for ; Tue, 14 Sep 2021 18:37:14 +0200 (CEST) Received: from tatooine.ideasonboard.com (unknown [IPv6:2a01:e0a:169:7140:d0a7:2575:a724:b30a]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 2EF11B2B; Tue, 14 Sep 2021 18:37:14 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1631637434; bh=PfXdmT0MSqrPPNRY1GQ4M39ygt+/+1cczX+dci+mllc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=KqQEGNJAwue4QsWaT1aUlKLa4feVBF57WNitFa3SC2fL2l2C45lJyGYwnCQ74YYQH GO5eNXVxdEqQk+hXH0PSwvvVKarL0Lrk9w6BR6FYsS99jjfe4mh/yAsA8yS5iuNwD+ srgakolV15jlni9nNBjmlylpofqHE322kiikwb8w= From: Jean-Michel Hautbois To: libcamera-devel@lists.libcamera.org Date: Tue, 14 Sep 2021 18:37:09 +0200 Message-Id: <20210914163709.85751-6-jeanmichel.hautbois@ideasonboard.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210914163709.85751-1-jeanmichel.hautbois@ideasonboard.com> References: <20210914163709.85751-1-jeanmichel.hautbois@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v8 5/5] ipa: ipu3: Replace ipa::ipu3::algorithms::Ipu3AwbCell X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The intel-ipu3.h public interface from the kernel does not define how to parse the statistics for a cell. This had to be identified by a process of reverse engineering, and later identifying the structures from [0] leading to our custom definition of struct Ipu3AwbCell. [0] https://chromium.googlesource.com/chromiumos/platform/arc-camera/+/refs/heads/master/hal/intel/include/ia_imaging/awb_public.h To improve the kernel interface, a proposal has been made to the linux-kernel [1] to incorporate the memory layout for each cell into the intel-ipu3 header directly. [1] https://lore.kernel.org/linux-media/20210831185140.77400-1-jeanmichel.hautbois@ideasonboard.com/ Update our local copy of the intel-ipu3.h to match the proposal and change the AGC and AWB algorithms to reference that structure directly, allowing us to remove the deprecated custom Ipu3AwbCell definition. Signed-off-by: Jean-Michel Hautbois Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- include/linux/intel-ipu3.h | 28 ++++++++++++++++++++++++++-- src/ipa/ipu3/algorithms/agc.cpp | 11 +++++------ src/ipa/ipu3/algorithms/awb.cpp | 13 ++++++------- src/ipa/ipu3/algorithms/awb.h | 10 ---------- 4 files changed, 37 insertions(+), 25 deletions(-) diff --git a/include/linux/intel-ipu3.h b/include/linux/intel-ipu3.h index ee0e6d0e..a9de8c11 100644 --- a/include/linux/intel-ipu3.h +++ b/include/linux/intel-ipu3.h @@ -59,6 +59,29 @@ struct ipu3_uapi_grid_config { __u16 y_end; } __attribute__((packed)); +/** + * struct ipu3_uapi_awb_set_item - Memory layout for each cell in AWB + * + * @Gr_avg: Green average for red lines in the cell. + * @R_avg: Red average in the cell. + * @B_avg: Blue average in the cell. + * @Gb_avg: Green average for blue lines in the cell. + * @sat_ratio: Saturation ratio in the cell. + * @padding0: Unused byte for padding. + * @padding1: Unused byte for padding. + * @padding2: Unused byte for padding. + */ +struct ipu3_uapi_awb_set_item { + unsigned char Gr_avg; + unsigned char R_avg; + unsigned char B_avg; + unsigned char Gb_avg; + unsigned char sat_ratio; + unsigned char padding0; + unsigned char padding1; + unsigned char padding2; +} __attribute__((packed)); + /* * The grid based data is divided into "slices" called set, each slice of setX * refers to ipu3_uapi_grid_config width * height_per_slice. @@ -72,7 +95,8 @@ struct ipu3_uapi_grid_config { IPU3_UAPI_AWB_MD_ITEM_SIZE) #define IPU3_UAPI_AWB_MAX_BUFFER_SIZE \ (IPU3_UAPI_AWB_MAX_SETS * \ - (IPU3_UAPI_AWB_SET_SIZE + IPU3_UAPI_AWB_SPARE_FOR_BUBBLES)) + (IPU3_UAPI_AWB_SET_SIZE + IPU3_UAPI_AWB_SPARE_FOR_BUBBLES)) / \ + sizeof(ipu3_uapi_awb_set_item) /** @@ -82,7 +106,7 @@ struct ipu3_uapi_grid_config { * the average values for each color channel. */ struct ipu3_uapi_awb_raw_buffer { - __u8 meta_data[IPU3_UAPI_AWB_MAX_BUFFER_SIZE] + struct ipu3_uapi_awb_set_item meta_data[IPU3_UAPI_AWB_MAX_BUFFER_SIZE] __attribute__((aligned(32))); } __attribute__((packed)); diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp index 52e8015e..a3e910b7 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -72,14 +72,13 @@ void Agc::processBrightness(const ipu3_uapi_stats_3a *stats, for (unsigned int cellY = 0; cellY < grid.height; cellY++) { for (unsigned int cellX = 0; cellX < grid.width; cellX++) { - uint32_t cellPosition = (cellY * grid.width + cellX) - * sizeof(Ipu3AwbCell); + uint32_t cellPosition = cellY * grid.width + cellX; /* Cast the initial IPU3 structure to simplify the reading */ - Ipu3AwbCell *currentCell = reinterpret_cast(const_cast(&stats->awb_raw_buffer.meta_data[cellPosition])); - if (currentCell->satRatio == 0) { - uint8_t Gr = currentCell->greenRedAvg; - uint8_t Gb = currentCell->greenBlueAvg; + const ipu3_uapi_awb_set_item *currentCell = &stats->awb_raw_buffer.meta_data[cellPosition]; + if (currentCell->sat_ratio == 0) { + uint8_t Gr = currentCell->Gr_avg; + uint8_t Gb = currentCell->Gb_avg; hist[(Gr + Gb) / 2]++; } } diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp index b96a6ebf..6d8438af 100644 --- a/src/ipa/ipu3/algorithms/awb.cpp +++ b/src/ipa/ipu3/algorithms/awb.cpp @@ -229,22 +229,21 @@ void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats, */ for (unsigned int cellY = 0; cellY < kAwbStatsSizeY * cellsPerZoneY; cellY++) { for (unsigned int cellX = 0; cellX < kAwbStatsSizeX * cellsPerZoneX; cellX++) { - uint32_t cellPosition = (cellY * grid.width + cellX) - * sizeof(Ipu3AwbCell); + uint32_t cellPosition = cellY * grid.width + cellX; uint32_t zoneX = cellX / cellsPerZoneX; uint32_t zoneY = cellY / cellsPerZoneY; uint32_t awbZonePosition = zoneY * kAwbStatsSizeX + zoneX; /* Cast the initial IPU3 structure to simplify the reading */ - Ipu3AwbCell *currentCell = reinterpret_cast(const_cast(&stats->awb_raw_buffer.meta_data[cellPosition])); - if (currentCell->satRatio == 0) { + const ipu3_uapi_awb_set_item *currentCell = &stats->awb_raw_buffer.meta_data[cellPosition]; + if (currentCell->sat_ratio == 0) { /* The cell is not saturated, use the current cell */ awbStats_[awbZonePosition].counted++; - uint32_t greenValue = currentCell->greenRedAvg + currentCell->greenBlueAvg; + uint32_t greenValue = currentCell->Gr_avg + currentCell->Gb_avg; awbStats_[awbZonePosition].sum.green += greenValue / 2; - awbStats_[awbZonePosition].sum.red += currentCell->redAvg; - awbStats_[awbZonePosition].sum.blue += currentCell->blueAvg; + awbStats_[awbZonePosition].sum.red += currentCell->R_avg; + awbStats_[awbZonePosition].sum.blue += currentCell->B_avg; } } } diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h index 3385ebe7..17905ae8 100644 --- a/src/ipa/ipu3/algorithms/awb.h +++ b/src/ipa/ipu3/algorithms/awb.h @@ -23,16 +23,6 @@ namespace ipa::ipu3::algorithms { static constexpr uint32_t kAwbStatsSizeX = 16; static constexpr uint32_t kAwbStatsSizeY = 12; -/* \todo Move the cell layout into intel-ipu3.h kernel header */ -struct Ipu3AwbCell { - unsigned char greenRedAvg; - unsigned char redAvg; - unsigned char blueAvg; - unsigned char greenBlueAvg; - unsigned char satRatio; - unsigned char padding[3]; -} __attribute__((packed)); - struct Accumulator { unsigned int counted; struct {