From patchwork Tue Sep 7 19:40:52 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 13733 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 8D31CBDB1D for ; Tue, 7 Sep 2021 19:40:31 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 9EA2269177; Tue, 7 Sep 2021 21:40:30 +0200 (CEST) Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id F390F6916F for ; Tue, 7 Sep 2021 21:40:28 +0200 (CEST) Received: (Authenticated sender: jacopo@jmondi.org) by relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 7E7DF240005; Tue, 7 Sep 2021 19:40:28 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Tue, 7 Sep 2021 21:40:52 +0200 Message-Id: <20210907194107.803730-3-jacopo@jmondi.org> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210907194107.803730-1-jacopo@jmondi.org> References: <20210907194107.803730-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 02/17] 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 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 Reviewed-by: Paul Elder Reviewed-by: Jean-Michel Hautbois --- 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 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(gdc.width); estIFHeight = std::clamp(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(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(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(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(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 % 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(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 % 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); /*