From 3bd3b95a09faefd10cb34e2c8847535a51234448 Mon Sep 17 00:00:00 2001 From: Jacopo Mondi Date: Tue, 7 Sep 2021 18:09:37 +0200 Subject: libcamera: ipu3: Centralize ImgU sizes definition 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 Reviewed-by: Paul Elder Reviewed-by: Jean-Michel Hautbois Reviewed-by: Laurent Pinchart --- src/libcamera/pipeline/ipu3/imgu.cpp | 86 +++++++++++++++--------------------- src/libcamera/pipeline/ipu3/imgu.h | 23 ++++++++++ src/libcamera/pipeline/ipu3/ipu3.cpp | 48 +++++++++----------- 3 files changed, 80 insertions(+), 77 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp index 3e1ef645..3ef0ef14 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 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(gdc.width); estIFHeight = std::clamp(estIFHeight, minIFHeight, iif.height); - ifHeight = utils::alignUp(estIFHeight, IF_ALIGN_H); + ifHeight = utils::alignUp(estIFHeight, ImgUDevice::kIFAlignHeight); 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(height); - if (!(bdsIntHeight % BDS_ALIGN_H)) { + if (!(bdsIntHeight % ImgUDevice::kBDSAlignHeight)) { foundIfHeight = ifHeight; bdsHeight = height; break; } } - ifHeight -= IF_ALIGN_H; + ifHeight -= ImgUDevice::kIFAlignHeight; } - ifHeight = utils::alignUp(estIFHeight, IF_ALIGN_H); + ifHeight = utils::alignUp(estIFHeight, ImgUDevice::kIFAlignHeight); 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(height); - if (!(bdsIntHeight % BDS_ALIGN_H)) { + if (!(bdsIntHeight % ImgUDevice::kBDSAlignHeight)) { foundIfHeight = ifHeight; bdsHeight = height; break; } } - ifHeight += IF_ALIGN_H; + ifHeight += ImgUDevice::kIFAlignHeight; } 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::kIFAlignHeight); 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(bdsHeight); - if (!(ifHeight % IF_ALIGN_H) && - !(bdsIntHeight % BDS_ALIGN_H)) { + if (!(ifHeight % ImgUDevice::kIFAlignHeight) && + !(bdsIntHeight % ImgUDevice::kBDSAlignHeight)) { pipeConfigs.push_back({ bdsSF, { iif.width, ifHeight }, { bdsWidth, bdsIntHeight }, gdc }); } } - ifHeight -= IF_ALIGN_H; + ifHeight -= ImgUDevice::kIFAlignHeight; } } } 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(iif.width) / sf; float bdsHeight = static_cast(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(bdsWidth); unsigned int bdsIntHeight = static_cast(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(iif.width) / sf; float bdsHeight = static_cast(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(bdsWidth); unsigned int bdsIntHeight = static_cast(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::kIFAlignWidth); + unsigned int ifHeight = utils::alignUp(in.height, ImgUDevice::kIFAlignHeight); + 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::kIFAlignHeight; } - ifWidth -= IF_ALIGN_W; + ifWidth -= ImgUDevice::kIFAlignWidth; } /* 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::kIFAlignWidth); + ifHeight = utils::alignUp(in.height, ImgUDevice::kIFAlignHeight); + 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::kIFAlignWidth; } - ifHeight -= IF_ALIGN_H; + ifHeight -= ImgUDevice::kIFAlignHeight; } if (pipeConfigs.size() == 0) { diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h index 9d491511..2b28d912 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 kIFAlignWidth = 2; + static constexpr unsigned int kIFAlignHeight = 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 4dd25243..ff4bad4f 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,10 +281,12 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() * https://bugs.libcamera.org/show_bug.cgi?id=32 */ if (rawSize.isNull()) - rawSize = maxYuvSize.expandedTo({40, 540}) - .grownBy({ IMGU_OUTPUT_WIDTH_MARGIN, - IMGU_OUTPUT_HEIGHT_MARGIN }) + rawSize = maxYuvSize.expandedTo({ ImgUDevice::kIFMaxCropWidth, + ImgUDevice::kIFMaxCropHeight }) + .grownBy({ ImgUDevice::kOutputMarginWidth, + ImgUDevice::kOutputMarginHeight }) .boundedTo(data_->cio2_.sensor()->resolution()); + cio2Configuration_ = data_->cio2_.generateConfiguration(rawSize); if (!cio2Configuration_.pixelFormat.isValid()) return Invalid; @@ -345,19 +341,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,13 +439,13 @@ 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) .shrunkBy({ 1, 1 }) - .alignedDownTo(IMGU_OUTPUT_WIDTH_MARGIN, - IMGU_OUTPUT_HEIGHT_MARGIN); + .alignedDownTo(ImgUDevice::kOutputMarginWidth, + ImgUDevice::kOutputMarginHeight); pixelFormat = formats::NV12; bufferCount = IPU3_BUFFER_COUNT; - streamFormats[pixelFormat] = { { IMGU_OUTPUT_MIN_SIZE, size } }; + streamFormats[pixelFormat] = { { ImgUDevice::kOutputMinSize, size } }; break; @@ -474,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; } @@ -1001,16 +997,16 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data) /* The strictly smaller size than the sensor resolution, aligned to margins. */ Size minSize = sensor->resolution().shrunkBy({ 1, 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 * size or the adjusted sensor resolution. */ minSize = IPU3ViewfinderSize.grownBy({ 1, 1 }) - .alignedUpTo(IMGU_OUTPUT_WIDTH_MARGIN, - IMGU_OUTPUT_HEIGHT_MARGIN) + .alignedUpTo(ImgUDevice::kOutputMarginWidth, + ImgUDevice::kOutputMarginHeight) .boundedTo(minSize); /* -- cgit v1.2.1