From patchwork Tue Jun 23 02:09:29 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 4136 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 83483603BD for ; Tue, 23 Jun 2020 04:09:49 +0200 (CEST) X-Halon-ID: 9d630613-b4f6-11ea-933e-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2eca.dip0.t-ipconnect.de [79.202.46.202]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id 9d630613-b4f6-11ea-933e-005056917a89; Tue, 23 Jun 2020 04:09:48 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Tue, 23 Jun 2020 04:09:29 +0200 Message-Id: <20200623020930.1781469-10-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 Subject: [libcamera-devel] [PATCH v3 09/10] libcamera: ipu3: cio2: Hide buffer allocation and freeing from users X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 Jun 2020 02:09:49 -0000 The allocation and freeing of buffers for the CIO2 is handled in the IPU3 pipeline handlers start() and stop() functions. These functions also call CIO2Device start() and stop() at the appropriate times so move the CIO2 buffer allocation/freeing inside the CIO2Device and reduce the complexity of the exposed interface. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi --- * Changes since v2 - Stop IMGU before CIO2 * Changes since v1 - Fix potential leaking of buffers if start fails. --- src/libcamera/pipeline/ipu3/cio2.cpp | 63 +++++++++++++--------------- src/libcamera/pipeline/ipu3/cio2.h | 2 - src/libcamera/pipeline/ipu3/ipu3.cpp | 14 ++----- 3 files changed, 32 insertions(+), 47 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp index 3d7348782b0fa6cb..efaa460b246697a6 100644 --- a/src/libcamera/pipeline/ipu3/cio2.cpp +++ b/src/libcamera/pipeline/ipu3/cio2.cpp @@ -207,44 +207,12 @@ CIO2Device::generateConfiguration(const Size &desiredSize) const return cfg; } -/** - * \brief Allocate frame buffers for the CIO2 output - * - * Allocate frame buffers in the CIO2 video device to be used to capture frames - * from the CIO2 output. The buffers are stored in the CIO2Device::buffers_ - * vector. - * - * \return Number of buffers allocated or negative error code - */ -int CIO2Device::allocateBuffers() -{ - int ret = output_->allocateBuffers(CIO2_BUFFER_COUNT, &buffers_); - if (ret < 0) - return ret; - - for (std::unique_ptr &buffer : buffers_) - availableBuffers_.push(buffer.get()); - - return ret; -} - int CIO2Device::exportBuffers(unsigned int count, std::vector> *buffers) { return output_->exportBuffers(count, buffers); } -void CIO2Device::freeBuffers() -{ - /* The default std::queue constructor is explicit with gcc 5 and 6. */ - availableBuffers_ = std::queue{}; - - buffers_.clear(); - - if (output_->releaseBuffers()) - LOG(IPU3, Error) << "Failed to release CIO2 buffers"; -} - FrameBuffer *CIO2Device::getBuffer() { if (availableBuffers_.empty()) { @@ -266,12 +234,39 @@ void CIO2Device::putBuffer(FrameBuffer *buffer) int CIO2Device::start() { - return output_->streamOn(); + int ret = output_->allocateBuffers(CIO2_BUFFER_COUNT, &buffers_); + if (ret < 0) + return ret; + + for (std::unique_ptr &buffer : buffers_) + availableBuffers_.push(buffer.get()); + + ret = output_->streamOn(); + if (ret) { + /* The default std::queue constructor is explicit with gcc 5 and 6. */ + availableBuffers_ = std::queue{}; + + buffers_.clear(); + + output_->releaseBuffers(); + } + + return ret; } int CIO2Device::stop() { - return output_->streamOff(); + int ret = output_->streamOff(); + + /* The default std::queue constructor is explicit with gcc 5 and 6. */ + availableBuffers_ = std::queue{}; + + buffers_.clear(); + + if (output_->releaseBuffers()) + LOG(IPU3, Error) << "Failed to release CIO2 buffers"; + + return ret; } int CIO2Device::queueBuffer(FrameBuffer *buffer) diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h index cc898f13fd73f865..405e6c03755367c4 100644 --- a/src/libcamera/pipeline/ipu3/cio2.h +++ b/src/libcamera/pipeline/ipu3/cio2.h @@ -37,10 +37,8 @@ public: StreamConfiguration generateConfiguration(const Size &desiredSize) const; - int allocateBuffers(); int exportBuffers(unsigned int count, std::vector> *buffers); - void freeBuffers(); FrameBuffer *getBuffer(); void putBuffer(FrameBuffer *buffer); diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 2d1ec707ea4b08fe..4818027e8db1f7a3 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -658,15 +658,10 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream, int PipelineHandlerIPU3::allocateBuffers(Camera *camera) { IPU3CameraData *data = cameraData(camera); - CIO2Device *cio2 = &data->cio2_; ImgUDevice *imgu = data->imgu_; unsigned int bufferCount; int ret; - ret = cio2->allocateBuffers(); - if (ret < 0) - return ret; - bufferCount = std::max({ data->outStream_.configuration().bufferCount, data->vfStream_.configuration().bufferCount, @@ -674,10 +669,8 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera) }); ret = imgu->allocateBuffers(data, bufferCount); - if (ret < 0) { - cio2->freeBuffers(); + if (ret < 0) return ret; - } return 0; } @@ -686,7 +679,6 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera) { IPU3CameraData *data = cameraData(camera); - data->cio2_.freeBuffers(); data->imgu_->freeBuffers(data); return 0; @@ -731,10 +723,10 @@ error: void PipelineHandlerIPU3::stop(Camera *camera) { IPU3CameraData *data = cameraData(camera); - int ret; + int ret = 0; - ret = data->cio2_.stop(); ret |= data->imgu_->stop(); + ret |= data->cio2_.stop(); if (ret) LOG(IPU3, Warning) << "Failed to stop camera " << camera->name();