From patchwork Wed Sep 24 12:47:11 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 24452 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 3AB5ABDB1C for ; Wed, 24 Sep 2025 12:47:42 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 34EB46B614; Wed, 24 Sep 2025 14:47:36 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="XHyZP5CK"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 307D96B5FB for ; Wed, 24 Sep 2025 14:47:18 +0200 (CEST) Received: from pb-laptop.local (185.221.140.70.nat.pool.zt.hu [185.221.140.70]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 3D26A1E36; Wed, 24 Sep 2025 14:45:54 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1758717954; bh=rRY8KqO5CkXTsdWJUkqirBQf/6DBZyG/gB8dlDWRQKM=; h=From:To:Subject:Date:In-Reply-To:References:From; b=XHyZP5CKunzZsY+Bsj1gOsEP8bKMqQamvZv9Hd3ORuCclchRZ7/x3m1mwTZjMP08a GdrJKs5qqr4Z47d/cBDBIbMNC99QRqQ6hcTjCWeETSJpD68eJbJDOD7dLXHWT5NX2X bpBn5v/av6lJL4//VTyw8Zlpp+g7Th/QnC5YFK8I= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org, Kieran Bingham Subject: [RFC PATCH v1 6/7] libcamera: camera: Add `applyControls()` Date: Wed, 24 Sep 2025 14:47:11 +0200 Message-ID: <20250924124713.3361707-7-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20250924124713.3361707-1-barnabas.pocze@ideasonboard.com> References: <20250924124713.3361707-1-barnabas.pocze@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 `Camera::applyControls()` whose purpose is to apply controls as soon as possible, without going through `Request::controls()`. A new virtual function `PipelineHandler::applyControlsDevice()` is provided for pipeline handler to implement fast-tracked application of controls. If the pipeline handler does not implement that functionality, or it fails, then a fallback mechanism is used. The controls will be saved for later, and they will be merged into the control list of the next request sent to the pipeline handler (`Camera::Private::waitingRequests_`). Signed-off-by: Barnabás Pőcze --- include/libcamera/camera.h | 1 + include/libcamera/internal/camera.h | 1 + include/libcamera/internal/pipeline_handler.h | 7 ++ src/libcamera/camera.cpp | 53 +++++++++ src/libcamera/pipeline_handler.cpp | 110 +++++++++++++++++- 5 files changed, 171 insertions(+), 1 deletion(-) diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index b24a29740..94f940496 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -147,6 +147,7 @@ public: std::unique_ptr createRequest(uint64_t cookie = 0); int queueRequest(Request *request); + int applyControls(ControlList &&controls); int start(const ControlList *controls = nullptr); int stop(); diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h index 0f50c14a5..5a77298b3 100644 --- a/include/libcamera/internal/camera.h +++ b/include/libcamera/internal/camera.h @@ -38,6 +38,7 @@ public: std::list queuedRequests_; std::deque waitingRequests_; + ControlList pendingControls_; ControlInfoMap controlInfo_; ControlList properties_; diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index e89d6a33e..16ff54bab 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -57,6 +57,7 @@ public: void registerRequest(Request *request); void queueRequest(Request *request); + int applyControls(Camera *camera, ControlList &&controls); bool completeBuffer(Request *request, FrameBuffer *buffer); void completeRequest(Request *request); @@ -75,6 +76,12 @@ protected: void hotplugMediaDevice(MediaDevice *media); virtual int queueRequestDevice(Camera *camera, Request *request) = 0; + + virtual int applyControlsDevice([[maybe_unused]] Camera *camera, [[maybe_unused]] const ControlList &controls) + { + return -EOPNOTSUPP; + } + virtual void stopDevice(Camera *camera) = 0; virtual bool acquireDevice(Camera *camera); diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 2e1e146a2..778445e36 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -637,6 +637,14 @@ Camera::Private::~Private() * queued requests was reached. */ +/** + * \var Camera::Private::pendingControls_ + * \brief The list of pending controls + * + * This list tracks the controls that need to be applied to the device with the + * next request going to PipelineHandler::queueRequestDevice(). + */ + /** * \var Camera::Private::controlInfo_ * \brief The set of controls supported by the camera @@ -1374,6 +1382,51 @@ int Camera::queueRequest(Request *request) return 0; } +/** + * \brief Apply controls immediately + * \param[in] controls The list of controls to apply + * + * This function tries to ensure that the controls in \a controls are applied + * to the camera as soon as possible. + * + * The exact guarantees are camera dependent, but it is guaranteed that the + * controls will be applied no later than if they were part of the next \ref Request "request" + * that the application \ref Camera::queueRequest() "queues". + * + * \context This function is \threadsafe. It may only be called when the camera + * is in the Running state as defined in \ref camera_operation. + * + * \return 0 on success or a negative error code otherwise + * \retval -ENODEV The camera has been disconnected from the system + * \retval -EACCES The camera is not running + */ +int Camera::applyControls(ControlList &&controls) +{ + Private *const d = _d(); + + /* + * \todo What to do if not running? Should it be stored and merged into the "start" `ControlList`? + * If yes, should that list of pending controls be overwritten/updated when stopped? + * And should it be cleared in `Camera::release()`? + */ + + int ret = d->isAccessAllowed(Private::CameraRunning); + if (ret < 0) + return ret; + + if (controls.empty()) + return 0; + + patchControlList(controls); + + /* + * \todo Or `ConnectionTypeBlocking` to get the return value? + */ + d->pipe_->invokeMethod(&PipelineHandler::applyControls, ConnectionTypeQueued, this, std::move(controls)); + + return 0; +} + /** * \brief Start capture from camera * \param[in] controls Controls to be applied before starting the Camera diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index d0d3fbc79..99bb2193f 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -387,6 +387,12 @@ void PipelineHandler::stop(Camera *camera) ASSERT(data->queuedRequests_.empty()); ASSERT(data->waitingRequests_.empty()); + /* + * \todo Or not? This probably needs discussions regarding + * the expected state of controls after a stop-start sequence. + */ + data->pendingControls_.clear(); + data->requestSequence_ = 0; } @@ -466,6 +472,54 @@ void PipelineHandler::queueRequest(Request *request) request->_d()->prepare(300ms); } +/** + * \brief Apply controls immediately + * \param[in] camera The camera + * \param[in] controls The controls to apply + * + * This function tries to apply \a controls immediately to the device by + * calling applyControlsDevice(). If that fails, then a fallback mechanism + * is used to ensure that \a controls will be merged into the control list + * of the next request submitted to the pipeline handler. + * + * This function also ensures that requests in Camera::Private::waitingRequests_ + * will not override the fast-tracked controls. + * + * \context This function is called from the CameraManager thread. + */ +int PipelineHandler::applyControls(Camera *camera, ControlList &&controls) +{ + Camera::Private *data = camera->_d(); + int res = applyControlsDevice(camera, controls); + + /* + * Prevent later requests from overriding the fast-tracked controls. + * \todo This is unfortunately slow. + */ + for (const auto &[k, v] : controls) { + for (Request *r : data->waitingRequests_) + r->controls().erase(k); + } + + if (res < 0) { + /* + * \todo Always fall back or only if EOPNOTSUPP is returned? + * Possibly there could be some way for the user to influence + * the fallback logic (e.g. `flags` argument for `Camera::applyControls()`). + */ + if (res != -EOPNOTSUPP) + LOG(Pipeline, Debug) << "Fast tracking controls failed: " << res; + + /* + * Fall back to adding the controls to the next request that enters the + * pipeline handler. See PipelineHandler::doQueueRequest(). + */ + data->pendingControls_.merge(std::move(controls), ControlList::MergePolicy::OverwriteExisting); + } + + return 0; +} + /** * \brief Queue one requests to the device */ @@ -484,9 +538,49 @@ void PipelineHandler::doQueueRequest(Request *request) return; } + if (!data->pendingControls_.empty()) { + /* + * Note that `ControlList::MergePolicy::KeepExisting` is used. This is + * needed to ensure that if `request` is newer than pendingControls_, + * then its controls take precedence. + * + * For older requests, `PipelineHandler::applyControls()` has already removed + * the controls that should be overridden. + * + * \todo How to handle (if at all) conflicting controls? + * \todo How to handle failure of `queueRequestDevice()`? + * After the merge it becomes impossible to retrieve the pending controls + * from `request->controls()`. Making a copy in such a hot-path is not ideal. + */ + request->controls().merge(std::move(data->pendingControls_), ControlList::MergePolicy::KeepExisting); + data->pendingControls_.clear(); + } + int ret = queueRequestDevice(camera, request); - if (ret) + if (ret) { + /* + * \todo What to do now? + * + * The original `pendingControls_` is not easily recoverable. + * + * Initially + * request->controls() = A u B + * pendingControls_ = A' u C + * + * then after the above merge + * request->controls() = A u B u C + * pendingControls_ = A' + * + * so recovering `pendingControls_` without a copy of at least + * `B` or `C` does not appear easy. + * + * But sometimes recovering is not even desirable: assume that the controls + * in `pendingControls_` causes the failure. In that case keeping them around + * will cause every single subsequent request to fail. + */ + cancelRequest(request); + } } /** @@ -532,6 +626,20 @@ void PipelineHandler::doQueueRequests(Camera *camera) * \return 0 on success or a negative error code otherwise */ +/** + * \fn PipelineHandler::applyControlsDevice() + * \brief Apply controls immediately + * \param[in] camera The camera + * \param[in] controls The controls to apply + * + * This function applies \a controls to \a camera immediately. + * + * \context This function is called from the CameraManager thread. + * + * \return 0 on success or a negative error code otherwise + * \return -EOPNOTSUPP if fast-tracking controls is not supported + */ + /** * \brief Complete a buffer for a request * \param[in] request The request the buffer belongs to