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; } } }