[libcamera-devel] [PATCH v8 4/5] ipa: ipu3: Make the naming consistent

Jean-Michel Hautbois jeanmichel.hautbois at ideasonboard.com
Tue Sep 14 18:37:08 CEST 2021


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 <jeanmichel.hautbois at ideasonboard.com>
---
 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 <algorithm>
 #include <chrono>
@@ -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<unsigned int>(statsAeGrid.x_end - statsAeGrid.x_start) + 1,
-			       static_cast<unsigned int>(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<Ipu3AwbCell *>(const_cast<uint8_t *>(&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<RGB> &zones)
 void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats,
 			   const ipu3_uapi_grid_config &grid)
 {
-	uint32_t regionWidth = round(grid.width / static_cast<double>(kAwbStatsSizeX));
-	uint32_t regionHeight = round(grid.height / static_cast<double>(kAwbStatsSizeY));
+	uint32_t cellsPerZoneX = round(grid.width / static_cast<double>(kAwbStatsSizeX));
+	uint32_t cellsPerZoneY = round(grid.height / static_cast<double>(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<Ipu3AwbCell *>(const_cast<uint8_t *>(&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;
 			}
 		}
 	}
-- 
2.30.2



More information about the libcamera-devel mailing list