From patchwork Mon Jul 7 07:53:35 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23744 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 65E2AC3237 for ; Mon, 7 Jul 2025 07:54:21 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 0FFF968E7F; Mon, 7 Jul 2025 09:54:20 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="jVX+gAeb"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 4904868E81 for ; Mon, 7 Jul 2025 09:54:16 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:c79f:85df:e7f5:4c31]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 078B9526; Mon, 7 Jul 2025 09:53:48 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1751874829; bh=pyOXhjpFCrj4pGIMuD0VpZ7r6CxT8aZTU1KTUkQFM2o=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=jVX+gAebAdyR/MrOKqjgloc+yU23DbMhO4hHpI1aag+mxXza/sK3pJ+x6KNJF/lhe iRxKyoxQ/55baDs4KJBhHMgLOrGu/xHu2CR/rX5eQXjcCuhhvM4hQHPTMSeGkWhUUS W1mqNEXYtx/Ut9M4ophnd2Fb2NY6tzlOP24fP/cU= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Umang Jain Subject: [PATCH v2 1/5] libcamera: pipeline_handler: Move waitingRequests_ into camera class Date: Mon, 7 Jul 2025 09:53:35 +0200 Message-ID: <20250707075400.9079-2-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.48.1 In-Reply-To: <20250707075400.9079-1-stefan.klug@ideasonboard.com> References: <20250707075400.9079-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" The waiting requests of one camera should not be able to influence queuing to another camera. Currently a request that is in the waitingQUeue and waiting for preparation blocks all requests of other cameras even if they are already prepared. To fix that replace the single waitingRequests_ queue with one queue per camera. Signed-off-by: Stefan Klug Reviewed-by: Umang Jain Reviewed-by: Kieran Bingham --- Changes in v2: - Fixed capturing on lambda expression - Added documentation for waitingRequests_ - Improved commit message Changes in v1: - Added camera param to doQueueRequests() which allows to remove a loop --- include/libcamera/internal/camera.h | 2 ++ include/libcamera/internal/pipeline_handler.h | 5 +--- src/libcamera/camera.cpp | 9 +++++++ src/libcamera/pipeline_handler.cpp | 26 ++++++++++++------- 4 files changed, 28 insertions(+), 14 deletions(-) diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h index 18f5c32a18e4..8a2e9ed5894d 100644 --- a/include/libcamera/internal/camera.h +++ b/include/libcamera/internal/camera.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -36,6 +37,7 @@ public: const PipelineHandler *pipe() const { return pipe_.get(); } std::list queuedRequests_; + std::queue waitingRequests_; ControlInfoMap controlInfo_; ControlList properties_; diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index 972a2fa65310..be017ad47219 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -8,7 +8,6 @@ #pragma once #include -#include #include #include #include @@ -89,13 +88,11 @@ private: virtual void disconnect(); void doQueueRequest(Request *request); - void doQueueRequests(); + void doQueueRequests(Camera *camera); std::vector> mediaDevices_; std::vector> cameras_; - std::queue waitingRequests_; - const char *name_; unsigned int useCount_; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index c180a5fdde93..2ae50eaeaa5a 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -628,6 +628,15 @@ Camera::Private::~Private() * PipelineHandler::completeRequest() */ +/** + * \var Camera::Private::waitingRequests_ + * \brief The queue of waiting requests + * + * This queue tracks all requests that can not yet be queued to the device. + * Either because they are not yet prepared or because the maximum number of + * queued requests was reached. + */ + /** * \var Camera::Private::controlInfo_ * \brief The set of controls supported by the camera diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index d84dff3c9f19..b50eda5e0f86 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -363,15 +363,16 @@ void PipelineHandler::stop(Camera *camera) /* 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 (!waitingRequests_.empty()) { - Request *request = waitingRequests_.front(); - waitingRequests_.pop(); + while (!data->waitingRequests_.empty()) { + Request *request = data->waitingRequests_.front(); + data->waitingRequests_.pop(); cancelRequest(request); } /* Make sure no requests are pending. */ - Camera::Private *data = camera->_d(); ASSERT(data->queuedRequests_.empty()); data->requestSequence_ = 0; @@ -414,7 +415,9 @@ void PipelineHandler::registerRequest(Request *request) * Connect the request prepared signal to notify the pipeline handler * when a request is ready to be processed. */ - request->_d()->prepared.connect(this, &PipelineHandler::doQueueRequests); + request->_d()->prepared.connect(this, [this, request]() { + doQueueRequests(request->_d()->camera()); + }); } /** @@ -444,7 +447,9 @@ void PipelineHandler::queueRequest(Request *request) { LIBCAMERA_TRACEPOINT(request_queue, request); - waitingRequests_.push(request); + Camera *camera = request->_d()->camera(); + Camera::Private *data = camera->_d(); + data->waitingRequests_.push(request); request->_d()->prepare(300ms); } @@ -478,15 +483,16 @@ void PipelineHandler::doQueueRequest(Request *request) * Iterate the list of waiting requests and queue them to the device one * by one if they have been prepared. */ -void PipelineHandler::doQueueRequests() +void PipelineHandler::doQueueRequests(Camera *camera) { - while (!waitingRequests_.empty()) { - Request *request = waitingRequests_.front(); + Camera::Private *data = camera->_d(); + while (!data->waitingRequests_.empty()) { + Request *request = data->waitingRequests_.front(); if (!request->_d()->prepared_) break; doQueueRequest(request); - waitingRequests_.pop(); + data->waitingRequests_.pop(); } } From patchwork Mon Jul 7 07:53:36 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23745 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 46D48C3237 for ; Mon, 7 Jul 2025 07:54:24 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 50EBE68E83; Mon, 7 Jul 2025 09:54:23 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Hi54uGBD"; 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 3BF7668E77 for ; Mon, 7 Jul 2025 09:54:18 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:c79f:85df:e7f5:4c31]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 9C82C9CA; Mon, 7 Jul 2025 09:53:51 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1751874831; bh=CHenzEB2uLrp8qlnX9B54j8HUaJQyD2xNOrdMXAZ+3s=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Hi54uGBDobbzkZdV6uy1fSTI6/9yGsVqAMM1kSSm0+0xQclpR30ErPsLVdnZItQRq xKJxz/Z9CqZQNjJa6LaqNzcCLcRsxvv5LtaCWuYlMoZwOijifuOERQ5GteKCsjHBt7 Plkjtz5wOcdpACEFRJuOUwvN+Qz83SDa35/gTGn0= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Kieran Bingham Subject: [PATCH v2 2/5] libcamera: pipeline_handler: Allow to limit the number of queued requests Date: Mon, 7 Jul 2025 09:53:36 +0200 Message-ID: <20250707075400.9079-3-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.48.1 In-Reply-To: <20250707075400.9079-1-stefan.klug@ideasonboard.com> References: <20250707075400.9079-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: Stefan Klug Signed-off-by: Kieran Bingham Signed-off-by: Stefan Klug Reviewed-by: Kieran Bingham --- 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 | 56 +++++++++++++++---- 2 files changed, 48 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..e8601be108f0 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,18 @@ 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 + * + * Some pipeline handlers do not work efficiently with too many requests queued + * into the device. Still it is beneficial for the application to circulate more + * than the minimum number of buffers. This variable limits the number of + * requests that get queued to the device at once. The rest is kept in a + * a waiting queue. + */ + /** * \fn PipelineHandler::name() * \brief Retrieve the pipeline handler name From patchwork Mon Jul 7 07:53:37 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23746 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 755CCC3237 for ; Mon, 7 Jul 2025 07:54:28 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1E69E68E8C; Mon, 7 Jul 2025 09:54:28 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="GPnrQ+0O"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5447068E81 for ; Mon, 7 Jul 2025 09:54:21 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:c79f:85df:e7f5:4c31]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id B08BD526; Mon, 7 Jul 2025 09:53:54 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1751874834; bh=+uVycowaiQOfCa898u9FWdwTkf07+tJWQlC/ayNmqOY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=GPnrQ+0OucW+GrHkcOR1d2KpOZmhjbqbuJ4qfzlBL4Syq9exXClDiLtuneLsoAu5o LAtAcXezT9dfQB7WxS4b07FOHJttZzrQhwCAGRjdPv7q9bkR872LxxoZWdbJ8uGYic l7SZGZvZtFV0cCEJ8yemW+F2zyxZ1jNlZJVyBaVo= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 3/5] pipeline: rkisp1: Limit the maximum number of buffers queued in Date: Mon, 7 Jul 2025 09:53:37 +0200 Message-ID: <20250707075400.9079-4-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.48.1 In-Reply-To: <20250707075400.9079-1-stefan.klug@ideasonboard.com> References: <20250707075400.9079-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" To keep the regulation of the algorithms as short as possible and to allow more buffers to be created than the v4l2 device allows to be queued, limit the amount of buffers that get queued into the device to the pipeline depth. Signed-off-by: Stefan Klug Reviewed-by: Kieran Bingham --- Changes in v1: - Replaced function overload with constructor param --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 675f0a7490a6..fff42359cbff 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -155,6 +155,12 @@ private: Transform combinedTransform_; }; +namespace { + +const unsigned int kPipelineDepth = 4; + +} // namespace + class PipelineHandlerRkISP1 : public PipelineHandler { public: @@ -684,7 +690,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() */ PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager) - : PipelineHandler(manager), hasSelfPath_(true), useDewarper_(false) + : PipelineHandler(manager, kPipelineDepth), hasSelfPath_(true), useDewarper_(false) { } From patchwork Mon Jul 7 07:53:38 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23747 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 7A5A3C3237 for ; Mon, 7 Jul 2025 07:54:30 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 9027068E8A; Mon, 7 Jul 2025 09:54:29 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="ap6ms3kr"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 26AB568E7F for ; Mon, 7 Jul 2025 09:54:24 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:c79f:85df:e7f5:4c31]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 7D48D9CA; Mon, 7 Jul 2025 09:53:57 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1751874837; bh=yM8Kf73Aeg3TPvTYh2uujN5Rh09/lPziHX1KXP2CNwc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ap6ms3krJPMAyGuXVTZpFrFl3vWALNHF8mItj7ELPWvd1NdV8fvNTbLm3Iho5vplx emBEEK0UNPWP2iCj8oBfSCgUtK/tVyIt3v6hS62VKHGdNOmx/Ja9QWu7PaE+21zOEn QfTAdrVs+P2gprcycph/ILkVF17dFmoy1pI1mCXQ= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 4/5] pipeline: rkisp1: Properly handle the bufferCount set in the stream configuration Date: Mon, 7 Jul 2025 09:53:38 +0200 Message-ID: <20250707075400.9079-5-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.48.1 In-Reply-To: <20250707075400.9079-1-stefan.klug@ideasonboard.com> References: <20250707075400.9079-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" The bufferCount is reset to a hardcoded value of 4 in RkISP1Path::validate(). Keep the default value of 4 but do not reset it, if it was changed. This allows the user to set bufferCount to an arbitrary number of buffers which then can be allocated for example by the FrameBufferAllocator. Internally maxQueuedRequestsDevice_ is used as buffer count which is the maximum number of buffers queued to the device at once. Signed-off-by: Stefan Klug --- Changes in v1: - Removed todo comment that was solved by this change --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 5 +++-- src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 7 ++----- src/libcamera/pipeline/rkisp1/rkisp1_path.h | 4 +--- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index fff42359cbff..bdaeb935ee06 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -790,6 +790,7 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera, return nullptr; cfg.colorSpace = colorSpace; + cfg.bufferCount = kPipelineDepth; config->addConfiguration(cfg); } @@ -1123,14 +1124,14 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL } if (data->mainPath_->isEnabled()) { - ret = mainPath_.start(); + ret = mainPath_.start(maxQueuedRequestsDevice_); if (ret) return ret; actions += [&]() { mainPath_.stop(); }; } if (hasSelfPath_ && data->selfPath_->isEnabled()) { - ret = selfPath_.start(); + ret = selfPath_.start(maxQueuedRequestsDevice_); if (ret) return ret; } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp index 64018dc5b2f4..8ea5500d4080 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp @@ -249,7 +249,6 @@ RkISP1Path::generateConfiguration(const CameraSensor *sensor, const Size &size, StreamConfiguration cfg(formats); cfg.pixelFormat = format; cfg.size = streamSize; - cfg.bufferCount = RKISP1_BUFFER_COUNT; return cfg; } @@ -383,7 +382,6 @@ RkISP1Path::validate(const CameraSensor *sensor, cfg->size.boundTo(maxResolution); cfg->size.expandTo(minResolution); - cfg->bufferCount = RKISP1_BUFFER_COUNT; V4L2DeviceFormat format; format.fourcc = video_->toV4L2PixelFormat(cfg->pixelFormat); @@ -480,15 +478,14 @@ int RkISP1Path::configure(const StreamConfiguration &config, return 0; } -int RkISP1Path::start() +int RkISP1Path::start(unsigned int bufferCount) { int ret; if (running_) return -EBUSY; - /* \todo Make buffer count user configurable. */ - ret = video_->importBuffers(RKISP1_BUFFER_COUNT); + ret = video_->importBuffers(bufferCount); if (ret) return ret; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h index 430181d371a7..0b60c499ac64 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h @@ -58,7 +58,7 @@ public: return video_->exportBuffers(bufferCount, buffers); } - int start(); + int start(unsigned int bufferCount); void stop(); int queueBuffer(FrameBuffer *buffer) { return video_->queueBuffer(buffer); } @@ -69,8 +69,6 @@ private: void populateFormats(); Size filterSensorResolution(const CameraSensor *sensor); - static constexpr unsigned int RKISP1_BUFFER_COUNT = 4; - const char *name_; bool running_; From patchwork Mon Jul 7 07:53:39 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23748 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 AB15FC3237 for ; Mon, 7 Jul 2025 07:54:32 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id EEC5968E8C; Mon, 7 Jul 2025 09:54:30 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="OmF3gamY"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 0FA6968E7F for ; Mon, 7 Jul 2025 09:54:27 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:c79f:85df:e7f5:4c31]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 6FEF6526; Mon, 7 Jul 2025 09:54:00 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1751874840; bh=cBLlwMkHnt0QokXhGoBIX4pd2dZBqswCjcQIZw5Nq2Q=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=OmF3gamYA5ph9AGiWh4BKXccqgHnd8LKl3C9y+Hu7EpR5u/Sy6oVb5d8x+0f8mNR8 0AEW1te3g3KYrqabmonWEWc6YjxUesjupMepVCYHwmtnbSOP5Qii7h8FKXRMl0AZ+A AY8muOfo99uaIwIZcet23wT6RCWnVYmfdCvfvUj4= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , =?utf-8?b?TsOtY29sYXMgRi4g?= =?utf-8?q?R=2E_A=2E_Prado?= , Paul Elder , =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v2 5/5] libcamera: pipeline: rkisp1: Don't rely on bufferCount Date: Mon, 7 Jul 2025 09:53:39 +0200 Message-ID: <20250707075400.9079-6-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.48.1 In-Reply-To: <20250707075400.9079-1-stefan.klug@ideasonboard.com> References: <20250707075400.9079-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" From: Nícolas F. R. A. Prado Currently the rkisp1 pipeline handler relies on bufferCount to decide on the number of parameter and statistics buffers to allocate internally. Instead, the number of internal buffers should be the minimum required by the pipeline to keep the requests flowing, in order to avoid wasting memory. Stop relying on bufferCount for these numbers and instead set them to kPipelineDepth, as this already limits the number of buffers queued to the driver. Signed-off-by: Nícolas F. R. A. Prado Signed-off-by: Paul Elder Signed-off-by: Sven Püschel Reviewed-by: Kieran Bingham --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index bdaeb935ee06..8591e4868e6f 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -157,6 +157,10 @@ private: namespace { +/* + * This many internal buffers (or rather parameter and statistics buffer + * pairs) ensures that the pipeline runs smoothly, without frame drops. + */ const unsigned int kPipelineDepth = 4; } // namespace @@ -990,24 +994,19 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) unsigned int ipaBufferId = 1; int ret; - unsigned int maxCount = std::max({ - data->mainPathStream_.configuration().bufferCount, - data->selfPathStream_.configuration().bufferCount, - }); - if (!isRaw_) { - ret = param_->allocateBuffers(maxCount, ¶mBuffers_); + ret = param_->allocateBuffers(kPipelineDepth, ¶mBuffers_); if (ret < 0) goto error; - ret = stat_->allocateBuffers(maxCount, &statBuffers_); + ret = stat_->allocateBuffers(kPipelineDepth, &statBuffers_); if (ret < 0) goto error; } /* If the dewarper is being used, allocate internal buffers for ISP. */ if (useDewarper_) { - ret = mainPath_.exportBuffers(maxCount, &mainPathBuffers_); + ret = mainPath_.exportBuffers(kPipelineDepth, &mainPathBuffers_); if (ret < 0) goto error;