[{"id":33176,"web_url":"https://patchwork.libcamera.org/comment/33176/","msgid":"<20250126200125.GA1133@pendragon.ideasonboard.com>","date":"2025-01-26T20:01:25","subject":"Re: [RFC PATCH v2 02/13] libcamera: simple: Increase the default\n\tnumber of streams to 2","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Milan,\n\nThank you for the patch.\n\nOn Fri, Jan 24, 2025 at 10:57:53PM +0100, Milan Zamazal wrote:\n> In order to allow providing a raw stream together with a processed\n> stream, we need two streams. If only one stream is eventually\n> requested, it doesn't harm to allocate two.\n> \n> This is a hack for the lack of a better easy option. The number of\n> streams is determined as a camera property in the pipeline matching.\n> The actual number of streams needed (one or two) is determined only when\n> examining roles in SimplePipelineHandler::generateConfiguration.\n> \n> It's not obvious what to do better about the number of the streams. It\n> seems that hardware pipelines utilize separate hardware streams while\n> with software ISP we have only a single input for multiple outputs. A\n> fixed number of streams also doesn't scale but is good enough for our\n> current use case, which is supporting a single processed + a single raw\n> stream, not more.\n\nYou also need to consider the case where the software ISP isn't enabled.\nThe camera won't be able to produce multiple streams in that case\n(assuming there's no hardware converter), so creating the camera with\ntwo streams is wrong.\n\nThe simple pipeline handler assumes there's a linear pipeline from the\ncamera sensor to a video capture device, and only supports a single\nstream. Branches in the hardware pipeline that would allow capturing\nmultiple streams from the same camera sensor are not supported. We have\nno plan to change that, as a device that can produce multiple streams\nwill likely be better supported by a dedicated pipeline handler.\n\nIn addition to the capture pipeline, the simple pipeline handler has\nsupported hardware converters that operate in a memory to memory\nfashion. The allows producing multiple streams by running the converter\nmultiple times on the same captured image. When a converter is available\nwe don't support capturing the stream produced by the capture pipeline.\nThat's an artificial restriction to simplify the implementation,\nconceptually we could think of it as a \"raw\" stream and expose it to\napplications. It will most likely not have a raw format though, as none\nof the converters support raw inputs, but it could deliver an additional\nstream at no cost.\n\nThe software ISP is similar to a converter. We could run it multiple\ntimes to produce multiple processed streams (at an obvious cost), and we\ncan also expose the images at the output of the hardware capture\npipeline as an additional stream (which, unlike for converters, is\nguaranteed to be a raw stream as the software ISP supports raw inputs\nonly).\n\nGiven those similarities between converters and the software ISP, I\nwould like to see a common abstraction being developed. The simple\npipeline handler shouldn't care about whether the captured frames are\nprocessed with a hardware converter or with the software ISP. That will\ntake more work, and I'm fine with a step by step approach. I would\nhowever like those steps to keep the final goal in mind.\n\nFor this specific patch, it means that the number of streams should\nremain one when no software ISP is present. Something along these lines\ncould do:\n\ndiff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\nindex 8ac24e6e3423..c8d801636c62 100644\n--- a/src/libcamera/pipeline/simple/simple.cpp\n+++ b/src/libcamera/pipeline/simple/simple.cpp\n@@ -1573,7 +1573,16 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n \t\t}\n \t}\n \n-\tswIspEnabled_ = info->swIspEnabled;\n+\tif (info->swIspEnabled) {\n+\t\t/*\n+\t\t * When the software ISP is enabled, the simple pipeline handler\n+\t\t * exposes the raw stream, giving a total of two streams. This\n+\t\t * is mutally exclusive with the presence of a converter.\n+\t\t */\n+\t\tASSERT(!converter_);\n+\t\tnumStreams = 2;\n+\t\tswIspEnabled_ = true;\n+\t}\n \n \t/* Locate the sensors. */\n \tstd::vector sensors = locateSensors(media);\n\n> Signed-off-by: Milan Zamazal \n> ---\n> src/libcamera/pipeline/simple/simple.cpp | 7 ++++++-\n> 1 file changed, 6 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 8ac24e6e..6e9bc630 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -1549,7 +1549,12 @@ int SimplePipelineHandler::resetRoutingTable(V4L2Subdevice *subdev)\n> bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)\n> {\n> \tconst SimplePipelineInfo *info = nullptr;\n> -\tunsigned int numStreams = 1;\n> +\t/*\n> +\t * Let's allocate two streams, for processed + raw streams.\n> +\t * It's OK if there is only a single stream.\n> +\t * TODO: This should be more flexible.\n\nWe use\n\n\t * \\todo This should be more flexible.\n\n'\\todo' is a doxygen tag. It won't get parsed here as the comment block\ndoesn't start with '/**', but we try to standardize on '\\todo' to\nfacilitate grepping.\n\n> +\t */\n> +\tunsigned int numStreams = 2;\n> \tMediaDevice *media;\n> \n> \tfor (const SimplePipelineInfo &inf : supportedDevices) {","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 BE124BEFBE\n\tfor ;\n\tSun, 26 Jan 2025 20:01:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B24496855D;\n\tSun, 26 Jan 2025 21:01:40 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4385F68506\n\tfor ;\n\tSun, 26 Jan 2025 21:01:38 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 04751BC0;\n\tSun, 26 Jan 2025 21:00:31 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"t6mbhIDJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1737921632;\n\tbh=tp4PEMVTYHM+ZI8hKuFimF8+ZsvA+xuc67uFcZWB6D4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=t6mbhIDJuBfWzSlYYw6ay1WSAGS8krZwilVmWbZ6oVOMhuA8oUvP5n7WlV2bmMgFY\n\tDBPIRQ0ZMK1VFNqKQ7tWFxq2svYqWCCb1PFXkZcetweWPgHF7CubVaAEqA8fr2RyCB\n\tY8bKaaqso/mf+2Iqtb60mEv3b+84Oa8bNag+LNUU=","Date":"Sun, 26 Jan 2025 22:01:25 +0200","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v2 02/13] libcamera: simple: Increase the default\n\tnumber of streams to 2","Message-ID":"<20250126200125.GA1133@pendragon.ideasonboard.com>","References":"<20250124215806.158024-1-mzamazal@redhat.com>\n\t<20250124215806.158024-3-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250124215806.158024-3-mzamazal@redhat.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]