From patchwork Mon Oct 11 15:11:40 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 14086 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 292B0C323E for ; Mon, 11 Oct 2021 15:11:15 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 951D868F57; Mon, 11 Oct 2021 17:11:14 +0200 (CEST) Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net [217.70.183.196]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 295676012B for ; Mon, 11 Oct 2021 17:11:12 +0200 (CEST) Received: (Authenticated sender: jacopo@jmondi.org) by relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 51563E0007; Mon, 11 Oct 2021 15:11:11 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Mon, 11 Oct 2021 17:11:40 +0200 Message-Id: <20211011151154.72856-3-jacopo@jmondi.org> X-Mailer: git-send-email 2.33.0 In-Reply-To: <20211011151154.72856-1-jacopo@jmondi.org> References: <20211011151154.72856-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 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 | 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 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::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(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(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(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(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::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); /*