From patchwork Thu Oct 14 17:41:54 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 14138 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 B4779C323E for ; Thu, 14 Oct 2021 17:41:27 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id B516968F59; Thu, 14 Oct 2021 19:41:26 +0200 (CEST) Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id DF22E68F4D for ; Thu, 14 Oct 2021 19:41:25 +0200 (CEST) Received: (Authenticated sender: jacopo@jmondi.org) by relay9-d.mail.gandi.net (Postfix) with ESMTPSA id C5AEBFF80B; Thu, 14 Oct 2021 17:41:24 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Thu, 14 Oct 2021 19:41:54 +0200 Message-Id: <20211014174208.50509-3-jacopo@jmondi.org> X-Mailer: git-send-email 2.33.0 In-Reply-To: <20211014174208.50509-1-jacopo@jmondi.org> References: <20211014174208.50509-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 02/16] libcamera: ipu3: Centralize ImgU sizes definition 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 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 3e1ef645ec93..3ef0ef144301 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 9d4915116087..2b28d912676a 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 8aab5041b3ea..76fe9edb7dbc 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); /*