{"id":4130,"url":"https://patchwork.libcamera.org/api/1.1/patches/4130/?format=json","web_url":"https://patchwork.libcamera.org/patch/4130/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20200623020930.1781469-4-niklas.soderlund@ragnatech.se>","date":"2020-06-23T02:09:23","name":"[libcamera-devel,v3,03/10] libcamera: ipu3: Breakout stream assignment to new function","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"1deca4518e75c19d9b696d063f4186190711aa94","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/1.1/people/5/?format=json","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/4130/mbox/","series":[{"id":1027,"url":"https://patchwork.libcamera.org/api/1.1/series/1027/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=1027","date":"2020-06-23T02:09:20","name":"libcamera: ipu3: Allow zero-copy RAW stream","version":3,"mbox":"https://patchwork.libcamera.org/series/1027/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/4130/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/4130/checks/","tags":{},"headers":{"Return-Path":"","Received":["from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net\n\t[195.74.38.228])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DA44B609D6\n\tfor ;\n\tTue, 23 Jun 2020 04:09:44 +0200 (CEST)","from bismarck.berto.se (p4fca2eca.dip0.t-ipconnect.de\n\t[79.202.46.202]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA\n\tid 9a9076d0-b4f6-11ea-933e-005056917a89;\n\tTue, 23 Jun 2020 04:09:43 +0200 (CEST)"],"X-Halon-ID":"9a9076d0-b4f6-11ea-933e-005056917a89","Authorized-sender":"niklas@soderlund.pp.se","From":"=?utf-8?q?Niklas_S=C3=B6derlund?= ","To":"libcamera-devel@lists.libcamera.org","Date":"Tue, 23 Jun 2020 04:09:23 +0200","Message-Id":"<20200623020930.1781469-4-niklas.soderlund@ragnatech.se>","X-Mailer":"git-send-email 2.27.0","In-Reply-To":"<20200623020930.1781469-1-niklas.soderlund@ragnatech.se>","References":"<20200623020930.1781469-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v3 03/10] libcamera: ipu3: Breakout stream\n\tassignment to new function","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","X-List-Received-Date":"Tue, 23 Jun 2020 02:09:46 -0000"},"content":"Selecting which stream is the most suitable for the requested\nconfiguration is mixed with adjusting the requested format when\nvalidating configurations. This is hard to read and got worse when\nsupport for Bayer formats was added. Break it out to a separate\nfunction.\n\nSigned-off-by: Niklas Söderlund \nReviewed-by: Laurent Pinchart \n---\n* Changes since v2\n- Document the fact that the number of requested streams in config_\n should be validated before calling assignStreams(). Add an ASSERT()\n and comment to catch the issue early if this should ever change.\n\n* Changes since v1\n- Update commit message.\n- Rename updateStreams() to assignStreams().\n- Revert and keep old comment on how streams are picked.\n- Do not modify behavior on how streams are picked which means\n assignStreams() now can't fail so no need for it to return an int,\n switch to void.\n---\n src/libcamera/pipeline/ipu3/ipu3.cpp | 88 ++++++++++++++++------------\n 1 file changed, 52 insertions(+), 36 deletions(-)","diff":"diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex c525e30a5ad35011..3b2ec684244881e5 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -191,6 +191,7 @@ private:\n \tstatic constexpr unsigned int IPU3_BUFFER_COUNT = 4;\n \tstatic constexpr unsigned int IPU3_MAX_STREAMS = 3;\n \n+\tvoid assignStreams();\n \tvoid adjustStream(StreamConfiguration &cfg, bool scale);\n \n \t/*\n@@ -257,6 +258,50 @@ IPU3CameraConfiguration::IPU3CameraConfiguration(Camera *camera,\n \tdata_ = data;\n }\n \n+void IPU3CameraConfiguration::assignStreams()\n+{\n+\t/*\n+\t * Verify and update all configuration entries, and assign a stream to\n+\t * each of them. The viewfinder stream can scale, while the output\n+\t * stream can crop only, so select the output stream when the requested\n+\t * resolution is equal to the sensor resolution, and the viewfinder\n+\t * stream otherwise.\n+\t */\n+\tstd::set availableStreams = {\n+\t\t&data_->outStream_,\n+\t\t&data_->vfStream_,\n+\t\t&data_->rawStream_,\n+\t};\n+\n+\t/*\n+\t * The caller is responsible to limit the number of requested streams\n+\t * to a number supported by the pipeline before calling this function.\n+\t */\n+\tASSERT(availableStreams.size() >= config_.size());\n+\n+\tstreams_.clear();\n+\tstreams_.reserve(config_.size());\n+\n+\tfor (const StreamConfiguration &cfg : config_) {\n+\t\tconst PixelFormatInfo &info =\n+\t\t\tPixelFormatInfo::info(cfg.pixelFormat);\n+\t\tconst IPU3Stream *stream;\n+\n+\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)\n+\t\t\tstream = &data_->rawStream_;\n+\t\telse if (cfg.size == sensorFormat_.size)\n+\t\t\tstream = &data_->outStream_;\n+\t\telse\n+\t\t\tstream = &data_->vfStream_;\n+\n+\t\tif (availableStreams.find(stream) == availableStreams.end())\n+\t\t\tstream = *availableStreams.begin();\n+\n+\t\tstreams_.push_back(stream);\n+\t\tavailableStreams.erase(stream);\n+\t}\n+}\n+\n void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale)\n {\n \t/* The only pixel format the driver supports is NV12. */\n@@ -343,41 +388,14 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n \tif (!sensorFormat_.size.width || !sensorFormat_.size.height)\n \t\tsensorFormat_.size = sensor->resolution();\n \n-\t/*\n-\t * Verify and update all configuration entries, and assign a stream to\n-\t * each of them. The viewfinder stream can scale, while the output\n-\t * stream can crop only, so select the output stream when the requested\n-\t * resolution is equal to the sensor resolution, and the viewfinder\n-\t * stream otherwise.\n-\t */\n-\tstd::set availableStreams = {\n-\t\t&data_->outStream_,\n-\t\t&data_->vfStream_,\n-\t\t&data_->rawStream_,\n-\t};\n-\n-\tstreams_.clear();\n-\tstreams_.reserve(config_.size());\n+\t/* Assign streams to each configuration entry. */\n+\tassignStreams();\n \n+\t/* Verify and adjust configuration if needed. */\n \tfor (unsigned int i = 0; i < config_.size(); ++i) {\n \t\tStreamConfiguration &cfg = config_[i];\n-\t\tconst PixelFormat pixelFormat = cfg.pixelFormat;\n-\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat);\n-\t\tconst Size size = cfg.size;\n-\t\tconst IPU3Stream *stream;\n-\n-\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW)\n-\t\t\tstream = &data_->rawStream_;\n-\t\telse if (cfg.size == sensorFormat_.size)\n-\t\t\tstream = &data_->outStream_;\n-\t\telse\n-\t\t\tstream = &data_->vfStream_;\n-\n-\t\tif (availableStreams.find(stream) == availableStreams.end())\n-\t\t\tstream = *availableStreams.begin();\n-\n-\t\tLOG(IPU3, Debug)\n-\t\t\t<< \"Assigned '\" << stream->name_ << \"' to stream \" << i;\n+\t\tconst StreamConfiguration oldCfg = cfg;\n+\t\tconst IPU3Stream *stream = streams_[i];\n \n \t\tif (stream->raw_) {\n \t\t\tconst auto &itFormat =\n@@ -394,15 +412,13 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()\n \n \t\tcfg.bufferCount = IPU3_BUFFER_COUNT;\n \n-\t\tif (cfg.pixelFormat != pixelFormat || cfg.size != size) {\n+\t\tif (cfg.pixelFormat != oldCfg.pixelFormat ||\n+\t\t cfg.size != oldCfg.size) {\n \t\t\tLOG(IPU3, Debug)\n \t\t\t\t<< \"Stream \" << i << \" configuration adjusted to \"\n \t\t\t\t<< cfg.toString();\n \t\t\tstatus = Adjusted;\n \t\t}\n-\n-\t\tstreams_.push_back(stream);\n-\t\tavailableStreams.erase(stream);\n \t}\n \n \treturn status;\n","prefixes":["libcamera-devel","v3","03/10"]}