[libcamera-devel] [PATCH v2 02/17] libcamera: ipu3: Centralize ImgU sizes definition

Jacopo Mondi jacopo at jmondi.org
Tue Sep 7 21:40:52 CEST 2021


The definition of several constants that describe the ImgU
characteristics are spread between two files: ipu3.cpp and imgu.cpp.

As the ipu3.cpp uses definitions from the imgu.cpp one, in order to
remove the usage of magic numbers, it is required to move the
definitions to a common header file where they are accessible to the
other .cpp modules.

Move all the definitions of the ImgU sizes and alignment in a dedicated
namespace in imgu.h and update their users accordingly.

Cosmetic changes, no functional changes intended.

Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
---
 src/libcamera/pipeline/ipu3/imgu.cpp | 86 +++++++++++-----------------
 src/libcamera/pipeline/ipu3/imgu.h   | 27 +++++++++
 src/libcamera/pipeline/ipu3/ipu3.cpp | 47 +++++++--------
 3 files changed, 83 insertions(+), 77 deletions(-)

diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
index 317e482a1498..441ff1b0705c 100644
--- a/src/libcamera/pipeline/ipu3/imgu.cpp
+++ b/src/libcamera/pipeline/ipu3/imgu.cpp
@@ -34,22 +34,6 @@ namespace {
  * at revision: 243d13446e44 ("Fix some bug for some resolutions")
  */
 
-static constexpr unsigned int FILTER_W = 4;
-static constexpr unsigned int FILTER_H = 4;
-
-static constexpr unsigned int IF_ALIGN_W = 2;
-static constexpr unsigned int IF_ALIGN_H = 4;
-
-static constexpr unsigned int BDS_ALIGN_W = 2;
-static constexpr unsigned int BDS_ALIGN_H = 4;
-
-static constexpr unsigned int IF_CROP_MAX_W = 40;
-static constexpr unsigned int IF_CROP_MAX_H = 540;
-
-static constexpr float BDS_SF_MAX = 2.5;
-static constexpr float BDS_SF_MIN = 1.0;
-static constexpr float BDS_SF_STEP = 0.03125;
-
 /* BSD scaling factors: min=1, max=2.5, step=1/32 */
 const std::vector<float> bdsScalingFactors = {
 	1, 1.03125, 1.0625, 1.09375, 1.125, 1.15625, 1.1875, 1.21875, 1.25,
@@ -124,8 +108,8 @@ bool isSameRatio(const Size &in, const Size &out)
 void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc,
 			unsigned int bdsWidth, float bdsSF)
 {
-	unsigned int minIFHeight = iif.height - IF_CROP_MAX_H;
-	unsigned int minBDSHeight = gdc.height + FILTER_H * 2;
+	unsigned int minIFHeight = iif.height - IMGU::IF_CROP_MAX_H;
+	unsigned int minBDSHeight = gdc.height + IMGU::FILTER_H * 2;
 	unsigned int ifHeight;
 	float bdsHeight;
 
@@ -135,7 +119,7 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc
 				    static_cast<float>(gdc.width);
 		estIFHeight = std::clamp<float>(estIFHeight, minIFHeight, iif.height);
 
-		ifHeight = utils::alignUp(estIFHeight, IF_ALIGN_H);
+		ifHeight = utils::alignUp(estIFHeight, IMGU::IF_ALIGN_H);
 		while (ifHeight >= minIFHeight && ifHeight <= iif.height &&
 		       ifHeight / bdsSF >= minBDSHeight) {
 
@@ -143,17 +127,17 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc
 			if (std::fmod(height, 1.0) == 0) {
 				unsigned int bdsIntHeight = static_cast<unsigned int>(height);
 
-				if (!(bdsIntHeight % BDS_ALIGN_H)) {
+				if (!(bdsIntHeight % IMGU::BDS_ALIGN_H)) {
 					foundIfHeight = ifHeight;
 					bdsHeight = height;
 					break;
 				}
 			}
 
-			ifHeight -= IF_ALIGN_H;
+			ifHeight -= IMGU::IF_ALIGN_H;
 		}
 
-		ifHeight = utils::alignUp(estIFHeight, IF_ALIGN_H);
+		ifHeight = utils::alignUp(estIFHeight, IMGU::IF_ALIGN_H);
 		while (ifHeight >= minIFHeight && ifHeight <= iif.height &&
 		       ifHeight / bdsSF >= minBDSHeight) {
 
@@ -161,14 +145,14 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc
 			if (std::fmod(height, 1.0) == 0) {
 				unsigned int bdsIntHeight = static_cast<unsigned int>(height);
 
-				if (!(bdsIntHeight % BDS_ALIGN_H)) {
+				if (!(bdsIntHeight % IMGU::BDS_ALIGN_H)) {
 					foundIfHeight = ifHeight;
 					bdsHeight = height;
 					break;
 				}
 			}
 
-			ifHeight += IF_ALIGN_H;
+			ifHeight += IMGU::IF_ALIGN_H;
 		}
 
 		if (foundIfHeight) {
@@ -179,32 +163,32 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc
 			return;
 		}
 	} else {
-		ifHeight = utils::alignUp(iif.height, IF_ALIGN_H);
+		ifHeight = utils::alignUp(iif.height, IMGU::IF_ALIGN_H);
 		while (ifHeight >= minIFHeight && ifHeight / bdsSF >= minBDSHeight) {
 
 			bdsHeight = ifHeight / bdsSF;
 			if (std::fmod(ifHeight, 1.0) == 0 && std::fmod(bdsHeight, 1.0) == 0) {
 				unsigned int bdsIntHeight = static_cast<unsigned int>(bdsHeight);
 
-				if (!(ifHeight % IF_ALIGN_H) &&
-				    !(bdsIntHeight % BDS_ALIGN_H)) {
+				if (!(ifHeight % IMGU::IF_ALIGN_H) &&
+				    !(bdsIntHeight % IMGU::BDS_ALIGN_H)) {
 					pipeConfigs.push_back({ bdsSF, { iif.width, ifHeight },
 								{ bdsWidth, bdsIntHeight }, gdc });
 				}
 			}
 
-			ifHeight -= IF_ALIGN_H;
+			ifHeight -= IMGU::IF_ALIGN_H;
 		}
 	}
 }
 
 void calculateBDS(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc, float bdsSF)
 {
-	unsigned int minBDSWidth = gdc.width + FILTER_W * 2;
-	unsigned int minBDSHeight = gdc.height + FILTER_H * 2;
+	unsigned int minBDSWidth = gdc.width + IMGU::FILTER_W * 2;
+	unsigned int minBDSHeight = gdc.height + IMGU::FILTER_H * 2;
 
 	float sf = bdsSF;
-	while (sf <= BDS_SF_MAX && sf >= BDS_SF_MIN) {
+	while (sf <= IMGU::BDS_SF_MAX && sf >= IMGU::BDS_SF_MIN) {
 		float bdsWidth = static_cast<float>(iif.width) / sf;
 		float bdsHeight = static_cast<float>(iif.height) / sf;
 
@@ -212,16 +196,16 @@ void calculateBDS(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc, floa
 		    std::fmod(bdsHeight, 1.0) == 0) {
 			unsigned int bdsIntWidth = static_cast<unsigned int>(bdsWidth);
 			unsigned int bdsIntHeight = static_cast<unsigned int>(bdsHeight);
-			if (!(bdsIntWidth % BDS_ALIGN_W) && bdsWidth >= minBDSWidth &&
-			    !(bdsIntHeight % BDS_ALIGN_H) && bdsHeight >= minBDSHeight)
+			if (!(bdsIntWidth % IMGU::BDS_ALIGN_W) && bdsWidth >= minBDSWidth &&
+			    !(bdsIntHeight % IMGU::BDS_ALIGN_H) && bdsHeight >= minBDSHeight)
 				calculateBDSHeight(pipe, iif, gdc, bdsIntWidth, sf);
 		}
 
-		sf += BDS_SF_STEP;
+		sf += IMGU::BDS_SF_STEP;
 	}
 
 	sf = bdsSF;
-	while (sf <= BDS_SF_MAX && sf >= BDS_SF_MIN) {
+	while (sf <= IMGU::BDS_SF_MAX && sf >= IMGU::BDS_SF_MIN) {
 		float bdsWidth = static_cast<float>(iif.width) / sf;
 		float bdsHeight = static_cast<float>(iif.height) / sf;
 
@@ -229,12 +213,12 @@ void calculateBDS(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc, floa
 		    std::fmod(bdsHeight, 1.0) == 0) {
 			unsigned int bdsIntWidth = static_cast<unsigned int>(bdsWidth);
 			unsigned int bdsIntHeight = static_cast<unsigned int>(bdsHeight);
-			if (!(bdsIntWidth % BDS_ALIGN_W) && bdsWidth >= minBDSWidth &&
-			    !(bdsIntHeight % BDS_ALIGN_H) && bdsHeight >= minBDSHeight)
+			if (!(bdsIntWidth % IMGU::BDS_ALIGN_W) && bdsWidth >= minBDSWidth &&
+			    !(bdsIntHeight % IMGU::BDS_ALIGN_H) && bdsHeight >= minBDSHeight)
 				calculateBDSHeight(pipe, iif, gdc, bdsIntWidth, sf);
 		}
 
-		sf -= BDS_SF_STEP;
+		sf -= IMGU::BDS_SF_STEP;
 	}
 }
 
@@ -412,7 +396,7 @@ ImgUDevice::PipeConfig ImgUDevice::calculatePipeConfig(Pipe *pipe)
 	 * \todo Filter out all resolutions < IF_CROP_MAX.
 	 * See https://bugs.libcamera.org/show_bug.cgi?id=32
 	 */
-	if (in.width < IF_CROP_MAX_W || in.height < IF_CROP_MAX_H) {
+	if (in.width < IMGU::IF_CROP_MAX_W || in.height < IMGU::IF_CROP_MAX_H) {
 		LOG(IPU3, Error) << "Input resolution " << in.toString()
 				 << " not supported";
 		return {};
@@ -424,25 +408,25 @@ ImgUDevice::PipeConfig ImgUDevice::calculatePipeConfig(Pipe *pipe)
 	float sf = findScaleFactor(bdsSF, bdsScalingFactors, true);
 
 	/* Populate the configurations vector by scaling width and height. */
-	unsigned int ifWidth = utils::alignUp(in.width, IF_ALIGN_W);
-	unsigned int ifHeight = utils::alignUp(in.height, IF_ALIGN_H);
-	unsigned int minIfWidth = in.width - IF_CROP_MAX_W;
-	unsigned int minIfHeight = in.height - IF_CROP_MAX_H;
+	unsigned int ifWidth = utils::alignUp(in.width, IMGU::IF_ALIGN_W);
+	unsigned int ifHeight = utils::alignUp(in.height, IMGU::IF_ALIGN_H);
+	unsigned int minIfWidth = in.width - IMGU::IF_CROP_MAX_W;
+	unsigned int minIfHeight = in.height - IMGU::IF_CROP_MAX_H;
 	while (ifWidth >= minIfWidth) {
 		while (ifHeight >= minIfHeight) {
 			Size iif{ ifWidth, ifHeight };
 			calculateBDS(pipe, iif, gdc, sf);
-			ifHeight -= IF_ALIGN_H;
+			ifHeight -= IMGU::IF_ALIGN_H;
 		}
 
-		ifWidth -= IF_ALIGN_W;
+		ifWidth -= IMGU::IF_ALIGN_W;
 	}
 
 	/* Repeat search by scaling width first. */
-	ifWidth = utils::alignUp(in.width, IF_ALIGN_W);
-	ifHeight = utils::alignUp(in.height, IF_ALIGN_H);
-	minIfWidth = in.width - IF_CROP_MAX_W;
-	minIfHeight = in.height - IF_CROP_MAX_H;
+	ifWidth = utils::alignUp(in.width, IMGU::IF_ALIGN_W);
+	ifHeight = utils::alignUp(in.height, IMGU::IF_ALIGN_H);
+	minIfWidth = in.width - IMGU::IF_CROP_MAX_W;
+	minIfHeight = in.height - IMGU::IF_CROP_MAX_H;
 	while (ifHeight >= minIfHeight) {
 		/*
 		 * \todo This procedure is probably broken:
@@ -451,10 +435,10 @@ ImgUDevice::PipeConfig ImgUDevice::calculatePipeConfig(Pipe *pipe)
 		while (ifWidth >= minIfWidth) {
 			Size iif{ ifWidth, ifHeight };
 			calculateBDS(pipe, iif, gdc, sf);
-			ifWidth -= IF_ALIGN_W;
+			ifWidth -= IMGU::IF_ALIGN_W;
 		}
 
-		ifHeight -= IF_ALIGN_H;
+		ifHeight -= IMGU::IF_ALIGN_H;
 	}
 
 	if (pipeConfigs.size() == 0) {
diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h
index 9d4915116087..df64cbaba5a7 100644
--- a/src/libcamera/pipeline/ipu3/imgu.h
+++ b/src/libcamera/pipeline/ipu3/imgu.h
@@ -15,6 +15,33 @@
 
 namespace libcamera {
 
+namespace IMGU {
+
+static constexpr unsigned int FILTER_W = 4;
+static constexpr unsigned int FILTER_H = 4;
+
+static constexpr unsigned int IF_ALIGN_W = 2;
+static constexpr unsigned int IF_ALIGN_H = 4;
+
+static constexpr unsigned int IF_CROP_MAX_W = 40;
+static constexpr unsigned int IF_CROP_MAX_H = 540;
+
+static constexpr unsigned int BDS_ALIGN_W = 2;
+static constexpr unsigned int BDS_ALIGN_H = 4;
+
+static constexpr float BDS_SF_MAX = 2.5;
+static constexpr float BDS_SF_MIN = 1.0;
+static constexpr float BDS_SF_STEP = 0.03125;
+
+static const Size OUTPUT_MIN_SIZE = { 2, 2 };
+static const Size OUTPUT_MAX_SIZE = { 4480, 34004 };
+static constexpr unsigned int OUTPUT_WIDTH_ALIGN = 64;
+static constexpr unsigned int OUTPUT_HEIGHT_ALIGN = 4;
+static constexpr unsigned int OUTPUT_WIDTH_MARGIN = 64;
+static constexpr unsigned int OUTPUT_HEIGHT_MARGIN = 32;
+
+} /* namespace IMGU */
+
 class FrameBuffer;
 class MediaDevice;
 class Size;
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 291338288685..89a05fab69ad 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -41,12 +41,6 @@ LOG_DEFINE_CATEGORY(IPU3)
 
 static constexpr unsigned int IPU3_BUFFER_COUNT = 4;
 static constexpr unsigned int IPU3_MAX_STREAMS = 3;
-static const Size IMGU_OUTPUT_MIN_SIZE = { 2, 2 };
-static const Size IMGU_OUTPUT_MAX_SIZE = { 4480, 34004 };
-static constexpr unsigned int IMGU_OUTPUT_WIDTH_ALIGN = 64;
-static constexpr unsigned int IMGU_OUTPUT_HEIGHT_ALIGN = 4;
-static constexpr unsigned int IMGU_OUTPUT_WIDTH_MARGIN = 64;
-static constexpr unsigned int IMGU_OUTPUT_HEIGHT_MARGIN = 32;
 static constexpr Size IPU3ViewfinderSize(1280, 720);
 
 static const ControlInfoMap::Map IPU3Controls = {
@@ -287,9 +281,10 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
 	 * https://bugs.libcamera.org/show_bug.cgi?id=32
 	 */
 	if (rawSize.isNull())
-		rawSize = maxYuvSize.alignedUpTo(40, 540)
-				    .alignedUpTo(IMGU_OUTPUT_WIDTH_MARGIN,
-						 IMGU_OUTPUT_HEIGHT_MARGIN)
+		rawSize = maxYuvSize.alignedUpTo(IMGU::IF_CROP_MAX_W,
+						 IMGU::IF_CROP_MAX_H)
+				    .alignedUpTo(IMGU::OUTPUT_WIDTH_MARGIN,
+						 IMGU::OUTPUT_HEIGHT_MARGIN)
 				    .boundedTo(data_->cio2_.sensor()->resolution());
 	cio2Configuration_ = data_->cio2_.generateConfiguration(rawSize);
 	if (!cio2Configuration_.pixelFormat.isValid())
@@ -345,19 +340,19 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
 			 */
 			unsigned int limit;
 			limit = utils::alignDown(cio2Configuration_.size.width - 1,
-						 IMGU_OUTPUT_WIDTH_MARGIN);
+						 IMGU::OUTPUT_WIDTH_MARGIN);
 			cfg->size.width = std::clamp(cfg->size.width,
-						     IMGU_OUTPUT_MIN_SIZE.width,
+						     IMGU::OUTPUT_MIN_SIZE.width,
 						     limit);
 
 			limit = utils::alignDown(cio2Configuration_.size.height - 1,
-						 IMGU_OUTPUT_HEIGHT_MARGIN);
+						 IMGU::OUTPUT_HEIGHT_MARGIN);
 			cfg->size.height = std::clamp(cfg->size.height,
-						      IMGU_OUTPUT_MIN_SIZE.height,
+						      IMGU::OUTPUT_MIN_SIZE.height,
 						      limit);
 
-			cfg->size.alignDownTo(IMGU_OUTPUT_WIDTH_ALIGN,
-					      IMGU_OUTPUT_HEIGHT_ALIGN);
+			cfg->size.alignDownTo(IMGU::OUTPUT_WIDTH_ALIGN,
+					      IMGU::OUTPUT_HEIGHT_ALIGN);
 
 			cfg->pixelFormat = formats::NV12;
 			cfg->bufferCount = IPU3_BUFFER_COUNT;
@@ -443,14 +438,14 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
 			 * \todo Clarify the alignment constraints as explained
 			 * in validate()
 			 */
-			size = sensorResolution.boundedTo(IMGU_OUTPUT_MAX_SIZE);
+			size = sensorResolution.boundedTo(IMGU::OUTPUT_MAX_SIZE);
 			size.width = utils::alignDown(size.width - 1,
-						      IMGU_OUTPUT_WIDTH_MARGIN);
+						      IMGU::OUTPUT_WIDTH_MARGIN);
 			size.height = utils::alignDown(size.height - 1,
-						       IMGU_OUTPUT_HEIGHT_MARGIN);
+						       IMGU::OUTPUT_HEIGHT_MARGIN);
 			pixelFormat = formats::NV12;
 			bufferCount = IPU3_BUFFER_COUNT;
-			streamFormats[pixelFormat] = { { IMGU_OUTPUT_MIN_SIZE, size } };
+			streamFormats[pixelFormat] = { { IMGU::OUTPUT_MIN_SIZE, size } };
 
 			break;
 
@@ -475,11 +470,11 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
 			 * to the ImgU output constraints.
 			 */
 			size = sensorResolution.boundedTo(IPU3ViewfinderSize)
-					       .alignedDownTo(IMGU_OUTPUT_WIDTH_ALIGN,
-							      IMGU_OUTPUT_HEIGHT_ALIGN);
+					       .alignedDownTo(IMGU::OUTPUT_WIDTH_ALIGN,
+							      IMGU::OUTPUT_HEIGHT_ALIGN);
 			pixelFormat = formats::NV12;
 			bufferCount = IPU3_BUFFER_COUNT;
-			streamFormats[pixelFormat] = { { IMGU_OUTPUT_MIN_SIZE, size } };
+			streamFormats[pixelFormat] = { { IMGU::OUTPUT_MIN_SIZE, size } };
 
 			break;
 		}
@@ -1003,8 +998,8 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data)
 	/* The strictly smaller size than the sensor resolution, aligned to margins. */
 	Size minSize = Size(sensor->resolution().width - 1,
 			    sensor->resolution().height - 1)
-		       .alignedDownTo(IMGU_OUTPUT_WIDTH_MARGIN,
-				      IMGU_OUTPUT_HEIGHT_MARGIN);
+		       .alignedDownTo(IMGU::OUTPUT_WIDTH_MARGIN,
+				      IMGU::OUTPUT_HEIGHT_MARGIN);
 
 	/*
 	 * Either the smallest margin-aligned size larger than the viewfinder
@@ -1012,8 +1007,8 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data)
 	 */
 	minSize = Size(IPU3ViewfinderSize.width + 1,
 		       IPU3ViewfinderSize.height + 1)
-		  .alignedUpTo(IMGU_OUTPUT_WIDTH_MARGIN,
-			       IMGU_OUTPUT_HEIGHT_MARGIN)
+		  .alignedUpTo(IMGU::OUTPUT_WIDTH_MARGIN,
+			       IMGU::OUTPUT_HEIGHT_MARGIN)
 		  .boundedTo(minSize);
 
 	/*
-- 
2.32.0



More information about the libcamera-devel mailing list