[{"id":34891,"web_url":"https://patchwork.libcamera.org/comment/34891/","msgid":"<n6rvwerr5bb73wxpdz5ruhyp4s3zlwvnl6hbyw4i7xpfa2hkfl@e3cfxpnrfnkh>","date":"2025-07-14T14:36:50","subject":"Re: [PATCH v10 5/8] libcamera: simple: Validate raw stream\n\tconfigurations","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"Hi Milan,\n\nOn Fri, Jul 11, 2025 at 07:53:38PM +0200, Milan Zamazal wrote:\n> SimpleCameraConfiguration::validate() looks for the best configuration.\n> As part of enabling raw stream support, the method must consider raw\n> streams in addition to the processed streams.\n> \n> If only a processed stream is requested, nothing changes.\n> \n> If only a raw stream is requested, the pixel format and output size may\n> not be adjusted.  The patch adds checks for this.\n\nCan you explain briefly why is that? My understanding of the current\nstatus-quo is that, it should be possible to adjust the Raw\nstreams/configuration as well. Please take a look at some examples:\n\t($) git grep -ni raw | grep -i adjust\n\nFor sake of completeness, the case where this is not adjust-able is\nwhen CameraConfiguration::sensorConfig is passed by the application.\n\n> \n> If both processed and raw streams are requested, things get more\n> complicated.  The raw stream is expected to be passed through intact and\n> all the adjustments are made for the processed streams.  We select a\n> pipe configuration for the processed streams.\n> \n> Note that with both processed and raw streams, the requested sizes must\n> be mutually matching, including resizing due to debayer requirements.\n> For example, the following `cam' setup is valid for imx219\n> \n>   cam -s role=viewfinder,width=1920,height=1080 \\\n>       -s role=raw,width=3280,height=2464\n> \n> rather than\n> \n>   cam -s role=viewfinder,width=1920,height=1080 \\\n>       -s role=raw,width=1920,height=1080\n> \n> due to the resolution of 1924x1080 actually selected for debayering to\n> 1920x1080.  It is the application responsibility to select the right\n> parameters for the raw stream.\n> \n> Setting up the right configurations is still not enough to make the raw\n> streams working.  Buffer handling must be changed in the simple\n> pipeline, which is addressed in followup patches.\n> \n> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> ---\n>  src/libcamera/pipeline/simple/simple.cpp | 119 ++++++++++++++++-------\n>  1 file changed, 85 insertions(+), 34 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 37abaa0e0..9d87a7ffa 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -27,6 +27,7 @@\n>  #include <libcamera/camera.h>\n>  #include <libcamera/color_space.h>\n>  #include <libcamera/control_ids.h>\n> +#include <libcamera/geometry.h>\n>  #include <libcamera/pixel_format.h>\n>  #include <libcamera/request.h>\n>  #include <libcamera/stream.h>\n> @@ -1186,6 +1187,20 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>  \t\t<< \"-\" << pipeConfig_->captureFormat\n>  \t\t<< \" for max stream size \" << maxStreamSize;\n>  \n> +\tbool rawRequested = false;\n> +\tbool processedRequested = false;\n> +\tfor (const auto &cfg : config_)\n> +\t\tif (cfg.colorSpace == ColorSpace::Raw) {\n> +\t\t\tif (rawRequested) {\n> +\t\t\t\tLOG(SimplePipeline, Error)\n> +\t\t\t\t\t<< \"Can't capture multiple raw streams\";\n> +\t\t\t\treturn Invalid;\n> +\t\t\t}\n> +\t\t\trawRequested = true;\n> +\t\t} else {\n> +\t\t\tprocessedRequested = true;\n> +\t\t}\n> +\n>  \t/*\n>  \t * Adjust the requested streams.\n>  \t *\n> @@ -1204,43 +1219,70 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>  \tfor (unsigned int i = 0; i < config_.size(); ++i) {\n>  \t\tStreamConfiguration &cfg = config_[i];\n>  \n> +\t\t/*\n> +\t\t * If both processed and raw streams are requested, the pipe\n> +\t\t * configuration is set up for the processed stream. The raw\n> +\t\t * configuration needs to be compared against the capture format and\n> +\t\t * size in such a case.\n> +\t\t */\n> +\t\tconst bool rawStream = cfg.colorSpace == ColorSpace::Raw;\n> +\t\tconst bool sideRawStream = rawStream && processedRequested;\n> +\n>  \t\t/* Adjust the pixel format and size. */\n> -\t\tauto it = std::find(pipeConfig_->outputFormats.begin(),\n> -\t\t\t\t    pipeConfig_->outputFormats.end(),\n> -\t\t\t\t    cfg.pixelFormat);\n> -\t\tif (it == pipeConfig_->outputFormats.end())\n> -\t\t\tit = pipeConfig_->outputFormats.begin();\n> -\n> -\t\tPixelFormat pixelFormat = *it;\n> -\t\tif (cfg.pixelFormat != pixelFormat) {\n> -\t\t\tLOG(SimplePipeline, Debug) << \"Adjusting pixel format\";\n> -\t\t\tcfg.pixelFormat = pixelFormat;\n> -\t\t\t/*\n> -\t\t\t * Do not touch the colour space for raw requested roles.\n> -\t\t\t * Even if the pixel format is non-raw (whatever it means), we\n> -\t\t\t * shouldn't try to interpret the colour space of raw data.\n> -\t\t\t */\n> -\t\t\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)\n> +\n> +\t\tif (!sideRawStream) {\n> +\t\t\tPixelFormat pixelFormat;\n> +\t\t\tif (rawStream) {\n> +\t\t\t\tpixelFormat = pipeConfig_->captureFormat;\n> +\t\t\t} else {\n> +\t\t\t\tauto it = std::find(pipeConfig_->outputFormats.begin(),\n> +\t\t\t\t\t\t    pipeConfig_->outputFormats.end(),\n> +\t\t\t\t\t\t    cfg.pixelFormat);\n> +\t\t\t\tif (it == pipeConfig_->outputFormats.end())\n> +\t\t\t\t\tit = pipeConfig_->outputFormats.begin();\n> +\t\t\t\tpixelFormat = *it;\n> +\t\t\t}\n> +\n> +\t\t\tif (cfg.pixelFormat != pixelFormat) {\n> +\t\t\t\tif (rawStream) {\n> +\t\t\t\t\tLOG(SimplePipeline, Info)\n> +\t\t\t\t\t\t<< \"Raw pixel format \"\n> +\t\t\t\t\t\t<< cfg.pixelFormat\n> +\t\t\t\t\t\t<< \" doesn't match any of the pipe output formats\";\n> +\t\t\t\t\treturn Invalid;\n> +\t\t\t\t}\n> +\t\t\t\tLOG(SimplePipeline, Debug)\n> +\t\t\t\t\t<< \"Adjusting pixel format from \" << cfg.pixelFormat\n> +\t\t\t\t\t<< \" to \" << pixelFormat;\n> +\t\t\t\tcfg.pixelFormat = pixelFormat;\n> +\t\t\t\t/*\n> +\t\t\t\t * Do not touch the colour space for raw requested roles.\n> +\t\t\t\t * Even if the pixel format is non-raw (whatever it means), we\n> +\t\t\t\t * shouldn't try to interpret the colour space of raw data.\n> +\t\t\t\t */\n> +\t\t\t\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)\n> +\t\t\t\t\tcfg.colorSpace->adjust(pixelFormat);\n> +\t\t\t\tstatus = Adjusted;\n> +\t\t\t}\n> +\n> +\t\t\tif (!cfg.colorSpace) {\n> +\t\t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);\n> +\t\t\t\tswitch (info.colourEncoding) {\n> +\t\t\t\tcase PixelFormatInfo::ColourEncodingRGB:\n> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Srgb;\n> +\t\t\t\t\tbreak;\n> +\t\t\t\tcase libcamera::PixelFormatInfo::ColourEncodingYUV:\n> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Sycc;\n> +\t\t\t\t\tbreak;\n> +\t\t\t\tdefault:\n> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Raw;\n> +\t\t\t\t}\n>  \t\t\t\tcfg.colorSpace->adjust(pixelFormat);\n> -\t\t\tstatus = Adjusted;\n> -\t\t}\n> -\t\tif (!cfg.colorSpace) {\n> -\t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);\n> -\t\t\tswitch (info.colourEncoding) {\n> -\t\t\tcase PixelFormatInfo::ColourEncodingRGB:\n> -\t\t\t\tcfg.colorSpace = ColorSpace::Srgb;\n> -\t\t\t\tbreak;\n> -\t\t\tcase libcamera::PixelFormatInfo::ColourEncodingYUV:\n> -\t\t\t\tcfg.colorSpace = ColorSpace::Sycc;\n> -\t\t\t\tbreak;\n> -\t\t\tdefault:\n> -\t\t\t\tcfg.colorSpace = ColorSpace::Raw;\n> +\t\t\t\tstatus = Adjusted;\n>  \t\t\t}\n> -\t\t\tcfg.colorSpace->adjust(pixelFormat);\n> -\t\t\tstatus = Adjusted;\n>  \t\t}\n>  \n> -\t\tif (!pipeConfig_->outputSizes.contains(cfg.size)) {\n> +\t\tif (!rawStream && !pipeConfig_->outputSizes.contains(cfg.size)) {\n>  \t\t\tSize adjustedSize = pipeConfig_->captureSize;\n>  \t\t\t/*\n>  \t\t\t * The converter (when present) may not be able to output\n> @@ -1259,11 +1301,20 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>  \n>  \t\t/* \\todo Create a libcamera core class to group format and size */\n>  \t\tif (cfg.pixelFormat != pipeConfig_->captureFormat ||\n> -\t\t    cfg.size != pipeConfig_->captureSize)\n> +\t\t    cfg.size != pipeConfig_->captureSize) {\n> +\t\t\tif (rawStream) {\n> +\t\t\t\tLOG(SimplePipeline, Info)\n> +\t\t\t\t\t<< \"Raw output format \" << cfg.pixelFormat\n> +\t\t\t\t\t<< \" and size \" << cfg.size\n> +\t\t\t\t\t<< \" not matching pipe format \" << pipeConfig_->captureFormat\n> +\t\t\t\t\t<< \" and size \" << pipeConfig_->captureSize;\n> +\t\t\t\treturn Invalid;\n> +\t\t\t}\n>  \t\t\tneedConversion_ = true;\n> +\t\t}\n>  \n>  \t\t/* Set the stride, frameSize and bufferCount. */\n> -\t\tif (needConversion_) {\n> +\t\tif (needConversion_ && !rawStream) {\n>  \t\t\tstd::tie(cfg.stride, cfg.frameSize) =\n>  \t\t\t\tdata_->converter_\n>  \t\t\t\t\t? data_->converter_->strideAndFrameSize(cfg.pixelFormat,\n> -- \n> 2.50.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 3AD71BE175\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 14 Jul 2025 14:36:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3CE2368F47;\n\tMon, 14 Jul 2025 16:36:52 +0200 (CEST)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4177868F3F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 14 Jul 2025 16:36:48 +0200 (CEST)","from [49.36.69.193] (helo=uajain)\n\tby fanzine2.igalia.com with esmtpsa \n\t(Cipher TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256)\n\t(Exim) id 1ubKIQ-00GRtX-Rq; Mon, 14 Jul 2025 16:36:47 +0200"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=igalia.com header.i=@igalia.com\n\theader.b=\"TDtiUe26\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com;\n\ts=20170329;\n\th=In-Reply-To:Content-Type:MIME-Version:References:Message-ID:\n\tSubject:Cc:To:From:Date:Sender:Reply-To:Content-Transfer-Encoding:Content-ID:\n\tContent-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc\n\t:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe:\n\tList-Post:List-Owner:List-Archive;\n\tbh=U9GIB7sJ88CZXdvyGKoEG2CIpj6IwYO2Z7Vyl2nKMNY=;\n\tb=TDtiUe26Qr/tiPci0mlmSufdNl\n\tLoz7h6RL4qyTqPIqovGR66S9hTo697kfS6q7KAebfMtdHgxXr8AL9DJhR5Tz70W2hdT4NfMwRCOA6\n\tCpOX/KsEcxe3oFGvKHbk0v6aTfS2FBkyo6HZcfzFYYZ91YxcYDHWKqv29r+AkR/dfOP0bhH7Ttiqp\n\tnDPdWPqBMYcCPyhsQbFn9LHPW39Z7D/ZWPbqTG+WsSGsvcViHULEIN+DZIs/rn5oT7UC0B1L9k9sZ\n\tZo5p0KFGZijPhJOWgpF5R3597bwXrlvQ4uLt7nOk5rql0qE44LEKj/Pr2b2nQ75PR8Na1ZDFhkarp\n\tk/JhCAeg==;","Date":"Mon, 14 Jul 2025 20:06:50 +0530","From":"Umang Jain <uajain@igalia.com>","To":"Milan Zamazal <mzamazal@redhat.com>","Cc":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t<laurent.pinchart@ideasonboard.com>, Kieran Bingham\n\t<kieran.bingham@ideasonboard.com>, =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t<barnabas.pocze@ideasonboard.com>, Paul Elder\n\t<paul.elder@ideasonboard.com>","Subject":"Re: [PATCH v10 5/8] libcamera: simple: Validate raw stream\n\tconfigurations","Message-ID":"<n6rvwerr5bb73wxpdz5ruhyp4s3zlwvnl6hbyw4i7xpfa2hkfl@e3cfxpnrfnkh>","References":"<20250711175345.90318-1-mzamazal@redhat.com>\n\t<20250711175345.90318-6-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20250711175345.90318-6-mzamazal@redhat.com>","User-Agent":"NeoMutt/20250510-dirty","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":34892,"web_url":"https://patchwork.libcamera.org/comment/34892/","msgid":"<85freyd9p9.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-07-14T16:21:54","subject":"Re: [PATCH v10 5/8] libcamera: simple: Validate raw stream\n\tconfigurations","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Umang,\n\nUmang Jain <uajain@igalia.com> writes:\n\n> Hi Milan,\n>\n> On Fri, Jul 11, 2025 at 07:53:38PM +0200, Milan Zamazal wrote:\n>> SimpleCameraConfiguration::validate() looks for the best configuration.\n>> As part of enabling raw stream support, the method must consider raw\n>> streams in addition to the processed streams.\n>> \n>> If only a processed stream is requested, nothing changes.\n>> \n>> If only a raw stream is requested, the pixel format and output size may\n>> not be adjusted.  The patch adds checks for this.\n>\n> Can you explain briefly why is that? My understanding of the current\n> status-quo is that, it should be possible to adjust the Raw\n> streams/configuration as well. Please take a look at some examples:\n> \t($) git grep -ni raw | grep -i adjust\n>\n> For sake of completeness, the case where this is not adjust-able is\n> when CameraConfiguration::sensorConfig is passed by the application.\n\nRight, I missed the difference between an explicitly provided\nconfiguration by the application and the case when a different raw\nformat may be selected.  I'll try to fix it.\n\n>> \n>> If both processed and raw streams are requested, things get more\n>> complicated.  The raw stream is expected to be passed through intact and\n>> all the adjustments are made for the processed streams.  We select a\n>> pipe configuration for the processed streams.\n>> \n>> Note that with both processed and raw streams, the requested sizes must\n>> be mutually matching, including resizing due to debayer requirements.\n>> For example, the following `cam' setup is valid for imx219\n>> \n>>   cam -s role=viewfinder,width=1920,height=1080 \\\n>>       -s role=raw,width=3280,height=2464\n>> \n>> rather than\n>> \n>>   cam -s role=viewfinder,width=1920,height=1080 \\\n>>       -s role=raw,width=1920,height=1080\n>> \n>> due to the resolution of 1924x1080 actually selected for debayering to\n>> 1920x1080.  It is the application responsibility to select the right\n>> parameters for the raw stream.\n>> \n>> Setting up the right configurations is still not enough to make the raw\n>> streams working.  Buffer handling must be changed in the simple\n>> pipeline, which is addressed in followup patches.\n>> \n>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n>> ---\n>>  src/libcamera/pipeline/simple/simple.cpp | 119 ++++++++++++++++-------\n>>  1 file changed, 85 insertions(+), 34 deletions(-)\n>> \n>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>> index 37abaa0e0..9d87a7ffa 100644\n>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>> @@ -27,6 +27,7 @@\n>>  #include <libcamera/camera.h>\n>>  #include <libcamera/color_space.h>\n>>  #include <libcamera/control_ids.h>\n>> +#include <libcamera/geometry.h>\n>>  #include <libcamera/pixel_format.h>\n>>  #include <libcamera/request.h>\n>>  #include <libcamera/stream.h>\n>> @@ -1186,6 +1187,20 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>  \t\t<< \"-\" << pipeConfig_->captureFormat\n>>  \t\t<< \" for max stream size \" << maxStreamSize;\n>>  \n>> +\tbool rawRequested = false;\n>> +\tbool processedRequested = false;\n>> +\tfor (const auto &cfg : config_)\n>> +\t\tif (cfg.colorSpace == ColorSpace::Raw) {\n>> +\t\t\tif (rawRequested) {\n>> +\t\t\t\tLOG(SimplePipeline, Error)\n>> +\t\t\t\t\t<< \"Can't capture multiple raw streams\";\n>> +\t\t\t\treturn Invalid;\n>> +\t\t\t}\n>> +\t\t\trawRequested = true;\n>> +\t\t} else {\n>> +\t\t\tprocessedRequested = true;\n>> +\t\t}\n>> +\n>>  \t/*\n>>  \t * Adjust the requested streams.\n>>  \t *\n>> @@ -1204,43 +1219,70 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>  \tfor (unsigned int i = 0; i < config_.size(); ++i) {\n>>  \t\tStreamConfiguration &cfg = config_[i];\n>>  \n>> +\t\t/*\n>> +\t\t * If both processed and raw streams are requested, the pipe\n>> +\t\t * configuration is set up for the processed stream. The raw\n>> +\t\t * configuration needs to be compared against the capture format and\n>> +\t\t * size in such a case.\n>> +\t\t */\n>> +\t\tconst bool rawStream = cfg.colorSpace == ColorSpace::Raw;\n>> +\t\tconst bool sideRawStream = rawStream && processedRequested;\n>> +\n>>  \t\t/* Adjust the pixel format and size. */\n>> -\t\tauto it = std::find(pipeConfig_->outputFormats.begin(),\n>> -\t\t\t\t    pipeConfig_->outputFormats.end(),\n>> -\t\t\t\t    cfg.pixelFormat);\n>> -\t\tif (it == pipeConfig_->outputFormats.end())\n>> -\t\t\tit = pipeConfig_->outputFormats.begin();\n>> -\n>> -\t\tPixelFormat pixelFormat = *it;\n>> -\t\tif (cfg.pixelFormat != pixelFormat) {\n>> -\t\t\tLOG(SimplePipeline, Debug) << \"Adjusting pixel format\";\n>> -\t\t\tcfg.pixelFormat = pixelFormat;\n>> -\t\t\t/*\n>> -\t\t\t * Do not touch the colour space for raw requested roles.\n>> -\t\t\t * Even if the pixel format is non-raw (whatever it means), we\n>> -\t\t\t * shouldn't try to interpret the colour space of raw data.\n>> -\t\t\t */\n>> -\t\t\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)\n>> +\n>> +\t\tif (!sideRawStream) {\n>> +\t\t\tPixelFormat pixelFormat;\n>> +\t\t\tif (rawStream) {\n>> +\t\t\t\tpixelFormat = pipeConfig_->captureFormat;\n>> +\t\t\t} else {\n>> +\t\t\t\tauto it = std::find(pipeConfig_->outputFormats.begin(),\n>> +\t\t\t\t\t\t    pipeConfig_->outputFormats.end(),\n>> +\t\t\t\t\t\t    cfg.pixelFormat);\n>> +\t\t\t\tif (it == pipeConfig_->outputFormats.end())\n>> +\t\t\t\t\tit = pipeConfig_->outputFormats.begin();\n>> +\t\t\t\tpixelFormat = *it;\n>> +\t\t\t}\n>> +\n>> +\t\t\tif (cfg.pixelFormat != pixelFormat) {\n>> +\t\t\t\tif (rawStream) {\n>> +\t\t\t\t\tLOG(SimplePipeline, Info)\n>> +\t\t\t\t\t\t<< \"Raw pixel format \"\n>> +\t\t\t\t\t\t<< cfg.pixelFormat\n>> +\t\t\t\t\t\t<< \" doesn't match any of the pipe output formats\";\n>> +\t\t\t\t\treturn Invalid;\n>> +\t\t\t\t}\n>> +\t\t\t\tLOG(SimplePipeline, Debug)\n>> +\t\t\t\t\t<< \"Adjusting pixel format from \" << cfg.pixelFormat\n>> +\t\t\t\t\t<< \" to \" << pixelFormat;\n>> +\t\t\t\tcfg.pixelFormat = pixelFormat;\n>> +\t\t\t\t/*\n>> +\t\t\t\t * Do not touch the colour space for raw requested roles.\n>> +\t\t\t\t * Even if the pixel format is non-raw (whatever it means), we\n>> +\t\t\t\t * shouldn't try to interpret the colour space of raw data.\n>> +\t\t\t\t */\n>> +\t\t\t\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)\n>> +\t\t\t\t\tcfg.colorSpace->adjust(pixelFormat);\n>> +\t\t\t\tstatus = Adjusted;\n>> +\t\t\t}\n>> +\n>> +\t\t\tif (!cfg.colorSpace) {\n>> +\t\t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);\n>> +\t\t\t\tswitch (info.colourEncoding) {\n>> +\t\t\t\tcase PixelFormatInfo::ColourEncodingRGB:\n>> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Srgb;\n>> +\t\t\t\t\tbreak;\n>> +\t\t\t\tcase libcamera::PixelFormatInfo::ColourEncodingYUV:\n>> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Sycc;\n>> +\t\t\t\t\tbreak;\n>> +\t\t\t\tdefault:\n>> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Raw;\n>> +\t\t\t\t}\n>>  \t\t\t\tcfg.colorSpace->adjust(pixelFormat);\n>> -\t\t\tstatus = Adjusted;\n>> -\t\t}\n>> -\t\tif (!cfg.colorSpace) {\n>> -\t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);\n>> -\t\t\tswitch (info.colourEncoding) {\n>> -\t\t\tcase PixelFormatInfo::ColourEncodingRGB:\n>> -\t\t\t\tcfg.colorSpace = ColorSpace::Srgb;\n>> -\t\t\t\tbreak;\n>> -\t\t\tcase libcamera::PixelFormatInfo::ColourEncodingYUV:\n>> -\t\t\t\tcfg.colorSpace = ColorSpace::Sycc;\n>> -\t\t\t\tbreak;\n>> -\t\t\tdefault:\n>> -\t\t\t\tcfg.colorSpace = ColorSpace::Raw;\n>> +\t\t\t\tstatus = Adjusted;\n>>  \t\t\t}\n>> -\t\t\tcfg.colorSpace->adjust(pixelFormat);\n>> -\t\t\tstatus = Adjusted;\n>>  \t\t}\n>>  \n>> -\t\tif (!pipeConfig_->outputSizes.contains(cfg.size)) {\n>> +\t\tif (!rawStream && !pipeConfig_->outputSizes.contains(cfg.size)) {\n>>  \t\t\tSize adjustedSize = pipeConfig_->captureSize;\n>>  \t\t\t/*\n>>  \t\t\t * The converter (when present) may not be able to output\n>> @@ -1259,11 +1301,20 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>  \n>>  \t\t/* \\todo Create a libcamera core class to group format and size */\n>>  \t\tif (cfg.pixelFormat != pipeConfig_->captureFormat ||\n>> -\t\t    cfg.size != pipeConfig_->captureSize)\n>> +\t\t    cfg.size != pipeConfig_->captureSize) {\n>> +\t\t\tif (rawStream) {\n>> +\t\t\t\tLOG(SimplePipeline, Info)\n>> +\t\t\t\t\t<< \"Raw output format \" << cfg.pixelFormat\n>> +\t\t\t\t\t<< \" and size \" << cfg.size\n>> +\t\t\t\t\t<< \" not matching pipe format \" << pipeConfig_->captureFormat\n>> +\t\t\t\t\t<< \" and size \" << pipeConfig_->captureSize;\n>> +\t\t\t\treturn Invalid;\n>> +\t\t\t}\n>>  \t\t\tneedConversion_ = true;\n>> +\t\t}\n>>  \n>>  \t\t/* Set the stride, frameSize and bufferCount. */\n>> -\t\tif (needConversion_) {\n>> +\t\tif (needConversion_ && !rawStream) {\n>>  \t\t\tstd::tie(cfg.stride, cfg.frameSize) =\n>>  \t\t\t\tdata_->converter_\n>>  \t\t\t\t\t? data_->converter_->strideAndFrameSize(cfg.pixelFormat,\n>> -- \n>> 2.50.1\n>>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 D8943C3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 14 Jul 2025 16:22:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9F4C168F4A;\n\tMon, 14 Jul 2025 18:22:02 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3813468F3F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 14 Jul 2025 18:22:01 +0200 (CEST)","from mail-ed1-f70.google.com (mail-ed1-f70.google.com\n\t[209.85.208.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-286-6rdM1ro7PAG8YQxIcmQPAw-1; Mon, 14 Jul 2025 12:21:58 -0400","by mail-ed1-f70.google.com with SMTP id\n\t4fb4d7f45d1cf-6077833ae13so3719483a12.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 14 Jul 2025 09:21:58 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\t4fb4d7f45d1cf-611c9524072sm6316112a12.20.2025.07.14.09.21.55\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 14 Jul 2025 09:21:55 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"FkTjZxAZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1752510120;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=sQuYjCeLjOB+j9UENI8lJqOK0QkYTPEvdzulaQo2SHs=;\n\tb=FkTjZxAZyTowKlQYS9r/xwHHAdybszXLORmkyfYiOAlgKXd2Pob8QJ+XIl5ChhIjUnh5JW\n\tdhx0TmokAYjM9CWMfQL7k+o+YYSgYV92jvMxQ8o46SpV+Q1iu3uT18cdnFfN3Ta/WQl/C9\n\tThIvjsTaj/qz7GNuTQkXoMDIJWfSTcM=","X-MC-Unique":"6rdM1ro7PAG8YQxIcmQPAw-1","X-Mimecast-MFC-AGG-ID":"6rdM1ro7PAG8YQxIcmQPAw_1752510118","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1752510117; x=1753114917;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=sQuYjCeLjOB+j9UENI8lJqOK0QkYTPEvdzulaQo2SHs=;\n\tb=dhx+2SOwmh1Iy9i6dfItPSe5Xe5i71u7gFuEUIUcHGz7NmQtWYgoTQNX35UOOqcxRI\n\tORjss3fFmjxYghOs7jcZc+BcCsE5u24e4q9pV1vWOPWNui23wueFHWhhVOPQ4GQAh4N6\n\tyroo8FeqxhggVTYBy/VgV6RNmKrK7E0OEjPPaX+thpy/cATGM9Fu5IX35V57VYS4x4JJ\n\tZLSh2iv6J/cfiytqiD4jtdH7GEhQVEKMK7vYEdjX4LQw/yHhn6quGCZYKKTHAk3HWhk8\n\tESOcCfaTpQ+8pVW5QPlual/0LVK5UZWL2RHpha/c9fRXiSpT6mkA68WklyjIvK/cnR3K\n\twfRQ==","X-Gm-Message-State":"AOJu0YwBn1djIeL03kverffx8WH8PfHnWB1DlikdHJ0yToL5fSF+v8su\n\tJO91Cfu/XNupRgNrS06cCsnrtcTldody8e1Iozhqu6hWfjFK/vpi9cJqNWAvKEsZnchxK81gtPQ\n\tHPmBBprEHmrBPaetmo8K8LLwlK54pRXlFl0DTT+xVigdEvmL7w1FHKMXzMgyfAfiEtJIU/v7DeW\n\tdk0/6l08I=","X-Gm-Gg":"ASbGncsPTmht6coTu8Y/8kkGyggBl92lbe9aaeWg2hJe2sxQ+jkDnAyBWgPgZ1wJ0ya\n\tE40F5MX3Pljq51hOaECXirRby/S9KAi/SMxvXBiDo3enIJ33cVeoCqk69x6lRRavTHdTb+SExNS\n\txXRuppt0fO+YkAUMF/cgszdFKnBUbTO56d+l/4nJDZso0LElYfquZUK8VVjssl50EYL245O1h84\n\t1PGBpNlZq8cNEJZxwxY0bUM3EePAZCOrIAmqme56v1WbwAflLNmIeX7xVnAHfImeruJSsoHn+16\n\t/AQN1jVdVETkdNhe6915YeRa0ZWXujcBA0g9TFg+FB2i3tMGOBeMBwZgbbpv6JaCCfJPIuWhamW\n\tEepPVo9TVRlWHrknz","X-Received":["by 2002:a05:6402:1e96:b0:607:ec18:9410 with SMTP id\n\t4fb4d7f45d1cf-611e76113a3mr11581197a12.3.1752510117035; \n\tMon, 14 Jul 2025 09:21:57 -0700 (PDT)","by 2002:a05:6402:1e96:b0:607:ec18:9410 with SMTP id\n\t4fb4d7f45d1cf-611e76113a3mr11581169a12.3.1752510116551; \n\tMon, 14 Jul 2025 09:21:56 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IEkp/Hu6xeNmqlklkzV0CmsJIZMpuIImhHiYcGWX55UcBbdoyiR/56ewfYQ+gomZ7XhcPCySw==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Umang Jain <uajain@igalia.com>","Cc":"libcamera-devel@lists.libcamera.org,  Laurent Pinchart\n\t<laurent.pinchart@ideasonboard.com>,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>, =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?=\n\t<barnabas.pocze@ideasonboard.com>, Paul Elder\n\t<paul.elder@ideasonboard.com>","Subject":"Re: [PATCH v10 5/8] libcamera: simple: Validate raw stream\n\tconfigurations","In-Reply-To":"<n6rvwerr5bb73wxpdz5ruhyp4s3zlwvnl6hbyw4i7xpfa2hkfl@e3cfxpnrfnkh>\n\t(Umang Jain's message of \"Mon, 14 Jul 2025 20:06:50 +0530\")","References":"<20250711175345.90318-1-mzamazal@redhat.com>\n\t<20250711175345.90318-6-mzamazal@redhat.com>\n\t<n6rvwerr5bb73wxpdz5ruhyp4s3zlwvnl6hbyw4i7xpfa2hkfl@e3cfxpnrfnkh>","Date":"Mon, 14 Jul 2025 18:21:54 +0200","Message-ID":"<85freyd9p9.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"HyqD7qA-hzSliGhbxdQvCc4hxOTXReSjsecfEfWEZbw_1752510118","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":34895,"web_url":"https://patchwork.libcamera.org/comment/34895/","msgid":"<789C12A8-ACEE-41B3-82BD-2254F4F0E38B@igalia.com>","date":"2025-07-14T18:13:46","subject":"Re: [PATCH v10 5/8] libcamera: simple: Validate raw stream\n\tconfigurations","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"Hi Milan,\n\n\nOn 14 July 2025 9:51:54 pm IST, Milan Zamazal <mzamazal@redhat.com> wrote:\n>Hi Umang,\n>\n>Umang Jain <uajain@igalia.com> writes:\n>\n>> Hi Milan,\n>>\n>> On Fri, Jul 11, 2025 at 07:53:38PM +0200, Milan Zamazal wrote:\n>>> SimpleCameraConfiguration::validate() looks for the best configuration.\n>>> As part of enabling raw stream support, the method must consider raw\n>>> streams in addition to the processed streams.\n>>> \n>>> If only a processed stream is requested, nothing changes.\n>>> \n>>> If only a raw stream is requested, the pixel format and output size may\n>>> not be adjusted.  The patch adds checks for this.\n>>\n>> Can you explain briefly why is that? My understanding of the current\n>> status-quo is that, it should be possible to adjust the Raw\n>> streams/configuration as well. Please take a look at some examples:\n>> \t($) git grep -ni raw | grep -i adjust\n>>\n>> For sake of completeness, the case where this is not adjust-able is\n>> when CameraConfiguration::sensorConfig is passed by the application.\n>\n>Right, I missed the difference between an explicitly provided\n>configuration by the application and the case when a different raw\n>format may be selected.  I'll try to fix it.\n\nI have a few patches in my branch that are in this direction. I can share the branch if you want (although it's under development, but I have rewritten the validation  and generate configuration logic to what I had in mind). \n\n>\n>>> \n>>> If both processed and raw streams are requested, things get more\n>>> complicated.  The raw stream is expected to be passed through intact and\n>>> all the adjustments are made for the processed streams.  We select a\n>>> pipe configuration for the processed streams.\n>>> \n>>> Note that with both processed and raw streams, the requested sizes must\n>>> be mutually matching, including resizing due to debayer requirements.\n>>> For example, the following `cam' setup is valid for imx219\n>>> \n>>>   cam -s role=viewfinder,width=1920,height=1080 \\\n>>>       -s role=raw,width=3280,height=2464\n>>> \n>>> rather than\n>>> \n>>>   cam -s role=viewfinder,width=1920,height=1080 \\\n>>>       -s role=raw,width=1920,height=1080\n>>> \n>>> due to the resolution of 1924x1080 actually selected for debayering to\n>>> 1920x1080.  It is the application responsibility to select the right\n>>> parameters for the raw stream.\n>>> \n>>> Setting up the right configurations is still not enough to make the raw\n>>> streams working.  Buffer handling must be changed in the simple\n>>> pipeline, which is addressed in followup patches.\n>>> \n>>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n>>> ---\n>>>  src/libcamera/pipeline/simple/simple.cpp | 119 ++++++++++++++++-------\n>>>  1 file changed, 85 insertions(+), 34 deletions(-)\n>>> \n>>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>>> index 37abaa0e0..9d87a7ffa 100644\n>>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>>> @@ -27,6 +27,7 @@\n>>>  #include <libcamera/camera.h>\n>>>  #include <libcamera/color_space.h>\n>>>  #include <libcamera/control_ids.h>\n>>> +#include <libcamera/geometry.h>\n>>>  #include <libcamera/pixel_format.h>\n>>>  #include <libcamera/request.h>\n>>>  #include <libcamera/stream.h>\n>>> @@ -1186,6 +1187,20 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>>  \t\t<< \"-\" << pipeConfig_->captureFormat\n>>>  \t\t<< \" for max stream size \" << maxStreamSize;\n>>>  \n>>> +\tbool rawRequested = false;\n>>> +\tbool processedRequested = false;\n>>> +\tfor (const auto &cfg : config_)\n>>> +\t\tif (cfg.colorSpace == ColorSpace::Raw) {\n>>> +\t\t\tif (rawRequested) {\n>>> +\t\t\t\tLOG(SimplePipeline, Error)\n>>> +\t\t\t\t\t<< \"Can't capture multiple raw streams\";\n>>> +\t\t\t\treturn Invalid;\n>>> +\t\t\t}\n>>> +\t\t\trawRequested = true;\n>>> +\t\t} else {\n>>> +\t\t\tprocessedRequested = true;\n>>> +\t\t}\n>>> +\n>>>  \t/*\n>>>  \t * Adjust the requested streams.\n>>>  \t *\n>>> @@ -1204,43 +1219,70 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>>  \tfor (unsigned int i = 0; i < config_.size(); ++i) {\n>>>  \t\tStreamConfiguration &cfg = config_[i];\n>>>  \n>>> +\t\t/*\n>>> +\t\t * If both processed and raw streams are requested, the pipe\n>>> +\t\t * configuration is set up for the processed stream. The raw\n>>> +\t\t * configuration needs to be compared against the capture format and\n>>> +\t\t * size in such a case.\n>>> +\t\t */\n>>> +\t\tconst bool rawStream = cfg.colorSpace == ColorSpace::Raw;\n>>> +\t\tconst bool sideRawStream = rawStream && processedRequested;\n>>> +\n>>>  \t\t/* Adjust the pixel format and size. */\n>>> -\t\tauto it = std::find(pipeConfig_->outputFormats.begin(),\n>>> -\t\t\t\t    pipeConfig_->outputFormats.end(),\n>>> -\t\t\t\t    cfg.pixelFormat);\n>>> -\t\tif (it == pipeConfig_->outputFormats.end())\n>>> -\t\t\tit = pipeConfig_->outputFormats.begin();\n>>> -\n>>> -\t\tPixelFormat pixelFormat = *it;\n>>> -\t\tif (cfg.pixelFormat != pixelFormat) {\n>>> -\t\t\tLOG(SimplePipeline, Debug) << \"Adjusting pixel format\";\n>>> -\t\t\tcfg.pixelFormat = pixelFormat;\n>>> -\t\t\t/*\n>>> -\t\t\t * Do not touch the colour space for raw requested roles.\n>>> -\t\t\t * Even if the pixel format is non-raw (whatever it means), we\n>>> -\t\t\t * shouldn't try to interpret the colour space of raw data.\n>>> -\t\t\t */\n>>> -\t\t\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)\n>>> +\n>>> +\t\tif (!sideRawStream) {\n>>> +\t\t\tPixelFormat pixelFormat;\n>>> +\t\t\tif (rawStream) {\n>>> +\t\t\t\tpixelFormat = pipeConfig_->captureFormat;\n>>> +\t\t\t} else {\n>>> +\t\t\t\tauto it = std::find(pipeConfig_->outputFormats.begin(),\n>>> +\t\t\t\t\t\t    pipeConfig_->outputFormats.end(),\n>>> +\t\t\t\t\t\t    cfg.pixelFormat);\n>>> +\t\t\t\tif (it == pipeConfig_->outputFormats.end())\n>>> +\t\t\t\t\tit = pipeConfig_->outputFormats.begin();\n>>> +\t\t\t\tpixelFormat = *it;\n>>> +\t\t\t}\n>>> +\n>>> +\t\t\tif (cfg.pixelFormat != pixelFormat) {\n>>> +\t\t\t\tif (rawStream) {\n>>> +\t\t\t\t\tLOG(SimplePipeline, Info)\n>>> +\t\t\t\t\t\t<< \"Raw pixel format \"\n>>> +\t\t\t\t\t\t<< cfg.pixelFormat\n>>> +\t\t\t\t\t\t<< \" doesn't match any of the pipe output formats\";\n>>> +\t\t\t\t\treturn Invalid;\n>>> +\t\t\t\t}\n>>> +\t\t\t\tLOG(SimplePipeline, Debug)\n>>> +\t\t\t\t\t<< \"Adjusting pixel format from \" << cfg.pixelFormat\n>>> +\t\t\t\t\t<< \" to \" << pixelFormat;\n>>> +\t\t\t\tcfg.pixelFormat = pixelFormat;\n>>> +\t\t\t\t/*\n>>> +\t\t\t\t * Do not touch the colour space for raw requested roles.\n>>> +\t\t\t\t * Even if the pixel format is non-raw (whatever it means), we\n>>> +\t\t\t\t * shouldn't try to interpret the colour space of raw data.\n>>> +\t\t\t\t */\n>>> +\t\t\t\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)\n>>> +\t\t\t\t\tcfg.colorSpace->adjust(pixelFormat);\n>>> +\t\t\t\tstatus = Adjusted;\n>>> +\t\t\t}\n>>> +\n>>> +\t\t\tif (!cfg.colorSpace) {\n>>> +\t\t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);\n>>> +\t\t\t\tswitch (info.colourEncoding) {\n>>> +\t\t\t\tcase PixelFormatInfo::ColourEncodingRGB:\n>>> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Srgb;\n>>> +\t\t\t\t\tbreak;\n>>> +\t\t\t\tcase libcamera::PixelFormatInfo::ColourEncodingYUV:\n>>> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Sycc;\n>>> +\t\t\t\t\tbreak;\n>>> +\t\t\t\tdefault:\n>>> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Raw;\n>>> +\t\t\t\t}\n>>>  \t\t\t\tcfg.colorSpace->adjust(pixelFormat);\n>>> -\t\t\tstatus = Adjusted;\n>>> -\t\t}\n>>> -\t\tif (!cfg.colorSpace) {\n>>> -\t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);\n>>> -\t\t\tswitch (info.colourEncoding) {\n>>> -\t\t\tcase PixelFormatInfo::ColourEncodingRGB:\n>>> -\t\t\t\tcfg.colorSpace = ColorSpace::Srgb;\n>>> -\t\t\t\tbreak;\n>>> -\t\t\tcase libcamera::PixelFormatInfo::ColourEncodingYUV:\n>>> -\t\t\t\tcfg.colorSpace = ColorSpace::Sycc;\n>>> -\t\t\t\tbreak;\n>>> -\t\t\tdefault:\n>>> -\t\t\t\tcfg.colorSpace = ColorSpace::Raw;\n>>> +\t\t\t\tstatus = Adjusted;\n>>>  \t\t\t}\n>>> -\t\t\tcfg.colorSpace->adjust(pixelFormat);\n>>> -\t\t\tstatus = Adjusted;\n>>>  \t\t}\n>>>  \n>>> -\t\tif (!pipeConfig_->outputSizes.contains(cfg.size)) {\n>>> +\t\tif (!rawStream && !pipeConfig_->outputSizes.contains(cfg.size)) {\n>>>  \t\t\tSize adjustedSize = pipeConfig_->captureSize;\n>>>  \t\t\t/*\n>>>  \t\t\t * The converter (when present) may not be able to output\n>>> @@ -1259,11 +1301,20 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>>  \n>>>  \t\t/* \\todo Create a libcamera core class to group format and size */\n>>>  \t\tif (cfg.pixelFormat != pipeConfig_->captureFormat ||\n>>> -\t\t    cfg.size != pipeConfig_->captureSize)\n>>> +\t\t    cfg.size != pipeConfig_->captureSize) {\n>>> +\t\t\tif (rawStream) {\n>>> +\t\t\t\tLOG(SimplePipeline, Info)\n>>> +\t\t\t\t\t<< \"Raw output format \" << cfg.pixelFormat\n>>> +\t\t\t\t\t<< \" and size \" << cfg.size\n>>> +\t\t\t\t\t<< \" not matching pipe format \" << pipeConfig_->captureFormat\n>>> +\t\t\t\t\t<< \" and size \" << pipeConfig_->captureSize;\n>>> +\t\t\t\treturn Invalid;\n>>> +\t\t\t}\n>>>  \t\t\tneedConversion_ = true;\n>>> +\t\t}\n>>>  \n>>>  \t\t/* Set the stride, frameSize and bufferCount. */\n>>> -\t\tif (needConversion_) {\n>>> +\t\tif (needConversion_ && !rawStream) {\n>>>  \t\t\tstd::tie(cfg.stride, cfg.frameSize) =\n>>>  \t\t\t\tdata_->converter_\n>>>  \t\t\t\t\t? data_->converter_->strideAndFrameSize(cfg.pixelFormat,\n>>> -- \n>>> 2.50.1\n>>> \n>\n\nRegards,\nUmang","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 8BB29BE175\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 14 Jul 2025 18:13:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C78CC68F4C;\n\tMon, 14 Jul 2025 20:13:55 +0200 (CEST)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C3B7D68F3F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 14 Jul 2025 20:13:53 +0200 (CEST)","from [49.36.69.193] (helo=[127.0.0.1])\n\tby fanzine2.igalia.com with esmtpsa \n\t(Cipher TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_128_GCM:128)\n\t(Exim) id 1ubNgW-00GWYf-Gs; Mon, 14 Jul 2025 20:13:52 +0200"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=igalia.com header.i=@igalia.com\n\theader.b=\"R+vOj0wI\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com;\n\ts=20170329;\n\th=Content-Transfer-Encoding:Content-Type:MIME-Version:Message-ID:\n\tReferences:In-Reply-To:Subject:CC:To:From:Date:Sender:Reply-To:Content-ID:\n\tContent-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc\n\t:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe:\n\tList-Post:List-Owner:List-Archive;\n\tbh=vas3t+xcdDoC7805H6ZDqmb+gIA9WqNqhgl09AQehB0=;\n\tb=R+vOj0wIBSP6qUVuzpHhTl7Koo\n\tXkaI3b8V+xM2k9Ix9s6LQXvtRN72Ja6zZSSvc2Vjwx2CyOi1pDxxxexZX0uYzwo3JJh8k79WR0Y19\n\tSH1vW3eCy6kEJyyXnqBwTkZs8hyXH+C8Gsk2oPFSXw61osArDzp6RLcd94q+UXEnr5piGZQ+2UjjV\n\tcxj2gqCexiv/XN3J2SzMbwvIdfmsXFwQJjU59Eeiwpml0Dcr3Uad6hnvySC2FkJ1cNKnbTDkFq6WV\n\tzlEhkMBZaHw3/xiFBrtlqjvZJn468L52FZEAKJV3DT7tYfeuJLr/fq9MZ3pw3Y54lP/yn+DBcaMGU\n\tP3PTHi6g==;","Date":"Mon, 14 Jul 2025 23:43:46 +0530","From":"Umang Jain <uajain@igalia.com>","To":"Milan Zamazal <mzamazal@redhat.com>","CC":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t<laurent.pinchart@ideasonboard.com>, Kieran Bingham\n\t<kieran.bingham@ideasonboard.com>, =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t<barnabas.pocze@ideasonboard.com>, Paul Elder\n\t<paul.elder@ideasonboard.com>","Subject":"Re: [PATCH v10 5/8] libcamera: simple: Validate raw stream\n\tconfigurations","User-Agent":"K-9 Mail for Android","In-Reply-To":"<85freyd9p9.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","References":"<20250711175345.90318-1-mzamazal@redhat.com>\n\t<20250711175345.90318-6-mzamazal@redhat.com>\n\t<n6rvwerr5bb73wxpdz5ruhyp4s3zlwvnl6hbyw4i7xpfa2hkfl@e3cfxpnrfnkh>\n\t<85freyd9p9.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Message-ID":"<789C12A8-ACEE-41B3-82BD-2254F4F0E38B@igalia.com>","MIME-Version":"1.0","Content-Type":"text/plain;\n charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":34956,"web_url":"https://patchwork.libcamera.org/comment/34956/","msgid":"<5ckm7wasrki7umzg2b2zzyhaokpdoiclgfnpz2g26enzzmgrvv@3ofdri4ljltc>","date":"2025-07-21T06:34:55","subject":"Re: [PATCH v10 5/8] libcamera: simple: Validate raw stream\n\tconfigurations","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"On Fri, Jul 11, 2025 at 07:53:38PM +0200, Milan Zamazal wrote:\n> SimpleCameraConfiguration::validate() looks for the best configuration.\n> As part of enabling raw stream support, the method must consider raw\n> streams in addition to the processed streams.\n> \n> If only a processed stream is requested, nothing changes.\n> \n> If only a raw stream is requested, the pixel format and output size may\n> not be adjusted.  The patch adds checks for this.\n> \n> If both processed and raw streams are requested, things get more\n> complicated.  The raw stream is expected to be passed through intact and\n> all the adjustments are made for the processed streams.  We select a\n> pipe configuration for the processed streams.\n> \n> Note that with both processed and raw streams, the requested sizes must\n> be mutually matching, including resizing due to debayer requirements.\n> For example, the following `cam' setup is valid for imx219\n> \n>   cam -s role=viewfinder,width=1920,height=1080 \\\n>       -s role=raw,width=3280,height=2464\n> \n> rather than\n> \n>   cam -s role=viewfinder,width=1920,height=1080 \\\n>       -s role=raw,width=1920,height=1080\n> \n> due to the resolution of 1924x1080 actually selected for debayering to\n> 1920x1080.  It is the application responsibility to select the right\n> parameters for the raw stream.\n> \n> Setting up the right configurations is still not enough to make the raw\n> streams working.  Buffer handling must be changed in the simple\n> pipeline, which is addressed in followup patches.\n> \n> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> ---\n>  src/libcamera/pipeline/simple/simple.cpp | 119 ++++++++++++++++-------\n>  1 file changed, 85 insertions(+), 34 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 37abaa0e0..9d87a7ffa 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -27,6 +27,7 @@\n>  #include <libcamera/camera.h>\n>  #include <libcamera/color_space.h>\n>  #include <libcamera/control_ids.h>\n> +#include <libcamera/geometry.h>\n>  #include <libcamera/pixel_format.h>\n>  #include <libcamera/request.h>\n>  #include <libcamera/stream.h>\n> @@ -1186,6 +1187,20 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>  \t\t<< \"-\" << pipeConfig_->captureFormat\n>  \t\t<< \" for max stream size \" << maxStreamSize;\n>  \n> +\tbool rawRequested = false;\n> +\tbool processedRequested = false;\n> +\tfor (const auto &cfg : config_)\n> +\t\tif (cfg.colorSpace == ColorSpace::Raw) {\n> +\t\t\tif (rawRequested) {\n> +\t\t\t\tLOG(SimplePipeline, Error)\n> +\t\t\t\t\t<< \"Can't capture multiple raw streams\";\n> +\t\t\t\treturn Invalid;\n> +\t\t\t}\n> +\t\t\trawRequested = true;\n> +\t\t} else {\n> +\t\t\tprocessedRequested = true;\n> +\t\t}\n> +\n>  \t/*\n>  \t * Adjust the requested streams.\n>  \t *\n> @@ -1204,43 +1219,70 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>  \tfor (unsigned int i = 0; i < config_.size(); ++i) {\n>  \t\tStreamConfiguration &cfg = config_[i];\n>  \n> +\t\t/*\n> +\t\t * If both processed and raw streams are requested, the pipe\n> +\t\t * configuration is set up for the processed stream. The raw\n> +\t\t * configuration needs to be compared against the capture format and\n> +\t\t * size in such a case.\n> +\t\t */\n> +\t\tconst bool rawStream = cfg.colorSpace == ColorSpace::Raw;\n\nWe should always inspect the Stream or the pixelformat to set flags like\nthis. Inspecting colorspace seems flaky, especially when it is a\nstd::optional<> in CameraConfiguration.\n\n> +\t\tconst bool sideRawStream = rawStream && processedRequested;\n> +\n>  \t\t/* Adjust the pixel format and size. */\n> -\t\tauto it = std::find(pipeConfig_->outputFormats.begin(),\n> -\t\t\t\t    pipeConfig_->outputFormats.end(),\n> -\t\t\t\t    cfg.pixelFormat);\n> -\t\tif (it == pipeConfig_->outputFormats.end())\n> -\t\t\tit = pipeConfig_->outputFormats.begin();\n> -\n> -\t\tPixelFormat pixelFormat = *it;\n> -\t\tif (cfg.pixelFormat != pixelFormat) {\n> -\t\t\tLOG(SimplePipeline, Debug) << \"Adjusting pixel format\";\n> -\t\t\tcfg.pixelFormat = pixelFormat;\n> -\t\t\t/*\n> -\t\t\t * Do not touch the colour space for raw requested roles.\n> -\t\t\t * Even if the pixel format is non-raw (whatever it means), we\n> -\t\t\t * shouldn't try to interpret the colour space of raw data.\n> -\t\t\t */\n> -\t\t\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)\n> +\n> +\t\tif (!sideRawStream) {\n> +\t\t\tPixelFormat pixelFormat;\n> +\t\t\tif (rawStream) {\n> +\t\t\t\tpixelFormat = pipeConfig_->captureFormat;\n> +\t\t\t} else {\n> +\t\t\t\tauto it = std::find(pipeConfig_->outputFormats.begin(),\n> +\t\t\t\t\t\t    pipeConfig_->outputFormats.end(),\n> +\t\t\t\t\t\t    cfg.pixelFormat);\n> +\t\t\t\tif (it == pipeConfig_->outputFormats.end())\n> +\t\t\t\t\tit = pipeConfig_->outputFormats.begin();\n> +\t\t\t\tpixelFormat = *it;\n> +\t\t\t}\n> +\n> +\t\t\tif (cfg.pixelFormat != pixelFormat) {\n> +\t\t\t\tif (rawStream) {\n> +\t\t\t\t\tLOG(SimplePipeline, Info)\n> +\t\t\t\t\t\t<< \"Raw pixel format \"\n> +\t\t\t\t\t\t<< cfg.pixelFormat\n> +\t\t\t\t\t\t<< \" doesn't match any of the pipe output formats\";\n> +\t\t\t\t\treturn Invalid;\n\nWe can chose the nearest raw format,size if we need to capture a raw stream.\nsimply return Adjusted in that case.\n\n> +\t\t\t\t}\n> +\t\t\t\tLOG(SimplePipeline, Debug)\n> +\t\t\t\t\t<< \"Adjusting pixel format from \" << cfg.pixelFormat\n> +\t\t\t\t\t<< \" to \" << pixelFormat;\n> +\t\t\t\tcfg.pixelFormat = pixelFormat;\n> +\t\t\t\t/*\n> +\t\t\t\t * Do not touch the colour space for raw requested roles.\n> +\t\t\t\t * Even if the pixel format is non-raw (whatever it means), we\n> +\t\t\t\t * shouldn't try to interpret the colour space of raw data.\n> +\t\t\t\t */\n> +\t\t\t\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)\n> +\t\t\t\t\tcfg.colorSpace->adjust(pixelFormat);\n> +\t\t\t\tstatus = Adjusted;\n> +\t\t\t}\n> +\n> +\t\t\tif (!cfg.colorSpace) {\n> +\t\t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);\n> +\t\t\t\tswitch (info.colourEncoding) {\n> +\t\t\t\tcase PixelFormatInfo::ColourEncodingRGB:\n> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Srgb;\n> +\t\t\t\t\tbreak;\n> +\t\t\t\tcase libcamera::PixelFormatInfo::ColourEncodingYUV:\n> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Sycc;\n> +\t\t\t\t\tbreak;\n> +\t\t\t\tdefault:\n> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Raw;\n> +\t\t\t\t}\n>  \t\t\t\tcfg.colorSpace->adjust(pixelFormat);\n> -\t\t\tstatus = Adjusted;\n> -\t\t}\n> -\t\tif (!cfg.colorSpace) {\n> -\t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);\n> -\t\t\tswitch (info.colourEncoding) {\n> -\t\t\tcase PixelFormatInfo::ColourEncodingRGB:\n> -\t\t\t\tcfg.colorSpace = ColorSpace::Srgb;\n> -\t\t\t\tbreak;\n> -\t\t\tcase libcamera::PixelFormatInfo::ColourEncodingYUV:\n> -\t\t\t\tcfg.colorSpace = ColorSpace::Sycc;\n> -\t\t\t\tbreak;\n> -\t\t\tdefault:\n> -\t\t\t\tcfg.colorSpace = ColorSpace::Raw;\n> +\t\t\t\tstatus = Adjusted;\n>  \t\t\t}\n> -\t\t\tcfg.colorSpace->adjust(pixelFormat);\n> -\t\t\tstatus = Adjusted;\n>  \t\t}\n\nI think I have suggested in one of the replies that colorspace handling\ncan be done separately, to progress them faster.\n\nIn my raw branch, I only set colorspace for raw streams, nothing else.\nI expect that other colorspace related should be addressed separately.\n>  \n> -\t\tif (!pipeConfig_->outputSizes.contains(cfg.size)) {\n> +\t\tif (!rawStream && !pipeConfig_->outputSizes.contains(cfg.size)) {\n>  \t\t\tSize adjustedSize = pipeConfig_->captureSize;\n>  \t\t\t/*\n>  \t\t\t * The converter (when present) may not be able to output\n> @@ -1259,11 +1301,20 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>  \n>  \t\t/* \\todo Create a libcamera core class to group format and size */\n>  \t\tif (cfg.pixelFormat != pipeConfig_->captureFormat ||\n> -\t\t    cfg.size != pipeConfig_->captureSize)\n> +\t\t    cfg.size != pipeConfig_->captureSize) {\n> +\t\t\tif (rawStream) {\n> +\t\t\t\tLOG(SimplePipeline, Info)\n> +\t\t\t\t\t<< \"Raw output format \" << cfg.pixelFormat\n> +\t\t\t\t\t<< \" and size \" << cfg.size\n> +\t\t\t\t\t<< \" not matching pipe format \" << pipeConfig_->captureFormat\n> +\t\t\t\t\t<< \" and size \" << pipeConfig_->captureSize;\n> +\t\t\t\treturn Invalid;\n> +\t\t\t}\n>  \t\t\tneedConversion_ = true;\n> +\t\t}\n>  \n>  \t\t/* Set the stride, frameSize and bufferCount. */\n> -\t\tif (needConversion_) {\n> +\t\tif (needConversion_ && !rawStream) {\n>  \t\t\tstd::tie(cfg.stride, cfg.frameSize) =\n>  \t\t\t\tdata_->converter_\n>  \t\t\t\t\t? data_->converter_->strideAndFrameSize(cfg.pixelFormat,\n> -- \n> 2.50.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 66A33C3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 21 Jul 2025 06:35:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6428868FC2;\n\tMon, 21 Jul 2025 08:35:00 +0200 (CEST)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 23C9D614F6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 21 Jul 2025 08:34:59 +0200 (CEST)","from [49.36.71.87] (helo=uajain) by fanzine2.igalia.com with\n\tesmtpsa \n\t(Cipher TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256)\n\t(Exim) id 1udk6z-001ap0-Eo; Mon, 21 Jul 2025 08:34:58 +0200"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=igalia.com header.i=@igalia.com\n\theader.b=\"CGyMIVuQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com;\n\ts=20170329;\n\th=In-Reply-To:Content-Type:MIME-Version:References:Message-ID:\n\tSubject:Cc:To:From:Date:Sender:Reply-To:Content-Transfer-Encoding:Content-ID:\n\tContent-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc\n\t:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe:\n\tList-Post:List-Owner:List-Archive;\n\tbh=+5ZqrrLzaliQAtOE8fjz1aK41350pBwFmE98saktUDw=;\n\tb=CGyMIVuQskj3LRlwNyK0lhqBNg\n\tYp/XtlDrMu2Q9G4G5ea867QeIGmGh2yGTOcytZwcWmhrBaRYERhiBsZbyYdDX6DlpBeuqgRQust9F\n\td3r3JXu6bW9FNbiOp1ZQHOPQYhjM89j6rb+vbdIk3uUM8dybo3UL2eHF3rTVZDoucLeHtssMnuq0p\n\tfnKOrB9BC8HM4YWRnJWwIwfxqoNvSEG5zj0QinVoZV0hGZMH4/XiTmq3Yju20K8orffegK5PMl6SN\n\tD8pgSi4eHYHGFlveHwE1ngyxEY+tOKGvsEjhAe3Z7jwBr7LN24OWhw54ppk9hpKcLjUIKgJxavk2N\n\tyIu0XIog==;","Date":"Mon, 21 Jul 2025 12:04:55 +0530","From":"Umang Jain <uajain@igalia.com>","To":"Milan Zamazal <mzamazal@redhat.com>","Cc":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t<laurent.pinchart@ideasonboard.com>, Kieran Bingham\n\t<kieran.bingham@ideasonboard.com>, =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t<barnabas.pocze@ideasonboard.com>, Paul Elder\n\t<paul.elder@ideasonboard.com>","Subject":"Re: [PATCH v10 5/8] libcamera: simple: Validate raw stream\n\tconfigurations","Message-ID":"<5ckm7wasrki7umzg2b2zzyhaokpdoiclgfnpz2g26enzzmgrvv@3ofdri4ljltc>","References":"<20250711175345.90318-1-mzamazal@redhat.com>\n\t<20250711175345.90318-6-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20250711175345.90318-6-mzamazal@redhat.com>","User-Agent":"NeoMutt/20250510-dirty","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35003,"web_url":"https://patchwork.libcamera.org/comment/35003/","msgid":"<85bjpdb8cq.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-07-21T20:24:21","subject":"Re: [PATCH v10 5/8] libcamera: simple: Validate raw stream\n\tconfigurations","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Umang,\n\nUmang Jain <uajain@igalia.com> writes:\n\n> On Fri, Jul 11, 2025 at 07:53:38PM +0200, Milan Zamazal wrote:\n>> SimpleCameraConfiguration::validate() looks for the best configuration.\n>> As part of enabling raw stream support, the method must consider raw\n>\n>> streams in addition to the processed streams.\n>> \n>> If only a processed stream is requested, nothing changes.\n>> \n>> If only a raw stream is requested, the pixel format and output size may\n>> not be adjusted.  The patch adds checks for this.\n>> \n>> If both processed and raw streams are requested, things get more\n>> complicated.  The raw stream is expected to be passed through intact and\n>> all the adjustments are made for the processed streams.  We select a\n>> pipe configuration for the processed streams.\n>> \n>> Note that with both processed and raw streams, the requested sizes must\n>> be mutually matching, including resizing due to debayer requirements.\n>> For example, the following `cam' setup is valid for imx219\n>> \n>>   cam -s role=viewfinder,width=1920,height=1080 \\\n>>       -s role=raw,width=3280,height=2464\n>> \n>> rather than\n>> \n>>   cam -s role=viewfinder,width=1920,height=1080 \\\n>>       -s role=raw,width=1920,height=1080\n>> \n>> due to the resolution of 1924x1080 actually selected for debayering to\n>> 1920x1080.  It is the application responsibility to select the right\n>> parameters for the raw stream.\n>> \n>> Setting up the right configurations is still not enough to make the raw\n>> streams working.  Buffer handling must be changed in the simple\n>> pipeline, which is addressed in followup patches.\n>> \n>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n>> ---\n>>  src/libcamera/pipeline/simple/simple.cpp | 119 ++++++++++++++++-------\n>>  1 file changed, 85 insertions(+), 34 deletions(-)\n>> \n>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>> index 37abaa0e0..9d87a7ffa 100644\n>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>> @@ -27,6 +27,7 @@\n>>  #include <libcamera/camera.h>\n>>  #include <libcamera/color_space.h>\n>>  #include <libcamera/control_ids.h>\n>> +#include <libcamera/geometry.h>\n>>  #include <libcamera/pixel_format.h>\n>>  #include <libcamera/request.h>\n>>  #include <libcamera/stream.h>\n>> @@ -1186,6 +1187,20 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>  \t\t<< \"-\" << pipeConfig_->captureFormat\n>>  \t\t<< \" for max stream size \" << maxStreamSize;\n>>  \n>> +\tbool rawRequested = false;\n>> +\tbool processedRequested = false;\n>> +\tfor (const auto &cfg : config_)\n>> +\t\tif (cfg.colorSpace == ColorSpace::Raw) {\n>> +\t\t\tif (rawRequested) {\n>> +\t\t\t\tLOG(SimplePipeline, Error)\n>> +\t\t\t\t\t<< \"Can't capture multiple raw streams\";\n>> +\t\t\t\treturn Invalid;\n>> +\t\t\t}\n>> +\t\t\trawRequested = true;\n>> +\t\t} else {\n>> +\t\t\tprocessedRequested = true;\n>> +\t\t}\n>> +\n>>  \t/*\n>>  \t * Adjust the requested streams.\n>>  \t *\n>> @@ -1204,43 +1219,70 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>  \tfor (unsigned int i = 0; i < config_.size(); ++i) {\n>>  \t\tStreamConfiguration &cfg = config_[i];\n>>  \n>> +\t\t/*\n>> +\t\t * If both processed and raw streams are requested, the pipe\n>> +\t\t * configuration is set up for the processed stream. The raw\n>> +\t\t * configuration needs to be compared against the capture format and\n>> +\t\t * size in such a case.\n>> +\t\t */\n>> +\t\tconst bool rawStream = cfg.colorSpace == ColorSpace::Raw;\n>\n> We should always inspect the Stream or the pixelformat to set flags like\n> this. Inspecting colorspace seems flaky, especially when it is a\n> std::optional<> in CameraConfiguration.\n\nMy idea was to (mis)use this to pass the intention about the stream role\nfrom generateConfiguration().  But selecting a consistent configuration\nalready in generateConfiguration(), something like what you do in your\npatch, looks like a better idea.\n\n>> +\t\tconst bool sideRawStream = rawStream && processedRequested;\n>> +\n>>  \t\t/* Adjust the pixel format and size. */\n>> -\t\tauto it = std::find(pipeConfig_->outputFormats.begin(),\n>> -\t\t\t\t    pipeConfig_->outputFormats.end(),\n>> -\t\t\t\t    cfg.pixelFormat);\n>> -\t\tif (it == pipeConfig_->outputFormats.end())\n>> -\t\t\tit = pipeConfig_->outputFormats.begin();\n>> -\n>> -\t\tPixelFormat pixelFormat = *it;\n>> -\t\tif (cfg.pixelFormat != pixelFormat) {\n>> -\t\t\tLOG(SimplePipeline, Debug) << \"Adjusting pixel format\";\n>> -\t\t\tcfg.pixelFormat = pixelFormat;\n>> -\t\t\t/*\n>> -\t\t\t * Do not touch the colour space for raw requested roles.\n>> -\t\t\t * Even if the pixel format is non-raw (whatever it means), we\n>> -\t\t\t * shouldn't try to interpret the colour space of raw data.\n>> -\t\t\t */\n>> -\t\t\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)\n>> +\n>> +\t\tif (!sideRawStream) {\n>> +\t\t\tPixelFormat pixelFormat;\n>> +\t\t\tif (rawStream) {\n>> +\t\t\t\tpixelFormat = pipeConfig_->captureFormat;\n>> +\t\t\t} else {\n>> +\t\t\t\tauto it = std::find(pipeConfig_->outputFormats.begin(),\n>> +\t\t\t\t\t\t    pipeConfig_->outputFormats.end(),\n>> +\t\t\t\t\t\t    cfg.pixelFormat);\n>> +\t\t\t\tif (it == pipeConfig_->outputFormats.end())\n>> +\t\t\t\t\tit = pipeConfig_->outputFormats.begin();\n>> +\t\t\t\tpixelFormat = *it;\n>> +\t\t\t}\n>> +\n>> +\t\t\tif (cfg.pixelFormat != pixelFormat) {\n>> +\t\t\t\tif (rawStream) {\n>> +\t\t\t\t\tLOG(SimplePipeline, Info)\n>> +\t\t\t\t\t\t<< \"Raw pixel format \"\n>> +\t\t\t\t\t\t<< cfg.pixelFormat\n>> +\t\t\t\t\t\t<< \" doesn't match any of the pipe output formats\";\n>> +\t\t\t\t\treturn Invalid;\n>\n> We can chose the nearest raw format,size if we need to capture a raw stream.\n> simply return Adjusted in that case.\n\nYes, unless a specific size for the stream was requested.\n\n>> +\t\t\t\t}\n>> +\t\t\t\tLOG(SimplePipeline, Debug)\n>> +\t\t\t\t\t<< \"Adjusting pixel format from \" << cfg.pixelFormat\n>> +\t\t\t\t\t<< \" to \" << pixelFormat;\n>> +\t\t\t\tcfg.pixelFormat = pixelFormat;\n>> +\t\t\t\t/*\n>> +\t\t\t\t * Do not touch the colour space for raw requested roles.\n>> +\t\t\t\t * Even if the pixel format is non-raw (whatever it means), we\n>> +\t\t\t\t * shouldn't try to interpret the colour space of raw data.\n>> +\t\t\t\t */\n>> +\t\t\t\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)\n>> +\t\t\t\t\tcfg.colorSpace->adjust(pixelFormat);\n>> +\t\t\t\tstatus = Adjusted;\n>> +\t\t\t}\n>> +\n>> +\t\t\tif (!cfg.colorSpace) {\n>> +\t\t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);\n>> +\t\t\t\tswitch (info.colourEncoding) {\n>> +\t\t\t\tcase PixelFormatInfo::ColourEncodingRGB:\n>> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Srgb;\n>> +\t\t\t\t\tbreak;\n>> +\t\t\t\tcase libcamera::PixelFormatInfo::ColourEncodingYUV:\n>> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Sycc;\n>> +\t\t\t\t\tbreak;\n>> +\t\t\t\tdefault:\n>> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Raw;\n>> +\t\t\t\t}\n>>  \t\t\t\tcfg.colorSpace->adjust(pixelFormat);\n>> -\t\t\tstatus = Adjusted;\n>> -\t\t}\n>> -\t\tif (!cfg.colorSpace) {\n>> -\t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);\n>> -\t\t\tswitch (info.colourEncoding) {\n>> -\t\t\tcase PixelFormatInfo::ColourEncodingRGB:\n>> -\t\t\t\tcfg.colorSpace = ColorSpace::Srgb;\n>> -\t\t\t\tbreak;\n>> -\t\t\tcase libcamera::PixelFormatInfo::ColourEncodingYUV:\n>> -\t\t\t\tcfg.colorSpace = ColorSpace::Sycc;\n>> -\t\t\t\tbreak;\n>> -\t\t\tdefault:\n>> -\t\t\t\tcfg.colorSpace = ColorSpace::Raw;\n>> +\t\t\t\tstatus = Adjusted;\n>>  \t\t\t}\n>> -\t\t\tcfg.colorSpace->adjust(pixelFormat);\n>> -\t\t\tstatus = Adjusted;\n>>  \t\t}\n>\n> I think I have suggested in one of the replies that colorspace handling\n> can be done separately, to progress them faster.\n\nThe hunk above is just moving the code.\n\n> In my raw branch, I only set colorspace for raw streams, nothing else.\n> I expect that other colorspace related should be addressed separately.\n\nThe basic colour space assignment is introduced in my \"Assign colour\nspaces in configurations\" patch, which is important to prevent crashes\nwhen the colour space is unset.\n\n>> -\t\tif (!pipeConfig_->outputSizes.contains(cfg.size)) {\n>> +\t\tif (!rawStream && !pipeConfig_->outputSizes.contains(cfg.size)) {\n>>  \t\t\tSize adjustedSize = pipeConfig_->captureSize;\n>>  \t\t\t/*\n>>  \t\t\t * The converter (when present) may not be able to output\n>> @@ -1259,11 +1301,20 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>  \n>>  \t\t/* \\todo Create a libcamera core class to group format and size */\n>>  \t\tif (cfg.pixelFormat != pipeConfig_->captureFormat ||\n>> -\t\t    cfg.size != pipeConfig_->captureSize)\n>> +\t\t    cfg.size != pipeConfig_->captureSize) {\n>> +\t\t\tif (rawStream) {\n>> +\t\t\t\tLOG(SimplePipeline, Info)\n>> +\t\t\t\t\t<< \"Raw output format \" << cfg.pixelFormat\n>> +\t\t\t\t\t<< \" and size \" << cfg.size\n>> +\t\t\t\t\t<< \" not matching pipe format \" << pipeConfig_->captureFormat\n>> +\t\t\t\t\t<< \" and size \" << pipeConfig_->captureSize;\n>> +\t\t\t\treturn Invalid;\n>> +\t\t\t}\n>>  \t\t\tneedConversion_ = true;\n>> +\t\t}\n>>  \n>>  \t\t/* Set the stride, frameSize and bufferCount. */\n>> -\t\tif (needConversion_) {\n>> +\t\tif (needConversion_ && !rawStream) {\n>>  \t\t\tstd::tie(cfg.stride, cfg.frameSize) =\n>>  \t\t\t\tdata_->converter_\n>>  \t\t\t\t\t? data_->converter_->strideAndFrameSize(cfg.pixelFormat,\n>> -- \n>> 2.50.1\n>>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 6E64BC3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 21 Jul 2025 20:24:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 92BE269015;\n\tMon, 21 Jul 2025 22:24:29 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ABD7468FB1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 21 Jul 2025 22:24:27 +0200 (CEST)","from mail-wr1-f70.google.com (mail-wr1-f70.google.com\n\t[209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-310-lu7ZhruTPfOzf3STpo0dLg-1; Mon, 21 Jul 2025 16:24:24 -0400","by mail-wr1-f70.google.com with SMTP id\n\tffacd0b85a97d-3b5fe97af5fso2008066f8f.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 21 Jul 2025 13:24:24 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-3b61ca5cfb0sm11398340f8f.83.2025.07.21.13.24.22\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 21 Jul 2025 13:24:22 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"OrQdhDyJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1753129466;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=n7Xro5iqaQY4zDxfGDaKpd8c9HxoAEH4BfoeaRz6eX8=;\n\tb=OrQdhDyJvYaevZGDTCzUvWiiWOmZG5+PDQpZg6fI2MCseHLGLkNbWy8YxCDBF2HZW8w1Xp\n\t5pjT3xWvpmqAekV8qknkBT4dTxWtJMCS4S7OaWGD3kWizoRrp7Dwnf2vXiMEciNVXogB3m\n\tC4xnz5SExxGtCjmH7UdMS7O9z/h7+dA=","X-MC-Unique":"lu7ZhruTPfOzf3STpo0dLg-1","X-Mimecast-MFC-AGG-ID":"lu7ZhruTPfOzf3STpo0dLg_1753129463","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1753129463; x=1753734263;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=n7Xro5iqaQY4zDxfGDaKpd8c9HxoAEH4BfoeaRz6eX8=;\n\tb=H77BSnh1pzIXPybfuH48VQJnuslTZrCWYDoHT1FPm258kCWO7DT0Zz3ScVAOPio5E1\n\tPrgF7CwzjDH/KeS548D3010a1giv1egrIQlQaMPOeIa0PqH9QLw4IO3uFkM7OOEN/VhQ\n\t0gJKvjTKFVJ2trRcm/Ygq/cg0scsQMlnQ3zcMbrpT20R7LbH7nyxcGTZPgWyd8AgETAH\n\to/bq+KoheJNJeFldp0ZWZdZUzkq+o3k9vOG559Tbbx/ZRz3DLbSHntnt25aQL0Hnk4MD\n\tSxrylfcf4rV9C2UujN6nuI3POMax43McrVjLs45WJ2gEKIU2QVRT989aMYkfrE7p0eDb\n\tTu8A==","X-Gm-Message-State":"AOJu0YxZfiL05ZxE0mXrmCiWV24GMo+7JYaSt1zf75EDIS8fRdGEEh01\n\tjYc/BXYGvGfq88WKJUowbQQoYJ5z13UfQlGORsZbaC11aOnV2H2k1cKxd6ImkBZ8jvzZizqNUBG\n\tMSjnhjmJYer9ALTMjJQPnlPuIeE5hHiYH627kGApEjDN0lR/sO/O7FK6ieuNOD0xMwBwENtVdSg\n\tU=","X-Gm-Gg":"ASbGnctdfLp5uaXibd11NANsOKS6KznS1EV5BZOBOOCMXgg+RHO0afFOoNvOX14cNbn\n\tlz8KK6FifLLeMayeSCwUnUepjm3FMoBYUoJPXF3AVbwfA/HJAOr2N8QaLkLjaZb3llo+tw1GS0O\n\tdjmB0yEHPycW3/nLQW7MHwpP2R3O4HbdhfA4ESRFiPr63gCRXI6HEdhLpt82E4ZYfyKwI44ybE/\n\t+PMUlw5TuqI8N1XiEVZQetm3cMHnye2xa7XZ549rw2o0IRZMItmyKb2G6nzdeAqNSXXOMgkZiVU\n\toK2ANZo/rbLMgc8TPyxlTlwt4jCzg09ynkn2p6/1HhR8nyETh5xEwuqe9TEKNTKwoWvqhIe9IQS\n\tQQdReGWaYimWbMr9w","X-Received":["by 2002:a05:6000:2dc3:b0:3b7:635d:b119 with SMTP id\n\tffacd0b85a97d-3b7635db132mr591315f8f.46.1753129463452; \n\tMon, 21 Jul 2025 13:24:23 -0700 (PDT)","by 2002:a05:6000:2dc3:b0:3b7:635d:b119 with SMTP id\n\tffacd0b85a97d-3b7635db132mr591297f8f.46.1753129462987; \n\tMon, 21 Jul 2025 13:24:22 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IHXghqnJ6d/LRQqwbbo2r22UFb/ZncsjN0/Tqw3IjdxocrLWJztlCL6ui4J85WHcZAR+mYniA==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Umang Jain <uajain@igalia.com>","Cc":"libcamera-devel@lists.libcamera.org,  Laurent Pinchart\n\t<laurent.pinchart@ideasonboard.com>,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>, =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?=\n\t<barnabas.pocze@ideasonboard.com>, Paul Elder\n\t<paul.elder@ideasonboard.com>","Subject":"Re: [PATCH v10 5/8] libcamera: simple: Validate raw stream\n\tconfigurations","In-Reply-To":"<5ckm7wasrki7umzg2b2zzyhaokpdoiclgfnpz2g26enzzmgrvv@3ofdri4ljltc>\n\t(Umang Jain's message of \"Mon, 21 Jul 2025 12:04:55 +0530\")","References":"<20250711175345.90318-1-mzamazal@redhat.com>\n\t<20250711175345.90318-6-mzamazal@redhat.com>\n\t<5ckm7wasrki7umzg2b2zzyhaokpdoiclgfnpz2g26enzzmgrvv@3ofdri4ljltc>","Date":"Mon, 21 Jul 2025 22:24:21 +0200","Message-ID":"<85bjpdb8cq.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"I_7rOXuSG0KPphtry2XCHOyIq6j45e9tyz_SHi0PjTc_1753129463","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35005,"web_url":"https://patchwork.libcamera.org/comment/35005/","msgid":"<lo36tlwsozc7zgneemzgn7eitrjmfd5bkay5hunhovajw3avrs@sx62fsghntrm>","date":"2025-07-22T04:43:30","subject":"Re: [PATCH v10 5/8] libcamera: simple: Validate raw stream\n\tconfigurations","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"On Mon, Jul 21, 2025 at 10:24:21PM +0200, Milan Zamazal wrote:\n> Hi Umang,\n> \n> Umang Jain <uajain@igalia.com> writes:\n> \n> > On Fri, Jul 11, 2025 at 07:53:38PM +0200, Milan Zamazal wrote:\n> >> SimpleCameraConfiguration::validate() looks for the best configuration.\n> >> As part of enabling raw stream support, the method must consider raw\n> >\n> >> streams in addition to the processed streams.\n> >> \n> >> If only a processed stream is requested, nothing changes.\n> >> \n> >> If only a raw stream is requested, the pixel format and output size may\n> >> not be adjusted.  The patch adds checks for this.\n> >> \n> >> If both processed and raw streams are requested, things get more\n> >> complicated.  The raw stream is expected to be passed through intact and\n> >> all the adjustments are made for the processed streams.  We select a\n> >> pipe configuration for the processed streams.\n> >> \n> >> Note that with both processed and raw streams, the requested sizes must\n> >> be mutually matching, including resizing due to debayer requirements.\n> >> For example, the following `cam' setup is valid for imx219\n> >> \n> >>   cam -s role=viewfinder,width=1920,height=1080 \\\n> >>       -s role=raw,width=3280,height=2464\n> >> \n> >> rather than\n> >> \n> >>   cam -s role=viewfinder,width=1920,height=1080 \\\n> >>       -s role=raw,width=1920,height=1080\n> >> \n> >> due to the resolution of 1924x1080 actually selected for debayering to\n> >> 1920x1080.  It is the application responsibility to select the right\n> >> parameters for the raw stream.\n> >> \n> >> Setting up the right configurations is still not enough to make the raw\n> >> streams working.  Buffer handling must be changed in the simple\n> >> pipeline, which is addressed in followup patches.\n> >> \n> >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> >> ---\n> >>  src/libcamera/pipeline/simple/simple.cpp | 119 ++++++++++++++++-------\n> >>  1 file changed, 85 insertions(+), 34 deletions(-)\n> >> \n> >> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> >> index 37abaa0e0..9d87a7ffa 100644\n> >> --- a/src/libcamera/pipeline/simple/simple.cpp\n> >> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> >> @@ -27,6 +27,7 @@\n> >>  #include <libcamera/camera.h>\n> >>  #include <libcamera/color_space.h>\n> >>  #include <libcamera/control_ids.h>\n> >> +#include <libcamera/geometry.h>\n> >>  #include <libcamera/pixel_format.h>\n> >>  #include <libcamera/request.h>\n> >>  #include <libcamera/stream.h>\n> >> @@ -1186,6 +1187,20 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> >>  \t\t<< \"-\" << pipeConfig_->captureFormat\n> >>  \t\t<< \" for max stream size \" << maxStreamSize;\n> >>  \n> >> +\tbool rawRequested = false;\n> >> +\tbool processedRequested = false;\n> >> +\tfor (const auto &cfg : config_)\n> >> +\t\tif (cfg.colorSpace == ColorSpace::Raw) {\n> >> +\t\t\tif (rawRequested) {\n> >> +\t\t\t\tLOG(SimplePipeline, Error)\n> >> +\t\t\t\t\t<< \"Can't capture multiple raw streams\";\n> >> +\t\t\t\treturn Invalid;\n> >> +\t\t\t}\n> >> +\t\t\trawRequested = true;\n> >> +\t\t} else {\n> >> +\t\t\tprocessedRequested = true;\n> >> +\t\t}\n> >> +\n> >>  \t/*\n> >>  \t * Adjust the requested streams.\n> >>  \t *\n> >> @@ -1204,43 +1219,70 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> >>  \tfor (unsigned int i = 0; i < config_.size(); ++i) {\n> >>  \t\tStreamConfiguration &cfg = config_[i];\n> >>  \n> >> +\t\t/*\n> >> +\t\t * If both processed and raw streams are requested, the pipe\n> >> +\t\t * configuration is set up for the processed stream. The raw\n> >> +\t\t * configuration needs to be compared against the capture format and\n> >> +\t\t * size in such a case.\n> >> +\t\t */\n> >> +\t\tconst bool rawStream = cfg.colorSpace == ColorSpace::Raw;\n> >\n> > We should always inspect the Stream or the pixelformat to set flags like\n> > this. Inspecting colorspace seems flaky, especially when it is a\n> > std::optional<> in CameraConfiguration.\n> \n> My idea was to (mis)use this to pass the intention about the stream role\n> from generateConfiguration().  But selecting a consistent configuration\n> already in generateConfiguration(), something like what you do in your\n> patch, looks like a better idea.\n> \n> >> +\t\tconst bool sideRawStream = rawStream && processedRequested;\n> >> +\n> >>  \t\t/* Adjust the pixel format and size. */\n> >> -\t\tauto it = std::find(pipeConfig_->outputFormats.begin(),\n> >> -\t\t\t\t    pipeConfig_->outputFormats.end(),\n> >> -\t\t\t\t    cfg.pixelFormat);\n> >> -\t\tif (it == pipeConfig_->outputFormats.end())\n> >> -\t\t\tit = pipeConfig_->outputFormats.begin();\n> >> -\n> >> -\t\tPixelFormat pixelFormat = *it;\n> >> -\t\tif (cfg.pixelFormat != pixelFormat) {\n> >> -\t\t\tLOG(SimplePipeline, Debug) << \"Adjusting pixel format\";\n> >> -\t\t\tcfg.pixelFormat = pixelFormat;\n> >> -\t\t\t/*\n> >> -\t\t\t * Do not touch the colour space for raw requested roles.\n> >> -\t\t\t * Even if the pixel format is non-raw (whatever it means), we\n> >> -\t\t\t * shouldn't try to interpret the colour space of raw data.\n> >> -\t\t\t */\n> >> -\t\t\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)\n> >> +\n> >> +\t\tif (!sideRawStream) {\n> >> +\t\t\tPixelFormat pixelFormat;\n> >> +\t\t\tif (rawStream) {\n> >> +\t\t\t\tpixelFormat = pipeConfig_->captureFormat;\n> >> +\t\t\t} else {\n> >> +\t\t\t\tauto it = std::find(pipeConfig_->outputFormats.begin(),\n> >> +\t\t\t\t\t\t    pipeConfig_->outputFormats.end(),\n> >> +\t\t\t\t\t\t    cfg.pixelFormat);\n> >> +\t\t\t\tif (it == pipeConfig_->outputFormats.end())\n> >> +\t\t\t\t\tit = pipeConfig_->outputFormats.begin();\n> >> +\t\t\t\tpixelFormat = *it;\n> >> +\t\t\t}\n> >> +\n> >> +\t\t\tif (cfg.pixelFormat != pixelFormat) {\n> >> +\t\t\t\tif (rawStream) {\n> >> +\t\t\t\t\tLOG(SimplePipeline, Info)\n> >> +\t\t\t\t\t\t<< \"Raw pixel format \"\n> >> +\t\t\t\t\t\t<< cfg.pixelFormat\n> >> +\t\t\t\t\t\t<< \" doesn't match any of the pipe output formats\";\n> >> +\t\t\t\t\treturn Invalid;\n> >\n> > We can chose the nearest raw format,size if we need to capture a raw stream.\n> > simply return Adjusted in that case.\n> \n> Yes, unless a specific size for the stream was requested.\n\nI mean for the size as well. If the specific size is requested, the\npipeline is free is to adjust it and return the closest it to satisfy\nthe user requirements.\n\n> \n> >> +\t\t\t\t}\n> >> +\t\t\t\tLOG(SimplePipeline, Debug)\n> >> +\t\t\t\t\t<< \"Adjusting pixel format from \" << cfg.pixelFormat\n> >> +\t\t\t\t\t<< \" to \" << pixelFormat;\n> >> +\t\t\t\tcfg.pixelFormat = pixelFormat;\n> >> +\t\t\t\t/*\n> >> +\t\t\t\t * Do not touch the colour space for raw requested roles.\n> >> +\t\t\t\t * Even if the pixel format is non-raw (whatever it means), we\n> >> +\t\t\t\t * shouldn't try to interpret the colour space of raw data.\n> >> +\t\t\t\t */\n> >> +\t\t\t\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)\n> >> +\t\t\t\t\tcfg.colorSpace->adjust(pixelFormat);\n> >> +\t\t\t\tstatus = Adjusted;\n> >> +\t\t\t}\n> >> +\n> >> +\t\t\tif (!cfg.colorSpace) {\n> >> +\t\t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);\n> >> +\t\t\t\tswitch (info.colourEncoding) {\n> >> +\t\t\t\tcase PixelFormatInfo::ColourEncodingRGB:\n> >> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Srgb;\n> >> +\t\t\t\t\tbreak;\n> >> +\t\t\t\tcase libcamera::PixelFormatInfo::ColourEncodingYUV:\n> >> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Sycc;\n> >> +\t\t\t\t\tbreak;\n> >> +\t\t\t\tdefault:\n> >> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Raw;\n> >> +\t\t\t\t}\n> >>  \t\t\t\tcfg.colorSpace->adjust(pixelFormat);\n> >> -\t\t\tstatus = Adjusted;\n> >> -\t\t}\n> >> -\t\tif (!cfg.colorSpace) {\n> >> -\t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);\n> >> -\t\t\tswitch (info.colourEncoding) {\n> >> -\t\t\tcase PixelFormatInfo::ColourEncodingRGB:\n> >> -\t\t\t\tcfg.colorSpace = ColorSpace::Srgb;\n> >> -\t\t\t\tbreak;\n> >> -\t\t\tcase libcamera::PixelFormatInfo::ColourEncodingYUV:\n> >> -\t\t\t\tcfg.colorSpace = ColorSpace::Sycc;\n> >> -\t\t\t\tbreak;\n> >> -\t\t\tdefault:\n> >> -\t\t\t\tcfg.colorSpace = ColorSpace::Raw;\n> >> +\t\t\t\tstatus = Adjusted;\n> >>  \t\t\t}\n> >> -\t\t\tcfg.colorSpace->adjust(pixelFormat);\n> >> -\t\t\tstatus = Adjusted;\n> >>  \t\t}\n> >\n> > I think I have suggested in one of the replies that colorspace handling\n> > can be done separately, to progress them faster.\n> \n> The hunk above is just moving the code.\n> \n> > In my raw branch, I only set colorspace for raw streams, nothing else.\n> > I expect that other colorspace related should be addressed separately.\n> \n> The basic colour space assignment is introduced in my \"Assign colour\n> spaces in configurations\" patch, which is important to prevent crashes\n> when the colour space is unset.\n\nYeah, this should definitely be separate. This is not related to RAW\nsupport.\n\n> \n> >> -\t\tif (!pipeConfig_->outputSizes.contains(cfg.size)) {\n> >> +\t\tif (!rawStream && !pipeConfig_->outputSizes.contains(cfg.size)) {\n> >>  \t\t\tSize adjustedSize = pipeConfig_->captureSize;\n> >>  \t\t\t/*\n> >>  \t\t\t * The converter (when present) may not be able to output\n> >> @@ -1259,11 +1301,20 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n> >>  \n> >>  \t\t/* \\todo Create a libcamera core class to group format and size */\n> >>  \t\tif (cfg.pixelFormat != pipeConfig_->captureFormat ||\n> >> -\t\t    cfg.size != pipeConfig_->captureSize)\n> >> +\t\t    cfg.size != pipeConfig_->captureSize) {\n> >> +\t\t\tif (rawStream) {\n> >> +\t\t\t\tLOG(SimplePipeline, Info)\n> >> +\t\t\t\t\t<< \"Raw output format \" << cfg.pixelFormat\n> >> +\t\t\t\t\t<< \" and size \" << cfg.size\n> >> +\t\t\t\t\t<< \" not matching pipe format \" << pipeConfig_->captureFormat\n> >> +\t\t\t\t\t<< \" and size \" << pipeConfig_->captureSize;\n> >> +\t\t\t\treturn Invalid;\n> >> +\t\t\t}\n> >>  \t\t\tneedConversion_ = true;\n> >> +\t\t}\n> >>  \n> >>  \t\t/* Set the stride, frameSize and bufferCount. */\n> >> -\t\tif (needConversion_) {\n> >> +\t\tif (needConversion_ && !rawStream) {\n> >>  \t\t\tstd::tie(cfg.stride, cfg.frameSize) =\n> >>  \t\t\t\tdata_->converter_\n> >>  \t\t\t\t\t? data_->converter_->strideAndFrameSize(cfg.pixelFormat,\n> >> -- \n> >> 2.50.1\n> >> \n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 4A833BDCC1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 22 Jul 2025 04:43:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 013C769018;\n\tTue, 22 Jul 2025 06:43:30 +0200 (CEST)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6CFAC614EE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 22 Jul 2025 06:43:28 +0200 (CEST)","from [49.36.71.87] (helo=uajain) by fanzine2.igalia.com with\n\tesmtpsa \n\t(Cipher TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256)\n\t(Exim) id 1ue4qb-001zvO-O9; Tue, 22 Jul 2025 06:43:27 +0200"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=igalia.com header.i=@igalia.com\n\theader.b=\"FvY8c8wy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com;\n\ts=20170329;\n\th=In-Reply-To:Content-Type:MIME-Version:References:Message-ID:\n\tSubject:Cc:To:From:Date:Sender:Reply-To:Content-Transfer-Encoding:Content-ID:\n\tContent-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc\n\t:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe:\n\tList-Post:List-Owner:List-Archive;\n\tbh=mGV56zfgUIjxbluwP+MpFRRNFqOI0oAATXC+wr1Ygpw=;\n\tb=FvY8c8wy9cDXed2BeXiLO7MIeP\n\tco3oxGhkEnAZUhhSCnZJe59i5Atz4otsVY2lVlW2FinomHJqbdEKS5xro2m7UOcdounUfQRmmPhAM\n\tuT2wIlgUQgl7tgUFS0dGFYBUt6RHuWw3y06fv1sq2gZ9siWB9lHWE6/Q1q4HiX5ANmBafkVlufidD\n\tWdQcz2ebzK1HcZufQBMA57rEcxFR/wCm5WKcv8aF78CyU+0Vy5cNVezQDPrMWYeavpMuXOdWR7guW\n\tqQq+RHSM75E2g/X66ZWLvwZrVcniKWr8J8pA8/d2Dessvz0KOXV6waOoN+/nOQkUbj6GUk6ScvlLd\n\t41OVteHA==;","Date":"Tue, 22 Jul 2025 10:13:30 +0530","From":"Umang Jain <uajain@igalia.com>","To":"Milan Zamazal <mzamazal@redhat.com>","Cc":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t<laurent.pinchart@ideasonboard.com>, Kieran Bingham\n\t<kieran.bingham@ideasonboard.com>, =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t<barnabas.pocze@ideasonboard.com>, Paul Elder\n\t<paul.elder@ideasonboard.com>","Subject":"Re: [PATCH v10 5/8] libcamera: simple: Validate raw stream\n\tconfigurations","Message-ID":"<lo36tlwsozc7zgneemzgn7eitrjmfd5bkay5hunhovajw3avrs@sx62fsghntrm>","References":"<20250711175345.90318-1-mzamazal@redhat.com>\n\t<20250711175345.90318-6-mzamazal@redhat.com>\n\t<5ckm7wasrki7umzg2b2zzyhaokpdoiclgfnpz2g26enzzmgrvv@3ofdri4ljltc>\n\t<85bjpdb8cq.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<85bjpdb8cq.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"NeoMutt/20250510-dirty","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35008,"web_url":"https://patchwork.libcamera.org/comment/35008/","msgid":"<85cy9sr6fk.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-07-22T08:11:27","subject":"Re: [PATCH v10 5/8] libcamera: simple: Validate raw stream\n\tconfigurations","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Umang Jain <uajain@igalia.com> writes:\n\n> On Mon, Jul 21, 2025 at 10:24:21PM +0200, Milan Zamazal wrote:\n>> Hi Umang,\n>> \n>\n>> Umang Jain <uajain@igalia.com> writes:\n>> \n>> > On Fri, Jul 11, 2025 at 07:53:38PM +0200, Milan Zamazal wrote:\n>> >> SimpleCameraConfiguration::validate() looks for the best configuration.\n>> >> As part of enabling raw stream support, the method must consider raw\n>> >\n>> >> streams in addition to the processed streams.\n>> >> \n>> >> If only a processed stream is requested, nothing changes.\n>> >> \n>> >> If only a raw stream is requested, the pixel format and output size may\n>> >> not be adjusted.  The patch adds checks for this.\n>> >> \n>> >> If both processed and raw streams are requested, things get more\n>> >> complicated.  The raw stream is expected to be passed through intact and\n>> >> all the adjustments are made for the processed streams.  We select a\n>> >> pipe configuration for the processed streams.\n>> >> \n>> >> Note that with both processed and raw streams, the requested sizes must\n>> >> be mutually matching, including resizing due to debayer requirements.\n>> >> For example, the following `cam' setup is valid for imx219\n>> >> \n>> >>   cam -s role=viewfinder,width=1920,height=1080 \\\n>> >>       -s role=raw,width=3280,height=2464\n>> >> \n>> >> rather than\n>> >> \n>> >>   cam -s role=viewfinder,width=1920,height=1080 \\\n>> >>       -s role=raw,width=1920,height=1080\n>> >> \n>> >> due to the resolution of 1924x1080 actually selected for debayering to\n>> >> 1920x1080.  It is the application responsibility to select the right\n>> >> parameters for the raw stream.\n>> >> \n>> >> Setting up the right configurations is still not enough to make the raw\n>> >> streams working.  Buffer handling must be changed in the simple\n>> >> pipeline, which is addressed in followup patches.\n>> >> \n>> >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n>> >> ---\n>> >>  src/libcamera/pipeline/simple/simple.cpp | 119 ++++++++++++++++-------\n>> >>  1 file changed, 85 insertions(+), 34 deletions(-)\n>> >> \n>> >> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>> >> index 37abaa0e0..9d87a7ffa 100644\n>> >> --- a/src/libcamera/pipeline/simple/simple.cpp\n>> >> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>> >> @@ -27,6 +27,7 @@\n>> >>  #include <libcamera/camera.h>\n>> >>  #include <libcamera/color_space.h>\n>> >>  #include <libcamera/control_ids.h>\n>> >> +#include <libcamera/geometry.h>\n>> >>  #include <libcamera/pixel_format.h>\n>> >>  #include <libcamera/request.h>\n>> >>  #include <libcamera/stream.h>\n>> >> @@ -1186,6 +1187,20 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>> >>  \t\t<< \"-\" << pipeConfig_->captureFormat\n>> >>  \t\t<< \" for max stream size \" << maxStreamSize;\n>> >>  \n>> >> +\tbool rawRequested = false;\n>> >> +\tbool processedRequested = false;\n>> >> +\tfor (const auto &cfg : config_)\n>> >> +\t\tif (cfg.colorSpace == ColorSpace::Raw) {\n>> >> +\t\t\tif (rawRequested) {\n>> >> +\t\t\t\tLOG(SimplePipeline, Error)\n>> >> +\t\t\t\t\t<< \"Can't capture multiple raw streams\";\n>> >> +\t\t\t\treturn Invalid;\n>> >> +\t\t\t}\n>> >> +\t\t\trawRequested = true;\n>> >> +\t\t} else {\n>> >> +\t\t\tprocessedRequested = true;\n>> >> +\t\t}\n>> >> +\n>> >>  \t/*\n>> >>  \t * Adjust the requested streams.\n>> >>  \t *\n>> >> @@ -1204,43 +1219,70 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>> >>  \tfor (unsigned int i = 0; i < config_.size(); ++i) {\n>> >>  \t\tStreamConfiguration &cfg = config_[i];\n>> >>  \n>> >> +\t\t/*\n>> >> +\t\t * If both processed and raw streams are requested, the pipe\n>> >> +\t\t * configuration is set up for the processed stream. The raw\n>> >> +\t\t * configuration needs to be compared against the capture format and\n>> >> +\t\t * size in such a case.\n>> >> +\t\t */\n>> >> +\t\tconst bool rawStream = cfg.colorSpace == ColorSpace::Raw;\n>> >\n>> > We should always inspect the Stream or the pixelformat to set flags like\n>> > this. Inspecting colorspace seems flaky, especially when it is a\n>> > std::optional<> in CameraConfiguration.\n>> \n>> My idea was to (mis)use this to pass the intention about the stream role\n>> from generateConfiguration().  But selecting a consistent configuration\n>> already in generateConfiguration(), something like what you do in your\n>> patch, looks like a better idea.\n>> \n>> >> +\t\tconst bool sideRawStream = rawStream && processedRequested;\n>> >> +\n>> >>  \t\t/* Adjust the pixel format and size. */\n>> >> -\t\tauto it = std::find(pipeConfig_->outputFormats.begin(),\n>> >> -\t\t\t\t    pipeConfig_->outputFormats.end(),\n>> >> -\t\t\t\t    cfg.pixelFormat);\n>> >> -\t\tif (it == pipeConfig_->outputFormats.end())\n>> >> -\t\t\tit = pipeConfig_->outputFormats.begin();\n>> >> -\n>> >> -\t\tPixelFormat pixelFormat = *it;\n>> >> -\t\tif (cfg.pixelFormat != pixelFormat) {\n>> >> -\t\t\tLOG(SimplePipeline, Debug) << \"Adjusting pixel format\";\n>> >> -\t\t\tcfg.pixelFormat = pixelFormat;\n>> >> -\t\t\t/*\n>> >> -\t\t\t * Do not touch the colour space for raw requested roles.\n>> >> -\t\t\t * Even if the pixel format is non-raw (whatever it means), we\n>> >> -\t\t\t * shouldn't try to interpret the colour space of raw data.\n>> >> -\t\t\t */\n>> >> -\t\t\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)\n>> >> +\n>> >> +\t\tif (!sideRawStream) {\n>> >> +\t\t\tPixelFormat pixelFormat;\n>> >> +\t\t\tif (rawStream) {\n>> >> +\t\t\t\tpixelFormat = pipeConfig_->captureFormat;\n>> >> +\t\t\t} else {\n>> >> +\t\t\t\tauto it = std::find(pipeConfig_->outputFormats.begin(),\n>> >> +\t\t\t\t\t\t    pipeConfig_->outputFormats.end(),\n>> >> +\t\t\t\t\t\t    cfg.pixelFormat);\n>> >> +\t\t\t\tif (it == pipeConfig_->outputFormats.end())\n>> >> +\t\t\t\t\tit = pipeConfig_->outputFormats.begin();\n>> >> +\t\t\t\tpixelFormat = *it;\n>> >> +\t\t\t}\n>> >> +\n>> >> +\t\t\tif (cfg.pixelFormat != pixelFormat) {\n>> >> +\t\t\t\tif (rawStream) {\n>> >> +\t\t\t\t\tLOG(SimplePipeline, Info)\n>> >> +\t\t\t\t\t\t<< \"Raw pixel format \"\n>> >> +\t\t\t\t\t\t<< cfg.pixelFormat\n>> >> +\t\t\t\t\t\t<< \" doesn't match any of the pipe output formats\";\n>> >> +\t\t\t\t\treturn Invalid;\n>> >\n>> > We can chose the nearest raw format,size if we need to capture a raw stream.\n>> > simply return Adjusted in that case.\n>> \n>> Yes, unless a specific size for the stream was requested.\n>\n> I mean for the size as well. If the specific size is requested, the\n> pipeline is free is to adjust it and return the closest it to satisfy\n> the user requirements.\n\nI see, OK.\n\n>> >> +\t\t\t\t}\n>> >> +\t\t\t\tLOG(SimplePipeline, Debug)\n>> >> +\t\t\t\t\t<< \"Adjusting pixel format from \" << cfg.pixelFormat\n>> >> +\t\t\t\t\t<< \" to \" << pixelFormat;\n>> >> +\t\t\t\tcfg.pixelFormat = pixelFormat;\n>> >> +\t\t\t\t/*\n>> >> +\t\t\t\t * Do not touch the colour space for raw requested roles.\n>> >> +\t\t\t\t * Even if the pixel format is non-raw (whatever it means), we\n>> >> +\t\t\t\t * shouldn't try to interpret the colour space of raw data.\n>> >> +\t\t\t\t */\n>> >> +\t\t\t\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)\n>> >> +\t\t\t\t\tcfg.colorSpace->adjust(pixelFormat);\n>> >> +\t\t\t\tstatus = Adjusted;\n>> >> +\t\t\t}\n>> >> +\n>> >> +\t\t\tif (!cfg.colorSpace) {\n>> >> +\t\t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);\n>> >> +\t\t\t\tswitch (info.colourEncoding) {\n>> >> +\t\t\t\tcase PixelFormatInfo::ColourEncodingRGB:\n>> >> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Srgb;\n>> >> +\t\t\t\t\tbreak;\n>> >> +\t\t\t\tcase libcamera::PixelFormatInfo::ColourEncodingYUV:\n>> >> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Sycc;\n>> >> +\t\t\t\t\tbreak;\n>> >> +\t\t\t\tdefault:\n>> >> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Raw;\n>> >> +\t\t\t\t}\n>> >>  \t\t\t\tcfg.colorSpace->adjust(pixelFormat);\n>> >> -\t\t\tstatus = Adjusted;\n>> >> -\t\t}\n>> >> -\t\tif (!cfg.colorSpace) {\n>> >> -\t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);\n>> >> -\t\t\tswitch (info.colourEncoding) {\n>> >> -\t\t\tcase PixelFormatInfo::ColourEncodingRGB:\n>> >> -\t\t\t\tcfg.colorSpace = ColorSpace::Srgb;\n>> >> -\t\t\t\tbreak;\n>> >> -\t\t\tcase libcamera::PixelFormatInfo::ColourEncodingYUV:\n>> >> -\t\t\t\tcfg.colorSpace = ColorSpace::Sycc;\n>> >> -\t\t\t\tbreak;\n>> >> -\t\t\tdefault:\n>> >> -\t\t\t\tcfg.colorSpace = ColorSpace::Raw;\n>> >> +\t\t\t\tstatus = Adjusted;\n>> >>  \t\t\t}\n>> >> -\t\t\tcfg.colorSpace->adjust(pixelFormat);\n>> >> -\t\t\tstatus = Adjusted;\n>> >>  \t\t}\n>> >\n>> > I think I have suggested in one of the replies that colorspace handling\n>> > can be done separately, to progress them faster.\n>> \n>> The hunk above is just moving the code.\n>> \n>> > In my raw branch, I only set colorspace for raw streams, nothing else.\n>> > I expect that other colorspace related should be addressed separately.\n>> \n>> The basic colour space assignment is introduced in my \"Assign colour\n>> spaces in configurations\" patch, which is important to prevent crashes\n>> when the colour space is unset.\n>\n> Yeah, this should definitely be separate. This is not related to RAW\n> support.\n>\n>> \n>> >> -\t\tif (!pipeConfig_->outputSizes.contains(cfg.size)) {\n>> >> +\t\tif (!rawStream && !pipeConfig_->outputSizes.contains(cfg.size)) {\n>> >>  \t\t\tSize adjustedSize = pipeConfig_->captureSize;\n>> >>  \t\t\t/*\n>> >>  \t\t\t * The converter (when present) may not be able to output\n>> >> @@ -1259,11 +1301,20 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>> >>  \n>> >>  \t\t/* \\todo Create a libcamera core class to group format and size */\n>> >>  \t\tif (cfg.pixelFormat != pipeConfig_->captureFormat ||\n>> >> -\t\t    cfg.size != pipeConfig_->captureSize)\n>> >> +\t\t    cfg.size != pipeConfig_->captureSize) {\n>> >> +\t\t\tif (rawStream) {\n>> >> +\t\t\t\tLOG(SimplePipeline, Info)\n>> >> +\t\t\t\t\t<< \"Raw output format \" << cfg.pixelFormat\n>> >> +\t\t\t\t\t<< \" and size \" << cfg.size\n>> >> +\t\t\t\t\t<< \" not matching pipe format \" << pipeConfig_->captureFormat\n>> >> +\t\t\t\t\t<< \" and size \" << pipeConfig_->captureSize;\n>> >> +\t\t\t\treturn Invalid;\n>> >> +\t\t\t}\n>> >>  \t\t\tneedConversion_ = true;\n>> >> +\t\t}\n>> >>  \n>> >>  \t\t/* Set the stride, frameSize and bufferCount. */\n>> >> -\t\tif (needConversion_) {\n>> >> +\t\tif (needConversion_ && !rawStream) {\n>> >>  \t\t\tstd::tie(cfg.stride, cfg.frameSize) =\n>> >>  \t\t\t\tdata_->converter_\n>> >>  \t\t\t\t\t? data_->converter_->strideAndFrameSize(cfg.pixelFormat,\n>> >> -- \n>> >> 2.50.1\n>> >> \n>>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 8CB20C3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 22 Jul 2025 08:11:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 56C906901D;\n\tTue, 22 Jul 2025 10:11:34 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 50D0A68FAC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 22 Jul 2025 10:11:33 +0200 (CEST)","from mail-wm1-f69.google.com (mail-wm1-f69.google.com\n\t[209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-663--TkKQm0qORevDk5TyEIBZw-1; Tue, 22 Jul 2025 04:11:30 -0400","by mail-wm1-f69.google.com with SMTP id\n\t5b1f17b1804b1-4538f375e86so47567835e9.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 22 Jul 2025 01:11:30 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb ([85.93.96.130])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-3b61ca4d732sm12626810f8f.61.2025.07.22.01.11.27\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 22 Jul 2025 01:11:28 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"hIAA5h7k\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1753171892;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=uzl5gDzCbqeVl7JI20fLkUQaVb/H2vmrYp8rUx9er2Q=;\n\tb=hIAA5h7kRLIwqbWt01ICMq2qVT/j8Ls4BSczJ7D5SqsUbxQSk6zzUy3aAW1yaAzikzIcOi\n\toavdUNx3Lx4gSD6IyB+2lGirobLNlS0sPyjCIk6FYva0JQ4E0vLuk2nNYKhx3wKGZbgpmN\n\t8AXq1xpL2dwTKlwNz6cKBAQOXtU77OI=","X-MC-Unique":"-TkKQm0qORevDk5TyEIBZw-1","X-Mimecast-MFC-AGG-ID":"-TkKQm0qORevDk5TyEIBZw_1753171889","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1753171889; x=1753776689;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=uzl5gDzCbqeVl7JI20fLkUQaVb/H2vmrYp8rUx9er2Q=;\n\tb=we4TnQDUm6do0f7WQpVsWe5gPFd/jQ/dQ+taNZ0I5VnYGKq+LqEI2K4fvaOBntFkic\n\tAOVynhe53yYDYDWECHViL74m9pkF0FLzLi0OnpZkEDXD2Do2aVzb/2EZf0S7vASZzM5g\n\tdrdtPLADInBDzftmhJaBEXRNV/5bDotspRk7frLBBt2k8sFGd18S5grpzaTrSxWH3bkg\n\tBgnck9AKxQxaiKq5rgc0oWxZwokTtqhol9RH5q3g3rLjMEKNxXU2dTaTUqtFlpjAWom5\n\tJnBEryKxbN9iF9e0oUOh8IL+loX1//O5qbfKw1okvZIW/jS5ypxmSkqmPLZPAzZJpUOo\n\tJhLw==","X-Gm-Message-State":"AOJu0YzqgqhoRbhLK7JWHsPZ+RT1BXkdEyr6J2e96BmUckYoVqrxq+p5\n\tqyCO8dolRsQTnhWurRggBc8lQk3Rga1VEYrExvD2sY22lSaoyHtNgkrYkCRnWHB0g75vBEIuNvX\n\td+S06vAxCqAE3Q0ca8hZiSZWOTxeeYF4hsLVCEs3dvUMMHyJt2UBNzzGRT9bYyvnh1gKROOKHCA\n\tY=","X-Gm-Gg":"ASbGncsdtDf5TRuTGKB/bIdecGWDyZer4Cd9vjDuc+ZE6vfcOpycZIDQkshdvFwp74q\n\tkZFE4/Oyi0c6SkUV4Uq5kOoaE9/Zuz1HyXziu63lAOy3VBZJfbiHrRQW1dzSFzKGA2m0et4GgVk\n\t1lZ+9fzwlnaSnTS3eThCT9OZsaIFfTh4xEr3yvOQ7Cf5uI5Hi1BIDANrlhlwHXEUPOMFyvoT6rp\n\ta57x3ejk0uFBaHo16zdUUVsbMWyUG6qO1A9MksjSG19oE7Lr+JzevXkZVJBASAVlZSctNtTXGNk\n\tqqUxMQyDC54luYNU5c44P0hHet+RogYm1OvXMHDrTlDVunqraS+x2EVVAFa8WzB8","X-Received":["by 2002:a05:600c:4746:b0:455:ed0f:e8d4 with SMTP id\n\t5b1f17b1804b1-456346daa1amr225476965e9.10.1753171889231; \n\tTue, 22 Jul 2025 01:11:29 -0700 (PDT)","by 2002:a05:600c:4746:b0:455:ed0f:e8d4 with SMTP id\n\t5b1f17b1804b1-456346daa1amr225476615e9.10.1753171888761; \n\tTue, 22 Jul 2025 01:11:28 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IECGDqj49A8kV082j3/pK1al+zy2iyyKnUa+yzfp2RmnleOUNjbZA8p6EFK3Fd6mUpdcWaDJw==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Umang Jain <uajain@igalia.com>","Cc":"libcamera-devel@lists.libcamera.org,  Laurent Pinchart\n\t<laurent.pinchart@ideasonboard.com>,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>, =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?=\n\t<barnabas.pocze@ideasonboard.com>, Paul Elder\n\t<paul.elder@ideasonboard.com>","Subject":"Re: [PATCH v10 5/8] libcamera: simple: Validate raw stream\n\tconfigurations","In-Reply-To":"<lo36tlwsozc7zgneemzgn7eitrjmfd5bkay5hunhovajw3avrs@sx62fsghntrm>\n\t(Umang Jain's message of \"Tue, 22 Jul 2025 10:13:30 +0530\")","References":"<20250711175345.90318-1-mzamazal@redhat.com>\n\t<20250711175345.90318-6-mzamazal@redhat.com>\n\t<5ckm7wasrki7umzg2b2zzyhaokpdoiclgfnpz2g26enzzmgrvv@3ofdri4ljltc>\n\t<85bjpdb8cq.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t<lo36tlwsozc7zgneemzgn7eitrjmfd5bkay5hunhovajw3avrs@sx62fsghntrm>","Date":"Tue, 22 Jul 2025 10:11:27 +0200","Message-ID":"<85cy9sr6fk.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"vtsxAUuDnzRWHp-OUAClODId_DNVv5SIAkDML1BKkCM_1753171889","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35020,"web_url":"https://patchwork.libcamera.org/comment/35020/","msgid":"<85seiophoe.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-07-22T11:51:29","subject":"Re: [PATCH v10 5/8] libcamera: simple: Validate raw stream\n\tconfigurations","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Milan Zamazal <mzamazal@redhat.com> writes:\n\n> Hi Umang,\n>\n> Umang Jain <uajain@igalia.com> writes:\n>\n>> On Fri, Jul 11, 2025 at 07:53:38PM +0200, Milan Zamazal wrote:\n>>> SimpleCameraConfiguration::validate() looks for the best configuration.\n>>> As part of enabling raw stream support, the method must consider raw\n>>\n>>> streams in addition to the processed streams.\n>>> \n>>> If only a processed stream is requested, nothing changes.\n>>> \n>>> If only a raw stream is requested, the pixel format and output size may\n>>> not be adjusted.  The patch adds checks for this.\n>>> \n>>> If both processed and raw streams are requested, things get more\n>>> complicated.  The raw stream is expected to be passed through intact and\n>>> all the adjustments are made for the processed streams.  We select a\n>>> pipe configuration for the processed streams.\n>>> \n>>> Note that with both processed and raw streams, the requested sizes must\n>>> be mutually matching, including resizing due to debayer requirements.\n>>> For example, the following `cam' setup is valid for imx219\n>>> \n>>>   cam -s role=viewfinder,width=1920,height=1080 \\\n>>>       -s role=raw,width=3280,height=2464\n>>> \n>>> rather than\n>>> \n>>>   cam -s role=viewfinder,width=1920,height=1080 \\\n>>>       -s role=raw,width=1920,height=1080\n>>> \n>>> due to the resolution of 1924x1080 actually selected for debayering to\n>>> 1920x1080.  It is the application responsibility to select the right\n>>> parameters for the raw stream.\n>>> \n>>> Setting up the right configurations is still not enough to make the raw\n>>> streams working.  Buffer handling must be changed in the simple\n>>> pipeline, which is addressed in followup patches.\n>>> \n>>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n>>> ---\n>>>  src/libcamera/pipeline/simple/simple.cpp | 119 ++++++++++++++++-------\n>>>  1 file changed, 85 insertions(+), 34 deletions(-)\n>>> \n>>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>>> index 37abaa0e0..9d87a7ffa 100644\n>>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>>> @@ -27,6 +27,7 @@\n>>>  #include <libcamera/camera.h>\n>>>  #include <libcamera/color_space.h>\n>>>  #include <libcamera/control_ids.h>\n>>> +#include <libcamera/geometry.h>\n>>>  #include <libcamera/pixel_format.h>\n>>>  #include <libcamera/request.h>\n>>>  #include <libcamera/stream.h>\n>>> @@ -1186,6 +1187,20 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>>  \t\t<< \"-\" << pipeConfig_->captureFormat\n>>>  \t\t<< \" for max stream size \" << maxStreamSize;\n>>>  \n>>> +\tbool rawRequested = false;\n>>> +\tbool processedRequested = false;\n>>> +\tfor (const auto &cfg : config_)\n>>> +\t\tif (cfg.colorSpace == ColorSpace::Raw) {\n>>> +\t\t\tif (rawRequested) {\n>>> +\t\t\t\tLOG(SimplePipeline, Error)\n>>> +\t\t\t\t\t<< \"Can't capture multiple raw streams\";\n>>> +\t\t\t\treturn Invalid;\n>>> +\t\t\t}\n>>> +\t\t\trawRequested = true;\n>>> +\t\t} else {\n>>> +\t\t\tprocessedRequested = true;\n>>> +\t\t}\n>>> +\n>>>  \t/*\n>>>  \t * Adjust the requested streams.\n>>>  \t *\n>>> @@ -1204,43 +1219,70 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>>  \tfor (unsigned int i = 0; i < config_.size(); ++i) {\n>>>  \t\tStreamConfiguration &cfg = config_[i];\n>>>  \n>>> +\t\t/*\n>>> +\t\t * If both processed and raw streams are requested, the pipe\n>>> +\t\t * configuration is set up for the processed stream. The raw\n>>> +\t\t * configuration needs to be compared against the capture format and\n>>> +\t\t * size in such a case.\n>>> +\t\t */\n>>> +\t\tconst bool rawStream = cfg.colorSpace == ColorSpace::Raw;\n>>\n>> We should always inspect the Stream or the pixelformat to set flags like\n>> this. Inspecting colorspace seems flaky, especially when it is a\n>> std::optional<> in CameraConfiguration.\n>\n> My idea was to (mis)use this to pass the intention about the stream role\n> from generateConfiguration().  But selecting a consistent configuration\n> already in generateConfiguration(), something like what you do in your\n> patch, looks like a better idea.\n\nHmm, but it depends on the assumption that raw streams are exactly those\nwith raw pixel formats, which I doubt is right.  If it was right then it\nwould work.  But if not then we have little means to do otherwise than\nmisusing the colour space.  Specifying raw/non-raw in\nStreamConfiguration would be completely clear but I've been hesitating\nto do that because it doesn't fit the other pipelines (and I'm not sure\nif it'd influence applications in any negative way).\n\nAm I getting confused too much?\n\n>>> +\t\tconst bool sideRawStream = rawStream && processedRequested;\n>>> +\n>>>  \t\t/* Adjust the pixel format and size. */\n>>> -\t\tauto it = std::find(pipeConfig_->outputFormats.begin(),\n>>> -\t\t\t\t    pipeConfig_->outputFormats.end(),\n>>> -\t\t\t\t    cfg.pixelFormat);\n>>> -\t\tif (it == pipeConfig_->outputFormats.end())\n>>> -\t\t\tit = pipeConfig_->outputFormats.begin();\n>>> -\n>>> -\t\tPixelFormat pixelFormat = *it;\n>>> -\t\tif (cfg.pixelFormat != pixelFormat) {\n>>> -\t\t\tLOG(SimplePipeline, Debug) << \"Adjusting pixel format\";\n>>> -\t\t\tcfg.pixelFormat = pixelFormat;\n>>> -\t\t\t/*\n>>> -\t\t\t * Do not touch the colour space for raw requested roles.\n>>> -\t\t\t * Even if the pixel format is non-raw (whatever it means), we\n>>> -\t\t\t * shouldn't try to interpret the colour space of raw data.\n>>> -\t\t\t */\n>>> -\t\t\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)\n>>> +\n>>> +\t\tif (!sideRawStream) {\n>>> +\t\t\tPixelFormat pixelFormat;\n>>> +\t\t\tif (rawStream) {\n>>> +\t\t\t\tpixelFormat = pipeConfig_->captureFormat;\n>>> +\t\t\t} else {\n>>> +\t\t\t\tauto it = std::find(pipeConfig_->outputFormats.begin(),\n>>> +\t\t\t\t\t\t    pipeConfig_->outputFormats.end(),\n>>> +\t\t\t\t\t\t    cfg.pixelFormat);\n>>> +\t\t\t\tif (it == pipeConfig_->outputFormats.end())\n>>> +\t\t\t\t\tit = pipeConfig_->outputFormats.begin();\n>>> +\t\t\t\tpixelFormat = *it;\n>>> +\t\t\t}\n>>> +\n>>> +\t\t\tif (cfg.pixelFormat != pixelFormat) {\n>>> +\t\t\t\tif (rawStream) {\n>>> +\t\t\t\t\tLOG(SimplePipeline, Info)\n>>> +\t\t\t\t\t\t<< \"Raw pixel format \"\n>>> +\t\t\t\t\t\t<< cfg.pixelFormat\n>>> +\t\t\t\t\t\t<< \" doesn't match any of the pipe output formats\";\n>>> +\t\t\t\t\treturn Invalid;\n>>\n>> We can chose the nearest raw format,size if we need to capture a raw stream.\n>> simply return Adjusted in that case.\n>\n> Yes, unless a specific size for the stream was requested.\n>\n>>> +\t\t\t\t}\n>>> +\t\t\t\tLOG(SimplePipeline, Debug)\n>>> +\t\t\t\t\t<< \"Adjusting pixel format from \" << cfg.pixelFormat\n>>> +\t\t\t\t\t<< \" to \" << pixelFormat;\n>>> +\t\t\t\tcfg.pixelFormat = pixelFormat;\n>>> +\t\t\t\t/*\n>>> +\t\t\t\t * Do not touch the colour space for raw requested roles.\n>>> +\t\t\t\t * Even if the pixel format is non-raw (whatever it means), we\n>>> +\t\t\t\t * shouldn't try to interpret the colour space of raw data.\n>>> +\t\t\t\t */\n>>> +\t\t\t\tif (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)\n>>> +\t\t\t\t\tcfg.colorSpace->adjust(pixelFormat);\n>>> +\t\t\t\tstatus = Adjusted;\n>>> +\t\t\t}\n>>> +\n>>> +\t\t\tif (!cfg.colorSpace) {\n>>> +\t\t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);\n>>> +\t\t\t\tswitch (info.colourEncoding) {\n>>> +\t\t\t\tcase PixelFormatInfo::ColourEncodingRGB:\n>>> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Srgb;\n>>> +\t\t\t\t\tbreak;\n>>> +\t\t\t\tcase libcamera::PixelFormatInfo::ColourEncodingYUV:\n>>> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Sycc;\n>>> +\t\t\t\t\tbreak;\n>>> +\t\t\t\tdefault:\n>>> +\t\t\t\t\tcfg.colorSpace = ColorSpace::Raw;\n>>> +\t\t\t\t}\n>>>  \t\t\t\tcfg.colorSpace->adjust(pixelFormat);\n>>> -\t\t\tstatus = Adjusted;\n>>> -\t\t}\n>>> -\t\tif (!cfg.colorSpace) {\n>>> -\t\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);\n>>> -\t\t\tswitch (info.colourEncoding) {\n>>> -\t\t\tcase PixelFormatInfo::ColourEncodingRGB:\n>>> -\t\t\t\tcfg.colorSpace = ColorSpace::Srgb;\n>>> -\t\t\t\tbreak;\n>>> -\t\t\tcase libcamera::PixelFormatInfo::ColourEncodingYUV:\n>>> -\t\t\t\tcfg.colorSpace = ColorSpace::Sycc;\n>>> -\t\t\t\tbreak;\n>>> -\t\t\tdefault:\n>>> -\t\t\t\tcfg.colorSpace = ColorSpace::Raw;\n>>> +\t\t\t\tstatus = Adjusted;\n>>>  \t\t\t}\n>>> -\t\t\tcfg.colorSpace->adjust(pixelFormat);\n>>> -\t\t\tstatus = Adjusted;\n>>>  \t\t}\n>>\n>> I think I have suggested in one of the replies that colorspace handling\n>> can be done separately, to progress them faster.\n>\n> The hunk above is just moving the code.\n>\n>> In my raw branch, I only set colorspace for raw streams, nothing else.\n>> I expect that other colorspace related should be addressed separately.\n>\n> The basic colour space assignment is introduced in my \"Assign colour\n> spaces in configurations\" patch, which is important to prevent crashes\n> when the colour space is unset.\n>\n>>> -\t\tif (!pipeConfig_->outputSizes.contains(cfg.size)) {\n>>> +\t\tif (!rawStream && !pipeConfig_->outputSizes.contains(cfg.size)) {\n>>>  \t\t\tSize adjustedSize = pipeConfig_->captureSize;\n>>>  \t\t\t/*\n>>>  \t\t\t * The converter (when present) may not be able to output\n>>> @@ -1259,11 +1301,20 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()\n>>>  \n>>>  \t\t/* \\todo Create a libcamera core class to group format and size */\n>>>  \t\tif (cfg.pixelFormat != pipeConfig_->captureFormat ||\n>>> -\t\t    cfg.size != pipeConfig_->captureSize)\n>>> +\t\t    cfg.size != pipeConfig_->captureSize) {\n>>> +\t\t\tif (rawStream) {\n>>> +\t\t\t\tLOG(SimplePipeline, Info)\n>>> +\t\t\t\t\t<< \"Raw output format \" << cfg.pixelFormat\n>>> +\t\t\t\t\t<< \" and size \" << cfg.size\n>>> +\t\t\t\t\t<< \" not matching pipe format \" << pipeConfig_->captureFormat\n>>> +\t\t\t\t\t<< \" and size \" << pipeConfig_->captureSize;\n>>> +\t\t\t\treturn Invalid;\n>>> +\t\t\t}\n>>>  \t\t\tneedConversion_ = true;\n>>> +\t\t}\n>>>  \n>>>  \t\t/* Set the stride, frameSize and bufferCount. */\n>>> -\t\tif (needConversion_) {\n>>> +\t\tif (needConversion_ && !rawStream) {\n>>>  \t\t\tstd::tie(cfg.stride, cfg.frameSize) =\n>>>  \t\t\t\tdata_->converter_\n>>>  \t\t\t\t\t? data_->converter_->strideAndFrameSize(cfg.pixelFormat,\n>>> -- \n>>> 2.50.1\n>>>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 4B2C6C3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 22 Jul 2025 11:51:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0B87569034;\n\tTue, 22 Jul 2025 13:51:37 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BA67468F93\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 22 Jul 2025 13:51:34 +0200 (CEST)","from mail-wr1-f71.google.com (mail-wr1-f71.google.com\n\t[209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-378-Q10ZF2LKOYmBoP9QOT5Rpg-1; Tue, 22 Jul 2025 07:51:32 -0400","by mail-wr1-f71.google.com with SMTP id\n\tffacd0b85a97d-3b20f50da27so2537062f8f.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 22 Jul 2025 04:51:32 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb ([85.93.96.130])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-4563b74f8a9sm130627015e9.26.2025.07.22.04.51.29\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 22 Jul 2025 04:51:29 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"A231Cjwi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1753185093;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=UhtgrzeELQzqX+YmHtgvR4tsgwXOldVmCKk8UDg1W5M=;\n\tb=A231CjwifSou+UsgoJrd3vYfg6fEjMDd27hwWh9kTv5nhGcfG8thwB/kDXmtav4m3VXVbT\n\txjgLWznpXLm6gfyJZhV17Ejg1N7Xm1MeHmKQ/nM2Ay19nNud/GqpvMX+4Vr+gNJDoOUa2+\n\t0rU4MCZB/aehq2Ybp4cSuZV2BbTY8Ms=","X-MC-Unique":"Q10ZF2LKOYmBoP9QOT5Rpg-1","X-Mimecast-MFC-AGG-ID":"Q10ZF2LKOYmBoP9QOT5Rpg_1753185091","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1753185091; x=1753789891;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=UhtgrzeELQzqX+YmHtgvR4tsgwXOldVmCKk8UDg1W5M=;\n\tb=QS6Ah2fgpJvv2WomYMBBEWuDSV1thGA3BdWuhvu4fskj2dBzt0k6xfKfiomlnYSCaV\n\tizy6cRm0w6f65mqOLvFatBn2/QpR00hw04nzWjaHtCjkfM1eTZLRPm9o1q0lfQnZIXxv\n\txB2f0o8z1oZqJissK1iSDGO31QxlmT0kUi8eL1owa+31HWla+F/vef0ZeTVSBqLrcauc\n\ttgC7vu0kSzMScWwSo64ZCTkSdASuAxkK3DAZ+Im195cmwfbu/m/orVzagk/ZFDk5JFW7\n\ttA725od0V3WP0DvlyF+epqf25XEtClIc0KKTnjTsA1BCUbv00GWKJTd2ruSagHnN42m4\n\ttIHg==","X-Gm-Message-State":"AOJu0YyZLQG0X3RWhXw9T0F2FtPE029eXupAvCSb66BB9qnKG9mjlib0\n\tQalugjeMqTUbE2hIhzii7bxZqNmIo6F80tg8JcpsnKUhSUE6Ye3pBDUlYxAdOUTAiAbIwhNWS3C\n\tq0aCMFu9LYAgR2wJCxyxqr7GeGA04G+WP4YuWqxRigOqmOHNb77k65XDD3xv2ShtjLvSbuNQf3q\n\tg=","X-Gm-Gg":"ASbGnctvptBAVEaNoytbGVF6vN0/T4cK2SMWuBoVFIWoaYVCE0pAf5QQ2C8v1NM0pyH\n\tYokC7QyOY/aKAv2GR3dwYGxvcQjXn794YLbwANBRdstR2OwAuUFn4UecQfJCC01fUi7CsVPT7wH\n\t5rXXnFn+xz442aEjXrWrK5t6zq+gEisMfdh1b6xHSYnhCs41HTAdn/uTi3YKCG5FvZwB1uh1pMK\n\t5U7aAxHz/LdrJ37kYmgWrocI/oihWkT19y7UHgNxyrcfMPtUHzXyGt27xVXkPOibE3BYefsXZq9\n\t74ZZSnLtXgtZj7V0ZvknrfrG7BE2A6ozLkRmUtF7ipSaakxT13V3gbkbd8Zk3r15","X-Received":["by 2002:a05:6000:25ca:b0:3b6:d0d:79c1 with SMTP id\n\tffacd0b85a97d-3b76348f75amr2388175f8f.10.1753185090959; \n\tTue, 22 Jul 2025 04:51:30 -0700 (PDT)","by 2002:a05:6000:25ca:b0:3b6:d0d:79c1 with SMTP id\n\tffacd0b85a97d-3b76348f75amr2388147f8f.10.1753185090416; \n\tTue, 22 Jul 2025 04:51:30 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IFPgW0Vm6F/u0+UxKNqE37lG7o2BgZofOszUctO/kaDRtLFMX9DC26+n7PRpDIbREPfjsvZPw==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Umang Jain <uajain@igalia.com>","Cc":"libcamera-devel@lists.libcamera.org,  Laurent Pinchart\n\t<laurent.pinchart@ideasonboard.com>,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>, =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?=\n\t<barnabas.pocze@ideasonboard.com>, Paul Elder\n\t<paul.elder@ideasonboard.com>","Subject":"Re: [PATCH v10 5/8] libcamera: simple: Validate raw stream\n\tconfigurations","In-Reply-To":"<85bjpdb8cq.fsf@mzamazal-thinkpadp1gen7.tpbc.csb> (Milan\n\tZamazal's message of \"Mon, 21 Jul 2025 22:24:21 +0200\")","References":"<20250711175345.90318-1-mzamazal@redhat.com>\n\t<20250711175345.90318-6-mzamazal@redhat.com>\n\t<5ckm7wasrki7umzg2b2zzyhaokpdoiclgfnpz2g26enzzmgrvv@3ofdri4ljltc>\n\t<85bjpdb8cq.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Date":"Tue, 22 Jul 2025 13:51:29 +0200","Message-ID":"<85seiophoe.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"M3qWCE4QCKqlvxHp58pF6rdvkHLB6L8tqKWA1u65kNo_1753185091","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]