Patch Detail
Show a patch.
GET /api/patches/25722/?format=api
{ "id": 25722, "url": "https://patchwork.libcamera.org/api/patches/25722/?format=api", "web_url": "https://patchwork.libcamera.org/patch/25722/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20260113000808.15395-3-laurent.pinchart@ideasonboard.com>", "date": "2026-01-13T00:07:34", "name": "[02/36] libcamera: request: Move all private member variables to Private class", "commit_ref": null, "pull_url": null, "state": "new", "archived": false, "hash": "ef068242cb6ef74e725761d7bbeb63d6ea01a716", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/25722/mbox/", "series": [ { "id": 5703, "url": "https://patchwork.libcamera.org/api/series/5703/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=5703", "date": "2026-01-13T00:07:32", "name": "libcamera: Global configuration file improvements", "version": 1, "mbox": "https://patchwork.libcamera.org/series/5703/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/25722/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/25722/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "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 25ED5BDCBF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 13 Jan 2026 00:08:36 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5B13D61FC2;\n\tTue, 13 Jan 2026 01:08:35 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DD35161FB7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 13 Jan 2026 01:08:32 +0100 (CET)", "from pendragon.ideasonboard.com (81-175-209-152.bb.dnainternet.fi\n\t[81.175.209.152])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 32DF250A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 13 Jan 2026 01:08:07 +0100 (CET)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"kOr+ovBq\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768262887;\n\tbh=r43+7FueC3TwfykC3fDq5vMzaTlx1yuToFECMfVCYyw=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=kOr+ovBq59VEcP2Dz8urQR4fidWvOp5netH03sIoYuTdfw3u+XlFaZu0JGmVrGc+F\n\thOouU5W51J0cDUub5wu/r90AUbo203ZvWa/KXtdJqmn2F6YMrGCdIW6LAbedFmHysx\n\tXkNtSyE4Xz3MeYFQChVLP1DQPn3U7jlqZfm9yNy0=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Subject": "[PATCH 02/36] libcamera: request: Move all private member variables\n\tto 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", "Content-Transfer-Encoding": "8bit", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "The Request class has a set of private member variables, with some of\nthem stored in the Request class itself, and some in the\nRequest::Private class. Storing the variables in the Request class\nitself has the advantage that accessors can be inline, at the cost of\nABI breakage if variables need to be added, removed or otherwise\nmodified.\n\nThe controls_ and metadata_ variables have recently been turned from\npointers to instances. This broke the ABI. To avoid further breakages,\nmove all remaining private member variables to Request::Private. The\nperformance impact of not inlining accessors will be negligible.\n\nWhile at it, drop an unneeded class forward declaration.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n include/libcamera/internal/request.h | 12 ++-\n include/libcamera/request.h | 15 +---\n src/libcamera/request.cpp | 106 +++++++++++++++------------\n 3 files changed, 71 insertions(+), 62 deletions(-)", "diff": "diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h\nindex 693097ee9a26..7715077b3f7c 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@@ -41,7 +41,7 @@ public:\n \tbool completeBuffer(FrameBuffer *buffer);\n \tvoid complete();\n \tvoid cancel();\n-\tvoid reset();\n+\tvoid reset(Request::ReuseFlag flags);\n \n \tvoid prepare(std::chrono::milliseconds timeout = 0ms);\n \tSignal<> prepared;\n@@ -56,14 +56,20 @@ private:\n \tvoid timeout();\n \n \tCamera *camera_;\n+\tconst uint64_t cookie_;\n+\n+\tStatus status_;\n \tbool cancelled_;\n \tuint32_t sequence_ = 0;\n \tbool prepared_ = false;\n \n+\tControlList controls_;\n+\tControlList metadata_;\n+\tBufferMap bufferMap_;\n+\n \tstd::unordered_set<FrameBuffer *> pending_;\n \tstd::map<FrameBuffer *, EventNotifier> notifiers_;\n \tstd::unique_ptr<Timer> timer_;\n-\tControlList metadata_;\n };\n \n } /* namespace libcamera */\ndiff --git a/include/libcamera/request.h b/include/libcamera/request.h\nindex 290983f61352..08ac6e8daba7 100644\n--- a/include/libcamera/request.h\n+++ b/include/libcamera/request.h\n@@ -22,7 +22,6 @@\n namespace libcamera {\n \n class Camera;\n-class CameraControlValidator;\n class FrameBuffer;\n class Stream;\n \n@@ -49,16 +48,16 @@ public:\n \n \tvoid reuse(ReuseFlag flags = Default);\n \n-\tControlList &controls() { return controls_; }\n+\tControlList &controls();\n \tconst ControlList &metadata() const;\n-\tconst BufferMap &buffers() const { return bufferMap_; }\n+\tconst BufferMap &buffers() const;\n \tint addBuffer(const Stream *stream, FrameBuffer *buffer,\n \t\t std::unique_ptr<Fence> &&fence = {});\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,12 +65,6 @@ public:\n \n private:\n \tLIBCAMERA_DISABLE_COPY(Request)\n-\n-\tControlList controls_;\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 57f1f060d5b4..9d30091a9af7 100644\n--- a/src/libcamera/request.cpp\n+++ b/src/libcamera/request.cpp\n@@ -52,12 +52,16 @@ LOG_DEFINE_CATEGORY(Request)\n \n /**\n * \\brief Create a Request::Private\n- * \\param camera The Camera that creates the request\n+ * \\param[in] camera The Camera that creates the request\n+ * \\param[in] cookie Opaque cookie for application use\n *\n * \\todo Add a validator for metadata controls.\n */\n-Request::Private::Private(Camera *camera)\n-\t: camera_(camera), cancelled_(false), metadata_(controls::controls)\n+Request::Private::Private(Camera *camera, uint64_t cookie)\n+\t: camera_(camera), cookie_(cookie), status_(RequestPending),\n+\t cancelled_(false),\n+\t controls_(camera->controls(), camera->_d()->validator()),\n+\t metadata_(controls::controls)\n {\n }\n \n@@ -132,7 +136,7 @@ void Request::Private::complete()\n \tASSERT(request->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@@ -174,18 +178,38 @@ void Request::Private::cancel()\n \n /**\n * \\brief Reset the request internal data to default values\n+ * \\param[in] flags Indicate whether or not to reuse the buffers\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+ * values as if the Request::Private instance had just been constructed, with\n+ * the exception of bufferMap_ if the Request::ReuseFlag::ReuseBuffers flag is\n+ * set in \\a flags.\n */\n-void Request::Private::reset()\n+void Request::Private::reset(Request::ReuseFlag flags)\n {\n-\tsequence_ = 0;\n+\tstatus_ = RequestPending;\n \tcancelled_ = false;\n+\tsequence_ = 0;\n \tprepared_ = false;\n+\n+\tcontrols_.clear();\n+\tmetadata_.clear();\n+\n \tpending_.clear();\n \tnotifiers_.clear();\n \ttimer_.reset();\n+\n+\tif (flags & ReuseBuffers) {\n+\t\tRequest *request = _o<Request>();\n+\n+\t\tfor (auto pair : bufferMap_) {\n+\t\t\tFrameBuffer *buffer = pair.second;\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@@ -286,9 +310,8 @@ void Request::Private::notifierActivated(FrameBuffer *buffer)\n \tASSERT(it != notifiers_.end());\n \tnotifiers_.erase(it);\n \n-\tRequest *request = _o<Request>();\n \tLOG(Request, Debug)\n-\t\t<< \"Request \" << request->cookie() << \" buffer \" << buffer\n+\t\t<< \"Request \" << cookie_ << \" buffer \" << buffer\n \t\t<< \" fence signalled\";\n \n \tif (!notifiers_.empty())\n@@ -305,8 +328,7 @@ void Request::Private::timeout()\n \tASSERT(!notifiers_.empty());\n \tnotifiers_.clear();\n \n-\tRequest *request = _o<Request>();\n-\tLOG(Request, Debug) << \"Request prepare timeout: \" << request->cookie();\n+\tLOG(Request, Debug) << \"Request prepare timeout: \" << cookie_;\n \n \tcancel();\n \n@@ -361,13 +383,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<Private>(camera)),\n-\t controls_(camera->controls(), camera->_d()->validator()),\n-\t cookie_(cookie), status_(RequestPending)\n+\t: Extensible(std::make_unique<Private>(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@@ -389,26 +409,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-\t_d()->metadata_.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@@ -422,6 +426,10 @@ 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 * \\brief Retrieve the request's metadata\n@@ -433,14 +441,19 @@ const ControlList &Request::metadata() const\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- * request to the FrameBuffer the Stream output should be directed to.\n+ * request to the FrameBuffer the Stream output should be directed to. If a\n+ * stream is not utilised in this request there will be no buffer for that\n+ * stream in the map.\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@@ -493,7 +506,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@@ -508,15 +521,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@@ -525,8 +529,8 @@ 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 it = _d()->bufferMap_.find(stream);\n+\tif (it == _d()->bufferMap_.end())\n \t\treturn nullptr;\n \n \treturn it->second;\n@@ -553,13 +557,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@@ -570,6 +576,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": [ "02/36" ] }