[libcamera-devel,v3,02/16] libcamera: ipu3: Centralize ImgU sizes definition
diff mbox series

Message ID 20211011151154.72856-3-jacopo@jmondi.org
State Superseded
Headers show
Series
  • IPU3 control info update and HAL frame durations
Related show

Commit Message

Jacopo Mondi Oct. 11, 2021, 3:11 p.m. UTC 
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 file, 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 alignments to the
ImgUDevice class as static constexpr and update their users accordingly.

Cosmetic changes, no functional changes intended.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
---
 src/libcamera/pipeline/ipu3/imgu.cpp | 86 +++++++++++-----------------
 src/libcamera/pipeline/ipu3/imgu.h   | 23 ++++++++
 src/libcamera/pipeline/ipu3/ipu3.cpp | 47 +++++++--------
 3 files changed, 79 insertions(+), 77 deletions(-)

Comments

Laurent Pinchart Oct. 13, 2021, 12:48 a.m. UTC | #1 
Hi Jacopo,

Thank you for the patch.

On Mon, Oct 11, 2021 at 05:11:40PM +0200, Jacopo Mondi wrote:
> 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 file, 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 alignments to the
> ImgUDevice class as static constexpr and update their users accordingly.
> 
> Cosmetic changes, no functional changes intended.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> ---
>  src/libcamera/pipeline/ipu3/imgu.cpp | 86 +++++++++++-----------------
>  src/libcamera/pipeline/ipu3/imgu.h   | 23 ++++++++
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 47 +++++++--------
>  3 files changed, 79 insertions(+), 77 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
> index 3e1ef645ec93..f326886bf3da 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 - ImgUDevice::kIFMaxCropHeight;
> +	unsigned int minBDSHeight = gdc.height + ImgUDevice::kFilterHeight * 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, ImgUDevice::kAlignHeight);
>  		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 % ImgUDevice::kBDSAlignHeight)) {
>  					foundIfHeight = ifHeight;
>  					bdsHeight = height;
>  					break;
>  				}
>  			}
>  
> -			ifHeight -= IF_ALIGN_H;
> +			ifHeight -= ImgUDevice::kAlignHeight;
>  		}
>  
> -		ifHeight = utils::alignUp(estIFHeight, IF_ALIGN_H);
> +		ifHeight = utils::alignUp(estIFHeight, ImgUDevice::kAlignHeight);
>  		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 % ImgUDevice::kBDSAlignHeight)) {
>  					foundIfHeight = ifHeight;
>  					bdsHeight = height;
>  					break;
>  				}
>  			}
>  
> -			ifHeight += IF_ALIGN_H;
> +			ifHeight += ImgUDevice::kAlignHeight;
>  		}
>  
>  		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, ImgUDevice::kAlignHeight);
>  		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 % ImgUDevice::kAlignHeight) &&
> +				    !(bdsIntHeight % ImgUDevice::kBDSAlignHeight)) {
>  					pipeConfigs.push_back({ bdsSF, { iif.width, ifHeight },
>  								{ bdsWidth, bdsIntHeight }, gdc });
>  				}
>  			}
>  
> -			ifHeight -= IF_ALIGN_H;
> +			ifHeight -= ImgUDevice::kAlignHeight;
>  		}
>  	}
>  }
>  
>  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 + ImgUDevice::kFilterWidth * 2;
> +	unsigned int minBDSHeight = gdc.height + ImgUDevice::kFilterHeight * 2;
>  
>  	float sf = bdsSF;
> -	while (sf <= BDS_SF_MAX && sf >= BDS_SF_MIN) {
> +	while (sf <= ImgUDevice::kBDSSfMax && sf >= ImgUDevice::kBDSSfMin) {
>  		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 % ImgUDevice::kBDSAlignWidth) && bdsWidth >= minBDSWidth &&
> +			    !(bdsIntHeight % ImgUDevice::kBDSAlignHeight) && bdsHeight >= minBDSHeight)
>  				calculateBDSHeight(pipe, iif, gdc, bdsIntWidth, sf);
>  		}
>  
> -		sf += BDS_SF_STEP;
> +		sf += ImgUDevice::kBDSSfStep;
>  	}
>  
>  	sf = bdsSF;
> -	while (sf <= BDS_SF_MAX && sf >= BDS_SF_MIN) {
> +	while (sf <= ImgUDevice::kBDSSfMax && sf >= ImgUDevice::kBDSSfMin) {
>  		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 % ImgUDevice::kBDSAlignWidth) && bdsWidth >= minBDSWidth &&
> +			    !(bdsIntHeight % ImgUDevice::kBDSAlignHeight) && bdsHeight >= minBDSHeight)
>  				calculateBDSHeight(pipe, iif, gdc, bdsIntWidth, sf);
>  		}
>  
> -		sf -= BDS_SF_STEP;
> +		sf -= ImgUDevice::kBDSSfStep;
>  	}
>  }
>  
> @@ -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 < ImgUDevice::kIFMaxCropWidth || in.height < ImgUDevice::kIFMaxCropHeight) {
>  		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, ImgUDevice::kAlignWidth);
> +	unsigned int ifHeight = utils::alignUp(in.height, ImgUDevice::kAlignHeight);
> +	unsigned int minIfWidth = in.width - ImgUDevice::kIFMaxCropWidth;
> +	unsigned int minIfHeight = in.height - ImgUDevice::kIFMaxCropHeight;
>  	while (ifWidth >= minIfWidth) {
>  		while (ifHeight >= minIfHeight) {
>  			Size iif{ ifWidth, ifHeight };
>  			calculateBDS(pipe, iif, gdc, sf);
> -			ifHeight -= IF_ALIGN_H;
> +			ifHeight -= ImgUDevice::kAlignHeight;
>  		}
>  
> -		ifWidth -= IF_ALIGN_W;
> +		ifWidth -= ImgUDevice::kAlignWidth;
>  	}
>  
>  	/* 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, ImgUDevice::kAlignWidth);
> +	ifHeight = utils::alignUp(in.height, ImgUDevice::kAlignHeight);
> +	minIfWidth = in.width - ImgUDevice::kIFMaxCropWidth;
> +	minIfHeight = in.height - ImgUDevice::kIFMaxCropHeight;
>  	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 -= ImgUDevice::kAlignWidth;
>  		}
>  
> -		ifHeight -= IF_ALIGN_H;
> +		ifHeight -= ImgUDevice::kAlignHeight;
>  	}
>  
>  	if (pipeConfigs.size() == 0) {
> diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h
> index 9d4915116087..690a22e67d47 100644
> --- a/src/libcamera/pipeline/ipu3/imgu.h
> +++ b/src/libcamera/pipeline/ipu3/imgu.h
> @@ -23,6 +23,29 @@ struct StreamConfiguration;
>  class ImgUDevice
>  {
>  public:
> +	static constexpr unsigned int kFilterWidth = 4;
> +	static constexpr unsigned int kFilterHeight = 4;
> +
> +	static constexpr unsigned int kAlignWidth = 2;
> +	static constexpr unsigned int kAlignHeight = 4;

I'd name these kIFAlignWidth and kIFAlignHeight, they were called
IF_ALIGN_W and IF_ALIGN_H.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
> +	static constexpr unsigned int kIFMaxCropWidth = 40;
> +	static constexpr unsigned int kIFMaxCropHeight = 540;
> +
> +	static constexpr unsigned int kBDSAlignWidth = 2;
> +	static constexpr unsigned int kBDSAlignHeight = 4;
> +
> +	static constexpr float kBDSSfMax = 2.5;
> +	static constexpr float kBDSSfMin = 1.0;
> +	static constexpr float kBDSSfStep = 0.03125;
> +
> +	static constexpr Size kOutputMinSize = { 2, 2 };
> +	static constexpr Size kOutputMaxSize = { 4480, 34004 };
> +	static constexpr unsigned int kOutputAlignWidth = 64;
> +	static constexpr unsigned int kOutputAlignHeight = 4;
> +	static constexpr unsigned int kOutputMarginWidth = 64;
> +	static constexpr unsigned int kOutputMarginHeight = 32;
> +
>  	struct PipeConfig {
>  		float bds_sf;
>  		Size iif;
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 5b2ab2ee6825..bb3826eee422 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.expandedTo({40, 540})
> -				    .alignedUpTo(IMGU_OUTPUT_WIDTH_MARGIN,
> -						 IMGU_OUTPUT_HEIGHT_MARGIN)
> +		rawSize = maxYuvSize.expandedTo({ImgUDevice::kIFMaxCropWidth,
> +						 ImgUDevice::kIFMaxCropHeight})
> +				    .alignedUpTo(ImgUDevice::kOutputMarginWidth,
> +						 ImgUDevice::kOutputMarginHeight)
>  				    .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);
> +						 ImgUDevice::kOutputMarginWidth);
>  			cfg->size.width = std::clamp(cfg->size.width,
> -						     IMGU_OUTPUT_MIN_SIZE.width,
> +						     ImgUDevice::kOutputMinSize.width,
>  						     limit);
>  
>  			limit = utils::alignDown(cio2Configuration_.size.height - 1,
> -						 IMGU_OUTPUT_HEIGHT_MARGIN);
> +						 ImgUDevice::kOutputMarginHeight);
>  			cfg->size.height = std::clamp(cfg->size.height,
> -						      IMGU_OUTPUT_MIN_SIZE.height,
> +						      ImgUDevice::kOutputMinSize.height,
>  						      limit);
>  
> -			cfg->size.alignDownTo(IMGU_OUTPUT_WIDTH_ALIGN,
> -					      IMGU_OUTPUT_HEIGHT_ALIGN);
> +			cfg->size.alignDownTo(ImgUDevice::kOutputAlignWidth,
> +					      ImgUDevice::kOutputAlignHeight);
>  
>  			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(ImgUDevice::kOutputMaxSize);
>  			size.width = utils::alignDown(size.width - 1,
> -						      IMGU_OUTPUT_WIDTH_MARGIN);
> +						      ImgUDevice::kOutputMarginWidth);
>  			size.height = utils::alignDown(size.height - 1,
> -						       IMGU_OUTPUT_HEIGHT_MARGIN);
> +						       ImgUDevice::kOutputMarginHeight);
>  			pixelFormat = formats::NV12;
>  			bufferCount = IPU3_BUFFER_COUNT;
> -			streamFormats[pixelFormat] = { { IMGU_OUTPUT_MIN_SIZE, size } };
> +			streamFormats[pixelFormat] = { { ImgUDevice::kOutputMinSize, 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(ImgUDevice::kOutputAlignWidth,
> +							      ImgUDevice::kOutputAlignHeight);
>  			pixelFormat = formats::NV12;
>  			bufferCount = IPU3_BUFFER_COUNT;
> -			streamFormats[pixelFormat] = { { IMGU_OUTPUT_MIN_SIZE, size } };
> +			streamFormats[pixelFormat] = { { ImgUDevice::kOutputMinSize, 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(ImgUDevice::kOutputMarginWidth,
> +				      ImgUDevice::kOutputMarginHeight);
>  
>  	/*
>  	 * 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(ImgUDevice::kOutputMarginWidth,
> +			       ImgUDevice::kOutputMarginHeight)
>  		  .boundedTo(minSize);
>  
>  	/*

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
index 3e1ef645ec93..f326886bf3da 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 - ImgUDevice::kIFMaxCropHeight;
+	unsigned int minBDSHeight = gdc.height + ImgUDevice::kFilterHeight * 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, ImgUDevice::kAlignHeight);
 		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 % ImgUDevice::kBDSAlignHeight)) {
 					foundIfHeight = ifHeight;
 					bdsHeight = height;
 					break;
 				}
 			}
 
-			ifHeight -= IF_ALIGN_H;
+			ifHeight -= ImgUDevice::kAlignHeight;
 		}
 
-		ifHeight = utils::alignUp(estIFHeight, IF_ALIGN_H);
+		ifHeight = utils::alignUp(estIFHeight, ImgUDevice::kAlignHeight);
 		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 % ImgUDevice::kBDSAlignHeight)) {
 					foundIfHeight = ifHeight;
 					bdsHeight = height;
 					break;
 				}
 			}
 
-			ifHeight += IF_ALIGN_H;
+			ifHeight += ImgUDevice::kAlignHeight;
 		}
 
 		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, ImgUDevice::kAlignHeight);
 		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 % ImgUDevice::kAlignHeight) &&
+				    !(bdsIntHeight % ImgUDevice::kBDSAlignHeight)) {
 					pipeConfigs.push_back({ bdsSF, { iif.width, ifHeight },
 								{ bdsWidth, bdsIntHeight }, gdc });
 				}
 			}
 
-			ifHeight -= IF_ALIGN_H;
+			ifHeight -= ImgUDevice::kAlignHeight;
 		}
 	}
 }
 
 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 + ImgUDevice::kFilterWidth * 2;
+	unsigned int minBDSHeight = gdc.height + ImgUDevice::kFilterHeight * 2;
 
 	float sf = bdsSF;
-	while (sf <= BDS_SF_MAX && sf >= BDS_SF_MIN) {
+	while (sf <= ImgUDevice::kBDSSfMax && sf >= ImgUDevice::kBDSSfMin) {
 		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 % ImgUDevice::kBDSAlignWidth) && bdsWidth >= minBDSWidth &&
+			    !(bdsIntHeight % ImgUDevice::kBDSAlignHeight) && bdsHeight >= minBDSHeight)
 				calculateBDSHeight(pipe, iif, gdc, bdsIntWidth, sf);
 		}
 
-		sf += BDS_SF_STEP;
+		sf += ImgUDevice::kBDSSfStep;
 	}
 
 	sf = bdsSF;
-	while (sf <= BDS_SF_MAX && sf >= BDS_SF_MIN) {
+	while (sf <= ImgUDevice::kBDSSfMax && sf >= ImgUDevice::kBDSSfMin) {
 		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 % ImgUDevice::kBDSAlignWidth) && bdsWidth >= minBDSWidth &&
+			    !(bdsIntHeight % ImgUDevice::kBDSAlignHeight) && bdsHeight >= minBDSHeight)
 				calculateBDSHeight(pipe, iif, gdc, bdsIntWidth, sf);
 		}
 
-		sf -= BDS_SF_STEP;
+		sf -= ImgUDevice::kBDSSfStep;
 	}
 }
 
@@ -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 < ImgUDevice::kIFMaxCropWidth || in.height < ImgUDevice::kIFMaxCropHeight) {
 		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, ImgUDevice::kAlignWidth);
+	unsigned int ifHeight = utils::alignUp(in.height, ImgUDevice::kAlignHeight);
+	unsigned int minIfWidth = in.width - ImgUDevice::kIFMaxCropWidth;
+	unsigned int minIfHeight = in.height - ImgUDevice::kIFMaxCropHeight;
 	while (ifWidth >= minIfWidth) {
 		while (ifHeight >= minIfHeight) {
 			Size iif{ ifWidth, ifHeight };
 			calculateBDS(pipe, iif, gdc, sf);
-			ifHeight -= IF_ALIGN_H;
+			ifHeight -= ImgUDevice::kAlignHeight;
 		}
 
-		ifWidth -= IF_ALIGN_W;
+		ifWidth -= ImgUDevice::kAlignWidth;
 	}
 
 	/* 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, ImgUDevice::kAlignWidth);
+	ifHeight = utils::alignUp(in.height, ImgUDevice::kAlignHeight);
+	minIfWidth = in.width - ImgUDevice::kIFMaxCropWidth;
+	minIfHeight = in.height - ImgUDevice::kIFMaxCropHeight;
 	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 -= ImgUDevice::kAlignWidth;
 		}
 
-		ifHeight -= IF_ALIGN_H;
+		ifHeight -= ImgUDevice::kAlignHeight;
 	}
 
 	if (pipeConfigs.size() == 0) {
diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h
index 9d4915116087..690a22e67d47 100644
--- a/src/libcamera/pipeline/ipu3/imgu.h
+++ b/src/libcamera/pipeline/ipu3/imgu.h
@@ -23,6 +23,29 @@  struct StreamConfiguration;
 class ImgUDevice
 {
 public:
+	static constexpr unsigned int kFilterWidth = 4;
+	static constexpr unsigned int kFilterHeight = 4;
+
+	static constexpr unsigned int kAlignWidth = 2;
+	static constexpr unsigned int kAlignHeight = 4;
+
+	static constexpr unsigned int kIFMaxCropWidth = 40;
+	static constexpr unsigned int kIFMaxCropHeight = 540;
+
+	static constexpr unsigned int kBDSAlignWidth = 2;
+	static constexpr unsigned int kBDSAlignHeight = 4;
+
+	static constexpr float kBDSSfMax = 2.5;
+	static constexpr float kBDSSfMin = 1.0;
+	static constexpr float kBDSSfStep = 0.03125;
+
+	static constexpr Size kOutputMinSize = { 2, 2 };
+	static constexpr Size kOutputMaxSize = { 4480, 34004 };
+	static constexpr unsigned int kOutputAlignWidth = 64;
+	static constexpr unsigned int kOutputAlignHeight = 4;
+	static constexpr unsigned int kOutputMarginWidth = 64;
+	static constexpr unsigned int kOutputMarginHeight = 32;
+
 	struct PipeConfig {
 		float bds_sf;
 		Size iif;
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 5b2ab2ee6825..bb3826eee422 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.expandedTo({40, 540})
-				    .alignedUpTo(IMGU_OUTPUT_WIDTH_MARGIN,
-						 IMGU_OUTPUT_HEIGHT_MARGIN)
+		rawSize = maxYuvSize.expandedTo({ImgUDevice::kIFMaxCropWidth,
+						 ImgUDevice::kIFMaxCropHeight})
+				    .alignedUpTo(ImgUDevice::kOutputMarginWidth,
+						 ImgUDevice::kOutputMarginHeight)
 				    .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);
+						 ImgUDevice::kOutputMarginWidth);
 			cfg->size.width = std::clamp(cfg->size.width,
-						     IMGU_OUTPUT_MIN_SIZE.width,
+						     ImgUDevice::kOutputMinSize.width,
 						     limit);
 
 			limit = utils::alignDown(cio2Configuration_.size.height - 1,
-						 IMGU_OUTPUT_HEIGHT_MARGIN);
+						 ImgUDevice::kOutputMarginHeight);
 			cfg->size.height = std::clamp(cfg->size.height,
-						      IMGU_OUTPUT_MIN_SIZE.height,
+						      ImgUDevice::kOutputMinSize.height,
 						      limit);
 
-			cfg->size.alignDownTo(IMGU_OUTPUT_WIDTH_ALIGN,
-					      IMGU_OUTPUT_HEIGHT_ALIGN);
+			cfg->size.alignDownTo(ImgUDevice::kOutputAlignWidth,
+					      ImgUDevice::kOutputAlignHeight);
 
 			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(ImgUDevice::kOutputMaxSize);
 			size.width = utils::alignDown(size.width - 1,
-						      IMGU_OUTPUT_WIDTH_MARGIN);
+						      ImgUDevice::kOutputMarginWidth);
 			size.height = utils::alignDown(size.height - 1,
-						       IMGU_OUTPUT_HEIGHT_MARGIN);
+						       ImgUDevice::kOutputMarginHeight);
 			pixelFormat = formats::NV12;
 			bufferCount = IPU3_BUFFER_COUNT;
-			streamFormats[pixelFormat] = { { IMGU_OUTPUT_MIN_SIZE, size } };
+			streamFormats[pixelFormat] = { { ImgUDevice::kOutputMinSize, 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(ImgUDevice::kOutputAlignWidth,
+							      ImgUDevice::kOutputAlignHeight);
 			pixelFormat = formats::NV12;
 			bufferCount = IPU3_BUFFER_COUNT;
-			streamFormats[pixelFormat] = { { IMGU_OUTPUT_MIN_SIZE, size } };
+			streamFormats[pixelFormat] = { { ImgUDevice::kOutputMinSize, 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(ImgUDevice::kOutputMarginWidth,
+				      ImgUDevice::kOutputMarginHeight);
 
 	/*
 	 * 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(ImgUDevice::kOutputMarginWidth,
+			       ImgUDevice::kOutputMarginHeight)
 		  .boundedTo(minSize);
 
 	/*