From patchwork Mon Mar 31 14:17:12 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: 23070 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 8B3D8C323E for ; Mon, 31 Mar 2025 14:17:24 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 348AD6897E; Mon, 31 Mar 2025 16:17:23 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="kQh7lUNQ"; 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 8A53568967 for ; Mon, 31 Mar 2025 16:17:21 +0200 (CEST) Received: from pb-laptop.local (185.221.143.221.nat.pool.zt.hu [185.221.143.221]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id EC564725 for ; Mon, 31 Mar 2025 16:15:29 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1743430530; bh=/2TDMo6sy3rw/LK2jyYrToXT3X8XIktbQVzSeHDe6yI=; h=From:To:Subject:Date:From; b=kQh7lUNQ7NdU0SVBg/zolj2injyDPHY6haIFWOuTxPD5e9wT/zbKCvZM1nKQT08+v r1eTPDP/5YRbGl4GCvRGICL4Qt8glfElgy5+8tBDMWX1tclmzB0YKF5jv1GPIXkpE1 L9W8AeCniYVo28lz3pUdsKFxwdMA0r0qNgtJ2GF4= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Subject: [PATCH v2 1/3] libcamera: request: Make `controls_`/`metadata_` members Date: Mon, 31 Mar 2025 16:17:12 +0200 Message-ID: <20250331141714.512538-1-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.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" The lifetimes of these two `ControlList`s are tied entirely to the request object, so simplify the code by making them member variables instead of manually managing their dynamic lifetime. Signed-off-by: Barnabás Pőcze Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- include/libcamera/request.h | 8 ++++---- src/libcamera/request.cpp | 17 ++++------------- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/include/libcamera/request.h b/include/libcamera/request.h index e214a9d13..3061e2fb0 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -49,8 +49,8 @@ public: void reuse(ReuseFlag flags = Default); - ControlList &controls() { return *controls_; } - ControlList &metadata() { return *metadata_; } + ControlList &controls() { return controls_; } + ControlList &metadata() { return metadata_; } const BufferMap &buffers() const { return bufferMap_; } int addBuffer(const Stream *stream, FrameBuffer *buffer, std::unique_ptr fence = nullptr); @@ -67,8 +67,8 @@ public: private: LIBCAMERA_DISABLE_COPY(Request) - ControlList *controls_; - ControlList *metadata_; + ControlList controls_; + ControlList metadata_; BufferMap bufferMap_; const uint64_t cookie_; diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index b206ac132..e7eb1c0c8 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -354,16 +354,10 @@ void Request::Private::timeout() */ Request::Request(Camera *camera, uint64_t cookie) : Extensible(std::make_unique(camera)), + controls_(controls::controls, camera->_d()->validator()), + metadata_(controls::controls), /* \todo Add a validator for metadata controls. */ cookie_(cookie), status_(RequestPending) { - controls_ = new ControlList(controls::controls, - camera->_d()->validator()); - - /** - * \todo Add a validator for metadata controls. - */ - metadata_ = new ControlList(controls::controls); - LIBCAMERA_TRACEPOINT(request_construct, this); LOG(Request, Debug) << "Created request - cookie: " << cookie_; @@ -372,9 +366,6 @@ Request::Request(Camera *camera, uint64_t cookie) Request::~Request() { LIBCAMERA_TRACEPOINT(request_destroy, this); - - delete metadata_; - delete controls_; } /** @@ -405,8 +396,8 @@ void Request::reuse(ReuseFlag flags) status_ = RequestPending; - controls_->clear(); - metadata_->clear(); + controls_.clear(); + metadata_.clear(); } /** From patchwork Mon Mar 31 14:17:13 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: 23071 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 CB3DAC323E for ; Mon, 31 Mar 2025 14:17:27 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5530668989; Mon, 31 Mar 2025 16:17:25 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="R6i7yfNU"; 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 CE7056897A for ; Mon, 31 Mar 2025 16:17:21 +0200 (CEST) Received: from pb-laptop.local (185.221.143.221.nat.pool.zt.hu [185.221.143.221]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 4049082A for ; Mon, 31 Mar 2025 16:15:30 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1743430530; bh=mDKs3DsA+6gULCwnaECzJszzkhmD/iBQI9xcY2dhuYs=; h=From:To:Subject:Date:In-Reply-To:References:From; b=R6i7yfNUG0P9PA+bbvReUZQbt8Tggegc0Q/1EbaplU9ah+4F6I9isO1Tufm3frJ+l kqPkBrhBX857ZvcdBcHrKTgITYY/CiqXPfuH08tqNu8CABVEsALDRM2oNLBeb0h+lK 1oF7jfULVm2fs59ThINqL6Hv9irqK3ivLNZZ47sM= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Subject: [PATCH v2 2/3] libcamera: request: Avoid double map lookup Date: Mon, 31 Mar 2025 16:17:13 +0200 Message-ID: <20250331141714.512538-2-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250331141714.512538-1-barnabas.pocze@ideasonboard.com> References: <20250331141714.512538-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" Use `try_emplace()` that more or less combines `find()` and `operator[]` in one function. Signed-off-by: Barnabás Pőcze Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- src/libcamera/request.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index e7eb1c0c8..fc364441a 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -475,15 +475,14 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer, return -EEXIST; } - auto it = bufferMap_.find(stream); - if (it != bufferMap_.end()) { + auto [it, inserted] = bufferMap_.try_emplace(stream, buffer); + if (!inserted) { LOG(Request, Error) << "FrameBuffer already set for stream"; return -EEXIST; } buffer->_d()->setRequest(this); _d()->pending_.insert(buffer); - bufferMap_[stream] = buffer; if (fence && fence->isValid()) buffer->_d()->setFence(std::move(fence)); From patchwork Mon Mar 31 14:17:14 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: 23072 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 DD957C323E for ; Mon, 31 Mar 2025 14:17:29 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 40A2D6898B; Mon, 31 Mar 2025 16:17:27 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Tgbxf+0T"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 0E2F86897E for ; Mon, 31 Mar 2025 16:17:22 +0200 (CEST) Received: from pb-laptop.local (185.221.143.221.nat.pool.zt.hu [185.221.143.221]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 87646965 for ; Mon, 31 Mar 2025 16:15:30 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1743430530; bh=wcwxfsQrBtsAAPX0DN19c5FfjdMIOX8ZsXgsq3XrVy8=; h=From:To:Subject:Date:In-Reply-To:References:From; b=Tgbxf+0TgwsIpNriyQlen3tvL4/pups47tlS/gNxC+KjSAIvFozxrYjwCUZ5Mwyv5 xCGvyw7fpd2Hf1NpNXmVFPidN9WumH+koBfLot/+EnlXt9XUmA3Wm7XVIPgpgzbpww IZMP6cq2oY69xeSVQYZkpOzfJ17NeCsTDmng9+bA= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Subject: [PATCH v2 3/3] libcamera: request: Move all members to internal private class Date: Mon, 31 Mar 2025 16:17:14 +0200 Message-ID: <20250331141714.512538-3-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250331141714.512538-1-barnabas.pocze@ideasonboard.com> References: <20250331141714.512538-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" Move all members of `Request` into `Request::Private`. Signed-off-by: Barnabás Pőcze Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- include/libcamera/internal/request.h | 10 ++- include/libcamera/request.h | 18 ++--- src/libcamera/request.cpp | 104 ++++++++++++++------------- 3 files changed, 69 insertions(+), 63 deletions(-) diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h index 73e9bb5cc..009ebbf14 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_; } @@ -39,7 +39,7 @@ public: bool completeBuffer(FrameBuffer *buffer); void complete(); void cancel(); - void reset(); + void reset(ReuseFlag flags); void prepare(std::chrono::milliseconds timeout = 0ms); Signal<> prepared; @@ -57,10 +57,16 @@ private: bool cancelled_; uint32_t sequence_ = 0; bool prepared_ = false; + Status status_ = RequestPending; + const uint64_t cookie_; std::unordered_set pending_; std::map> notifiers_; std::unique_ptr timer_; + + ControlList controls_; + ControlList metadata_; + BufferMap bufferMap_; }; } /* namespace libcamera */ diff --git a/include/libcamera/request.h b/include/libcamera/request.h index 3061e2fb0..609b55885 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -49,16 +49,17 @@ public: void reuse(ReuseFlag flags = Default); - ControlList &controls() { return controls_; } - ControlList &metadata() { return metadata_; } - const BufferMap &buffers() const { return bufferMap_; } + ControlList &controls(); + ControlList &metadata(); + + const BufferMap &buffers() const; int addBuffer(const Stream *stream, FrameBuffer *buffer, std::unique_ptr fence = nullptr); 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,13 +67,6 @@ public: private: LIBCAMERA_DISABLE_COPY(Request) - - ControlList controls_; - ControlList metadata_; - 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 fc364441a..1f1be1c7e 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -53,9 +53,12 @@ LOG_DEFINE_CATEGORY(Request) /** * \brief Create a Request::Private * \param camera The Camera that creates the request + * \param cookie Opaque cookie for application use */ -Request::Private::Private(Camera *camera) - : camera_(camera), cancelled_(false) +Request::Private::Private(Camera *camera, uint64_t cookie) + : camera_(camera), cancelled_(false), cookie_(cookie), + controls_(controls::controls, camera->_d()->validator()), + metadata_(controls::controls) /* \todo Add a validator for metadata controls. */ { } @@ -121,10 +124,10 @@ void Request::Private::complete() { Request *request = _o(); - ASSERT(request->status() == RequestPending); + ASSERT(status_ == RequestPending); ASSERT(!hasPendingBuffers()); - request->status_ = cancelled_ ? RequestCancelled : RequestComplete; + status_ = cancelled_ ? RequestCancelled : RequestComplete; LOG(Request, Debug) << request->toString(); @@ -158,26 +161,39 @@ void Request::Private::cancel() { LIBCAMERA_TRACEPOINT(request_cancel, this); - Request *request = _o(); - ASSERT(request->status() == RequestPending); + ASSERT(status_ == RequestPending); doCancelRequest(); } /** - * \brief Reset the request internal data to default values - * - * After calling this function, all request internal data will have default - * values as if the Request::Private instance had just been constructed. + * \copydoc Request::reuse() + * \sa Request::reuse() */ -void Request::Private::reset() +void Request::Private::reset(Request::ReuseFlag flags) { sequence_ = 0; cancelled_ = false; prepared_ = false; + status_ = RequestPending; + pending_.clear(); notifiers_.clear(); timer_.reset(); + + controls_.clear(); + metadata_.clear(); + + if (flags & ReuseBuffers) { + auto *request = _o(); + + for (const auto &[stream, buffer] : bufferMap_) { + buffer->_d()->setRequest(request); + pending_.insert(buffer); + } + } else { + bufferMap_.clear(); + } } /* @@ -353,14 +369,11 @@ void Request::Private::timeout() * completely opaque to libcamera. */ Request::Request(Camera *camera, uint64_t cookie) - : Extensible(std::make_unique(camera)), - controls_(controls::controls, camera->_d()->validator()), - metadata_(controls::controls), /* \todo Add a validator for metadata controls. */ - 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() @@ -382,26 +395,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(); - 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 @@ -415,9 +412,12 @@ void Request::reuse(ReuseFlag flags) * * \return A reference to the ControlList in this request */ +ControlList &Request::controls() +{ + return _d()->controls_; +} /** - * \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 @@ -425,6 +425,10 @@ void Request::reuse(ReuseFlag flags) * * \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 @@ -475,7 +479,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; @@ -490,15 +494,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 @@ -507,20 +502,25 @@ 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 &bufferMap = _d()->bufferMap_; + + const auto it = bufferMap.find(stream); + if (it == bufferMap.end()) return nullptr; return it->second; } /** - * \fn Request::metadata() * \brief Retrieve the request's metadata * \todo Offer a read-only API towards applications while keeping a read/write * API internally. * \return The metadata associated with the request */ +ControlList &Request::metadata() +{ + return _d()->metadata_; +} /** * \brief Retrieve the sequence number for the request @@ -543,13 +543,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 @@ -560,6 +562,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