From patchwork Sun Jan 12 01:02:11 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: 2613 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1BF9060705 for ; Sun, 12 Jan 2020 02:03:31 +0100 (CET) X-Halon-ID: 56df4051-34d7-11ea-b6d8-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p54ac5d7b.dip0.t-ipconnect.de [84.172.93.123]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id 56df4051-34d7-11ea-b6d8-005056917f90; Sun, 12 Jan 2020 02:03:27 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Sun, 12 Jan 2020 02:02:11 +0100 Message-Id: <20200112010212.2609025-32-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20200112010212.2609025-1-niklas.soderlund@ragnatech.se> References: <20200112010212.2609025-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 31/32] libcamera: pipeline: Remove explicit buffer handling 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: Sun, 12 Jan 2020 01:03:31 -0000 With the FrameBuffer interface in place there is no need for the Camera to call into the specific pipelines allocation and freeing of buffers as it no longer needs to be synchronized with buffer allocation by the application. Remove the function prototypes in the pipeline handler base class and fold the functionality in the pipelines start() and stop() functions where needed. A follow up patch will remove the now no-op Camera::allocateBuffers() and Camera::freeBuffers(). Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- src/libcamera/camera.cpp | 8 +------ src/libcamera/include/pipeline_handler.h | 5 ---- src/libcamera/pipeline/ipu3/ipu3.cpp | 24 ++++++++++++-------- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 24 ++++++++++++-------- src/libcamera/pipeline/uvcvideo.cpp | 17 -------------- src/libcamera/pipeline/vimc.cpp | 17 -------------- src/libcamera/pipeline_handler.cpp | 29 ------------------------ 7 files changed, 30 insertions(+), 94 deletions(-) diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 3fe40feb88be7324..f3a7578d0834a9d6 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -726,12 +726,6 @@ int Camera::allocateBuffers() return -EINVAL; } - int ret = pipe_->allocateBuffers(this, activeStreams_); - if (ret) { - LOG(Camera, Error) << "Failed to allocate buffers"; - return ret; - } - state_ = CameraPrepared; return 0; @@ -752,7 +746,7 @@ int Camera::freeBuffers() state_ = CameraConfigured; - return pipe_->freeBuffers(this, activeStreams_); + return 0; } /** diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 27f3852c6c87843f..a6c1e1fbae384405 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -75,11 +75,6 @@ public: virtual int importFrameBuffers(Camera *camera, Stream *stream) = 0; virtual void freeFrameBuffers(Camera *camera, Stream *stream) = 0; - virtual int allocateBuffers(Camera *camera, - const std::set &streams) = 0; - virtual int freeBuffers(Camera *camera, - const std::set &streams) = 0; - virtual int start(Camera *camera) = 0; virtual void stop(Camera *camera) = 0; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 1ea4d938ad88ae21..7894084a025e9946 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -211,11 +211,6 @@ public: int importFrameBuffers(Camera *camera, Stream *stream) override; void freeFrameBuffers(Camera *camera, Stream *stream) override; - int allocateBuffers(Camera *camera, - const std::set &streams) override; - int freeBuffers(Camera *camera, - const std::set &streams) override; - int start(Camera *camera) override; void stop(Camera *camera) override; @@ -232,6 +227,9 @@ private: int registerCameras(); + int allocateBuffers(Camera *camera); + int freeBuffers(Camera *camera); + ImgUDevice imgu0_; ImgUDevice imgu1_; MediaDevice *cio2MediaDev_; @@ -652,8 +650,7 @@ void PipelineHandlerIPU3::freeFrameBuffers(Camera *camera, Stream *stream) * In order to be able to start the 'viewfinder' and 'stat' nodes, we need * memory to be reserved. */ -int PipelineHandlerIPU3::allocateBuffers(Camera *camera, - const std::set &streams) +int PipelineHandlerIPU3::allocateBuffers(Camera *camera) { IPU3CameraData *data = cameraData(camera); IPU3Stream *outStream = &data->outStream_; @@ -716,13 +713,12 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, return 0; error: - freeBuffers(camera, streams); + freeBuffers(camera); return ret; } -int PipelineHandlerIPU3::freeBuffers(Camera *camera, - const std::set &streams) +int PipelineHandlerIPU3::freeBuffers(Camera *camera) { IPU3CameraData *data = cameraData(camera); @@ -739,6 +735,11 @@ int PipelineHandlerIPU3::start(Camera *camera) ImgUDevice *imgu = data->imgu_; int ret; + /* Allocate buffers for internal pipeline usage. */ + ret = allocateBuffers(camera); + if (ret) + return ret; + /* * Start the ImgU video devices, buffers will be queued to the * ImgU output and viewfinder when requests will be queued. @@ -757,6 +758,7 @@ int PipelineHandlerIPU3::start(Camera *camera) return 0; error: + freeBuffers(camera); LOG(IPU3, Error) << "Failed to start camera " << camera->name(); return ret; @@ -772,6 +774,8 @@ void PipelineHandlerIPU3::stop(Camera *camera) if (ret) LOG(IPU3, Warning) << "Failed to stop camera " << camera->name(); + + freeBuffers(camera); } int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index da6e079f51620234..389a99cf52bd1cea 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -178,11 +178,6 @@ public: int importFrameBuffers(Camera *camera, Stream *stream) override; void freeFrameBuffers(Camera *camera, Stream *stream) override; - int allocateBuffers(Camera *camera, - const std::set &streams) override; - int freeBuffers(Camera *camera, - const std::set &streams) override; - int start(Camera *camera) override; void stop(Camera *camera) override; @@ -208,6 +203,9 @@ private: void paramReady(FrameBuffer *buffer); void statReady(FrameBuffer *buffer); + int allocateBuffers(Camera *camera); + int freeBuffers(Camera *camera); + MediaDevice *media_; V4L2Subdevice *dphy_; V4L2Subdevice *isp_; @@ -675,8 +673,7 @@ void PipelineHandlerRkISP1::freeFrameBuffers(Camera *camera, Stream *stream) video_->releaseBuffers(); } -int PipelineHandlerRkISP1::allocateBuffers(Camera *camera, - const std::set &streams) +int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) { RkISP1CameraData *data = cameraData(camera); unsigned int count = 1; @@ -720,8 +717,7 @@ error: return ret; } -int PipelineHandlerRkISP1::freeBuffers(Camera *camera, - const std::set &streams) +int PipelineHandlerRkISP1::freeBuffers(Camera *camera) { RkISP1CameraData *data = cameraData(camera); @@ -755,10 +751,16 @@ int PipelineHandlerRkISP1::start(Camera *camera) RkISP1CameraData *data = cameraData(camera); int ret; + /* Allocate buffers for internal pipeline usage. */ + ret = allocateBuffers(camera); + if (ret) + return ret; + data->frame_ = 0; ret = param_->streamOn(); if (ret) { + freeBuffers(camera); LOG(RkISP1, Error) << "Failed to start parameters " << camera->name(); return ret; @@ -767,6 +769,7 @@ int PipelineHandlerRkISP1::start(Camera *camera) ret = stat_->streamOn(); if (ret) { param_->streamOff(); + freeBuffers(camera); LOG(RkISP1, Error) << "Failed to start statistics " << camera->name(); return ret; @@ -776,6 +779,7 @@ int PipelineHandlerRkISP1::start(Camera *camera) if (ret) { param_->streamOff(); stat_->streamOff(); + freeBuffers(camera); LOG(RkISP1, Error) << "Failed to start camera " << camera->name(); @@ -820,6 +824,8 @@ void PipelineHandlerRkISP1::stop(Camera *camera) data->timeline_.reset(); + freeBuffers(camera); + activeCamera_ = nullptr; } diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 67d29b7919e46801..47916ffb453672d4 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -70,11 +70,6 @@ public: int importFrameBuffers(Camera *camera, Stream *stream) override; void freeFrameBuffers(Camera *camera, Stream *stream) override; - int allocateBuffers(Camera *camera, - const std::set &streams) override; - int freeBuffers(Camera *camera, - const std::set &streams) override; - int start(Camera *camera) override; void stop(Camera *camera) override; @@ -222,18 +217,6 @@ void PipelineHandlerUVC::freeFrameBuffers(Camera *camera, Stream *stream) data->video_->releaseBuffers(); } -int PipelineHandlerUVC::allocateBuffers(Camera *camera, - const std::set &streams) -{ - return 0; -} - -int PipelineHandlerUVC::freeBuffers(Camera *camera, - const std::set &streams) -{ - return 0; -} - int PipelineHandlerUVC::start(Camera *camera) { UVCCameraData *data = cameraData(camera); diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 4cfdb5ae79ce5779..1700ac967299b106 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -87,11 +87,6 @@ public: int importFrameBuffers(Camera *camera, Stream *stream) override; void freeFrameBuffers(Camera *camera, Stream *stream) override; - int allocateBuffers(Camera *camera, - const std::set &streams) override; - int freeBuffers(Camera *camera, - const std::set &streams) override; - int start(Camera *camera) override; void stop(Camera *camera) override; @@ -288,18 +283,6 @@ void PipelineHandlerVimc::freeFrameBuffers(Camera *camera, Stream *stream) data->video_->releaseBuffers(); } -int PipelineHandlerVimc::allocateBuffers(Camera *camera, - const std::set &streams) -{ - return 0; -} - -int PipelineHandlerVimc::freeBuffers(Camera *camera, - const std::set &streams) -{ - return 0; -} - int PipelineHandlerVimc::start(Camera *camera) { VimcCameraData *data = cameraData(camera); diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 0348e3cfa68ed6ec..2fd65b468b9cd84f 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -356,35 +356,6 @@ const ControlInfoMap &PipelineHandler::controls(Camera *camera) * helper. */ -/** - * \fn PipelineHandler::allocateBuffers() - * \brief Allocate buffers for a stream - * \param[in] camera The camera the \a stream belongs to - * \param[in] streams The set of streams to allocate buffers for - * - * This method allocates buffers internally in the pipeline handler for each - * stream in the \a streams buffer set, and associates them with the stream's - * buffer pool. - * - * The intended caller of this method is the Camera class. - * - * \return 0 on success or a negative error code otherwise - */ - -/** - * \fn PipelineHandler::freeBuffers() - * \brief Free all buffers associated with a stream - * \param[in] camera The camera the \a stream belongs to - * \param[in] streams The set of streams to free buffers from - * - * After a capture session has been stopped all buffers associated with each - * stream shall be freed. - * - * The intended caller of this method is the Camera class. - * - * \return 0 on success or a negative error code otherwise - */ - /** * \fn PipelineHandler::start() * \brief Start capturing from a group of streams