Patch Detail
Show a patch.
GET /api/1.1/patches/12896/?format=api
{ "id": 12896, "url": "https://patchwork.libcamera.org/api/1.1/patches/12896/?format=api", "web_url": "https://patchwork.libcamera.org/patch/12896/", "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": "<20210711170359.300-4-laurent.pinchart@ideasonboard.com>", "date": "2021-07-11T17:03:59", "name": "[libcamera-devel,v2,3/3] libcamera: framebuffer: Make FrameBuffer class Extensible", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "98b53fbfdde93eeb29b53802f98603543e194261", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/1.1/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/12896/mbox/", "series": [ { "id": 2225, "url": "https://patchwork.libcamera.org/api/1.1/series/2225/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=2225", "date": "2021-07-11T17:03:56", "name": "libcamera: Make Framebuffer class Extensible", "version": 2, "mbox": "https://patchwork.libcamera.org/series/2225/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/12896/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/12896/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 5A3FDC3225\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 11 Jul 2021 17:04:53 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0C35868523;\n\tSun, 11 Jul 2021 19:04:53 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A92EF6851B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 11 Jul 2021 19:04:49 +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 49794255\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 11 Jul 2021 19:04:49 +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=\"HRBsNFnd\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1626023089;\n\tbh=oTJ4JSdVKRE6swaW6R5yBvS4ixscrI8uHyWH0KrCG28=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=HRBsNFnd0yfh9KecJQYieTSDtLTOocdlpatXji7O/tXdCJjAHJsu/k7I/SjC8RM48\n\tjT68VrClf3FxtdjZI+3QaShExaJfPOsD2vcDC+ng9Qqig9zw9KRiBsDhqhxH5A3B3R\n\tyOvPqeNscIdW7zYvO9l3/byZKtOk8Jf4gwFAdaaI=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Sun, 11 Jul 2021 20:03:59 +0300", "Message-Id": "<20210711170359.300-4-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.31.1", "In-Reply-To": "<20210711170359.300-1-laurent.pinchart@ideasonboard.com>", "References": "<20210711170359.300-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v2 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": "<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": "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 <laurent.pinchart@ideasonboard.com>\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\nReviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n---\nChanges since v1:\n\n- Drop extraneous semicolon\n- Use _d() instead of LIBCAMERA_D_PTR()\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, 46 insertions(+), 24 deletions(-)", "diff": "diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h\nindex baf22a466907..28307890eac9 100644\n--- a/include/libcamera/framebuffer.h\n+++ b/include/libcamera/framebuffer.h\n@@ -35,8 +35,10 @@ struct FrameMetadata {\n \tstd::vector<Plane> 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<Plane> &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<Plane> 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..40bf64b0a4fe 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<Plane> &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<Plane> &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,10 @@ FrameBuffer::FrameBuffer(const std::vector<Plane> &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-\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+Request *FrameBuffer::request() const\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 <libcamera/stream.h>\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 <libcamera/framebuffer.h>\n #include <libcamera/request.h>\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 <libcamera/stream.h>\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", "v2", "3/3" ] }