{"id":10316,"url":"https://patchwork.libcamera.org/api/patches/10316/?format=json","web_url":"https://patchwork.libcamera.org/patch/10316/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/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":"<20201103010251.14689-5-laurent.pinchart@ideasonboard.com>","date":"2020-11-03T01:02:51","name":"[libcamera-devel,v3,4/4] libcamera: camera: Inherit from Extensible","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"272929fd186da8a01a16fddb9a47716f92b776c4","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/10316/mbox/","series":[{"id":1437,"url":"https://patchwork.libcamera.org/api/series/1437/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=1437","date":"2020-11-03T01:02:47","name":"libcamera: Implement d-pointer design pattern","version":3,"mbox":"https://patchwork.libcamera.org/series/1437/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/10316/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/10316/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 159B6BDB8B\n\tfor ;\n\tTue, 3 Nov 2020 01:03:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CF37362BA6;\n\tTue, 3 Nov 2020 02:03:47 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 02A0C62B92\n\tfor ;\n\tTue, 3 Nov 2020 02:03:45 +0100 (CET)","from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 93616332\n\tfor ;\n\tTue, 3 Nov 2020 02:03:44 +0100 (CET)"],"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=\"VHLqXUrT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1604365424;\n\tbh=gyzZcjGFYh7iA9G7qvZ3Uz+XHnlBhEAWbsUbq4/3Ndo=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=VHLqXUrTp+qnLO96AKBJ0ruotrrixeky+smqq+MHpQfEpluF9NWYl1qUuKkvoJs1c\n\tkrniugZMC3rQ7EkJ09BdUDo7+aLg9iSiZth9CKU/T/4AIYSLMVp3flc/6U/QHaoTUL\n\tV2Et7jNmQNrUHWoTan1sFvc+MNPloOavkhrSXwOE=","From":"Laurent Pinchart ","To":"libcamera-devel@lists.libcamera.org","Date":"Tue, 3 Nov 2020 03:02:51 +0200","Message-Id":"<20201103010251.14689-5-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.27.0","In-Reply-To":"<20201103010251.14689-1-laurent.pinchart@ideasonboard.com>","References":"<20201103010251.14689-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Subject":"[libcamera-devel] [PATCH v3 4/4] libcamera: camera: Inherit from\n\tExtensible","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","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"Use the d-pointer infrastructure offered by the Extensible class to\nreplace the custom implementation.\n\nSigned-off-by: Laurent Pinchart \nReviewed-by: Niklas Söderlund \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 \n \n #include \n+#include \n #include \n #include \n #include \n@@ -70,8 +71,11 @@ protected:\n \tstd::vector config_;\n };\n \n-class Camera final : public Object, public std::enable_shared_from_this\n+class Camera final : public Object, public std::enable_shared_from_this,\n+\t\t public Extensible\n {\n+\tLIBCAMERA_DECLARE_PRIVATE(Camera)\n+\n public:\n \tstatic std::shared_ptr create(PipelineHandler *pipe,\n \t\t\t\t\t const std::string &id,\n@@ -107,9 +111,6 @@ private:\n \t const std::set &streams);\n \t~Camera();\n \n-\tclass Private;\n-\tstd::unique_ptr 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..dffbd6bd5a10 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 &streams);\n \t~Private();\n \n@@ -301,10 +303,11 @@ private:\n \tstd::atomic 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 &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::create(PipelineHandler *pipe,\n */\n const std::string &Camera::id() const\n {\n-\treturn p_->id_;\n+\tconst Private *const d = LIBCAMERA_D_PTR();\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 &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();\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> *buffers)\n {\n-\tint ret = p_->isAccessAllowed(Private::CameraConfigured);\n+\tPrivate *const d = LIBCAMERA_D_PTR();\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();\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();\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();\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();\n+\treturn d->pipe_->properties(this);\n }\n \n /**\n@@ -707,7 +721,8 @@ const ControlList &Camera::properties() const\n */\n const std::set &Camera::streams() const\n {\n-\treturn p_->streams_;\n+\tconst Private *const d = LIBCAMERA_D_PTR();\n+\treturn d->streams_;\n }\n \n /**\n@@ -728,15 +743,17 @@ const std::set &Camera::streams() const\n */\n std::unique_ptr 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();\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 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();\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 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();\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 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();\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();\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();\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","v3","4/4"]}