From patchwork Tue Jan 13 00:07:34 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 25722 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 25ED5BDCBF for ; Tue, 13 Jan 2026 00:08:36 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5B13D61FC2; Tue, 13 Jan 2026 01:08:35 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="kOr+ovBq"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id DD35161FB7 for ; Tue, 13 Jan 2026 01:08:32 +0100 (CET) Received: from pendragon.ideasonboard.com (81-175-209-152.bb.dnainternet.fi [81.175.209.152]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 32DF250A for ; Tue, 13 Jan 2026 01:08:07 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1768262887; bh=r43+7FueC3TwfykC3fDq5vMzaTlx1yuToFECMfVCYyw=; h=From:To:Subject:Date:In-Reply-To:References:From; b=kOr+ovBq59VEcP2Dz8urQR4fidWvOp5netH03sIoYuTdfw3u+XlFaZu0JGmVrGc+F hOouU5W51J0cDUub5wu/r90AUbo203ZvWa/KXtdJqmn2F6YMrGCdIW6LAbedFmHysx XkNtSyE4Xz3MeYFQChVLP1DQPn3U7jlqZfm9yNy0= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Subject: [PATCH 02/36] libcamera: request: Move all private member variables to Private class Date: Tue, 13 Jan 2026 02:07:34 +0200 Message-ID: <20260113000808.15395-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.51.2 In-Reply-To: <20260113000808.15395-1-laurent.pinchart@ideasonboard.com> References: <20260113000808.15395-1-laurent.pinchart@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 Request class has a set of private member variables, with some of them stored in the Request class itself, and some in the Request::Private class. Storing the variables in the Request class itself has the advantage that accessors can be inline, at the cost of ABI breakage if variables need to be added, removed or otherwise modified. The controls_ and metadata_ variables have recently been turned from pointers to instances. This broke the ABI. To avoid further breakages, move all remaining private member variables to Request::Private. The performance impact of not inlining accessors will be negligible. While at it, drop an unneeded class forward declaration. Signed-off-by: Laurent Pinchart --- include/libcamera/internal/request.h | 12 ++- include/libcamera/request.h | 15 +--- src/libcamera/request.cpp | 106 +++++++++++++++------------ 3 files changed, 71 insertions(+), 62 deletions(-) diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h index 693097ee9a26..7715077b3f7c 100644 --- a/include/libcamera/internal/request.h +++ b/include/libcamera/internal/request.h @@ -30,7 +30,7 @@ class Request::Private : public Extensible::Private LIBCAMERA_DECLARE_PUBLIC(Request) public: - Private(Camera *camera); + Private(Camera *camera, uint64_t cookie); ~Private(); Camera *camera() const { return camera_; } @@ -41,7 +41,7 @@ public: bool completeBuffer(FrameBuffer *buffer); void complete(); void cancel(); - void reset(); + void reset(Request::ReuseFlag flags); void prepare(std::chrono::milliseconds timeout = 0ms); Signal<> prepared; @@ -56,14 +56,20 @@ private: void timeout(); Camera *camera_; + const uint64_t cookie_; + + Status status_; bool cancelled_; uint32_t sequence_ = 0; bool prepared_ = false; + ControlList controls_; + ControlList metadata_; + BufferMap bufferMap_; + std::unordered_set pending_; std::map notifiers_; std::unique_ptr timer_; - ControlList metadata_; }; } /* namespace libcamera */ diff --git a/include/libcamera/request.h b/include/libcamera/request.h index 290983f61352..08ac6e8daba7 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -22,7 +22,6 @@ namespace libcamera { class Camera; -class CameraControlValidator; class FrameBuffer; class Stream; @@ -49,16 +48,16 @@ public: void reuse(ReuseFlag flags = Default); - ControlList &controls() { return controls_; } + ControlList &controls(); const ControlList &metadata() const; - const BufferMap &buffers() const { return bufferMap_; } + const BufferMap &buffers() const; int addBuffer(const Stream *stream, FrameBuffer *buffer, std::unique_ptr &&fence = {}); FrameBuffer *findBuffer(const Stream *stream) const; uint32_t sequence() const; - uint64_t cookie() const { return cookie_; } - Status status() const { return status_; } + uint64_t cookie() const; + Status status() const; bool hasPendingBuffers() const; @@ -66,12 +65,6 @@ public: private: LIBCAMERA_DISABLE_COPY(Request) - - ControlList controls_; - BufferMap bufferMap_; - - const uint64_t cookie_; - Status status_; }; std::ostream &operator<<(std::ostream &out, const Request &r); diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index 57f1f060d5b4..9d30091a9af7 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -52,12 +52,16 @@ LOG_DEFINE_CATEGORY(Request) /** * \brief Create a Request::Private - * \param camera The Camera that creates the request + * \param[in] camera The Camera that creates the request + * \param[in] cookie Opaque cookie for application use * * \todo Add a validator for metadata controls. */ -Request::Private::Private(Camera *camera) - : camera_(camera), cancelled_(false), metadata_(controls::controls) +Request::Private::Private(Camera *camera, uint64_t cookie) + : camera_(camera), cookie_(cookie), status_(RequestPending), + cancelled_(false), + controls_(camera->controls(), camera->_d()->validator()), + metadata_(controls::controls) { } @@ -132,7 +136,7 @@ void Request::Private::complete() ASSERT(request->status() == RequestPending); ASSERT(!hasPendingBuffers()); - request->status_ = cancelled_ ? RequestCancelled : RequestComplete; + status_ = cancelled_ ? RequestCancelled : RequestComplete; LOG(Request, Debug) << request->toString(); @@ -174,18 +178,38 @@ void Request::Private::cancel() /** * \brief Reset the request internal data to default values + * \param[in] flags Indicate whether or not to reuse the buffers * * After calling this function, all request internal data will have default - * values as if the Request::Private instance had just been constructed. + * values as if the Request::Private instance had just been constructed, with + * the exception of bufferMap_ if the Request::ReuseFlag::ReuseBuffers flag is + * set in \a flags. */ -void Request::Private::reset() +void Request::Private::reset(Request::ReuseFlag flags) { - sequence_ = 0; + status_ = RequestPending; cancelled_ = false; + sequence_ = 0; prepared_ = false; + + controls_.clear(); + metadata_.clear(); + pending_.clear(); notifiers_.clear(); timer_.reset(); + + if (flags & ReuseBuffers) { + Request *request = _o(); + + for (auto pair : bufferMap_) { + FrameBuffer *buffer = pair.second; + buffer->_d()->setRequest(request); + pending_.insert(buffer); + } + } else { + bufferMap_.clear(); + } } /* @@ -286,9 +310,8 @@ void Request::Private::notifierActivated(FrameBuffer *buffer) ASSERT(it != notifiers_.end()); notifiers_.erase(it); - Request *request = _o(); LOG(Request, Debug) - << "Request " << request->cookie() << " buffer " << buffer + << "Request " << cookie_ << " buffer " << buffer << " fence signalled"; if (!notifiers_.empty()) @@ -305,8 +328,7 @@ void Request::Private::timeout() ASSERT(!notifiers_.empty()); notifiers_.clear(); - Request *request = _o(); - LOG(Request, Debug) << "Request prepare timeout: " << request->cookie(); + LOG(Request, Debug) << "Request prepare timeout: " << cookie_; cancel(); @@ -361,13 +383,11 @@ void Request::Private::timeout() * completely opaque to libcamera. */ Request::Request(Camera *camera, uint64_t cookie) - : Extensible(std::make_unique(camera)), - controls_(camera->controls(), camera->_d()->validator()), - cookie_(cookie), status_(RequestPending) + : Extensible(std::make_unique(camera, cookie)) { LIBCAMERA_TRACEPOINT(request_construct, this); - LOG(Request, Debug) << "Created request - cookie: " << cookie_; + LOG(Request, Debug) << "Created request - cookie: " << cookie; } Request::~Request() @@ -389,26 +409,10 @@ void Request::reuse(ReuseFlag flags) { LIBCAMERA_TRACEPOINT(request_reuse, this); - _d()->reset(); - - if (flags & ReuseBuffers) { - for (auto pair : bufferMap_) { - FrameBuffer *buffer = pair.second; - buffer->_d()->setRequest(this); - _d()->pending_.insert(buffer); - } - } else { - bufferMap_.clear(); - } - - status_ = RequestPending; - - controls_.clear(); - _d()->metadata_.clear(); + _d()->reset(flags); } /** - * \fn Request::controls() * \brief Retrieve the request's ControlList * * Requests store a list of controls to be applied to all frames captured for @@ -422,6 +426,10 @@ void Request::reuse(ReuseFlag flags) * * \return A reference to the ControlList in this request */ +ControlList &Request::controls() +{ + return _d()->controls_; +} /** * \brief Retrieve the request's metadata @@ -433,14 +441,19 @@ const ControlList &Request::metadata() const } /** - * \fn Request::buffers() * \brief Retrieve the request's streams to buffers map * * Return a reference to the map that associates each Stream part of the - * request to the FrameBuffer the Stream output should be directed to. + * request to the FrameBuffer the Stream output should be directed to. If a + * stream is not utilised in this request there will be no buffer for that + * stream in the map. * * \return The map of Stream to FrameBuffer */ +const Request::BufferMap &Request::buffers() const +{ + return _d()->bufferMap_; +} /** * \brief Add a FrameBuffer with its associated Stream to the Request @@ -493,7 +506,7 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer, return -EEXIST; } - auto [it, inserted] = bufferMap_.try_emplace(stream, buffer); + auto [it, inserted] = _d()->bufferMap_.try_emplace(stream, buffer); if (!inserted) { LOG(Request, Error) << "FrameBuffer already set for stream"; return -EEXIST; @@ -508,15 +521,6 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer, return 0; } -/** - * \var Request::bufferMap_ - * \brief Mapping of streams to buffers for this request - * - * The bufferMap_ tracks the buffers associated with each stream. If a stream is - * not utilised in this request there will be no buffer for that stream in the - * map. - */ - /** * \brief Return the buffer associated with a stream * \param[in] stream The stream the buffer is associated to @@ -525,8 +529,8 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer, */ FrameBuffer *Request::findBuffer(const Stream *stream) const { - const auto it = bufferMap_.find(stream); - if (it == bufferMap_.end()) + const auto it = _d()->bufferMap_.find(stream); + if (it == _d()->bufferMap_.end()) return nullptr; return it->second; @@ -553,13 +557,15 @@ uint32_t Request::sequence() const } /** - * \fn Request::cookie() * \brief Retrieve the cookie set when the request was created * \return The request cookie */ +uint64_t Request::cookie() const +{ + return _d()->cookie_; +} /** - * \fn Request::status() * \brief Retrieve the request completion status * * The request status indicates whether the request has completed successfully @@ -570,6 +576,10 @@ uint32_t Request::sequence() const * * \return The request completion status */ +Request::Status Request::status() const +{ + return _d()->status_; +} /** * \brief Check if a request has buffers yet to be completed