{"id":14526,"url":"https://patchwork.libcamera.org/api/1.1/patches/14526/?format=json","web_url":"https://patchwork.libcamera.org/patch/14526/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/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":"<20211019154915.2123366-1-kieran.bingham@ideasonboard.com>","date":"2021-10-19T15:49:15","name":"libcamera: request: Make it extensible","commit_ref":null,"pull_url":null,"state":"rejected","archived":false,"hash":"9eeba03db2bb2ab5a0949e1e89be88bd205300d5","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/1.1/people/4/?format=json","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/14526/mbox/","series":[{"id":2706,"url":"https://patchwork.libcamera.org/api/1.1/series/2706/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=2706","date":"2021-10-19T15:49:15","name":"libcamera: request: Make it extensible","version":1,"mbox":"https://patchwork.libcamera.org/series/2706/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/14526/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/14526/checks/","tags":{},"headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":["parsemail@patchwork.libcamera.org","kbingham@ideasonboard.com"],"Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 5718CBDB1C\n\tfor ;\n\tWed, 10 Nov 2021 14:01:52 +0000 (UTC)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net\n\t[86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E6FA1501\n\tfor ;\n\tWed, 10 Nov 2021 15:01:51 +0100 (CET)","from perceval.ideasonboard.com\n\tby perceval.ideasonboard.com with LMTP id SKrrNf7obmEGNAAA4E0KoQ\n\t(envelope-from )\n\tfor ; Tue, 19 Oct 2021 17:49:18 +0200","from Monstersaurus.ksquared.org.uk.beta.tailscale.net\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net\n\t[86.31.172.11])\tby\n\tperceval.ideasonboard.com (Postfix) with ESMTPSA id AC71612A;\n\tTue, 19 Oct 2021 17:49:18 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1636552912;\n\tbh=w/QHewBj4dTtJgxt8wXCLKHt/9AuF4VyhXdqb7avJ2U=;\n\th=From:To:Cc:Subject:Date:Resent-From:Resent-To:From;\n\tb=syurQuAi01mw3+v2eRxpPUS2g3nHVtYBpWw9oHrod6DfoDuNfxNSTZo+afz/H+Gvg\n\txhiMQwMgol5Y5XGK6odo8dMxCxeD7+EkHcC/V6NJmJNT1cZJurxNPe/OSZSH4TXrV1\n\tbSaBB3TuPmeIXPjJiYIHypUMM3wDF10qm/A9498I=","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634658558;\n\tbh=w/QHewBj4dTtJgxt8wXCLKHt/9AuF4VyhXdqb7avJ2U=;\n\th=From:To:Cc:Subject:Date:From;\n\tb=vMDIznmV/3pQJbUu0HjoVmC+IOAp5WI34BHykqaXqnL46In4BlP2wVgAF/9KKFVVz\n\tsQSb7pK5S79ZiECf4dpH4JyGLzsFDC5kTlbiGi70TEfGu6kH8yJ6RJBsfZL/Bd5M13\n\tRMmomFmaSpalU1KEAH8Lsu6P32CkxKIYbztQ3hEA="],"From":"Kieran Bingham ","To":"libcamera devel ","Cc":"Kieran Bingham ","Subject":"[PATCH] libcamera: request: Make it extensible","Date":"Tue, 19 Oct 2021 16:49:15 +0100","Message-Id":"<20211019154915.2123366-1-kieran.bingham@ideasonboard.com>","X-Mailer":"git-send-email 2.30.2","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","X-TUID":"VvZsZrHmO+DA","Resent-From":"Kieran Bingham ","Resent-To":"parsemail@patchwork.libcamera.org"},"content":"Provide an extensible private object for the Request class.\nThis allows us to make modifications to the private API and storage of\nrequests without affecting the public API or ABI.\n\nInitially the pending_ structure and hasPendingBuffers are adapted to\nsupport this move.\n\nSigned-off-by: Kieran Bingham \n---\nv2:\n - Drop all the previous assertions that were added here.\n - Focus on making the Request use the d pointer design.\n - Move Request::Private to internal/request.h to make it available\n to other internal components such as PipelineHandlers\n\nThis directly competes with Laurent's implementation. Posted mostly as\nreference, I don't mind which one goes forward.\n\nIf Laurent's goes in, I'll just have to pull out the remaining parts of\nthis and apply on top.\n\n---\n include/libcamera/internal/meson.build | 1 +\n include/libcamera/internal/request.h | 32 +++++++++++++++++++++\n include/libcamera/request.h | 7 +++--\n src/libcamera/request.cpp | 39 ++++++++++++++++++--------\n 4 files changed, 64 insertions(+), 15 deletions(-)\n create mode 100644 include/libcamera/internal/request.h","diff":"diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build\nindex 665fd6de4ed3..cae65b0604ff 100644\n--- a/include/libcamera/internal/meson.build\n+++ b/include/libcamera/internal/meson.build\n@@ -34,6 +34,7 @@ libcamera_internal_headers = files([\n 'pipeline_handler.h',\n 'process.h',\n 'pub_key.h',\n+ 'request.h',\n 'source_paths.h',\n 'sysfs.h',\n 'v4l2_device.h',\ndiff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h\nnew file mode 100644\nindex 000000000000..630ae60e8b6c\n--- /dev/null\n+++ b/include/libcamera/internal/request.h\n@@ -0,0 +1,32 @@\n+/* SPDX-License-Identifier: LGPL-2.1-or-later */\n+/*\n+ * Copyright (C) 2021, Google Inc.\n+ *\n+ * request.h - Internal capture request handling\n+ */\n+#ifndef __LIBCAMERA_INTERNAL_REQUEST_H__\n+#define __LIBCAMERA_INTERNAL_REQUEST_H__\n+\n+#include \n+\n+#include \n+\n+#include \n+\n+namespace libcamera {\n+\n+class Camera;\n+class CameraControlValidator;\n+class FrameBuffer;\n+\n+class Request::Private : public Extensible::Private\n+{\n+\tLIBCAMERA_DECLARE_PUBLIC(Request)\n+\n+private:\n+\tstd::unordered_set pending_;\n+};\n+\n+} /* namespace libcamera */\n+\n+#endif /* __LIBCAMERA_INTERNAL_REQUEST_H__ */\ndiff --git a/include/libcamera/request.h b/include/libcamera/request.h\nindex 2d361c9d97dc..71d088551d94 100644\n--- a/include/libcamera/request.h\n+++ b/include/libcamera/request.h\n@@ -25,8 +25,10 @@ class CameraControlValidator;\n class FrameBuffer;\n class Stream;\n \n-class Request\n+class Request : public Extensible\n {\n+\tLIBCAMERA_DECLARE_PRIVATE()\n+\n public:\n \tenum Status {\n \t\tRequestPending,\n@@ -56,7 +58,7 @@ public:\n \tuint64_t cookie() const { return cookie_; }\n \tStatus status() const { return status_; }\n \n-\tbool hasPendingBuffers() const { return !pending_.empty(); }\n+\tbool hasPendingBuffers() const;\n \n \tstd::string toString() const;\n \n@@ -75,7 +77,6 @@ private:\n \tControlList *controls_;\n \tControlList *metadata_;\n \tBufferMap bufferMap_;\n-\tstd::unordered_set pending_;\n \n \tuint32_t sequence_;\n \tconst uint64_t cookie_;\ndiff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\nindex f95ce4db5eaa..d363d7a7b005 100644\n--- a/src/libcamera/request.cpp\n+++ b/src/libcamera/request.cpp\n@@ -5,7 +5,7 @@\n * request.cpp - Capture request handling\n */\n \n-#include \n+#include \"libcamera/internal/request.h\"\n \n #include \n #include \n@@ -22,14 +22,26 @@\n #include \"libcamera/internal/tracepoints.h\"\n \n /**\n- * \\file request.h\n+ * \\file libcamera/request.h\n * \\brief Describes a frame capture request to be processed by a camera\n+ *\n+ * \\file internal/request.h\n+ * \\brief Provides the internal implementation details for a Request\n */\n \n namespace libcamera {\n \n LOG_DEFINE_CATEGORY(Request)\n \n+/**\n+ * \\class Request::Private\n+ * \\brief Base class for FrameBuffer private data\n+ *\n+ * The Request::Private class stores all private data associated with a request.\n+ * It implements the d-pointer design pattern to hide core Request data from the\n+ * public API, and exposes utility functions to pipeline handlers.\n+ */\n+\n /**\n * \\enum Request::Status\n * Request completion status\n@@ -74,8 +86,9 @@ LOG_DEFINE_CATEGORY(Request)\n * completely opaque to libcamera.\n */\n Request::Request(Camera *camera, uint64_t cookie)\n-\t: camera_(camera), sequence_(0), cookie_(cookie),\n-\t status_(RequestPending), cancelled_(false)\n+\t: Extensible(std::make_unique()), camera_(camera),\n+\t sequence_(0), cookie_(cookie), status_(RequestPending),\n+\t cancelled_(false)\n {\n \t/**\n \t * \\todo Should the Camera expose a validator instance, to avoid\n@@ -117,12 +130,11 @@ void Request::reuse(ReuseFlag flags)\n {\n \tLIBCAMERA_TRACEPOINT(request_reuse, this);\n \n-\tpending_.clear();\n+\t_d()->pending_.clear();\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\tpending_.insert(buffer);\n \t\t}\n \t} else {\n \t\tbufferMap_.clear();\n@@ -192,7 +204,7 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer)\n \t}\n \n \tbuffer->_d()->setRequest(this);\n-\tpending_.insert(buffer);\n+\t_d()->pending_.insert(buffer);\n \tbufferMap_[stream] = buffer;\n \n \treturn 0;\n@@ -267,12 +279,15 @@ FrameBuffer *Request::findBuffer(const Stream *stream) const\n */\n \n /**\n- * \\fn Request::hasPendingBuffers()\n * \\brief Check if a request has buffers yet to be completed\n *\n * \\return True if the request has buffers pending for completion, false\n * otherwise\n */\n+bool Request::hasPendingBuffers() const\n+{\n+\treturn !_d()->pending_.empty();\n+}\n \n /**\n * \\brief Complete a queued request\n@@ -307,12 +322,12 @@ void Request::cancel()\n \n \tASSERT(status_ == RequestPending);\n \n-\tfor (FrameBuffer *buffer : pending_) {\n+\tfor (FrameBuffer *buffer : _d()->pending_) {\n \t\tbuffer->cancel();\n \t\tcamera_->bufferCompleted.emit(this, buffer);\n \t}\n \n-\tpending_.clear();\n+\t_d()->pending_.clear();\n \tcancelled_ = true;\n }\n \n@@ -333,7 +348,7 @@ bool Request::completeBuffer(FrameBuffer *buffer)\n {\n \tLIBCAMERA_TRACEPOINT(request_complete_buffer, this, buffer);\n \n-\tint ret = pending_.erase(buffer);\n+\tint ret = _d()->pending_.erase(buffer);\n \tASSERT(ret == 1);\n \n \tbuffer->_d()->setRequest(nullptr);\n@@ -361,7 +376,7 @@ std::string Request::toString() const\n \n \t/* Example Output: Request(55:P:1/2:6523524) */\n \tss << \"Request(\" << sequence_ << \":\" << statuses[status_] << \":\"\n-\t << pending_.size() << \"/\" << bufferMap_.size() << \":\"\n+\t << _d()->pending_.size() << \"/\" << bufferMap_.size() << \":\"\n \t << cookie_ << \")\";\n \n \treturn ss.str();\n","prefixes":[]}