[{"id":31515,"web_url":"https://patchwork.libcamera.org/comment/31515/","msgid":"","date":"2024-10-01T18:53:22","subject":"Re: [PATCH 2/2] libcamera: pipeline: Use isRaw() helper to check RAW\n\tpixel formats","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Umang\n\nOn Mon, Sep 30, 2024 at 08:50:39PM GMT, Umang Jain wrote:\n> Use the isRaw() helper provided by the PixelFormat class in order to\n> determine if the given pixel format is a raw format or not.\n\nI'm a bit worried our definition of RAW colour encoding is somewhat\nmis-leading, but indeed that's how we use it already\n\n>\n> Signed-off-by: Umang Jain \n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> ---\n> src/ipa/rkisp1/rkisp1.cpp | 3 +--\n> src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 13 ++++--------\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 6 ++----\n> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 20 +++++--------------\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 +++------\n> src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 13 +++++-------\n> 6 files changed, 20 insertions(+), 44 deletions(-)\n>\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 9e161cab..d0917c19 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -273,8 +273,7 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig,\n> \tcontext_.configuration.raw = std::any_of(streamConfig.begin(), streamConfig.end(),\n> \t\t[](auto &cfg) -> bool {\n> \t\t\tPixelFormat pixelFormat{ cfg.second.pixelFormat };\n> -\t\t\tconst PixelFormatInfo &format = PixelFormatInfo::info(pixelFormat);\n> -\t\t\treturn format.colourEncoding == PixelFormatInfo::ColourEncodingRAW;\n> +\t\t\treturn pixelFormat.isRaw();\n> \t\t});\n>\n> \tfor (auto const &a : algorithms()) {\n> diff --git a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> index 72aa6c75..e531ff5e 100644\n> --- a/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> +++ b/src/libcamera/pipeline/imx8-isi/imx8-isi.cpp\n> @@ -313,8 +313,7 @@ unsigned int ISICameraData::getYuvMediaBusFormat(const PixelFormat &pixelFormat)\n>\n> unsigned int ISICameraData::getMediaBusFormat(PixelFormat *pixelFormat) const\n> {\n> -\tif (PixelFormatInfo::info(*pixelFormat).colourEncoding ==\n> -\t PixelFormatInfo::ColourEncodingRAW)\n> +\tif (pixelFormat->isRaw())\n> \t\treturn getRawMediaBusFormat(pixelFormat);\n>\n> \treturn getYuvMediaBusFormat(*pixelFormat);\n> @@ -453,8 +452,7 @@ ISICameraConfiguration::validateYuv(std::set &availableStreams,\n> \t\tLOG(ISI, Debug) << \"Stream \" << i << \": \" << cfg.toString();\n>\n> \t\t/* If the stream is RAW or not supported default it to YUYV. */\n> -\t\tconst PixelFormatInfo &cfgInfo = PixelFormatInfo::info(cfg.pixelFormat);\n> -\t\tif (cfgInfo.colourEncoding == PixelFormatInfo::ColourEncodingRAW ||\n> +\t\tif (cfg.pixelFormat.isRaw() ||\n> \t\t !formatsMap_.count(cfg.pixelFormat)) {\n>\n> \t\t\tLOG(ISI, Debug) << \"Stream \" << i << \" format: \"\n> @@ -522,10 +520,8 @@ CameraConfiguration::Status ISICameraConfiguration::validate()\n> \t\tmaxResolution.width = std::min(2048U, maxResolution.width);\n>\n> \t/* Validate streams according to the format of the first one. */\n> -\tconst PixelFormatInfo info = PixelFormatInfo::info(config_[0].pixelFormat);\n> -\n> \tStatus validationStatus;\n> -\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)\n> +\tif (config_[0].pixelFormat.isRaw())\n> \t\tvalidationStatus = validateRaw(availableStreams, maxResolution);\n> \telse\n> \t\tvalidationStatus = validateYuv(availableStreams, maxResolution);\n> @@ -652,8 +648,7 @@ StreamConfiguration PipelineHandlerISI::generateYUVConfiguration(Camera *camera,\n> \tstd::map> streamFormats;\n>\n> \tfor (const auto &[pixFmt, pipeFmt] : ISICameraConfiguration::formatsMap_) {\n> -\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);\n> -\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)\n> +\t\tif (pixFmt.isRaw())\n> \t\t\tcontinue;\n>\n> \t\tstreamFormats[pixFmt] = { { kMinISISize, sensorSize } };\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 430aa902..4e7f43ac 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -215,9 +215,7 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n> \tSize rawSize;\n>\n> \tfor (const StreamConfiguration &cfg : config_) {\n> -\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);\n> -\n> -\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {\n> +\t\tif (cfg.pixelFormat.isRaw()) {\n> \t\t\trawCount++;\n> \t\t\trawSize = std::max(rawSize, cfg.size);\n> \t\t} else {\n> @@ -285,7 +283,7 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n>\n> \t\tLOG(IPU3, Debug) << \"Validating stream: \" << config_[i].toString();\n>\n> -\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {\n> +\t\tif (config_[i].pixelFormat.isRaw()) {\n> \t\t\t/* Initialize the RAW stream with the CIO2 configuration. */\n> \t\t\tcfg->size = cio2Configuration_.size;\n> \t\t\tcfg->pixelFormat = cio2Configuration_.pixelFormat;\n> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> index 45c71c1d..a659a99c 100644\n> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> @@ -31,16 +31,6 @@\n> #include \"libcamera/internal/v4l2_subdevice.h\"\n> #include \"libcamera/internal/v4l2_videodevice.h\"\n>\n> -namespace {\n> -\n> -bool isFormatRaw(const libcamera::PixelFormat &pixFmt)\n> -{\n> -\treturn libcamera::PixelFormatInfo::info(pixFmt).colourEncoding ==\n> -\t libcamera::PixelFormatInfo::ColourEncodingRAW;\n> -}\n> -\n> -} /* namespace */\n> -\n> namespace libcamera {\n>\n> LOG_DEFINE_CATEGORY(MaliC55)\n> @@ -227,7 +217,7 @@ PixelFormat MaliC55CameraData::bestRawFormat() const\n> \t */\n> \tfor (const auto &maliFormat : maliC55FmtToCode) {\n> \t\tPixelFormat pixFmt = maliFormat.first;\n> -\t\tif (!isFormatRaw(pixFmt))\n> +\t\tif (!pixFmt.isRaw())\n> \t\t\tcontinue;\n>\n> \t\tunsigned int rawCode = maliFormat.second;\n> @@ -330,7 +320,7 @@ CameraConfiguration::Status MaliC55CameraConfiguration::validate()\n> \tbool frPipeAvailable = true;\n> \tStreamConfiguration *rawConfig = nullptr;\n> \tfor (StreamConfiguration &config : config_) {\n> -\t\tif (!isFormatRaw(config.pixelFormat))\n> +\t\tif (!config.pixelFormat.isRaw())\n> \t\t\tcontinue;\n>\n> \t\tif (rawConfig) {\n> @@ -375,7 +365,7 @@ CameraConfiguration::Status MaliC55CameraConfiguration::validate()\n> \t/* Adjust processed streams. */\n> \tSize maxYuvSize;\n> \tfor (StreamConfiguration &config : config_) {\n> -\t\tif (isFormatRaw(config.pixelFormat))\n> +\t\tif (config.pixelFormat.isRaw())\n> \t\t\tcontinue;\n>\n> \t\t/* Adjust format and size for processed streams. */\n> @@ -599,7 +589,7 @@ PipelineHandlerMaliC55::generateConfiguration(Camera *camera,\n> \t\tstd::map> formats;\n> \t\tfor (const auto &maliFormat : maliC55FmtToCode) {\n> \t\t\tPixelFormat pixFmt = maliFormat.first;\n> -\t\t\tbool isRaw = isFormatRaw(pixFmt);\n> +\t\t\tbool isRaw = pixFmt.isRaw();\n>\n> \t\t\t/* RAW formats are only supported on the FR pipe. */\n> \t\t\tif (pipe != &pipes_[MaliC55FR] && isRaw)\n> @@ -795,7 +785,7 @@ int PipelineHandlerMaliC55::configure(Camera *camera,\n> \t\tStream *stream = streamConfig.stream();\n> \t\tMaliC55Pipe *pipe = pipeFromStream(data, stream);\n>\n> -\t\tif (isFormatRaw(streamConfig.pixelFormat))\n> +\t\tif (streamConfig.pixelFormat.isRaw())\n> \t\t\tret = configureRawStream(data, streamConfig, subdevFormat);\n> \t\telse\n> \t\t\tret = configureProcessedStream(data, streamConfig, subdevFormat);\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index c02c7cf3..6cee7893 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -485,8 +485,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> \t */\n> \tif (config_.size() > 1) {\n> \t\tfor (const auto &cfg : config_) {\n> -\t\t\tif (PixelFormatInfo::info(cfg.pixelFormat).colourEncoding ==\n> -\t\t\t PixelFormatInfo::ColourEncodingRAW) {\n> +\t\t\tif (cfg.pixelFormat.isRaw()) {\n> \t\t\t\tconfig_.resize(1);\n> \t\t\t\tstatus = Adjusted;\n> \t\t\t\tbreak;\n> @@ -566,8 +565,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> \tSize maxSize;\n>\n> \tfor (const StreamConfiguration &cfg : config_) {\n> -\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);\n> -\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)\n> +\t\tif (cfg.pixelFormat.isRaw())\n> \t\t\trawFormat = cfg.pixelFormat;\n>\n> \t\tmaxSize = std::max(maxSize, cfg.size);\n> @@ -749,8 +747,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> \t\t<< \" crop \" << rect;\n>\n> \tconst PixelFormat &streamFormat = config->at(0).pixelFormat;\n> -\tconst PixelFormatInfo &info = PixelFormatInfo::info(streamFormat);\n> -\tisRaw_ = info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;\n> +\tisRaw_ = streamFormat.isRaw();\n>\n> \t/* YUYV8_2X8 is required on the ISP source path pad for YUV output. */\n> \tif (!isRaw_)\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> index da8d25c3..cb6117b9 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> @@ -182,10 +182,8 @@ RkISP1Path::generateConfiguration(const CameraSensor *sensor, const Size &size,\n> \tPixelFormat rawFormat;\n>\n> \tfor (const auto &format : streamFormats_) {\n> -\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(format);\n> -\n> \t\t/* Populate stream formats for non-RAW configurations. */\n> -\t\tif (info.colourEncoding != PixelFormatInfo::ColourEncodingRAW) {\n> +\t\tif (!format.isRaw()) {\n> \t\t\tif (role == StreamRole::Raw)\n> \t\t\t\tcontinue;\n>\n> @@ -217,6 +215,7 @@ RkISP1Path::generateConfiguration(const CameraSensor *sensor, const Size &size,\n> \t\t * Store the raw format with the highest bits per pixel for\n> \t\t * later usage.\n> \t\t */\n> +\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(format);\n> \t\tif (info.bitsPerPixel > rawBitsPerPixel) {\n> \t\t\trawBitsPerPixel = info.bitsPerPixel;\n> \t\t\trawFormat = format;\n> @@ -272,9 +271,7 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor,\n> \tbool found = false;\n>\n> \tfor (const auto &format : streamFormats_) {\n> -\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(format);\n> -\n> -\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {\n> +\t\tif (format.isRaw()) {\n> \t\t\t/* Skip raw formats not supported by the sensor. */\n> \t\t\tuint32_t mbusCode = formatToMediaBus.at(format);\n> \t\t\tif (std::find(mbusCodes.begin(), mbusCodes.end(), mbusCode) ==\n> @@ -285,6 +282,7 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor,\n> \t\t\t * Store the raw format with the highest bits per pixel\n> \t\t\t * for later usage.\n> \t\t\t */\n> +\t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(format);\n> \t\t\tif (info.bitsPerPixel > rawBitsPerPixel) {\n> \t\t\t\trawBitsPerPixel = info.bitsPerPixel;\n> \t\t\t\trawFormat = format;\n> @@ -297,8 +295,7 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor,\n> \t\t}\n> \t}\n>\n> -\tbool isRaw = PixelFormatInfo::info(cfg->pixelFormat).colourEncoding ==\n> -\t\t PixelFormatInfo::ColourEncodingRAW;\n> +\tbool isRaw = cfg->pixelFormat.isRaw();\n>\n> \t/*\n> \t * If no raw format supported by the sensor has been found, use a\n> --\n> 2.45.2\n>","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 2945CC3257\n\tfor ;\n\tTue, 1 Oct 2024 18:53:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DF7556351F;\n\tTue, 1 Oct 2024 20:53:28 +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 9165460553\n\tfor ;\n\tTue, 1 Oct 2024 20:53:27 +0200 (CEST)","from ideasonboard.com (unknown [5.77.89.72])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EC9B8A1A;\n\tTue, 1 Oct 2024 20:51:54 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"iXXAOvmx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727808715;\n\tbh=4nqhxejS9O3jTLUd6lyRFLY8Uh7qe+4zH0dDpeDokJI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=iXXAOvmxlLIZzq8PaFLkEQgHSg/1Mrvi4LS9YzqABwmHndfe2fFJUCb2rhjvxYl++\n\t8qR45HuzI8kVnZFzuogRVqBaRZ1lVRW3PfliV5j31b1oMm8F7iZD3PoyZpRl89luUq\n\tYmFBeYKkT57w0hxchBjH7XFjS5Tpc+8YhMPeZusw=","Date":"Tue, 1 Oct 2024 20:53:22 +0200","From":"Jacopo Mondi ","To":"Umang Jain ","Cc":"libcamera-devel@lists.libcamera.org, \n\tLaurent Pinchart ,\n\tJacopo Mondi , \n\tKieran Bingham ,\n\tDaniel Scally ","Subject":"Re: [PATCH 2/2] libcamera: pipeline: Use isRaw() helper to check RAW\n\tpixel formats","Message-ID":"","References":"<20240930152039.72459-1-umang.jain@ideasonboard.com>\n\t<20240930152039.72459-3-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240930152039.72459-3-umang.jain@ideasonboard.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]