Patch Detail
Show a patch.
GET /api/1.1/patches/23072/?format=api
{ "id": 23072, "url": "https://patchwork.libcamera.org/api/1.1/patches/23072/?format=api", "web_url": "https://patchwork.libcamera.org/patch/23072/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/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": "<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/1.1/people/216/?format=api", "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/1.1/series/5093/?format=api", "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": "<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 DD957C323E\n\tfor <parsemail@patchwork.libcamera.org>;\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 <libcamera-devel@lists.libcamera.org>;\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 <libcamera-devel@lists.libcamera.org>;\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?= <barnabas.pocze@ideasonboard.com>", "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": "<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": "Move all members of `Request` into `Request::Private`.\n\nSigned-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\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<FrameBuffer *> pending_;\n \tstd::map<FrameBuffer *, std::unique_ptr<EventNotifier>> notifiers_;\n \tstd::unique_ptr<Timer> 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> 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<Request>();\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<Request>();\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<Request>();\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<Private>(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<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@@ -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" ] }