{"id":8433,"url":"https://patchwork.libcamera.org/api/1.1/patches/8433/?format=json","web_url":"https://patchwork.libcamera.org/patch/8433/","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":"<20200625223900.1282164-9-niklas.soderlund@ragnatech.se>","date":"2020-06-25T22:38:59","name":"[libcamera-devel,v4,8/9] libcamera: ipu3: cio2: Hide buffer allocation and freeing from users","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"ac74a3ead5b5b590c819b406e120b2dc01f6536d","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/8433/mbox/","series":[{"id":1042,"url":"https://patchwork.libcamera.org/api/1.1/series/1042/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=1042","date":"2020-06-25T22:38:51","name":"libcamera: ipu3: Allow zero-copy RAW stream","version":4,"mbox":"https://patchwork.libcamera.org/series/1042/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/8433/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/8433/checks/","tags":{},"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 1315AC2E67\n\tfor ;\n\tThu, 25 Jun 2020 22:39:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A67F760BEA;\n\tFri, 26 Jun 2020 00:39:26 +0200 (CEST)","from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net\n\t[195.74.38.227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DFDB760AF5\n\tfor ;\n\tFri, 26 Jun 2020 00:39:24 +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 b606c2c9-b734-11ea-933e-005056917a89;\n\tFri, 26 Jun 2020 00:39:21 +0200 (CEST)"],"X-Halon-ID":"b606c2c9-b734-11ea-933e-005056917a89","Authorized-sender":"niklas@soderlund.pp.se","From":"=?utf-8?q?Niklas_S=C3=B6derlund?= ","To":"libcamera-devel@lists.libcamera.org","Date":"Fri, 26 Jun 2020 00:38:59 +0200","Message-Id":"<20200625223900.1282164-9-niklas.soderlund@ragnatech.se>","X-Mailer":"git-send-email 2.27.0","In-Reply-To":"<20200625223900.1282164-1-niklas.soderlund@ragnatech.se>","References":"<20200625223900.1282164-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Subject":"[libcamera-devel] [PATCH v4 8/9] libcamera: ipu3: cio2: Hide buffer\n\tallocation and freeing from users","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","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"The allocation and freeing of buffers for the CIO2 is handled in the\nIPU3 pipeline handlers start() and stop() functions. These functions\nalso call CIO2Device start() and stop() at the appropriate times so\nmove the CIO2 buffer allocation/freeing inside the CIO2Device and reduce\nthe complexity of the exposed interface.\n\nSigned-off-by: Niklas Söderlund \nReviewed-by: Jacopo Mondi \n---\n* Changes since v3\n- Keep CIO2Device::freeBuffers() but make it private.\n\n* Changes since v2\n- Stop IMGU before CIO2\n\n* Changes since v1\n- Fix potential leaking of buffers if start fails.\n---\n src/libcamera/pipeline/ipu3/cio2.cpp | 62 +++++++++++++---------------\n src/libcamera/pipeline/ipu3/cio2.h | 4 +-\n src/libcamera/pipeline/ipu3/ipu3.cpp | 14 ++-----\n 3 files changed, 33 insertions(+), 47 deletions(-)","diff":"diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\nindex 972787a83dede203..d698cd80cba212a1 100644\n--- a/src/libcamera/pipeline/ipu3/cio2.cpp\n+++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n@@ -207,44 +207,12 @@ CIO2Device::generateConfiguration(Size size) const\n \treturn cfg;\n }\n \n-/**\n- * \\brief Allocate frame buffers for the CIO2 output\n- *\n- * Allocate frame buffers in the CIO2 video device to be used to capture frames\n- * from the CIO2 output. The buffers are stored in the CIO2Device::buffers_\n- * vector.\n- *\n- * \\return Number of buffers allocated or negative error code\n- */\n-int CIO2Device::allocateBuffers()\n-{\n-\tint ret = output_->allocateBuffers(CIO2_BUFFER_COUNT, &buffers_);\n-\tif (ret < 0)\n-\t\treturn ret;\n-\n-\tfor (std::unique_ptr &buffer : buffers_)\n-\t\tavailableBuffers_.push(buffer.get());\n-\n-\treturn ret;\n-}\n-\n int CIO2Device::exportBuffers(unsigned int count,\n \t\t\t std::vector> *buffers)\n {\n \treturn output_->exportBuffers(count, buffers);\n }\n \n-void CIO2Device::freeBuffers()\n-{\n-\t/* The default std::queue constructor is explicit with gcc 5 and 6. */\n-\tavailableBuffers_ = std::queue{};\n-\n-\tbuffers_.clear();\n-\n-\tif (output_->releaseBuffers())\n-\t\tLOG(IPU3, Error) << \"Failed to release CIO2 buffers\";\n-}\n-\n FrameBuffer *CIO2Device::getBuffer()\n {\n \tif (availableBuffers_.empty()) {\n@@ -266,12 +234,27 @@ void CIO2Device::putBuffer(FrameBuffer *buffer)\n \n int CIO2Device::start()\n {\n-\treturn output_->streamOn();\n+\tint ret = output_->allocateBuffers(CIO2_BUFFER_COUNT, &buffers_);\n+\tif (ret < 0)\n+\t\treturn ret;\n+\n+\tfor (std::unique_ptr &buffer : buffers_)\n+\t\tavailableBuffers_.push(buffer.get());\n+\n+\tret = output_->streamOn();\n+\tif (ret)\n+\t\tfreeBuffers();\n+\n+\treturn ret;\n }\n \n int CIO2Device::stop()\n {\n-\treturn output_->streamOff();\n+\tint ret = output_->streamOff();\n+\n+\tfreeBuffers();\n+\n+\treturn ret;\n }\n \n int CIO2Device::queueBuffer(FrameBuffer *buffer)\n@@ -279,6 +262,17 @@ int CIO2Device::queueBuffer(FrameBuffer *buffer)\n \treturn output_->queueBuffer(buffer);\n }\n \n+void CIO2Device::freeBuffers()\n+{\n+\t/* The default std::queue constructor is explicit with gcc 5 and 6. */\n+\tavailableBuffers_ = std::queue{};\n+\n+\tbuffers_.clear();\n+\n+\tif (output_->releaseBuffers())\n+\t\tLOG(IPU3, Error) << \"Failed to release CIO2 buffers\";\n+}\n+\n void CIO2Device::cio2BufferReady(FrameBuffer *buffer)\n {\n \tbufferReady.emit(buffer);\ndiff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h\nindex 8cb90c9da7f19ecb..47c6f010eae6a64d 100644\n--- a/src/libcamera/pipeline/ipu3/cio2.h\n+++ b/src/libcamera/pipeline/ipu3/cio2.h\n@@ -37,10 +37,8 @@ public:\n \n \tStreamConfiguration generateConfiguration(Size size) const;\n \n-\tint allocateBuffers();\n \tint exportBuffers(unsigned int count,\n \t\t\t std::vector> *buffers);\n-\tvoid freeBuffers();\n \n \tFrameBuffer *getBuffer();\n \tvoid putBuffer(FrameBuffer *buffer);\n@@ -54,6 +52,8 @@ public:\n \tSignal bufferReady;\n \n private:\n+\tvoid freeBuffers();\n+\n \tvoid cio2BufferReady(FrameBuffer *buffer);\n \n \tCameraSensor *sensor_;\ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex 2d1ec707ea4b08fe..4818027e8db1f7a3 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -658,15 +658,10 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,\n int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n {\n \tIPU3CameraData *data = cameraData(camera);\n-\tCIO2Device *cio2 = &data->cio2_;\n \tImgUDevice *imgu = data->imgu_;\n \tunsigned int bufferCount;\n \tint ret;\n \n-\tret = cio2->allocateBuffers();\n-\tif (ret < 0)\n-\t\treturn ret;\n-\n \tbufferCount = std::max({\n \t\tdata->outStream_.configuration().bufferCount,\n \t\tdata->vfStream_.configuration().bufferCount,\n@@ -674,10 +669,8 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n \t});\n \n \tret = imgu->allocateBuffers(data, bufferCount);\n-\tif (ret < 0) {\n-\t\tcio2->freeBuffers();\n+\tif (ret < 0)\n \t\treturn ret;\n-\t}\n \n \treturn 0;\n }\n@@ -686,7 +679,6 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera)\n {\n \tIPU3CameraData *data = cameraData(camera);\n \n-\tdata->cio2_.freeBuffers();\n \tdata->imgu_->freeBuffers(data);\n \n \treturn 0;\n@@ -731,10 +723,10 @@ error:\n void PipelineHandlerIPU3::stop(Camera *camera)\n {\n \tIPU3CameraData *data = cameraData(camera);\n-\tint ret;\n+\tint ret = 0;\n \n-\tret = data->cio2_.stop();\n \tret |= data->imgu_->stop();\n+\tret |= data->cio2_.stop();\n \tif (ret)\n \t\tLOG(IPU3, Warning) << \"Failed to stop camera \"\n \t\t\t\t << camera->name();\n","prefixes":["libcamera-devel","v4","8/9"]}