From patchwork Tue Aug 31 22:37:02 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: 23009 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by patchwork.libcamera.org (Postfix) with ESMTPS id CAAEEC0DA4 for ; Mon, 24 Mar 2025 17:12:39 +0000 (UTC) Received: from pendragon.ideasonboard.com (cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 522C8455 for ; Mon, 24 Mar 2025 18:10:53 +0100 (CET) Delivered-To: kbingham@ideasonboard.com Received: from perceval.ideasonboard.com by perceval.ideasonboard.com with LMTP id yBcMHjqvLmFtcAAA4E0KoQ (envelope-from ) for ; Wed, 01 Sep 2021 00:37:46 +0200 Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by perceval.ideasonboard.com (Postfix) with ESMTPS id 6D8DC8F; Wed, 1 Sep 2021 00:37:46 +0200 (CEST) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1302F6917E; 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 BAB6760258 for ; Wed, 1 Sep 2021 00:37:33 +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 D32D61F43866; Tue, 31 Aug 2021 23:37:31 +0100 (BST) From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Tue, 31 Aug 2021 19:37:02 -0300 Message-Id: <20210831223705.1928000-3-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 2/5] libcamera: pipeline: uvcvideo: 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" X-TUID: 4UL7mSPzsoKm Resent-From: Kieran Bingham Resent-To: parsemail@patchwork.libcamera.org Add an internal queue that stores requests until there are 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 --- Changes in v2: - Added a counter to keep track of the number of available buffer slots - Moved cancellation of pending requests to after video devices stop - Added error log on failure to process controls src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 98 ++++++++++++++++---- 1 file changed, 81 insertions(+), 17 deletions(-) diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp index 5977312a795d..5e73a8e682dd 100644 --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -46,8 +47,22 @@ public: ControlInfoMap::Map *ctrls); void bufferReady(FrameBuffer *buffer); + void queuePendingRequests(); + void cancelPendingRequests(); + + void setAvailableBufferSlotCount(unsigned int count) { availableBufferSlotCount_ = count; } + std::unique_ptr video_; Stream stream_; + + std::queue pendingRequests_; + +private: + int processControls(Request *request); + int processControl(ControlList *controls, unsigned int id, + const ControlValue &value); + + unsigned int availableBufferSlotCount_; }; class UVCCameraConfiguration : public CameraConfiguration @@ -83,10 +98,6 @@ public: private: std::string generateId(const UVCCameraData *data); - int processControl(ControlList *controls, unsigned int id, - const ControlValue &value); - int processControls(UVCCameraData *data, Request *request); - UVCCameraData *cameraData(Camera *camera) { return static_cast(camera->_d()); @@ -240,6 +251,8 @@ int PipelineHandlerUVC::start(Camera *camera, [[maybe_unused]] const ControlList if (ret < 0) return ret; + data->setAvailableBufferSlotCount(kUVCBufferSlotCount); + ret = data->video_->streamOn(); if (ret < 0) { data->video_->releaseBuffers(); @@ -253,10 +266,11 @@ void PipelineHandlerUVC::stop(Camera *camera) { UVCCameraData *data = cameraData(camera); data->video_->streamOff(); + data->cancelPendingRequests(); data->video_->releaseBuffers(); } -int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id, +int UVCCameraData::processControl(ControlList *controls, unsigned int id, const ControlValue &value) { uint32_t cid; @@ -337,9 +351,9 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id, return 0; } -int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) +int UVCCameraData::processControls(Request *request) { - ControlList controls(data->video_->controls()); + ControlList controls(video_->controls()); for (auto it : request->controls()) { unsigned int id = it.first; @@ -353,7 +367,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) << "Setting control " << utils::hex(ctrl.first) << " to " << ctrl.second.toString(); - int ret = data->video_->setControls(&controls); + int ret = video_->setControls(&controls); if (ret) { LOG(UVC, Error) << "Failed to set controls: " << ret; return ret < 0 ? ret : -EINVAL; @@ -365,21 +379,16 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request) { UVCCameraData *data = cameraData(camera); - FrameBuffer *buffer = request->findBuffer(&data->stream_); - if (!buffer) { + + if (!request->findBuffer(&data->stream_)) { LOG(UVC, Error) << "Attempt to queue request with invalid stream"; return -ENOENT; } - int ret = processControls(data, request); - if (ret < 0) - return ret; - - ret = data->video_->queueBuffer(buffer); - if (ret < 0) - return ret; + data->pendingRequests_.push(request); + data->queuePendingRequests(); return 0; } @@ -670,6 +679,61 @@ void UVCCameraData::bufferReady(FrameBuffer *buffer) pipe()->completeBuffer(request, buffer); pipe()->completeRequest(request); + + availableBufferSlotCount_++; + + queuePendingRequests(); +} + +void UVCCameraData::queuePendingRequests() +{ + while (!pendingRequests_.empty() && availableBufferSlotCount_) { + Request *request = pendingRequests_.front(); + FrameBuffer *buffer = request->findBuffer(&stream_); + + 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; + } + + 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(); + } +} + +void UVCCameraData::cancelPendingRequests() +{ + while (!pendingRequests_.empty()) { + Request *request = pendingRequests_.front(); + FrameBuffer *buffer = request->findBuffer(&stream_); + + buffer->cancel(); + pipe()->completeBuffer(request, buffer); + pipe()->completeRequest(request); + + pendingRequests_.pop(); + } } REGISTER_PIPELINE_HANDLER(PipelineHandlerUVC) 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:04 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: 23010 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by patchwork.libcamera.org (Postfix) with ESMTPS id B7D95C0DA4 for ; Mon, 24 Mar 2025 17:12:47 +0000 (UTC) Received: from pendragon.ideasonboard.com (cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 40F6CA8F for ; Mon, 24 Mar 2025 18:11:01 +0100 (CET) Delivered-To: kbingham@ideasonboard.com Received: from perceval.ideasonboard.com by perceval.ideasonboard.com with LMTP id 2NvwBTuvLmFucAAA4E0KoQ (envelope-from ) for ; Wed, 01 Sep 2021 00:37:47 +0200 Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by perceval.ideasonboard.com (Postfix) with ESMTPS id 07BAC8F; Wed, 1 Sep 2021 00:37:47 +0200 (CEST) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 9BB6069183; 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 81E6B69166 for ; Wed, 1 Sep 2021 00:37:38 +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 8E51C1F4396E; Tue, 31 Aug 2021 23:37:36 +0100 (BST) From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Tue, 31 Aug 2021 19:37:04 -0300 Message-Id: <20210831223705.1928000-5-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 4/5] libcamera: pipeline: simple: 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" X-TUID: mPWV82qDk+0V Resent-From: Kieran Bingham Resent-To: parsemail@patchwork.libcamera.org 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 --- I wasn't able to test this patch as I don't have any of the target hardware. Changes in v2: - New src/libcamera/pipeline/simple/simple.cpp | 101 ++++++++++++++++++----- 1 file changed, 81 insertions(+), 20 deletions(-) diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 6deba5d7dd61..e9abe5e291f4 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -167,6 +167,9 @@ public: int setupFormats(V4L2SubdeviceFormat *format, V4L2Subdevice::Whence whence); + void queuePendingRequests(); + void cancelPendingRequests(); + unsigned int streamIndex(const Stream *stream) const { return stream - &streams_.front(); @@ -198,6 +201,10 @@ public: std::queue> converterQueue_; const SimplePipelineInfo *deviceInfo; + + std::queue pendingRequests_; + + unsigned int avaliableBufferSlotCount_; }; class SimpleCameraConfiguration : public CameraConfiguration @@ -546,6 +553,66 @@ int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format, return 0; } +void SimpleCameraData::queuePendingRequests() +{ + while (!pendingRequests_.empty() && avaliableBufferSlotCount_) { + Request *request = pendingRequests_.front(); + + if (useConverter_) { + /* + * If conversion is needed, push the buffer(s) to the + * converter queue, it will be handed to the converter + * in the capture completion handler. + */ + std::map buffers; + + for (auto &[stream, buffer] : request->buffers()) + buffers.emplace(streamIndex(stream), buffer); + + converterQueue_.push(std::move(buffers)); + } else { + /* + * Since we're not using a converter we can assume + * there's a single stream. + */ + FrameBuffer *buffer = request->buffers().at(0); + + int ret = video_->queueBuffer(buffer); + if (ret < 0) { + LOG(SimplePipeline, Error) + << "Failed to queue buffer with error " + << ret << ". Cancelling buffer."; + buffer->cancel(); + pipe()->completeBuffer(request, buffer); + pipe()->completeRequest(request); + pendingRequests_.pop(); + + continue; + } + } + + avaliableBufferSlotCount_--; + + pendingRequests_.pop(); + } +} + +void SimpleCameraData::cancelPendingRequests() +{ + while (!pendingRequests_.empty()) { + Request *request = pendingRequests_.front(); + + for (auto const &it : request->buffers()) { + it.second->cancel(); + pipe()->completeBuffer(request, it.second); + } + + pipe()->completeRequest(request); + + pendingRequests_.pop(); + } +} + /* ----------------------------------------------------------------------------- * Camera Configuration */ @@ -852,6 +919,8 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL if (ret < 0) return ret; + data->avaliableBufferSlotCount_ = kSimpleBufferSlotCount; + ret = video->streamOn(); if (ret < 0) { stop(camera); @@ -885,6 +954,7 @@ void SimplePipelineHandler::stop(Camera *camera) converter_->stop(); video->streamOff(); + data->cancelPendingRequests(); video->releaseBuffers(); data->converterBuffers_.clear(); @@ -894,27 +964,9 @@ void SimplePipelineHandler::stop(Camera *camera) int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request) { SimpleCameraData *data = cameraData(camera); - int ret; - - std::map buffers; - - for (auto &[stream, buffer] : request->buffers()) { - /* - * If conversion is needed, push the buffer to the converter - * queue, it will be handed to the converter in the capture - * completion handler. - */ - if (data->useConverter_) { - buffers.emplace(data->streamIndex(stream), buffer); - } else { - ret = data->video_->queueBuffer(buffer); - if (ret < 0) - return ret; - } - } - if (data->useConverter_) - data->converterQueue_.push(std::move(buffers)); + data->pendingRequests_.push(request); + data->queuePendingRequests(); return 0; } @@ -1151,6 +1203,8 @@ void SimplePipelineHandler::bufferReady(FrameBuffer *buffer) Request *request = buffer->request(); completeBuffer(request, buffer); completeRequest(request); + data->avaliableBufferSlotCount_++; + data->queuePendingRequests(); return; } @@ -1221,6 +1275,9 @@ void SimplePipelineHandler::bufferReady(FrameBuffer *buffer) /* Otherwise simply complete the request. */ completeBuffer(request, buffer); completeRequest(request); + + data->avaliableBufferSlotCount_++; + data->queuePendingRequests(); } void SimplePipelineHandler::converterInputDone(FrameBuffer *buffer) @@ -1235,11 +1292,15 @@ void SimplePipelineHandler::converterInputDone(FrameBuffer *buffer) void SimplePipelineHandler::converterOutputDone(FrameBuffer *buffer) { ASSERT(activeCamera_); + SimpleCameraData *data = cameraData(activeCamera_); /* Complete the buffer and the request. */ Request *request = buffer->request(); if (completeBuffer(request, buffer)) completeRequest(request); + + data->avaliableBufferSlotCount_++; + data->queuePendingRequests(); } REGISTER_PIPELINE_HANDLER(SimplePipelineHandler) 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