Patch Detail
Show a patch.
GET /api/1.1/patches/10111/?format=api
{ "id": 10111, "url": "https://patchwork.libcamera.org/api/1.1/patches/10111/?format=api", "web_url": "https://patchwork.libcamera.org/patch/10111/", "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": "<20201020014005.12783-6-laurent.pinchart@ideasonboard.com>", "date": "2020-10-20T01:40:05", "name": "[libcamera-devel,v2,5/5] libcamera: camera: Inherit from Extensible", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "0a907da538b550ef671df701ab5d71c07d20f313", "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/10111/mbox/", "series": [ { "id": 1386, "url": "https://patchwork.libcamera.org/api/1.1/series/1386/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=1386", "date": "2020-10-20T01:40:00", "name": "libcamera: Implement d-pointer design pattern", "version": 2, "mbox": "https://patchwork.libcamera.org/series/1386/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/10111/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/10111/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 B59EDBDB1F\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 20 Oct 2020 01:41:02 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 826BE613C6;\n\tTue, 20 Oct 2020 03:41:02 +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 07120613C4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 20 Oct 2020 03:40:58 +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 89C32309\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 20 Oct 2020 03:40:57 +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=\"nGdnWSR0\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1603158057;\n\tbh=CwQQ2JCE6HH8mYZQ8L+NdCxqaymtTPr13XUK5pyvKWM=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=nGdnWSR0Ydc/hVVCkx4ARPGc8V+Z0D9L31PSSoJLl4JJMZ/LjxhgjPfE1LGtXrPtY\n\tdKPY/NQ4AK4eXs2FbaJ0csQExpcYdpc+BfWynAj9jTSiX9/asVwe9q3OnnGrMXRspV\n\t1gc0VO7G2VMlL6lGdLjIHRQbQ22q5MWVOeiJLpYc=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Tue, 20 Oct 2020 04:40:05 +0300", "Message-Id": "<20201020014005.12783-6-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.27.0", "In-Reply-To": "<20201020014005.12783-1-laurent.pinchart@ideasonboard.com>", "References": "<20201020014005.12783-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Subject": "[libcamera-devel] [PATCH v2 5/5] libcamera: camera: Inherit from\n\tExtensible", "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>", "Content-Type": "text/plain; charset=\"utf-8\"", "Content-Transfer-Encoding": "base64", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "Use the d-pointer infrastructure offered by the Extensible class to\nreplace the custom implementation.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n---\nChanges since v1:\n\n- Declare variables explicitly\n---\n include/libcamera/camera.h | 9 +--\n src/libcamera/camera.cpp | 123 ++++++++++++++++++++++---------------\n 2 files changed, 80 insertions(+), 52 deletions(-)", "diff": "diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\nindex 79ff8d6b67a4..5c5f1a05c77d 100644\n--- a/include/libcamera/camera.h\n+++ b/include/libcamera/camera.h\n@@ -13,6 +13,7 @@\n #include <string>\n \n #include <libcamera/controls.h>\n+#include <libcamera/extensible.h>\n #include <libcamera/object.h>\n #include <libcamera/request.h>\n #include <libcamera/signal.h>\n@@ -70,8 +71,11 @@ protected:\n \tstd::vector<StreamConfiguration> config_;\n };\n \n-class Camera final : public Object, public std::enable_shared_from_this<Camera>\n+class Camera final : public Object, public std::enable_shared_from_this<Camera>,\n+\t\t public Extensible\n {\n+\tLIBCAMERA_DECLARE_PRIVATE(Camera)\n+\n public:\n \tstatic std::shared_ptr<Camera> create(PipelineHandler *pipe,\n \t\t\t\t\t const std::string &id,\n@@ -107,9 +111,6 @@ private:\n \t const std::set<Stream *> &streams);\n \t~Camera();\n \n-\tclass Private;\n-\tstd::unique_ptr<Private> p_;\n-\n \tfriend class PipelineHandler;\n \tvoid disconnect();\n \tvoid requestComplete(Request *request);\ndiff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\nindex 9590ab7249d3..a6181bba8cc7 100644\n--- a/src/libcamera/camera.cpp\n+++ b/src/libcamera/camera.cpp\n@@ -270,8 +270,10 @@ std::size_t CameraConfiguration::size() const\n * \\brief The vector of stream configurations\n */\n \n-class Camera::Private\n+class Camera::Private : public Extensible::Private\n {\n+\tLIBCAMERA_DECLARE_PUBLIC(Camera)\n+\n public:\n \tenum State {\n \t\tCameraAvailable,\n@@ -280,7 +282,7 @@ public:\n \t\tCameraRunning,\n \t};\n \n-\tPrivate(PipelineHandler *pipe, const std::string &id,\n+\tPrivate(Camera *camera, PipelineHandler *pipe, const std::string &id,\n \t\tconst std::set<Stream *> &streams);\n \t~Private();\n \n@@ -301,10 +303,11 @@ private:\n \tstd::atomic<State> state_;\n };\n \n-Camera::Private::Private(PipelineHandler *pipe, const std::string &id,\n+Camera::Private::Private(Camera *camera, PipelineHandler *pipe,\n+\t\t\t const std::string &id,\n \t\t\t const std::set<Stream *> &streams)\n-\t: pipe_(pipe->shared_from_this()), id_(id), streams_(streams),\n-\t disconnected_(false), state_(CameraAvailable)\n+\t: Extensible::Private(camera), pipe_(pipe->shared_from_this()), id_(id),\n+\t streams_(streams), disconnected_(false), state_(CameraAvailable)\n {\n }\n \n@@ -519,7 +522,8 @@ std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,\n */\n const std::string &Camera::id() const\n {\n-\treturn p_->id_;\n+\tconst Private *const d = LIBCAMERA_D_PTR(Camera);\n+\treturn d->id_;\n }\n \n /**\n@@ -547,7 +551,7 @@ const std::string &Camera::id() const\n \n Camera::Camera(PipelineHandler *pipe, const std::string &id,\n \t const std::set<Stream *> &streams)\n-\t: p_(new Private(pipe, id, streams))\n+\t: Extensible(new Private(this, pipe, id, streams))\n {\n }\n \n@@ -569,28 +573,32 @@ Camera::~Camera()\n */\n void Camera::disconnect()\n {\n+\tPrivate *const d = LIBCAMERA_D_PTR(Camera);\n+\n \tLOG(Camera, Debug) << \"Disconnecting camera \" << id();\n \n-\tp_->disconnect();\n+\td->disconnect();\n \tdisconnected.emit(this);\n }\n \n int Camera::exportFrameBuffers(Stream *stream,\n \t\t\t std::vector<std::unique_ptr<FrameBuffer>> *buffers)\n {\n-\tint ret = p_->isAccessAllowed(Private::CameraConfigured);\n+\tPrivate *const d = LIBCAMERA_D_PTR(Camera);\n+\n+\tint ret = d->isAccessAllowed(Private::CameraConfigured);\n \tif (ret < 0)\n \t\treturn ret;\n \n \tif (streams().find(stream) == streams().end())\n \t\treturn -EINVAL;\n \n-\tif (p_->activeStreams_.find(stream) == p_->activeStreams_.end())\n+\tif (d->activeStreams_.find(stream) == d->activeStreams_.end())\n \t\treturn -EINVAL;\n \n-\treturn p_->pipe_->invokeMethod(&PipelineHandler::exportFrameBuffers,\n-\t\t\t\t ConnectionTypeBlocking, this, stream,\n-\t\t\t\t buffers);\n+\treturn d->pipe_->invokeMethod(&PipelineHandler::exportFrameBuffers,\n+\t\t\t\t ConnectionTypeBlocking, this, stream,\n+\t\t\t\t buffers);\n }\n \n /**\n@@ -619,21 +627,23 @@ int Camera::exportFrameBuffers(Stream *stream,\n */\n int Camera::acquire()\n {\n+\tPrivate *const d = LIBCAMERA_D_PTR(Camera);\n+\n \t/*\n \t * No manual locking is required as PipelineHandler::lock() is\n \t * thread-safe.\n \t */\n-\tint ret = p_->isAccessAllowed(Private::CameraAvailable);\n+\tint ret = d->isAccessAllowed(Private::CameraAvailable);\n \tif (ret < 0)\n \t\treturn ret == -EACCES ? -EBUSY : ret;\n \n-\tif (!p_->pipe_->lock()) {\n+\tif (!d->pipe_->lock()) {\n \t\tLOG(Camera, Info)\n \t\t\t<< \"Pipeline handler in use by another process\";\n \t\treturn -EBUSY;\n \t}\n \n-\tp_->setState(Private::CameraAcquired);\n+\td->setState(Private::CameraAcquired);\n \n \treturn 0;\n }\n@@ -654,14 +664,16 @@ int Camera::acquire()\n */\n int Camera::release()\n {\n-\tint ret = p_->isAccessAllowed(Private::CameraAvailable,\n-\t\t\t\t Private::CameraConfigured, true);\n+\tPrivate *const d = LIBCAMERA_D_PTR(Camera);\n+\n+\tint ret = d->isAccessAllowed(Private::CameraAvailable,\n+\t\t\t\t Private::CameraConfigured, true);\n \tif (ret < 0)\n \t\treturn ret == -EACCES ? -EBUSY : ret;\n \n-\tp_->pipe_->unlock();\n+\td->pipe_->unlock();\n \n-\tp_->setState(Private::CameraAvailable);\n+\td->setState(Private::CameraAvailable);\n \n \treturn 0;\n }\n@@ -678,7 +690,8 @@ int Camera::release()\n */\n const ControlInfoMap &Camera::controls() const\n {\n-\treturn p_->pipe_->controls(this);\n+\tconst Private *const d = LIBCAMERA_D_PTR(Camera);\n+\treturn d->pipe_->controls(this);\n }\n \n /**\n@@ -691,7 +704,8 @@ const ControlInfoMap &Camera::controls() const\n */\n const ControlList &Camera::properties() const\n {\n-\treturn p_->pipe_->properties(this);\n+\tconst Private *const d = LIBCAMERA_D_PTR(Camera);\n+\treturn d->pipe_->properties(this);\n }\n \n /**\n@@ -707,7 +721,8 @@ const ControlList &Camera::properties() const\n */\n const std::set<Stream *> &Camera::streams() const\n {\n-\treturn p_->streams_;\n+\tconst Private *const d = LIBCAMERA_D_PTR(Camera);\n+\treturn d->streams_;\n }\n \n /**\n@@ -728,15 +743,17 @@ const std::set<Stream *> &Camera::streams() const\n */\n std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(const StreamRoles &roles)\n {\n-\tint ret = p_->isAccessAllowed(Private::CameraAvailable,\n-\t\t\t\t Private::CameraRunning);\n+\tPrivate *const d = LIBCAMERA_D_PTR(Camera);\n+\n+\tint ret = d->isAccessAllowed(Private::CameraAvailable,\n+\t\t\t\t Private::CameraRunning);\n \tif (ret < 0)\n \t\treturn nullptr;\n \n \tif (roles.size() > streams().size())\n \t\treturn nullptr;\n \n-\tCameraConfiguration *config = p_->pipe_->generateConfiguration(this, roles);\n+\tCameraConfiguration *config = d->pipe_->generateConfiguration(this, roles);\n \tif (!config) {\n \t\tLOG(Camera, Debug)\n \t\t\t<< \"Pipeline handler failed to generate configuration\";\n@@ -787,8 +804,10 @@ std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(const StreamR\n */\n int Camera::configure(CameraConfiguration *config)\n {\n-\tint ret = p_->isAccessAllowed(Private::CameraAcquired,\n-\t\t\t\t Private::CameraConfigured);\n+\tPrivate *const d = LIBCAMERA_D_PTR(Camera);\n+\n+\tint ret = d->isAccessAllowed(Private::CameraAcquired,\n+\t\t\t\t Private::CameraConfigured);\n \tif (ret < 0)\n \t\treturn ret;\n \n@@ -810,26 +829,26 @@ int Camera::configure(CameraConfiguration *config)\n \n \tLOG(Camera, Info) << msg.str();\n \n-\tret = p_->pipe_->invokeMethod(&PipelineHandler::configure,\n-\t\t\t\t ConnectionTypeBlocking, this, config);\n+\tret = d->pipe_->invokeMethod(&PipelineHandler::configure,\n+\t\t\t\t ConnectionTypeBlocking, this, config);\n \tif (ret)\n \t\treturn ret;\n \n-\tp_->activeStreams_.clear();\n+\td->activeStreams_.clear();\n \tfor (const StreamConfiguration &cfg : *config) {\n \t\tStream *stream = cfg.stream();\n \t\tif (!stream) {\n \t\t\tLOG(Camera, Fatal)\n \t\t\t\t<< \"Pipeline handler failed to update stream configuration\";\n-\t\t\tp_->activeStreams_.clear();\n+\t\t\td->activeStreams_.clear();\n \t\t\treturn -EINVAL;\n \t\t}\n \n \t\tstream->configuration_ = cfg;\n-\t\tp_->activeStreams_.insert(stream);\n+\t\td->activeStreams_.insert(stream);\n \t}\n \n-\tp_->setState(Private::CameraConfigured);\n+\td->setState(Private::CameraConfigured);\n \n \treturn 0;\n }\n@@ -857,8 +876,10 @@ int Camera::configure(CameraConfiguration *config)\n */\n std::unique_ptr<Request> Camera::createRequest(uint64_t cookie)\n {\n-\tint ret = p_->isAccessAllowed(Private::CameraConfigured,\n-\t\t\t\t Private::CameraRunning);\n+\tPrivate *const d = LIBCAMERA_D_PTR(Camera);\n+\n+\tint ret = d->isAccessAllowed(Private::CameraConfigured,\n+\t\t\t\t Private::CameraRunning);\n \tif (ret < 0)\n \t\treturn nullptr;\n \n@@ -889,7 +910,9 @@ std::unique_ptr<Request> Camera::createRequest(uint64_t cookie)\n */\n int Camera::queueRequest(Request *request)\n {\n-\tint ret = p_->isAccessAllowed(Private::CameraRunning);\n+\tPrivate *const d = LIBCAMERA_D_PTR(Camera);\n+\n+\tint ret = d->isAccessAllowed(Private::CameraRunning);\n \tif (ret < 0)\n \t\treturn ret;\n \n@@ -907,14 +930,14 @@ int Camera::queueRequest(Request *request)\n \tfor (auto const &it : request->buffers()) {\n \t\tconst Stream *stream = it.first;\n \n-\t\tif (p_->activeStreams_.find(stream) == p_->activeStreams_.end()) {\n+\t\tif (d->activeStreams_.find(stream) == d->activeStreams_.end()) {\n \t\t\tLOG(Camera, Error) << \"Invalid request\";\n \t\t\treturn -EINVAL;\n \t\t}\n \t}\n \n-\treturn p_->pipe_->invokeMethod(&PipelineHandler::queueRequest,\n-\t\t\t\t ConnectionTypeQueued, this, request);\n+\treturn d->pipe_->invokeMethod(&PipelineHandler::queueRequest,\n+\t\t\t\t ConnectionTypeQueued, this, request);\n }\n \n /**\n@@ -935,18 +958,20 @@ int Camera::queueRequest(Request *request)\n */\n int Camera::start()\n {\n-\tint ret = p_->isAccessAllowed(Private::CameraConfigured);\n+\tPrivate *const d = LIBCAMERA_D_PTR(Camera);\n+\n+\tint ret = d->isAccessAllowed(Private::CameraConfigured);\n \tif (ret < 0)\n \t\treturn ret;\n \n \tLOG(Camera, Debug) << \"Starting capture\";\n \n-\tret = p_->pipe_->invokeMethod(&PipelineHandler::start,\n-\t\t\t\t ConnectionTypeBlocking, this);\n+\tret = d->pipe_->invokeMethod(&PipelineHandler::start,\n+\t\t\t\t ConnectionTypeBlocking, this);\n \tif (ret)\n \t\treturn ret;\n \n-\tp_->setState(Private::CameraRunning);\n+\td->setState(Private::CameraRunning);\n \n \treturn 0;\n }\n@@ -967,16 +992,18 @@ int Camera::start()\n */\n int Camera::stop()\n {\n-\tint ret = p_->isAccessAllowed(Private::CameraRunning);\n+\tPrivate *const d = LIBCAMERA_D_PTR(Camera);\n+\n+\tint ret = d->isAccessAllowed(Private::CameraRunning);\n \tif (ret < 0)\n \t\treturn ret;\n \n \tLOG(Camera, Debug) << \"Stopping capture\";\n \n-\tp_->setState(Private::CameraConfigured);\n+\td->setState(Private::CameraConfigured);\n \n-\tp_->pipe_->invokeMethod(&PipelineHandler::stop, ConnectionTypeBlocking,\n-\t\t\t\tthis);\n+\td->pipe_->invokeMethod(&PipelineHandler::stop, ConnectionTypeBlocking,\n+\t\t\t this);\n \n \treturn 0;\n }\n", "prefixes": [ "libcamera-devel", "v2", "5/5" ] }