[{"id":15286,"web_url":"https://patchwork.libcamera.org/comment/15286/","msgid":"","date":"2021-02-22T13:08:12","subject":"Re: [libcamera-devel] [PATCH 09/20] libcamera: pipeline: simple:\n\tconverter: Add multi-stream support","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 31/01/2021 22:46, Laurent Pinchart wrote:\n> While the M2M device backing the converter doesn't support multiple\n> streams natively, it can be run once per stream to produce multiple\n> outputs from the same input, with different output formats and sizes.\n\nWell, that's going to get a bit more fun ;-)\n\n> To support this, create a class to model a stream and move control of\n> the M2M device to the Stream class. The SimpleConverter class then\n> creates stream instances and iterates over them. Each stream needs its\n> own instance of the V4L2M2MDevice, to support different output\n> configurations. The SimpleConverter class retains a device instance to\n> support the query operations.\n\nThis reminds me of a discussion on linux-media, where having multiple\nopens of an M2M device gets quite difficult to distinguish between the\ndifferent opened contexts in the kernel debug messages. Each queue has a\ndifferent type, but the separate open contexts are harder to distinguish.\n\nI hope our multiple streams log outputs can be clearly separated\n(presumably with a stream index)?\n\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/pipeline/simple/converter.cpp | 294 ++++++++++++++------\n> src/libcamera/pipeline/simple/converter.h | 44 ++-\n> src/libcamera/pipeline/simple/simple.cpp | 6 +-\n> 3 files changed, 250 insertions(+), 94 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp\n> index 8324baedc198..3db162d9edb8 100644\n> --- a/src/libcamera/pipeline/simple/converter.cpp\n> +++ b/src/libcamera/pipeline/simple/converter.cpp\n> @@ -17,12 +17,157 @@\n> \n> #include \"libcamera/internal/log.h\"\n> #include \"libcamera/internal/media_device.h\"\n> +#include \"libcamera/internal/utils.h\"\n> #include \"libcamera/internal/v4l2_videodevice.h\"\n> \n> namespace libcamera {\n> \n> LOG_DECLARE_CATEGORY(SimplePipeline)\n> \n> +/* -----------------------------------------------------------------------------\n> + * SimpleConverter::Stream\n> + */\n> +\n> +SimpleConverter::Stream::Stream(SimpleConverter *converter)\n> +\t: converter_(converter)\n> +{\n> +\tm2m_ = std::make_unique(converter->deviceNode_);\n> +\n> +\tm2m_->output()->bufferReady.connect(this, &Stream::outputBufferReady);\n> +\tm2m_->capture()->bufferReady.connect(this, &Stream::captureBufferReady);\n> +\n> +\tint ret = m2m_->open();\n> +\tif (ret < 0)\n> +\t\tm2m_.reset();\n> +}\n> +\n> +int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg,\n> +\t\t\t\t const StreamConfiguration &outputCfg)\n> +{\n> +\tV4L2PixelFormat videoFormat =\n> +\t\tm2m_->output()->toV4L2PixelFormat(inputCfg.pixelFormat);\n> +\n> +\tV4L2DeviceFormat format;\n> +\tformat.fourcc = videoFormat;\n> +\tformat.size = inputCfg.size;\n> +\tformat.planesCount = 1;\n> +\tformat.planes[0].bpl = inputCfg.stride;\n> +\n> +\tint ret = m2m_->output()->setFormat(&format);\n> +\tif (ret < 0) {\n> +\t\tLOG(SimplePipeline, Error)\n> +\t\t\t<< \"Failed to set input format: \" << strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tif (format.fourcc != videoFormat || format.size != inputCfg.size ||\n> +\t format.planes[0].bpl != inputCfg.stride) {\n> +\t\tLOG(SimplePipeline, Error)\n> +\t\t\t<< \"Input format not supported\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\t/* Set the pixel format and size on the output. */\n> +\tvideoFormat = m2m_->capture()->toV4L2PixelFormat(outputCfg.pixelFormat);\n> +\tformat = {};\n> +\tformat.fourcc = videoFormat;\n> +\tformat.size = outputCfg.size;\n> +\n> +\tret = m2m_->capture()->setFormat(&format);\n> +\tif (ret < 0) {\n> +\t\tLOG(SimplePipeline, Error)\n> +\t\t\t<< \"Failed to set output format: \" << strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tif (format.fourcc != videoFormat || format.size != outputCfg.size) {\n> +\t\tLOG(SimplePipeline, Error)\n> +\t\t\t<< \"Output format not supported\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tinputBufferCount_ = inputCfg.bufferCount;\n> +\toutputBufferCount_ = outputCfg.bufferCount;\n> +\n> +\treturn 0;\n> +}\n> +\n> +int SimpleConverter::Stream::exportBuffers(unsigned int count,\n> +\t\t\t\t\t std::vector> *buffers)\n> +{\n> +\treturn m2m_->capture()->exportBuffers(count, buffers);\n> +}\n> +\n> +int SimpleConverter::Stream::start()\n> +{\n> +\tint ret = m2m_->output()->importBuffers(inputBufferCount_);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> +\tret = m2m_->capture()->importBuffers(outputBufferCount_);\n> +\tif (ret < 0) {\n> +\t\tstop();\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tret = m2m_->output()->streamOn();\n> +\tif (ret < 0) {\n> +\t\tstop();\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tret = m2m_->capture()->streamOn();\n> +\tif (ret < 0) {\n> +\t\tstop();\n> +\t\treturn ret;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +void SimpleConverter::Stream::stop()\n> +{\n> +\tm2m_->capture()->streamOff();\n> +\tm2m_->output()->streamOff();\n> +\tm2m_->capture()->releaseBuffers();\n> +\tm2m_->output()->releaseBuffers();\n> +}\n> +\n> +int SimpleConverter::Stream::queueBuffers(FrameBuffer *input,\n> +\t\t\t\t\t FrameBuffer *output)\n> +{\n> +\tint ret = m2m_->output()->queueBuffer(input);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> +\tret = m2m_->capture()->queueBuffer(output);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> +\treturn 0;\n> +}\n> +\n> +void SimpleConverter::Stream::outputBufferReady(FrameBuffer *buffer)\n> +{\n> +\tauto it = converter_->queue_.find(buffer);\n> +\tif (it == converter_->queue_.end())\n> +\t\treturn;\n> +\n> +\tif (!--it->second) {\n> +\t\tconverter_->inputBufferReady.emit(buffer);\n> +\t\tconverter_->queue_.erase(it);\n> +\t}\n> +}\n> +\n> +void SimpleConverter::Stream::captureBufferReady(FrameBuffer *buffer)\n> +{\n> +\tconverter_->outputBufferReady.emit(buffer);\n> +}\n> +\n> +/* -----------------------------------------------------------------------------\n> + * SimpleConverter\n> + */\n> +\n> SimpleConverter::SimpleConverter(MediaDevice *media)\n> {\n> \t/*\n> @@ -37,16 +182,14 @@ SimpleConverter::SimpleConverter(MediaDevice *media)\n> \tif (it == entities.end())\n> \t\treturn;\n> \n> -\tm2m_ = std::make_unique((*it)->deviceNode());\n> +\tdeviceNode_ = (*it)->deviceNode();\n> \n> +\tm2m_ = std::make_unique(deviceNode_);\n> \tint ret = m2m_->open();\n> \tif (ret < 0) {\n> \t\tm2m_.reset();\n> \t\treturn;\n> \t}\n> -\n> -\tm2m_->output()->bufferReady.connect(this, &SimpleConverter::m2mInputBufferReady);\n> -\tm2m_->capture()->bufferReady.connect(this, &SimpleConverter::m2mOutputBufferReady);\n> }\n> \n> std::vector SimpleConverter::formats(PixelFormat input)\n> @@ -141,86 +284,54 @@ SimpleConverter::strideAndFrameSize(const PixelFormat &pixelFormat,\n> }\n> \n> int SimpleConverter::configure(const StreamConfiguration &inputCfg,\n> -\t\t\t const StreamConfiguration &outputCfg)\n> +\t\t\t const std::vector> &outputCfgs)\n> {\n> -\tint ret;\n> +\tint ret = 0;\n> \n> -\tV4L2PixelFormat videoFormat =\n> -\t\tm2m_->output()->toV4L2PixelFormat(inputCfg.pixelFormat);\n> +\tstreams_.clear();\n> +\tstreams_.reserve(outputCfgs.size());\n> \n> -\tV4L2DeviceFormat format;\n> -\tformat.fourcc = videoFormat;\n> -\tformat.size = inputCfg.size;\n> -\tformat.planesCount = 1;\n> -\tformat.planes[0].bpl = inputCfg.stride;\n> +\tfor (const StreamConfiguration &outputCfg : outputCfgs) {\n> +\t\tStream &stream = streams_.emplace_back(this);\n> \n> -\tret = m2m_->output()->setFormat(&format);\n> -\tif (ret < 0) {\n> -\t\tLOG(SimplePipeline, Error)\n> -\t\t\t<< \"Failed to set input format: \" << strerror(-ret);\n> -\t\treturn ret;\n> -\t}\n> +\t\tif (!stream.isValid()) {\n> +\t\t\tLOG(SimplePipeline, Error) << \"Failed to create stream\";\n> +\t\t\tret = -EINVAL;\n> +\t\t\tbreak;\n> +\t\t}\n> \n> -\tif (format.fourcc != videoFormat || format.size != inputCfg.size ||\n> -\t format.planes[0].bpl != inputCfg.stride) {\n> -\t\tLOG(SimplePipeline, Error)\n> -\t\t\t<< \"Input format not supported\";\n> -\t\treturn -EINVAL;\n> +\t\tret = stream.configure(inputCfg, outputCfg);\n> +\t\tif (ret < 0)\n> +\t\t\tbreak;\n> \t}\n> \n> -\t/* Set the pixel format and size on the output. */\n> -\tvideoFormat = m2m_->capture()->toV4L2PixelFormat(outputCfg.pixelFormat);\n> -\tformat = {};\n> -\tformat.fourcc = videoFormat;\n> -\tformat.size = outputCfg.size;\n> -\n> -\tret = m2m_->capture()->setFormat(&format);\n> \tif (ret < 0) {\n> -\t\tLOG(SimplePipeline, Error)\n> -\t\t\t<< \"Failed to set output format: \" << strerror(-ret);\n> +\t\tstreams_.clear();\n> \t\treturn ret;\n> \t}\n> \n> -\tif (format.fourcc != videoFormat || format.size != outputCfg.size) {\n> -\t\tLOG(SimplePipeline, Error)\n> -\t\t\t<< \"Output format not supported\";\n> -\t\treturn -EINVAL;\n> -\t}\n> -\n> -\tinputBufferCount_ = inputCfg.bufferCount;\n> -\toutputBufferCount_ = outputCfg.bufferCount;\n> -\n> \treturn 0;\n> }\n> \n> -int SimpleConverter::exportBuffers(unsigned int count,\n> +int SimpleConverter::exportBuffers(unsigned int output, unsigned int count,\n> \t\t\t\t std::vector> *buffers)\n> {\n> -\treturn m2m_->capture()->exportBuffers(count, buffers);\n> +\tif (output >= streams_.size())\n> +\t\treturn -EINVAL;\n> +\n> +\treturn streams_[output].exportBuffers(count, buffers);\n> }\n> \n> int SimpleConverter::start()\n> {\n> -\tint ret = m2m_->output()->importBuffers(inputBufferCount_);\n> -\tif (ret < 0)\n> -\t\treturn ret;\n> +\tint ret;\n> \n> -\tret = m2m_->capture()->importBuffers(outputBufferCount_);\n> -\tif (ret < 0) {\n> -\t\tstop();\n> -\t\treturn ret;\n> -\t}\n> -\n> -\tret = m2m_->output()->streamOn();\n> -\tif (ret < 0) {\n> -\t\tstop();\n> -\t\treturn ret;\n> -\t}\n> -\n> -\tret = m2m_->capture()->streamOn();\n> -\tif (ret < 0) {\n> -\t\tstop();\n> -\t\treturn ret;\n> +\tfor (Stream &stream : utils::reverse(streams_)) {\n\nDo we have to start in reverse order?\nI understand stopping them in the reverse order ... but starting?\n\n\n> +\t\tret = stream.start();\n> +\t\tif (ret < 0) {\n> +\t\t\tstop();\n> +\t\t\treturn ret;\n> +\t\t}\n> \t}\n> \n> \treturn 0;\n> @@ -228,33 +339,48 @@ int SimpleConverter::start()\n> \n> void SimpleConverter::stop()\n> {\n> -\tm2m_->capture()->streamOff();\n> -\tm2m_->output()->streamOff();\n> -\tm2m_->capture()->releaseBuffers();\n> -\tm2m_->output()->releaseBuffers();\n> +\tfor (Stream &stream : utils::reverse(streams_))\n> +\t\tstream.stop();\n\nmuch nicer ;-)\n\n> }\n> \n> -int SimpleConverter::queueBuffers(FrameBuffer *input, FrameBuffer *output)\n> +int SimpleConverter::queueBuffers(FrameBuffer *input,\n> +\t\t\t\t const std::map &outputs)\n> {\n> -\tint ret = m2m_->output()->queueBuffer(input);\n> -\tif (ret < 0)\n> -\t\treturn ret;\n> +\tunsigned int mask = 0;\n> +\tint ret;\n> \n> -\tret = m2m_->capture()->queueBuffer(output);\n> -\tif (ret < 0)\n> -\t\treturn ret;\n> +\t/* Validate the outputs as a sanity check. */\n> +\tif (outputs.empty())\n> +\t\treturn -EINVAL;\n> +\n> +\tfor (auto [index, buffer] : outputs) {\n> +\t\tif (index >= streams_.size())\n> +\t\t\treturn -EINVAL;\n> +\t\tif (mask & (1 << index))\n\nI presume mask is checking to see/prevent there being multiple copies of\nthe same stream in outputs or something?\n\nBut this isn't very clear or readable - so a comment at least would help.\n\n\n> +\t\t\treturn -EINVAL;\n> +\t\tif (!buffer)\n> +\t\t\treturn -EINVAL;\n> +\n> +\t\tmask |= 1 << index;\n> +\t}\n> +\n> +\t/* Queue the input and output buffers to all the streams. */\n> +\tfor (auto [index, buffer] : outputs) {\n> +\t\tret = streams_[index].queueBuffers(input, buffer);\n> +\t\tif (ret < 0)\n> +\t\t\treturn ret;\n> +\t}\n> +\n> +\t/*\n> +\t * Add the input buffer to the queue, with the number of streams as a\n> +\t * reference count. Completion of the input buffer will be signalled by\n> +\t * the stream that releases the last reference.\n> +\t */\n> +\tqueue_.emplace(std::piecewise_construct,\n> +\t\t std::forward_as_tuple(input),\n> +\t\t std::forward_as_tuple(outputs.size()));\n> \n> \treturn 0;\n> }\n> \n> -void SimpleConverter::m2mInputBufferReady(FrameBuffer *buffer)\n> -{\n> -\tinputBufferReady.emit(buffer);\n> -}\n> -\n> -void SimpleConverter::m2mOutputBufferReady(FrameBuffer *buffer)\n> -{\n> -\toutputBufferReady.emit(buffer);\n> -}\n> -\n> } /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h\n> index 739b24df0200..176978eefe48 100644\n> --- a/src/libcamera/pipeline/simple/converter.h\n> +++ b/src/libcamera/pipeline/simple/converter.h\n> @@ -8,7 +8,10 @@\n> #ifndef __LIBCAMERA_PIPELINE_SIMPLE_CONVERTER_H__\n> #define __LIBCAMERA_PIPELINE_SIMPLE_CONVERTER_H__\n> \n> +#include \n> +#include \n> #include \n> +#include \n> #include \n> #include \n> \n> @@ -37,26 +40,53 @@ public:\n> \tstrideAndFrameSize(const PixelFormat &pixelFormat, const Size &size);\n> \n> \tint configure(const StreamConfiguration &inputCfg,\n> -\t\t const StreamConfiguration &outputCfg);\n> -\tint exportBuffers(unsigned int count,\n> +\t\t const std::vector> &outputCfg);\n> +\tint exportBuffers(unsigned int ouput, unsigned int count,\n> \t\t\t std::vector> *buffers);\n> \n> \tint start();\n> \tvoid stop();\n> \n> -\tint queueBuffers(FrameBuffer *input, FrameBuffer *output);\n> +\tint queueBuffers(FrameBuffer *input,\n> +\t\t\t const std::map &outputs);\n> \n> \tSignal inputBufferReady;\n> \tSignal outputBufferReady;\n> \n> private:\n> -\tvoid m2mInputBufferReady(FrameBuffer *buffer);\n> -\tvoid m2mOutputBufferReady(FrameBuffer *buffer);\n> +\tclass Stream\n> +\t{\n> +\tpublic:\n> +\t\tStream(SimpleConverter *converter);\n> \n> +\t\tbool isValid() const { return m2m_ != nullptr; }\n> +\n> +\t\tint configure(const StreamConfiguration &inputCfg,\n> +\t\t\t const StreamConfiguration &outputCfg);\n> +\t\tint exportBuffers(unsigned int count,\n> +\t\t\t\t std::vector> *buffers);\n> +\n> +\t\tint start();\n> +\t\tvoid stop();\n> +\n> +\t\tint queueBuffers(FrameBuffer *input, FrameBuffer *output);\n> +\n> +\tprivate:\n> +\t\tvoid captureBufferReady(FrameBuffer *buffer);\n> +\t\tvoid outputBufferReady(FrameBuffer *buffer);\n> +\n> +\t\tSimpleConverter *converter_;\n> +\t\tstd::unique_ptr m2m_;\n> +\n> +\t\tunsigned int inputBufferCount_;\n> +\t\tunsigned int outputBufferCount_;\n> +\t};\n> +\n> +\tstd::string deviceNode_;\n> \tstd::unique_ptr m2m_;\n> \n> -\tunsigned int inputBufferCount_;\n> -\tunsigned int outputBufferCount_;\n> +\tstd::vector streams_;\n> +\tstd::map queue_;\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 7f9c57234256..b7a890ab772e 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -610,7 +610,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> \t\tinputCfg.stride = captureFormat.planes[0].bpl;\n> \t\tinputCfg.bufferCount = cfg.bufferCount;\n> \n> -\t\tret = converter_->configure(inputCfg, cfg);\n> +\t\tret = converter_->configure(inputCfg, { cfg });\n> \t\tif (ret < 0) {\n> \t\t\tLOG(SimplePipeline, Error)\n> \t\t\t\t<< \"Unable to configure converter\";\n> @@ -636,7 +636,7 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n> \t * whether the converter is used or not.\n> \t */\n> \tif (useConverter_)\n> -\t\treturn converter_->exportBuffers(count, buffers);\n> +\t\treturn converter_->exportBuffers(0, count, buffers);\n\nI was wondering if it would be better to reference the Stream objects\nrather than indexes, but I can see that might become confusing with our\nStream *stream we already have ...\n\nAnd for now, we only have a single stream so index zero is enough for\nnow ...\n\nOnly a few trivial comments.\nWith those handled as you wish:\n\nReviewed-by: Kieran Bingham \n\n> \telse\n> \t\treturn data->video_->exportBuffers(count, buffers);\n> }\n> @@ -917,7 +917,7 @@ void SimplePipelineHandler::bufferReady(FrameBuffer *buffer)\n> \t\tFrameBuffer *output = converterQueue_.front();\n> \t\tconverterQueue_.pop();\n> \n> -\t\tconverter_->queueBuffers(buffer, output);\n> +\t\tconverter_->queueBuffers(buffer, { { 0, output } });\n> \t\treturn;\n> \t}\n> \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 D6C3EBD1F6\n\tfor ;\n\tMon, 22 Feb 2021 13:08:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5660168A0B;\n\tMon, 22 Feb 2021 14:08:16 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B5DBE637DA\n\tfor ;\n\tMon, 22 Feb 2021 14:08:15 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1F0B866;\n\tMon, 22 Feb 2021 14:08:15 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"K7VEfgiL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1613999295;\n\tbh=fRJKDpCHGiIN7EVAaVvRStKhcsf/N9yYbVJs6ApeNS8=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=K7VEfgiLAIRok4TeX82bz+EaVhRzUd3xYvYRk6RzvNtUPZX8HxjZ6hxzIS4pU0oM7\n\t1+x+3ZcFfy4rTifLr8jY+7l/R0DhlasoOhbffFbPzhZl09hmhSuLdLVFyCIpB1XEJg\n\t9W3Zi1hH+26QsuqFCi6nP3UsngaYTG05GspG+H58=","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210131224702.8838-1-laurent.pinchart@ideasonboard.com>\n\t<20210131224702.8838-10-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"","Date":"Mon, 22 Feb 2021 13:08:12 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20210131224702.8838-10-laurent.pinchart@ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 09/20] libcamera: pipeline: simple:\n\tconverter: Add multi-stream support","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","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"Phi-Bang Nguyen ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15363,"web_url":"https://patchwork.libcamera.org/comment/15363/","msgid":"","date":"2021-03-01T22:19:02","subject":"Re: [libcamera-devel] [PATCH 09/20] libcamera: pipeline: simple:\n\tconverter: Add multi-stream support","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Mon, Feb 22, 2021 at 01:08:12PM +0000, Kieran Bingham wrote:\n> On 31/01/2021 22:46, Laurent Pinchart wrote:\n> > While the M2M device backing the converter doesn't support multiple\n> > streams natively, it can be run once per stream to produce multiple\n> > outputs from the same input, with different output formats and sizes.\n> \n> Well, that's going to get a bit more fun ;-)\n> \n> > To support this, create a class to model a stream and move control of\n> > the M2M device to the Stream class. The SimpleConverter class then\n> > creates stream instances and iterates over them. Each stream needs its\n> > own instance of the V4L2M2MDevice, to support different output\n> > configurations. The SimpleConverter class retains a device instance to\n> > support the query operations.\n> \n> This reminds me of a discussion on linux-media, where having multiple\n> opens of an M2M device gets quite difficult to distinguish between the\n> different opened contexts in the kernel debug messages. Each queue has a\n> different type, but the separate open contexts are harder to distinguish.\n> \n> I hope our multiple streams log outputs can be clearly separated\n> (presumably with a stream index)?\n\nGood point... Seems I need to fix that :-)\n\nBeside handling this in the converter, we could print the fd in\nV4L2VideoDevice::logPrefix(), do you think that would be useful ?\n\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/libcamera/pipeline/simple/converter.cpp | 294 ++++++++++++++------\n> > src/libcamera/pipeline/simple/converter.h | 44 ++-\n> > src/libcamera/pipeline/simple/simple.cpp | 6 +-\n> > 3 files changed, 250 insertions(+), 94 deletions(-)\n> > \n> > diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp\n> > index 8324baedc198..3db162d9edb8 100644\n> > --- a/src/libcamera/pipeline/simple/converter.cpp\n> > +++ b/src/libcamera/pipeline/simple/converter.cpp\n> > @@ -17,12 +17,157 @@\n> > \n> > #include \"libcamera/internal/log.h\"\n> > #include \"libcamera/internal/media_device.h\"\n> > +#include \"libcamera/internal/utils.h\"\n> > #include \"libcamera/internal/v4l2_videodevice.h\"\n> > \n> > namespace libcamera {\n> > \n> > LOG_DECLARE_CATEGORY(SimplePipeline)\n> > \n> > +/* -----------------------------------------------------------------------------\n> > + * SimpleConverter::Stream\n> > + */\n> > +\n> > +SimpleConverter::Stream::Stream(SimpleConverter *converter)\n> > +\t: converter_(converter)\n> > +{\n> > +\tm2m_ = std::make_unique(converter->deviceNode_);\n> > +\n> > +\tm2m_->output()->bufferReady.connect(this, &Stream::outputBufferReady);\n> > +\tm2m_->capture()->bufferReady.connect(this, &Stream::captureBufferReady);\n> > +\n> > +\tint ret = m2m_->open();\n> > +\tif (ret < 0)\n> > +\t\tm2m_.reset();\n> > +}\n> > +\n> > +int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg,\n> > +\t\t\t\t const StreamConfiguration &outputCfg)\n> > +{\n> > +\tV4L2PixelFormat videoFormat =\n> > +\t\tm2m_->output()->toV4L2PixelFormat(inputCfg.pixelFormat);\n> > +\n> > +\tV4L2DeviceFormat format;\n> > +\tformat.fourcc = videoFormat;\n> > +\tformat.size = inputCfg.size;\n> > +\tformat.planesCount = 1;\n> > +\tformat.planes[0].bpl = inputCfg.stride;\n> > +\n> > +\tint ret = m2m_->output()->setFormat(&format);\n> > +\tif (ret < 0) {\n> > +\t\tLOG(SimplePipeline, Error)\n> > +\t\t\t<< \"Failed to set input format: \" << strerror(-ret);\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > +\tif (format.fourcc != videoFormat || format.size != inputCfg.size ||\n> > +\t format.planes[0].bpl != inputCfg.stride) {\n> > +\t\tLOG(SimplePipeline, Error)\n> > +\t\t\t<< \"Input format not supported\";\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > +\t/* Set the pixel format and size on the output. */\n> > +\tvideoFormat = m2m_->capture()->toV4L2PixelFormat(outputCfg.pixelFormat);\n> > +\tformat = {};\n> > +\tformat.fourcc = videoFormat;\n> > +\tformat.size = outputCfg.size;\n> > +\n> > +\tret = m2m_->capture()->setFormat(&format);\n> > +\tif (ret < 0) {\n> > +\t\tLOG(SimplePipeline, Error)\n> > +\t\t\t<< \"Failed to set output format: \" << strerror(-ret);\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > +\tif (format.fourcc != videoFormat || format.size != outputCfg.size) {\n> > +\t\tLOG(SimplePipeline, Error)\n> > +\t\t\t<< \"Output format not supported\";\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > +\tinputBufferCount_ = inputCfg.bufferCount;\n> > +\toutputBufferCount_ = outputCfg.bufferCount;\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +int SimpleConverter::Stream::exportBuffers(unsigned int count,\n> > +\t\t\t\t\t std::vector> *buffers)\n> > +{\n> > +\treturn m2m_->capture()->exportBuffers(count, buffers);\n> > +}\n> > +\n> > +int SimpleConverter::Stream::start()\n> > +{\n> > +\tint ret = m2m_->output()->importBuffers(inputBufferCount_);\n> > +\tif (ret < 0)\n> > +\t\treturn ret;\n> > +\n> > +\tret = m2m_->capture()->importBuffers(outputBufferCount_);\n> > +\tif (ret < 0) {\n> > +\t\tstop();\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > +\tret = m2m_->output()->streamOn();\n> > +\tif (ret < 0) {\n> > +\t\tstop();\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > +\tret = m2m_->capture()->streamOn();\n> > +\tif (ret < 0) {\n> > +\t\tstop();\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +void SimpleConverter::Stream::stop()\n> > +{\n> > +\tm2m_->capture()->streamOff();\n> > +\tm2m_->output()->streamOff();\n> > +\tm2m_->capture()->releaseBuffers();\n> > +\tm2m_->output()->releaseBuffers();\n> > +}\n> > +\n> > +int SimpleConverter::Stream::queueBuffers(FrameBuffer *input,\n> > +\t\t\t\t\t FrameBuffer *output)\n> > +{\n> > +\tint ret = m2m_->output()->queueBuffer(input);\n> > +\tif (ret < 0)\n> > +\t\treturn ret;\n> > +\n> > +\tret = m2m_->capture()->queueBuffer(output);\n> > +\tif (ret < 0)\n> > +\t\treturn ret;\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +void SimpleConverter::Stream::outputBufferReady(FrameBuffer *buffer)\n> > +{\n> > +\tauto it = converter_->queue_.find(buffer);\n> > +\tif (it == converter_->queue_.end())\n> > +\t\treturn;\n> > +\n> > +\tif (!--it->second) {\n> > +\t\tconverter_->inputBufferReady.emit(buffer);\n> > +\t\tconverter_->queue_.erase(it);\n> > +\t}\n> > +}\n> > +\n> > +void SimpleConverter::Stream::captureBufferReady(FrameBuffer *buffer)\n> > +{\n> > +\tconverter_->outputBufferReady.emit(buffer);\n> > +}\n> > +\n> > +/* -----------------------------------------------------------------------------\n> > + * SimpleConverter\n> > + */\n> > +\n> > SimpleConverter::SimpleConverter(MediaDevice *media)\n> > {\n> > \t/*\n> > @@ -37,16 +182,14 @@ SimpleConverter::SimpleConverter(MediaDevice *media)\n> > \tif (it == entities.end())\n> > \t\treturn;\n> > \n> > -\tm2m_ = std::make_unique((*it)->deviceNode());\n> > +\tdeviceNode_ = (*it)->deviceNode();\n> > \n> > +\tm2m_ = std::make_unique(deviceNode_);\n> > \tint ret = m2m_->open();\n> > \tif (ret < 0) {\n> > \t\tm2m_.reset();\n> > \t\treturn;\n> > \t}\n> > -\n> > -\tm2m_->output()->bufferReady.connect(this, &SimpleConverter::m2mInputBufferReady);\n> > -\tm2m_->capture()->bufferReady.connect(this, &SimpleConverter::m2mOutputBufferReady);\n> > }\n> > \n> > std::vector SimpleConverter::formats(PixelFormat input)\n> > @@ -141,86 +284,54 @@ SimpleConverter::strideAndFrameSize(const PixelFormat &pixelFormat,\n> > }\n> > \n> > int SimpleConverter::configure(const StreamConfiguration &inputCfg,\n> > -\t\t\t const StreamConfiguration &outputCfg)\n> > +\t\t\t const std::vector> &outputCfgs)\n> > {\n> > -\tint ret;\n> > +\tint ret = 0;\n> > \n> > -\tV4L2PixelFormat videoFormat =\n> > -\t\tm2m_->output()->toV4L2PixelFormat(inputCfg.pixelFormat);\n> > +\tstreams_.clear();\n> > +\tstreams_.reserve(outputCfgs.size());\n> > \n> > -\tV4L2DeviceFormat format;\n> > -\tformat.fourcc = videoFormat;\n> > -\tformat.size = inputCfg.size;\n> > -\tformat.planesCount = 1;\n> > -\tformat.planes[0].bpl = inputCfg.stride;\n> > +\tfor (const StreamConfiguration &outputCfg : outputCfgs) {\n> > +\t\tStream &stream = streams_.emplace_back(this);\n> > \n> > -\tret = m2m_->output()->setFormat(&format);\n> > -\tif (ret < 0) {\n> > -\t\tLOG(SimplePipeline, Error)\n> > -\t\t\t<< \"Failed to set input format: \" << strerror(-ret);\n> > -\t\treturn ret;\n> > -\t}\n> > +\t\tif (!stream.isValid()) {\n> > +\t\t\tLOG(SimplePipeline, Error) << \"Failed to create stream\";\n> > +\t\t\tret = -EINVAL;\n> > +\t\t\tbreak;\n> > +\t\t}\n> > \n> > -\tif (format.fourcc != videoFormat || format.size != inputCfg.size ||\n> > -\t format.planes[0].bpl != inputCfg.stride) {\n> > -\t\tLOG(SimplePipeline, Error)\n> > -\t\t\t<< \"Input format not supported\";\n> > -\t\treturn -EINVAL;\n> > +\t\tret = stream.configure(inputCfg, outputCfg);\n> > +\t\tif (ret < 0)\n> > +\t\t\tbreak;\n> > \t}\n> > \n> > -\t/* Set the pixel format and size on the output. */\n> > -\tvideoFormat = m2m_->capture()->toV4L2PixelFormat(outputCfg.pixelFormat);\n> > -\tformat = {};\n> > -\tformat.fourcc = videoFormat;\n> > -\tformat.size = outputCfg.size;\n> > -\n> > -\tret = m2m_->capture()->setFormat(&format);\n> > \tif (ret < 0) {\n> > -\t\tLOG(SimplePipeline, Error)\n> > -\t\t\t<< \"Failed to set output format: \" << strerror(-ret);\n> > +\t\tstreams_.clear();\n> > \t\treturn ret;\n> > \t}\n> > \n> > -\tif (format.fourcc != videoFormat || format.size != outputCfg.size) {\n> > -\t\tLOG(SimplePipeline, Error)\n> > -\t\t\t<< \"Output format not supported\";\n> > -\t\treturn -EINVAL;\n> > -\t}\n> > -\n> > -\tinputBufferCount_ = inputCfg.bufferCount;\n> > -\toutputBufferCount_ = outputCfg.bufferCount;\n> > -\n> > \treturn 0;\n> > }\n> > \n> > -int SimpleConverter::exportBuffers(unsigned int count,\n> > +int SimpleConverter::exportBuffers(unsigned int output, unsigned int count,\n> > \t\t\t\t std::vector> *buffers)\n> > {\n> > -\treturn m2m_->capture()->exportBuffers(count, buffers);\n> > +\tif (output >= streams_.size())\n> > +\t\treturn -EINVAL;\n> > +\n> > +\treturn streams_[output].exportBuffers(count, buffers);\n> > }\n> > \n> > int SimpleConverter::start()\n> > {\n> > -\tint ret = m2m_->output()->importBuffers(inputBufferCount_);\n> > -\tif (ret < 0)\n> > -\t\treturn ret;\n> > +\tint ret;\n> > \n> > -\tret = m2m_->capture()->importBuffers(outputBufferCount_);\n> > -\tif (ret < 0) {\n> > -\t\tstop();\n> > -\t\treturn ret;\n> > -\t}\n> > -\n> > -\tret = m2m_->output()->streamOn();\n> > -\tif (ret < 0) {\n> > -\t\tstop();\n> > -\t\treturn ret;\n> > -\t}\n> > -\n> > -\tret = m2m_->capture()->streamOn();\n> > -\tif (ret < 0) {\n> > -\t\tstop();\n> > -\t\treturn ret;\n> > +\tfor (Stream &stream : utils::reverse(streams_)) {\n> \n> Do we have to start in reverse order?\n> I understand stopping them in the reverse order ... but starting?\n\nIndeed, I'll fix that.\n\n> > +\t\tret = stream.start();\n> > +\t\tif (ret < 0) {\n> > +\t\t\tstop();\n> > +\t\t\treturn ret;\n> > +\t\t}\n> > \t}\n> > \n> > \treturn 0;\n> > @@ -228,33 +339,48 @@ int SimpleConverter::start()\n> > \n> > void SimpleConverter::stop()\n> > {\n> > -\tm2m_->capture()->streamOff();\n> > -\tm2m_->output()->streamOff();\n> > -\tm2m_->capture()->releaseBuffers();\n> > -\tm2m_->output()->releaseBuffers();\n> > +\tfor (Stream &stream : utils::reverse(streams_))\n> > +\t\tstream.stop();\n> \n> much nicer ;-)\n\nI have to confess I really like how C++ makes this possible. The price\nto pay, of course, is total madness after studying templates.\n\n> > }\n> > \n> > -int SimpleConverter::queueBuffers(FrameBuffer *input, FrameBuffer *output)\n> > +int SimpleConverter::queueBuffers(FrameBuffer *input,\n> > +\t\t\t\t const std::map &outputs)\n> > {\n> > -\tint ret = m2m_->output()->queueBuffer(input);\n> > -\tif (ret < 0)\n> > -\t\treturn ret;\n> > +\tunsigned int mask = 0;\n> > +\tint ret;\n> > \n> > -\tret = m2m_->capture()->queueBuffer(output);\n> > -\tif (ret < 0)\n> > -\t\treturn ret;\n> > +\t/* Validate the outputs as a sanity check. */\n> > +\tif (outputs.empty())\n> > +\t\treturn -EINVAL;\n> > +\n> > +\tfor (auto [index, buffer] : outputs) {\n> > +\t\tif (index >= streams_.size())\n> > +\t\t\treturn -EINVAL;\n> > +\t\tif (mask & (1 << index))\n> \n> I presume mask is checking to see/prevent there being multiple copies of\n> the same stream in outputs or something?\n> \n> But this isn't very clear or readable - so a comment at least would help.\n\nThat's correct. I'll add a comment.\n\n> > +\t\t\treturn -EINVAL;\n> > +\t\tif (!buffer)\n> > +\t\t\treturn -EINVAL;\n> > +\n> > +\t\tmask |= 1 << index;\n> > +\t}\n> > +\n> > +\t/* Queue the input and output buffers to all the streams. */\n> > +\tfor (auto [index, buffer] : outputs) {\n> > +\t\tret = streams_[index].queueBuffers(input, buffer);\n> > +\t\tif (ret < 0)\n> > +\t\t\treturn ret;\n> > +\t}\n> > +\n> > +\t/*\n> > +\t * Add the input buffer to the queue, with the number of streams as a\n> > +\t * reference count. Completion of the input buffer will be signalled by\n> > +\t * the stream that releases the last reference.\n> > +\t */\n> > +\tqueue_.emplace(std::piecewise_construct,\n> > +\t\t std::forward_as_tuple(input),\n> > +\t\t std::forward_as_tuple(outputs.size()));\n> > \n> > \treturn 0;\n> > }\n> > \n> > -void SimpleConverter::m2mInputBufferReady(FrameBuffer *buffer)\n> > -{\n> > -\tinputBufferReady.emit(buffer);\n> > -}\n> > -\n> > -void SimpleConverter::m2mOutputBufferReady(FrameBuffer *buffer)\n> > -{\n> > -\toutputBufferReady.emit(buffer);\n> > -}\n> > -\n> > } /* namespace libcamera */\n> > diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h\n> > index 739b24df0200..176978eefe48 100644\n> > --- a/src/libcamera/pipeline/simple/converter.h\n> > +++ b/src/libcamera/pipeline/simple/converter.h\n> > @@ -8,7 +8,10 @@\n> > #ifndef __LIBCAMERA_PIPELINE_SIMPLE_CONVERTER_H__\n> > #define __LIBCAMERA_PIPELINE_SIMPLE_CONVERTER_H__\n> > \n> > +#include \n> > +#include \n> > #include \n> > +#include \n> > #include \n> > #include \n> > \n> > @@ -37,26 +40,53 @@ public:\n> > \tstrideAndFrameSize(const PixelFormat &pixelFormat, const Size &size);\n> > \n> > \tint configure(const StreamConfiguration &inputCfg,\n> > -\t\t const StreamConfiguration &outputCfg);\n> > -\tint exportBuffers(unsigned int count,\n> > +\t\t const std::vector> &outputCfg);\n> > +\tint exportBuffers(unsigned int ouput, unsigned int count,\n> > \t\t\t std::vector> *buffers);\n> > \n> > \tint start();\n> > \tvoid stop();\n> > \n> > -\tint queueBuffers(FrameBuffer *input, FrameBuffer *output);\n> > +\tint queueBuffers(FrameBuffer *input,\n> > +\t\t\t const std::map &outputs);\n> > \n> > \tSignal inputBufferReady;\n> > \tSignal outputBufferReady;\n> > \n> > private:\n> > -\tvoid m2mInputBufferReady(FrameBuffer *buffer);\n> > -\tvoid m2mOutputBufferReady(FrameBuffer *buffer);\n> > +\tclass Stream\n> > +\t{\n> > +\tpublic:\n> > +\t\tStream(SimpleConverter *converter);\n> > \n> > +\t\tbool isValid() const { return m2m_ != nullptr; }\n> > +\n> > +\t\tint configure(const StreamConfiguration &inputCfg,\n> > +\t\t\t const StreamConfiguration &outputCfg);\n> > +\t\tint exportBuffers(unsigned int count,\n> > +\t\t\t\t std::vector> *buffers);\n> > +\n> > +\t\tint start();\n> > +\t\tvoid stop();\n> > +\n> > +\t\tint queueBuffers(FrameBuffer *input, FrameBuffer *output);\n> > +\n> > +\tprivate:\n> > +\t\tvoid captureBufferReady(FrameBuffer *buffer);\n> > +\t\tvoid outputBufferReady(FrameBuffer *buffer);\n> > +\n> > +\t\tSimpleConverter *converter_;\n> > +\t\tstd::unique_ptr m2m_;\n> > +\n> > +\t\tunsigned int inputBufferCount_;\n> > +\t\tunsigned int outputBufferCount_;\n> > +\t};\n> > +\n> > +\tstd::string deviceNode_;\n> > \tstd::unique_ptr m2m_;\n> > \n> > -\tunsigned int inputBufferCount_;\n> > -\tunsigned int outputBufferCount_;\n> > +\tstd::vector streams_;\n> > +\tstd::map queue_;\n> > };\n> > \n> > } /* namespace libcamera */\n> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > index 7f9c57234256..b7a890ab772e 100644\n> > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > @@ -610,7 +610,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> > \t\tinputCfg.stride = captureFormat.planes[0].bpl;\n> > \t\tinputCfg.bufferCount = cfg.bufferCount;\n> > \n> > -\t\tret = converter_->configure(inputCfg, cfg);\n> > +\t\tret = converter_->configure(inputCfg, { cfg });\n> > \t\tif (ret < 0) {\n> > \t\t\tLOG(SimplePipeline, Error)\n> > \t\t\t\t<< \"Unable to configure converter\";\n> > @@ -636,7 +636,7 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n> > \t * whether the converter is used or not.\n> > \t */\n> > \tif (useConverter_)\n> > -\t\treturn converter_->exportBuffers(count, buffers);\n> > +\t\treturn converter_->exportBuffers(0, count, buffers);\n> \n> I was wondering if it would be better to reference the Stream objects\n> rather than indexes, but I can see that might become confusing with our\n> Stream *stream we already have ...\n\nI've thought about that, but it would require exposing the Stream class.\nIf this was a public API I'd put more effort in the design :-)\n\n> And for now, we only have a single stream so index zero is enough for\n> now ...\n> \n> Only a few trivial comments.\n> With those handled as you wish:\n> \n> Reviewed-by: Kieran Bingham \n> \n> > \telse\n> > \t\treturn data->video_->exportBuffers(count, buffers);\n> > }\n> > @@ -917,7 +917,7 @@ void SimplePipelineHandler::bufferReady(FrameBuffer *buffer)\n> > \t\tFrameBuffer *output = converterQueue_.front();\n> > \t\tconverterQueue_.pop();\n> > \n> > -\t\tconverter_->queueBuffers(buffer, output);\n> > +\t\tconverter_->queueBuffers(buffer, { { 0, output } });\n> > \t\treturn;\n> > \t}\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 5D581BD1F1\n\tfor ;\n\tMon, 1 Mar 2021 22:19:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C160C68A7D;\n\tMon, 1 Mar 2021 23:19:32 +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 3D06A60521\n\tfor ;\n\tMon, 1 Mar 2021 23:19:31 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 93C44332;\n\tMon, 1 Mar 2021 23:19:30 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"u+GRYi/i\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1614637170;\n\tbh=kPsMFckbNLqJwTR4iYfmCvu/QA7taPPdSJXsnjfl+HY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=u+GRYi/iZii1WbD73aCrcysykStN6wPTSDbsIbNGmc2cGvrp1TMQ2cXmDBRZqXFUA\n\tBuqzhkNTeE4f2URrdatlHY7ZdiiYWHzFBDWmz3tvtqyjvfe582YrgDU2rf1MJ1En/Z\n\tFZFkZrEgBsr52JfGcAgbA4FlJTLKJd2roGp1EOHU=","Date":"Tue, 2 Mar 2021 00:19:02 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210131224702.8838-1-laurent.pinchart@ideasonboard.com>\n\t<20210131224702.8838-10-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 09/20] libcamera: pipeline: simple:\n\tconverter: Add multi-stream support","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","Cc":"Phi-Bang Nguyen ,\n\tlibcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15373,"web_url":"https://patchwork.libcamera.org/comment/15373/","msgid":"<20210302001120.GH3084@pyrite.rasen.tech>","date":"2021-03-02T00:11:20","subject":"Re: [libcamera-devel] [PATCH 09/20] libcamera: pipeline: simple:\n\tconverter: Add multi-stream support","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Laurent and Kieran,\n\nOn Tue, Mar 02, 2021 at 12:19:02AM +0200, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> On Mon, Feb 22, 2021 at 01:08:12PM +0000, Kieran Bingham wrote:\n> > On 31/01/2021 22:46, Laurent Pinchart wrote:\n> > > While the M2M device backing the converter doesn't support multiple\n> > > streams natively, it can be run once per stream to produce multiple\n> > > outputs from the same input, with different output formats and sizes.\n> > \n> > Well, that's going to get a bit more fun ;-)\n> > \n> > > To support this, create a class to model a stream and move control of\n> > > the M2M device to the Stream class. The SimpleConverter class then\n> > > creates stream instances and iterates over them. Each stream needs its\n> > > own instance of the V4L2M2MDevice, to support different output\n> > > configurations. The SimpleConverter class retains a device instance to\n> > > support the query operations.\n> > \n> > This reminds me of a discussion on linux-media, where having multiple\n> > opens of an M2M device gets quite difficult to distinguish between the\n> > different opened contexts in the kernel debug messages. Each queue has a\n> > different type, but the separate open contexts are harder to distinguish.\n> > \n> > I hope our multiple streams log outputs can be clearly separated\n> > (presumably with a stream index)?\n> \n> Good point... Seems I need to fix that :-)\n> \n> Beside handling this in the converter, we could print the fd in\n> V4L2VideoDevice::logPrefix(), do you think that would be useful ?\n\nOoh yeah I think that's a good idea.\n\n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > src/libcamera/pipeline/simple/converter.cpp | 294 ++++++++++++++------\n> > > src/libcamera/pipeline/simple/converter.h | 44 ++-\n> > > src/libcamera/pipeline/simple/simple.cpp | 6 +-\n> > > 3 files changed, 250 insertions(+), 94 deletions(-)\n> > > \n> > > diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp\n> > > index 8324baedc198..3db162d9edb8 100644\n> > > --- a/src/libcamera/pipeline/simple/converter.cpp\n> > > +++ b/src/libcamera/pipeline/simple/converter.cpp\n> > > @@ -17,12 +17,157 @@\n> > > \n> > > #include \"libcamera/internal/log.h\"\n> > > #include \"libcamera/internal/media_device.h\"\n> > > +#include \"libcamera/internal/utils.h\"\n> > > #include \"libcamera/internal/v4l2_videodevice.h\"\n> > > \n> > > namespace libcamera {\n> > > \n> > > LOG_DECLARE_CATEGORY(SimplePipeline)\n> > > \n> > > +/* -----------------------------------------------------------------------------\n> > > + * SimpleConverter::Stream\n> > > + */\n> > > +\n> > > +SimpleConverter::Stream::Stream(SimpleConverter *converter)\n> > > +\t: converter_(converter)\n> > > +{\n> > > +\tm2m_ = std::make_unique(converter->deviceNode_);\n> > > +\n> > > +\tm2m_->output()->bufferReady.connect(this, &Stream::outputBufferReady);\n> > > +\tm2m_->capture()->bufferReady.connect(this, &Stream::captureBufferReady);\n> > > +\n> > > +\tint ret = m2m_->open();\n> > > +\tif (ret < 0)\n> > > +\t\tm2m_.reset();\n> > > +}\n> > > +\n> > > +int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg,\n> > > +\t\t\t\t const StreamConfiguration &outputCfg)\n> > > +{\n> > > +\tV4L2PixelFormat videoFormat =\n> > > +\t\tm2m_->output()->toV4L2PixelFormat(inputCfg.pixelFormat);\n> > > +\n> > > +\tV4L2DeviceFormat format;\n> > > +\tformat.fourcc = videoFormat;\n> > > +\tformat.size = inputCfg.size;\n> > > +\tformat.planesCount = 1;\n> > > +\tformat.planes[0].bpl = inputCfg.stride;\n> > > +\n> > > +\tint ret = m2m_->output()->setFormat(&format);\n> > > +\tif (ret < 0) {\n> > > +\t\tLOG(SimplePipeline, Error)\n> > > +\t\t\t<< \"Failed to set input format: \" << strerror(-ret);\n> > > +\t\treturn ret;\n> > > +\t}\n> > > +\n> > > +\tif (format.fourcc != videoFormat || format.size != inputCfg.size ||\n> > > +\t format.planes[0].bpl != inputCfg.stride) {\n> > > +\t\tLOG(SimplePipeline, Error)\n> > > +\t\t\t<< \"Input format not supported\";\n> > > +\t\treturn -EINVAL;\n> > > +\t}\n> > > +\n> > > +\t/* Set the pixel format and size on the output. */\n> > > +\tvideoFormat = m2m_->capture()->toV4L2PixelFormat(outputCfg.pixelFormat);\n> > > +\tformat = {};\n> > > +\tformat.fourcc = videoFormat;\n> > > +\tformat.size = outputCfg.size;\n\nWhy doesn't the output need plane information?\n\n> > > +\n> > > +\tret = m2m_->capture()->setFormat(&format);\n> > > +\tif (ret < 0) {\n> > > +\t\tLOG(SimplePipeline, Error)\n> > > +\t\t\t<< \"Failed to set output format: \" << strerror(-ret);\n> > > +\t\treturn ret;\n> > > +\t}\n> > > +\n> > > +\tif (format.fourcc != videoFormat || format.size != outputCfg.size) {\n> > > +\t\tLOG(SimplePipeline, Error)\n> > > +\t\t\t<< \"Output format not supported\";\n> > > +\t\treturn -EINVAL;\n> > > +\t}\n> > > +\n> > > +\tinputBufferCount_ = inputCfg.bufferCount;\n> > > +\toutputBufferCount_ = outputCfg.bufferCount;\n> > > +\n> > > +\treturn 0;\n> > > +}\n> > > +\n> > > +int SimpleConverter::Stream::exportBuffers(unsigned int count,\n> > > +\t\t\t\t\t std::vector> *buffers)\n> > > +{\n> > > +\treturn m2m_->capture()->exportBuffers(count, buffers);\n> > > +}\n> > > +\n> > > +int SimpleConverter::Stream::start()\n> > > +{\n> > > +\tint ret = m2m_->output()->importBuffers(inputBufferCount_);\n> > > +\tif (ret < 0)\n> > > +\t\treturn ret;\n> > > +\n> > > +\tret = m2m_->capture()->importBuffers(outputBufferCount_);\n> > > +\tif (ret < 0) {\n> > > +\t\tstop();\n> > > +\t\treturn ret;\n> > > +\t}\n> > > +\n> > > +\tret = m2m_->output()->streamOn();\n> > > +\tif (ret < 0) {\n> > > +\t\tstop();\n> > > +\t\treturn ret;\n> > > +\t}\n> > > +\n> > > +\tret = m2m_->capture()->streamOn();\n> > > +\tif (ret < 0) {\n> > > +\t\tstop();\n> > > +\t\treturn ret;\n> > > +\t}\n> > > +\n> > > +\treturn 0;\n> > > +}\n> > > +\n> > > +void SimpleConverter::Stream::stop()\n> > > +{\n> > > +\tm2m_->capture()->streamOff();\n> > > +\tm2m_->output()->streamOff();\n> > > +\tm2m_->capture()->releaseBuffers();\n> > > +\tm2m_->output()->releaseBuffers();\n> > > +}\n> > > +\n> > > +int SimpleConverter::Stream::queueBuffers(FrameBuffer *input,\n> > > +\t\t\t\t\t FrameBuffer *output)\n> > > +{\n> > > +\tint ret = m2m_->output()->queueBuffer(input);\n> > > +\tif (ret < 0)\n> > > +\t\treturn ret;\n> > > +\n> > > +\tret = m2m_->capture()->queueBuffer(output);\n> > > +\tif (ret < 0)\n> > > +\t\treturn ret;\n> > > +\n> > > +\treturn 0;\n> > > +}\n> > > +\n> > > +void SimpleConverter::Stream::outputBufferReady(FrameBuffer *buffer)\n> > > +{\n> > > +\tauto it = converter_->queue_.find(buffer);\n> > > +\tif (it == converter_->queue_.end())\n> > > +\t\treturn;\n> > > +\n> > > +\tif (!--it->second) {\n> > > +\t\tconverter_->inputBufferReady.emit(buffer);\n> > > +\t\tconverter_->queue_.erase(it);\n> > > +\t}\n> > > +}\n> > > +\n> > > +void SimpleConverter::Stream::captureBufferReady(FrameBuffer *buffer)\n> > > +{\n> > > +\tconverter_->outputBufferReady.emit(buffer);\n> > > +}\n> > > +\n> > > +/* -----------------------------------------------------------------------------\n> > > + * SimpleConverter\n> > > + */\n> > > +\n> > > SimpleConverter::SimpleConverter(MediaDevice *media)\n> > > {\n> > > \t/*\n> > > @@ -37,16 +182,14 @@ SimpleConverter::SimpleConverter(MediaDevice *media)\n> > > \tif (it == entities.end())\n> > > \t\treturn;\n> > > \n> > > -\tm2m_ = std::make_unique((*it)->deviceNode());\n> > > +\tdeviceNode_ = (*it)->deviceNode();\n> > > \n> > > +\tm2m_ = std::make_unique(deviceNode_);\n> > > \tint ret = m2m_->open();\n> > > \tif (ret < 0) {\n> > > \t\tm2m_.reset();\n> > > \t\treturn;\n> > > \t}\n> > > -\n> > > -\tm2m_->output()->bufferReady.connect(this, &SimpleConverter::m2mInputBufferReady);\n> > > -\tm2m_->capture()->bufferReady.connect(this, &SimpleConverter::m2mOutputBufferReady);\n> > > }\n> > > \n> > > std::vector SimpleConverter::formats(PixelFormat input)\n> > > @@ -141,86 +284,54 @@ SimpleConverter::strideAndFrameSize(const PixelFormat &pixelFormat,\n> > > }\n> > > \n> > > int SimpleConverter::configure(const StreamConfiguration &inputCfg,\n> > > -\t\t\t const StreamConfiguration &outputCfg)\n> > > +\t\t\t const std::vector> &outputCfgs)\n> > > {\n> > > -\tint ret;\n> > > +\tint ret = 0;\n> > > \n> > > -\tV4L2PixelFormat videoFormat =\n> > > -\t\tm2m_->output()->toV4L2PixelFormat(inputCfg.pixelFormat);\n> > > +\tstreams_.clear();\n> > > +\tstreams_.reserve(outputCfgs.size());\n> > > \n> > > -\tV4L2DeviceFormat format;\n> > > -\tformat.fourcc = videoFormat;\n> > > -\tformat.size = inputCfg.size;\n> > > -\tformat.planesCount = 1;\n> > > -\tformat.planes[0].bpl = inputCfg.stride;\n> > > +\tfor (const StreamConfiguration &outputCfg : outputCfgs) {\n> > > +\t\tStream &stream = streams_.emplace_back(this);\n> > > \n> > > -\tret = m2m_->output()->setFormat(&format);\n> > > -\tif (ret < 0) {\n> > > -\t\tLOG(SimplePipeline, Error)\n> > > -\t\t\t<< \"Failed to set input format: \" << strerror(-ret);\n> > > -\t\treturn ret;\n> > > -\t}\n> > > +\t\tif (!stream.isValid()) {\n> > > +\t\t\tLOG(SimplePipeline, Error) << \"Failed to create stream\";\n> > > +\t\t\tret = -EINVAL;\n> > > +\t\t\tbreak;\n> > > +\t\t}\n> > > \n> > > -\tif (format.fourcc != videoFormat || format.size != inputCfg.size ||\n> > > -\t format.planes[0].bpl != inputCfg.stride) {\n> > > -\t\tLOG(SimplePipeline, Error)\n> > > -\t\t\t<< \"Input format not supported\";\n> > > -\t\treturn -EINVAL;\n> > > +\t\tret = stream.configure(inputCfg, outputCfg);\n> > > +\t\tif (ret < 0)\n> > > +\t\t\tbreak;\n> > > \t}\n> > > \n> > > -\t/* Set the pixel format and size on the output. */\n> > > -\tvideoFormat = m2m_->capture()->toV4L2PixelFormat(outputCfg.pixelFormat);\n> > > -\tformat = {};\n> > > -\tformat.fourcc = videoFormat;\n> > > -\tformat.size = outputCfg.size;\n> > > -\n> > > -\tret = m2m_->capture()->setFormat(&format);\n> > > \tif (ret < 0) {\n> > > -\t\tLOG(SimplePipeline, Error)\n> > > -\t\t\t<< \"Failed to set output format: \" << strerror(-ret);\n> > > +\t\tstreams_.clear();\n> > > \t\treturn ret;\n> > > \t}\n> > > \n> > > -\tif (format.fourcc != videoFormat || format.size != outputCfg.size) {\n> > > -\t\tLOG(SimplePipeline, Error)\n> > > -\t\t\t<< \"Output format not supported\";\n> > > -\t\treturn -EINVAL;\n> > > -\t}\n> > > -\n> > > -\tinputBufferCount_ = inputCfg.bufferCount;\n> > > -\toutputBufferCount_ = outputCfg.bufferCount;\n> > > -\n> > > \treturn 0;\n> > > }\n> > > \n> > > -int SimpleConverter::exportBuffers(unsigned int count,\n> > > +int SimpleConverter::exportBuffers(unsigned int output, unsigned int count,\n> > > \t\t\t\t std::vector> *buffers)\n> > > {\n> > > -\treturn m2m_->capture()->exportBuffers(count, buffers);\n> > > +\tif (output >= streams_.size())\n> > > +\t\treturn -EINVAL;\n> > > +\n> > > +\treturn streams_[output].exportBuffers(count, buffers);\n> > > }\n> > > \n> > > int SimpleConverter::start()\n> > > {\n> > > -\tint ret = m2m_->output()->importBuffers(inputBufferCount_);\n> > > -\tif (ret < 0)\n> > > -\t\treturn ret;\n> > > +\tint ret;\n> > > \n> > > -\tret = m2m_->capture()->importBuffers(outputBufferCount_);\n> > > -\tif (ret < 0) {\n> > > -\t\tstop();\n> > > -\t\treturn ret;\n> > > -\t}\n> > > -\n> > > -\tret = m2m_->output()->streamOn();\n> > > -\tif (ret < 0) {\n> > > -\t\tstop();\n> > > -\t\treturn ret;\n> > > -\t}\n> > > -\n> > > -\tret = m2m_->capture()->streamOn();\n> > > -\tif (ret < 0) {\n> > > -\t\tstop();\n> > > -\t\treturn ret;\n> > > +\tfor (Stream &stream : utils::reverse(streams_)) {\n> > \n> > Do we have to start in reverse order?\n> > I understand stopping them in the reverse order ... but starting?\n> \n> Indeed, I'll fix that.\n\nAh, I was wondering about the reasoning for this. Now I see.\n\n> > > +\t\tret = stream.start();\n> > > +\t\tif (ret < 0) {\n> > > +\t\t\tstop();\n> > > +\t\t\treturn ret;\n> > > +\t\t}\n> > > \t}\n> > > \n> > > \treturn 0;\n> > > @@ -228,33 +339,48 @@ int SimpleConverter::start()\n> > > \n> > > void SimpleConverter::stop()\n> > > {\n> > > -\tm2m_->capture()->streamOff();\n> > > -\tm2m_->output()->streamOff();\n> > > -\tm2m_->capture()->releaseBuffers();\n> > > -\tm2m_->output()->releaseBuffers();\n> > > +\tfor (Stream &stream : utils::reverse(streams_))\n> > > +\t\tstream.stop();\n> > \n> > much nicer ;-)\n> \n> I have to confess I really like how C++ makes this possible. The price\n> to pay, of course, is total madness after studying templates.\n> \n> > > }\n> > > \n> > > -int SimpleConverter::queueBuffers(FrameBuffer *input, FrameBuffer *output)\n> > > +int SimpleConverter::queueBuffers(FrameBuffer *input,\n> > > +\t\t\t\t const std::map &outputs)\n> > > {\n> > > -\tint ret = m2m_->output()->queueBuffer(input);\n> > > -\tif (ret < 0)\n> > > -\t\treturn ret;\n> > > +\tunsigned int mask = 0;\n> > > +\tint ret;\n> > > \n> > > -\tret = m2m_->capture()->queueBuffer(output);\n> > > -\tif (ret < 0)\n> > > -\t\treturn ret;\n> > > +\t/* Validate the outputs as a sanity check. */\n> > > +\tif (outputs.empty())\n> > > +\t\treturn -EINVAL;\n> > > +\n> > > +\tfor (auto [index, buffer] : outputs) {\n> > > +\t\tif (index >= streams_.size())\n> > > +\t\t\treturn -EINVAL;\n> > > +\t\tif (mask & (1 << index))\n> > \n> > I presume mask is checking to see/prevent there being multiple copies of\n> > the same stream in outputs or something?\n> > \n> > But this isn't very clear or readable - so a comment at least would help.\n> \n> That's correct. I'll add a comment.\n\nI thought it was fine, just that maybe mask could be named something\nelse. But I can't think of anything better, so maybe a comment would be\neasier.\n\n> > > +\t\t\treturn -EINVAL;\n> > > +\t\tif (!buffer)\n> > > +\t\t\treturn -EINVAL;\n> > > +\n> > > +\t\tmask |= 1 << index;\n> > > +\t}\n> > > +\n> > > +\t/* Queue the input and output buffers to all the streams. */\n> > > +\tfor (auto [index, buffer] : outputs) {\n> > > +\t\tret = streams_[index].queueBuffers(input, buffer);\n> > > +\t\tif (ret < 0)\n> > > +\t\t\treturn ret;\n> > > +\t}\n> > > +\n> > > +\t/*\n> > > +\t * Add the input buffer to the queue, with the number of streams as a\n> > > +\t * reference count. Completion of the input buffer will be signalled by\n> > > +\t * the stream that releases the last reference.\n> > > +\t */\n> > > +\tqueue_.emplace(std::piecewise_construct,\n> > > +\t\t std::forward_as_tuple(input),\n> > > +\t\t std::forward_as_tuple(outputs.size()));\n> > > \n> > > \treturn 0;\n> > > }\n> > > \n> > > -void SimpleConverter::m2mInputBufferReady(FrameBuffer *buffer)\n> > > -{\n> > > -\tinputBufferReady.emit(buffer);\n> > > -}\n> > > -\n> > > -void SimpleConverter::m2mOutputBufferReady(FrameBuffer *buffer)\n> > > -{\n> > > -\toutputBufferReady.emit(buffer);\n> > > -}\n> > > -\n> > > } /* namespace libcamera */\n> > > diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h\n> > > index 739b24df0200..176978eefe48 100644\n> > > --- a/src/libcamera/pipeline/simple/converter.h\n> > > +++ b/src/libcamera/pipeline/simple/converter.h\n> > > @@ -8,7 +8,10 @@\n> > > #ifndef __LIBCAMERA_PIPELINE_SIMPLE_CONVERTER_H__\n> > > #define __LIBCAMERA_PIPELINE_SIMPLE_CONVERTER_H__\n> > > \n> > > +#include \n> > > +#include \n> > > #include \n> > > +#include \n> > > #include \n> > > #include \n> > > \n> > > @@ -37,26 +40,53 @@ public:\n> > > \tstrideAndFrameSize(const PixelFormat &pixelFormat, const Size &size);\n> > > \n> > > \tint configure(const StreamConfiguration &inputCfg,\n> > > -\t\t const StreamConfiguration &outputCfg);\n> > > -\tint exportBuffers(unsigned int count,\n> > > +\t\t const std::vector> &outputCfg);\n> > > +\tint exportBuffers(unsigned int ouput, unsigned int count,\n> > > \t\t\t std::vector> *buffers);\n> > > \n> > > \tint start();\n> > > \tvoid stop();\n> > > \n> > > -\tint queueBuffers(FrameBuffer *input, FrameBuffer *output);\n> > > +\tint queueBuffers(FrameBuffer *input,\n> > > +\t\t\t const std::map &outputs);\n> > > \n> > > \tSignal inputBufferReady;\n> > > \tSignal outputBufferReady;\n> > > \n> > > private:\n> > > -\tvoid m2mInputBufferReady(FrameBuffer *buffer);\n> > > -\tvoid m2mOutputBufferReady(FrameBuffer *buffer);\n> > > +\tclass Stream\n> > > +\t{\n> > > +\tpublic:\n> > > +\t\tStream(SimpleConverter *converter);\n> > > \n> > > +\t\tbool isValid() const { return m2m_ != nullptr; }\n> > > +\n> > > +\t\tint configure(const StreamConfiguration &inputCfg,\n> > > +\t\t\t const StreamConfiguration &outputCfg);\n> > > +\t\tint exportBuffers(unsigned int count,\n> > > +\t\t\t\t std::vector> *buffers);\n> > > +\n> > > +\t\tint start();\n> > > +\t\tvoid stop();\n> > > +\n> > > +\t\tint queueBuffers(FrameBuffer *input, FrameBuffer *output);\n> > > +\n> > > +\tprivate:\n> > > +\t\tvoid captureBufferReady(FrameBuffer *buffer);\n> > > +\t\tvoid outputBufferReady(FrameBuffer *buffer);\n> > > +\n> > > +\t\tSimpleConverter *converter_;\n> > > +\t\tstd::unique_ptr m2m_;\n> > > +\n> > > +\t\tunsigned int inputBufferCount_;\n> > > +\t\tunsigned int outputBufferCount_;\n> > > +\t};\n> > > +\n> > > +\tstd::string deviceNode_;\n> > > \tstd::unique_ptr m2m_;\n> > > \n> > > -\tunsigned int inputBufferCount_;\n> > > -\tunsigned int outputBufferCount_;\n> > > +\tstd::vector streams_;\n> > > +\tstd::map queue_;\n> > > };\n> > > \n> > > } /* namespace libcamera */\n> > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > > index 7f9c57234256..b7a890ab772e 100644\n> > > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > > @@ -610,7 +610,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> > > \t\tinputCfg.stride = captureFormat.planes[0].bpl;\n> > > \t\tinputCfg.bufferCount = cfg.bufferCount;\n> > > \n> > > -\t\tret = converter_->configure(inputCfg, cfg);\n> > > +\t\tret = converter_->configure(inputCfg, { cfg });\n> > > \t\tif (ret < 0) {\n> > > \t\t\tLOG(SimplePipeline, Error)\n> > > \t\t\t\t<< \"Unable to configure converter\";\n> > > @@ -636,7 +636,7 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n> > > \t * whether the converter is used or not.\n> > > \t */\n> > > \tif (useConverter_)\n> > > -\t\treturn converter_->exportBuffers(count, buffers);\n> > > +\t\treturn converter_->exportBuffers(0, count, buffers);\n> > \n> > I was wondering if it would be better to reference the Stream objects\n> > rather than indexes, but I can see that might become confusing with our\n> > Stream *stream we already have ...\n> \n> I've thought about that, but it would require exposing the Stream class.\n> If this was a public API I'd put more effort in the design :-)\n\nHeh. I think indexes are fine. Due to the masking above I guess we're\nlimited to 31 streams?\n\n\nReviewed-by: Paul Elder \n\n> > And for now, we only have a single stream so index zero is enough for\n> > now ...\n> > \n> > Only a few trivial comments.\n> > With those handled as you wish:\n> > \n> > Reviewed-by: Kieran Bingham \n> > \n> > > \telse\n> > > \t\treturn data->video_->exportBuffers(count, buffers);\n> > > }\n> > > @@ -917,7 +917,7 @@ void SimplePipelineHandler::bufferReady(FrameBuffer *buffer)\n> > > \t\tFrameBuffer *output = converterQueue_.front();\n> > > \t\tconverterQueue_.pop();\n> > > \n> > > -\t\tconverter_->queueBuffers(buffer, output);\n> > > +\t\tconverter_->queueBuffers(buffer, { { 0, output } });\n> > > \t\treturn;\n> > > \t}\n> > > \n> \n> -- \n> Regards,\n> \n> Laurent Pinchart\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 77BCFBD1F1\n\tfor ;\n\tTue, 2 Mar 2021 00:11:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C251560522;\n\tTue, 2 Mar 2021 01:11:30 +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 A4BBF602E9\n\tfor ;\n\tTue, 2 Mar 2021 01:11:28 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4252045D;\n\tTue, 2 Mar 2021 01:11:25 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"vXUkNqFV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1614643888;\n\tbh=rCVfgz1C7OvmwUWQtirvQkTAYi2j8YEAT1yZj/zDkDA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=vXUkNqFVq831jTuaTAHNsIf8/4wJKZH7iZ2X16FR42flipz7gm95r3d9abGjymhdT\n\tUORSHfNRzda4KeGy4v7GRKGVZe+b4wFiDLQM4bdO2Kdn0t7YyWZA9+W4I9kyfWkUMn\n\tbn0i9skjpIN6qKL2NV6TsWmKbvthCxUK2kDgd544=","Date":"Tue, 2 Mar 2021 09:11:20 +0900","From":"paul.elder@ideasonboard.com","To":"Laurent Pinchart ","Message-ID":"<20210302001120.GH3084@pyrite.rasen.tech>","References":"<20210131224702.8838-1-laurent.pinchart@ideasonboard.com>\n\t<20210131224702.8838-10-laurent.pinchart@ideasonboard.com>\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 09/20] libcamera: pipeline: simple:\n\tconverter: Add multi-stream support","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","Cc":"Phi-Bang Nguyen ,\n\tlibcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15376,"web_url":"https://patchwork.libcamera.org/comment/15376/","msgid":"","date":"2021-03-02T00:33:07","subject":"Re: [libcamera-devel] [PATCH 09/20] libcamera: pipeline: simple:\n\tconverter: Add multi-stream support","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nOn Tue, Mar 02, 2021 at 09:11:20AM +0900, paul.elder@ideasonboard.com wrote:\n> On Tue, Mar 02, 2021 at 12:19:02AM +0200, Laurent Pinchart wrote:\n> > On Mon, Feb 22, 2021 at 01:08:12PM +0000, Kieran Bingham wrote:\n> > > On 31/01/2021 22:46, Laurent Pinchart wrote:\n> > > > While the M2M device backing the converter doesn't support multiple\n> > > > streams natively, it can be run once per stream to produce multiple\n> > > > outputs from the same input, with different output formats and sizes.\n> > > \n> > > Well, that's going to get a bit more fun ;-)\n> > > \n> > > > To support this, create a class to model a stream and move control of\n> > > > the M2M device to the Stream class. The SimpleConverter class then\n> > > > creates stream instances and iterates over them. Each stream needs its\n> > > > own instance of the V4L2M2MDevice, to support different output\n> > > > configurations. The SimpleConverter class retains a device instance to\n> > > > support the query operations.\n> > > \n> > > This reminds me of a discussion on linux-media, where having multiple\n> > > opens of an M2M device gets quite difficult to distinguish between the\n> > > different opened contexts in the kernel debug messages. Each queue has a\n> > > different type, but the separate open contexts are harder to distinguish.\n> > > \n> > > I hope our multiple streams log outputs can be clearly separated\n> > > (presumably with a stream index)?\n> > \n> > Good point... Seems I need to fix that :-)\n> > \n> > Beside handling this in the converter, we could print the fd in\n> > V4L2VideoDevice::logPrefix(), do you think that would be useful ?\n> \n> Ooh yeah I think that's a good idea.\n> \n> > > > Signed-off-by: Laurent Pinchart \n> > > > ---\n> > > > src/libcamera/pipeline/simple/converter.cpp | 294 ++++++++++++++------\n> > > > src/libcamera/pipeline/simple/converter.h | 44 ++-\n> > > > src/libcamera/pipeline/simple/simple.cpp | 6 +-\n> > > > 3 files changed, 250 insertions(+), 94 deletions(-)\n> > > > \n> > > > diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp\n> > > > index 8324baedc198..3db162d9edb8 100644\n> > > > --- a/src/libcamera/pipeline/simple/converter.cpp\n> > > > +++ b/src/libcamera/pipeline/simple/converter.cpp\n> > > > @@ -17,12 +17,157 @@\n> > > > \n> > > > #include \"libcamera/internal/log.h\"\n> > > > #include \"libcamera/internal/media_device.h\"\n> > > > +#include \"libcamera/internal/utils.h\"\n> > > > #include \"libcamera/internal/v4l2_videodevice.h\"\n> > > > \n> > > > namespace libcamera {\n> > > > \n> > > > LOG_DECLARE_CATEGORY(SimplePipeline)\n> > > > \n> > > > +/* -----------------------------------------------------------------------------\n> > > > + * SimpleConverter::Stream\n> > > > + */\n> > > > +\n> > > > +SimpleConverter::Stream::Stream(SimpleConverter *converter)\n> > > > +\t: converter_(converter)\n> > > > +{\n> > > > +\tm2m_ = std::make_unique(converter->deviceNode_);\n> > > > +\n> > > > +\tm2m_->output()->bufferReady.connect(this, &Stream::outputBufferReady);\n> > > > +\tm2m_->capture()->bufferReady.connect(this, &Stream::captureBufferReady);\n> > > > +\n> > > > +\tint ret = m2m_->open();\n> > > > +\tif (ret < 0)\n> > > > +\t\tm2m_.reset();\n> > > > +}\n> > > > +\n> > > > +int SimpleConverter::Stream::configure(const StreamConfiguration &inputCfg,\n> > > > +\t\t\t\t const StreamConfiguration &outputCfg)\n> > > > +{\n> > > > +\tV4L2PixelFormat videoFormat =\n> > > > +\t\tm2m_->output()->toV4L2PixelFormat(inputCfg.pixelFormat);\n> > > > +\n> > > > +\tV4L2DeviceFormat format;\n> > > > +\tformat.fourcc = videoFormat;\n> > > > +\tformat.size = inputCfg.size;\n> > > > +\tformat.planesCount = 1;\n> > > > +\tformat.planes[0].bpl = inputCfg.stride;\n> > > > +\n> > > > +\tint ret = m2m_->output()->setFormat(&format);\n> > > > +\tif (ret < 0) {\n> > > > +\t\tLOG(SimplePipeline, Error)\n> > > > +\t\t\t<< \"Failed to set input format: \" << strerror(-ret);\n> > > > +\t\treturn ret;\n> > > > +\t}\n> > > > +\n> > > > +\tif (format.fourcc != videoFormat || format.size != inputCfg.size ||\n> > > > +\t format.planes[0].bpl != inputCfg.stride) {\n> > > > +\t\tLOG(SimplePipeline, Error)\n> > > > +\t\t\t<< \"Input format not supported\";\n> > > > +\t\treturn -EINVAL;\n> > > > +\t}\n> > > > +\n> > > > +\t/* Set the pixel format and size on the output. */\n> > > > +\tvideoFormat = m2m_->capture()->toV4L2PixelFormat(outputCfg.pixelFormat);\n> > > > +\tformat = {};\n> > > > +\tformat.fourcc = videoFormat;\n> > > > +\tformat.size = outputCfg.size;\n> \n> Why doesn't the output need plane information?\n\nsetFormat() will set the number of planes, stride and frame size.\nYou're right though, we should fix this, but it will require validate()\nto also honour stride and frame size, which it doesn't today. We can fix\nthis on top.\n\n> > > > +\n> > > > +\tret = m2m_->capture()->setFormat(&format);\n> > > > +\tif (ret < 0) {\n> > > > +\t\tLOG(SimplePipeline, Error)\n> > > > +\t\t\t<< \"Failed to set output format: \" << strerror(-ret);\n> > > > +\t\treturn ret;\n> > > > +\t}\n> > > > +\n> > > > +\tif (format.fourcc != videoFormat || format.size != outputCfg.size) {\n> > > > +\t\tLOG(SimplePipeline, Error)\n> > > > +\t\t\t<< \"Output format not supported\";\n> > > > +\t\treturn -EINVAL;\n> > > > +\t}\n> > > > +\n> > > > +\tinputBufferCount_ = inputCfg.bufferCount;\n> > > > +\toutputBufferCount_ = outputCfg.bufferCount;\n> > > > +\n> > > > +\treturn 0;\n> > > > +}\n> > > > +\n> > > > +int SimpleConverter::Stream::exportBuffers(unsigned int count,\n> > > > +\t\t\t\t\t std::vector> *buffers)\n> > > > +{\n> > > > +\treturn m2m_->capture()->exportBuffers(count, buffers);\n> > > > +}\n> > > > +\n> > > > +int SimpleConverter::Stream::start()\n> > > > +{\n> > > > +\tint ret = m2m_->output()->importBuffers(inputBufferCount_);\n> > > > +\tif (ret < 0)\n> > > > +\t\treturn ret;\n> > > > +\n> > > > +\tret = m2m_->capture()->importBuffers(outputBufferCount_);\n> > > > +\tif (ret < 0) {\n> > > > +\t\tstop();\n> > > > +\t\treturn ret;\n> > > > +\t}\n> > > > +\n> > > > +\tret = m2m_->output()->streamOn();\n> > > > +\tif (ret < 0) {\n> > > > +\t\tstop();\n> > > > +\t\treturn ret;\n> > > > +\t}\n> > > > +\n> > > > +\tret = m2m_->capture()->streamOn();\n> > > > +\tif (ret < 0) {\n> > > > +\t\tstop();\n> > > > +\t\treturn ret;\n> > > > +\t}\n> > > > +\n> > > > +\treturn 0;\n> > > > +}\n> > > > +\n> > > > +void SimpleConverter::Stream::stop()\n> > > > +{\n> > > > +\tm2m_->capture()->streamOff();\n> > > > +\tm2m_->output()->streamOff();\n> > > > +\tm2m_->capture()->releaseBuffers();\n> > > > +\tm2m_->output()->releaseBuffers();\n> > > > +}\n> > > > +\n> > > > +int SimpleConverter::Stream::queueBuffers(FrameBuffer *input,\n> > > > +\t\t\t\t\t FrameBuffer *output)\n> > > > +{\n> > > > +\tint ret = m2m_->output()->queueBuffer(input);\n> > > > +\tif (ret < 0)\n> > > > +\t\treturn ret;\n> > > > +\n> > > > +\tret = m2m_->capture()->queueBuffer(output);\n> > > > +\tif (ret < 0)\n> > > > +\t\treturn ret;\n> > > > +\n> > > > +\treturn 0;\n> > > > +}\n> > > > +\n> > > > +void SimpleConverter::Stream::outputBufferReady(FrameBuffer *buffer)\n> > > > +{\n> > > > +\tauto it = converter_->queue_.find(buffer);\n> > > > +\tif (it == converter_->queue_.end())\n> > > > +\t\treturn;\n> > > > +\n> > > > +\tif (!--it->second) {\n> > > > +\t\tconverter_->inputBufferReady.emit(buffer);\n> > > > +\t\tconverter_->queue_.erase(it);\n> > > > +\t}\n> > > > +}\n> > > > +\n> > > > +void SimpleConverter::Stream::captureBufferReady(FrameBuffer *buffer)\n> > > > +{\n> > > > +\tconverter_->outputBufferReady.emit(buffer);\n> > > > +}\n> > > > +\n> > > > +/* -----------------------------------------------------------------------------\n> > > > + * SimpleConverter\n> > > > + */\n> > > > +\n> > > > SimpleConverter::SimpleConverter(MediaDevice *media)\n> > > > {\n> > > > \t/*\n> > > > @@ -37,16 +182,14 @@ SimpleConverter::SimpleConverter(MediaDevice *media)\n> > > > \tif (it == entities.end())\n> > > > \t\treturn;\n> > > > \n> > > > -\tm2m_ = std::make_unique((*it)->deviceNode());\n> > > > +\tdeviceNode_ = (*it)->deviceNode();\n> > > > \n> > > > +\tm2m_ = std::make_unique(deviceNode_);\n> > > > \tint ret = m2m_->open();\n> > > > \tif (ret < 0) {\n> > > > \t\tm2m_.reset();\n> > > > \t\treturn;\n> > > > \t}\n> > > > -\n> > > > -\tm2m_->output()->bufferReady.connect(this, &SimpleConverter::m2mInputBufferReady);\n> > > > -\tm2m_->capture()->bufferReady.connect(this, &SimpleConverter::m2mOutputBufferReady);\n> > > > }\n> > > > \n> > > > std::vector SimpleConverter::formats(PixelFormat input)\n> > > > @@ -141,86 +284,54 @@ SimpleConverter::strideAndFrameSize(const PixelFormat &pixelFormat,\n> > > > }\n> > > > \n> > > > int SimpleConverter::configure(const StreamConfiguration &inputCfg,\n> > > > -\t\t\t const StreamConfiguration &outputCfg)\n> > > > +\t\t\t const std::vector> &outputCfgs)\n> > > > {\n> > > > -\tint ret;\n> > > > +\tint ret = 0;\n> > > > \n> > > > -\tV4L2PixelFormat videoFormat =\n> > > > -\t\tm2m_->output()->toV4L2PixelFormat(inputCfg.pixelFormat);\n> > > > +\tstreams_.clear();\n> > > > +\tstreams_.reserve(outputCfgs.size());\n> > > > \n> > > > -\tV4L2DeviceFormat format;\n> > > > -\tformat.fourcc = videoFormat;\n> > > > -\tformat.size = inputCfg.size;\n> > > > -\tformat.planesCount = 1;\n> > > > -\tformat.planes[0].bpl = inputCfg.stride;\n> > > > +\tfor (const StreamConfiguration &outputCfg : outputCfgs) {\n> > > > +\t\tStream &stream = streams_.emplace_back(this);\n> > > > \n> > > > -\tret = m2m_->output()->setFormat(&format);\n> > > > -\tif (ret < 0) {\n> > > > -\t\tLOG(SimplePipeline, Error)\n> > > > -\t\t\t<< \"Failed to set input format: \" << strerror(-ret);\n> > > > -\t\treturn ret;\n> > > > -\t}\n> > > > +\t\tif (!stream.isValid()) {\n> > > > +\t\t\tLOG(SimplePipeline, Error) << \"Failed to create stream\";\n> > > > +\t\t\tret = -EINVAL;\n> > > > +\t\t\tbreak;\n> > > > +\t\t}\n> > > > \n> > > > -\tif (format.fourcc != videoFormat || format.size != inputCfg.size ||\n> > > > -\t format.planes[0].bpl != inputCfg.stride) {\n> > > > -\t\tLOG(SimplePipeline, Error)\n> > > > -\t\t\t<< \"Input format not supported\";\n> > > > -\t\treturn -EINVAL;\n> > > > +\t\tret = stream.configure(inputCfg, outputCfg);\n> > > > +\t\tif (ret < 0)\n> > > > +\t\t\tbreak;\n> > > > \t}\n> > > > \n> > > > -\t/* Set the pixel format and size on the output. */\n> > > > -\tvideoFormat = m2m_->capture()->toV4L2PixelFormat(outputCfg.pixelFormat);\n> > > > -\tformat = {};\n> > > > -\tformat.fourcc = videoFormat;\n> > > > -\tformat.size = outputCfg.size;\n> > > > -\n> > > > -\tret = m2m_->capture()->setFormat(&format);\n> > > > \tif (ret < 0) {\n> > > > -\t\tLOG(SimplePipeline, Error)\n> > > > -\t\t\t<< \"Failed to set output format: \" << strerror(-ret);\n> > > > +\t\tstreams_.clear();\n> > > > \t\treturn ret;\n> > > > \t}\n> > > > \n> > > > -\tif (format.fourcc != videoFormat || format.size != outputCfg.size) {\n> > > > -\t\tLOG(SimplePipeline, Error)\n> > > > -\t\t\t<< \"Output format not supported\";\n> > > > -\t\treturn -EINVAL;\n> > > > -\t}\n> > > > -\n> > > > -\tinputBufferCount_ = inputCfg.bufferCount;\n> > > > -\toutputBufferCount_ = outputCfg.bufferCount;\n> > > > -\n> > > > \treturn 0;\n> > > > }\n> > > > \n> > > > -int SimpleConverter::exportBuffers(unsigned int count,\n> > > > +int SimpleConverter::exportBuffers(unsigned int output, unsigned int count,\n> > > > \t\t\t\t std::vector> *buffers)\n> > > > {\n> > > > -\treturn m2m_->capture()->exportBuffers(count, buffers);\n> > > > +\tif (output >= streams_.size())\n> > > > +\t\treturn -EINVAL;\n> > > > +\n> > > > +\treturn streams_[output].exportBuffers(count, buffers);\n> > > > }\n> > > > \n> > > > int SimpleConverter::start()\n> > > > {\n> > > > -\tint ret = m2m_->output()->importBuffers(inputBufferCount_);\n> > > > -\tif (ret < 0)\n> > > > -\t\treturn ret;\n> > > > +\tint ret;\n> > > > \n> > > > -\tret = m2m_->capture()->importBuffers(outputBufferCount_);\n> > > > -\tif (ret < 0) {\n> > > > -\t\tstop();\n> > > > -\t\treturn ret;\n> > > > -\t}\n> > > > -\n> > > > -\tret = m2m_->output()->streamOn();\n> > > > -\tif (ret < 0) {\n> > > > -\t\tstop();\n> > > > -\t\treturn ret;\n> > > > -\t}\n> > > > -\n> > > > -\tret = m2m_->capture()->streamOn();\n> > > > -\tif (ret < 0) {\n> > > > -\t\tstop();\n> > > > -\t\treturn ret;\n> > > > +\tfor (Stream &stream : utils::reverse(streams_)) {\n> > > \n> > > Do we have to start in reverse order?\n> > > I understand stopping them in the reverse order ... but starting?\n> > \n> > Indeed, I'll fix that.\n> \n> Ah, I was wondering about the reasoning for this. Now I see.\n> \n> > > > +\t\tret = stream.start();\n> > > > +\t\tif (ret < 0) {\n> > > > +\t\t\tstop();\n> > > > +\t\t\treturn ret;\n> > > > +\t\t}\n> > > > \t}\n> > > > \n> > > > \treturn 0;\n> > > > @@ -228,33 +339,48 @@ int SimpleConverter::start()\n> > > > \n> > > > void SimpleConverter::stop()\n> > > > {\n> > > > -\tm2m_->capture()->streamOff();\n> > > > -\tm2m_->output()->streamOff();\n> > > > -\tm2m_->capture()->releaseBuffers();\n> > > > -\tm2m_->output()->releaseBuffers();\n> > > > +\tfor (Stream &stream : utils::reverse(streams_))\n> > > > +\t\tstream.stop();\n> > > \n> > > much nicer ;-)\n> > \n> > I have to confess I really like how C++ makes this possible. The price\n> > to pay, of course, is total madness after studying templates.\n> > \n> > > > }\n> > > > \n> > > > -int SimpleConverter::queueBuffers(FrameBuffer *input, FrameBuffer *output)\n> > > > +int SimpleConverter::queueBuffers(FrameBuffer *input,\n> > > > +\t\t\t\t const std::map &outputs)\n> > > > {\n> > > > -\tint ret = m2m_->output()->queueBuffer(input);\n> > > > -\tif (ret < 0)\n> > > > -\t\treturn ret;\n> > > > +\tunsigned int mask = 0;\n> > > > +\tint ret;\n> > > > \n> > > > -\tret = m2m_->capture()->queueBuffer(output);\n> > > > -\tif (ret < 0)\n> > > > -\t\treturn ret;\n> > > > +\t/* Validate the outputs as a sanity check. */\n> > > > +\tif (outputs.empty())\n> > > > +\t\treturn -EINVAL;\n> > > > +\n> > > > +\tfor (auto [index, buffer] : outputs) {\n> > > > +\t\tif (index >= streams_.size())\n> > > > +\t\t\treturn -EINVAL;\n> > > > +\t\tif (mask & (1 << index))\n> > > \n> > > I presume mask is checking to see/prevent there being multiple copies of\n> > > the same stream in outputs or something?\n> > > \n> > > But this isn't very clear or readable - so a comment at least would help.\n> > \n> > That's correct. I'll add a comment.\n> \n> I thought it was fine, just that maybe mask could be named something\n> else. But I can't think of anything better, so maybe a comment would be\n> easier.\n> \n> > > > +\t\t\treturn -EINVAL;\n> > > > +\t\tif (!buffer)\n> > > > +\t\t\treturn -EINVAL;\n> > > > +\n> > > > +\t\tmask |= 1 << index;\n> > > > +\t}\n> > > > +\n> > > > +\t/* Queue the input and output buffers to all the streams. */\n> > > > +\tfor (auto [index, buffer] : outputs) {\n> > > > +\t\tret = streams_[index].queueBuffers(input, buffer);\n> > > > +\t\tif (ret < 0)\n> > > > +\t\t\treturn ret;\n> > > > +\t}\n> > > > +\n> > > > +\t/*\n> > > > +\t * Add the input buffer to the queue, with the number of streams as a\n> > > > +\t * reference count. Completion of the input buffer will be signalled by\n> > > > +\t * the stream that releases the last reference.\n> > > > +\t */\n> > > > +\tqueue_.emplace(std::piecewise_construct,\n> > > > +\t\t std::forward_as_tuple(input),\n> > > > +\t\t std::forward_as_tuple(outputs.size()));\n> > > > \n> > > > \treturn 0;\n> > > > }\n> > > > \n> > > > -void SimpleConverter::m2mInputBufferReady(FrameBuffer *buffer)\n> > > > -{\n> > > > -\tinputBufferReady.emit(buffer);\n> > > > -}\n> > > > -\n> > > > -void SimpleConverter::m2mOutputBufferReady(FrameBuffer *buffer)\n> > > > -{\n> > > > -\toutputBufferReady.emit(buffer);\n> > > > -}\n> > > > -\n> > > > } /* namespace libcamera */\n> > > > diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h\n> > > > index 739b24df0200..176978eefe48 100644\n> > > > --- a/src/libcamera/pipeline/simple/converter.h\n> > > > +++ b/src/libcamera/pipeline/simple/converter.h\n> > > > @@ -8,7 +8,10 @@\n> > > > #ifndef __LIBCAMERA_PIPELINE_SIMPLE_CONVERTER_H__\n> > > > #define __LIBCAMERA_PIPELINE_SIMPLE_CONVERTER_H__\n> > > > \n> > > > +#include \n> > > > +#include \n> > > > #include \n> > > > +#include \n> > > > #include \n> > > > #include \n> > > > \n> > > > @@ -37,26 +40,53 @@ public:\n> > > > \tstrideAndFrameSize(const PixelFormat &pixelFormat, const Size &size);\n> > > > \n> > > > \tint configure(const StreamConfiguration &inputCfg,\n> > > > -\t\t const StreamConfiguration &outputCfg);\n> > > > -\tint exportBuffers(unsigned int count,\n> > > > +\t\t const std::vector> &outputCfg);\n> > > > +\tint exportBuffers(unsigned int ouput, unsigned int count,\n> > > > \t\t\t std::vector> *buffers);\n> > > > \n> > > > \tint start();\n> > > > \tvoid stop();\n> > > > \n> > > > -\tint queueBuffers(FrameBuffer *input, FrameBuffer *output);\n> > > > +\tint queueBuffers(FrameBuffer *input,\n> > > > +\t\t\t const std::map &outputs);\n> > > > \n> > > > \tSignal inputBufferReady;\n> > > > \tSignal outputBufferReady;\n> > > > \n> > > > private:\n> > > > -\tvoid m2mInputBufferReady(FrameBuffer *buffer);\n> > > > -\tvoid m2mOutputBufferReady(FrameBuffer *buffer);\n> > > > +\tclass Stream\n> > > > +\t{\n> > > > +\tpublic:\n> > > > +\t\tStream(SimpleConverter *converter);\n> > > > \n> > > > +\t\tbool isValid() const { return m2m_ != nullptr; }\n> > > > +\n> > > > +\t\tint configure(const StreamConfiguration &inputCfg,\n> > > > +\t\t\t const StreamConfiguration &outputCfg);\n> > > > +\t\tint exportBuffers(unsigned int count,\n> > > > +\t\t\t\t std::vector> *buffers);\n> > > > +\n> > > > +\t\tint start();\n> > > > +\t\tvoid stop();\n> > > > +\n> > > > +\t\tint queueBuffers(FrameBuffer *input, FrameBuffer *output);\n> > > > +\n> > > > +\tprivate:\n> > > > +\t\tvoid captureBufferReady(FrameBuffer *buffer);\n> > > > +\t\tvoid outputBufferReady(FrameBuffer *buffer);\n> > > > +\n> > > > +\t\tSimpleConverter *converter_;\n> > > > +\t\tstd::unique_ptr m2m_;\n> > > > +\n> > > > +\t\tunsigned int inputBufferCount_;\n> > > > +\t\tunsigned int outputBufferCount_;\n> > > > +\t};\n> > > > +\n> > > > +\tstd::string deviceNode_;\n> > > > \tstd::unique_ptr m2m_;\n> > > > \n> > > > -\tunsigned int inputBufferCount_;\n> > > > -\tunsigned int outputBufferCount_;\n> > > > +\tstd::vector streams_;\n> > > > +\tstd::map queue_;\n> > > > };\n> > > > \n> > > > } /* namespace libcamera */\n> > > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> > > > index 7f9c57234256..b7a890ab772e 100644\n> > > > --- a/src/libcamera/pipeline/simple/simple.cpp\n> > > > +++ b/src/libcamera/pipeline/simple/simple.cpp\n> > > > @@ -610,7 +610,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> > > > \t\tinputCfg.stride = captureFormat.planes[0].bpl;\n> > > > \t\tinputCfg.bufferCount = cfg.bufferCount;\n> > > > \n> > > > -\t\tret = converter_->configure(inputCfg, cfg);\n> > > > +\t\tret = converter_->configure(inputCfg, { cfg });\n> > > > \t\tif (ret < 0) {\n> > > > \t\t\tLOG(SimplePipeline, Error)\n> > > > \t\t\t\t<< \"Unable to configure converter\";\n> > > > @@ -636,7 +636,7 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n> > > > \t * whether the converter is used or not.\n> > > > \t */\n> > > > \tif (useConverter_)\n> > > > -\t\treturn converter_->exportBuffers(count, buffers);\n> > > > +\t\treturn converter_->exportBuffers(0, count, buffers);\n> > > \n> > > I was wondering if it would be better to reference the Stream objects\n> > > rather than indexes, but I can see that might become confusing with our\n> > > Stream *stream we already have ...\n> > \n> > I've thought about that, but it would require exposing the Stream class.\n> > If this was a public API I'd put more effort in the design :-)\n> \n> Heh. I think indexes are fine. Due to the masking above I guess we're\n> limited to 31 streams?\n\nI'd be surprised if a converter had enough bandwidth to run more than 32\npasses within a single frame interval :-)\n\n> Reviewed-by: Paul Elder \n> \n> > > And for now, we only have a single stream so index zero is enough for\n> > > now ...\n> > > \n> > > Only a few trivial comments.\n> > > With those handled as you wish:\n> > > \n> > > Reviewed-by: Kieran Bingham \n> > > \n> > > > \telse\n> > > > \t\treturn data->video_->exportBuffers(count, buffers);\n> > > > }\n> > > > @@ -917,7 +917,7 @@ void SimplePipelineHandler::bufferReady(FrameBuffer *buffer)\n> > > > \t\tFrameBuffer *output = converterQueue_.front();\n> > > > \t\tconverterQueue_.pop();\n> > > > \n> > > > -\t\tconverter_->queueBuffers(buffer, output);\n> > > > +\t\tconverter_->queueBuffers(buffer, { { 0, output } });\n> > > > \t\treturn;\n> > > > \t}\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 15782BD808\n\tfor ;\n\tTue, 2 Mar 2021 00:33:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D43E468A8F;\n\tTue, 2 Mar 2021 01:33:36 +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 F3741602E9\n\tfor ;\n\tTue, 2 Mar 2021 01:33:35 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 734EA45D;\n\tTue, 2 Mar 2021 01:33:35 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"nQniGybZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1614645215;\n\tbh=h2ENcFzofJpUCNJPc/PVD6e+Sd63J9qAUo69dkQuipc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nQniGybZhB+tyyPX1m2XdIQs6FiWnqFa+2gMcqx90eSSrreCuCBi+jxCiWwlZKLxL\n\twN7BMBUBbpu7+VAdEu37ZfnJaTju5Yn0NvNQE+Ly1BB2+C/+7P7I1rR6hNye+TJkZO\n\tXI+I5fDwTm3IHE10x7SK1mT2Qi0stTM8LrOvz0xY=","Date":"Tue, 2 Mar 2021 02:33:07 +0200","From":"Laurent Pinchart ","To":"paul.elder@ideasonboard.com","Message-ID":"","References":"<20210131224702.8838-1-laurent.pinchart@ideasonboard.com>\n\t<20210131224702.8838-10-laurent.pinchart@ideasonboard.com>\n\t\n\t\n\t<20210302001120.GH3084@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210302001120.GH3084@pyrite.rasen.tech>","Subject":"Re: [libcamera-devel] [PATCH 09/20] libcamera: pipeline: simple:\n\tconverter: Add multi-stream support","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","Cc":"Phi-Bang Nguyen ,\n\tlibcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15399,"web_url":"https://patchwork.libcamera.org/comment/15399/","msgid":"<7bff1c2d-acfe-c1ac-15da-780ee7e6be5b@ideasonboard.com>","date":"2021-03-02T10:12:31","subject":"Re: [libcamera-devel] [PATCH 09/20] libcamera: pipeline: simple:\n\tconverter: Add multi-stream support","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 02/03/2021 00:33, Laurent Pinchart wrote:\n>>>> I was wondering if it would be better to reference the Stream objects\n>>>> rather than indexes, but I can see that might become confusing with our\n>>>> Stream *stream we already have ...\n>>>\n>>> I've thought about that, but it would require exposing the Stream class.\n>>> If this was a public API I'd put more effort in the design :-)\n>>\n>> Heh. I think indexes are fine. Due to the masking above I guess we're\n>> limited to 31 streams?\n> \n> I'd be surprised if a converter had enough bandwidth to run more than 32\n> passes within a single frame interval :-)\n\nGiven that there is now an internal detail here restricting the number\nof streams, even if we're unlikely to create that many, should there be\nan assert if someone tries to go over that amount?\n\nWe're unlikely to get this high, so it's optional - but you never know\nwhat some crazy person might do ;-)\n\nPerhaps it could also catch bugs if someone is adding streams without\ncare to remove or some other category of bug though.","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 6796DBD808\n\tfor ;\n\tTue, 2 Mar 2021 10:12:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DFC3D68A95;\n\tTue, 2 Mar 2021 11:12:36 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6929B60106\n\tfor ;\n\tTue, 2 Mar 2021 11:12:35 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9A3D045D;\n\tTue, 2 Mar 2021 11:12:34 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"bW8I4Mpd\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1614679954;\n\tbh=B+O99C+eiomQAqzLihalZDH1gOuEoOzDKvjjGY2Y4oQ=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=bW8I4MpdH+JmeKdgyhZ+JuQ/EywxULB/dRT0Dg+W9qto+zCDpr8MSTobzRkLGZCyD\n\tqGiFDNiRf54DV6R1YE510owWQVYKVcfVZRC14lNgy7FLrURCKRMLgWEkMVoVRbuIxD\n\tfDcA/obsxZwYnxnSzerFpgdXgj0KEv5p1ZADdaAk=","To":"Laurent Pinchart ,\n\tpaul.elder@ideasonboard.com","References":"<20210131224702.8838-1-laurent.pinchart@ideasonboard.com>\n\t<20210131224702.8838-10-laurent.pinchart@ideasonboard.com>\n\t\n\t\n\t<20210302001120.GH3084@pyrite.rasen.tech>\n\t","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<7bff1c2d-acfe-c1ac-15da-780ee7e6be5b@ideasonboard.com>","Date":"Tue, 2 Mar 2021 10:12:31 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 09/20] libcamera: pipeline: simple:\n\tconverter: Add multi-stream support","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","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"Phi-Bang Nguyen ,\n\tlibcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15402,"web_url":"https://patchwork.libcamera.org/comment/15402/","msgid":"","date":"2021-03-02T10:16:12","subject":"Re: [libcamera-devel] [PATCH 09/20] libcamera: pipeline: simple:\n\tconverter: Add multi-stream support","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Tue, Mar 02, 2021 at 10:12:31AM +0000, Kieran Bingham wrote:\n> On 02/03/2021 00:33, Laurent Pinchart wrote:\n> >>>> I was wondering if it would be better to reference the Stream objects\n> >>>> rather than indexes, but I can see that might become confusing with our\n> >>>> Stream *stream we already have ...\n> >>>\n> >>> I've thought about that, but it would require exposing the Stream class.\n> >>> If this was a public API I'd put more effort in the design :-)\n> >>\n> >> Heh. I think indexes are fine. Due to the masking above I guess we're\n> >> limited to 31 streams?\n> > \n> > I'd be surprised if a converter had enough bandwidth to run more than 32\n> > passes within a single frame interval :-)\n> \n> Given that there is now an internal detail here restricting the number\n> of streams, even if we're unlikely to create that many, should there be\n> an assert if someone tries to go over that amount?\n> \n> We're unlikely to get this high, so it's optional - but you never know\n> what some crazy person might do ;-)\n> \n> Perhaps it could also catch bugs if someone is adding streams without\n> care to remove or some other category of bug though.\n\nI think I'd classify this in the category of bugs that really can never\nhappen. You can quote me on this in the future ;-)","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 07EF4BD1F1\n\tfor ;\n\tTue, 2 Mar 2021 10:16:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 91E8468A91;\n\tTue, 2 Mar 2021 11:16:43 +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 9A7D560106\n\tfor ;\n\tTue, 2 Mar 2021 11:16:41 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EA0EE45D;\n\tTue, 2 Mar 2021 11:16:40 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"kobfcDXe\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1614680201;\n\tbh=h/xabVstshXvtLqcwUtux307bximyIzQh/Iui00W05M=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kobfcDXeZG+5gfIRcjWvdlSaLo/zgaSEVjF5RMfgbJcq96mrM7VdNwFRHRSBMPByv\n\tFdaoYL/D2GABPBq2EoTsHi4oFRYvE65+ktG22LFY7TzVb2/bMsKNtw7hYMfeYCALiw\n\tMpqX71la+Xw1N4JZF0Yv4rK8+5KL5DM0VhpkP324=","Date":"Tue, 2 Mar 2021 12:16:12 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210131224702.8838-1-laurent.pinchart@ideasonboard.com>\n\t<20210131224702.8838-10-laurent.pinchart@ideasonboard.com>\n\t\n\t\n\t<20210302001120.GH3084@pyrite.rasen.tech>\n\t\n\t<7bff1c2d-acfe-c1ac-15da-780ee7e6be5b@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<7bff1c2d-acfe-c1ac-15da-780ee7e6be5b@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 09/20] libcamera: pipeline: simple:\n\tconverter: Add multi-stream support","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","Cc":"Phi-Bang Nguyen ,\n\tlibcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15410,"web_url":"https://patchwork.libcamera.org/comment/15410/","msgid":"","date":"2021-03-02T10:47:05","subject":"Re: [libcamera-devel] [PATCH 09/20] libcamera: pipeline: simple:\n\tconverter: Add multi-stream support","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 02/03/2021 10:16, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> On Tue, Mar 02, 2021 at 10:12:31AM +0000, Kieran Bingham wrote:\n>> On 02/03/2021 00:33, Laurent Pinchart wrote:\n>>>>>> I was wondering if it would be better to reference the Stream objects\n>>>>>> rather than indexes, but I can see that might become confusing with our\n>>>>>> Stream *stream we already have ...\n>>>>>\n>>>>> I've thought about that, but it would require exposing the Stream class.\n>>>>> If this was a public API I'd put more effort in the design :-)\n>>>>\n>>>> Heh. I think indexes are fine. Due to the masking above I guess we're\n>>>> limited to 31 streams?\n>>>\n>>> I'd be surprised if a converter had enough bandwidth to run more than 32\n>>> passes within a single frame interval :-)\n>>\n>> Given that there is now an internal detail here restricting the number\n>> of streams, even if we're unlikely to create that many, should there be\n>> an assert if someone tries to go over that amount?\n>>\n>> We're unlikely to get this high, so it's optional - but you never know\n>> what some crazy person might do ;-)\n>>\n>> Perhaps it could also catch bugs if someone is adding streams without\n>> care to remove or some other category of bug though.\n> \n> I think I'd classify this in the category of bugs that really can never\n> happen. You can quote me on this in the future ;-)\n\nI look forward to that ;-)","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 5025CBD1F1\n\tfor ;\n\tTue, 2 Mar 2021 10:47:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1D3F068AA0;\n\tTue, 2 Mar 2021 11:47:10 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C04E668A92\n\tfor ;\n\tTue, 2 Mar 2021 11:47:08 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4EC8A45D;\n\tTue, 2 Mar 2021 11:47:08 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"p02HSGAZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1614682028;\n\tbh=60W9Qgl/OSel3rTf1mOAdiRg0II3CNvGSM4MuhMEXZU=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=p02HSGAZxOgF9BFIZ8h+aH914FuQIFDDMXC5YpN13lb8DxZqgm8GZGZBVRkGbbW79\n\tB17f5h1Z6u5/JVs1jhmCQ8WDyptAzBFIPBA3KoudPumKLACiz1T/hXCjB63Q+HQHcy\n\tKgHY6Qf1Rpu/LEqAE9ceZYbt/zkZihmQx//Ov9ro=","To":"Laurent Pinchart ","References":"<20210131224702.8838-1-laurent.pinchart@ideasonboard.com>\n\t<20210131224702.8838-10-laurent.pinchart@ideasonboard.com>\n\t\n\t\n\t<20210302001120.GH3084@pyrite.rasen.tech>\n\t\n\t<7bff1c2d-acfe-c1ac-15da-780ee7e6be5b@ideasonboard.com>\n\t","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"","Date":"Tue, 2 Mar 2021 10:47:05 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 09/20] libcamera: pipeline: simple:\n\tconverter: Add multi-stream support","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","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"Phi-Bang Nguyen ,\n\tlibcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]