[{"id":36460,"web_url":"https://patchwork.libcamera.org/comment/36460/","msgid":"","date":"2025-10-25T09:00:39","subject":"Re: [PATCH v13 8/8] libcamera: simple: Make raw streams working","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"On Tue, Oct 21, 2025 at 08:27:15PM +0200, Milan Zamazal wrote:\n> When a raw stream is requested, whether alone or together with a\n> processed stream, its buffers must be handled outside the software ISP\n> machinery. They serve as output buffers, even when a processed stream\n> is produced.\n> \n> At most one raw stream and at most one processed stream are supported\n> and can be combined. An example of producing both raw and processed\n> files using `cam' application:\n> \n> cam -c1 -C100 -Ffile# \\\n> -s role=viewfinder,width=1920,height=1080,pixelformat=RGB888 \\\n> -s role=raw,width=3280,height=2464,pixelformat=SRGGB8 \\\n> \n> Note the difference in viewfinder and raw stream sizes due to the fact\n> that debayering requires enlarging the image width, which enforces\n> selecting a larger sensor resolution in this case.\n> \n> In order to track whether a raw stream is requested and which one it is,\n> SimpleCameraData::rawStream_ member variable is introduced.\n> \n> This is the final step to make raw streams working.\n> \n> Signed-off-by: Milan Zamazal \n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 50 +++++++++++++++++-------\n> 1 file changed, 36 insertions(+), 14 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 0c3284feb..607b07949 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -333,6 +333,7 @@ public:\n> \t};\n> \n> \tstd::vector streams_;\n> +\tStream *rawStream_;\n> \n> \t/*\n> \t * All entities in the pipeline, from the camera sensor to the video\n> @@ -371,6 +372,11 @@ private:\n> \tvoid ispStatsReady(uint32_t frame, uint32_t bufferId);\n> \tvoid metadataReady(uint32_t frame, const ControlList &metadata);\n> \tvoid setSensorControls(const ControlList &sensorControls);\n> +\n> +\tbool processedRequested() const\n> +\t{\n> +\t\treturn streams_.size() - (rawStream_ ? 1 : 0) > 0;\n> +\t}\n> };\n> \n> class SimpleCameraConfiguration : public CameraConfiguration\n> @@ -467,7 +473,7 @@ private:\n> SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n> \t\t\t\t unsigned int numStreams,\n> \t\t\t\t MediaEntity *sensor)\n> -\t: Camera::Private(pipe), streams_(numStreams)\n> +\t: Camera::Private(pipe), streams_(numStreams), rawStream_(nullptr)\n> {\n> \t/*\n> \t * Find the shortest path from the camera sensor to a video capture\n> @@ -878,10 +884,13 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> \t * point converting an erroneous buffer.\n> \t */\n> \tif (buffer->metadata().status != FrameMetadata::FrameSuccess) {\n> -\t\tif (!useConversion_) {\n> +\t\tif (rawStream_) {\n\n[1]\n\nI really don't understand the change here. Also doesn't it break the case\nwhen only one processed stream is requested and an error occured ?\n\n> \t\t\t/* No conversion, just complete the request. */\n> \t\t\tRequest *request = buffer->request();\n> \t\t\tpipe->completeBuffer(request, buffer);\n> +\t\t\tSimpleFrameInfo *info = frameInfo_.find(request->sequence());\n> +\t\t\tif (info)\n> +\t\t\t\tinfo->metadataRequired = false;\n\nIf this change is trying to adapt the if condition in\ntryCompleteRequest(), I believe this should be a different\npatch. For now, I don't see how it relates to raw stream enabling.\n\n> \t\t\ttryCompleteRequest(request);\n> \t\t\treturn;\n> \t\t}\n> @@ -891,7 +900,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> \t\t * buffer for capture (unless the stream is being stopped), and\n> \t\t * complete the request with all the user-facing buffers.\n> \t\t */\n> -\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled)\n> +\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled &&\n> +\t\t !rawStream_)\n\nDo you need to check for !rawStream_ again? The codepath would have\nreturned early already if I consider the change above at [1], no? \n\n> \t\t\tvideo_->queueBuffer(buffer);\n> \n> \t\tif (conversionQueue_.empty())\n> @@ -940,13 +950,14 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> \t */\n> \tif (useConversion_) {\n> \t\tif (conversionQueue_.empty()) {\n> -\t\t\tvideo_->queueBuffer(buffer);\n> +\t\t\tif (!rawStream_)\n> +\t\t\t\tvideo_->queueBuffer(buffer);\n> \t\t\treturn;\n> \t\t}\n> \n> \t\tif (converter_)\n> \t\t\tconverter_->queueBuffers(buffer, conversionQueue_.front().outputs);\n> -\t\telse\n> +\t\telse if (processedRequested())\n\nIsn't this implicit, if useConversion_=True, you either go through\nconverter_ or softISP. We don't need to check for processedRequested()\nhere.\n\n> \t\t\t/*\n> \t\t\t * request->sequence() cannot be retrieved from `buffer' inside\n> \t\t\t * queueBuffers because unique_ptr's make buffer->request() invalid\n> @@ -956,6 +967,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> \t\t\t\t\t conversionQueue_.front().outputs);\n> \n> \t\tconversionQueue_.pop();\n> +\t\tif (rawStream_)\n> +\t\t\tpipe->completeBuffer(request, buffer);\n\nThis shouldn't be needed either, the function has pipe->completeBuffer()\nout of this if block.\n\n> \t\treturn;\n> \t}\n> \n> @@ -993,7 +1006,8 @@ void SimpleCameraData::tryCompleteRequest(Request *request)\n> void SimpleCameraData::conversionInputDone(FrameBuffer *buffer)\n> {\n> \t/* Queue the input buffer back for capture. */\n> -\tvideo_->queueBuffer(buffer);\n> +\tif (!rawStream_)\n> +\t\tvideo_->queueBuffer(buffer);\n\nDon't you need to also complete the raw buffer in else block ?\nThis would be the case where only raw stream is requested. Or is it done\nelsewhere?\n\n> }\n> \n> void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n> @@ -1518,11 +1532,15 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> \n> \tfor (unsigned int i = 0; i < config->size(); ++i) {\n> \t\tStreamConfiguration &cfg = config->at(i);\n> +\t\tbool rawStream = isRaw(cfg);\n> \n> \t\tcfg.setStream(&data->streams_[i]);\n> \n> -\t\tif (data->useConversion_ && !isRaw(cfg))\n> +\t\tif (data->useConversion_ && !rawStream)\n> \t\t\toutputCfgs.push_back(cfg);\n> +\n> +\t\tif (rawStream)\n> +\t\t\tdata->rawStream_ = &data->streams_[i];\n> \t}\n> \n> \tif (outputCfgs.empty())\n> @@ -1553,7 +1571,7 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n> \t * Export buffers on the converter or capture video node, depending on\n> \t * whether the converter is used or not.\n> \t */\n> -\tif (data->useConversion_)\n> +\tif (data->useConversion_ && stream != data->rawStream_)\n> \t\treturn data->converter_\n> \t\t\t ? data->converter_->exportBuffers(stream, count, buffers)\n> \t\t\t : data->swIsp_->exportBuffers(stream, count, buffers);\n> @@ -1576,7 +1594,7 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n> \t\treturn -EBUSY;\n> \t}\n> \n> -\tif (data->useConversion_) {\n> +\tif (data->useConversion_ && !data->rawStream_) {\n> \t\t/*\n> \t\t * When using the converter allocate a fixed number of internal\n> \t\t * buffers.\n> @@ -1584,8 +1602,11 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n> \t\tret = video->allocateBuffers(kNumInternalBuffers,\n> \t\t\t\t\t &data->conversionBuffers_);\n> \t} else {\n> -\t\t/* Otherwise, prepare for using buffers from the only stream. */\n> -\t\tStream *stream = &data->streams_[0];\n> +\t\t/*\n> +\t\t * Otherwise, prepare for using buffers from either the raw stream, if\n> +\t\t * requested, or the only stream configured.\n> +\t\t */\n> +\t\tStream *stream = (data->rawStream_ ? data->rawStream_ : &data->streams_[0]);\n> \t\tret = video->importBuffers(stream->configuration().bufferCount);\n> \t}\n> \tif (ret < 0) {\n> @@ -1626,8 +1647,9 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n> \t\t}\n> \n> \t\t/* Queue all internal buffers for capture. */\n> -\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n> -\t\t\tvideo->queueBuffer(buffer.get());\n> +\t\tif (!data->rawStream_)\n> +\t\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n> +\t\t\t\tvideo->queueBuffer(buffer.get());\n> \t}\n> \n> \treturn 0;\n> @@ -1678,7 +1700,7 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)\n> \t\t * queue, it will be handed to the converter in the capture\n> \t\t * completion handler.\n> \t\t */\n> -\t\tif (data->useConversion_) {\n> +\t\tif (data->useConversion_ && stream != data->rawStream_) {\n> \t\t\tbuffers.emplace(stream, buffer);\n> \t\t\tmetadataRequired = !!data->swIsp_;\n> \t\t} else {\n> -- \n> 2.51.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 22485C3259\n\tfor ;\n\tSat, 25 Oct 2025 09:00:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5680B6068F;\n\tSat, 25 Oct 2025 11:00:14 +0200 (CEST)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A18A9605F3\n\tfor ;\n\tSat, 25 Oct 2025 11:00:11 +0200 (CEST)","from [85.255.233.179] (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 1vCa8A-00F3hG-LH; Sat, 25 Oct 2025 11:00:10 +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=\"khGp9Awo\"; 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=pPpJo8ir4OiJBNYpjdsaH3CJDiDclYN3zezEAvrrUrk=;\n\tb=khGp9AworRV8NEh04J1q4DKnVi\n\tmtyyeRJKYPYAYPAgskviqZYP7sC/KqGkZvsgdMFNFtTt+1Ehc6YGp5vZXYpm5yFZw9PuCtre1Aaj9\n\tdcEGOUO63hrTRlZspJ71b8T6ke5r8u17+mQPrmD/7xxaUwiUjWPTYm0+9S6utw38YioZGYBEsFPDQ\n\tA7bJpUVJS/bHByFxM/bJEKHR1+XcumFejqeJXbsFBHmFXNIO6TDxUUMCS9uHB8Qij93Xt+w7kCus3\n\t4/DGdIHkZFK6uYaCY1SrligQL9sQfPpftD05sYZS0qANCrndcLQcTEP/egE9Uz7rdm2FRqLElSJIC\n\tBR99gwBg==;","Date":"Sat, 25 Oct 2025 10:00:39 +0100","From":"Umang Jain ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t, Kieran Bingham\n\t, =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t, Paul Elder\n\t","Subject":"Re: [PATCH v13 8/8] libcamera: simple: Make raw streams working","Message-ID":"","References":"<20251021182716.29274-1-mzamazal@redhat.com>\n\t<20251021182716.29274-9-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20251021182716.29274-9-mzamazal@redhat.com>","User-Agent":"NeoMutt/20250905-dirty","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":36461,"web_url":"https://patchwork.libcamera.org/comment/36461/","msgid":"<7crwp5hfbuacvp2ezazjwfzv4vgdzyseh2bdnao6wq4cukpz3z@zk76tzg47zyu>","date":"2025-10-25T09:06:42","subject":"Re: [PATCH v13 8/8] libcamera: simple: Make raw streams working","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"I reviewed most of the patches and am OK with them. I prefer a slight\nrework in 4/8 and have highlighted it (with a reference diff I\nimplemented earlier).\n\nUnfortuantely, I did find patch 8/8 hard-to-follow :(\nBut if it's just me, it would be better if someone else should take a\nlook.\n\n\nOn Tue, Oct 21, 2025 at 08:27:15PM +0200, Milan Zamazal wrote:\n> When a raw stream is requested, whether alone or together with a\n> processed stream, its buffers must be handled outside the software ISP\n> machinery. They serve as output buffers, even when a processed stream\n> is produced.\n> \n> At most one raw stream and at most one processed stream are supported\n> and can be combined. An example of producing both raw and processed\n> files using `cam' application:\n> \n> cam -c1 -C100 -Ffile# \\\n> -s role=viewfinder,width=1920,height=1080,pixelformat=RGB888 \\\n> -s role=raw,width=3280,height=2464,pixelformat=SRGGB8 \\\n> \n> Note the difference in viewfinder and raw stream sizes due to the fact\n> that debayering requires enlarging the image width, which enforces\n> selecting a larger sensor resolution in this case.\n> \n> In order to track whether a raw stream is requested and which one it is,\n> SimpleCameraData::rawStream_ member variable is introduced.\n> \n> This is the final step to make raw streams working.\n> \n> Signed-off-by: Milan Zamazal \n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 50 +++++++++++++++++-------\n> 1 file changed, 36 insertions(+), 14 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 0c3284feb..607b07949 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -333,6 +333,7 @@ public:\n> \t};\n> \n> \tstd::vector streams_;\n> +\tStream *rawStream_;\n> \n> \t/*\n> \t * All entities in the pipeline, from the camera sensor to the video\n> @@ -371,6 +372,11 @@ private:\n> \tvoid ispStatsReady(uint32_t frame, uint32_t bufferId);\n> \tvoid metadataReady(uint32_t frame, const ControlList &metadata);\n> \tvoid setSensorControls(const ControlList &sensorControls);\n> +\n> +\tbool processedRequested() const\n> +\t{\n> +\t\treturn streams_.size() - (rawStream_ ? 1 : 0) > 0;\n> +\t}\n> };\n> \n> class SimpleCameraConfiguration : public CameraConfiguration\n> @@ -467,7 +473,7 @@ private:\n> SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n> \t\t\t\t unsigned int numStreams,\n> \t\t\t\t MediaEntity *sensor)\n> -\t: Camera::Private(pipe), streams_(numStreams)\n> +\t: Camera::Private(pipe), streams_(numStreams), rawStream_(nullptr)\n> {\n> \t/*\n> \t * Find the shortest path from the camera sensor to a video capture\n> @@ -878,10 +884,13 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> \t * point converting an erroneous buffer.\n> \t */\n> \tif (buffer->metadata().status != FrameMetadata::FrameSuccess) {\n> -\t\tif (!useConversion_) {\n> +\t\tif (rawStream_) {\n> \t\t\t/* No conversion, just complete the request. */\n> \t\t\tRequest *request = buffer->request();\n> \t\t\tpipe->completeBuffer(request, buffer);\n> +\t\t\tSimpleFrameInfo *info = frameInfo_.find(request->sequence());\n> +\t\t\tif (info)\n> +\t\t\t\tinfo->metadataRequired = false;\n> \t\t\ttryCompleteRequest(request);\n> \t\t\treturn;\n> \t\t}\n> @@ -891,7 +900,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> \t\t * buffer for capture (unless the stream is being stopped), and\n> \t\t * complete the request with all the user-facing buffers.\n> \t\t */\n> -\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled)\n> +\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled &&\n> +\t\t !rawStream_)\n> \t\t\tvideo_->queueBuffer(buffer);\n> \n> \t\tif (conversionQueue_.empty())\n> @@ -940,13 +950,14 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> \t */\n> \tif (useConversion_) {\n> \t\tif (conversionQueue_.empty()) {\n> -\t\t\tvideo_->queueBuffer(buffer);\n> +\t\t\tif (!rawStream_)\n> +\t\t\t\tvideo_->queueBuffer(buffer);\n> \t\t\treturn;\n> \t\t}\n> \n> \t\tif (converter_)\n> \t\t\tconverter_->queueBuffers(buffer, conversionQueue_.front().outputs);\n> -\t\telse\n> +\t\telse if (processedRequested())\n> \t\t\t/*\n> \t\t\t * request->sequence() cannot be retrieved from `buffer' inside\n> \t\t\t * queueBuffers because unique_ptr's make buffer->request() invalid\n> @@ -956,6 +967,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> \t\t\t\t\t conversionQueue_.front().outputs);\n> \n> \t\tconversionQueue_.pop();\n> +\t\tif (rawStream_)\n> +\t\t\tpipe->completeBuffer(request, buffer);\n> \t\treturn;\n> \t}\n> \n> @@ -993,7 +1006,8 @@ void SimpleCameraData::tryCompleteRequest(Request *request)\n> void SimpleCameraData::conversionInputDone(FrameBuffer *buffer)\n> {\n> \t/* Queue the input buffer back for capture. */\n> -\tvideo_->queueBuffer(buffer);\n> +\tif (!rawStream_)\n> +\t\tvideo_->queueBuffer(buffer);\n> }\n> \n> void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n> @@ -1518,11 +1532,15 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> \n> \tfor (unsigned int i = 0; i < config->size(); ++i) {\n> \t\tStreamConfiguration &cfg = config->at(i);\n> +\t\tbool rawStream = isRaw(cfg);\n> \n> \t\tcfg.setStream(&data->streams_[i]);\n> \n> -\t\tif (data->useConversion_ && !isRaw(cfg))\n> +\t\tif (data->useConversion_ && !rawStream)\n> \t\t\toutputCfgs.push_back(cfg);\n> +\n> +\t\tif (rawStream)\n> +\t\t\tdata->rawStream_ = &data->streams_[i];\n> \t}\n> \n> \tif (outputCfgs.empty())\n> @@ -1553,7 +1571,7 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n> \t * Export buffers on the converter or capture video node, depending on\n> \t * whether the converter is used or not.\n> \t */\n> -\tif (data->useConversion_)\n> +\tif (data->useConversion_ && stream != data->rawStream_)\n> \t\treturn data->converter_\n> \t\t\t ? data->converter_->exportBuffers(stream, count, buffers)\n> \t\t\t : data->swIsp_->exportBuffers(stream, count, buffers);\n> @@ -1576,7 +1594,7 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n> \t\treturn -EBUSY;\n> \t}\n> \n> -\tif (data->useConversion_) {\n> +\tif (data->useConversion_ && !data->rawStream_) {\n> \t\t/*\n> \t\t * When using the converter allocate a fixed number of internal\n> \t\t * buffers.\n> @@ -1584,8 +1602,11 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n> \t\tret = video->allocateBuffers(kNumInternalBuffers,\n> \t\t\t\t\t &data->conversionBuffers_);\n> \t} else {\n> -\t\t/* Otherwise, prepare for using buffers from the only stream. */\n> -\t\tStream *stream = &data->streams_[0];\n> +\t\t/*\n> +\t\t * Otherwise, prepare for using buffers from either the raw stream, if\n> +\t\t * requested, or the only stream configured.\n> +\t\t */\n> +\t\tStream *stream = (data->rawStream_ ? data->rawStream_ : &data->streams_[0]);\n> \t\tret = video->importBuffers(stream->configuration().bufferCount);\n> \t}\n> \tif (ret < 0) {\n> @@ -1626,8 +1647,9 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n> \t\t}\n> \n> \t\t/* Queue all internal buffers for capture. */\n> -\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n> -\t\t\tvideo->queueBuffer(buffer.get());\n> +\t\tif (!data->rawStream_)\n> +\t\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n> +\t\t\t\tvideo->queueBuffer(buffer.get());\n> \t}\n> \n> \treturn 0;\n> @@ -1678,7 +1700,7 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)\n> \t\t * queue, it will be handed to the converter in the capture\n> \t\t * completion handler.\n> \t\t */\n> -\t\tif (data->useConversion_) {\n> +\t\tif (data->useConversion_ && stream != data->rawStream_) {\n> \t\t\tbuffers.emplace(stream, buffer);\n> \t\t\tmetadataRequired = !!data->swIsp_;\n> \t\t} else {\n> -- \n> 2.51.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6BB91BE080\n\tfor ;\n\tSat, 25 Oct 2025 09:06:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 67643605F3;\n\tSat, 25 Oct 2025 11:06:16 +0200 (CEST)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4FBC3605F3\n\tfor ;\n\tSat, 25 Oct 2025 11:06:14 +0200 (CEST)","from [85.255.233.179] (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 1vCaE1-00F3mS-G5; Sat, 25 Oct 2025 11:06:13 +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=\"KTFUvihY\"; 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=ReiGnl/Wiq/MblhxaSuUUzKCMJZwhWjyS8BteIrnoyo=;\n\tb=KTFUvihYcM8YIFiaCC6vXLaw0w\n\tskjDQk+wVYxqY19JH78tGZiZGEex3Mz/ybdJV2yQT2bzaiqat3G9pt7ouBE0T0S9cuZot8pV9ApWs\n\tRRV50tCeId7PTP7e5y3jc5q39UhhDJjiLTOwbzgX/JC9i+JtR9xmlcU4+xLJW33oGoHtIdImJ+Cxg\n\tOjhOPx8a5TrATnE0fqsGqfNecVCnvBrNDnK51pm8EAJkXZqQ7TsrcyYU0bs39aq1jDeAq8pGLDYLU\n\t03bW5fyW3t1UANseCqDrC9LMbb9ZZOQ4LpJDGSkJnnMhwrOm4/EkJVcLUKnvtr6n/Ib87+ooYv7Kh\n\tda3QMEBw==;","Date":"Sat, 25 Oct 2025 10:06:42 +0100","From":"Umang Jain ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t, Kieran Bingham\n\t, =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t, Paul Elder\n\t","Subject":"Re: [PATCH v13 8/8] libcamera: simple: Make raw streams working","Message-ID":"<7crwp5hfbuacvp2ezazjwfzv4vgdzyseh2bdnao6wq4cukpz3z@zk76tzg47zyu>","References":"<20251021182716.29274-1-mzamazal@redhat.com>\n\t<20251021182716.29274-9-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20251021182716.29274-9-mzamazal@redhat.com>","User-Agent":"NeoMutt/20250905-dirty","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":36462,"web_url":"https://patchwork.libcamera.org/comment/36462/","msgid":"","date":"2025-10-25T11:41:40","subject":"Re: [PATCH v13 8/8] libcamera: simple: Make raw streams working","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"On Tue, Oct 21, 2025 at 08:27:15PM +0200, Milan Zamazal wrote:\n> When a raw stream is requested, whether alone or together with a\n> processed stream, its buffers must be handled outside the software ISP\n> machinery. They serve as output buffers, even when a processed stream\n> is produced.\n> \n> At most one raw stream and at most one processed stream are supported\n> and can be combined. An example of producing both raw and processed\n> files using `cam' application:\n> \n> cam -c1 -C100 -Ffile# \\\n> -s role=viewfinder,width=1920,height=1080,pixelformat=RGB888 \\\n> -s role=raw,width=3280,height=2464,pixelformat=SRGGB8 \\\n> \n> Note the difference in viewfinder and raw stream sizes due to the fact\n> that debayering requires enlarging the image width, which enforces\n> selecting a larger sensor resolution in this case.\n> \n> In order to track whether a raw stream is requested and which one it is,\n> SimpleCameraData::rawStream_ member variable is introduced.\n> \n> This is the final step to make raw streams working.\n> \n> Signed-off-by: Milan Zamazal \n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 50 +++++++++++++++++-------\n> 1 file changed, 36 insertions(+), 14 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 0c3284feb..607b07949 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -333,6 +333,7 @@ public:\n> \t};\n> \n> \tstd::vector streams_;\n> +\tStream *rawStream_;\n> \n> \t/*\n> \t * All entities in the pipeline, from the camera sensor to the video\n> @@ -371,6 +372,11 @@ private:\n> \tvoid ispStatsReady(uint32_t frame, uint32_t bufferId);\n> \tvoid metadataReady(uint32_t frame, const ControlList &metadata);\n> \tvoid setSensorControls(const ControlList &sensorControls);\n> +\n> +\tbool processedRequested() const\n> +\t{\n> +\t\treturn streams_.size() - (rawStream_ ? 1 : 0) > 0;\n> +\t}\n> };\n> \n> class SimpleCameraConfiguration : public CameraConfiguration\n> @@ -467,7 +473,7 @@ private:\n> SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n> \t\t\t\t unsigned int numStreams,\n> \t\t\t\t MediaEntity *sensor)\n> -\t: Camera::Private(pipe), streams_(numStreams)\n> +\t: Camera::Private(pipe), streams_(numStreams), rawStream_(nullptr)\n> {\n> \t/*\n> \t * Find the shortest path from the camera sensor to a video capture\n> @@ -878,10 +884,13 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> \t * point converting an erroneous buffer.\n> \t */\n> \tif (buffer->metadata().status != FrameMetadata::FrameSuccess) {\n> -\t\tif (!useConversion_) {\n> +\t\tif (rawStream_) {\n> \t\t\t/* No conversion, just complete the request. */\n> \t\t\tRequest *request = buffer->request();\n> \t\t\tpipe->completeBuffer(request, buffer);\n> +\t\t\tSimpleFrameInfo *info = frameInfo_.find(request->sequence());\n> +\t\t\tif (info)\n> +\t\t\t\tinfo->metadataRequired = false;\n> \t\t\ttryCompleteRequest(request);\n> \t\t\treturn;\n> \t\t}\n> @@ -891,7 +900,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> \t\t * buffer for capture (unless the stream is being stopped), and\n> \t\t * complete the request with all the user-facing buffers.\n> \t\t */\n> -\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled)\n> +\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled &&\n> +\t\t !rawStream_)\n> \t\t\tvideo_->queueBuffer(buffer);\n> \n> \t\tif (conversionQueue_.empty())\n> @@ -940,13 +950,14 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> \t */\n> \tif (useConversion_) {\n> \t\tif (conversionQueue_.empty()) {\n> -\t\t\tvideo_->queueBuffer(buffer);\n> +\t\t\tif (!rawStream_)\n> +\t\t\t\tvideo_->queueBuffer(buffer);\n> \t\t\treturn;\n> \t\t}\n> \n> \t\tif (converter_)\n> \t\t\tconverter_->queueBuffers(buffer, conversionQueue_.front().outputs);\n> -\t\telse\n> +\t\telse if (processedRequested())\n> \t\t\t/*\n> \t\t\t * request->sequence() cannot be retrieved from `buffer' inside\n> \t\t\t * queueBuffers because unique_ptr's make buffer->request() invalid\n> @@ -956,6 +967,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n> \t\t\t\t\t conversionQueue_.front().outputs);\n> \n> \t\tconversionQueue_.pop();\n> +\t\tif (rawStream_)\n> +\t\t\tpipe->completeBuffer(request, buffer);\n> \t\treturn;\n> \t}\n> \n> @@ -993,7 +1006,8 @@ void SimpleCameraData::tryCompleteRequest(Request *request)\n> void SimpleCameraData::conversionInputDone(FrameBuffer *buffer)\n> {\n> \t/* Queue the input buffer back for capture. */\n> -\tvideo_->queueBuffer(buffer);\n> +\tif (!rawStream_)\n> +\t\tvideo_->queueBuffer(buffer);\n> }\n> \n> void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n> @@ -1518,11 +1532,15 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> \n> \tfor (unsigned int i = 0; i < config->size(); ++i) {\n> \t\tStreamConfiguration &cfg = config->at(i);\n> +\t\tbool rawStream = isRaw(cfg);\n> \n> \t\tcfg.setStream(&data->streams_[i]);\n> \n> -\t\tif (data->useConversion_ && !isRaw(cfg))\n> +\t\tif (data->useConversion_ && !rawStream)\n> \t\t\toutputCfgs.push_back(cfg);\n> +\n> +\t\tif (rawStream)\n> +\t\t\tdata->rawStream_ = &data->streams_[i];\n\nLast comment, just realised:\n\ndata->rawStream_ should be initialised to nullptr (outside this loop)\neach time the camera is configured, otherwise it can lead to subtle bugs\nif the camera configure() is called multiple times with varying\nconfigurations.\n> \t}\n> \n> \tif (outputCfgs.empty())\n> @@ -1553,7 +1571,7 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n> \t * Export buffers on the converter or capture video node, depending on\n> \t * whether the converter is used or not.\n> \t */\n> -\tif (data->useConversion_)\n> +\tif (data->useConversion_ && stream != data->rawStream_)\n> \t\treturn data->converter_\n> \t\t\t ? data->converter_->exportBuffers(stream, count, buffers)\n> \t\t\t : data->swIsp_->exportBuffers(stream, count, buffers);\n> @@ -1576,7 +1594,7 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n> \t\treturn -EBUSY;\n> \t}\n> \n> -\tif (data->useConversion_) {\n> +\tif (data->useConversion_ && !data->rawStream_) {\n> \t\t/*\n> \t\t * When using the converter allocate a fixed number of internal\n> \t\t * buffers.\n> @@ -1584,8 +1602,11 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n> \t\tret = video->allocateBuffers(kNumInternalBuffers,\n> \t\t\t\t\t &data->conversionBuffers_);\n> \t} else {\n> -\t\t/* Otherwise, prepare for using buffers from the only stream. */\n> -\t\tStream *stream = &data->streams_[0];\n> +\t\t/*\n> +\t\t * Otherwise, prepare for using buffers from either the raw stream, if\n> +\t\t * requested, or the only stream configured.\n> +\t\t */\n> +\t\tStream *stream = (data->rawStream_ ? data->rawStream_ : &data->streams_[0]);\n> \t\tret = video->importBuffers(stream->configuration().bufferCount);\n> \t}\n> \tif (ret < 0) {\n> @@ -1626,8 +1647,9 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n> \t\t}\n> \n> \t\t/* Queue all internal buffers for capture. */\n> -\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n> -\t\t\tvideo->queueBuffer(buffer.get());\n> +\t\tif (!data->rawStream_)\n> +\t\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n> +\t\t\t\tvideo->queueBuffer(buffer.get());\n> \t}\n> \n> \treturn 0;\n> @@ -1678,7 +1700,7 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)\n> \t\t * queue, it will be handed to the converter in the capture\n> \t\t * completion handler.\n> \t\t */\n> -\t\tif (data->useConversion_) {\n> +\t\tif (data->useConversion_ && stream != data->rawStream_) {\n> \t\t\tbuffers.emplace(stream, buffer);\n> \t\t\tmetadataRequired = !!data->swIsp_;\n> \t\t} else {\n> -- \n> 2.51.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id BB7DBC3259\n\tfor ;\n\tSat, 25 Oct 2025 11:41:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0CB2B606A4;\n\tSat, 25 Oct 2025 13:41:15 +0200 (CEST)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 18E6760697\n\tfor ;\n\tSat, 25 Oct 2025 13:41:12 +0200 (CEST)","from amazon1-vf-gw.lnd.cw.net ([195.89.103.118] 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 1vCcdz-00F5pL-DS; Sat, 25 Oct 2025 13:41:11 +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=\"Ry//vv3c\"; 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=9yCaaqRyOdVukH3LJlgtGGy9Wh3VkVnAtmF1YFkXHSc=;\n\tb=Ry//vv3cY0tW58XeN6OjbNcrkP\n\t6eJnD77IAtL1CtGPTTrzBXjDAIR4xgBzX0yChQ3ThWNeOjsLQVqtZjpy91VZLA2ab32F0D8akzi4j\n\tm6prguEZ6rPeRrlwNsDAqhdghF7Cu2uWi1aX5qWFBFwYd+O1+omrgcQid2vsJrD88Or9mPsIpc5Ue\n\tj+cjZ0SbQhPGjkMiG+DGNzDR9ZMQXzzHf6GxmXYr1M537w/vjIB0yIX62kteX8vfidj5F+OH2P0U2\n\tNe+ShByBr8SjLlcocLSeqW/IkBoBgtYtv/hZS/uvEdZmCxMI9wqrjwaqEafgBGYRBiCd0KbNS54w9\n\to92S1epw==;","Date":"Sat, 25 Oct 2025 12:41:40 +0100","From":"Umang Jain ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t, Kieran Bingham\n\t, =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t, Paul Elder\n\t","Subject":"Re: [PATCH v13 8/8] libcamera: simple: Make raw streams working","Message-ID":"","References":"<20251021182716.29274-1-mzamazal@redhat.com>\n\t<20251021182716.29274-9-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20251021182716.29274-9-mzamazal@redhat.com>","User-Agent":"NeoMutt/20250905-dirty","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":36601,"web_url":"https://patchwork.libcamera.org/comment/36601/","msgid":"<855xbttuup.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-11-01T19:29:18","subject":"Re: [PATCH v13 8/8] libcamera: simple: Make raw streams working","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Umang,\n\nUmang Jain writes:\n\n> On Tue, Oct 21, 2025 at 08:27:15PM +0200, Milan Zamazal wrote:\n>> When a raw stream is requested, whether alone or together with a\n>> processed stream, its buffers must be handled outside the software ISP\n>\n>> machinery. They serve as output buffers, even when a processed stream\n>> is produced.\n>> \n>> At most one raw stream and at most one processed stream are supported\n>> and can be combined. An example of producing both raw and processed\n>> files using `cam' application:\n>> \n>> cam -c1 -C100 -Ffile# \\\n>> -s role=viewfinder,width=1920,height=1080,pixelformat=RGB888 \\\n>> -s role=raw,width=3280,height=2464,pixelformat=SRGGB8 \\\n>> \n>> Note the difference in viewfinder and raw stream sizes due to the fact\n>> that debayering requires enlarging the image width, which enforces\n>> selecting a larger sensor resolution in this case.\n>> \n>> In order to track whether a raw stream is requested and which one it is,\n>> SimpleCameraData::rawStream_ member variable is introduced.\n>> \n>> This is the final step to make raw streams working.\n>> \n>> Signed-off-by: Milan Zamazal \n>> ---\n>> src/libcamera/pipeline/simple/simple.cpp | 50 +++++++++++++++++-------\n>> 1 file changed, 36 insertions(+), 14 deletions(-)\n>> \n>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>> index 0c3284feb..607b07949 100644\n>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>> @@ -333,6 +333,7 @@ public:\n>> \t};\n>> \n>> \tstd::vector streams_;\n>> +\tStream *rawStream_;\n>> \n>> \t/*\n>> \t * All entities in the pipeline, from the camera sensor to the video\n>> @@ -371,6 +372,11 @@ private:\n>> \tvoid ispStatsReady(uint32_t frame, uint32_t bufferId);\n>> \tvoid metadataReady(uint32_t frame, const ControlList &metadata);\n>> \tvoid setSensorControls(const ControlList &sensorControls);\n>> +\n>> +\tbool processedRequested() const\n>> +\t{\n>> +\t\treturn streams_.size() - (rawStream_ ? 1 : 0) > 0;\n>> +\t}\n>> };\n>> \n>> class SimpleCameraConfiguration : public CameraConfiguration\n>> @@ -467,7 +473,7 @@ private:\n>> SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n>> \t\t\t\t unsigned int numStreams,\n>> \t\t\t\t MediaEntity *sensor)\n>> -\t: Camera::Private(pipe), streams_(numStreams)\n>> +\t: Camera::Private(pipe), streams_(numStreams), rawStream_(nullptr)\n>> {\n>> \t/*\n>> \t * Find the shortest path from the camera sensor to a video capture\n>> @@ -878,10 +884,13 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>> \t * point converting an erroneous buffer.\n>> \t */\n>> \tif (buffer->metadata().status != FrameMetadata::FrameSuccess) {\n>> -\t\tif (!useConversion_) {\n>> +\t\tif (rawStream_) {\n>\n> [1]\n>\n> I really don't understand the change here. \n\nThe idea is that when a raw stream is requested then the buffers must be\nhandled basically as without software ISP because they serve as output\nbuffers (unlike with a pure software ISP case).\n\n> Also doesn't it break the case when only one processed stream is\n> requested and an error occured ?\n\nIt's the same as before in such a case -- the condition is false.\n\n>> \t\t\t/* No conversion, just complete the request. */\n>> \t\t\tRequest *request = buffer->request();\n>> \t\t\tpipe->completeBuffer(request, buffer);\n>> +\t\t\tSimpleFrameInfo *info = frameInfo_.find(request->sequence());\n>> +\t\t\tif (info)\n>> +\t\t\t\tinfo->metadataRequired = false;\n>\n> If this change is trying to adapt the if condition in\n> tryCompleteRequest(), I believe this should be a different\n> patch. For now, I don't see how it relates to raw stream enabling.\n\nI don't understand what you mean exactly here. The purpose of this\nchange is that now, when a processed stream may be available in addition\nto a raw stream, we must ensure that we won't hang waiting for metadata\nwhen an error occurs.\n\n>> \t\t\ttryCompleteRequest(request);\n>> \t\t\treturn;\n>> \t\t}\n>> @@ -891,7 +900,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>> \t\t * buffer for capture (unless the stream is being stopped), and\n>> \t\t * complete the request with all the user-facing buffers.\n>> \t\t */\n>> -\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled)\n>> +\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled &&\n>> +\t\t !rawStream_)\n>\n> Do you need to check for !rawStream_ again? The codepath would have\n> returned early already if I consider the change above at [1], no? \n\nRight, I'll remove this.\n\n>> \t\t\tvideo_->queueBuffer(buffer);\n>> \n>> \t\tif (conversionQueue_.empty())\n>> @@ -940,13 +950,14 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>> \t */\n>> \tif (useConversion_) {\n>> \t\tif (conversionQueue_.empty()) {\n>> -\t\t\tvideo_->queueBuffer(buffer);\n>> +\t\t\tif (!rawStream_)\n>> +\t\t\t\tvideo_->queueBuffer(buffer);\n>> \t\t\treturn;\n>> \t\t}\n>> \n>> \t\tif (converter_)\n>> \t\t\tconverter_->queueBuffers(buffer, conversionQueue_.front().outputs);\n>> -\t\telse\n>> +\t\telse if (processedRequested())\n>\n> Isn't this implicit, if useConversion_=True, you either go through\n> converter_ or softISP. We don't need to check for processedRequested()\n> here.\n\nRight, this condition is unnecessary, as well as processedRequested\nmethod then.\n\n>> \t\t\t/*\n>> \t\t\t * request->sequence() cannot be retrieved from `buffer' inside\n>> \t\t\t * queueBuffers because unique_ptr's make buffer->request() invalid\n>> @@ -956,6 +967,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>> \t\t\t\t\t conversionQueue_.front().outputs);\n>> \n>> \t\tconversionQueue_.pop();\n>> +\t\tif (rawStream_)\n>> +\t\t\tpipe->completeBuffer(request, buffer);\n>\n> This shouldn't be needed either, the function has pipe->completeBuffer()\n> out of this if block.\n\nBut this won't be called because of the return statement at the\nfollowing line:\n\n>> \t\treturn;\n>> \t}\n>> \n>> @@ -993,7 +1006,8 @@ void SimpleCameraData::tryCompleteRequest(Request *request)\n>> void SimpleCameraData::conversionInputDone(FrameBuffer *buffer)\n>> {\n>> \t/* Queue the input buffer back for capture. */\n>> -\tvideo_->queueBuffer(buffer);\n>> +\tif (!rawStream_)\n>> +\t\tvideo_->queueBuffer(buffer);\n>\n> Don't you need to also complete the raw buffer in else block ?\n> This would be the case where only raw stream is requested. Or is it done\n> elsewhere?\n\nIf only a raw stream is requested then buffers should be handled the\nsame way as before this patch. This method is not called in such a\ncase.\n\n>> }\n>> \n>> void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n>> @@ -1518,11 +1532,15 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n>> \n>> \tfor (unsigned int i = 0; i < config->size(); ++i) {\n>> \t\tStreamConfiguration &cfg = config->at(i);\n>> +\t\tbool rawStream = isRaw(cfg);\n>> \n>> \t\tcfg.setStream(&data->streams_[i]);\n>> \n>> -\t\tif (data->useConversion_ && !isRaw(cfg))\n>> +\t\tif (data->useConversion_ && !rawStream)\n>> \t\t\toutputCfgs.push_back(cfg);\n>> +\n>> +\t\tif (rawStream)\n>> +\t\t\tdata->rawStream_ = &data->streams_[i];\n>> \t}\n>> \n>> \tif (outputCfgs.empty())\n>> @@ -1553,7 +1571,7 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n>> \t * Export buffers on the converter or capture video node, depending on\n>> \t * whether the converter is used or not.\n>> \t */\n>> -\tif (data->useConversion_)\n>> +\tif (data->useConversion_ && stream != data->rawStream_)\n>> \t\treturn data->converter_\n>> \t\t\t ? data->converter_->exportBuffers(stream, count, buffers)\n>> \t\t\t : data->swIsp_->exportBuffers(stream, count, buffers);\n>> @@ -1576,7 +1594,7 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>> \t\treturn -EBUSY;\n>> \t}\n>> \n>> -\tif (data->useConversion_) {\n>> +\tif (data->useConversion_ && !data->rawStream_) {\n>> \t\t/*\n>> \t\t * When using the converter allocate a fixed number of internal\n>> \t\t * buffers.\n>> @@ -1584,8 +1602,11 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>> \t\tret = video->allocateBuffers(kNumInternalBuffers,\n>> \t\t\t\t\t &data->conversionBuffers_);\n>> \t} else {\n>> -\t\t/* Otherwise, prepare for using buffers from the only stream. */\n>> -\t\tStream *stream = &data->streams_[0];\n>> +\t\t/*\n>> +\t\t * Otherwise, prepare for using buffers from either the raw stream, if\n>> +\t\t * requested, or the only stream configured.\n>> +\t\t */\n>> +\t\tStream *stream = (data->rawStream_ ? data->rawStream_ : &data->streams_[0]);\n>> \t\tret = video->importBuffers(stream->configuration().bufferCount);\n>> \t}\n>> \tif (ret < 0) {\n>> @@ -1626,8 +1647,9 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>> \t\t}\n>> \n>> \t\t/* Queue all internal buffers for capture. */\n>> -\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n>> -\t\t\tvideo->queueBuffer(buffer.get());\n>> +\t\tif (!data->rawStream_)\n>> +\t\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n>> +\t\t\t\tvideo->queueBuffer(buffer.get());\n>> \t}\n>> \n>> \treturn 0;\n>> @@ -1678,7 +1700,7 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)\n>> \t\t * queue, it will be handed to the converter in the capture\n>> \t\t * completion handler.\n>> \t\t */\n>> -\t\tif (data->useConversion_) {\n>> +\t\tif (data->useConversion_ && stream != data->rawStream_) {\n>> \t\t\tbuffers.emplace(stream, buffer);\n>> \t\t\tmetadataRequired = !!data->swIsp_;\n>> \t\t} else {\n>> -- \n>> 2.51.0\n>> \n\nUmang Jain writes:\n\n> I reviewed most of the patches and am OK with them. I prefer a slight\n> rework in 4/8 and have highlighted it (with a reference diff I\n> implemented earlier).\n\nI'll change 4/8 to be close to your implementation, with some small\ncorrections. \n\n> Unfortuantely, I did find patch 8/8 hard-to-follow :(\n\nI'm not sure what to do to make it easier, maybe the comments above help\na bit?\n\n> But if it's just me, it would be better if someone else should take a\n> look.\n>\n>\n> On Tue, Oct 21, 2025 at 08:27:15PM +0200, Milan Zamazal wrote:\n>> When a raw stream is requested, whether alone or together with a\n>> processed stream, its buffers must be handled outside the software ISP\n>> machinery. They serve as output buffers, even when a processed stream\n>> is produced.\n>> \n>> At most one raw stream and at most one processed stream are supported\n>> and can be combined. An example of producing both raw and processed\n>> files using `cam' application:\n>> \n>> cam -c1 -C100 -Ffile# \\\n>> -s role=viewfinder,width=1920,height=1080,pixelformat=RGB888 \\\n>> -s role=raw,width=3280,height=2464,pixelformat=SRGGB8 \\\n>> \n>> Note the difference in viewfinder and raw stream sizes due to the fact\n>> that debayering requires enlarging the image width, which enforces\n>> selecting a larger sensor resolution in this case.\n>> \n>> In order to track whether a raw stream is requested and which one it is,\n>> SimpleCameraData::rawStream_ member variable is introduced.\n>> \n>> This is the final step to make raw streams working.\n>> \n>> Signed-off-by: Milan Zamazal \n>> ---\n>> src/libcamera/pipeline/simple/simple.cpp | 50 +++++++++++++++++-------\n>> 1 file changed, 36 insertions(+), 14 deletions(-)\n>> \n>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>> index 0c3284feb..607b07949 100644\n>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>> @@ -333,6 +333,7 @@ public:\n>> \t};\n>> \n>> \tstd::vector streams_;\n>> +\tStream *rawStream_;\n>> \n>> \t/*\n>> \t * All entities in the pipeline, from the camera sensor to the video\n>> @@ -371,6 +372,11 @@ private:\n>> \tvoid ispStatsReady(uint32_t frame, uint32_t bufferId);\n>> \tvoid metadataReady(uint32_t frame, const ControlList &metadata);\n>> \tvoid setSensorControls(const ControlList &sensorControls);\n>> +\n>> +\tbool processedRequested() const\n>> +\t{\n>> +\t\treturn streams_.size() - (rawStream_ ? 1 : 0) > 0;\n>> +\t}\n>> };\n>> \n>> class SimpleCameraConfiguration : public CameraConfiguration\n>> @@ -467,7 +473,7 @@ private:\n>> SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n>> \t\t\t\t unsigned int numStreams,\n>> \t\t\t\t MediaEntity *sensor)\n>> -\t: Camera::Private(pipe), streams_(numStreams)\n>> +\t: Camera::Private(pipe), streams_(numStreams), rawStream_(nullptr)\n>> {\n>> \t/*\n>> \t * Find the shortest path from the camera sensor to a video capture\n>> @@ -878,10 +884,13 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>> \t * point converting an erroneous buffer.\n>> \t */\n>> \tif (buffer->metadata().status != FrameMetadata::FrameSuccess) {\n>> -\t\tif (!useConversion_) {\n>> +\t\tif (rawStream_) {\n>> \t\t\t/* No conversion, just complete the request. */\n>> \t\t\tRequest *request = buffer->request();\n>> \t\t\tpipe->completeBuffer(request, buffer);\n>> +\t\t\tSimpleFrameInfo *info = frameInfo_.find(request->sequence());\n>> +\t\t\tif (info)\n>> +\t\t\t\tinfo->metadataRequired = false;\n>> \t\t\ttryCompleteRequest(request);\n>> \t\t\treturn;\n>> \t\t}\n>> @@ -891,7 +900,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>> \t\t * buffer for capture (unless the stream is being stopped), and\n>> \t\t * complete the request with all the user-facing buffers.\n>> \t\t */\n>> -\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled)\n>> +\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled &&\n>> +\t\t !rawStream_)\n>> \t\t\tvideo_->queueBuffer(buffer);\n>> \n>> \t\tif (conversionQueue_.empty())\n>> @@ -940,13 +950,14 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>> \t */\n>> \tif (useConversion_) {\n>> \t\tif (conversionQueue_.empty()) {\n>> -\t\t\tvideo_->queueBuffer(buffer);\n>> +\t\t\tif (!rawStream_)\n>> +\t\t\t\tvideo_->queueBuffer(buffer);\n>> \t\t\treturn;\n>> \t\t}\n>> \n>> \t\tif (converter_)\n>> \t\t\tconverter_->queueBuffers(buffer, conversionQueue_.front().outputs);\n>> -\t\telse\n>> +\t\telse if (processedRequested())\n>> \t\t\t/*\n>> \t\t\t * request->sequence() cannot be retrieved from `buffer' inside\n>> \t\t\t * queueBuffers because unique_ptr's make buffer->request() invalid\n>> @@ -956,6 +967,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>> \t\t\t\t\t conversionQueue_.front().outputs);\n>> \n>> \t\tconversionQueue_.pop();\n>> +\t\tif (rawStream_)\n>> +\t\t\tpipe->completeBuffer(request, buffer);\n>> \t\treturn;\n>> \t}\n>> \n>> @@ -993,7 +1006,8 @@ void SimpleCameraData::tryCompleteRequest(Request *request)\n>> void SimpleCameraData::conversionInputDone(FrameBuffer *buffer)\n>> {\n>> \t/* Queue the input buffer back for capture. */\n>> -\tvideo_->queueBuffer(buffer);\n>> +\tif (!rawStream_)\n>> +\t\tvideo_->queueBuffer(buffer);\n>> }\n>> \n>> void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n>> @@ -1518,11 +1532,15 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n>> \n>> \tfor (unsigned int i = 0; i < config->size(); ++i) {\n>> \t\tStreamConfiguration &cfg = config->at(i);\n>> +\t\tbool rawStream = isRaw(cfg);\n>> \n>> \t\tcfg.setStream(&data->streams_[i]);\n>> \n>> -\t\tif (data->useConversion_ && !isRaw(cfg))\n>> +\t\tif (data->useConversion_ && !rawStream)\n>> \t\t\toutputCfgs.push_back(cfg);\n>> +\n>> +\t\tif (rawStream)\n>> +\t\t\tdata->rawStream_ = &data->streams_[i];\n>> \t}\n>> \n>> \tif (outputCfgs.empty())\n>> @@ -1553,7 +1571,7 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n>> \t * Export buffers on the converter or capture video node, depending on\n>> \t * whether the converter is used or not.\n>> \t */\n>> -\tif (data->useConversion_)\n>> +\tif (data->useConversion_ && stream != data->rawStream_)\n>> \t\treturn data->converter_\n>> \t\t\t ? data->converter_->exportBuffers(stream, count, buffers)\n>> \t\t\t : data->swIsp_->exportBuffers(stream, count, buffers);\n>> @@ -1576,7 +1594,7 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>> \t\treturn -EBUSY;\n>> \t}\n>> \n>> -\tif (data->useConversion_) {\n>> +\tif (data->useConversion_ && !data->rawStream_) {\n>> \t\t/*\n>> \t\t * When using the converter allocate a fixed number of internal\n>> \t\t * buffers.\n>> @@ -1584,8 +1602,11 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>> \t\tret = video->allocateBuffers(kNumInternalBuffers,\n>> \t\t\t\t\t &data->conversionBuffers_);\n>> \t} else {\n>> -\t\t/* Otherwise, prepare for using buffers from the only stream. */\n>> -\t\tStream *stream = &data->streams_[0];\n>> +\t\t/*\n>> +\t\t * Otherwise, prepare for using buffers from either the raw stream, if\n>> +\t\t * requested, or the only stream configured.\n>> +\t\t */\n>> +\t\tStream *stream = (data->rawStream_ ? data->rawStream_ : &data->streams_[0]);\n>> \t\tret = video->importBuffers(stream->configuration().bufferCount);\n>> \t}\n>> \tif (ret < 0) {\n>> @@ -1626,8 +1647,9 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>> \t\t}\n>> \n>> \t\t/* Queue all internal buffers for capture. */\n>> -\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n>> -\t\t\tvideo->queueBuffer(buffer.get());\n>> +\t\tif (!data->rawStream_)\n>> +\t\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n>> +\t\t\t\tvideo->queueBuffer(buffer.get());\n>> \t}\n>> \n>> \treturn 0;\n>> @@ -1678,7 +1700,7 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)\n>> \t\t * queue, it will be handed to the converter in the capture\n>> \t\t * completion handler.\n>> \t\t */\n>> -\t\tif (data->useConversion_) {\n>> +\t\tif (data->useConversion_ && stream != data->rawStream_) {\n>> \t\t\tbuffers.emplace(stream, buffer);\n>> \t\t\tmetadataRequired = !!data->swIsp_;\n>> \t\t} else {\n>> -- \n>> 2.51.0\n>> \n\nUmang Jain writes:\n\n> On Tue, Oct 21, 2025 at 08:27:15PM +0200, Milan Zamazal wrote:\n>> When a raw stream is requested, whether alone or together with a\n>> processed stream, its buffers must be handled outside the software ISP\n>\n>> machinery. They serve as output buffers, even when a processed stream\n>> is produced.\n>> \n>> At most one raw stream and at most one processed stream are supported\n>> and can be combined. An example of producing both raw and processed\n>> files using `cam' application:\n>> \n>> cam -c1 -C100 -Ffile# \\\n>> -s role=viewfinder,width=1920,height=1080,pixelformat=RGB888 \\\n>> -s role=raw,width=3280,height=2464,pixelformat=SRGGB8 \\\n>> \n>> Note the difference in viewfinder and raw stream sizes due to the fact\n>> that debayering requires enlarging the image width, which enforces\n>> selecting a larger sensor resolution in this case.\n>> \n>> In order to track whether a raw stream is requested and which one it is,\n>> SimpleCameraData::rawStream_ member variable is introduced.\n>> \n>> This is the final step to make raw streams working.\n>> \n>> Signed-off-by: Milan Zamazal \n>> ---\n>> src/libcamera/pipeline/simple/simple.cpp | 50 +++++++++++++++++-------\n>> 1 file changed, 36 insertions(+), 14 deletions(-)\n>> \n>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>> index 0c3284feb..607b07949 100644\n>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>> @@ -333,6 +333,7 @@ public:\n>> \t};\n>> \n>> \tstd::vector streams_;\n>> +\tStream *rawStream_;\n>> \n>> \t/*\n>> \t * All entities in the pipeline, from the camera sensor to the video\n>> @@ -371,6 +372,11 @@ private:\n>> \tvoid ispStatsReady(uint32_t frame, uint32_t bufferId);\n>> \tvoid metadataReady(uint32_t frame, const ControlList &metadata);\n>> \tvoid setSensorControls(const ControlList &sensorControls);\n>> +\n>> +\tbool processedRequested() const\n>> +\t{\n>> +\t\treturn streams_.size() - (rawStream_ ? 1 : 0) > 0;\n>> +\t}\n>> };\n>> \n>> class SimpleCameraConfiguration : public CameraConfiguration\n>> @@ -467,7 +473,7 @@ private:\n>> SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n>> \t\t\t\t unsigned int numStreams,\n>> \t\t\t\t MediaEntity *sensor)\n>> -\t: Camera::Private(pipe), streams_(numStreams)\n>> +\t: Camera::Private(pipe), streams_(numStreams), rawStream_(nullptr)\n>> {\n>> \t/*\n>> \t * Find the shortest path from the camera sensor to a video capture\n>> @@ -878,10 +884,13 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>> \t * point converting an erroneous buffer.\n>> \t */\n>> \tif (buffer->metadata().status != FrameMetadata::FrameSuccess) {\n>> -\t\tif (!useConversion_) {\n>> +\t\tif (rawStream_) {\n>> \t\t\t/* No conversion, just complete the request. */\n>> \t\t\tRequest *request = buffer->request();\n>> \t\t\tpipe->completeBuffer(request, buffer);\n>> +\t\t\tSimpleFrameInfo *info = frameInfo_.find(request->sequence());\n>> +\t\t\tif (info)\n>> +\t\t\t\tinfo->metadataRequired = false;\n>> \t\t\ttryCompleteRequest(request);\n>> \t\t\treturn;\n>> \t\t}\n>> @@ -891,7 +900,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>> \t\t * buffer for capture (unless the stream is being stopped), and\n>> \t\t * complete the request with all the user-facing buffers.\n>> \t\t */\n>> -\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled)\n>> +\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled &&\n>> +\t\t !rawStream_)\n>> \t\t\tvideo_->queueBuffer(buffer);\n>> \n>> \t\tif (conversionQueue_.empty())\n>> @@ -940,13 +950,14 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>> \t */\n>> \tif (useConversion_) {\n>> \t\tif (conversionQueue_.empty()) {\n>> -\t\t\tvideo_->queueBuffer(buffer);\n>> +\t\t\tif (!rawStream_)\n>> +\t\t\t\tvideo_->queueBuffer(buffer);\n>> \t\t\treturn;\n>> \t\t}\n>> \n>> \t\tif (converter_)\n>> \t\t\tconverter_->queueBuffers(buffer, conversionQueue_.front().outputs);\n>> -\t\telse\n>> +\t\telse if (processedRequested())\n>> \t\t\t/*\n>> \t\t\t * request->sequence() cannot be retrieved from `buffer' inside\n>> \t\t\t * queueBuffers because unique_ptr's make buffer->request() invalid\n>> @@ -956,6 +967,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>> \t\t\t\t\t conversionQueue_.front().outputs);\n>> \n>> \t\tconversionQueue_.pop();\n>> +\t\tif (rawStream_)\n>> +\t\t\tpipe->completeBuffer(request, buffer);\n>> \t\treturn;\n>> \t}\n>> \n>> @@ -993,7 +1006,8 @@ void SimpleCameraData::tryCompleteRequest(Request *request)\n>> void SimpleCameraData::conversionInputDone(FrameBuffer *buffer)\n>> {\n>> \t/* Queue the input buffer back for capture. */\n>> -\tvideo_->queueBuffer(buffer);\n>> +\tif (!rawStream_)\n>> +\t\tvideo_->queueBuffer(buffer);\n>> }\n>> \n>> void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n>> @@ -1518,11 +1532,15 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n>> \n>> \tfor (unsigned int i = 0; i < config->size(); ++i) {\n>> \t\tStreamConfiguration &cfg = config->at(i);\n>> +\t\tbool rawStream = isRaw(cfg);\n>> \n>> \t\tcfg.setStream(&data->streams_[i]);\n>> \n>> -\t\tif (data->useConversion_ && !isRaw(cfg))\n>> +\t\tif (data->useConversion_ && !rawStream)\n>> \t\t\toutputCfgs.push_back(cfg);\n>> +\n>> +\t\tif (rawStream)\n>> +\t\t\tdata->rawStream_ = &data->streams_[i];\n>\n> Last comment, just realised:\n>\n> data->rawStream_ should be initialised to nullptr (outside this loop)\n> each time the camera is configured, otherwise it can lead to subtle bugs\n> if the camera configure() is called multiple times with varying\n> configurations.\n\nRight, I'll add it.\n\n>> \t}\n>> \n>> \tif (outputCfgs.empty())\n>> @@ -1553,7 +1571,7 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n>> \t * Export buffers on the converter or capture video node, depending on\n>> \t * whether the converter is used or not.\n>> \t */\n>> -\tif (data->useConversion_)\n>> +\tif (data->useConversion_ && stream != data->rawStream_)\n>> \t\treturn data->converter_\n>> \t\t\t ? data->converter_->exportBuffers(stream, count, buffers)\n>> \t\t\t : data->swIsp_->exportBuffers(stream, count, buffers);\n>> @@ -1576,7 +1594,7 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>> \t\treturn -EBUSY;\n>> \t}\n>> \n>> -\tif (data->useConversion_) {\n>> +\tif (data->useConversion_ && !data->rawStream_) {\n>> \t\t/*\n>> \t\t * When using the converter allocate a fixed number of internal\n>> \t\t * buffers.\n>> @@ -1584,8 +1602,11 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>> \t\tret = video->allocateBuffers(kNumInternalBuffers,\n>> \t\t\t\t\t &data->conversionBuffers_);\n>> \t} else {\n>> -\t\t/* Otherwise, prepare for using buffers from the only stream. */\n>> -\t\tStream *stream = &data->streams_[0];\n>> +\t\t/*\n>> +\t\t * Otherwise, prepare for using buffers from either the raw stream, if\n>> +\t\t * requested, or the only stream configured.\n>> +\t\t */\n>> +\t\tStream *stream = (data->rawStream_ ? data->rawStream_ : &data->streams_[0]);\n>> \t\tret = video->importBuffers(stream->configuration().bufferCount);\n>> \t}\n>> \tif (ret < 0) {\n>> @@ -1626,8 +1647,9 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>> \t\t}\n>> \n>> \t\t/* Queue all internal buffers for capture. */\n>> -\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n>> -\t\t\tvideo->queueBuffer(buffer.get());\n>> +\t\tif (!data->rawStream_)\n>> +\t\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n>> +\t\t\t\tvideo->queueBuffer(buffer.get());\n>> \t}\n>> \n>> \treturn 0;\n>> @@ -1678,7 +1700,7 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)\n>> \t\t * queue, it will be handed to the converter in the capture\n>> \t\t * completion handler.\n>> \t\t */\n>> -\t\tif (data->useConversion_) {\n>> +\t\tif (data->useConversion_ && stream != data->rawStream_) {\n>> \t\t\tbuffers.emplace(stream, buffer);\n>> \t\t\tmetadataRequired = !!data->swIsp_;\n>> \t\t} else {\n>> -- \n>> 2.51.0\n>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 066D5C3241\n\tfor ;\n\tSat, 1 Nov 2025 19:33:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 66EF06086F;\n\tSat, 1 Nov 2025 20:33:01 +0100 (CET)","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 7EF6D606D5\n\tfor ;\n\tSat, 1 Nov 2025 20:32:59 +0100 (CET)","from mail-wm1-f71.google.com (mail-wm1-f71.google.com\n\t[209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-612-w4_WSVzXMYW6zF1thB79cQ-1; Sat, 01 Nov 2025 15:29:32 -0400","by mail-wm1-f71.google.com with SMTP id\n\t5b1f17b1804b1-475dc22db7eso38990685e9.2\n\tfor ;\n\tSat, 01 Nov 2025 12:29:22 -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-429c13edc36sm10688068f8f.37.2025.11.01.12.29.18\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 01 Nov 2025 12:29:19 -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=\"KEC3W/u2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1762025578;\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=DqyYRXW0aeNiyJottZkTBXLv5RrQcDtj8ijrfyoCv0c=;\n\tb=KEC3W/u2oLyOZB9UJMeeVeGwdXBnvgTfNZLpQPsLmYbZUHMuQytikikH0hfUA9E8fgebeU\n\tMytkuCv8iVMC0FvtDh7mpiqH9WmVdfroHOsW/rCoyof0gA6kU2wzYcBNSMutoLssplicXe\n\tEIJhEBMy/GUkZEyPZ/H7HYWfoulMtQI=","X-MC-Unique":"w4_WSVzXMYW6zF1thB79cQ-1","X-Mimecast-MFC-AGG-ID":"w4_WSVzXMYW6zF1thB79cQ_1762025361","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1762025361; x=1762630161;\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=DqyYRXW0aeNiyJottZkTBXLv5RrQcDtj8ijrfyoCv0c=;\n\tb=YOp9FXIpTRnlSUBVx6xjJYeEUBVxMr5L/FVmwa88gdTYSJ7PzmUf3qU7wkF0HSi2dk\n\tRRWTqgpmEZVDzpPMNuSyRY9dM3HtAEBNSA5XbLM0aS64l7wof09Z0lKD/7Oks5omYU2U\n\thHnje269TyLRMISefECwNqwRcKaYVjCHDi7MdlZHKW79geeEaeiCKaWjkvl7qViVrwB9\n\tDYIVBHfPq+HWRLhFD/vXI8vBk+Jl53D4MOGSfBW4p/I+/O5IVNP6LMUrAWgYNkKvXDhu\n\tcr+Z1CknJtJlQnsUuUMkvXqffn9j8ihxccLndXPSYiX6PnARV4raG/DPXTpPD6+zVx/8\n\tIU0g==","X-Gm-Message-State":"AOJu0YwpmUwFdRZiNH65CNycWGA/FutJhPrE/JpIo7ezFHqyYFB5iKLi\n\t2ZM/kstTPKi8cFaNRXh4teK61C8GnqEsF7FOS9B6oox9vZSIEdqbn30kIs0ypwyxdE17jxSRldQ\n\tWLb2lfWiaSQfb34Oeoduk76rIWBhXnrNij3VdYVuIJgwhfNBNDFw2A+PAVpknrRC8ASV62UjvtI\n\ts=","X-Gm-Gg":"ASbGnctRVBqf3T35aO4CoCqy+jPhDPlKdstPG454He16E29oKYu1GaJqR7ogkmD7Dr4\n\t/ImDy9qDxcoUFxOyz5KM25jw6vzeW604tyd/hp/xhR7gu1TpXVnJiElGgA5TEBoAPv8+lEfBK4C\n\tbh6ZYbhRu9wjtEMT/I814ziGBgIN4v+5YNvIeA05NyNjo8SoozArx2RLljWB+MxlHVe4/mG0PyQ\n\tA3MEKWRr5w8S2haquYeCvNambqgPJ0IdBPbTXPbWU6kaOf7W6DOFdGi6VWgwrK/8XA4xiq/xHK5\n\tPYx/Bm2DduLA8K13Mjo39XXjLXlMZTPAn6YTQ4KsBPWL4sP/Ra6JHnPsAI6PgY3rYXM4z1vgoEU\n\tef2t1fauBhma97iwMrpRFsagrZmrYSBOo//e0wzL0o8cfr9drZenF","X-Received":["by 2002:a5d:584b:0:b0:3ec:2529:b4e5 with SMTP id\n\tffacd0b85a97d-429bd6aa4e9mr7832940f8f.38.1762025360858; \n\tSat, 01 Nov 2025 12:29:20 -0700 (PDT)","by 2002:a5d:584b:0:b0:3ec:2529:b4e5 with SMTP id\n\tffacd0b85a97d-429bd6aa4e9mr7832922f8f.38.1762025360258; \n\tSat, 01 Nov 2025 12:29:20 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IG3E7Q/rUfbo3xt1c3E+ZSZw5oPkQENCv11oCEHig8uU3CZp4PFV9mdjEd4fiEvBc+NAbXVWA==","From":"Milan Zamazal ","To":"Umang Jain ","Cc":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t,\n\tKieran Bingham , =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?=\n\t, Paul Elder\n\t","Subject":"Re: [PATCH v13 8/8] libcamera: simple: Make raw streams working","In-Reply-To":"\n\t(Umang Jain's message of \"Sat, 25 Oct 2025 10:00:39 +0100\")","References":"<20251021182716.29274-1-mzamazal@redhat.com>\n\t<20251021182716.29274-9-mzamazal@redhat.com>\n\t","Date":"Sat, 01 Nov 2025 20:29:18 +0100","Message-ID":"<855xbttuup.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":"aDt6Ju05J_-JOMejVTvydqV7BRHrptib8zF_V_16LnU_1762025361","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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":36670,"web_url":"https://patchwork.libcamera.org/comment/36670/","msgid":"","date":"2025-11-03T17:57:49","subject":"Re: [PATCH v13 8/8] libcamera: simple: Make raw streams working","submitter":{"id":232,"url":"https://patchwork.libcamera.org/api/people/232/","name":"Umang Jain","email":"uajain@igalia.com"},"content":"On 2025-11-01 19:29, Milan Zamazal wrote:\n> Hi Umang,\n> \n> Umang Jain writes:\n> \n>> On Tue, Oct 21, 2025 at 08:27:15PM +0200, Milan Zamazal wrote:\n>>> When a raw stream is requested, whether alone or together with a\n>>> processed stream, its buffers must be handled outside the software ISP\n>>\n>>> machinery. They serve as output buffers, even when a processed stream\n>>> is produced.\n>>> \n>>> At most one raw stream and at most one processed stream are supported\n>>> and can be combined. An example of producing both raw and processed\n>>> files using `cam' application:\n>>> \n>>> cam -c1 -C100 -Ffile# \\\n>>> -s role=viewfinder,width=1920,height=1080,pixelformat=RGB888 \\\n>>> -s role=raw,width=3280,height=2464,pixelformat=SRGGB8 \\\n>>> \n>>> Note the difference in viewfinder and raw stream sizes due to the fact\n>>> that debayering requires enlarging the image width, which enforces\n>>> selecting a larger sensor resolution in this case.\n>>> \n>>> In order to track whether a raw stream is requested and which one it is,\n>>> SimpleCameraData::rawStream_ member variable is introduced.\n>>> \n>>> This is the final step to make raw streams working.\n>>> \n>>> Signed-off-by: Milan Zamazal \n>>> ---\n>>> src/libcamera/pipeline/simple/simple.cpp | 50 +++++++++++++++++-------\n>>> 1 file changed, 36 insertions(+), 14 deletions(-)\n>>> \n>>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>>> index 0c3284feb..607b07949 100644\n>>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>>> @@ -333,6 +333,7 @@ public:\n>>> \t};\n>>> \n>>> \tstd::vector streams_;\n>>> +\tStream *rawStream_;\n>>> \n>>> \t/*\n>>> \t * All entities in the pipeline, from the camera sensor to the video\n>>> @@ -371,6 +372,11 @@ private:\n>>> \tvoid ispStatsReady(uint32_t frame, uint32_t bufferId);\n>>> \tvoid metadataReady(uint32_t frame, const ControlList &metadata);\n>>> \tvoid setSensorControls(const ControlList &sensorControls);\n>>> +\n>>> +\tbool processedRequested() const\n>>> +\t{\n>>> +\t\treturn streams_.size() - (rawStream_ ? 1 : 0) > 0;\n>>> +\t}\n>>> };\n>>> \n>>> class SimpleCameraConfiguration : public CameraConfiguration\n>>> @@ -467,7 +473,7 @@ private:\n>>> SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n>>> \t\t\t\t unsigned int numStreams,\n>>> \t\t\t\t MediaEntity *sensor)\n>>> -\t: Camera::Private(pipe), streams_(numStreams)\n>>> +\t: Camera::Private(pipe), streams_(numStreams), rawStream_(nullptr)\n>>> {\n>>> \t/*\n>>> \t * Find the shortest path from the camera sensor to a video capture\n>>> @@ -878,10 +884,13 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>> \t * point converting an erroneous buffer.\n>>> \t */\n>>> \tif (buffer->metadata().status != FrameMetadata::FrameSuccess) {\n>>> -\t\tif (!useConversion_) {\n>>> +\t\tif (rawStream_) {\n>>\n>> [1]\n>>\n>> I really don't understand the change here. \n> \n> The idea is that when a raw stream is requested then the buffers must be\n> handled basically as without software ISP because they serve as output\n> buffers (unlike with a pure software ISP case).\n> \n>> Also doesn't it break the case when only one processed stream is\n>> requested and an error occured ?\n> \n> It's the same as before in such a case -- the condition is false.\n\nHow come? In case of only processed request = 1,\n\nuseConversion_ (same value of needConversion_) = false as\n\n needConversion_ = config_.size() > 1\n\nso\n if (!useConversion_) is, if (true) \n\nwhereas with your change in case of processed request = 1,\n\n if (rawStream_) is, if (nullptr) = if (false)\n\nThe condition is not same as before.\n\n> \n>>> \t\t\t/* No conversion, just complete the request. */\n>>> \t\t\tRequest *request = buffer->request();\n>>> \t\t\tpipe->completeBuffer(request, buffer);\n>>> +\t\t\tSimpleFrameInfo *info = frameInfo_.find(request->sequence());\n>>> +\t\t\tif (info)\n>>> +\t\t\t\tinfo->metadataRequired = false;\n>>\n>> If this change is trying to adapt the if condition in\n>> tryCompleteRequest(), I believe this should be a different\n>> patch. For now, I don't see how it relates to raw stream enabling.\n> \n> I don't understand what you mean exactly here. The purpose of this\n> change is that now, when a processed stream may be available in addition\n> to a raw stream, we must ensure that we won't hang waiting for metadata\n> when an error occurs.\n> \n>>> \t\t\ttryCompleteRequest(request);\n>>> \t\t\treturn;\n>>> \t\t}\n>>> @@ -891,7 +900,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>> \t\t * buffer for capture (unless the stream is being stopped), and\n>>> \t\t * complete the request with all the user-facing buffers.\n>>> \t\t */\n>>> -\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled)\n>>> +\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled &&\n>>> +\t\t !rawStream_)\n>>\n>> Do you need to check for !rawStream_ again? The codepath would have\n>> returned early already if I consider the change above at [1], no? \n> \n> Right, I'll remove this.\n> \n>>> \t\t\tvideo_->queueBuffer(buffer);\n>>> \n>>> \t\tif (conversionQueue_.empty())\n>>> @@ -940,13 +950,14 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>> \t */\n>>> \tif (useConversion_) {\n>>> \t\tif (conversionQueue_.empty()) {\n>>> -\t\t\tvideo_->queueBuffer(buffer);\n>>> +\t\t\tif (!rawStream_)\n>>> +\t\t\t\tvideo_->queueBuffer(buffer);\n>>> \t\t\treturn;\n>>> \t\t}\n>>> \n>>> \t\tif (converter_)\n>>> \t\t\tconverter_->queueBuffers(buffer, conversionQueue_.front().outputs);\n>>> -\t\telse\n>>> +\t\telse if (processedRequested())\n>>\n>> Isn't this implicit, if useConversion_=True, you either go through\n>> converter_ or softISP. We don't need to check for processedRequested()\n>> here.\n> \n> Right, this condition is unnecessary, as well as processedRequested\n> method then.\n> \n>>> \t\t\t/*\n>>> \t\t\t * request->sequence() cannot be retrieved from `buffer' inside\n>>> \t\t\t * queueBuffers because unique_ptr's make buffer->request() invalid\n>>> @@ -956,6 +967,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>> \t\t\t\t\t conversionQueue_.front().outputs);\n>>> \n>>> \t\tconversionQueue_.pop();\n>>> +\t\tif (rawStream_)\n>>> +\t\t\tpipe->completeBuffer(request, buffer);\n>>\n>> This shouldn't be needed either, the function has pipe->completeBuffer()\n>> out of this if block.\n> \n> But this won't be called because of the return statement at the\n> following line:\n> \n>>> \t\treturn;\n>>> \t}\n>>> \n>>> @@ -993,7 +1006,8 @@ void SimpleCameraData::tryCompleteRequest(Request *request)\n>>> void SimpleCameraData::conversionInputDone(FrameBuffer *buffer)\n>>> {\n>>> \t/* Queue the input buffer back for capture. */\n>>> -\tvideo_->queueBuffer(buffer);\n>>> +\tif (!rawStream_)\n>>> +\t\tvideo_->queueBuffer(buffer);\n>>\n>> Don't you need to also complete the raw buffer in else block ?\n>> This would be the case where only raw stream is requested. Or is it done\n>> elsewhere?\n> \n> If only a raw stream is requested then buffers should be handled the\n> same way as before this patch. This method is not called in such a\n> case.\n> \n>>> }\n>>> \n>>> void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n>>> @@ -1518,11 +1532,15 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n>>> \n>>> \tfor (unsigned int i = 0; i < config->size(); ++i) {\n>>> \t\tStreamConfiguration &cfg = config->at(i);\n>>> +\t\tbool rawStream = isRaw(cfg);\n>>> \n>>> \t\tcfg.setStream(&data->streams_[i]);\n>>> \n>>> -\t\tif (data->useConversion_ && !isRaw(cfg))\n>>> +\t\tif (data->useConversion_ && !rawStream)\n>>> \t\t\toutputCfgs.push_back(cfg);\n>>> +\n>>> +\t\tif (rawStream)\n>>> +\t\t\tdata->rawStream_ = &data->streams_[i];\n>>> \t}\n>>> \n>>> \tif (outputCfgs.empty())\n>>> @@ -1553,7 +1571,7 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n>>> \t * Export buffers on the converter or capture video node, depending on\n>>> \t * whether the converter is used or not.\n>>> \t */\n>>> -\tif (data->useConversion_)\n>>> +\tif (data->useConversion_ && stream != data->rawStream_)\n>>> \t\treturn data->converter_\n>>> \t\t\t ? data->converter_->exportBuffers(stream, count, buffers)\n>>> \t\t\t : data->swIsp_->exportBuffers(stream, count, buffers);\n>>> @@ -1576,7 +1594,7 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>>> \t\treturn -EBUSY;\n>>> \t}\n>>> \n>>> -\tif (data->useConversion_) {\n>>> +\tif (data->useConversion_ && !data->rawStream_) {\n>>> \t\t/*\n>>> \t\t * When using the converter allocate a fixed number of internal\n>>> \t\t * buffers.\n>>> @@ -1584,8 +1602,11 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>>> \t\tret = video->allocateBuffers(kNumInternalBuffers,\n>>> \t\t\t\t\t &data->conversionBuffers_);\n>>> \t} else {\n>>> -\t\t/* Otherwise, prepare for using buffers from the only stream. */\n>>> -\t\tStream *stream = &data->streams_[0];\n>>> +\t\t/*\n>>> +\t\t * Otherwise, prepare for using buffers from either the raw stream, if\n>>> +\t\t * requested, or the only stream configured.\n>>> +\t\t */\n>>> +\t\tStream *stream = (data->rawStream_ ? data->rawStream_ : &data->streams_[0]);\n>>> \t\tret = video->importBuffers(stream->configuration().bufferCount);\n>>> \t}\n>>> \tif (ret < 0) {\n>>> @@ -1626,8 +1647,9 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>>> \t\t}\n>>> \n>>> \t\t/* Queue all internal buffers for capture. */\n>>> -\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n>>> -\t\t\tvideo->queueBuffer(buffer.get());\n>>> +\t\tif (!data->rawStream_)\n>>> +\t\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n>>> +\t\t\t\tvideo->queueBuffer(buffer.get());\n>>> \t}\n>>> \n>>> \treturn 0;\n>>> @@ -1678,7 +1700,7 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)\n>>> \t\t * queue, it will be handed to the converter in the capture\n>>> \t\t * completion handler.\n>>> \t\t */\n>>> -\t\tif (data->useConversion_) {\n>>> +\t\tif (data->useConversion_ && stream != data->rawStream_) {\n>>> \t\t\tbuffers.emplace(stream, buffer);\n>>> \t\t\tmetadataRequired = !!data->swIsp_;\n>>> \t\t} else {\n>>> -- \n>>> 2.51.0\n>>> \n> \n> Umang Jain writes:\n> \n>> I reviewed most of the patches and am OK with them. I prefer a slight\n>> rework in 4/8 and have highlighted it (with a reference diff I\n>> implemented earlier).\n> \n> I'll change 4/8 to be close to your implementation, with some small\n> corrections. \n> \n>> Unfortuantely, I did find patch 8/8 hard-to-follow :(\n> \n> I'm not sure what to do to make it easier, maybe the comments above help\n> a bit?\n> \n>> But if it's just me, it would be better if someone else should take a\n>> look.\n>>\n>>\n>> On Tue, Oct 21, 2025 at 08:27:15PM +0200, Milan Zamazal wrote:\n>>> When a raw stream is requested, whether alone or together with a\n>>> processed stream, its buffers must be handled outside the software ISP\n>>> machinery. They serve as output buffers, even when a processed stream\n>>> is produced.\n>>> \n>>> At most one raw stream and at most one processed stream are supported\n>>> and can be combined. An example of producing both raw and processed\n>>> files using `cam' application:\n>>> \n>>> cam -c1 -C100 -Ffile# \\\n>>> -s role=viewfinder,width=1920,height=1080,pixelformat=RGB888 \\\n>>> -s role=raw,width=3280,height=2464,pixelformat=SRGGB8 \\\n>>> \n>>> Note the difference in viewfinder and raw stream sizes due to the fact\n>>> that debayering requires enlarging the image width, which enforces\n>>> selecting a larger sensor resolution in this case.\n>>> \n>>> In order to track whether a raw stream is requested and which one it is,\n>>> SimpleCameraData::rawStream_ member variable is introduced.\n>>> \n>>> This is the final step to make raw streams working.\n>>> \n>>> Signed-off-by: Milan Zamazal \n>>> ---\n>>> src/libcamera/pipeline/simple/simple.cpp | 50 +++++++++++++++++-------\n>>> 1 file changed, 36 insertions(+), 14 deletions(-)\n>>> \n>>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>>> index 0c3284feb..607b07949 100644\n>>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>>> @@ -333,6 +333,7 @@ public:\n>>> \t};\n>>> \n>>> \tstd::vector streams_;\n>>> +\tStream *rawStream_;\n>>> \n>>> \t/*\n>>> \t * All entities in the pipeline, from the camera sensor to the video\n>>> @@ -371,6 +372,11 @@ private:\n>>> \tvoid ispStatsReady(uint32_t frame, uint32_t bufferId);\n>>> \tvoid metadataReady(uint32_t frame, const ControlList &metadata);\n>>> \tvoid setSensorControls(const ControlList &sensorControls);\n>>> +\n>>> +\tbool processedRequested() const\n>>> +\t{\n>>> +\t\treturn streams_.size() - (rawStream_ ? 1 : 0) > 0;\n>>> +\t}\n>>> };\n>>> \n>>> class SimpleCameraConfiguration : public CameraConfiguration\n>>> @@ -467,7 +473,7 @@ private:\n>>> SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n>>> \t\t\t\t unsigned int numStreams,\n>>> \t\t\t\t MediaEntity *sensor)\n>>> -\t: Camera::Private(pipe), streams_(numStreams)\n>>> +\t: Camera::Private(pipe), streams_(numStreams), rawStream_(nullptr)\n>>> {\n>>> \t/*\n>>> \t * Find the shortest path from the camera sensor to a video capture\n>>> @@ -878,10 +884,13 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>> \t * point converting an erroneous buffer.\n>>> \t */\n>>> \tif (buffer->metadata().status != FrameMetadata::FrameSuccess) {\n>>> -\t\tif (!useConversion_) {\n>>> +\t\tif (rawStream_) {\n>>> \t\t\t/* No conversion, just complete the request. */\n>>> \t\t\tRequest *request = buffer->request();\n>>> \t\t\tpipe->completeBuffer(request, buffer);\n>>> +\t\t\tSimpleFrameInfo *info = frameInfo_.find(request->sequence());\n>>> +\t\t\tif (info)\n>>> +\t\t\t\tinfo->metadataRequired = false;\n>>> \t\t\ttryCompleteRequest(request);\n>>> \t\t\treturn;\n>>> \t\t}\n>>> @@ -891,7 +900,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>> \t\t * buffer for capture (unless the stream is being stopped), and\n>>> \t\t * complete the request with all the user-facing buffers.\n>>> \t\t */\n>>> -\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled)\n>>> +\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled &&\n>>> +\t\t !rawStream_)\n>>> \t\t\tvideo_->queueBuffer(buffer);\n>>> \n>>> \t\tif (conversionQueue_.empty())\n>>> @@ -940,13 +950,14 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>> \t */\n>>> \tif (useConversion_) {\n>>> \t\tif (conversionQueue_.empty()) {\n>>> -\t\t\tvideo_->queueBuffer(buffer);\n>>> +\t\t\tif (!rawStream_)\n>>> +\t\t\t\tvideo_->queueBuffer(buffer);\n>>> \t\t\treturn;\n>>> \t\t}\n>>> \n>>> \t\tif (converter_)\n>>> \t\t\tconverter_->queueBuffers(buffer, conversionQueue_.front().outputs);\n>>> -\t\telse\n>>> +\t\telse if (processedRequested())\n>>> \t\t\t/*\n>>> \t\t\t * request->sequence() cannot be retrieved from `buffer' inside\n>>> \t\t\t * queueBuffers because unique_ptr's make buffer->request() invalid\n>>> @@ -956,6 +967,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>> \t\t\t\t\t conversionQueue_.front().outputs);\n>>> \n>>> \t\tconversionQueue_.pop();\n>>> +\t\tif (rawStream_)\n>>> +\t\t\tpipe->completeBuffer(request, buffer);\n>>> \t\treturn;\n>>> \t}\n>>> \n>>> @@ -993,7 +1006,8 @@ void SimpleCameraData::tryCompleteRequest(Request *request)\n>>> void SimpleCameraData::conversionInputDone(FrameBuffer *buffer)\n>>> {\n>>> \t/* Queue the input buffer back for capture. */\n>>> -\tvideo_->queueBuffer(buffer);\n>>> +\tif (!rawStream_)\n>>> +\t\tvideo_->queueBuffer(buffer);\n>>> }\n>>> \n>>> void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n>>> @@ -1518,11 +1532,15 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n>>> \n>>> \tfor (unsigned int i = 0; i < config->size(); ++i) {\n>>> \t\tStreamConfiguration &cfg = config->at(i);\n>>> +\t\tbool rawStream = isRaw(cfg);\n>>> \n>>> \t\tcfg.setStream(&data->streams_[i]);\n>>> \n>>> -\t\tif (data->useConversion_ && !isRaw(cfg))\n>>> +\t\tif (data->useConversion_ && !rawStream)\n>>> \t\t\toutputCfgs.push_back(cfg);\n>>> +\n>>> +\t\tif (rawStream)\n>>> +\t\t\tdata->rawStream_ = &data->streams_[i];\n>>> \t}\n>>> \n>>> \tif (outputCfgs.empty())\n>>> @@ -1553,7 +1571,7 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n>>> \t * Export buffers on the converter or capture video node, depending on\n>>> \t * whether the converter is used or not.\n>>> \t */\n>>> -\tif (data->useConversion_)\n>>> +\tif (data->useConversion_ && stream != data->rawStream_)\n>>> \t\treturn data->converter_\n>>> \t\t\t ? data->converter_->exportBuffers(stream, count, buffers)\n>>> \t\t\t : data->swIsp_->exportBuffers(stream, count, buffers);\n>>> @@ -1576,7 +1594,7 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>>> \t\treturn -EBUSY;\n>>> \t}\n>>> \n>>> -\tif (data->useConversion_) {\n>>> +\tif (data->useConversion_ && !data->rawStream_) {\n>>> \t\t/*\n>>> \t\t * When using the converter allocate a fixed number of internal\n>>> \t\t * buffers.\n>>> @@ -1584,8 +1602,11 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>>> \t\tret = video->allocateBuffers(kNumInternalBuffers,\n>>> \t\t\t\t\t &data->conversionBuffers_);\n>>> \t} else {\n>>> -\t\t/* Otherwise, prepare for using buffers from the only stream. */\n>>> -\t\tStream *stream = &data->streams_[0];\n>>> +\t\t/*\n>>> +\t\t * Otherwise, prepare for using buffers from either the raw stream, if\n>>> +\t\t * requested, or the only stream configured.\n>>> +\t\t */\n>>> +\t\tStream *stream = (data->rawStream_ ? data->rawStream_ : &data->streams_[0]);\n>>> \t\tret = video->importBuffers(stream->configuration().bufferCount);\n>>> \t}\n>>> \tif (ret < 0) {\n>>> @@ -1626,8 +1647,9 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>>> \t\t}\n>>> \n>>> \t\t/* Queue all internal buffers for capture. */\n>>> -\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n>>> -\t\t\tvideo->queueBuffer(buffer.get());\n>>> +\t\tif (!data->rawStream_)\n>>> +\t\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n>>> +\t\t\t\tvideo->queueBuffer(buffer.get());\n>>> \t}\n>>> \n>>> \treturn 0;\n>>> @@ -1678,7 +1700,7 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)\n>>> \t\t * queue, it will be handed to the converter in the capture\n>>> \t\t * completion handler.\n>>> \t\t */\n>>> -\t\tif (data->useConversion_) {\n>>> +\t\tif (data->useConversion_ && stream != data->rawStream_) {\n>>> \t\t\tbuffers.emplace(stream, buffer);\n>>> \t\t\tmetadataRequired = !!data->swIsp_;\n>>> \t\t} else {\n>>> -- \n>>> 2.51.0\n>>> \n> \n> Umang Jain writes:\n> \n>> On Tue, Oct 21, 2025 at 08:27:15PM +0200, Milan Zamazal wrote:\n>>> When a raw stream is requested, whether alone or together with a\n>>> processed stream, its buffers must be handled outside the software ISP\n>>\n>>> machinery. They serve as output buffers, even when a processed stream\n>>> is produced.\n>>> \n>>> At most one raw stream and at most one processed stream are supported\n>>> and can be combined. An example of producing both raw and processed\n>>> files using `cam' application:\n>>> \n>>> cam -c1 -C100 -Ffile# \\\n>>> -s role=viewfinder,width=1920,height=1080,pixelformat=RGB888 \\\n>>> -s role=raw,width=3280,height=2464,pixelformat=SRGGB8 \\\n>>> \n>>> Note the difference in viewfinder and raw stream sizes due to the fact\n>>> that debayering requires enlarging the image width, which enforces\n>>> selecting a larger sensor resolution in this case.\n>>> \n>>> In order to track whether a raw stream is requested and which one it is,\n>>> SimpleCameraData::rawStream_ member variable is introduced.\n>>> \n>>> This is the final step to make raw streams working.\n>>> \n>>> Signed-off-by: Milan Zamazal \n>>> ---\n>>> src/libcamera/pipeline/simple/simple.cpp | 50 +++++++++++++++++-------\n>>> 1 file changed, 36 insertions(+), 14 deletions(-)\n>>> \n>>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>>> index 0c3284feb..607b07949 100644\n>>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>>> @@ -333,6 +333,7 @@ public:\n>>> \t};\n>>> \n>>> \tstd::vector streams_;\n>>> +\tStream *rawStream_;\n>>> \n>>> \t/*\n>>> \t * All entities in the pipeline, from the camera sensor to the video\n>>> @@ -371,6 +372,11 @@ private:\n>>> \tvoid ispStatsReady(uint32_t frame, uint32_t bufferId);\n>>> \tvoid metadataReady(uint32_t frame, const ControlList &metadata);\n>>> \tvoid setSensorControls(const ControlList &sensorControls);\n>>> +\n>>> +\tbool processedRequested() const\n>>> +\t{\n>>> +\t\treturn streams_.size() - (rawStream_ ? 1 : 0) > 0;\n>>> +\t}\n>>> };\n>>> \n>>> class SimpleCameraConfiguration : public CameraConfiguration\n>>> @@ -467,7 +473,7 @@ private:\n>>> SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n>>> \t\t\t\t unsigned int numStreams,\n>>> \t\t\t\t MediaEntity *sensor)\n>>> -\t: Camera::Private(pipe), streams_(numStreams)\n>>> +\t: Camera::Private(pipe), streams_(numStreams), rawStream_(nullptr)\n>>> {\n>>> \t/*\n>>> \t * Find the shortest path from the camera sensor to a video capture\n>>> @@ -878,10 +884,13 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>> \t * point converting an erroneous buffer.\n>>> \t */\n>>> \tif (buffer->metadata().status != FrameMetadata::FrameSuccess) {\n>>> -\t\tif (!useConversion_) {\n>>> +\t\tif (rawStream_) {\n>>> \t\t\t/* No conversion, just complete the request. */\n>>> \t\t\tRequest *request = buffer->request();\n>>> \t\t\tpipe->completeBuffer(request, buffer);\n>>> +\t\t\tSimpleFrameInfo *info = frameInfo_.find(request->sequence());\n>>> +\t\t\tif (info)\n>>> +\t\t\t\tinfo->metadataRequired = false;\n>>> \t\t\ttryCompleteRequest(request);\n>>> \t\t\treturn;\n>>> \t\t}\n>>> @@ -891,7 +900,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>> \t\t * buffer for capture (unless the stream is being stopped), and\n>>> \t\t * complete the request with all the user-facing buffers.\n>>> \t\t */\n>>> -\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled)\n>>> +\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled &&\n>>> +\t\t !rawStream_)\n>>> \t\t\tvideo_->queueBuffer(buffer);\n>>> \n>>> \t\tif (conversionQueue_.empty())\n>>> @@ -940,13 +950,14 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>> \t */\n>>> \tif (useConversion_) {\n>>> \t\tif (conversionQueue_.empty()) {\n>>> -\t\t\tvideo_->queueBuffer(buffer);\n>>> +\t\t\tif (!rawStream_)\n>>> +\t\t\t\tvideo_->queueBuffer(buffer);\n>>> \t\t\treturn;\n>>> \t\t}\n>>> \n>>> \t\tif (converter_)\n>>> \t\t\tconverter_->queueBuffers(buffer, conversionQueue_.front().outputs);\n>>> -\t\telse\n>>> +\t\telse if (processedRequested())\n>>> \t\t\t/*\n>>> \t\t\t * request->sequence() cannot be retrieved from `buffer' inside\n>>> \t\t\t * queueBuffers because unique_ptr's make buffer->request() invalid\n>>> @@ -956,6 +967,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>> \t\t\t\t\t conversionQueue_.front().outputs);\n>>> \n>>> \t\tconversionQueue_.pop();\n>>> +\t\tif (rawStream_)\n>>> +\t\t\tpipe->completeBuffer(request, buffer);\n>>> \t\treturn;\n>>> \t}\n>>> \n>>> @@ -993,7 +1006,8 @@ void SimpleCameraData::tryCompleteRequest(Request *request)\n>>> void SimpleCameraData::conversionInputDone(FrameBuffer *buffer)\n>>> {\n>>> \t/* Queue the input buffer back for capture. */\n>>> -\tvideo_->queueBuffer(buffer);\n>>> +\tif (!rawStream_)\n>>> +\t\tvideo_->queueBuffer(buffer);\n>>> }\n>>> \n>>> void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n>>> @@ -1518,11 +1532,15 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n>>> \n>>> \tfor (unsigned int i = 0; i < config->size(); ++i) {\n>>> \t\tStreamConfiguration &cfg = config->at(i);\n>>> +\t\tbool rawStream = isRaw(cfg);\n>>> \n>>> \t\tcfg.setStream(&data->streams_[i]);\n>>> \n>>> -\t\tif (data->useConversion_ && !isRaw(cfg))\n>>> +\t\tif (data->useConversion_ && !rawStream)\n>>> \t\t\toutputCfgs.push_back(cfg);\n>>> +\n>>> +\t\tif (rawStream)\n>>> +\t\t\tdata->rawStream_ = &data->streams_[i];\n>>\n>> Last comment, just realised:\n>>\n>> data->rawStream_ should be initialised to nullptr (outside this loop)\n>> each time the camera is configured, otherwise it can lead to subtle bugs\n>> if the camera configure() is called multiple times with varying\n>> configurations.\n> \n> Right, I'll add it.\n> \n>>> \t}\n>>> \n>>> \tif (outputCfgs.empty())\n>>> @@ -1553,7 +1571,7 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n>>> \t * Export buffers on the converter or capture video node, depending on\n>>> \t * whether the converter is used or not.\n>>> \t */\n>>> -\tif (data->useConversion_)\n>>> +\tif (data->useConversion_ && stream != data->rawStream_)\n>>> \t\treturn data->converter_\n>>> \t\t\t ? data->converter_->exportBuffers(stream, count, buffers)\n>>> \t\t\t : data->swIsp_->exportBuffers(stream, count, buffers);\n>>> @@ -1576,7 +1594,7 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>>> \t\treturn -EBUSY;\n>>> \t}\n>>> \n>>> -\tif (data->useConversion_) {\n>>> +\tif (data->useConversion_ && !data->rawStream_) {\n>>> \t\t/*\n>>> \t\t * When using the converter allocate a fixed number of internal\n>>> \t\t * buffers.\n>>> @@ -1584,8 +1602,11 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>>> \t\tret = video->allocateBuffers(kNumInternalBuffers,\n>>> \t\t\t\t\t &data->conversionBuffers_);\n>>> \t} else {\n>>> -\t\t/* Otherwise, prepare for using buffers from the only stream. */\n>>> -\t\tStream *stream = &data->streams_[0];\n>>> +\t\t/*\n>>> +\t\t * Otherwise, prepare for using buffers from either the raw stream, if\n>>> +\t\t * requested, or the only stream configured.\n>>> +\t\t */\n>>> +\t\tStream *stream = (data->rawStream_ ? data->rawStream_ : &data->streams_[0]);\n>>> \t\tret = video->importBuffers(stream->configuration().bufferCount);\n>>> \t}\n>>> \tif (ret < 0) {\n>>> @@ -1626,8 +1647,9 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>>> \t\t}\n>>> \n>>> \t\t/* Queue all internal buffers for capture. */\n>>> -\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n>>> -\t\t\tvideo->queueBuffer(buffer.get());\n>>> +\t\tif (!data->rawStream_)\n>>> +\t\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n>>> +\t\t\t\tvideo->queueBuffer(buffer.get());\n>>> \t}\n>>> \n>>> \treturn 0;\n>>> @@ -1678,7 +1700,7 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)\n>>> \t\t * queue, it will be handed to the converter in the capture\n>>> \t\t * completion handler.\n>>> \t\t */\n>>> -\t\tif (data->useConversion_) {\n>>> +\t\tif (data->useConversion_ && stream != data->rawStream_) {\n>>> \t\t\tbuffers.emplace(stream, buffer);\n>>> \t\t\tmetadataRequired = !!data->swIsp_;\n>>> \t\t} else {\n>>> -- \n>>> 2.51.0\n>>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 1F91EC3241\n\tfor ;\n\tMon, 3 Nov 2025 17:57:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 697A3609D8;\n\tMon, 3 Nov 2025 18:57:57 +0100 (CET)","from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 10228606A0\n\tfor ;\n\tMon, 3 Nov 2025 18:57:55 +0100 (CET)","from maestria.local.igalia.com ([192.168.10.14]\n\thelo=mail.igalia.com) by fanzine2.igalia.com with esmtps \n\t(Cipher TLS1.3:ECDHE_SECP256R1__RSA_PSS_RSAE_SHA256__AES_256_GCM:256)\n\t(Exim) id 1vFyoT-000zei-2s; Mon, 03 Nov 2025 18:57:53 +0100","from webmail.service.igalia.com ([192.168.21.45])\n\tby mail.igalia.com with esmtp (Exim)\n\tid 1vFyoQ-006IVp-7Y; Mon, 03 Nov 2025 18:57:52 +0100","from localhost ([127.0.0.1] helo=webmail.igalia.com)\n\tby webmail with esmtp (Exim 4.96) (envelope-from )\n\tid 1vFyoP-002Y7k-27; Mon, 03 Nov 2025 18:57:50 +0100"],"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=\"ChzetUDj\"; 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:Message-ID:References:\n\tIn-Reply-To:Subject:Cc:To:From:Date:MIME-Version: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=RR7FIy6M4xgiE2J5eNrNEHcdRZDQbjFWAneG2lhu+TQ=;\n\tb=ChzetUDjODq76bQ3+NrUNECEKw\n\twk1OLPOAa+ssRb8RundiZ/GkEvAJ/+A9WQ9dMmZ8OdffpkgL4mw0UYo9WOct4I6yhm3HNpieJdx+m\n\tjRTnrjt0gKDY7pLCFUExt3iwXUyBoFGvQEQry7DA+l5F1oNtPKNivahfQX8lsUHySULKOV3hizWkE\n\tnpz/+5xev5zO4ZT2xh1eG2j4Ft1cVkdLatJvOoCk/nKJVR+e3dL0CC3r91krn+0I9C27Ax1YhmYH1\n\t6ujt0fglejyHzSueZVozwOsyiweHZ7CwBb0S2BopjoagIUOKC1fxFwrG5OrSSgs8kmMOeCYmLbYMN\n\tq4kA64wQ==;","MIME-Version":"1.0","Date":"Mon, 03 Nov 2025 17:57:49 +0000","From":"uajain ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t,\n\tKieran Bingham , =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?= ,\n\tPaul Elder ","Subject":"Re: [PATCH v13 8/8] libcamera: simple: Make raw streams working","In-Reply-To":"<855xbttuup.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","References":"<20251021182716.29274-1-mzamazal@redhat.com>\n\t<20251021182716.29274-9-mzamazal@redhat.com>\n\t\n\t<855xbttuup.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Message-ID":"","X-Sender":"uajain@igalia.com","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","X-Spam-Report":"NO, Score=-2.2, Tests=ALL_TRUSTED=-3,AWL=0.000,BAYES_50=0.8","X-Spam-Score":"-21","X-Spam-Bar":"--","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":36671,"web_url":"https://patchwork.libcamera.org/comment/36671/","msgid":"<85ldkm9277.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-11-03T22:33:00","subject":"Re: [PATCH v13 8/8] libcamera: simple: Make raw streams working","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"uajain writes:\n\n> On 2025-11-01 19:29, Milan Zamazal wrote:\n>> Hi Umang,\n>> \n>\n>> Umang Jain writes:\n>> \n>>> On Tue, Oct 21, 2025 at 08:27:15PM +0200, Milan Zamazal wrote:\n>>>> When a raw stream is requested, whether alone or together with a\n>>>> processed stream, its buffers must be handled outside the software ISP\n>>>\n>>>> machinery. They serve as output buffers, even when a processed stream\n>>>> is produced.\n>>>> \n>>>> At most one raw stream and at most one processed stream are supported\n>>>> and can be combined. An example of producing both raw and processed\n>>>> files using `cam' application:\n>>>> \n>>>> cam -c1 -C100 -Ffile# \\\n>>>> -s role=viewfinder,width=1920,height=1080,pixelformat=RGB888 \\\n>>>> -s role=raw,width=3280,height=2464,pixelformat=SRGGB8 \\\n>>>> \n>>>> Note the difference in viewfinder and raw stream sizes due to the fact\n>>>> that debayering requires enlarging the image width, which enforces\n>>>> selecting a larger sensor resolution in this case.\n>>>> \n>>>> In order to track whether a raw stream is requested and which one it is,\n>>>> SimpleCameraData::rawStream_ member variable is introduced.\n>>>> \n>>>> This is the final step to make raw streams working.\n>>>> \n>>>> Signed-off-by: Milan Zamazal \n>>>> ---\n>>>> src/libcamera/pipeline/simple/simple.cpp | 50 +++++++++++++++++-------\n>>>> 1 file changed, 36 insertions(+), 14 deletions(-)\n>>>> \n>>>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>>>> index 0c3284feb..607b07949 100644\n>>>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>>>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>>>> @@ -333,6 +333,7 @@ public:\n>>>> \t};\n>>>> \n>>>> \tstd::vector streams_;\n>>>> +\tStream *rawStream_;\n>>>> \n>>>> \t/*\n>>>> \t * All entities in the pipeline, from the camera sensor to the video\n>>>> @@ -371,6 +372,11 @@ private:\n>>>> \tvoid ispStatsReady(uint32_t frame, uint32_t bufferId);\n>>>> \tvoid metadataReady(uint32_t frame, const ControlList &metadata);\n>>>> \tvoid setSensorControls(const ControlList &sensorControls);\n>>>> +\n>>>> +\tbool processedRequested() const\n>>>> +\t{\n>>>> +\t\treturn streams_.size() - (rawStream_ ? 1 : 0) > 0;\n>>>> +\t}\n>>>> };\n>>>> \n>>>> class SimpleCameraConfiguration : public CameraConfiguration\n>>>> @@ -467,7 +473,7 @@ private:\n>>>> SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n>>>> \t\t\t\t unsigned int numStreams,\n>>>> \t\t\t\t MediaEntity *sensor)\n>>>> -\t: Camera::Private(pipe), streams_(numStreams)\n>>>> +\t: Camera::Private(pipe), streams_(numStreams), rawStream_(nullptr)\n>>>> {\n>>>> \t/*\n>>>> \t * Find the shortest path from the camera sensor to a video capture\n>>>> @@ -878,10 +884,13 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>>> \t * point converting an erroneous buffer.\n>>>> \t */\n>>>> \tif (buffer->metadata().status != FrameMetadata::FrameSuccess) {\n>>>> -\t\tif (!useConversion_) {\n>>>> +\t\tif (rawStream_) {\n>>>\n>>> [1]\n>>>\n>>> I really don't understand the change here. \n>> \n>> The idea is that when a raw stream is requested then the buffers must be\n>> handled basically as without software ISP because they serve as output\n>> buffers (unlike with a pure software ISP case).\n>> \n>>> Also doesn't it break the case when only one processed stream is\n>>> requested and an error occured ?\n>> \n>> It's the same as before in such a case -- the condition is false.\n>\n> How come? In case of only processed request = 1,\n>\n> useConversion_ (same value of needConversion_) = false as\n>\n> needConversion_ = config_.size() > 1\n>\n> so\n> if (!useConversion_) is, if (true) \n>\n> whereas with your change in case of processed request = 1,\n>\n> if (rawStream_) is, if (nullptr) = if (false)\n>\n> The condition is not same as before.\n\nYou're right, sorry for the confusion. I'll fix the condition in v15.\n\nI think I got confused by the fact that with software ISP\nneedConversion_ is true:\n\n if (cfg.pixelFormat != pipeConfig_->captureFormat ||\n cfg.size != pipeConfig_->captureSize)\n \tneedConversion_ = true;\n\nBut it doesn't necessarily mean that all non-raw (in the sense of\nisRaw()) streams need conversion so the condition should still check for\nuseConversion. A similar problem may also be present elsewhere.\n\n>>>> \t\t\t/* No conversion, just complete the request. */\n>>>> \t\t\tRequest *request = buffer->request();\n>>>> \t\t\tpipe->completeBuffer(request, buffer);\n>>>> +\t\t\tSimpleFrameInfo *info = frameInfo_.find(request->sequence());\n>>>> +\t\t\tif (info)\n>>>> +\t\t\t\tinfo->metadataRequired = false;\n>>>\n>>> If this change is trying to adapt the if condition in\n>>> tryCompleteRequest(), I believe this should be a different\n>>> patch. For now, I don't see how it relates to raw stream enabling.\n>> \n>> I don't understand what you mean exactly here. The purpose of this\n>> change is that now, when a processed stream may be available in addition\n>> to a raw stream, we must ensure that we won't hang waiting for metadata\n>> when an error occurs.\n>> \n>>>> \t\t\ttryCompleteRequest(request);\n>>>> \t\t\treturn;\n>>>> \t\t}\n>>>> @@ -891,7 +900,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>>> \t\t * buffer for capture (unless the stream is being stopped), and\n>>>> \t\t * complete the request with all the user-facing buffers.\n>>>> \t\t */\n>>>> -\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled)\n>>>> +\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled &&\n>>>> +\t\t !rawStream_)\n>>>\n>>> Do you need to check for !rawStream_ again? The codepath would have\n>>> returned early already if I consider the change above at [1], no? \n>> \n>> Right, I'll remove this.\n>> \n>>>> \t\t\tvideo_->queueBuffer(buffer);\n>>>> \n>>>> \t\tif (conversionQueue_.empty())\n>>>> @@ -940,13 +950,14 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>>> \t */\n>>>> \tif (useConversion_) {\n>>>> \t\tif (conversionQueue_.empty()) {\n>>>> -\t\t\tvideo_->queueBuffer(buffer);\n>>>> +\t\t\tif (!rawStream_)\n>>>> +\t\t\t\tvideo_->queueBuffer(buffer);\n>>>> \t\t\treturn;\n>>>> \t\t}\n>>>> \n>>>> \t\tif (converter_)\n>>>> \t\t\tconverter_->queueBuffers(buffer, conversionQueue_.front().outputs);\n>>>> -\t\telse\n>>>> +\t\telse if (processedRequested())\n>>>\n>>> Isn't this implicit, if useConversion_=True, you either go through\n>>> converter_ or softISP. We don't need to check for processedRequested()\n>>> here.\n>> \n>> Right, this condition is unnecessary, as well as processedRequested\n>> method then.\n>> \n>>>> \t\t\t/*\n>>>> \t\t\t * request->sequence() cannot be retrieved from `buffer' inside\n>>>> \t\t\t * queueBuffers because unique_ptr's make buffer->request() invalid\n>>>> @@ -956,6 +967,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>>> \t\t\t\t\t conversionQueue_.front().outputs);\n>>>> \n>>>> \t\tconversionQueue_.pop();\n>>>> +\t\tif (rawStream_)\n>>>> +\t\t\tpipe->completeBuffer(request, buffer);\n>>>\n>>> This shouldn't be needed either, the function has pipe->completeBuffer()\n>>> out of this if block.\n>> \n>> But this won't be called because of the return statement at the\n>> following line:\n>> \n>>>> \t\treturn;\n>>>> \t}\n>>>> \n>>>> @@ -993,7 +1006,8 @@ void SimpleCameraData::tryCompleteRequest(Request *request)\n>>>> void SimpleCameraData::conversionInputDone(FrameBuffer *buffer)\n>>>> {\n>>>> \t/* Queue the input buffer back for capture. */\n>>>> -\tvideo_->queueBuffer(buffer);\n>>>> +\tif (!rawStream_)\n>>>> +\t\tvideo_->queueBuffer(buffer);\n>>>\n>>> Don't you need to also complete the raw buffer in else block ?\n>>> This would be the case where only raw stream is requested. Or is it done\n>>> elsewhere?\n>> \n>> If only a raw stream is requested then buffers should be handled the\n>> same way as before this patch. This method is not called in such a\n>> case.\n>> \n>>>> }\n>>>> \n>>>> void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n>>>> @@ -1518,11 +1532,15 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n>>>> \n>>>> \tfor (unsigned int i = 0; i < config->size(); ++i) {\n>>>> \t\tStreamConfiguration &cfg = config->at(i);\n>>>> +\t\tbool rawStream = isRaw(cfg);\n>>>> \n>>>> \t\tcfg.setStream(&data->streams_[i]);\n>>>> \n>>>> -\t\tif (data->useConversion_ && !isRaw(cfg))\n>>>> +\t\tif (data->useConversion_ && !rawStream)\n>>>> \t\t\toutputCfgs.push_back(cfg);\n>>>> +\n>>>> +\t\tif (rawStream)\n>>>> +\t\t\tdata->rawStream_ = &data->streams_[i];\n>>>> \t}\n>>>> \n>>>> \tif (outputCfgs.empty())\n>>>> @@ -1553,7 +1571,7 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n>>>> \t * Export buffers on the converter or capture video node, depending on\n>>>> \t * whether the converter is used or not.\n>>>> \t */\n>>>> -\tif (data->useConversion_)\n>>>> +\tif (data->useConversion_ && stream != data->rawStream_)\n>>>> \t\treturn data->converter_\n>>>> \t\t\t ? data->converter_->exportBuffers(stream, count, buffers)\n>>>> \t\t\t : data->swIsp_->exportBuffers(stream, count, buffers);\n>>>> @@ -1576,7 +1594,7 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>>>> \t\treturn -EBUSY;\n>>>> \t}\n>>>> \n>>>> -\tif (data->useConversion_) {\n>>>> +\tif (data->useConversion_ && !data->rawStream_) {\n>>>> \t\t/*\n>>>> \t\t * When using the converter allocate a fixed number of internal\n>>>> \t\t * buffers.\n>>>> @@ -1584,8 +1602,11 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>>>> \t\tret = video->allocateBuffers(kNumInternalBuffers,\n>>>> \t\t\t\t\t &data->conversionBuffers_);\n>>>> \t} else {\n>>>> -\t\t/* Otherwise, prepare for using buffers from the only stream. */\n>>>> -\t\tStream *stream = &data->streams_[0];\n>>>> +\t\t/*\n>>>> +\t\t * Otherwise, prepare for using buffers from either the raw stream, if\n>>>> +\t\t * requested, or the only stream configured.\n>>>> +\t\t */\n>>>> +\t\tStream *stream = (data->rawStream_ ? data->rawStream_ : &data->streams_[0]);\n>>>> \t\tret = video->importBuffers(stream->configuration().bufferCount);\n>>>> \t}\n>>>> \tif (ret < 0) {\n>>>> @@ -1626,8 +1647,9 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>>>> \t\t}\n>>>> \n>>>> \t\t/* Queue all internal buffers for capture. */\n>>>> -\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n>>>> -\t\t\tvideo->queueBuffer(buffer.get());\n>>>> +\t\tif (!data->rawStream_)\n>>>> +\t\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n>>>> +\t\t\t\tvideo->queueBuffer(buffer.get());\n>>>> \t}\n>>>> \n>>>> \treturn 0;\n>>>> @@ -1678,7 +1700,7 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)\n>>>> \t\t * queue, it will be handed to the converter in the capture\n>>>> \t\t * completion handler.\n>>>> \t\t */\n>>>> -\t\tif (data->useConversion_) {\n>>>> +\t\tif (data->useConversion_ && stream != data->rawStream_) {\n>>>> \t\t\tbuffers.emplace(stream, buffer);\n>>>> \t\t\tmetadataRequired = !!data->swIsp_;\n>>>> \t\t} else {\n>>>> -- \n>>>> 2.51.0\n>>>> \n>> \n>> Umang Jain writes:\n>> \n>>> I reviewed most of the patches and am OK with them. I prefer a slight\n>>> rework in 4/8 and have highlighted it (with a reference diff I\n>>> implemented earlier).\n>> \n>> I'll change 4/8 to be close to your implementation, with some small\n>> corrections. \n>> \n>>> Unfortuantely, I did find patch 8/8 hard-to-follow :(\n>> \n>> I'm not sure what to do to make it easier, maybe the comments above help\n>> a bit?\n>> \n>>> But if it's just me, it would be better if someone else should take a\n>>> look.\n>>>\n>>>\n>>> On Tue, Oct 21, 2025 at 08:27:15PM +0200, Milan Zamazal wrote:\n>>>> When a raw stream is requested, whether alone or together with a\n>>>> processed stream, its buffers must be handled outside the software ISP\n>>>> machinery. They serve as output buffers, even when a processed stream\n>>>> is produced.\n>>>> \n>>>> At most one raw stream and at most one processed stream are supported\n>>>> and can be combined. An example of producing both raw and processed\n>>>> files using `cam' application:\n>>>> \n>>>> cam -c1 -C100 -Ffile# \\\n>>>> -s role=viewfinder,width=1920,height=1080,pixelformat=RGB888 \\\n>>>> -s role=raw,width=3280,height=2464,pixelformat=SRGGB8 \\\n>>>> \n>>>> Note the difference in viewfinder and raw stream sizes due to the fact\n>>>> that debayering requires enlarging the image width, which enforces\n>>>> selecting a larger sensor resolution in this case.\n>>>> \n>>>> In order to track whether a raw stream is requested and which one it is,\n>>>> SimpleCameraData::rawStream_ member variable is introduced.\n>>>> \n>>>> This is the final step to make raw streams working.\n>>>> \n>>>> Signed-off-by: Milan Zamazal \n>>>> ---\n>>>> src/libcamera/pipeline/simple/simple.cpp | 50 +++++++++++++++++-------\n>>>> 1 file changed, 36 insertions(+), 14 deletions(-)\n>>>> \n>>>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>>>> index 0c3284feb..607b07949 100644\n>>>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>>>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>>>> @@ -333,6 +333,7 @@ public:\n>>>> \t};\n>>>> \n>>>> \tstd::vector streams_;\n>>>> +\tStream *rawStream_;\n>>>> \n>>>> \t/*\n>>>> \t * All entities in the pipeline, from the camera sensor to the video\n>>>> @@ -371,6 +372,11 @@ private:\n>>>> \tvoid ispStatsReady(uint32_t frame, uint32_t bufferId);\n>>>> \tvoid metadataReady(uint32_t frame, const ControlList &metadata);\n>>>> \tvoid setSensorControls(const ControlList &sensorControls);\n>>>> +\n>>>> +\tbool processedRequested() const\n>>>> +\t{\n>>>> +\t\treturn streams_.size() - (rawStream_ ? 1 : 0) > 0;\n>>>> +\t}\n>>>> };\n>>>> \n>>>> class SimpleCameraConfiguration : public CameraConfiguration\n>>>> @@ -467,7 +473,7 @@ private:\n>>>> SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n>>>> \t\t\t\t unsigned int numStreams,\n>>>> \t\t\t\t MediaEntity *sensor)\n>>>> -\t: Camera::Private(pipe), streams_(numStreams)\n>>>> +\t: Camera::Private(pipe), streams_(numStreams), rawStream_(nullptr)\n>>>> {\n>>>> \t/*\n>>>> \t * Find the shortest path from the camera sensor to a video capture\n>>>> @@ -878,10 +884,13 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>>> \t * point converting an erroneous buffer.\n>>>> \t */\n>>>> \tif (buffer->metadata().status != FrameMetadata::FrameSuccess) {\n>>>> -\t\tif (!useConversion_) {\n>>>> +\t\tif (rawStream_) {\n>>>> \t\t\t/* No conversion, just complete the request. */\n>>>> \t\t\tRequest *request = buffer->request();\n>>>> \t\t\tpipe->completeBuffer(request, buffer);\n>>>> +\t\t\tSimpleFrameInfo *info = frameInfo_.find(request->sequence());\n>>>> +\t\t\tif (info)\n>>>> +\t\t\t\tinfo->metadataRequired = false;\n>>>> \t\t\ttryCompleteRequest(request);\n>>>> \t\t\treturn;\n>>>> \t\t}\n>>>> @@ -891,7 +900,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>>> \t\t * buffer for capture (unless the stream is being stopped), and\n>>>> \t\t * complete the request with all the user-facing buffers.\n>>>> \t\t */\n>>>> -\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled)\n>>>> +\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled &&\n>>>> +\t\t !rawStream_)\n>>>> \t\t\tvideo_->queueBuffer(buffer);\n>>>> \n>>>> \t\tif (conversionQueue_.empty())\n>>>> @@ -940,13 +950,14 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>>> \t */\n>>>> \tif (useConversion_) {\n>>>> \t\tif (conversionQueue_.empty()) {\n>>>> -\t\t\tvideo_->queueBuffer(buffer);\n>>>> +\t\t\tif (!rawStream_)\n>>>> +\t\t\t\tvideo_->queueBuffer(buffer);\n>>>> \t\t\treturn;\n>>>> \t\t}\n>>>> \n>>>> \t\tif (converter_)\n>>>> \t\t\tconverter_->queueBuffers(buffer, conversionQueue_.front().outputs);\n>>>> -\t\telse\n>>>> +\t\telse if (processedRequested())\n>>>> \t\t\t/*\n>>>> \t\t\t * request->sequence() cannot be retrieved from `buffer' inside\n>>>> \t\t\t * queueBuffers because unique_ptr's make buffer->request() invalid\n>>>> @@ -956,6 +967,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>>> \t\t\t\t\t conversionQueue_.front().outputs);\n>>>> \n>>>> \t\tconversionQueue_.pop();\n>>>> +\t\tif (rawStream_)\n>>>> +\t\t\tpipe->completeBuffer(request, buffer);\n>>>> \t\treturn;\n>>>> \t}\n>>>> \n>>>> @@ -993,7 +1006,8 @@ void SimpleCameraData::tryCompleteRequest(Request *request)\n>>>> void SimpleCameraData::conversionInputDone(FrameBuffer *buffer)\n>>>> {\n>>>> \t/* Queue the input buffer back for capture. */\n>>>> -\tvideo_->queueBuffer(buffer);\n>>>> +\tif (!rawStream_)\n>>>> +\t\tvideo_->queueBuffer(buffer);\n>>>> }\n>>>> \n>>>> void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n>>>> @@ -1518,11 +1532,15 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n>>>> \n>>>> \tfor (unsigned int i = 0; i < config->size(); ++i) {\n>>>> \t\tStreamConfiguration &cfg = config->at(i);\n>>>> +\t\tbool rawStream = isRaw(cfg);\n>>>> \n>>>> \t\tcfg.setStream(&data->streams_[i]);\n>>>> \n>>>> -\t\tif (data->useConversion_ && !isRaw(cfg))\n>>>> +\t\tif (data->useConversion_ && !rawStream)\n>>>> \t\t\toutputCfgs.push_back(cfg);\n>>>> +\n>>>> +\t\tif (rawStream)\n>>>> +\t\t\tdata->rawStream_ = &data->streams_[i];\n>>>> \t}\n>>>> \n>>>> \tif (outputCfgs.empty())\n>>>> @@ -1553,7 +1571,7 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n>>>> \t * Export buffers on the converter or capture video node, depending on\n>>>> \t * whether the converter is used or not.\n>>>> \t */\n>>>> -\tif (data->useConversion_)\n>>>> +\tif (data->useConversion_ && stream != data->rawStream_)\n>>>> \t\treturn data->converter_\n>>>> \t\t\t ? data->converter_->exportBuffers(stream, count, buffers)\n>>>> \t\t\t : data->swIsp_->exportBuffers(stream, count, buffers);\n>>>> @@ -1576,7 +1594,7 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>>>> \t\treturn -EBUSY;\n>>>> \t}\n>>>> \n>>>> -\tif (data->useConversion_) {\n>>>> +\tif (data->useConversion_ && !data->rawStream_) {\n>>>> \t\t/*\n>>>> \t\t * When using the converter allocate a fixed number of internal\n>>>> \t\t * buffers.\n>>>> @@ -1584,8 +1602,11 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>>>> \t\tret = video->allocateBuffers(kNumInternalBuffers,\n>>>> \t\t\t\t\t &data->conversionBuffers_);\n>>>> \t} else {\n>>>> -\t\t/* Otherwise, prepare for using buffers from the only stream. */\n>>>> -\t\tStream *stream = &data->streams_[0];\n>>>> +\t\t/*\n>>>> +\t\t * Otherwise, prepare for using buffers from either the raw stream, if\n>>>> +\t\t * requested, or the only stream configured.\n>>>> +\t\t */\n>>>> +\t\tStream *stream = (data->rawStream_ ? data->rawStream_ : &data->streams_[0]);\n>>>> \t\tret = video->importBuffers(stream->configuration().bufferCount);\n>>>> \t}\n>>>> \tif (ret < 0) {\n>>>> @@ -1626,8 +1647,9 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>>>> \t\t}\n>>>> \n>>>> \t\t/* Queue all internal buffers for capture. */\n>>>> -\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n>>>> -\t\t\tvideo->queueBuffer(buffer.get());\n>>>> +\t\tif (!data->rawStream_)\n>>>> +\t\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n>>>> +\t\t\t\tvideo->queueBuffer(buffer.get());\n>>>> \t}\n>>>> \n>>>> \treturn 0;\n>>>> @@ -1678,7 +1700,7 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)\n>>>> \t\t * queue, it will be handed to the converter in the capture\n>>>> \t\t * completion handler.\n>>>> \t\t */\n>>>> -\t\tif (data->useConversion_) {\n>>>> +\t\tif (data->useConversion_ && stream != data->rawStream_) {\n>>>> \t\t\tbuffers.emplace(stream, buffer);\n>>>> \t\t\tmetadataRequired = !!data->swIsp_;\n>>>> \t\t} else {\n>>>> -- \n>>>> 2.51.0\n>>>> \n>> \n>> Umang Jain writes:\n>> \n>>> On Tue, Oct 21, 2025 at 08:27:15PM +0200, Milan Zamazal wrote:\n>>>> When a raw stream is requested, whether alone or together with a\n>>>> processed stream, its buffers must be handled outside the software ISP\n>>>\n>>>> machinery. They serve as output buffers, even when a processed stream\n>>>> is produced.\n>>>> \n>>>> At most one raw stream and at most one processed stream are supported\n>>>> and can be combined. An example of producing both raw and processed\n>>>> files using `cam' application:\n>>>> \n>>>> cam -c1 -C100 -Ffile# \\\n>>>> -s role=viewfinder,width=1920,height=1080,pixelformat=RGB888 \\\n>>>> -s role=raw,width=3280,height=2464,pixelformat=SRGGB8 \\\n>>>> \n>>>> Note the difference in viewfinder and raw stream sizes due to the fact\n>>>> that debayering requires enlarging the image width, which enforces\n>>>> selecting a larger sensor resolution in this case.\n>>>> \n>>>> In order to track whether a raw stream is requested and which one it is,\n>>>> SimpleCameraData::rawStream_ member variable is introduced.\n>>>> \n>>>> This is the final step to make raw streams working.\n>>>> \n>>>> Signed-off-by: Milan Zamazal \n>>>> ---\n>>>> src/libcamera/pipeline/simple/simple.cpp | 50 +++++++++++++++++-------\n>>>> 1 file changed, 36 insertions(+), 14 deletions(-)\n>>>> \n>>>> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n>>>> index 0c3284feb..607b07949 100644\n>>>> --- a/src/libcamera/pipeline/simple/simple.cpp\n>>>> +++ b/src/libcamera/pipeline/simple/simple.cpp\n>>>> @@ -333,6 +333,7 @@ public:\n>>>> \t};\n>>>> \n>>>> \tstd::vector streams_;\n>>>> +\tStream *rawStream_;\n>>>> \n>>>> \t/*\n>>>> \t * All entities in the pipeline, from the camera sensor to the video\n>>>> @@ -371,6 +372,11 @@ private:\n>>>> \tvoid ispStatsReady(uint32_t frame, uint32_t bufferId);\n>>>> \tvoid metadataReady(uint32_t frame, const ControlList &metadata);\n>>>> \tvoid setSensorControls(const ControlList &sensorControls);\n>>>> +\n>>>> +\tbool processedRequested() const\n>>>> +\t{\n>>>> +\t\treturn streams_.size() - (rawStream_ ? 1 : 0) > 0;\n>>>> +\t}\n>>>> };\n>>>> \n>>>> class SimpleCameraConfiguration : public CameraConfiguration\n>>>> @@ -467,7 +473,7 @@ private:\n>>>> SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,\n>>>> \t\t\t\t unsigned int numStreams,\n>>>> \t\t\t\t MediaEntity *sensor)\n>>>> -\t: Camera::Private(pipe), streams_(numStreams)\n>>>> +\t: Camera::Private(pipe), streams_(numStreams), rawStream_(nullptr)\n>>>> {\n>>>> \t/*\n>>>> \t * Find the shortest path from the camera sensor to a video capture\n>>>> @@ -878,10 +884,13 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>>> \t * point converting an erroneous buffer.\n>>>> \t */\n>>>> \tif (buffer->metadata().status != FrameMetadata::FrameSuccess) {\n>>>> -\t\tif (!useConversion_) {\n>>>> +\t\tif (rawStream_) {\n>>>> \t\t\t/* No conversion, just complete the request. */\n>>>> \t\t\tRequest *request = buffer->request();\n>>>> \t\t\tpipe->completeBuffer(request, buffer);\n>>>> +\t\t\tSimpleFrameInfo *info = frameInfo_.find(request->sequence());\n>>>> +\t\t\tif (info)\n>>>> +\t\t\t\tinfo->metadataRequired = false;\n>>>> \t\t\ttryCompleteRequest(request);\n>>>> \t\t\treturn;\n>>>> \t\t}\n>>>> @@ -891,7 +900,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>>> \t\t * buffer for capture (unless the stream is being stopped), and\n>>>> \t\t * complete the request with all the user-facing buffers.\n>>>> \t\t */\n>>>> -\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled)\n>>>> +\t\tif (buffer->metadata().status != FrameMetadata::FrameCancelled &&\n>>>> +\t\t !rawStream_)\n>>>> \t\t\tvideo_->queueBuffer(buffer);\n>>>> \n>>>> \t\tif (conversionQueue_.empty())\n>>>> @@ -940,13 +950,14 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>>> \t */\n>>>> \tif (useConversion_) {\n>>>> \t\tif (conversionQueue_.empty()) {\n>>>> -\t\t\tvideo_->queueBuffer(buffer);\n>>>> +\t\t\tif (!rawStream_)\n>>>> +\t\t\t\tvideo_->queueBuffer(buffer);\n>>>> \t\t\treturn;\n>>>> \t\t}\n>>>> \n>>>> \t\tif (converter_)\n>>>> \t\t\tconverter_->queueBuffers(buffer, conversionQueue_.front().outputs);\n>>>> -\t\telse\n>>>> +\t\telse if (processedRequested())\n>>>> \t\t\t/*\n>>>> \t\t\t * request->sequence() cannot be retrieved from `buffer' inside\n>>>> \t\t\t * queueBuffers because unique_ptr's make buffer->request() invalid\n>>>> @@ -956,6 +967,8 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)\n>>>> \t\t\t\t\t conversionQueue_.front().outputs);\n>>>> \n>>>> \t\tconversionQueue_.pop();\n>>>> +\t\tif (rawStream_)\n>>>> +\t\t\tpipe->completeBuffer(request, buffer);\n>>>> \t\treturn;\n>>>> \t}\n>>>> \n>>>> @@ -993,7 +1006,8 @@ void SimpleCameraData::tryCompleteRequest(Request *request)\n>>>> void SimpleCameraData::conversionInputDone(FrameBuffer *buffer)\n>>>> {\n>>>> \t/* Queue the input buffer back for capture. */\n>>>> -\tvideo_->queueBuffer(buffer);\n>>>> +\tif (!rawStream_)\n>>>> +\t\tvideo_->queueBuffer(buffer);\n>>>> }\n>>>> \n>>>> void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)\n>>>> @@ -1518,11 +1532,15 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n>>>> \n>>>> \tfor (unsigned int i = 0; i < config->size(); ++i) {\n>>>> \t\tStreamConfiguration &cfg = config->at(i);\n>>>> +\t\tbool rawStream = isRaw(cfg);\n>>>> \n>>>> \t\tcfg.setStream(&data->streams_[i]);\n>>>> \n>>>> -\t\tif (data->useConversion_ && !isRaw(cfg))\n>>>> +\t\tif (data->useConversion_ && !rawStream)\n>>>> \t\t\toutputCfgs.push_back(cfg);\n>>>> +\n>>>> +\t\tif (rawStream)\n>>>> +\t\t\tdata->rawStream_ = &data->streams_[i];\n>>>\n>>> Last comment, just realised:\n>>>\n>>> data->rawStream_ should be initialised to nullptr (outside this loop)\n>>> each time the camera is configured, otherwise it can lead to subtle bugs\n>>> if the camera configure() is called multiple times with varying\n>>> configurations.\n>> \n>> Right, I'll add it.\n>> \n>>>> \t}\n>>>> \n>>>> \tif (outputCfgs.empty())\n>>>> @@ -1553,7 +1571,7 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n>>>> \t * Export buffers on the converter or capture video node, depending on\n>>>> \t * whether the converter is used or not.\n>>>> \t */\n>>>> -\tif (data->useConversion_)\n>>>> +\tif (data->useConversion_ && stream != data->rawStream_)\n>>>> \t\treturn data->converter_\n>>>> \t\t\t ? data->converter_->exportBuffers(stream, count, buffers)\n>>>> \t\t\t : data->swIsp_->exportBuffers(stream, count, buffers);\n>>>> @@ -1576,7 +1594,7 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>>>> \t\treturn -EBUSY;\n>>>> \t}\n>>>> \n>>>> -\tif (data->useConversion_) {\n>>>> +\tif (data->useConversion_ && !data->rawStream_) {\n>>>> \t\t/*\n>>>> \t\t * When using the converter allocate a fixed number of internal\n>>>> \t\t * buffers.\n>>>> @@ -1584,8 +1602,11 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>>>> \t\tret = video->allocateBuffers(kNumInternalBuffers,\n>>>> \t\t\t\t\t &data->conversionBuffers_);\n>>>> \t} else {\n>>>> -\t\t/* Otherwise, prepare for using buffers from the only stream. */\n>>>> -\t\tStream *stream = &data->streams_[0];\n>>>> +\t\t/*\n>>>> +\t\t * Otherwise, prepare for using buffers from either the raw stream, if\n>>>> +\t\t * requested, or the only stream configured.\n>>>> +\t\t */\n>>>> +\t\tStream *stream = (data->rawStream_ ? data->rawStream_ : &data->streams_[0]);\n>>>> \t\tret = video->importBuffers(stream->configuration().bufferCount);\n>>>> \t}\n>>>> \tif (ret < 0) {\n>>>> @@ -1626,8 +1647,9 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL\n>>>> \t\t}\n>>>> \n>>>> \t\t/* Queue all internal buffers for capture. */\n>>>> -\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n>>>> -\t\t\tvideo->queueBuffer(buffer.get());\n>>>> +\t\tif (!data->rawStream_)\n>>>> +\t\t\tfor (std::unique_ptr &buffer : data->conversionBuffers_)\n>>>> +\t\t\t\tvideo->queueBuffer(buffer.get());\n>>>> \t}\n>>>> \n>>>> \treturn 0;\n>>>> @@ -1678,7 +1700,7 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)\n>>>> \t\t * queue, it will be handed to the converter in the capture\n>>>> \t\t * completion handler.\n>>>> \t\t */\n>>>> -\t\tif (data->useConversion_) {\n>>>> +\t\tif (data->useConversion_ && stream != data->rawStream_) {\n>>>> \t\t\tbuffers.emplace(stream, buffer);\n>>>> \t\t\tmetadataRequired = !!data->swIsp_;\n>>>> \t\t} else {\n>>>> -- \n>>>> 2.51.0\n>>>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 3BCECBDE4C\n\tfor ;\n\tMon, 3 Nov 2025 22:33:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6E7DD606A0;\n\tMon, 3 Nov 2025 23:33:09 +0100 (CET)","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 777BC606A0\n\tfor ;\n\tMon, 3 Nov 2025 23:33:07 +0100 (CET)","from mail-ed1-f72.google.com (mail-ed1-f72.google.com\n\t[209.85.208.72]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-371-sLcwW2jiN4erQUtVTdcbkg-1; Mon, 03 Nov 2025 17:33:04 -0500","by mail-ed1-f72.google.com with SMTP id\n\t4fb4d7f45d1cf-640b8087663so4008476a12.3\n\tfor ;\n\tMon, 03 Nov 2025 14:33:04 -0800 (PST)","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-640e6a7f2a5sm472485a12.34.2025.11.03.14.33.01\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 03 Nov 2025 14:33:02 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"KfV1una0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1762209186;\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=hgtIGlXpm0SHmlAObo5ryYtiklkGwvjTNuEkgRXIUT8=;\n\tb=KfV1una0NV0g7Ib/zP1xbBTPcGIuRz7awW9m5NjjqQRTo/Xnqj7EF4NcTk4LV+H0BhWhOj\n\tWrNUtFvfAnk67WylBtUVocL2BdzY8/tbYLmes2xKpfy20nGXnQJif3w1e/I9/rPpa2ockC\n\te5eqr96zraz/nUbthROkDXfRJzJbphY=","X-MC-Unique":"sLcwW2jiN4erQUtVTdcbkg-1","X-Mimecast-MFC-AGG-ID":"sLcwW2jiN4erQUtVTdcbkg_1762209184","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1762209184; x=1762813984;\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=hgtIGlXpm0SHmlAObo5ryYtiklkGwvjTNuEkgRXIUT8=;\n\tb=US2XfkrBya142GLAijXDVj4afW/ze+O42PYS77dZ2MRfhlsZj7Zh4yz5KFWXaB8+wu\n\tRxx9RGtRNla6lAy98FF32oBmN0u7lwhL86ZZdeTUdKQfDj5MO2KvWte6zyhH3ubgS/Qf\n\tpxuACoEkFtYcXbLL1VwHdBnb0hnt1M7YE6tOh7M0MxbjU3IhsapdHK0RJ4SAuDpzy42c\n\tpnd/iUzOag4ZhA1D5S2Icbrim8l/wx5559foE4dPmDU8VG98d7Y0WgEV38YHPhGEq4Kh\n\tSuG4sFrA0gVYQIFxN6WPC8VFmcG9A0uUPXRIEXPweqp56SIB+TrBCQM6Sc1XJHcbNAsj\n\tTtXQ==","X-Gm-Message-State":"AOJu0Ywrk8VCCWRTBBSZliCEGGFVJznWacvcJR7r0OpcDcaav21k02Ps\n\to1NH5R1DbMY6qeYo+wnRfO+EaMld7K8E2hyeFuZBkxKzhWU5JfyyaqO9t+b1pc3KLb04LWQXnw8\n\tPRtdzHmcjwa/215QvATVlZCAglc6DKbfmZzHNGmnD8aAaq8d5MVGNwAIVcCN+6fGFANdXE8O2y6\n\to=","X-Gm-Gg":"ASbGncsRztuDFNe+RASs8NJCPB7gjGjSbibu+ZBJkZTNt4zm2cgDhMsfkYtBh/Ij+XR\n\tYSm/XGGLnvxQZj9foumNz28vX/yUMRUtg8kFxxQ0RhKV48HFXFatblexzmlBei1Mt8ruIR+z13m\n\tOmB9mogOWWmqfcNLPXvsAfIEh71dMScgZlOwUT15bhnm6mkE5ENARPkTyD+ygAoEbMsgzs24Ovn\n\tJojuafEUU9+o4nIJPqAeQvJGNJI2llJkj4GxP6Fon/DyOcZwUd7ZKbuCP8Paeui6v9Tqlh6tvTF\n\tidtlmTJou4vdfnARIiIa9Fo+V/rSweV0HKWksL6U1x45CvHlnblhouvAfmLR4Y6yYJwlk1WVgXm\n\t2K5rzJqNDAzW5ASyNyarbfgDMmKVp3XTC1ulWp2Y3Shn5p4uM47lQ","X-Received":["by 2002:a05:6402:520f:b0:637:ee0d:383d with SMTP id\n\t4fb4d7f45d1cf-64077041327mr13490606a12.3.1762209183500; \n\tMon, 03 Nov 2025 14:33:03 -0800 (PST)","by 2002:a05:6402:520f:b0:637:ee0d:383d with SMTP id\n\t4fb4d7f45d1cf-64077041327mr13490569a12.3.1762209182927; \n\tMon, 03 Nov 2025 14:33:02 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IENdV+GscZd5smK38uvOMOL9O3f4pBHDOYns/Ud/T5FFmX0x4WZZQlQroqlONdWX1wB96fQIw==","From":"Milan Zamazal ","To":"uajain ","Cc":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t,\n\tKieran Bingham , =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?=\n\t, Paul Elder\n\t","Subject":"Re: [PATCH v13 8/8] libcamera: simple: Make raw streams working","In-Reply-To":"\n\t(uajain@igalia.com's message of \"Mon, 03 Nov 2025 17:57:49 +0000\")","References":"<20251021182716.29274-1-mzamazal@redhat.com>\n\t<20251021182716.29274-9-mzamazal@redhat.com>\n\t\n\t<855xbttuup.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t","Date":"Mon, 03 Nov 2025 23:33:00 +0100","Message-ID":"<85ldkm9277.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":"YcjZKMlZkSbEWLRSfuKX28YSCPo8us62NMSDi_ZL-yI_1762209184","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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]