From patchwork Tue Aug 31 22:37:03 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 13588 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id DE030C3243 for ; Tue, 31 Aug 2021 22:37:47 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 53D5F69180; Wed, 1 Sep 2021 00:37:46 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 10A5060258 for ; Wed, 1 Sep 2021 00:37:36 +0200 (CEST) Received: from localhost.localdomain (unknown [IPv6:2804:14c:1a9:2434:b693:c9:5cb6:b688]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: nfraprado) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 355441F43958; Tue, 31 Aug 2021 23:37:33 +0100 (BST) From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Tue, 31 Aug 2021 19:37:03 -0300 Message-Id: <20210831223705.1928000-4-nfraprado@collabora.com> X-Mailer: git-send-email 2.33.0 In-Reply-To: <20210831223705.1928000-1-nfraprado@collabora.com> References: <20210831223705.1928000-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 3/5] libcamera: pipeline: rkisp1: Add internal request queue X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kernel@collabora.com, =?utf-8?q?Andr=C3=A9_Almeida?= Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Add an internal queue that stores requests until there are internal buffers and V4L2 buffer slots available. This avoids the need to cancel requests when there is a shortage of said buffers. Signed-off-by: Nícolas F. R. A. Prado Reviewed-by: Laurent Pinchart --- Changes in v2: - Moved cancellation of pending requests to after video devices stop src/libcamera/pipeline/rkisp1/rkisp1.cpp | 73 +++++++++++++++++++----- 1 file changed, 59 insertions(+), 14 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 03a8d1d26e30..6aca4083af72 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -91,6 +91,9 @@ public: PipelineHandlerRkISP1 *pipe(); int loadIPA(unsigned int hwRevision); + void queuePendingRequests(); + void cancelPendingRequests(); + Stream mainPathStream_; Stream selfPathStream_; std::unique_ptr sensor_; @@ -104,6 +107,8 @@ public: std::unique_ptr ipa_; + std::queue pendingRequests_; + private: void queueFrameAction(unsigned int frame, const ipa::rkisp1::RkISP1Action &action); @@ -210,13 +215,13 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req unsigned int frame = data->frame_; if (pipe_->availableParamBuffers_.empty()) { - LOG(RkISP1, Error) << "Parameters buffer underrun"; + LOG(RkISP1, Debug) << "Parameters buffer underrun"; return nullptr; } FrameBuffer *paramBuffer = pipe_->availableParamBuffers_.front(); if (pipe_->availableStatBuffers_.empty()) { - LOG(RkISP1, Error) << "Statisitc buffer underrun"; + LOG(RkISP1, Debug) << "Statistic buffer underrun"; return nullptr; } FrameBuffer *statBuffer = pipe_->availableStatBuffers_.front(); @@ -386,6 +391,52 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta pipe()->tryCompleteRequest(info->request); } +void RkISP1CameraData::queuePendingRequests() +{ + while (!pendingRequests_.empty()) { + Request *request = pendingRequests_.front(); + + /* + * If there aren't internal buffers available, we break and try + * again later. If there are, we're guaranteed to also have V4L2 + * buffer slots available to queue the request, since we should + * always have more (or equal) buffer slots than internal + * buffers. + */ + RkISP1FrameInfo *info = frameInfo_.create(this, request); + if (!info) + break; + + ipa::rkisp1::RkISP1Event ev; + ev.op = ipa::rkisp1::EventQueueRequest; + ev.frame = frame_; + ev.bufferId = info->paramBuffer->cookie(); + ev.controls = request->controls(); + ipa_->processEvent(ev); + + frame_++; + + pendingRequests_.pop(); + } +} + +void RkISP1CameraData::cancelPendingRequests() +{ + while (!pendingRequests_.empty()) { + Request *request = pendingRequests_.front(); + + for (auto it : request->buffers()) { + FrameBuffer *buffer = it.second; + buffer->cancel(); + pipe()->completeBuffer(request, buffer); + } + + pipe()->completeRequest(request); + + pendingRequests_.pop(); + } +} + RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera, RkISP1CameraData *data) : CameraConfiguration() @@ -855,6 +906,8 @@ void PipelineHandlerRkISP1::stop(Camera *camera) LOG(RkISP1, Warning) << "Failed to stop parameters for " << camera->id(); + data->cancelPendingRequests(); + ASSERT(data->queuedRequests_.empty()); data->frameInfo_.clear(); @@ -867,18 +920,8 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request) { RkISP1CameraData *data = cameraData(camera); - RkISP1FrameInfo *info = data->frameInfo_.create(data, request); - if (!info) - return -ENOENT; - - ipa::rkisp1::RkISP1Event ev; - ev.op = ipa::rkisp1::EventQueueRequest; - ev.frame = data->frame_; - ev.bufferId = info->paramBuffer->cookie(); - ev.controls = request->controls(); - data->ipa_->processEvent(ev); - - data->frame_++; + data->pendingRequests_.push(request); + data->queuePendingRequests(); return 0; } @@ -1077,6 +1120,8 @@ void PipelineHandlerRkISP1::tryCompleteRequest(Request *request) data->frameInfo_.destroy(info->frame); completeRequest(request); + + data->queuePendingRequests(); } void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer) From patchwork Tue Aug 31 22:37:05 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 13589 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 725EFC3245 for ; Tue, 31 Aug 2021 22:37:48 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D30F269178; Wed, 1 Sep 2021 00:37:47 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id CE1F06916F for ; Wed, 1 Sep 2021 00:37:40 +0200 (CEST) Received: from localhost.localdomain (unknown [IPv6:2804:14c:1a9:2434:b693:c9:5cb6:b688]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: nfraprado) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 06F8D1F43973; Tue, 31 Aug 2021 23:37:38 +0100 (BST) From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Tue, 31 Aug 2021 19:37:05 -0300 Message-Id: <20210831223705.1928000-6-nfraprado@collabora.com> X-Mailer: git-send-email 2.33.0 In-Reply-To: <20210831223705.1928000-1-nfraprado@collabora.com> References: <20210831223705.1928000-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 5/5] Documentation: guides: pipeline-handler: Document internal queue pattern X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kernel@collabora.com, =?utf-8?q?Andr=C3=A9_Almeida?= Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Pipeline handlers need to implement a common pattern of adding queued requests to an internal queue and queuing them to the video devices as the buffer slots there become available. Add this pattern to the vivid example in the pipeline-handler guide so it's clear that new pipeline handlers should also implement it. Signed-off-by: Nícolas F. R. A. Prado --- Changes in v2: - New Documentation/guides/pipeline-handler.rst | 129 +++++++++++++++++----- 1 file changed, 99 insertions(+), 30 deletions(-) diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst index 3ee79b98c4dc..96d4490d06fa 100644 --- a/Documentation/guides/pipeline-handler.rst +++ b/Documentation/guides/pipeline-handler.rst @@ -447,11 +447,27 @@ it will be used: int init(); void bufferReady(FrameBuffer *buffer); + void queuePendingRequests(); + void cancelPendingRequests(); + + void setAvailableBufferSlotCount(unsigned int count) { availableBufferSlotCount_ = count; } + MediaDevice *media_; V4L2VideoDevice *video_; Stream stream_; + + std::queue pendingRequests_; + + private: + unsigned int availableBufferSlotCount_; }; +And the following include needs to be added: + +.. code-block:: cpp + + #include + This example pipeline handler handles a single video device and supports a single stream, represented by the ``VividCameraData`` class members. More complex pipeline handlers might register cameras composed of several video @@ -1218,7 +1234,8 @@ algorithms, or other devices you should also stop them. Of course we also need to handle the corresponding actions to stop streaming on a device, Add the following to the ``stop`` function, to stop the stream with -the `streamOff`_ function and release all buffers. +the `streamOff`_ function and release all buffers. Additionally we need to +cancel pending requests, the reason will become clearer in the next section. .. _streamOff: http://libcamera.org/api-html/classlibcamera_1_1V4L2VideoDevice.html#a61998710615bdf7aa25a046c8565ed66 @@ -1226,6 +1243,7 @@ the `streamOff`_ function and release all buffers. VividCameraData *data = cameraData(camera); data->video_->streamOff(); + data->cancelPendingRequests(); data->video_->releaseBuffers(); Queuing requests between applications and hardware @@ -1246,25 +1264,76 @@ directly with the `queueBuffer`_ function provided by the V4L2VideoDevice. .. _findBuffer: http://libcamera.org/api-html/classlibcamera_1_1Request.html#ac66050aeb9b92c64218945158559c4d4 .. _queueBuffer: http://libcamera.org/api-html/classlibcamera_1_1V4L2VideoDevice.html#a594cd594686a8c1cf9ae8dba0b2a8a75 +Since the pipeline handler has a finite number of buffer slots allocated through +importBuffers(), in order to not drop frames when more requests than that are +queued by the application, we need to implement a queue to hold the requests and +queue them to the devices as slots become available. + Replace the stubbed contents of ``queueRequestDevice`` with the following: .. code-block:: cpp VividCameraData *data = cameraData(camera); - FrameBuffer *buffer = request->findBuffer(&data->stream_); - if (!buffer) { + if (!request->findBuffer(&data->stream_)) { LOG(VIVID, Error) << "Attempt to queue request with invalid stream"; return -ENOENT; } - int ret = data->video_->queueBuffer(buffer); - if (ret < 0) - return ret; + data->pendingRequests_.push(request); + data->queuePendingRequests(); return 0; +Now create the ``queuePendingRequests`` function for ``VividCameraData`` and +inside it add: + +.. code-block:: cpp + + while (!pendingRequests_.empty() && availableBufferSlotCount_) { + Request *request = pendingRequests_.front(); + FrameBuffer *buffer = request->findBuffer(&stream_); + + ret = video_->queueBuffer(buffer); + if (ret < 0) { + LOG(UVC, Error) << "Failed to queue buffer with error " + << ret << ". Cancelling buffer."; + buffer->cancel(); + pipe()->completeBuffer(request, buffer); + pipe()->completeRequest(request); + pendingRequests_.pop(); + + continue; + } + + availableBufferSlotCount_--; + + pendingRequests_.pop(); + } + +Create ``cancelPendingRequests`` as well with: + +.. code-block:: cpp + + while (!pendingRequests_.empty()) { + Request *request = pendingRequests_.front(); + FrameBuffer *buffer = request->findBuffer(&stream_); + + buffer->cancel(); + pipe()->completeBuffer(request, buffer); + pipe()->completeRequest(request); + + pendingRequests_.pop(); + } + +Finally we need to initialize the buffer slot count in ``start()``. Add the +following line there: + +.. code-block:: cpp + + data->setAvailableBufferSlotCount(count); + Processing controls ~~~~~~~~~~~~~~~~~~~ @@ -1339,31 +1408,27 @@ where appropriate by setting controls on V4L2Subdevices directly. Each pipeline handler is responsible for understanding the correct procedure for applying controls to the device they support. -This example pipeline handler applies controls during the `queueRequestDevice`_ -function for each request, and applies them to the capture device through the -capture node. - -.. _queueRequestDevice: http://libcamera.org/api-html/classlibcamera_1_1PipelineHandler.html#a106914cca210640c9da9ee1f0419e83c +This example pipeline handler applies controls as soon as the request is queued +to the device from ``queuePendingRequests``, and applies them to the capture +device through the capture node. -In the ``queueRequestDevice`` function, replace the following: +In the ``queuePendingRequests`` function, before the line ``ret = +video_->queueBuffer(buffer);``, add the following: .. code-block:: cpp - int ret = data->video_->queueBuffer(buffer); - if (ret < 0) - return ret; - -With the following code: - -.. code-block:: cpp - - int ret = processControls(data, request); - if (ret < 0) - return ret; - - ret = data->video_->queueBuffer(buffer); - if (ret < 0) - return ret; + int ret = processControls(request); + if (ret < 0) { + LOG(UVC, Error) << "Failed to process controls with" + << " error " << ret << ". Cancelling" + << " buffer."; + buffer->cancel(); + pipe()->completeBuffer(request, buffer); + pipe()->completeRequest(request); + pendingRequests_.pop(); + + continue; + } We also need to add the following include directive to support the control value translation operations: @@ -1427,9 +1492,10 @@ VividCameradata::init() impelementation. The ``bufferReady`` function obtains the request from the buffer using the ``request`` function, and notifies the ``Camera`` that the buffer and request are completed. In this simpler pipeline handler, there is only one -stream, so it completes the request immediately. You can find a more complex -example of event handling with supporting multiple streams in the libcamera -code-base. +stream, so it completes the request immediately. It also updates the number of +available buffer slots and tries to queue any pending requests. You can find a +more complex example of event handling with support to multiple streams in the +libcamera code-base. .. TODO: Add link @@ -1441,6 +1507,9 @@ code-base. pipe_->completeBuffer(request, buffer); pipe_->completeRequest(request); + + availableBufferSlotCount_++; + queuePendingRequests(); } Testing a pipeline handler