[{"id":27894,"web_url":"https://patchwork.libcamera.org/comment/27894/","msgid":"<20230926223308.GL5854@pendragon.ideasonboard.com>","date":"2023-09-26T22:33:08","subject":"Re: [libcamera-devel] [PATCH v5 07/13] libcamera: rpi: Make\n\tisRaw/isYuv/isRgb static functions","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Sep 21, 2023 at 06:55:44PM +0200, Jacopo Mondi via libcamera-devel wrote:\n> From: Naushir Patuck \n> \n> Move the existing isRaw()/isYuv()/isRgb()into a static function of\n> PipelineHandlerBase. This will allow then to be shared with the\n\ns/then/them/\n\n> pipeline handler derived class.\n> \n> Signed-off-by: Naushir Patuck \n> Signed-off-by: Jacopo Mondi \n> Reviewed-by: Jacopo Mondi \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> .../pipeline/rpi/common/pipeline_base.cpp | 52 +++++++++----------\n> .../pipeline/rpi/common/pipeline_base.h | 4 ++\n> 2 files changed, 30 insertions(+), 26 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index 502cdff0051b..f6f6185cbfef 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -37,12 +37,6 @@ namespace {\n> \n> constexpr unsigned int defaultRawBitDepth = 12;\n> \n> -bool isRaw(const PixelFormat &pixFmt)\n> -{\n> -\t/* This test works for both Bayer and raw mono formats. */\n> -\treturn BayerFormat::fromPixelFormat(pixFmt).isValid();\n> -}\n> -\n> PixelFormat mbusCodeToPixelFormat(unsigned int mbus_code,\n> \t\t\t\t BayerFormat::Packing packingReq)\n> {\n> @@ -91,22 +85,6 @@ std::optional findValidColorSpace(const ColorSpace &colourSpace)\n> \treturn std::nullopt;\n> }\n> \n> -bool isRgb(const PixelFormat &pixFmt)\n> -{\n> -\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);\n> -\treturn info.colourEncoding == PixelFormatInfo::ColourEncodingRGB;\n> -}\n> -\n> -bool isYuv(const PixelFormat &pixFmt)\n> -{\n> -\t/* The code below would return true for raw mono streams, so weed those out first. */\n> -\tif (isRaw(pixFmt))\n> -\t\treturn false;\n> -\n> -\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);\n> -\treturn info.colourEncoding == PixelFormatInfo::ColourEncodingYUV;\n> -}\n> -\n> } /* namespace */\n> \n> /*\n> @@ -129,7 +107,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validateColorSpaces([[maybe_\n> \n> \tfor (auto cfg : config_) {\n> \t\t/* First fix up raw streams to have the \"raw\" colour space. */\n> -\t\tif (isRaw(cfg.pixelFormat)) {\n> +\t\tif (PipelineHandlerBase::isRaw(cfg.pixelFormat)) {\n> \t\t\t/* If there was no value here, that doesn't count as \"adjusted\". */\n> \t\t\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)\n> \t\t\t\tstatus = Adjusted;\n> @@ -156,13 +134,13 @@ CameraConfiguration::Status RPiCameraConfiguration::validateColorSpaces([[maybe_\n> \t\tif (cfg.colorSpace == ColorSpace::Raw)\n> \t\t\tcontinue;\n> \n> -\t\tif (isYuv(cfg.pixelFormat) && cfg.colorSpace != yuvColorSpace_) {\n> +\t\tif (PipelineHandlerBase::isYuv(cfg.pixelFormat) && cfg.colorSpace != yuvColorSpace_) {\n> \t\t\t/* Again, no value means \"not adjusted\". */\n> \t\t\tif (cfg.colorSpace)\n> \t\t\t\tstatus = Adjusted;\n> \t\t\tcfg.colorSpace = yuvColorSpace_;\n> \t\t}\n> -\t\tif (isRgb(cfg.pixelFormat) && cfg.colorSpace != rgbColorSpace_) {\n> +\t\tif (PipelineHandlerBase::isRgb(cfg.pixelFormat) && cfg.colorSpace != rgbColorSpace_) {\n> \t\t\t/* Be nice, and let the YUV version count as non-adjusted too. */\n> \t\t\tif (cfg.colorSpace && cfg.colorSpace != yuvColorSpace_)\n> \t\t\t\tstatus = Adjusted;\n> @@ -203,7 +181,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> \n> \tstd::vector rawStreams, outStreams;\n> \tfor (const auto &[index, cfg] : utils::enumerate(config_)) {\n> -\t\tif (isRaw(cfg.pixelFormat))\n> +\t\tif (PipelineHandlerBase::isRaw(cfg.pixelFormat))\n> \t\t\trawStreams.emplace_back(index, &cfg);\n> \t\telse\n> \t\t\toutStreams.emplace_back(index, &cfg);\n> @@ -351,6 +329,28 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> \treturn status;\n> }\n> \n> +bool PipelineHandlerBase::isRgb(const PixelFormat &pixFmt)\n> +{\n> +\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);\n> +\treturn info.colourEncoding == PixelFormatInfo::ColourEncodingRGB;\n> +}\n> +\n> +bool PipelineHandlerBase::isYuv(const PixelFormat &pixFmt)\n> +{\n> +\t/* The code below would return true for raw mono streams, so weed those out first. */\n> +\tif (PipelineHandlerBase::isRaw(pixFmt))\n> +\t\treturn false;\n> +\n> +\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);\n> +\treturn info.colourEncoding == PixelFormatInfo::ColourEncodingYUV;\n> +}\n> +\n> +bool PipelineHandlerBase::isRaw(const PixelFormat &pixFmt)\n> +{\n> +\t/* This test works for both Bayer and raw mono formats. */\n> +\treturn BayerFormat::fromPixelFormat(pixFmt).isValid();\n> +}\n> +\n> V4L2DeviceFormat PipelineHandlerBase::toV4L2DeviceFormat(const V4L2VideoDevice *dev,\n> \t\t\t\t\t\t\t const V4L2SubdeviceFormat &format,\n> \t\t\t\t\t\t\t BayerFormat::Packing packingReq)\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> index 3e7c487fb0bf..135b74392140 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> @@ -213,6 +213,10 @@ public:\n> \t{\n> \t}\n> \n> +\tstatic bool isRgb(const PixelFormat &pixFmt);\n> +\tstatic bool isYuv(const PixelFormat &pixFmt);\n> +\tstatic bool isRaw(const PixelFormat &pixFmt);\n> +\n> \tstatic V4L2DeviceFormat toV4L2DeviceFormat(const V4L2VideoDevice *dev,\n> \t\t\t\t\t\t const V4L2SubdeviceFormat &format,\n> \t\t\t\t\t\t BayerFormat::Packing packingReq);","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6C3BFBD808\n\tfor ;\n\tTue, 26 Sep 2023 22:33:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8698462945;\n\tWed, 27 Sep 2023 00:33:00 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B2C8F62931\n\tfor ;\n\tWed, 27 Sep 2023 00:32:58 +0200 (CEST)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5DDC913C5;\n\tWed, 27 Sep 2023 00:31:17 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1695767580;\n\tbh=1Buep2gJxmIW1iXSEFM0A6O/MMMB3yBcjqUmm6KsHnI=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=lvSbVfYhEoNUuSvVSC3RkjWvjHhvAPco1CdRYLNuGcgZwb4lin635sblOPM7RO16O\n\t9H6pIX9kETKoSAh8dg8stuDQQPOPVMGyjJf3yNXFXiKrEIGgAqQvG/gTp+1Lg1qA0u\n\tWZrOhbhrL6ioSvhRTmSiNo+axn1hW9CktUmnKphTo04QLOydT50/qrY1R2U5JUve+o\n\ttu/1CTHZmXIiJ0La/HWaVpWsM7++NrCTGiF5HyHKiJoLWuzHFbSNSp41Q0UUklYJ8/\n\tcDA8T90qLj2KQPswbYIEoOojXDDN2rQZuO3YCP7ilH7QCOexlSWP2e8ajNqpl9kNqC\n\t+MZTzBvVyFNdw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1695767477;\n\tbh=1Buep2gJxmIW1iXSEFM0A6O/MMMB3yBcjqUmm6KsHnI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=CC5XDZqf/7mNNTiajliCusGqAckuALTpUNl/jSl1LXgZEEpfbWLVG8IK2GtiqNQSC\n\tfaBJx8zth5u46pSU76FbOa/SMmkojeBZzvxzvcWqeW2lK2ULAzPaFwQmBnljtAZGQl\n\t0hr/h8m8gCJXRudJ2hYbHVT2akADypARaVXTCFIg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"CC5XDZqf\"; dkim-atps=neutral","Date":"Wed, 27 Sep 2023 01:33:08 +0300","To":"Jacopo Mondi ","Message-ID":"<20230926223308.GL5854@pendragon.ideasonboard.com>","References":"<20230921165550.50956-1-jacopo.mondi@ideasonboard.com>\n\t<20230921165550.50956-8-jacopo.mondi@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230921165550.50956-8-jacopo.mondi@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 07/13] libcamera: rpi: Make\n\tisRaw/isYuv/isRgb static functions","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]