From patchwork Thu Jul 17 12:59:22 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23840 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 14BC1C3237 for ; Thu, 17 Jul 2025 12:59:45 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C496468F8D; Thu, 17 Jul 2025 14:59:44 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="gtlnfJNJ"; dkim-atps=neutral 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 A204B68F7A for ; Thu, 17 Jul 2025 14:59:41 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:7b93:8acd:d82d:248d]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id A0B641E74; Thu, 17 Jul 2025 14:59:07 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1752757147; bh=FtP3LBEAlKfycED7DyqSkyT0cPc5DT8/6QdDfB3TVPs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=gtlnfJNJpJlUTXWWfusfdDqT9AOrBrmhg/YBwVzAJ/lfXDDde7wOm0fdw36YTIzoo 7VeiNqYioRMvUR0SQjFvrukeP+qH9fSXYe3I5JCj6Msox2Xu+MeQpa1aCcsisUE/h7 rlOQeJGiKJpFeX5+Nv3u9oRVuURVb33/0BU5oyX8= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Kieran Bingham , Umang Jain , =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v3 2/5] libcamera: pipeline_handler: Allow to limit the number of queued requests Date: Thu, 17 Jul 2025 14:59:22 +0200 Message-ID: <20250717125931.2848300-3-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.48.1 In-Reply-To: <20250717125931.2848300-1-stefan.klug@ideasonboard.com> References: <20250717125931.2848300-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Add a maxQueuedRequestsDevice constructor parameter to allow pipeline handler classes to limit the maximum number of requests that get queued to the device in queueRequestDevice(). The default value is set to an arbitrary number of 32 which is big enough for all currently known use cases. Signed-off-by: Kieran Bingham Signed-off-by: Stefan Klug Reviewed-by: Umang Jain Tested-By: Sven Püschel --- Changes in v3: - Updated documentation for maxQueuedRequestsDevice_ - Collected tag Changes in v2: - Moved the fix to properly handle the waitingRequests queue when stopping the device into this patch as they belong together - Added documentation for maxQueuedRequestsDevice_ - Fixed an issue with doQueueRequests() beeing called recursively Changes in v1: - Used a const member variable to carry the maximum number of requests - Improved commit message - Added docs --- include/libcamera/internal/pipeline_handler.h | 4 +- src/libcamera/pipeline_handler.cpp | 58 +++++++++++++++---- 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index be017ad47219..e89d6a33e398 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -33,7 +33,8 @@ class PipelineHandler : public std::enable_shared_from_this, public Object { public: - PipelineHandler(CameraManager *manager); + PipelineHandler(CameraManager *manager, + unsigned int maxQueuedRequestsDevice = 32); virtual ~PipelineHandler(); virtual bool match(DeviceEnumerator *enumerator) = 0; @@ -80,6 +81,7 @@ protected: virtual void releaseDevice(Camera *camera); CameraManager *manager_; + const unsigned int maxQueuedRequestsDevice_; private: void unlockMediaDevices(); diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index b50eda5e0f86..e5f9e55c9783 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -62,13 +62,17 @@ LOG_DEFINE_CATEGORY(Pipeline) /** * \brief Construct a PipelineHandler instance * \param[in] manager The camera manager + * \param[in] maxQueuedRequestsDevice The maximum number of requests queued to + * the device * * In order to honour the std::enable_shared_from_this<> contract, * PipelineHandler instances shall never be constructed manually, but always * through the PipelineHandlerFactoryBase::create() function. */ -PipelineHandler::PipelineHandler(CameraManager *manager) - : manager_(manager), useCount_(0) +PipelineHandler::PipelineHandler(CameraManager *manager, + unsigned int maxQueuedRequestsDevice) + : manager_(manager), maxQueuedRequestsDevice_(maxQueuedRequestsDevice), + useCount_(0) { } @@ -360,20 +364,28 @@ void PipelineHandler::unlockMediaDevices() */ void PipelineHandler::stop(Camera *camera) { + /* + * Take all waiting requests so that they are not requeued in response + * to completeRequest() being called inside stopDevice(). Cancel them + * after the device to keep them in order. + */ + Camera::Private *data = camera->_d(); + std::queue waitingRequests; + waitingRequests.swap(data->waitingRequests_); + /* Stop the pipeline handler and let the queued requests complete. */ stopDevice(camera); - Camera::Private *data = camera->_d(); - /* Cancel and signal as complete all waiting requests. */ - while (!data->waitingRequests_.empty()) { - Request *request = data->waitingRequests_.front(); - data->waitingRequests_.pop(); + while (!waitingRequests.empty()) { + Request *request = waitingRequests.front(); + waitingRequests.pop(); cancelRequest(request); } /* Make sure no requests are pending. */ ASSERT(data->queuedRequests_.empty()); + ASSERT(data->waitingRequests_.empty()); data->requestSequence_ = 0; } @@ -430,9 +442,9 @@ void PipelineHandler::registerRequest(Request *request) * requests which have to be prepared to make sure they are ready for being * queued to the pipeline handler. * - * The queue of waiting requests is iterated and all prepared requests are - * passed to the pipeline handler in the same order they have been queued by - * calling this function. + * The queue of waiting requests is iterated and up to \a + * maxQueuedRequestsDevice_ prepared requests are passed to the pipeline handler + * in the same order they have been queued by calling this function. * * If a Request fails during the preparation phase or if the pipeline handler * fails in queuing the request to the hardware the request is cancelled. @@ -487,12 +499,19 @@ void PipelineHandler::doQueueRequests(Camera *camera) { Camera::Private *data = camera->_d(); while (!data->waitingRequests_.empty()) { + if (data->queuedRequests_.size() == maxQueuedRequestsDevice_) + break; + Request *request = data->waitingRequests_.front(); if (!request->_d()->prepared_) break; - doQueueRequest(request); + /* + * Pop the request first, in case doQueueRequests() is called + * recursively from within doQueueRequest() + */ data->waitingRequests_.pop(); + doQueueRequest(request); } } @@ -568,6 +587,9 @@ void PipelineHandler::completeRequest(Request *request) data->queuedRequests_.pop_front(); camera->requestComplete(req); } + + /* Allow any waiting requests to be queued to the pipeline. */ + doQueueRequests(camera); } /** @@ -768,6 +790,20 @@ void PipelineHandler::disconnect() * constant for the whole lifetime of the pipeline handler. */ +/** + * \var PipelineHandler::maxQueuedRequestsDevice_ + * \brief The maximum number of requests the pipeline handler shall queue to the + * device + * + * maxQueuedRequestsDevice_ limits the number of request that the + * pipeline handler shall queue to the underlying hardware, in order to + * saturate the pipeline with requests. The application may choose to queue + * as many requests as it desires, however only maxQueuedRequestsDevice_ + * requests will be queued to the hardware at a given point in time. The + * remaining requests will be kept waiting in the internal waiting + * queue, to be queued at a later stage. + */ + /** * \fn PipelineHandler::name() * \brief Retrieve the pipeline handler name