{"id":23072,"url":"https://patchwork.libcamera.org/api/patches/23072/?format=json","web_url":"https://patchwork.libcamera.org/patch/23072/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20250331141714.512538-3-barnabas.pocze@ideasonboard.com>","date":"2025-03-31T14:17:14","name":"[v2,3/3] libcamera: request: Move all members to internal private class","commit_ref":null,"pull_url":null,"state":"deferred","archived":false,"hash":"10d32c14fb5802da175489c8bef6a32f956545a9","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/?format=json","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/23072/mbox/","series":[{"id":5093,"url":"https://patchwork.libcamera.org/api/series/5093/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=5093","date":"2025-03-31T14:17:12","name":"[v2,1/3] libcamera: request: Make `controls_`/`metadata_` members","version":2,"mbox":"https://patchwork.libcamera.org/series/5093/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/23072/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/23072/checks/","tags":{},"headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id DD957C323E\n\tfor ;\n\tMon, 31 Mar 2025 14:17:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 40A2D6898B;\n\tMon, 31 Mar 2025 16:17:27 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0E2F86897E\n\tfor ;\n\tMon, 31 Mar 2025 16:17:22 +0200 (CEST)","from pb-laptop.local (185.221.143.221.nat.pool.zt.hu\n\t[185.221.143.221])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 87646965\n\tfor ;\n\tMon, 31 Mar 2025 16:15:30 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Tgbxf+0T\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743430530;\n\tbh=wcwxfsQrBtsAAPX0DN19c5FfjdMIOX8ZsXgsq3XrVy8=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=Tgbxf+0TgwsIpNriyQlen3tvL4/pups47tlS/gNxC+KjSAIvFozxrYjwCUZ5Mwyv5\n\txCGvyw7fpd2Hf1NpNXmVFPidN9WumH+koBfLot/+EnlXt9XUmA3Wm7XVIPgpgzbpww\n\tIZMP6cq2oY69xeSVQYZkpOzfJ17NeCsTDmng9+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\n\tprivate 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","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"Move all members of `Request` into `Request::Private`.\n\nSigned-off-by: Barnabás Pőcze \n---\n include/libcamera/internal/request.h | 10 ++-\n include/libcamera/request.h | 18 ++---\n src/libcamera/request.cpp | 104 ++++++++++++++-------------\n 3 files changed, 69 insertions(+), 63 deletions(-)","diff":"diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h\nindex 73e9bb5cc..009ebbf14 100644\n--- a/include/libcamera/internal/request.h\n+++ b/include/libcamera/internal/request.h\n@@ -30,7 +30,7 @@ class Request::Private : public Extensible::Private\n \tLIBCAMERA_DECLARE_PUBLIC(Request)\n \n public:\n-\tPrivate(Camera *camera);\n+\tPrivate(Camera *camera, uint64_t cookie);\n \t~Private();\n \n \tCamera *camera() const { return camera_; }\n@@ -39,7 +39,7 @@ public:\n \tbool completeBuffer(FrameBuffer *buffer);\n \tvoid complete();\n \tvoid cancel();\n-\tvoid reset();\n+\tvoid reset(ReuseFlag flags);\n \n \tvoid prepare(std::chrono::milliseconds timeout = 0ms);\n \tSignal<> prepared;\n@@ -57,10 +57,16 @@ private:\n \tbool cancelled_;\n \tuint32_t sequence_ = 0;\n \tbool prepared_ = false;\n+\tStatus status_ = RequestPending;\n+\tconst uint64_t cookie_;\n \n \tstd::unordered_set pending_;\n \tstd::map> notifiers_;\n \tstd::unique_ptr timer_;\n+\n+\tControlList controls_;\n+\tControlList metadata_;\n+\tBufferMap bufferMap_;\n };\n \n } /* namespace libcamera */\ndiff --git a/include/libcamera/request.h b/include/libcamera/request.h\nindex 3061e2fb0..609b55885 100644\n--- a/include/libcamera/request.h\n+++ b/include/libcamera/request.h\n@@ -49,16 +49,17 @@ public:\n \n \tvoid reuse(ReuseFlag flags = Default);\n \n-\tControlList &controls() { return controls_; }\n-\tControlList &metadata() { return metadata_; }\n-\tconst BufferMap &buffers() const { return bufferMap_; }\n+\tControlList &controls();\n+\tControlList &metadata();\n+\n+\tconst BufferMap &buffers() const;\n \tint addBuffer(const Stream *stream, FrameBuffer *buffer,\n \t\t std::unique_ptr fence = nullptr);\n \tFrameBuffer *findBuffer(const Stream *stream) const;\n \n \tuint32_t sequence() const;\n-\tuint64_t cookie() const { return cookie_; }\n-\tStatus status() const { return status_; }\n+\tuint64_t cookie() const;\n+\tStatus status() const;\n \n \tbool hasPendingBuffers() const;\n \n@@ -66,13 +67,6 @@ public:\n \n private:\n \tLIBCAMERA_DISABLE_COPY(Request)\n-\n-\tControlList controls_;\n-\tControlList metadata_;\n-\tBufferMap bufferMap_;\n-\n-\tconst uint64_t cookie_;\n-\tStatus status_;\n };\n \n std::ostream &operator<<(std::ostream &out, const Request &r);\ndiff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\nindex fc364441a..1f1be1c7e 100644\n--- a/src/libcamera/request.cpp\n+++ b/src/libcamera/request.cpp\n@@ -53,9 +53,12 @@ LOG_DEFINE_CATEGORY(Request)\n /**\n * \\brief Create a Request::Private\n * \\param camera The Camera that creates the request\n+ * \\param cookie Opaque cookie for application use\n */\n-Request::Private::Private(Camera *camera)\n-\t: camera_(camera), cancelled_(false)\n+Request::Private::Private(Camera *camera, uint64_t cookie)\n+\t: camera_(camera), cancelled_(false), cookie_(cookie),\n+\t controls_(controls::controls, camera->_d()->validator()),\n+\t metadata_(controls::controls) /* \\todo Add a validator for metadata controls. */\n {\n }\n \n@@ -121,10 +124,10 @@ void Request::Private::complete()\n {\n \tRequest *request = _o();\n \n-\tASSERT(request->status() == RequestPending);\n+\tASSERT(status_ == RequestPending);\n \tASSERT(!hasPendingBuffers());\n \n-\trequest->status_ = cancelled_ ? RequestCancelled : RequestComplete;\n+\tstatus_ = cancelled_ ? RequestCancelled : RequestComplete;\n \n \tLOG(Request, Debug) << request->toString();\n \n@@ -158,26 +161,39 @@ void Request::Private::cancel()\n {\n \tLIBCAMERA_TRACEPOINT(request_cancel, this);\n \n-\tRequest *request = _o();\n-\tASSERT(request->status() == RequestPending);\n+\tASSERT(status_ == RequestPending);\n \n \tdoCancelRequest();\n }\n \n /**\n- * \\brief Reset the request internal data to default values\n- *\n- * After calling this function, all request internal data will have default\n- * values as if the Request::Private instance had just been constructed.\n+ * \\copydoc Request::reuse()\n+ * \\sa Request::reuse()\n */\n-void Request::Private::reset()\n+void Request::Private::reset(Request::ReuseFlag flags)\n {\n \tsequence_ = 0;\n \tcancelled_ = false;\n \tprepared_ = false;\n+\tstatus_ = RequestPending;\n+\n \tpending_.clear();\n \tnotifiers_.clear();\n \ttimer_.reset();\n+\n+\tcontrols_.clear();\n+\tmetadata_.clear();\n+\n+\tif (flags & ReuseBuffers) {\n+\t\tauto *request = _o();\n+\n+\t\tfor (const auto &[stream, buffer] : bufferMap_) {\n+\t\t\tbuffer->_d()->setRequest(request);\n+\t\t\tpending_.insert(buffer);\n+\t\t}\n+\t} else {\n+\t\tbufferMap_.clear();\n+\t}\n }\n \n /*\n@@ -353,14 +369,11 @@ void Request::Private::timeout()\n * completely opaque to libcamera.\n */\n Request::Request(Camera *camera, uint64_t cookie)\n-\t: Extensible(std::make_unique(camera)),\n-\t controls_(controls::controls, camera->_d()->validator()),\n-\t metadata_(controls::controls), /* \\todo Add a validator for metadata controls. */\n-\t cookie_(cookie), status_(RequestPending)\n+\t: Extensible(std::make_unique(camera, cookie))\n {\n \tLIBCAMERA_TRACEPOINT(request_construct, this);\n \n-\tLOG(Request, Debug) << \"Created request - cookie: \" << cookie_;\n+\tLOG(Request, Debug) << \"Created request - cookie: \" << cookie;\n }\n \n Request::~Request()\n@@ -382,26 +395,10 @@ void Request::reuse(ReuseFlag flags)\n {\n \tLIBCAMERA_TRACEPOINT(request_reuse, this);\n \n-\t_d()->reset();\n-\n-\tif (flags & ReuseBuffers) {\n-\t\tfor (auto pair : bufferMap_) {\n-\t\t\tFrameBuffer *buffer = pair.second;\n-\t\t\tbuffer->_d()->setRequest(this);\n-\t\t\t_d()->pending_.insert(buffer);\n-\t\t}\n-\t} else {\n-\t\tbufferMap_.clear();\n-\t}\n-\n-\tstatus_ = RequestPending;\n-\n-\tcontrols_.clear();\n-\tmetadata_.clear();\n+\t_d()->reset(flags);\n }\n \n /**\n- * \\fn Request::controls()\n * \\brief Retrieve the request's ControlList\n *\n * Requests store a list of controls to be applied to all frames captured for\n@@ -415,9 +412,12 @@ void Request::reuse(ReuseFlag flags)\n *\n * \\return A reference to the ControlList in this request\n */\n+ControlList &Request::controls()\n+{\n+\treturn _d()->controls_;\n+}\n \n /**\n- * \\fn Request::buffers()\n * \\brief Retrieve the request's streams to buffers map\n *\n * Return a reference to the map that associates each Stream part of the\n@@ -425,6 +425,10 @@ void Request::reuse(ReuseFlag flags)\n *\n * \\return The map of Stream to FrameBuffer\n */\n+const Request::BufferMap &Request::buffers() const\n+{\n+\treturn _d()->bufferMap_;\n+}\n \n /**\n * \\brief Add a FrameBuffer with its associated Stream to the Request\n@@ -475,7 +479,7 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,\n \t\treturn -EEXIST;\n \t}\n \n-\tauto [it, inserted] = bufferMap_.try_emplace(stream, buffer);\n+\tauto [it, inserted] = _d()->bufferMap_.try_emplace(stream, buffer);\n \tif (!inserted) {\n \t\tLOG(Request, Error) << \"FrameBuffer already set for stream\";\n \t\treturn -EEXIST;\n@@ -490,15 +494,6 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,\n \treturn 0;\n }\n \n-/**\n- * \\var Request::bufferMap_\n- * \\brief Mapping of streams to buffers for this request\n- *\n- * The bufferMap_ tracks the buffers associated with each stream. If a stream is\n- * not utilised in this request there will be no buffer for that stream in the\n- * map.\n- */\n-\n /**\n * \\brief Return the buffer associated with a stream\n * \\param[in] stream The stream the buffer is associated to\n@@ -507,20 +502,25 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer,\n */\n FrameBuffer *Request::findBuffer(const Stream *stream) const\n {\n-\tconst auto it = bufferMap_.find(stream);\n-\tif (it == bufferMap_.end())\n+\tconst auto &bufferMap = _d()->bufferMap_;\n+\n+\tconst auto it = bufferMap.find(stream);\n+\tif (it == bufferMap.end())\n \t\treturn nullptr;\n \n \treturn it->second;\n }\n \n /**\n- * \\fn Request::metadata()\n * \\brief Retrieve the request's metadata\n * \\todo Offer a read-only API towards applications while keeping a read/write\n * API internally.\n * \\return The metadata associated with the request\n */\n+ControlList &Request::metadata()\n+{\n+\treturn _d()->metadata_;\n+}\n \n /**\n * \\brief Retrieve the sequence number for the request\n@@ -543,13 +543,15 @@ uint32_t Request::sequence() const\n }\n \n /**\n- * \\fn Request::cookie()\n * \\brief Retrieve the cookie set when the request was created\n * \\return The request cookie\n */\n+uint64_t Request::cookie() const\n+{\n+\treturn _d()->cookie_;\n+}\n \n /**\n- * \\fn Request::status()\n * \\brief Retrieve the request completion status\n *\n * The request status indicates whether the request has completed successfully\n@@ -560,6 +562,10 @@ uint32_t Request::sequence() const\n *\n * \\return The request completion status\n */\n+Request::Status Request::status() const\n+{\n+\treturn _d()->status_;\n+}\n \n /**\n * \\brief Check if a request has buffers yet to be completed\n","prefixes":["v2","3/3"]}