From patchwork Sat Jul 13 17:23:36 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1682 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 4ABD66175B for ; Sat, 13 Jul 2019 19:24:26 +0200 (CEST) Received: from pendragon.ideasonboard.com (softbank126209254147.bbtec.net [126.209.254.147]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 214C52B2 for ; Sat, 13 Jul 2019 19:24:24 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1563038666; bh=8keGl6+P+yeUQhUtgdMlPQu/yfmfkIQHGfk+bzLPOb4=; h=From:To:Subject:Date:In-Reply-To:References:From; b=RHpWHMFvBjR9O8LIbR5chNoOAd3sOd8xzaqn0hoTHpblUYRPnaveoeCFdIVQgruz8 UnqlLH5n5eKz5KTr7YemZ7KnGF6VnAlYjXzTS9AjFIYo9zJMUC2rHlLF7qMCAceGvL jXKoRtyygxBfv6X+brXbSznSk8RJMliuux7H6H7o= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 13 Jul 2019 20:23:36 +0300 Message-Id: <20190713172351.25452-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> References: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 01/16] libcamera: camera: Don't move buffers away from request at completion X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 Jul 2019 17:24:26 -0000 For a historical reason that isn't fully understood, the request completion handler in the Camera class moves all buffers away from the request's buffer map to a local variable before emitting the request completion signal. There's no reason to do so, and it makes it impossible for requests to access buffers in their destructor. Fix it. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/camera.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 592dfd39eacc..094f1b63b7f1 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -894,8 +894,7 @@ int Camera::stop() */ void Camera::requestComplete(Request *request) { - std::map buffers(std::move(request->bufferMap_)); - requestCompleted.emit(request, buffers); + requestCompleted.emit(request, request->bufferMap_); delete request; } From patchwork Sat Jul 13 17:23:37 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1683 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1A93E6175D for ; Sat, 13 Jul 2019 19:24:28 +0200 (CEST) Received: from pendragon.ideasonboard.com (softbank126209254147.bbtec.net [126.209.254.147]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BD85A2B2 for ; Sat, 13 Jul 2019 19:24:26 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1563038667; bh=CgQ2O/+AQhBnbdoGGmgOysR85DTzo4B4OUSV1DEUV4o=; h=From:To:Subject:Date:In-Reply-To:References:From; b=HwJBjqfDgOfFTNdLG9507rho6r9AQGtQJirAw5eEO1uuI1sQ2rV3vA1rPUFr/CyJy 9Clhc/XoAsy905aMjUu9pFahDjP2Pmz/ODTqVu8wG/XP+w4zGL0GDaRQh42ccklELA 0kcV1zahq5EbupRw+SJTzkmlKfrHs2oo9w11+jWU= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 13 Jul 2019 20:23:37 +0300 Message-Id: <20190713172351.25452-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> References: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 02/16] libcamera: camera: Don't check buffer count before freeing buffers X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 Jul 2019 17:24:28 -0000 There's no need to check if buffers have been allocated before freeing them as the BufferPool::destroyBuffers() method is a no-op when no buffers have been allocated. Document this fact explicitly, and remove the buffer count check. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/buffer.cpp | 3 +++ src/libcamera/camera.cpp | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index d86278a8a90a..c0a020942f5c 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -311,6 +311,9 @@ void BufferPool::createBuffers(unsigned int count) /** * \brief Release all buffers from pool + * + * If no buffers have been created or if buffers have already been released no + * operation is performed. */ void BufferPool::destroyBuffers() { diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 094f1b63b7f1..810cb1295f34 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -740,9 +740,6 @@ int Camera::freeBuffers() return -EACCES; for (Stream *stream : activeStreams_) { - if (!stream->bufferPool().count()) - continue; - /* * All mappings must be destroyed before buffers can be freed * by the V4L2 device that has allocated them. From patchwork Sat Jul 13 17:23:38 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1684 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id AAEB361764 for ; Sat, 13 Jul 2019 19:24:29 +0200 (CEST) Received: from pendragon.ideasonboard.com (softbank126209254147.bbtec.net [126.209.254.147]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 7F419443 for ; Sat, 13 Jul 2019 19:24:28 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1563038669; bh=JpS0md5byVimHIwfq6tkafs+RmoUx4IkuE22MpNVQzA=; h=From:To:Subject:Date:In-Reply-To:References:From; b=Cu96yvCw4WvZn2Rn3cE8c+ULoyozfrrX3ztRzfkE75Peo8kHJsdRUBLa7GX50KGPf VG4ttl+qfBxkfLxlOYbi8lNVueqR7T+8sO6WcogW5dNfZw3+lUNwVW/Vbj9xjxRGHR wGIJ205qRdkWhA6p2I3LCa4Zm5NgTHGFX0DGZwJA= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 13 Jul 2019 20:23:38 +0300 Message-Id: <20190713172351.25452-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> References: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 03/16] libcamera: pipeline_handler: Simplify request completion X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 Jul 2019 17:24:29 -0000 libcamera guarantees that requests complete in sequence. This requirement is currently pushed down to pipeline handlers. Three out of four of our pipeline handlers implement that requirement based on the sole assumption that buffers will always complete in sequeuence, while the IPU3 pipeline handler implements a more complex logic. It turns out that the logic can be moved to the base PipelineHandler class with support from the Request class. Do so to simplify the pipeline handlers. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/pipeline/ipu3/ipu3.cpp | 10 ++------- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +--- src/libcamera/pipeline/uvcvideo.cpp | 2 +- src/libcamera/pipeline/vimc.cpp | 2 +- src/libcamera/pipeline_handler.cpp | 26 ++++++++++++++++-------- 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index e4efb9722f76..1abd20e5fee5 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -927,14 +927,8 @@ void IPU3CameraData::imguOutputBufferReady(Buffer *buffer) /* Request not completed yet, return here. */ return; - /* Complete the pending requests in queueing order. */ - while (1) { - request = queuedRequests_.front(); - if (request->hasPendingBuffers()) - break; - - pipe_->completeRequest(camera_, request); - } + /* Mark the request as complete. */ + pipe_->completeRequest(camera_, request); } /** diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 4a5898d25f91..383236435a7f 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -491,9 +491,7 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) void PipelineHandlerRkISP1::bufferReady(Buffer *buffer) { ASSERT(activeCamera_); - - RkISP1CameraData *data = cameraData(activeCamera_); - Request *request = data->queuedRequests_.front(); + Request *request = buffer->request(); completeBuffer(activeCamera_, request, buffer); completeRequest(activeCamera_, request); diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 72313cfd246f..387d71ef567c 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -382,7 +382,7 @@ int UVCCameraData::init(MediaEntity *entity) void UVCCameraData::bufferReady(Buffer *buffer) { - Request *request = queuedRequests_.front(); + Request *request = buffer->request(); pipe_->completeBuffer(camera_, request, buffer); pipe_->completeRequest(camera_, request); diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index f8a58be060bb..ff2529c974fd 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -376,7 +376,7 @@ int VimcCameraData::init(MediaDevice *media) void VimcCameraData::bufferReady(Buffer *buffer) { - Request *request = queuedRequests_.front(); + Request *request = buffer->request(); pipe_->completeBuffer(camera_, request, buffer); pipe_->completeRequest(camera_, request); diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 0283e4e5ad51..c609200252f1 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -391,8 +391,9 @@ int PipelineHandler::queueRequest(Camera *camera, Request *request) * This method shall be called by pipeline handlers to signal completion of the * \a buffer part of the \a request. It notifies applications of buffer * completion and updates the request's internal buffer tracking. The request - * is not completed automatically when the last buffer completes, pipeline - * handlers shall complete requests explicitly with completeRequest(). + * is not completed automatically when the last buffer completes to give + * pipeline handlers a chance to perform any operation that may still be + * needed. They shall complete requests explicitly with completeRequest(). * * \return True if all buffers contained in the request have completed, false * otherwise @@ -413,17 +414,24 @@ bool PipelineHandler::completeBuffer(Camera *camera, Request *request, * request has completed. The request is deleted and shall not be accessed once * this method returns. * - * The pipeline handler shall ensure that requests complete in the same order - * they are submitted. + * This method ensures that requests will be returned to the application in + * submission order, the pipeline handler may call it on any complete request + * without any ordering constraint. */ void PipelineHandler::completeRequest(Camera *camera, Request *request) { - CameraData *data = cameraData(camera); - ASSERT(request == data->queuedRequests_.front()); - data->queuedRequests_.pop_front(); - request->complete(Request::RequestComplete); - camera->requestComplete(request); + + CameraData *data = cameraData(camera); + + while (!data->queuedRequests_.empty()) { + request = data->queuedRequests_.front(); + if (request->hasPendingBuffers()) + break; + + data->queuedRequests_.pop_front(); + camera->requestComplete(request); + } } /** From patchwork Sat Jul 13 17:23:39 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1685 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 32BBD61760 for ; Sat, 13 Jul 2019 19:24:31 +0200 (CEST) Received: from pendragon.ideasonboard.com (softbank126209254147.bbtec.net [126.209.254.147]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 15218443 for ; Sat, 13 Jul 2019 19:24:29 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1563038670; bh=v+B5QVuTUkUMF8r2djLCIK6Dhh8rF/ynuBhdrwfO7gE=; h=From:To:Subject:Date:In-Reply-To:References:From; b=DhJ4d8F6/8/4YXcYwVe2dDdjLeMP5CDaGLGj3IHqMItK2WouTta7g+m2FE6m0Kkou gDbc3VlIC8bvDdEBuDJmeVOtCdMWDbbAzqiuza48YufsKcoOP0HfNhLUoY4r97ag7r xZbDVzjr696Hp0GQeGDzt950++9Eg2l0/7wwobxw= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 13 Jul 2019 20:23:39 +0300 Message-Id: <20190713172351.25452-5-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> References: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 04/16] libcamera: request: Add cookie to make request tracking easier X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 Jul 2019 17:24:31 -0000 Applications often have to map requests queued to a camera to external resources. To make this easy, add a 64-bit integer cookie to the Request class that is set when the request is created and can be retrieved at any time, especially in the request completion handler. The cookie is completely transparent for libcamera and is never modified. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- include/libcamera/camera.h | 3 ++- include/libcamera/request.h | 5 ++++- src/libcamera/camera.cpp | 10 ++++++++-- src/libcamera/request.cpp | 18 ++++++++++++++++-- 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 6d693d9a6c7a..21fac550f412 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -93,7 +94,7 @@ public: int allocateBuffers(); int freeBuffers(); - Request *createRequest(); + Request *createRequest(uint64_t cookie = 0); int queueRequest(Request *request); int start(); diff --git a/include/libcamera/request.h b/include/libcamera/request.h index a93468d7c8b7..dd165bc21c03 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -8,6 +8,7 @@ #define __LIBCAMERA_REQUEST_H__ #include +#include #include #include @@ -29,7 +30,7 @@ public: RequestCancelled, }; - explicit Request(Camera *camera); + Request(Camera *camera, uint64_t cookie = 0); Request(const Request &) = delete; Request &operator=(const Request &) = delete; @@ -38,6 +39,7 @@ public: int setBuffers(const std::map &streamMap); Buffer *findBuffer(Stream *stream) const; + uint64_t cookie() const { return cookie_; } Status status() const { return status_; } bool hasPendingBuffers() const { return !pending_.empty(); } @@ -56,6 +58,7 @@ private: std::map bufferMap_; std::unordered_set pending_; + uint64_t cookie_; Status status_; }; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 810cb1295f34..1f307654ab01 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -754,10 +754,16 @@ int Camera::freeBuffers() /** * \brief Create a request object for the camera + * \param[in] cookie Opaque cookie for application use * * This method creates an empty request for the application to fill with * buffers and paramaters, and queue for capture. * + * The \a cookie is stored in the request and is accessible through the + * Request::cookie() method at any time. It is typically used by applications + * to map the request to an external resource in the request completion + * handler, and is completely opaque to libcamera. + * * The ownership of the returned request is passed to the caller, which is * responsible for either queueing the request or deleting it. * @@ -766,12 +772,12 @@ int Camera::freeBuffers() * * \return A pointer to the newly created request, or nullptr on error */ -Request *Camera::createRequest() +Request *Camera::createRequest(uint64_t cookie) { if (disconnected_ || !stateBetween(CameraPrepared, CameraRunning)) return nullptr; - return new Request(this); + return new Request(this, cookie); } /** diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index f0b5985814bd..8cf41a43a80e 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -46,9 +46,17 @@ LOG_DEFINE_CATEGORY(Request) /** * \brief Create a capture request for a camera * \param[in] camera The camera that creates the request + * \param[in] cookie Opaque cookie for application use + * + * The \a cookie is stored in the request and is accessible through the + * cookie() method at any time. It is typically used by applications to map the + * request to an external resource in the request completion handler, and is + * completely opaque to libcamera. + * */ -Request::Request(Camera *camera) - : camera_(camera), controls_(camera), status_(RequestPending) +Request::Request(Camera *camera, uint64_t cookie) + : camera_(camera), controls_(camera), cookie_(cookie), + status_(RequestPending) { } @@ -119,6 +127,12 @@ Buffer *Request::findBuffer(Stream *stream) const return it->second; } +/** + * \fn Request::cookie() + * \brief Retrieve the cookie set when the request was created + * \return The request cookie + */ + /** * \fn Request::status() * \brief Retrieve the request completion status From patchwork Sat Jul 13 17:23:40 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1686 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E8EB36175D for ; Sat, 13 Jul 2019 19:24:32 +0200 (CEST) Received: from pendragon.ideasonboard.com (softbank126209254147.bbtec.net [126.209.254.147]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id AB4AD443 for ; Sat, 13 Jul 2019 19:24:31 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1563038672; bh=/T/maYXqvjknYjID7loUR6hiI9T0SX+2uWGMy9JT7iY=; h=From:To:Subject:Date:In-Reply-To:References:From; b=Zr9CozOF0b1i8JgtzKEHryg5g8xgzJdzSMo0mBXaZXt4hXH3CH+PiwSKhCh1KRMOr lh1uoFsWMiIETKJxxNdvcEmQEOmtfD74GAvipsZnog5Nomy2ucK3chKcEzIp8OQVVw OUUUfZf/WPkSwXUFKNnJROZCEHGIEEbWHMlElW44= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 13 Jul 2019 20:23:40 +0300 Message-Id: <20190713172351.25452-6-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> References: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 05/16] libcamera: v4l2_videodevice: Add helper to queue all buffers X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 Jul 2019 17:24:33 -0000 When starting the stream on a capture video device it is often needed to queue all the allocated buffers. Add a helper method to do so, and refactor the existing queueBuffer() method to make it clearer. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/include/v4l2_videodevice.h | 1 + src/libcamera/pipeline/ipu3/ipu3.cpp | 13 ++--- src/libcamera/v4l2_videodevice.cpp | 63 +++++++++++++++++++++--- test/v4l2_videodevice/buffer_sharing.cpp | 8 ++- test/v4l2_videodevice/capture_async.cpp | 8 ++- 5 files changed, 69 insertions(+), 24 deletions(-) diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index b92df882568f..f948a41fb8c1 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -139,6 +139,7 @@ public: int releaseBuffers(); int queueBuffer(Buffer *buffer); + int queueAllBuffers(std::vector> *buffers); Signal bufferReady; int streamOn(); diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 1abd20e5fee5..50457891e288 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -131,6 +131,7 @@ public: CameraSensor *sensor_; BufferPool pool_; + std::vector> buffers_; }; class IPU3Stream : public Stream @@ -1430,11 +1431,9 @@ int CIO2Device::start() { int ret; - for (Buffer &buffer : pool_.buffers()) { - ret = output_->queueBuffer(&buffer); - if (ret) - return ret; - } + ret = output_->queueAllBuffers(&buffers_); + if (ret) + return ret; ret = output_->streamOn(); if (ret) @@ -1445,7 +1444,9 @@ int CIO2Device::start() int CIO2Device::stop() { - return output_->streamOff(); + int ret = output_->streamOff(); + buffers_.clear(); + return ret; } int CIO2Device::mediaBusToFormat(unsigned int code) diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 2d1e87a76c6f..af5ba803de45 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -894,8 +894,8 @@ int V4L2VideoDevice::releaseBuffers() */ int V4L2VideoDevice::queueBuffer(Buffer *buffer) { + struct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {}; struct v4l2_buffer buf = {}; - struct v4l2_plane planes[VIDEO_MAX_PLANES] = {}; int ret; buf.index = buffer->index(); @@ -904,21 +904,20 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer) buf.field = V4L2_FIELD_NONE; bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type); + const std::vector &planes = buffer->planes(); if (buf.memory == V4L2_MEMORY_DMABUF) { if (multiPlanar) { - for (unsigned int p = 0; - p < buffer->planes().size(); - p++) - planes[p].m.fd = buffer->planes()[p].dmabuf(); + for (unsigned int p = 0; p < planes.size(); ++p) + v4l2Planes[p].m.fd = planes[p].dmabuf(); } else { - buf.m.fd = buffer->planes()[0].dmabuf(); + buf.m.fd = planes[0].dmabuf(); } } if (multiPlanar) { - buf.length = buffer->planes().size(); - buf.m.planes = planes; + buf.length = planes.size(); + buf.m.planes = v4l2Planes; } if (V4L2_TYPE_IS_OUTPUT(bufferType_)) { @@ -944,6 +943,54 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer) return 0; } +/** + * \brief Queue all buffers into the video device + * \param[out] buffers A vector of queued Buffer instances + * + * When starting video capture users of the video device often need to queue + * all allocated buffers to the device. This helper method simplifies the + * implementation of the user by queuing all buffers and populating the + * \a buffers vector with Buffer instances for each queued buffer. + * + * This method is meant to be used with video capture devices internal to a + * pipeline handler, such as ISP statistics capture devices, or raw CSI-2 + * receivers. For video capture devices facing applications, buffers shall + * instead be queued when requests are received, and for video output devices, + * buffers shall be queued when frames are ready to be output. + * + * The caller shall ensure that the \a buffers vector remains valid until all + * the queued buffers are dequeued, either during capture, or by stopping the + * video device. + * + * Calling this method on an output device or on a device that has buffers + * already queued is an error and will return -EINVAL. + * + * \return 0 on success or a negative error code otherwise + */ +int V4L2VideoDevice::queueAllBuffers(std::vector> *buffers) +{ + int ret; + + if (queuedBuffersCount_) + return -EINVAL; + + if (V4L2_TYPE_IS_OUTPUT(bufferType_)) + return -EINVAL; + + buffers->clear(); + + for (unsigned int i = 0; i < bufferPool_->count(); ++i) { + Buffer *buffer = new Buffer(); + buffer->index_ = i; + buffers->emplace_back(buffer); + ret = queueBuffer(buffer); + if (ret) + return ret; + } + + return 0; +} + /** * \brief Dequeue the next available buffer from the video device * diff --git a/test/v4l2_videodevice/buffer_sharing.cpp b/test/v4l2_videodevice/buffer_sharing.cpp index cc724b226619..459bd238fe97 100644 --- a/test/v4l2_videodevice/buffer_sharing.cpp +++ b/test/v4l2_videodevice/buffer_sharing.cpp @@ -117,11 +117,9 @@ protected: capture_->bufferReady.connect(this, &BufferSharingTest::captureBufferReady); output_->bufferReady.connect(this, &BufferSharingTest::outputBufferReady); - /* Queue all the buffers to the capture device. */ - for (Buffer &buffer : pool_.buffers()) { - if (capture_->queueBuffer(&buffer)) - return TestFail; - } + std::vector> buffers; + if (capture_->queueAllBuffers(&buffers)) + return TestFail; ret = capture_->streamOn(); if (ret) { diff --git a/test/v4l2_videodevice/capture_async.cpp b/test/v4l2_videodevice/capture_async.cpp index cea4fffbf7a5..c6d6ec6b74bd 100644 --- a/test/v4l2_videodevice/capture_async.cpp +++ b/test/v4l2_videodevice/capture_async.cpp @@ -46,11 +46,9 @@ protected: capture_->bufferReady.connect(this, &CaptureAsyncTest::receiveBuffer); - /* Queue all the buffers to the device. */ - for (Buffer &b : pool_.buffers()) { - if (capture_->queueBuffer(&b)) - return TestFail; - } + std::vector> buffers; + if (capture_->queueAllBuffers(&buffers)) + return TestFail; ret = capture_->streamOn(); if (ret) From patchwork Sat Jul 13 17:23:41 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1687 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 09A886175D for ; Sat, 13 Jul 2019 19:24:35 +0200 (CEST) Received: from pendragon.ideasonboard.com (softbank126209254147.bbtec.net [126.209.254.147]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 86B142B2 for ; Sat, 13 Jul 2019 19:24:33 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1563038674; bh=SVZM/RZQRsI+9BbTvQyNfG3b6MrlDwmv6HLWgJSiKoA=; h=From:To:Subject:Date:In-Reply-To:References:From; b=ftktXEvAq1oOzU/BPz4sqHSLYP6Mfh2VGHl4JHkxJ0Cmbsl1tl/U7SBK0wncFry1e BTjWwRQq9NvDRudz4UWPyAaazMAIkqCgtlgU+hhFcumn69hqWlMMrVi1zeTK9bRIiN qk1Ot7RY8IxBnwZRn4hfBN6Buza5Tc+7uf06BgFE= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 13 Jul 2019 20:23:41 +0300 Message-Id: <20190713172351.25452-7-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> References: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 06/16] libcamera: buffer: Split memory information to BufferMemory X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 Jul 2019 17:24:35 -0000 The Buffer class is a large beast the stores information about the buffer memory, dynamic metadata related to the frame stored in the buffer, and buffer reference data (in the index). In order to implement buffer import we will need to extend this with dmabuf file descriptors, making usage of the class even more complex. Refactor the Buffer class by splitting the buffer memory information to a BufferMemory class, and repurposing the Buffer class to reference a buffer and to store dynamic metadata. The BufferMemory class becomes a long term storage, valid and stable from the time buffer memory is allocated to the time it is freed. The Buffer class, on the other hand, becomes transient, is created on demand when an application requires a buffer, is given to a request, and is deleted when the request completes. Buffer and BufferMemory don't need to be copied, so their copy constructor and assignment operators are deleted. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- include/libcamera/buffer.h | 55 +++++--- include/libcamera/request.h | 4 +- include/libcamera/stream.h | 4 + src/cam/buffer_writer.cpp | 4 +- src/cam/buffer_writer.h | 3 +- src/cam/capture.cpp | 35 +++-- src/libcamera/buffer.cpp | 164 ++++++++++++++--------- src/libcamera/include/v4l2_videodevice.h | 8 +- src/libcamera/request.cpp | 35 +++-- src/libcamera/stream.cpp | 24 ++++ src/libcamera/v4l2_videodevice.cpp | 40 +++--- src/qcam/main_window.cpp | 40 ++++-- src/qcam/main_window.h | 2 +- test/camera/capture.cpp | 19 ++- test/camera/statemachine.cpp | 6 +- 15 files changed, 298 insertions(+), 145 deletions(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index 260a62e9e77e..f5ba6207bcef 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -14,6 +14,7 @@ namespace libcamera { class BufferPool; class Request; +class Stream; class Plane final { @@ -36,6 +37,30 @@ private: void *mem_; }; +class BufferMemory final +{ +public: + std::vector &planes() { return planes_; } + +private: + std::vector planes_; +}; + +class BufferPool final +{ +public: + ~BufferPool(); + + void createBuffers(unsigned int count); + void destroyBuffers(); + + unsigned int count() const { return buffers_.size(); } + std::vector &buffers() { return buffers_; } + +private: + std::vector buffers_; +}; + class Buffer final { public: @@ -45,20 +70,24 @@ public: BufferCancelled, }; - Buffer(); + Buffer(unsigned int index = -1, const Buffer *metadata = nullptr); + Buffer(const Buffer &) = delete; + Buffer &operator=(const Buffer &) = delete; unsigned int index() const { return index_; } + unsigned int bytesused() const { return bytesused_; } uint64_t timestamp() const { return timestamp_; } unsigned int sequence() const { return sequence_; } + Status status() const { return status_; } - std::vector &planes() { return planes_; } Request *request() const { return request_; } + Stream *stream() const { return stream_; } private: - friend class BufferPool; friend class PipelineHandler; friend class Request; + friend class Stream; friend class V4L2VideoDevice; void cancel(); @@ -66,28 +95,14 @@ private: void setRequest(Request *request) { request_ = request; } unsigned int index_; + unsigned int bytesused_; uint64_t timestamp_; unsigned int sequence_; + Status status_; - - std::vector planes_; Request *request_; -}; - -class BufferPool final -{ -public: - ~BufferPool(); - - void createBuffers(unsigned int count); - void destroyBuffers(); - - unsigned int count() const { return buffers_.size(); } - std::vector &buffers() { return buffers_; } - -private: - std::vector buffers_; + Stream *stream_; }; } /* namespace libcamera */ diff --git a/include/libcamera/request.h b/include/libcamera/request.h index dd165bc21c03..7a60be617645 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -8,6 +8,7 @@ #define __LIBCAMERA_REQUEST_H__ #include +#include #include #include @@ -33,10 +34,11 @@ public: Request(Camera *camera, uint64_t cookie = 0); Request(const Request &) = delete; Request &operator=(const Request &) = delete; + ~Request(); ControlList &controls() { return controls_; } const std::map &buffers() const { return bufferMap_; } - int setBuffers(const std::map &streamMap); + int addBuffer(std::unique_ptr buffer); Buffer *findBuffer(Stream *stream) const; uint64_t cookie() const { return cookie_; } diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index 5b4fea324ce4..f595a630eb4a 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -8,6 +8,7 @@ #define __LIBCAMERA_STREAM_H__ #include +#include #include #include @@ -66,6 +67,9 @@ class Stream { public: Stream(); + + std::unique_ptr createBuffer(unsigned int index); + BufferPool &bufferPool() { return bufferPool_; } const StreamConfiguration &configuration() const { return configuration_; } diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp index e0374ffcb319..b7f2ed4f2d2d 100644 --- a/src/cam/buffer_writer.cpp +++ b/src/cam/buffer_writer.cpp @@ -19,7 +19,7 @@ BufferWriter::BufferWriter(const std::string &pattern) { } -int BufferWriter::write(libcamera::Buffer *buffer, +int BufferWriter::write(libcamera::Buffer *buffer, libcamera::BufferMemory *mem, const std::string &streamName) { std::string filename; @@ -41,7 +41,7 @@ int BufferWriter::write(libcamera::Buffer *buffer, if (fd == -1) return -errno; - for (libcamera::Plane &plane : buffer->planes()) { + for (libcamera::Plane &plane : mem->planes()) { void *data = plane.mem(); unsigned int length = plane.length(); diff --git a/src/cam/buffer_writer.h b/src/cam/buffer_writer.h index 7bf785d1e832..9bea205fac1d 100644 --- a/src/cam/buffer_writer.h +++ b/src/cam/buffer_writer.h @@ -16,7 +16,8 @@ class BufferWriter public: BufferWriter(const std::string &pattern = "frame-#.bin"); - int write(libcamera::Buffer *buffer, const std::string &streamName); + int write(libcamera::Buffer *buffer, libcamera::BufferMemory *mem, + const std::string &streamName); private: std::string pattern_; diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp index 6b842d73390d..1cf59063c93b 100644 --- a/src/cam/capture.cpp +++ b/src/cam/capture.cpp @@ -95,13 +95,14 @@ int Capture::capture(EventLoop *loop) std::map map; for (StreamConfiguration &cfg : *config_) { Stream *stream = cfg.stream(); - map[stream] = &stream->bufferPool().buffers()[i]; - } + std::unique_ptr buffer = stream->createBuffer(i); - ret = request->setBuffers(map); - if (ret < 0) { - std::cerr << "Can't set buffers for request" << std::endl; - return ret; + ret = request->addBuffer(std::move(buffer)); + if (ret < 0) { + std::cerr << "Can't set buffer for request" + << std::endl; + return ret; + } } requests.push_back(request); @@ -155,6 +156,7 @@ void Capture::requestComplete(Request *request, const std::mapfirst; Buffer *buffer = it->second; + BufferMemory *mem = &stream->bufferPool().buffers()[buffer->index()]; const std::string &name = streamName_[stream]; info << " " << name @@ -163,17 +165,34 @@ void Capture::requestComplete(Request *request, const std::mapbytesused(); if (writer_) - writer_->write(buffer, name); + writer_->write(buffer, mem, name); } std::cout << info.str() << std::endl; + /* + * Create a new request and populate it with one buffer for each + * stream. + */ request = camera_->createRequest(); if (!request) { std::cerr << "Can't create request" << std::endl; return; } - request->setBuffers(buffers); + for (auto it = buffers.begin(); it != buffers.end(); ++it) { + Stream *stream = it->first; + Buffer *buffer = it->second; + unsigned int index = buffer->index(); + + std::unique_ptr newBuffer = stream->createBuffer(index); + if (!newBuffer) { + std::cerr << "Can't create buffer " << index << std::endl; + return; + } + + request->addBuffer(std::move(newBuffer)); + } + camera_->queueRequest(request); } diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index c0a020942f5c..ecbf25246a55 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -173,12 +173,78 @@ void *Plane::mem() */ /** - * \class Buffer + * \class BufferMemory * \brief A memory buffer to store an image * - * The Buffer class represents the memory buffers used to store a - * full frame image, which may contain multiple separate memory Plane - * objects if the image format is multi-planar. + * The BufferMemory class represents the memory buffers used to store full frame + * images, which may contain multiple separate memory Plane objects if the + * image format is multi-planar. + */ + +/** + * \fn BufferMemory::planes() + * \brief Retrieve the planes within the buffer + * \return A reference to a vector holding all Planes within the buffer + */ + +/** + * \class BufferPool + * \brief A pool of buffers + * + * The BufferPool class groups together a collection of Buffers to store frames. + * The buffers must be exported by a device before they can be imported into + * another device for further use. + */ + +BufferPool::~BufferPool() +{ + destroyBuffers(); +} + +/** + * \brief Create buffers in the Pool + * \param[in] count The number of buffers to create + */ +void BufferPool::createBuffers(unsigned int count) +{ + buffers_.resize(count); +} + +/** + * \brief Release all buffers from pool + * + * If no buffers have been created or if buffers have already been released no + * operation is performed. + */ +void BufferPool::destroyBuffers() +{ + buffers_.resize(0); +} + +/** + * \fn BufferPool::count() + * \brief Retrieve the number of buffers contained within the pool + * \return The number of buffers contained in the pool + */ + +/** + * \fn BufferPool::buffers() + * \brief Retrieve all the buffers in the pool + * \return A vector containing all the buffers in the pool. + */ + +/** + * \class Buffer + * \brief A buffer handle and dynamic metadata + * + * The Buffer class references a buffer memory and associates dynamic metadata + * related to the frame contained in the buffer. It allows referencing buffer + * memory through a single interface regardless of whether the memory is + * allocated internally in libcamera or provided externally through dmabuf. + * + * Buffer instances are allocated dynamically for a stream through + * Stream::createBuffer(), added to a request with Request::addBuffer() and + * deleted automatically after the request complete handler returns. */ /** @@ -195,9 +261,26 @@ void *Plane::mem() * invalid and shall not be used. */ -Buffer::Buffer() - : index_(-1), request_(nullptr) +/** + * \brief Construct a buffer not associated with any stream + * + * This method constructs an orphaned buffer not associated with any stream. It + * is not meant to be called by applications, they should instead create buffers + * for a stream with Stream::createBuffer(). + */ +Buffer::Buffer(unsigned int index, const Buffer *metadata) + : index_(index), status_(Buffer::BufferSuccess), request_(nullptr), + stream_(nullptr) { + if (metadata) { + bytesused_ = metadata->bytesused_; + sequence_ = metadata->sequence_; + timestamp_ = metadata->timestamp_; + } else { + bytesused_ = 0; + sequence_ = 0; + timestamp_ = 0; + } } /** @@ -206,12 +289,6 @@ Buffer::Buffer() * \return The buffer index */ -/** - * \fn Buffer::planes() - * \brief Retrieve the planes within the buffer - * \return A reference to a vector holding all Planes within the buffer - */ - /** * \fn Buffer::bytesused() * \brief Retrieve the number of bytes occupied by the data in the buffer @@ -264,6 +341,19 @@ Buffer::Buffer() * \sa Buffer::setRequest() */ +/** + * \fn Buffer::stream() + * \brief Retrieve the stream this buffer is associated with + * + * A Buffer is associated to the stream that created it with + * Stream::createBuffer() and the association is valid until the buffer is + * destroyed. Buffer instances that are created directly are not associated + * with any stream. + * + * \return The Stream the Buffer is associated with, or nullptr if the buffer + * is not associated with a stream + */ + /** * \brief Mark a buffer as cancel by setting its status to BufferCancelled */ @@ -282,54 +372,4 @@ void Buffer::cancel() * The intended callers are Request::prepare() and Request::completeBuffer(). */ -/** - * \class BufferPool - * \brief A pool of buffers - * - * The BufferPool class groups together a collection of Buffers to store frames. - * The buffers must be exported by a device before they can be imported into - * another device for further use. - */ - -BufferPool::~BufferPool() -{ - destroyBuffers(); -} - -/** - * \brief Create buffers in the Pool - * \param[in] count The number of buffers to create - */ -void BufferPool::createBuffers(unsigned int count) -{ - unsigned int index = 0; - - buffers_.resize(count); - for (Buffer &buffer : buffers_) - buffer.index_ = index++; -} - -/** - * \brief Release all buffers from pool - * - * If no buffers have been created or if buffers have already been released no - * operation is performed. - */ -void BufferPool::destroyBuffers() -{ - buffers_.resize(0); -} - -/** - * \fn BufferPool::count() - * \brief Retrieve the number of buffers contained within the pool - * \return The number of buffers contained in the pool - */ - -/** - * \fn BufferPool::buffers() - * \brief Retrieve all the buffers in the pool - * \return A vector containing all the buffers in the pool. - */ - } /* namespace libcamera */ diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index f948a41fb8c1..8238a39b0da4 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -7,7 +7,6 @@ #ifndef __LIBCAMERA_V4L2_VIDEODEVICE_H__ #define __LIBCAMERA_V4L2_VIDEODEVICE_H__ -#include #include #include @@ -23,6 +22,7 @@ namespace libcamera { class Buffer; +class BufferMemory; class BufferPool; class EventNotifier; class MediaDevice; @@ -165,8 +165,8 @@ private: std::vector enumSizes(unsigned int pixelFormat); int requestBuffers(unsigned int count); - int createPlane(Buffer *buffer, unsigned int plane, - unsigned int length); + int createPlane(BufferMemory *buffer, unsigned int index, + unsigned int plane, unsigned int length); Buffer *dequeueBuffer(); void bufferAvailable(EventNotifier *notifier); @@ -177,7 +177,7 @@ private: enum v4l2_memory memoryType_; BufferPool *bufferPool_; - std::atomic queuedBuffersCount_; + std::map queuedBuffers_; EventNotifier *fdEvent_; }; diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index 8cf41a43a80e..45e7133febb0 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -60,6 +60,14 @@ Request::Request(Camera *camera, uint64_t cookie) { } +Request::~Request() +{ + for (auto it : bufferMap_) { + Buffer *buffer = it.second; + delete buffer; + } +} + /** * \fn Request::controls() * \brief Retrieve the request's ControlList @@ -87,19 +95,30 @@ Request::Request(Camera *camera, uint64_t cookie) */ /** - * \brief Set the streams to capture with associated buffers - * \param[in] streamMap The map of streams to buffers + * \brief Store a Buffer with its associated Stream in the Request + * \param[in] buffer The Buffer to store in the request + * + * Ownership of the buffer is passed to the request. It will be deleted when + * the request is destroyed after completing. + * + * A request can only contain one buffer per stream. If a buffer has already + * been added to the request for the same stream, this method returns -EEXIST. + * * \return 0 on success or a negative error code otherwise - * \retval -EBUSY Buffers have already been set + * \retval -EEXIST The request already contains a buffer for the stream */ -int Request::setBuffers(const std::map &streamMap) +int Request::addBuffer(std::unique_ptr buffer) { - if (!bufferMap_.empty()) { - LOG(Request, Error) << "Buffers already set"; - return -EBUSY; + Stream *stream = buffer->stream(); + + auto it = bufferMap_.find(stream); + if (it != bufferMap_.end()) { + LOG(Request, Error) << "Buffer already set for stream"; + return -EEXIST; } - bufferMap_ = streamMap; + bufferMap_[stream] = buffer.release(); + return 0; } diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index d8e87c62281c..6d80646b55cd 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -407,6 +407,30 @@ Stream::Stream() { } +/** + * \brief Create a Buffer instance + * \param[in] index The desired buffer index + * + * This method creates a Buffer instance that references a BufferMemory from + * the stream's buffers pool by its \a index. The index shall be lower than the + * number of buffers in the pool. + * + * \return A newly created Buffer on success or nullptr otherwise + */ +std::unique_ptr Stream::createBuffer(unsigned int index) +{ + if (index >= bufferPool_.count()) { + LOG(Stream, Error) << "Invalid buffer index " << index; + return nullptr; + } + + Buffer *buffer = new Buffer(); + buffer->index_ = index; + buffer->stream_ = this; + + return std::unique_ptr(buffer); +} + /** * \fn Stream::bufferPool() * \brief Retrieve the buffer pool for the stream diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index af5ba803de45..65b4098abc05 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -268,8 +268,7 @@ const std::string V4L2DeviceFormat::toString() const * \param[in] deviceNode The file-system path to the video device node */ V4L2VideoDevice::V4L2VideoDevice(const std::string &deviceNode) - : V4L2Device(deviceNode), bufferPool_(nullptr), - queuedBuffersCount_(0), fdEvent_(nullptr) + : V4L2Device(deviceNode), bufferPool_(nullptr), fdEvent_(nullptr) { /* * We default to an MMAP based CAPTURE video device, however this will @@ -764,7 +763,7 @@ int V4L2VideoDevice::exportBuffers(BufferPool *pool) for (i = 0; i < pool->count(); ++i) { struct v4l2_plane planes[VIDEO_MAX_PLANES] = {}; struct v4l2_buffer buf = {}; - Buffer &buffer = pool->buffers()[i]; + BufferMemory &buffer = pool->buffers()[i]; buf.index = i; buf.type = bufferType_; @@ -782,13 +781,13 @@ int V4L2VideoDevice::exportBuffers(BufferPool *pool) if (V4L2_TYPE_IS_MULTIPLANAR(buf.type)) { for (unsigned int p = 0; p < buf.length; ++p) { - ret = createPlane(&buffer, p, + ret = createPlane(&buffer, i, p, buf.m.planes[p].length); if (ret) break; } } else { - ret = createPlane(&buffer, 0, buf.length); + ret = createPlane(&buffer, i, 0, buf.length); } if (ret) { @@ -808,19 +807,19 @@ int V4L2VideoDevice::exportBuffers(BufferPool *pool) return 0; } -int V4L2VideoDevice::createPlane(Buffer *buffer, unsigned int planeIndex, - unsigned int length) +int V4L2VideoDevice::createPlane(BufferMemory *buffer, unsigned int index, + unsigned int planeIndex, unsigned int length) { struct v4l2_exportbuffer expbuf = {}; int ret; LOG(V4L2, Debug) - << "Buffer " << buffer->index() + << "Buffer " << index << " plane " << planeIndex << ": length=" << length; expbuf.type = bufferType_; - expbuf.index = buffer->index(); + expbuf.index = index; expbuf.plane = planeIndex; expbuf.flags = O_RDWR; @@ -904,7 +903,8 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer) buf.field = V4L2_FIELD_NONE; bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type); - const std::vector &planes = buffer->planes(); + BufferMemory *mem = &bufferPool_->buffers()[buf.index]; + const std::vector &planes = mem->planes(); if (buf.memory == V4L2_MEMORY_DMABUF) { if (multiPlanar) { @@ -937,9 +937,11 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer) return ret; } - if (queuedBuffersCount_++ == 0) + if (queuedBuffers_.empty()) fdEvent_->setEnabled(true); + queuedBuffers_[buf.index] = buffer; + return 0; } @@ -971,7 +973,7 @@ int V4L2VideoDevice::queueAllBuffers(std::vector> *buffe { int ret; - if (queuedBuffersCount_) + if (!queuedBuffers_.empty()) return -EINVAL; if (V4L2_TYPE_IS_OUTPUT(bufferType_)) @@ -980,8 +982,7 @@ int V4L2VideoDevice::queueAllBuffers(std::vector> *buffe buffers->clear(); for (unsigned int i = 0; i < bufferPool_->count(); ++i) { - Buffer *buffer = new Buffer(); - buffer->index_ = i; + Buffer *buffer = new Buffer(i); buffers->emplace_back(buffer); ret = queueBuffer(buffer); if (ret) @@ -1022,11 +1023,14 @@ Buffer *V4L2VideoDevice::dequeueBuffer() ASSERT(buf.index < bufferPool_->count()); - if (--queuedBuffersCount_ == 0) + auto it = queuedBuffers_.find(buf.index); + Buffer *buffer = it->second; + queuedBuffers_.erase(it); + + if (queuedBuffers_.empty()) fdEvent_->setEnabled(false); - Buffer *buffer = &bufferPool_->buffers()[buf.index]; - + buffer->index_ = buf.index; buffer->bytesused_ = buf.bytesused; buffer->timestamp_ = buf.timestamp.tv_sec * 1000000000ULL + buf.timestamp.tv_usec * 1000ULL; @@ -1101,7 +1105,7 @@ int V4L2VideoDevice::streamOff() return ret; } - queuedBuffersCount_ = 0; + queuedBuffers_.clear(); fdEvent_->setEnabled(false); return 0; diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 907d2423ed15..6ecf30e33bcf 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -142,8 +142,7 @@ int MainWindow::startCapture() BufferPool &pool = stream->bufferPool(); std::vector requests; - - for (Buffer &buffer : pool.buffers()) { + for (unsigned int i = 0; i < pool.count(); ++i) { Request *request = camera_->createRequest(); if (!request) { std::cerr << "Can't create request" << std::endl; @@ -151,11 +150,15 @@ int MainWindow::startCapture() goto error; } - std::map map; - map[stream] = &buffer; - ret = request->setBuffers(map); + std::unique_ptr buffer = stream->createBuffer(i); + if (!buffer) { + std::cerr << "Can't create buffer " << i << std::endl; + goto error; + } + + ret = request->addBuffer(std::move(buffer)); if (ret < 0) { - std::cerr << "Can't set buffers for request" << std::endl; + std::cerr << "Can't set buffer for request" << std::endl; goto error; } @@ -219,6 +222,7 @@ void MainWindow::requestComplete(Request *request, framesCaptured_++; + Stream *stream = buffers.begin()->first; Buffer *buffer = buffers.begin()->second; double fps = buffer->timestamp() - lastBufferTime_; @@ -232,7 +236,8 @@ void MainWindow::requestComplete(Request *request, << " fps: " << std::fixed << std::setprecision(2) << fps << std::endl; - display(buffer); + BufferMemory *mem = &stream->bufferPool().buffers()[buffer->index()]; + display(buffer, mem); request = camera_->createRequest(); if (!request) { @@ -240,16 +245,29 @@ void MainWindow::requestComplete(Request *request, return; } - request->setBuffers(buffers); + for (auto it = buffers.begin(); it != buffers.end(); ++it) { + Stream *stream = it->first; + Buffer *buffer = it->second; + unsigned int index = buffer->index(); + + std::unique_ptr newBuffer = stream->createBuffer(index); + if (!newBuffer) { + std::cerr << "Can't create buffer " << index << std::endl; + return; + } + + request->addBuffer(std::move(newBuffer)); + } + camera_->queueRequest(request); } -int MainWindow::display(Buffer *buffer) +int MainWindow::display(Buffer *buffer, BufferMemory *mem) { - if (buffer->planes().size() != 1) + if (mem->planes().size() != 1) return -EINVAL; - Plane &plane = buffer->planes().front(); + Plane &plane = mem->planes().front(); unsigned char *raw = static_cast(plane.mem()); viewfinder_->display(raw, buffer->bytesused()); diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h index f58cb6a65b2b..b4f6f747e16e 100644 --- a/src/qcam/main_window.h +++ b/src/qcam/main_window.h @@ -48,7 +48,7 @@ private: void requestComplete(Request *request, const std::map &buffers); - int display(Buffer *buffer); + int display(Buffer *buffer, BufferMemory *mem); QString title_; QTimer titleTimer_; diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index 7ce247cc482d..ff1cbd6cbac0 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -34,9 +34,13 @@ protected: completeRequestsCount_++; - /* Reuse the buffers for a new request. */ + /* Create a new request. */ + Stream *stream = buffers.begin()->first; + Buffer *buffer = buffers.begin()->second; + std::unique_ptr newBuffer = stream->createBuffer(buffer->index()); + request = camera_->createRequest(); - request->setBuffers(buffers); + request->addBuffer(std::move(newBuffer)); camera_->queueRequest(request); } @@ -78,15 +82,20 @@ protected: Stream *stream = cfg.stream(); BufferPool &pool = stream->bufferPool(); std::vector requests; - for (Buffer &buffer : pool.buffers()) { + for (unsigned int i = 0; i < pool.count(); ++i) { Request *request = camera_->createRequest(); if (!request) { cout << "Failed to create request" << endl; return TestFail; } - std::map map = { { stream, &buffer } }; - if (request->setBuffers(map)) { + std::unique_ptr buffer = stream->createBuffer(i); + if (!buffer) { + cout << "Failed to create buffer " << i << endl; + return TestFail; + } + + if (request->addBuffer(std::move(buffer))) { cout << "Failed to associating buffer with request" << endl; return TestFail; } diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp index 84d2a6fab5f0..12d5e0e1d534 100644 --- a/test/camera/statemachine.cpp +++ b/test/camera/statemachine.cpp @@ -211,10 +211,8 @@ protected: return TestFail; Stream *stream = *camera_->streams().begin(); - BufferPool &pool = stream->bufferPool(); - Buffer &buffer = pool.buffers().front(); - std::map map = { { stream, &buffer } }; - if (request->setBuffers(map)) + std::unique_ptr buffer = stream->createBuffer(0); + if (request->addBuffer(std::move(buffer))) return TestFail; if (camera_->queueRequest(request)) From patchwork Sat Jul 13 17:23:42 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1688 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9D6D06175E for ; Sat, 13 Jul 2019 19:24:36 +0200 (CEST) Received: from pendragon.ideasonboard.com (softbank126209254147.bbtec.net [126.209.254.147]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 6A8A02B2 for ; Sat, 13 Jul 2019 19:24:35 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1563038676; bh=ZGaT60/q1Y42I4mA9Yzu7B/9ClD1S7qlOSh5pU++Zfk=; h=From:To:Subject:Date:In-Reply-To:References:From; b=QD5KdXHiUfqkQVr2ULxB7oXsIv0p9NnCgKqkiZbJAaGzLRHrR4XySzsMKUkR9D/DZ ywOQC6pMMdCWWuzzYgSw9D1UUV4eU6bYGvDaKnIPbgjD26WG6XgL/CnwjnfSga1ZwG Ofrp4tNfN5c5OgH12rEkrAtg9jWfaUZDz4t5HOU8= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 13 Jul 2019 20:23:42 +0300 Message-Id: <20190713172351.25452-8-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> References: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 07/16] libcamera: v4l2_videodevice: Signal buffer completion at streamoff time X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 Jul 2019 17:24:36 -0000 When stopping the stream buffers have been queued, in which case their completion is never be notified to the user. This can lead to memory leaks. Fix it by notifying completion of all queued buffers with the status set to error. As a result the base PipelineHandler implementation can be simplified, as all requests complete as the result of stopping the stream. The stop() method that manually completes all queued requests isn't needed anymore. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- include/libcamera/request.h | 3 ++- src/libcamera/include/pipeline_handler.h | 2 +- src/libcamera/pipeline/ipu3/ipu3.cpp | 2 -- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 -- src/libcamera/pipeline/uvcvideo.cpp | 1 - src/libcamera/pipeline/vimc.cpp | 1 - src/libcamera/pipeline_handler.cpp | 29 +++--------------------- src/libcamera/request.cpp | 14 ++++++++---- src/libcamera/v4l2_videodevice.cpp | 14 +++++++++++- test/v4l2_videodevice/buffer_sharing.cpp | 6 +++++ 10 files changed, 34 insertions(+), 40 deletions(-) diff --git a/include/libcamera/request.h b/include/libcamera/request.h index 7a60be617645..00c68345b5b4 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -51,7 +51,7 @@ private: friend class PipelineHandler; int prepare(); - void complete(Status status); + void complete(); bool completeBuffer(Buffer *buffer); @@ -62,6 +62,7 @@ private: uint64_t cookie_; Status status_; + bool cancelled_; }; } /* namespace libcamera */ diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index f836d5d1a600..1fdef9cea40f 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -74,7 +74,7 @@ public: const std::set &streams) = 0; virtual int start(Camera *camera) = 0; - virtual void stop(Camera *camera); + virtual void stop(Camera *camera) = 0; virtual int queueRequest(Camera *camera, Request *request); diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 50457891e288..ffc066a15ae9 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -711,8 +711,6 @@ void PipelineHandlerIPU3::stop(Camera *camera) if (ret) LOG(IPU3, Warning) << "Failed to stop camera " << camera->name(); - - PipelineHandler::stop(camera); } int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 383236435a7f..cc33a2cb2572 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -354,8 +354,6 @@ void PipelineHandlerRkISP1::stop(Camera *camera) LOG(RkISP1, Warning) << "Failed to stop camera " << camera->name(); - PipelineHandler::stop(camera); - activeCamera_ = nullptr; } diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 387d71ef567c..b299c5da8471 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -220,7 +220,6 @@ void PipelineHandlerUVC::stop(Camera *camera) { UVCCameraData *data = cameraData(camera); data->video_->streamOff(); - PipelineHandler::stop(camera); } int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index ff2529c974fd..7d96c3645c06 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -222,7 +222,6 @@ void PipelineHandlerVimc::stop(Camera *camera) { VimcCameraData *data = cameraData(camera); data->video_->streamOff(); - PipelineHandler::stop(camera); } int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index c609200252f1..83938a71f615 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -328,31 +328,7 @@ const ControlInfoMap &PipelineHandler::controls(Camera *camera) * * This method stops capturing and processing requests immediately. All pending * requests are cancelled and complete immediately in an error state. - * - * Pipeline handlers shall override this method to stop the pipeline, ensure - * that all pending request completion signaled through completeRequest() have - * returned, and call the base implementation of the stop() method as the last - * step of their implementation. The base implementation cancels all requests - * queued but not yet complete. */ -void PipelineHandler::stop(Camera *camera) -{ - CameraData *data = cameraData(camera); - - while (!data->queuedRequests_.empty()) { - Request *request = data->queuedRequests_.front(); - data->queuedRequests_.pop_front(); - - while (!request->pending_.empty()) { - Buffer *buffer = *request->pending_.begin(); - buffer->cancel(); - completeBuffer(camera, request, buffer); - } - - request->complete(Request::RequestCancelled); - camera->requestComplete(request); - } -} /** * \fn PipelineHandler::queueRequest() @@ -420,15 +396,16 @@ bool PipelineHandler::completeBuffer(Camera *camera, Request *request, */ void PipelineHandler::completeRequest(Camera *camera, Request *request) { - request->complete(Request::RequestComplete); + request->complete(); CameraData *data = cameraData(camera); while (!data->queuedRequests_.empty()) { request = data->queuedRequests_.front(); - if (request->hasPendingBuffers()) + if (request->status() == Request::RequestPending) break; + ASSERT(!request->hasPendingBuffers()); data->queuedRequests_.pop_front(); camera->requestComplete(request); } diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index 45e7133febb0..19131472710b 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -56,7 +56,7 @@ LOG_DEFINE_CATEGORY(Request) */ Request::Request(Camera *camera, uint64_t cookie) : camera_(camera), controls_(camera), cookie_(cookie), - status_(RequestPending) + status_(RequestPending), cancelled_(false) { } @@ -199,14 +199,15 @@ int Request::prepare() /** * \brief Complete a queued request - * \param[in] status The request completion status * - * Mark the request as complete by updating its status to \a status. + * Mark the request as complete by updating its status to RequestComplete, + * unless buffers have been cancelled in which case the status is set to + * RequestCancelled. */ -void Request::complete(Status status) +void Request::complete() { ASSERT(!hasPendingBuffers()); - status_ = status; + status_ = cancelled_ ? RequestCancelled : RequestComplete; } /** @@ -229,6 +230,9 @@ bool Request::completeBuffer(Buffer *buffer) buffer->setRequest(nullptr); + if (buffer->status() == Buffer::BufferCancelled) + cancelled_ = true; + return !hasPendingBuffers(); } diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 65b4098abc05..aa3136378997 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1090,7 +1090,9 @@ int V4L2VideoDevice::streamOn() * \brief Stop the video stream * * Buffers that are still queued when the video stream is stopped are - * implicitly dequeued, but no bufferReady signal is emitted for them. + * immediately dequeued with their status set to Buffer::BufferError, + * and the bufferReady signal is emitted for them. The order in which those + * buffers are dequeued is not specified. * * \return 0 on success or a negative error code otherwise */ @@ -1105,6 +1107,16 @@ int V4L2VideoDevice::streamOff() return ret; } + /* Send back all queued buffers. */ + for (auto it : queuedBuffers_) { + unsigned int index = it.first; + Buffer *buffer = it.second; + + buffer->index_ = index; + buffer->cancel(); + bufferReady.emit(buffer); + } + queuedBuffers_.clear(); fdEvent_->setEnabled(false); diff --git a/test/v4l2_videodevice/buffer_sharing.cpp b/test/v4l2_videodevice/buffer_sharing.cpp index 459bd238fe97..4da6ba466f40 100644 --- a/test/v4l2_videodevice/buffer_sharing.cpp +++ b/test/v4l2_videodevice/buffer_sharing.cpp @@ -95,6 +95,9 @@ protected: std::cout << "Received capture buffer: " << buffer->index() << " sequence " << buffer->sequence() << std::endl; + if (buffer->status() != Buffer::BufferSuccess) + return; + output_->queueBuffer(buffer); framesCaptured_++; } @@ -104,6 +107,9 @@ protected: std::cout << "Received output buffer: " << buffer->index() << " sequence " << buffer->sequence() << std::endl; + if (buffer->status() != Buffer::BufferSuccess) + return; + capture_->queueBuffer(buffer); framesOutput_++; } From patchwork Sat Jul 13 17:23:43 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1689 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 659E86175D for ; Sat, 13 Jul 2019 19:24:38 +0200 (CEST) Received: from pendragon.ideasonboard.com (softbank126209254147.bbtec.net [126.209.254.147]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 2277C443 for ; Sat, 13 Jul 2019 19:24:36 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1563038678; bh=okmAxtUruVB8pinmiOgzJjtkfvoBMrA2UCfcIicLLKc=; h=From:To:Subject:Date:In-Reply-To:References:From; b=SLgwLZj8A9DjQdLhFPqRrg2CnMh98LoD92SA8E84vrbDDhaU7px1y51z3XeEDJgYC ztMjBG/4WJACxoc8Z8fYOA2GTOha46lzpCcMvFsFRR0KhU1PWYIpzRTdnhMqRdeR76 7fU2lu72SUiNoqa48OzXIXW8kLWzPCegKJ9Mi+Lc= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 13 Jul 2019 20:23:43 +0300 Message-Id: <20190713172351.25452-9-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> References: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 08/16] libcamera: pipeline: ipu3: Use stream configuration to get buffers count X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 Jul 2019 17:24:38 -0000 Access the number of allocated buffer for the streams through the stream configuration instead of the stream's buffers pool. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index ffc066a15ae9..488e89f25c5e 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -635,7 +635,7 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, * of buffers as the active ones. */ if (!outStream->active_) { - bufferCount = vfStream->bufferPool().count(); + bufferCount = vfStream->configuration().bufferCount; outStream->device_->pool->createBuffers(bufferCount); ret = imgu->exportBuffers(outStream->device_, outStream->device_->pool); @@ -644,7 +644,7 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, } if (!vfStream->active_) { - bufferCount = outStream->bufferPool().count(); + bufferCount = outStream->configuration().bufferCount; vfStream->device_->pool->createBuffers(bufferCount); ret = imgu->exportBuffers(vfStream->device_, vfStream->device_->pool); From patchwork Sat Jul 13 17:23:44 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1690 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 4757660C23 for ; Sat, 13 Jul 2019 19:24:40 +0200 (CEST) Received: from pendragon.ideasonboard.com (softbank126209254147.bbtec.net [126.209.254.147]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id ED33E2B2 for ; Sat, 13 Jul 2019 19:24:38 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1563038680; bh=AQnkuXuCVS9ffEDIOY8qsL428MAKHXoik31YeXIxO6Y=; h=From:To:Subject:Date:In-Reply-To:References:From; b=TM58QijIc5HsOH222DxNQX73ZcWeAf/kas5OJXDaxhRpsIA93TbNtZUtjFTmjokLD AeEm3lTFbnN6hPKdUhJtobaP6CU8+O9ZAbUHEGTRfhpak/C3Wrv5v++qecTrlnwpUa p/GuovPWeMgokAiIkzjMuB7fOY31BCIfwsr2rVu0= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 13 Jul 2019 20:23:44 +0300 Message-Id: <20190713172351.25452-10-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> References: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 09/16] libcamera: Stop using Stream::bufferPool to get the number of buffers X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 Jul 2019 17:24:40 -0000 The cam and qcam applications, as well as the camera capture test case, access the Stream::bufferPool in order to know how many requests to initially queue. As part of an effort to remove access to the buffer pool from applications, use the buffer count from the stream configuration instead. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/cam/capture.cpp | 6 ++---- src/qcam/main_window.cpp | 3 +-- test/camera/capture.cpp | 3 +-- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp index 1cf59063c93b..66ec1abaa5ac 100644 --- a/src/cam/capture.cpp +++ b/src/cam/capture.cpp @@ -74,10 +74,8 @@ int Capture::capture(EventLoop *loop) /* Identify the stream with the least number of buffers. */ unsigned int nbuffers = UINT_MAX; - for (StreamConfiguration &cfg : *config_) { - Stream *stream = cfg.stream(); - nbuffers = std::min(nbuffers, stream->bufferPool().count()); - } + for (StreamConfiguration &cfg : *config_) + nbuffers = std::min(nbuffers, cfg.bufferCount); /* * TODO: make cam tool smarter to support still capture by for diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 6ecf30e33bcf..7023e1158fb6 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -140,9 +140,8 @@ int MainWindow::startCapture() return ret; } - BufferPool &pool = stream->bufferPool(); std::vector requests; - for (unsigned int i = 0; i < pool.count(); ++i) { + for (unsigned int i = 0; i < cfg.bufferCount; ++i) { Request *request = camera_->createRequest(); if (!request) { std::cerr << "Can't create request" << std::endl; diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index ff1cbd6cbac0..e6bf7a8d9ebb 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -80,9 +80,8 @@ protected: } Stream *stream = cfg.stream(); - BufferPool &pool = stream->bufferPool(); std::vector requests; - for (unsigned int i = 0; i < pool.count(); ++i) { + for (unsigned int i = 0; i < cfg.bufferCount; ++i) { Request *request = camera_->createRequest(); if (!request) { cout << "Failed to create request" << endl; From patchwork Sat Jul 13 17:23:45 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1691 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id ECC5760C23 for ; Sat, 13 Jul 2019 19:24:41 +0200 (CEST) Received: from pendragon.ideasonboard.com (softbank126209254147.bbtec.net [126.209.254.147]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id C3A072B2 for ; Sat, 13 Jul 2019 19:24:40 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1563038681; bh=g8EfUqPeiObCJp19TGmeSC4qb8hFzsMqFl8ySC2N0HU=; h=From:To:Subject:Date:In-Reply-To:References:From; b=C79tTY0eY+mXO52NUMg6zHWsn/wU0g1xQW+QpegmSByQ7L9/VZFysLhR8LN++Ttj2 FUolzWztPkbumK3jsJV7uK6xYRYPcTRJ4uZ6FverNHKnDK2V+L2PbWZmDW11DEfmRf mtJ7MziS8knPtwJtUiBLEFkT/BdvuX1XmVV8z21U= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 13 Jul 2019 20:23:45 +0300 Message-Id: <20190713172351.25452-11-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> References: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 10/16] libcamera: stream: Shorten access to the bufferPool X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 Jul 2019 17:24:42 -0000 From: Jacopo Mondi All interactions with the Stream's buffers currently go through the BufferPool. In order to shorten accessing the buffers array, and eventually restrict access to the Stream's internal buffer pool, provide operations to access, create and destroy buffers. It is still possible to access the pool for pipeline handlers to populate it by exporting buffers from a video device to Stream's pool. Signed-off-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- include/libcamera/stream.h | 4 ++++ src/cam/capture.cpp | 2 +- src/libcamera/camera.cpp | 4 ++-- src/libcamera/stream.cpp | 36 ++++++++++++++++++++++++++++++++++-- src/qcam/main_window.cpp | 2 +- 5 files changed, 42 insertions(+), 6 deletions(-) diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index f595a630eb4a..bc14fb60480e 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -71,11 +71,15 @@ public: std::unique_ptr createBuffer(unsigned int index); BufferPool &bufferPool() { return bufferPool_; } + std::vector &buffers() { return bufferPool_.buffers(); } const StreamConfiguration &configuration() const { return configuration_; } protected: friend class Camera; + void createBuffers(unsigned int count); + void destroyBuffers(); + BufferPool bufferPool_; StreamConfiguration configuration_; }; diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp index 66ec1abaa5ac..5ffa4ae291da 100644 --- a/src/cam/capture.cpp +++ b/src/cam/capture.cpp @@ -154,7 +154,7 @@ void Capture::requestComplete(Request *request, const std::mapfirst; Buffer *buffer = it->second; - BufferMemory *mem = &stream->bufferPool().buffers()[buffer->index()]; + BufferMemory *mem = &stream->buffers()[buffer->index()]; const std::string &name = streamName_[stream]; info << " " << name diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 1f307654ab01..61d3e821f48f 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -683,7 +683,7 @@ int Camera::configure(CameraConfiguration *config) * Allocate buffer objects in the pool. * Memory will be allocated and assigned later. */ - stream->bufferPool().createBuffers(cfg.bufferCount); + stream->createBuffers(cfg.bufferCount); } state_ = CameraConfigured; @@ -744,7 +744,7 @@ int Camera::freeBuffers() * All mappings must be destroyed before buffers can be freed * by the V4L2 device that has allocated them. */ - stream->bufferPool().destroyBuffers(); + stream->destroyBuffers(); } state_ = CameraConfigured; diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index 6d80646b55cd..884fbfebd52c 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -435,19 +435,51 @@ std::unique_ptr Stream::createBuffer(unsigned int index) * \fn Stream::bufferPool() * \brief Retrieve the buffer pool for the stream * - * The buffer pool handles the buffers used to capture frames at the output of - * the stream. It is initially created empty and shall be populated with + * The buffer pool handles the memory buffers used to store frames for the + * stream. It is initially created empty and shall be populated with * buffers before being used. * * \return A reference to the buffer pool */ +/** + * \fn Stream::buffers() + * \brief Retrieve the memory buffers in the Stream's buffer pool + * \return The list of stream's memory buffers + */ + /** * \fn Stream::configuration() * \brief Retrieve the active configuration of the stream * \return The active configuration of the stream */ +/** + * \brief Create buffers for the stream + * \param count The number of buffers to create + * + * Create \a count empty buffers in the Stream's buffer pool. + */ +void Stream::createBuffers(unsigned int count) +{ + destroyBuffers(); + if (count == 0) + return; + + bufferPool_.createBuffers(count); +} + +/** + * \brief Destroy buffers in the stream + * + * If no buffers have been created or if buffers have already been destroyed no + * operation is performed. + */ +void Stream::destroyBuffers() +{ + bufferPool_.destroyBuffers(); +} + /** * \var Stream::bufferPool_ * \brief The pool of buffers associated with the stream diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 7023e1158fb6..92f888323f0b 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -235,7 +235,7 @@ void MainWindow::requestComplete(Request *request, << " fps: " << std::fixed << std::setprecision(2) << fps << std::endl; - BufferMemory *mem = &stream->bufferPool().buffers()[buffer->index()]; + BufferMemory *mem = &stream->buffers()[buffer->index()]; display(buffer, mem); request = camera_->createRequest(); From patchwork Sat Jul 13 17:23:46 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1692 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B3A436175B for ; Sat, 13 Jul 2019 19:24:43 +0200 (CEST) Received: from pendragon.ideasonboard.com (softbank126209254147.bbtec.net [126.209.254.147]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 6FA482B2 for ; Sat, 13 Jul 2019 19:24:42 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1563038683; bh=5VAj2gisFfQhEnJnqo7buZXlLkxoi+kGfK+QtD0il9k=; h=From:To:Subject:Date:In-Reply-To:References:From; b=ZZfbRSfwgs0+RCbPFi7efdaaHbZFpkNoynOna5rStK/F0YrOzv8fB1WBZtKnn8UzC bRUhaCL7XWUrpazwN3wlq78ie48kHimOGR5inu7OVEw0Im1AhQYs7GdXffojV/qO+b S7p/MCts38u/fFp1siGriCRMYtVxnLdFlUpdZe9g= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 13 Jul 2019 20:23:46 +0300 Message-Id: <20190713172351.25452-12-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> References: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 11/16] libcamera: stream: Add Stream memory type X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 Jul 2019 17:24:44 -0000 From: Jacopo Mondi Define the memory type a Stream uses and allow application to set it through the associated StreamConfiguration. A Stream can use either internal or external memory allocation methods, depending on where the data produced by the stream is actually saved. Signed-off-by: Jacopo Mondi Reviewed-by: Niklas Söderlund Signed-off-by: Laurent Pinchart --- include/libcamera/stream.h | 10 +++++++++- src/libcamera/camera.cpp | 2 +- src/libcamera/stream.cpp | 35 ++++++++++++++++++++++++++++++++--- 3 files changed, 42 insertions(+), 5 deletions(-) diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index bc14fb60480e..08eb8cc7d5c7 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -35,6 +35,11 @@ private: std::map> formats_; }; +enum MemoryType { + InternalMemory, + ExternalMemory, +}; + struct StreamConfiguration { StreamConfiguration(); StreamConfiguration(const StreamFormats &formats); @@ -42,6 +47,7 @@ struct StreamConfiguration { unsigned int pixelFormat; Size size; + MemoryType memoryType; unsigned int bufferCount; Stream *stream() const { return stream_; } @@ -73,15 +79,17 @@ public: BufferPool &bufferPool() { return bufferPool_; } std::vector &buffers() { return bufferPool_.buffers(); } const StreamConfiguration &configuration() const { return configuration_; } + MemoryType memoryType() const { return memoryType_; } protected: friend class Camera; - void createBuffers(unsigned int count); + void createBuffers(MemoryType memory, unsigned int count); void destroyBuffers(); BufferPool bufferPool_; StreamConfiguration configuration_; + MemoryType memoryType_; }; } /* namespace libcamera */ diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 61d3e821f48f..af69607b19e6 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -683,7 +683,7 @@ int Camera::configure(CameraConfiguration *config) * Allocate buffer objects in the pool. * Memory will be allocated and assigned later. */ - stream->createBuffers(cfg.bufferCount); + stream->createBuffers(cfg.memoryType, cfg.bufferCount); } state_ = CameraConfigured; diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index 884fbfebd52c..94aa4810f6b9 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -263,6 +263,17 @@ SizeRange StreamFormats::range(unsigned int pixelformat) const return range; } +/** + * \enum MemoryType + * \brief Define the memory type used by a Stream + * \var MemoryType::InternalMemory + * The Stream uses memory allocated internally by the library and exported to + * applications. + * \var MemoryType::ExternalMemory + * The Stream uses memory allocated externally by application and imported in + * the library. + */ + /** * \struct StreamConfiguration * \brief Configuration parameters for a stream @@ -276,7 +287,7 @@ SizeRange StreamFormats::range(unsigned int pixelformat) const * handlers provied StreamFormats. */ StreamConfiguration::StreamConfiguration() - : stream_(nullptr) + : memoryType(InternalMemory), stream_(nullptr) { } @@ -284,7 +295,7 @@ StreamConfiguration::StreamConfiguration() * \brief Construct a configuration with stream formats */ StreamConfiguration::StreamConfiguration(const StreamFormats &formats) - : stream_(nullptr), formats_(formats) + : memoryType(InternalMemory), stream_(nullptr), formats_(formats) { } @@ -301,6 +312,11 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats) * format described in V4L2 using the V4L2_PIX_FMT_* definitions. */ +/** + * \var StreamConfiguration::memoryType + * \brief The memory type the stream shall use + */ + /** * \var StreamConfiguration::bufferCount * \brief Requested number of buffers to allocate for the stream @@ -454,18 +470,26 @@ std::unique_ptr Stream::createBuffer(unsigned int index) * \return The active configuration of the stream */ +/** + * \fn Stream::memoryType() + * \brief Retrieve the stream memory type + * \return The memory type used by the stream + */ + /** * \brief Create buffers for the stream * \param count The number of buffers to create + * \param memory The stream memory type * * Create \a count empty buffers in the Stream's buffer pool. */ -void Stream::createBuffers(unsigned int count) +void Stream::createBuffers(MemoryType memory, unsigned int count) { destroyBuffers(); if (count == 0) return; + memoryType_ = memory; bufferPool_.createBuffers(count); } @@ -497,4 +521,9 @@ void Stream::destroyBuffers() * next call to Camera::configure() regardless of if it includes the stream. */ +/** + * \var Stream::memoryType_ + * \brief The stream memory type + */ + } /* namespace libcamera */ From patchwork Sat Jul 13 17:23:47 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1693 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3C9E960C23 for ; Sat, 13 Jul 2019 19:24:45 +0200 (CEST) Received: from pendragon.ideasonboard.com (softbank126209254147.bbtec.net [126.209.254.147]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 2C4852B2 for ; Sat, 13 Jul 2019 19:24:43 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1563038685; bh=TJRuO3cldATEFSTkNSOhV2RNzkwoVuSxkA+kPa9Z7A8=; h=From:To:Subject:Date:In-Reply-To:References:From; b=MQ15meMCVSFvPvijCUnDuVmw76POj1Q3ziPCXrUWYYp5E3Ywd7i146rNZBogUWzUh WKjFHkZgAs886w7ONmWA0V+9jxmokyJ9Z2OzEVF2HhCOdN6N/7eD1KtBPz2jdJASAC /YnemjJ1mkx1zfkk/UqWnXLdv5VYvc7XrC2l4R1I= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 13 Jul 2019 20:23:47 +0300 Message-Id: <20190713172351.25452-13-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> References: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 12/16] libcamera: buffer: Add dmabuf file descriptors X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 Jul 2019 17:24:45 -0000 From: Jacopo Mondi In addition to referencing buffer memory by index, add support to referencing it using dmabuf file descriptors. This will be used to reference buffer memory allocated outside of libcamera and import it. The dmabuf file descriptors are stored in an array in the Buffer class, and a new Stream::createBuffer() overload is added to construct a buffer from dmabuf file descriptor. Signed-off-by: Jacopo Mondi Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- include/libcamera/buffer.h | 3 +++ include/libcamera/stream.h | 1 + src/libcamera/buffer.cpp | 13 ++++++++++- src/libcamera/request.cpp | 5 ++++ src/libcamera/stream.cpp | 47 +++++++++++++++++++++++++++++++++++++- 5 files changed, 67 insertions(+), 2 deletions(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index f5ba6207bcef..f8569a6b67f3 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -7,6 +7,7 @@ #ifndef __LIBCAMERA_BUFFER_H__ #define __LIBCAMERA_BUFFER_H__ +#include #include #include @@ -75,6 +76,7 @@ public: Buffer &operator=(const Buffer &) = delete; unsigned int index() const { return index_; } + const std::array &dmabufs() const { return dmabuf_; } unsigned int bytesused() const { return bytesused_; } uint64_t timestamp() const { return timestamp_; } @@ -95,6 +97,7 @@ private: void setRequest(Request *request) { request_ = request; } unsigned int index_; + std::array dmabuf_; unsigned int bytesused_; uint64_t timestamp_; diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index 08eb8cc7d5c7..1883d9e9b9c5 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -75,6 +75,7 @@ public: Stream(); std::unique_ptr createBuffer(unsigned int index); + std::unique_ptr createBuffer(const std::array &fds); BufferPool &bufferPool() { return bufferPool_; } std::vector &buffers() { return bufferPool_.buffers(); } diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index ecbf25246a55..99358633a088 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -269,7 +269,8 @@ void BufferPool::destroyBuffers() * for a stream with Stream::createBuffer(). */ Buffer::Buffer(unsigned int index, const Buffer *metadata) - : index_(index), status_(Buffer::BufferSuccess), request_(nullptr), + : index_(index), dmabuf_({ -1, -1, -1 }), + status_(Buffer::BufferSuccess), request_(nullptr), stream_(nullptr) { if (metadata) { @@ -289,6 +290,16 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata) * \return The buffer index */ +/** + * \fn Buffer::dmabufs() + * \brief Retrieve the dmabuf file descriptors for all buffer planes + * + * The dmabufs array contains one dmabuf file descriptor per plane. Unused + * entries are set to -1. + * + * \return The dmabuf file descriptors + */ + /** * \fn Buffer::bytesused() * \brief Retrieve the number of bytes occupied by the data in the buffer diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index 19131472710b..ee2158fc7a9c 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -106,10 +106,15 @@ Request::~Request() * * \return 0 on success or a negative error code otherwise * \retval -EEXIST The request already contains a buffer for the stream + * \retval -EINVAL The buffer does not reference a valid Stream */ int Request::addBuffer(std::unique_ptr buffer) { Stream *stream = buffer->stream(); + if (!stream) { + LOG(Request, Error) << "Invalid stream reference"; + return -EINVAL; + } auto it = bufferMap_.find(stream); if (it != bufferMap_.end()) { diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index 94aa4810f6b9..e6aa1b643a37 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -424,17 +424,26 @@ Stream::Stream() } /** - * \brief Create a Buffer instance + * \brief Create a Buffer instance referencing the memory buffer \a index * \param[in] index The desired buffer index * * This method creates a Buffer instance that references a BufferMemory from * the stream's buffers pool by its \a index. The index shall be lower than the * number of buffers in the pool. * + * This method is only valid for streams that use the InternalMemory type. It + * will return a null pointer when called on streams using the ExternalMemory + * type. + * * \return A newly created Buffer on success or nullptr otherwise */ std::unique_ptr Stream::createBuffer(unsigned int index) { + if (memoryType_ != InternalMemory) { + LOG(Stream, Error) << "Invalid stream memory type"; + return nullptr; + } + if (index >= bufferPool_.count()) { LOG(Stream, Error) << "Invalid buffer index " << index; return nullptr; @@ -447,6 +456,42 @@ std::unique_ptr Stream::createBuffer(unsigned int index) return std::unique_ptr(buffer); } +/** + * \brief Create a Buffer instance that represents a memory area identified by + * dmabuf file descriptors + * \param[in] fds The dmabuf file descriptors for each plane + * + * This method creates a Buffer instance that references buffer memory + * allocated outside of libcamera through dmabuf file descriptors. The \a + * dmabuf array shall contain a file descriptor for each plane in the buffer, + * and unused entries shall be set to -1. + * + * The buffer is created without a valid index, as it does not yet map to any of + * the stream's BufferMemory instances. An index will be assigned at the time + * the buffer is queued to the camera in a request. Applications may thus + * create any number of Buffer instances, providing that no more than the + * number of buffers allocated for the stream are queued at any given time. + * + * This method is only valid for streams that use the InternalMemory type. It + * will return a null pointer when called on streams using the ExternalMemory + * type. + * + * \return A newly created Buffer on success or nullptr otherwise + */ +std::unique_ptr Stream::createBuffer(const std::array &fds) +{ + if (memoryType_ != ExternalMemory) { + LOG(Stream, Error) << "Invalid stream memory type"; + return nullptr; + } + + Buffer *buffer = new Buffer(); + buffer->dmabuf_ = fds; + buffer->stream_ = this; + + return std::unique_ptr(buffer); +} + /** * \fn Stream::bufferPool() * \brief Retrieve the buffer pool for the stream From patchwork Sat Jul 13 17:23:48 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1694 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C91276175D for ; Sat, 13 Jul 2019 19:24:46 +0200 (CEST) Received: from pendragon.ideasonboard.com (softbank126209254147.bbtec.net [126.209.254.147]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id A5176443 for ; Sat, 13 Jul 2019 19:24:45 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1563038686; bh=WwzHjo3EpbabPlrEc9YQlp+qaNt+gRdbsBYtHx0VJbA=; h=From:To:Subject:Date:In-Reply-To:References:From; b=q4cjq8RYMCZvjUYVWfXI/6oQ127omqO80nX65/tb/VpsGSBAUB0BXfxpWMsG9Ptn6 3gW4g1+n93RSEb8PQHg1mHOnKOjjd3akXmSUYKl5a9Dw2BMrodIeK8g+50bfR7UOvI 2gvuS6IPdZyjYNksZM3tFKWNsgza4r7UWfUHjy7w= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 13 Jul 2019 20:23:48 +0300 Message-Id: <20190713172351.25452-14-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> References: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 13/16] libcamera: buffer: Add an accessor to the BufferMemory X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 Jul 2019 17:24:47 -0000 Buffer instances reference memory, which is modelled internally by a BufferMemory instance. Store a pointer to the BufferMemory in the Buffer class, and populate it when the buffer is queued to the camera through a request. This is useful for applications to access the buffer memory in the buffer or request completion handler. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- include/libcamera/buffer.h | 3 +++ src/cam/buffer_writer.cpp | 4 ++-- src/cam/buffer_writer.h | 3 +-- src/cam/capture.cpp | 3 +-- src/libcamera/buffer.cpp | 11 +++++++++++ src/libcamera/camera.cpp | 4 ++++ src/qcam/main_window.cpp | 7 +++---- src/qcam/main_window.h | 2 +- 8 files changed, 26 insertions(+), 11 deletions(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index f8569a6b67f3..fc5c7d4c41b6 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -77,6 +77,7 @@ public: unsigned int index() const { return index_; } const std::array &dmabufs() const { return dmabuf_; } + BufferMemory *mem() { return mem_; } unsigned int bytesused() const { return bytesused_; } uint64_t timestamp() const { return timestamp_; } @@ -87,6 +88,7 @@ public: Stream *stream() const { return stream_; } private: + friend class Camera; friend class PipelineHandler; friend class Request; friend class Stream; @@ -98,6 +100,7 @@ private: unsigned int index_; std::array dmabuf_; + BufferMemory *mem_; unsigned int bytesused_; uint64_t timestamp_; diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp index b7f2ed4f2d2d..1c044b063467 100644 --- a/src/cam/buffer_writer.cpp +++ b/src/cam/buffer_writer.cpp @@ -19,8 +19,7 @@ BufferWriter::BufferWriter(const std::string &pattern) { } -int BufferWriter::write(libcamera::Buffer *buffer, libcamera::BufferMemory *mem, - const std::string &streamName) +int BufferWriter::write(libcamera::Buffer *buffer, const std::string &streamName) { std::string filename; size_t pos; @@ -41,6 +40,7 @@ int BufferWriter::write(libcamera::Buffer *buffer, libcamera::BufferMemory *mem, if (fd == -1) return -errno; + libcamera::BufferMemory *mem = buffer->mem(); for (libcamera::Plane &plane : mem->planes()) { void *data = plane.mem(); unsigned int length = plane.length(); diff --git a/src/cam/buffer_writer.h b/src/cam/buffer_writer.h index 9bea205fac1d..7bf785d1e832 100644 --- a/src/cam/buffer_writer.h +++ b/src/cam/buffer_writer.h @@ -16,8 +16,7 @@ class BufferWriter public: BufferWriter(const std::string &pattern = "frame-#.bin"); - int write(libcamera::Buffer *buffer, libcamera::BufferMemory *mem, - const std::string &streamName); + int write(libcamera::Buffer *buffer, const std::string &streamName); private: std::string pattern_; diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp index 5ffa4ae291da..df9602de4ab8 100644 --- a/src/cam/capture.cpp +++ b/src/cam/capture.cpp @@ -154,7 +154,6 @@ void Capture::requestComplete(Request *request, const std::mapfirst; Buffer *buffer = it->second; - BufferMemory *mem = &stream->buffers()[buffer->index()]; const std::string &name = streamName_[stream]; info << " " << name @@ -163,7 +162,7 @@ void Capture::requestComplete(Request *request, const std::mapbytesused(); if (writer_) - writer_->write(buffer, mem, name); + writer_->write(buffer, name); } std::cout << info.str() << std::endl; diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 99358633a088..fe29c2f68619 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -300,6 +300,17 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata) * \return The dmabuf file descriptors */ +/** + * \fn Buffer::mem() + * \brief Retrieve the BufferMemory this buffer is associated with + * + * The association between the buffer and a BufferMemory instance is valid from + * the time the request containing this buffer is queued to a camera to the end + * of that request's completion handler. + * + * \return The BufferMemory this buffer is associated with + */ + /** * \fn Buffer::bytesused() * \brief Retrieve the number of bytes occupied by the data in the buffer diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index af69607b19e6..db15fd46cd51 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -811,10 +811,14 @@ int Camera::queueRequest(Request *request) for (auto const &it : request->buffers()) { Stream *stream = it.first; + Buffer *buffer = it.second; + if (activeStreams_.find(stream) == activeStreams_.end()) { LOG(Camera, Error) << "Invalid request"; return -EINVAL; } + + buffer->mem_ = &stream->buffers()[buffer->index_]; } int ret = request->prepare(); diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 92f888323f0b..5c26ab8ebf70 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -221,7 +221,6 @@ void MainWindow::requestComplete(Request *request, framesCaptured_++; - Stream *stream = buffers.begin()->first; Buffer *buffer = buffers.begin()->second; double fps = buffer->timestamp() - lastBufferTime_; @@ -235,8 +234,7 @@ void MainWindow::requestComplete(Request *request, << " fps: " << std::fixed << std::setprecision(2) << fps << std::endl; - BufferMemory *mem = &stream->buffers()[buffer->index()]; - display(buffer, mem); + display(buffer); request = camera_->createRequest(); if (!request) { @@ -261,8 +259,9 @@ void MainWindow::requestComplete(Request *request, camera_->queueRequest(request); } -int MainWindow::display(Buffer *buffer, BufferMemory *mem) +int MainWindow::display(Buffer *buffer) { + BufferMemory *mem = buffer->mem(); if (mem->planes().size() != 1) return -EINVAL; diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h index b4f6f747e16e..f58cb6a65b2b 100644 --- a/src/qcam/main_window.h +++ b/src/qcam/main_window.h @@ -48,7 +48,7 @@ private: void requestComplete(Request *request, const std::map &buffers); - int display(Buffer *buffer, BufferMemory *mem); + int display(Buffer *buffer); QString title_; QTimer titleTimer_; From patchwork Sat Jul 13 17:23:49 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1695 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9B3CE6184A for ; Sat, 13 Jul 2019 19:24:48 +0200 (CEST) Received: from pendragon.ideasonboard.com (softbank126209254147.bbtec.net [126.209.254.147]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 741A82B2 for ; Sat, 13 Jul 2019 19:24:47 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1563038688; bh=3dTyrFsQUbZh0XLYEO9BsnS5zVsLeirk24ORz+fMC6o=; h=From:To:Subject:Date:In-Reply-To:References:From; b=CiOmlmKBzSkWdhOWeTOcSNDJPxKnN6yVQc+J1m2YdJa4BTQbBQP6ViRs7K1oW2GEJ e1MxQDi9FUzDk1Gw5AI57G5LIrDMo79zhlqcaIeNPs1N++gzISa87y8vGnOc6P+5cX OeQEOGWFlpE2dQoyI777zs9dDSLFXf3ZI0WDOWlQ= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 13 Jul 2019 20:23:49 +0300 Message-Id: <20190713172351.25452-15-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> References: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 14/16] libcamera: stream: Map external buffers to indexes X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 Jul 2019 17:24:48 -0000 From: Jacopo Mondi Add and use an operation to assign to Buffer representing external memory locations an index at queueRequest() time. The index is used to identify the memory buffer to be queued to the video device once the buffer will be queued in a Request. In order to minimize relocations in the V4L2 backend, this method provides a best-effort caching mechanisms that attempts to reuse BufferMemory previously mapped to the buffer's dmabuf file descriptors, if any. Signed-off-by: Jacopo Mondi Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- include/libcamera/buffer.h | 2 + include/libcamera/stream.h | 6 +++ src/libcamera/camera.cpp | 20 +++++++- src/libcamera/stream.cpp | 96 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 123 insertions(+), 1 deletion(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index fc5c7d4c41b6..7b657509ab5f 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -30,6 +30,8 @@ public: unsigned int length() const { return length_; } private: + friend class Stream; + int mmap(); int munmap(); diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index 1883d9e9b9c5..2e619cdf0e89 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -85,12 +85,18 @@ public: protected: friend class Camera; + int mapBuffer(const Buffer *buffer); + void unmapBuffer(const Buffer *buffer); + void createBuffers(MemoryType memory, unsigned int count); void destroyBuffers(); BufferPool bufferPool_; StreamConfiguration configuration_; MemoryType memoryType_; + +private: + std::vector, unsigned int>> bufferCache_; }; } /* namespace libcamera */ diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index db15fd46cd51..f2deb38da367 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -800,6 +800,7 @@ Request *Camera::createRequest(uint64_t cookie) * \retval -ENODEV The camera has been disconnected from the system * \retval -EACCES The camera is not running so requests can't be queued * \retval -EINVAL The request is invalid + * \retval -ENOMEM No buffer memory was available to handle the request */ int Camera::queueRequest(Request *request) { @@ -818,6 +819,16 @@ int Camera::queueRequest(Request *request) return -EINVAL; } + if (stream->memoryType() == ExternalMemory) { + int index = stream->mapBuffer(buffer); + if (index < 0) { + LOG(Camera, Error) << "No buffer memory available"; + return -ENOMEM; + } + + buffer->index_ = index; + } + buffer->mem_ = &stream->buffers()[buffer->index_]; } @@ -901,7 +912,14 @@ int Camera::stop() */ void Camera::requestComplete(Request *request) { - requestCompleted.emit(request, request->bufferMap_); + for (auto it : request->buffers()) { + Stream *stream = it.first; + Buffer *buffer = it.second; + if (stream->memoryType() == ExternalMemory) + stream->unmapBuffer(buffer); + } + + requestCompleted.emit(request, request->buffers()); delete request; } diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index e6aa1b643a37..729d36b31712 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -13,6 +13,8 @@ #include #include +#include + #include "log.h" /** @@ -476,6 +478,8 @@ std::unique_ptr Stream::createBuffer(unsigned int index) * will return a null pointer when called on streams using the ExternalMemory * type. * + * \sa Stream::mapBuffer() + * * \return A newly created Buffer on success or nullptr otherwise */ std::unique_ptr Stream::createBuffer(const std::array &fds) @@ -521,6 +525,86 @@ std::unique_ptr Stream::createBuffer(const std::array &fds) * \return The memory type used by the stream */ +/** + * \brief Map a Buffer to a buffer memory index + * \param[in] buffer The buffer to map to a buffer memory index + * + * Streams configured to use externally allocated memory need to maintain a + * best-effort association between the memory area the \a buffer represents + * and the associated buffer memory in the Stream's pool. + * + * The buffer memory to use, once the \a buffer reaches the video device, + * is selected using the index assigned to the \a buffer and to minimize + * relocations in the V4L2 back-end, this operation provides a best-effort + * caching mechanism that associates to the dmabuf file descriptors contained + * in the \a buffer the index of the buffer memory that was lastly queued with + * those file descriptors set. + * + * If the Stream uses internally allocated memory, the index of the memory + * buffer to use will match the one request at Stream::createBuffer(unsigned int) + * time, and no mapping is thus required. + * + * \return The buffer memory index for the buffer on success, or a negative + * error code otherwise + * \retval -ENOMEM No buffer memory was available to map the buffer + */ +int Stream::mapBuffer(const Buffer *buffer) +{ + ASSERT(memoryType_ == ExternalMemory); + + if (bufferCache_.empty()) + return -ENOMEM; + + const std::array &dmabufs = buffer->dmabufs(); + + /* + * Try to find a previously mapped buffer in the cache. If we miss, use + * the oldest entry in the cache. + */ + auto map = std::find_if(bufferCache_.begin(), bufferCache_.end(), + [&](std::pair, unsigned int> &entry) { + return entry.first == dmabufs; + }); + if (map == bufferCache_.end()) + map = bufferCache_.begin(); + + /* + * Update the dmabuf file descriptors of the entry. We can't assume that + * identical file descriptor numbers refer to the same dmabuf object as + * it may have been closed and its file descriptor reused. We thus need + * to update the plane's internally cached mmap()ed memory. + */ + unsigned int index = map->second; + BufferMemory *mem = &bufferPool_.buffers()[index]; + mem->planes().clear(); + + for (unsigned int i = 0; i < dmabufs.size(); ++i) { + if (dmabufs[i] == -1) + break; + + mem->planes().emplace_back(); + mem->planes().back().setDmabuf(dmabufs[i], 0); + } + + /* Remove the buffer from the cache and return its index. */ + bufferCache_.erase(map); + return index; +} + +/** + * \brief Unmap a Buffer from its buffer memory + * \param[in] buffer The buffer to unmap + * + * This method releases the buffer memory entry that was mapped by mapBuffer(), + * making it available for new mappings. + */ +void Stream::unmapBuffer(const Buffer *buffer) +{ + ASSERT(memoryType_ == ExternalMemory); + + bufferCache_.emplace_back(buffer->dmabufs(), buffer->index()); +} + /** * \brief Create buffers for the stream * \param count The number of buffers to create @@ -536,6 +620,18 @@ void Stream::createBuffers(MemoryType memory, unsigned int count) memoryType_ = memory; bufferPool_.createBuffers(count); + + /* Streams with internal memory usage do not need buffer mapping. */ + if (memoryType_ == InternalMemory) + return; + + /* + * Prepare for buffer mapping by adding all buffer memory entries to the + * cache. + */ + bufferCache_.clear(); + for (unsigned int i = 0; i < bufferPool_.count(); ++i) + bufferCache_.emplace_back(std::array{ -1, -1, -1 }, i); } /** From patchwork Sat Jul 13 17:23:50 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1696 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5B4706175B for ; Sat, 13 Jul 2019 19:24:50 +0200 (CEST) Received: from pendragon.ideasonboard.com (softbank126209254147.bbtec.net [126.209.254.147]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 0A2642B2 for ; Sat, 13 Jul 2019 19:24:48 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1563038690; bh=VGHOiKNEy9gHqqvdAPRgaT+SmF7KogL50EqUPlP/Jk4=; h=From:To:Subject:Date:In-Reply-To:References:From; b=fS/P2qQiOb9V9Ov1sgMvWTodhGXS59VwUJDunyjGhkFanRtZ8nwrKzY5T2fvL7CMC mM6w7lkyoSt85wpn7aoTf7rQQSZTe0QvzCTJmImOZJEDE4RIUJIebsA1c4K1t4xqwZ 7E4LVgg5Cn8X4xPV3v59PCvULJP0rZPgm0dmkSyo= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 13 Jul 2019 20:23:50 +0300 Message-Id: <20190713172351.25452-16-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> References: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 15/16] libcamera: pipeline: Support importing buffers X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 Jul 2019 17:24:50 -0000 From: Jacopo Mondi Add support for importing external buffers in all pipeline handlers. Use the stream memory type in the pipeline handlers during buffer allocation to import buffers to or export buffers from the video device. Signed-off-by: Jacopo Mondi Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/pipeline/ipu3/ipu3.cpp | 49 ++++++++++++++++++------ src/libcamera/pipeline/rkisp1/rkisp1.cpp | 6 ++- src/libcamera/pipeline/uvcvideo.cpp | 5 ++- src/libcamera/pipeline/vimc.cpp | 5 ++- 4 files changed, 51 insertions(+), 14 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 488e89f25c5e..9ac5a5387dec 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -69,8 +69,9 @@ public: int configureOutput(ImgUOutput *output, const StreamConfiguration &cfg); - int importBuffers(BufferPool *pool); - int exportBuffers(ImgUOutput *output, BufferPool *pool); + int importInputBuffers(BufferPool *pool); + int importOutputBuffers(ImgUOutput *output, BufferPool *pool); + int exportOutputBuffers(ImgUOutput *output, BufferPool *pool); void freeBuffers(); int start(); @@ -605,7 +606,7 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, if (!pool) return -ENOMEM; - ret = imgu->importBuffers(pool); + ret = imgu->importInputBuffers(pool); if (ret) goto error; @@ -616,7 +617,7 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, */ bufferCount = pool->count(); imgu->stat_.pool->createBuffers(bufferCount); - ret = imgu->exportBuffers(&imgu->stat_, imgu->stat_.pool); + ret = imgu->exportOutputBuffers(&imgu->stat_, imgu->stat_.pool); if (ret) goto error; @@ -625,7 +626,10 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, IPU3Stream *stream = static_cast(s); ImgUDevice::ImgUOutput *dev = stream->device_; - ret = imgu->exportBuffers(dev, &stream->bufferPool()); + if (stream->memoryType() == InternalMemory) + ret = imgu->exportOutputBuffers(dev, &stream->bufferPool()); + else + ret = imgu->importOutputBuffers(dev, &stream->bufferPool()); if (ret) goto error; } @@ -637,8 +641,8 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, if (!outStream->active_) { bufferCount = vfStream->configuration().bufferCount; outStream->device_->pool->createBuffers(bufferCount); - ret = imgu->exportBuffers(outStream->device_, - outStream->device_->pool); + ret = imgu->exportOutputBuffers(outStream->device_, + outStream->device_->pool); if (ret) goto error; } @@ -646,8 +650,8 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, if (!vfStream->active_) { bufferCount = outStream->configuration().bufferCount; vfStream->device_->pool->createBuffers(bufferCount); - ret = imgu->exportBuffers(vfStream->device_, - vfStream->device_->pool); + ret = imgu->exportOutputBuffers(vfStream->device_, + vfStream->device_->pool); if (ret) goto error; } @@ -1113,7 +1117,7 @@ int ImgUDevice::configureOutput(ImgUOutput *output, * \param[in] pool The buffer pool to import * \return 0 on success or a negative error code otherwise */ -int ImgUDevice::importBuffers(BufferPool *pool) +int ImgUDevice::importInputBuffers(BufferPool *pool) { int ret = input_->importBuffers(pool); if (ret) { @@ -1134,7 +1138,7 @@ int ImgUDevice::importBuffers(BufferPool *pool) * * \return 0 on success or a negative error code otherwise */ -int ImgUDevice::exportBuffers(ImgUOutput *output, BufferPool *pool) +int ImgUDevice::exportOutputBuffers(ImgUOutput *output, BufferPool *pool) { int ret = output->dev->exportBuffers(pool); if (ret) { @@ -1146,6 +1150,29 @@ int ImgUDevice::exportBuffers(ImgUOutput *output, BufferPool *pool) return 0; } +/** + * \brief Reserve buffers in \a output from the provided \a pool + * \param[in] output The ImgU output device + * \param[in] pool The buffer pool used to reserve buffers in \a output + * + * Reserve a number of buffers equal to the number of buffers in \a pool + * in the \a output device. + * + * \return 0 on success or a negative error code otherwise + */ +int ImgUDevice::importOutputBuffers(ImgUOutput *output, BufferPool *pool) +{ + int ret = output->dev->importBuffers(pool); + if (ret) { + LOG(IPU3, Error) + << "Failed to import buffer in " << output->name + << " ImgU device"; + return ret; + } + + return 0; +} + /** * \brief Release buffers for all the ImgU video devices */ diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index cc33a2cb2572..efa9604bc1e3 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -319,7 +319,11 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera, const std::set &streams) { Stream *stream = *streams.begin(); - return video_->exportBuffers(&stream->bufferPool()); + + if (stream->memoryType() == InternalMemory) + return video_->exportBuffers(&stream->bufferPool()); + else + return video_->importBuffers(&stream->bufferPool()); } int PipelineHandlerRkISP1::freeBuffers(Camera *camera, diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index b299c5da8471..8965210550d2 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -200,7 +200,10 @@ int PipelineHandlerUVC::allocateBuffers(Camera *camera, LOG(UVC, Debug) << "Requesting " << cfg.bufferCount << " buffers"; - return data->video_->exportBuffers(&stream->bufferPool()); + if (stream->memoryType() == InternalMemory) + return data->video_->exportBuffers(&stream->bufferPool()); + else + return data->video_->importBuffers(&stream->bufferPool()); } int PipelineHandlerUVC::freeBuffers(Camera *camera, diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 7d96c3645c06..61b68a32ea50 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -202,7 +202,10 @@ int PipelineHandlerVimc::allocateBuffers(Camera *camera, LOG(VIMC, Debug) << "Requesting " << cfg.bufferCount << " buffers"; - return data->video_->exportBuffers(&stream->bufferPool()); + if (stream->memoryType() == InternalMemory) + return data->video_->exportBuffers(&stream->bufferPool()); + else + return data->video_->importBuffers(&stream->bufferPool()); } int PipelineHandlerVimc::freeBuffers(Camera *camera, From patchwork Sat Jul 13 17:23:51 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1697 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2073660C23 for ; Sat, 13 Jul 2019 19:24:52 +0200 (CEST) Received: from pendragon.ideasonboard.com (softbank126209254147.bbtec.net [126.209.254.147]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id CFE1D443 for ; Sat, 13 Jul 2019 19:24:50 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1563038691; bh=dEx/9rE0de8TZG4UG9BJzrtnnGFClrqNJCCsYS0sLeQ=; h=From:To:Subject:Date:In-Reply-To:References:From; b=j8P8/rHmxDkHRYmQ9vpXkfDh4LWlF2KiZxTSPpMKMkBskyEABp7eqv8q8eMcIw5Sl KqP0a60Ahe+tFTB0daiHJbCIypPPdbiSrNiJ/z1l6IN3mlPp6Y75xa2HjsOXT9oJWY q6uEp+CzulBr0SvoTanthpUMAY6dhRmV/yE7pr7c= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 13 Jul 2019 20:23:51 +0300 Message-Id: <20190713172351.25452-17-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> References: <20190713172351.25452-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 16/16] test: camera: Add buffer import and mapping test X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 Jul 2019 17:24:52 -0000 From: Jacopo Mondi Test buffer importing and mapping by streaming the VIMC camera to VIVID video output device performing zero-copy memory sharing using dmabuf file descriptors. The test cycle 20 buffers between the camera and the output with a 1:1 buffer index to dmabuf fd mapping, then randomises the mapping with the same number of buffers on each side for 20 more frames, to finally increase the number of buffers on the output side for the 20 last frames. No remapping of dmabuf fd to buffer index should occur for the first 40 frames. Signed-off-by: Jacopo Mondi Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- test/camera/buffer_import.cpp | 425 ++++++++++++++++++++++++++++++++++ test/camera/meson.build | 1 + 2 files changed, 426 insertions(+) create mode 100644 test/camera/buffer_import.cpp diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp new file mode 100644 index 000000000000..d6e4fd5bf6ad --- /dev/null +++ b/test/camera/buffer_import.cpp @@ -0,0 +1,425 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * libcamera Camera API tests + * + * Test importing buffers exported from the VIVID output device into a Camera + */ + +#include +#include +#include +#include +#include + +#include "device_enumerator.h" +#include "media_device.h" +#include "v4l2_videodevice.h" + +#include "camera_test.h" + +using namespace libcamera; + +/* Keep SINK_BUFFER_COUNT > CAMERA_BUFFER_COUNT + 1 */ +static constexpr unsigned int SINK_BUFFER_COUNT = 8; +static constexpr unsigned int CAMERA_BUFFER_COUNT = 4; + +class FrameSink +{ +public: + int init() + { + int ret; + + /* Locate and open the video device. */ + std::string videoDeviceName = "vivid-000-vid-out"; + + std::unique_ptr enumerator = + DeviceEnumerator::create(); + if (!enumerator) { + std::cout << "Failed to create device enumerator" << std::endl; + return TestFail; + } + + if (enumerator->enumerate()) { + std::cout << "Failed to enumerate media devices" << std::endl; + return TestFail; + } + + DeviceMatch dm("vivid"); + dm.add(videoDeviceName); + + media_ = enumerator->search(dm); + if (!media_) { + std::cout << "No vivid output device available" << std::endl; + return TestSkip; + } + + video_ = V4L2VideoDevice::fromEntityName(media_.get(), videoDeviceName); + if (!video_) { + std::cout << "Unable to open " << videoDeviceName << std::endl; + return TestFail; + } + + if (video_->open()) + return TestFail; + + /* Configure the format. */ + ret = video_->getFormat(&format_); + if (ret) { + std::cout << "Failed to get format on output device" << std::endl; + return ret; + } + + format_.size.width = 640; + format_.size.height = 480; + format_.fourcc = V4L2_PIX_FMT_RGB24; + format_.planesCount = 1; + format_.planes[0].size = 640 * 480 * 3; + format_.planes[0].bpl = 640 * 3; + + if (video_->setFormat(&format_)) { + cleanup(); + return TestFail; + } + + /* Export the buffers to a pool. */ + pool_.createBuffers(SINK_BUFFER_COUNT); + ret = video_->exportBuffers(&pool_); + if (ret) { + std::cout << "Failed to export buffers" << std::endl; + cleanup(); + return TestFail; + } + + /* Only use the first CAMERA_BUFFER_COUNT buffers to start with. */ + availableBuffers_.resize(CAMERA_BUFFER_COUNT); + std::iota(availableBuffers_.begin(), availableBuffers_.end(), 0); + + /* Connect the buffer ready signal. */ + video_->bufferReady.connect(this, &FrameSink::bufferComplete); + + return TestPass; + } + + void cleanup() + { + if (video_) { + video_->streamOff(); + video_->releaseBuffers(); + video_->close(); + delete video_; + } + + if (media_) + media_->release(); + } + + int start() + { + requestsCount_ = 0; + done_ = false; + + int ret = video_->streamOn(); + if (ret < 0) + return ret; + + /* Queue all the initial requests. */ + for (unsigned int index = 0; index < CAMERA_BUFFER_COUNT; ++index) + queueRequest(index); + + return 0; + } + + int stop() + { + return video_->streamOff(); + } + + void requestComplete(uint64_t cookie, const Buffer *metadata) + { + unsigned int index = cookie; + + Buffer *buffer = new Buffer(index, metadata); + int ret = video_->queueBuffer(buffer); + if (ret < 0) + std::cout << "Failed to queue buffer to sink" << std::endl; + } + + bool done() const { return done_; } + const V4L2DeviceFormat &format() const { return format_; } + + Signal requestReady; + +private: + void queueRequest(unsigned int index) + { + auto it = std::find(availableBuffers_.begin(), + availableBuffers_.end(), index); + availableBuffers_.erase(it); + + uint64_t cookie = index; + BufferMemory &mem = pool_.buffers()[index]; + int dmabuf = mem.planes()[0].dmabuf(); + + requestReady.emit(cookie, dmabuf); + + requestsCount_++; + } + + void bufferComplete(Buffer *buffer) + { + availableBuffers_.push_back(buffer->index()); + + /* + * Pick the buffer for the next request among the available + * buffers. + * + * For the first 20 frames, select the buffer that has just + * completed to keep the mapping of dmabuf fds to buffers + * unchanged in the camera. + * + * For the next 20 frames, cycle randomly over the available + * buffers. The mapping should still be kept unchanged, as the + * camera should map using the cached fds. + * + * For the last 20 frames, cycles through all buffers, which + * should trash the mappings. + */ + unsigned int index = buffer->index(); + delete buffer; + + std::cout << "Completed buffer, request=" << requestsCount_ + << ", available buffers=" << availableBuffers_.size() + << std::endl; + + if (requestsCount_ >= 60) { + if (availableBuffers_.size() == SINK_BUFFER_COUNT) + done_ = true; + return; + } + + if (requestsCount_ == 40) { + /* Add the remaining of the buffers. */ + for (unsigned int i = CAMERA_BUFFER_COUNT; + i < SINK_BUFFER_COUNT; ++i) + availableBuffers_.push_back(i); + } + + if (requestsCount_ >= 20) { + /* + * Wait until we have enough buffers to make this + * meaningful. Preferably half of the camera buffers, + * but no less than 2 in any case. + */ + const unsigned int min_pool_size = + std::min(CAMERA_BUFFER_COUNT / 2, 2U); + if (availableBuffers_.size() < min_pool_size) + return; + + /* Pick a buffer at random. */ + unsigned int pos = random_() % availableBuffers_.size(); + index = availableBuffers_[pos]; + } + + queueRequest(index); + } + + std::shared_ptr media_; + V4L2VideoDevice *video_; + BufferPool pool_; + V4L2DeviceFormat format_; + + unsigned int requestsCount_; + std::vector availableBuffers_; + std::random_device random_; + + bool done_; +}; + +class BufferImportTest : public CameraTest +{ +public: + BufferImportTest() + : CameraTest() + { + } + + void queueRequest(uint64_t cookie, int dmabuf) + { + Request *request = camera_->createRequest(cookie); + + std::unique_ptr buffer = stream_->createBuffer({ dmabuf, -1, -1 }); + request->addBuffer(move(buffer)); + camera_->queueRequest(request); + } + +protected: + void bufferComplete(Request *request, Buffer *buffer) + { + if (buffer->status() != Buffer::BufferSuccess) + return; + + unsigned int index = buffer->index(); + int dmabuf = buffer->dmabufs()[0]; + + /* Record dmabuf to index remappings. */ + bool remapped = false; + if (bufferMappings_.find(index) != bufferMappings_.end()) { + if (bufferMappings_[index] != dmabuf) + remapped = true; + } + + std::cout << "Completed request " << framesCaptured_ + << ": dmabuf fd " << dmabuf + << " -> index " << index + << " (" << (remapped ? 'R' : '-') << ")" + << std::endl; + + if (remapped) + bufferRemappings_.push_back(framesCaptured_); + + bufferMappings_[index] = dmabuf; + framesCaptured_++; + + sink_.requestComplete(request->cookie(), buffer); + + if (framesCaptured_ == 60) + sink_.stop(); + } + + int initCamera() + { + if (camera_->acquire()) { + std::cout << "Failed to acquire the camera" << std::endl; + return TestFail; + } + + /* + * Configure the Stream to work with externally allocated + * buffers by setting the memoryType to ExternalMemory. + */ + std::unique_ptr config; + config = camera_->generateConfiguration({ StreamRole::VideoRecording }); + if (!config || config->size() != 1) { + std::cout << "Failed to generate configuration" << std::endl; + return TestFail; + } + + const V4L2DeviceFormat &format = sink_.format(); + + StreamConfiguration &cfg = config->at(0); + cfg.size = format.size; + cfg.pixelFormat = format.fourcc; + cfg.bufferCount = CAMERA_BUFFER_COUNT; + cfg.memoryType = ExternalMemory; + + if (camera_->configure(config.get())) { + std::cout << "Failed to set configuration" << std::endl; + return TestFail; + } + + stream_ = cfg.stream(); + + /* Allocate buffers. */ + if (camera_->allocateBuffers()) { + std::cout << "Failed to allocate buffers" << std::endl; + return TestFail; + } + + /* Connect the buffer completed signal. */ + camera_->bufferCompleted.connect(this, &BufferImportTest::bufferComplete); + + return TestPass; + } + + int init() + { + int ret = CameraTest::init(); + if (ret) + return ret; + + ret = sink_.init(); + if (ret != TestPass) { + cleanup(); + return ret; + } + + ret = initCamera(); + if (ret != TestPass) { + cleanup(); + return ret; + } + + sink_.requestReady.connect(this, &BufferImportTest::queueRequest); + return TestPass; + } + + int run() + { + int ret; + + framesCaptured_ = 0; + + if (camera_->start()) { + std::cout << "Failed to start camera" << std::endl; + return TestFail; + } + + ret = sink_.start(); + if (ret < 0) { + std::cout << "Failed to start sink" << std::endl; + return TestFail; + } + + EventDispatcher *dispatcher = CameraManager::instance()->eventDispatcher(); + + Timer timer; + timer.start(3000); + while (timer.isRunning() && !sink_.done()) + dispatcher->processEvents(); + + std::cout << framesCaptured_ << " frames captured, " + << bufferRemappings_.size() << " buffers remapped" + << std::endl; + + if (framesCaptured_ < 60) { + std::cout << "Too few frames captured" << std::endl; + return TestFail; + } + + if (bufferRemappings_.empty()) { + std::cout << "No buffer remappings" << std::endl; + return TestFail; + } + + if (bufferRemappings_[0] < 40) { + std::cout << "Early buffer remapping" << std::endl; + return TestFail; + } + + return TestPass; + } + + void cleanup() + { + sink_.cleanup(); + + camera_->stop(); + camera_->freeBuffers(); + + CameraTest::cleanup(); + } + +private: + Stream *stream_; + + std::map bufferMappings_; + std::vector bufferRemappings_; + unsigned int framesCaptured_; + + FrameSink sink_; +}; + +TEST_REGISTER(BufferImportTest); diff --git a/test/camera/meson.build b/test/camera/meson.build index 35e97ce5de1a..d6fd66b8f89e 100644 --- a/test/camera/meson.build +++ b/test/camera/meson.build @@ -3,6 +3,7 @@ camera_tests = [ [ 'configuration_default', 'configuration_default.cpp' ], [ 'configuration_set', 'configuration_set.cpp' ], + [ 'buffer_import', 'buffer_import.cpp' ], [ 'statemachine', 'statemachine.cpp' ], [ 'capture', 'capture.cpp' ], ]