From patchwork Tue Dec 2 12:27:30 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: 25285 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 65BA8BD80A for ; Tue, 2 Dec 2025 12:27:34 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 11E0660C79; Tue, 2 Dec 2025 13:27:34 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="ILYEo7+0"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D753560A7B for ; Tue, 2 Dec 2025 13:27:32 +0100 (CET) Received: from pb-laptop.local (185.182.214.104.nat.pool.zt.hu [185.182.214.104]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 0DB6A9FC for ; Tue, 2 Dec 2025 13:25:19 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1764678319; bh=zC3PVKtt1NxdCi49ocXaJCxicxDzz+eF5jucTCG5hXA=; h=From:To:Subject:Date:From; b=ILYEo7+0wOVX2CXuMRCGczxUI9Rb5gZc9doPBUVgD8Zcy8a0Rgaqjxrbp1eU8T7ix 0ylrXJgN/sr71a1Cfirpm55pWqTd6aZ7vw6YZJ5LgrP0EK4CmBVhEtSXDPxnL6FU/E nRrGbZJuwk4B2Ss5K6PTx2a3iOFPO/q49qiCAaS0= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Subject: [RFC PATCH v1] libcamera: request: Add "queued" flag Date: Tue, 2 Dec 2025 13:27:30 +0100 Message-ID: <20251202122730.1579065-1-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.52.0 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 an atomic flag which is used to check if the request has been queued using `Camera::queueRequest()`. This is used to diagnose misuses of requests, especially double queueing. Link: https://gitlab.freedesktop.org/camera/libcamera/-/issues/197 Signed-off-by: Barnabás Pőcze --- This is another approach to detect misuses of Request objects. https://patchwork.libcamera.org/patch/25280/ --- include/libcamera/internal/request.h | 11 +++++++++++ src/libcamera/camera.cpp | 8 +++++++- src/libcamera/pipeline_handler.cpp | 3 +++ src/libcamera/request.cpp | 22 ++++++++++++++++++++++ 4 files changed, 43 insertions(+), 1 deletion(-) -- 2.52.0 diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h index 78cb99f360..0e6216d6b5 100644 --- a/include/libcamera/internal/request.h +++ b/include/libcamera/internal/request.h @@ -7,6 +7,7 @@ #pragma once +#include #include #include #include @@ -35,6 +36,15 @@ public: Camera *camera() const { return camera_; } bool hasPendingBuffers() const; + bool hasBeenQueued() const { return queued_.load(std::memory_order_relaxed); } + + bool tryQueue() + { + bool expected = false; + return queued_.compare_exchange_strong(expected, true, std::memory_order_relaxed); + } + + void unQueue() { queued_.store(false, std::memory_order_relaxed); } bool completeBuffer(FrameBuffer *buffer); void complete(); @@ -57,6 +67,7 @@ private: bool cancelled_; uint32_t sequence_ = 0; bool prepared_ = false; + std::atomic_bool queued_ = false; std::unordered_set pending_; std::map notifiers_; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 2e1e146a25..8fa45376d3 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -1340,11 +1340,15 @@ int Camera::queueRequest(Request *request) return -EXDEV; } - if (request->status() != Request::RequestPending) { + if (request->status() != Request::RequestPending || !request->_d()->tryQueue()) { LOG(Camera, Error) << request->toString() << " is not valid"; return -EINVAL; } + utils::scope_exit queuedFlagGuard([&] { + request->_d()->unQueue(); + }); + /* * The camera state may change until the end of the function. No locking * is however needed as PipelineHandler::queueRequest() will handle @@ -1371,6 +1375,8 @@ int Camera::queueRequest(Request *request) d->pipe_->invokeMethod(&PipelineHandler::queueRequest, ConnectionTypeQueued, request); + queuedFlagGuard.release(); + return 0; } diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 5c469e5bad..fade0b040d 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -470,6 +470,8 @@ void PipelineHandler::queueRequest(Request *request) { LIBCAMERA_TRACEPOINT(request_queue, request); + ASSERT(request->_d()->hasBeenQueued()); + Camera *camera = request->_d()->camera(); Camera::Private *data = camera->_d(); data->waitingRequests_.push(request); @@ -591,6 +593,7 @@ void PipelineHandler::completeRequest(Request *request) while (!data->queuedRequests_.empty()) { Request *req = data->queuedRequests_.front(); + ASSERT(req->_d()->hasBeenQueued()); if (req->status() == Request::RequestPending) break; diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index 60565f5984..6df73ba13a 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -82,6 +82,23 @@ bool Request::Private::hasPendingBuffers() const return !pending_.empty(); } +/** + * \fn Request::Private::hasBeenQueued() + * \brief Check if a the request has the "queued" flag set + * \return True if the request has been marked as queued + */ + +/** + * \fn Request::Private::tryQueue() + * \brief Try to set the "queued" flag if not already set + * \return True if it has been set successfully + */ + +/** + * \fn Request::Private::unQueue() + * \brief Clear the "queued" flag + */ + /** * \brief Complete a buffer for the request * \param[in] buffer The buffer that has completed @@ -123,6 +140,7 @@ void Request::Private::complete() ASSERT(request->status() == RequestPending); ASSERT(!hasPendingBuffers()); + ASSERT(hasBeenQueued()); request->status_ = cancelled_ ? RequestCancelled : RequestComplete; @@ -160,6 +178,7 @@ void Request::Private::cancel() Request *request = _o(); ASSERT(request->status() == RequestPending); + ASSERT(hasBeenQueued()); doCancelRequest(); } @@ -175,6 +194,7 @@ void Request::Private::reset() sequence_ = 0; cancelled_ = false; prepared_ = false; + unQueue(); pending_.clear(); notifiers_.clear(); timer_.reset(); @@ -391,6 +411,8 @@ void Request::reuse(ReuseFlag flags) { LIBCAMERA_TRACEPOINT(request_reuse, this); + ASSERT(!(status_ == RequestPending && _d()->hasBeenQueued())); + _d()->reset(); if (flags & ReuseBuffers) {