{"id":12805,"url":"https://patchwork.libcamera.org/api/1.1/patches/12805/?format=json","web_url":"https://patchwork.libcamera.org/patch/12805/","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":"<20210704232817.8205-4-laurent.pinchart@ideasonboard.com>","date":"2021-07-04T23:28:17","name":"[libcamera-devel,3/3] libcamera: framebuffer: Make FrameBuffer class Extensible","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"b7a9ff454e7918ad12800947d89d7af9fad089f9","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/1.1/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":{"id":14,"url":"https://patchwork.libcamera.org/api/1.1/users/14/?format=json","username":"pinchartl","first_name":"Laurent","last_name":"Pinchart","email":"laurent.pinchart@ideasonboard.com"},"mbox":"https://patchwork.libcamera.org/patch/12805/mbox/","series":[{"id":2206,"url":"https://patchwork.libcamera.org/api/1.1/series/2206/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=2206","date":"2021-07-04T23:28:14","name":"libcamera: Make Framebuffer class Extensible","version":1,"mbox":"https://patchwork.libcamera.org/series/2206/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/12805/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/12805/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 5C454C3223\n\tfor ;\n\tSun, 4 Jul 2021 23:29:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BB2F868509;\n\tMon, 5 Jul 2021 01:29:05 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 092EE60285\n\tfor ;\n\tMon, 5 Jul 2021 01:29:03 +0200 (CEST)","from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9AB052E4;\n\tMon, 5 Jul 2021 01:29:02 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"rXxyKSMu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1625441342;\n\tbh=f4mBgqo2UbWxfmCfWGseTbkEI5ADgdXsN4wFD71pwMQ=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=rXxyKSMujniIruuqqo34MRS0RuTuWnrBGW/fwOu1Q9OlYWWYdKkicd9fTOs63y7Q7\n\tPMQyPv3vTQ7xwuAF7suFqLzlE9ofAynJZYUnJbz28iFdXFco5yJJvSs7mvzV7VyjX+\n\tyiAH+I0DhbkcjnEimnye0kZKUxzasrfWQGpjb8Tw=","From":"Laurent Pinchart ","To":"libcamera-devel@lists.libcamera.org","Date":"Mon, 5 Jul 2021 02:28:17 +0300","Message-Id":"<20210704232817.8205-4-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.31.1","In-Reply-To":"<20210704232817.8205-1-laurent.pinchart@ideasonboard.com>","References":"<20210704232817.8205-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH 3/3] libcamera: framebuffer: Make\n\tFrameBuffer class Extensible","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":"Implement the D-Pointer design pattern in the FrameBuffer class to allow\nchanging internal data without affecting the public ABI.\n\nMove the request_ field and the setRequest() function to the\nFrameBuffer::Private class. This allows hiding the setRequest() function\nfrom the public API, removing one todo item. More fields may be moved\nlater.\n\nSigned-off-by: Laurent Pinchart \n---\n include/libcamera/framebuffer.h | 8 +++---\n include/libcamera/internal/framebuffer.h | 13 +++++++++\n src/libcamera/framebuffer.cpp | 34 +++++++++++++++---------\n src/libcamera/pipeline/ipu3/cio2.cpp | 3 ++-\n src/libcamera/pipeline/ipu3/frames.cpp | 5 ++--\n src/libcamera/request.cpp | 7 ++---\n 6 files changed, 47 insertions(+), 23 deletions(-)","diff":"diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h\nindex baf22a466907..7265e816b036 100644\n--- a/include/libcamera/framebuffer.h\n+++ b/include/libcamera/framebuffer.h\n@@ -35,8 +35,10 @@ struct FrameMetadata {\n \tstd::vector planes;\n };\n \n-class FrameBuffer final\n+class FrameBuffer final : public Extensible\n {\n+\tLIBCAMERA_DECLARE_PRIVATE();\n+\n public:\n \tstruct Plane {\n \t\tFileDescriptor fd;\n@@ -47,8 +49,7 @@ public:\n \n \tconst std::vector &planes() const { return planes_; }\n \n-\tRequest *request() const { return request_; }\n-\tvoid setRequest(Request *request) { request_ = request; }\n+\tRequest *request() const;\n \tconst FrameMetadata &metadata() const { return metadata_; }\n \n \tunsigned int cookie() const { return cookie_; }\n@@ -63,7 +64,6 @@ private:\n \n \tstd::vector planes_;\n \n-\tRequest *request_;\n \tFrameMetadata metadata_;\n \n \tunsigned int cookie_;\ndiff --git a/include/libcamera/internal/framebuffer.h b/include/libcamera/internal/framebuffer.h\nindex 0c76fc62af1d..a11e895d9b88 100644\n--- a/include/libcamera/internal/framebuffer.h\n+++ b/include/libcamera/internal/framebuffer.h\n@@ -47,6 +47,19 @@ public:\n \tMappedFrameBuffer(const FrameBuffer *buffer, int flags);\n };\n \n+class FrameBuffer::Private : public Extensible::Private\n+{\n+\tLIBCAMERA_DECLARE_PUBLIC(FrameBuffer)\n+\n+public:\n+\tPrivate(FrameBuffer *buffer);\n+\n+\tvoid setRequest(Request *request) { request_ = request; }\n+\n+private:\n+\tRequest *request_;\n+};\n+\n } /* namespace libcamera */\n \n #endif /* __LIBCAMERA_INTERNAL_FRAMEBUFFER_H__ */\ndiff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp\nindex 4bde556c4013..5e13e281fb8c 100644\n--- a/src/libcamera/framebuffer.cpp\n+++ b/src/libcamera/framebuffer.cpp\n@@ -100,6 +100,21 @@ LOG_DEFINE_CATEGORY(Buffer)\n * \\brief Array of per-plane metadata\n */\n \n+FrameBuffer::Private::Private(FrameBuffer *buffer)\n+\t: Extensible::Private(buffer), request_(nullptr)\n+{\n+}\n+\n+/**\n+ * \\fn FrameBuffer::Private::setRequest()\n+ * \\brief Set the request this buffer belongs to\n+ * \\param[in] request Request to set\n+ *\n+ * For buffers added to requests by applications, this method is called by\n+ * Request::addBuffer() or Request::reuse(). For buffers internal to pipeline\n+ * handlers, it is called by the pipeline handlers themselves.\n+ */\n+\n /**\n * \\class FrameBuffer\n * \\brief Frame buffer data and its associated dynamic metadata\n@@ -161,7 +176,7 @@ LOG_DEFINE_CATEGORY(Buffer)\n * \\param[in] cookie Cookie\n */\n FrameBuffer::FrameBuffer(const std::vector &planes, unsigned int cookie)\n-\t: planes_(planes), request_(nullptr), cookie_(cookie)\n+\t: Extensible(new Private(this)), planes_(planes), cookie_(cookie)\n {\n }\n \n@@ -172,7 +187,6 @@ FrameBuffer::FrameBuffer(const std::vector &planes, unsigned int cookie)\n */\n \n /**\n- * \\fn FrameBuffer::request()\n * \\brief Retrieve the request this buffer belongs to\n *\n * The intended callers of this method are buffer completion handlers that\n@@ -185,18 +199,12 @@ FrameBuffer::FrameBuffer(const std::vector &planes, unsigned int cookie)\n * \\return The Request the FrameBuffer belongs to, or nullptr if the buffer is\n * not associated with a request\n */\n+Request *FrameBuffer::request() const\n+{\n+\tconst Private *const d = LIBCAMERA_D_PTR();\n \n-/**\n- * \\fn FrameBuffer::setRequest()\n- * \\brief Set the request this buffer belongs to\n- * \\param[in] request Request to set\n- *\n- * For buffers added to requests by applications, this method is called by\n- * Request::addBuffer() or Request::reuse(). For buffers internal to pipeline\n- * handlers, it is called by the pipeline handlers themselves.\n- *\n- * \\todo Shall be hidden from applications with a d-pointer design.\n- */\n+\treturn d->request_;\n+}\n \n /**\n * \\fn FrameBuffer::metadata()\ndiff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\nindex 1be2cbcd5cac..1bcd580e251c 100644\n--- a/src/libcamera/pipeline/ipu3/cio2.cpp\n+++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n@@ -14,6 +14,7 @@\n #include \n \n #include \"libcamera/internal/camera_sensor.h\"\n+#include \"libcamera/internal/framebuffer.h\"\n #include \"libcamera/internal/media_device.h\"\n #include \"libcamera/internal/v4l2_subdevice.h\"\n \n@@ -278,7 +279,7 @@ FrameBuffer *CIO2Device::queueBuffer(Request *request, FrameBuffer *rawBuffer)\n \n \t\tbuffer = availableBuffers_.front();\n \t\tavailableBuffers_.pop();\n-\t\tbuffer->setRequest(request);\n+\t\tbuffer->_d()->setRequest(request);\n \t}\n \n \tint ret = output_->queueBuffer(buffer);\ndiff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp\nindex ce5ccbf18e41..a4c3477cd9ef 100644\n--- a/src/libcamera/pipeline/ipu3/frames.cpp\n+++ b/src/libcamera/pipeline/ipu3/frames.cpp\n@@ -10,6 +10,7 @@\n #include \n #include \n \n+#include \"libcamera/internal/framebuffer.h\"\n #include \"libcamera/internal/pipeline_handler.h\"\n #include \"libcamera/internal/v4l2_videodevice.h\"\n \n@@ -56,8 +57,8 @@ IPU3Frames::Info *IPU3Frames::create(Request *request)\n \tFrameBuffer *paramBuffer = availableParamBuffers_.front();\n \tFrameBuffer *statBuffer = availableStatBuffers_.front();\n \n-\tparamBuffer->setRequest(request);\n-\tstatBuffer->setRequest(request);\n+\tparamBuffer->_d()->setRequest(request);\n+\tstatBuffer->_d()->setRequest(request);\n \n \tavailableParamBuffers_.pop();\n \tavailableStatBuffers_.pop();\ndiff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\nindex 5faf3c71ff02..c095c9f45b09 100644\n--- a/src/libcamera/request.cpp\n+++ b/src/libcamera/request.cpp\n@@ -18,6 +18,7 @@\n #include \n \n #include \"libcamera/internal/camera_controls.h\"\n+#include \"libcamera/internal/framebuffer.h\"\n #include \"libcamera/internal/tracepoints.h\"\n \n /**\n@@ -121,7 +122,7 @@ void Request::reuse(ReuseFlag flags)\n \tif (flags & ReuseBuffers) {\n \t\tfor (auto pair : bufferMap_) {\n \t\t\tFrameBuffer *buffer = pair.second;\n-\t\t\tbuffer->setRequest(this);\n+\t\t\tbuffer->_d()->setRequest(this);\n \t\t\tpending_.insert(buffer);\n \t\t}\n \t} else {\n@@ -191,7 +192,7 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer)\n \t\treturn -EEXIST;\n \t}\n \n-\tbuffer->setRequest(this);\n+\tbuffer->_d()->setRequest(this);\n \tpending_.insert(buffer);\n \tbufferMap_[stream] = buffer;\n \n@@ -336,7 +337,7 @@ bool Request::completeBuffer(FrameBuffer *buffer)\n \tint ret = pending_.erase(buffer);\n \tASSERT(ret == 1);\n \n-\tbuffer->setRequest(nullptr);\n+\tbuffer->_d()->setRequest(nullptr);\n \n \tif (buffer->metadata().status == FrameMetadata::FrameCancelled)\n \t\tcancelled_ = true;\n","prefixes":["libcamera-devel","3/3"]}