[{"id":569,"web_url":"https://patchwork.libcamera.org/comment/569/","msgid":"<20190124224056.ldd7wr6ubgzans66@uno.localdomain>","date":"2019-01-24T22:40:56","subject":"Re: [libcamera-devel] [PATCH 03/10] libcamera: camera: Associate\n\tcameras with their pipeline handler","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Thu, Jan 24, 2019 at 12:16:44PM +0200, Laurent Pinchart wrote:\n> From: Niklas Söderlund \n>\n> The PipelineHandler which creates a Camera is responsible for serving\n> any operation requested by the user. In order forward the public API\n> calls, the camera needs to store a reference to its pipeline handler.\n>\n> Signed-off-by: Niklas Söderlund \n> Signed-off-by: Laurent Pinchart \n> ---\n> Changes since v1:\n>\n> - Create pipeline handlers is shared pointers, make them inherit from\n> std::enable_shared_from_this<> and stored them in shared pointers.\n> ---\n> include/libcamera/camera.h | 8 ++++++--\n> include/libcamera/camera_manager.h | 2 +-\n> src/libcamera/camera.cpp | 16 ++++++++++------\n> src/libcamera/camera_manager.cpp | 17 +++++++++--------\n> src/libcamera/include/pipeline_handler.h | 9 +++++----\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-\n> src/libcamera/pipeline/uvcvideo.cpp | 2 +-\n> src/libcamera/pipeline/vimc.cpp | 9 +--------\n> src/libcamera/pipeline_handler.cpp | 10 ++++++++++\n> 9 files changed, 44 insertions(+), 31 deletions(-)\n>\n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index 2ea1a6883311..efafb9e28c56 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -12,10 +12,13 @@\n>\n> namespace libcamera {\n>\n> +class PipelineHandler;\n> +\n> class Camera final\n> {\n> public:\n> -\tstatic std::shared_ptr create(const std::string &name);\n> +\tstatic std::shared_ptr create(PipelineHandler *pipe,\n> +\t\t\t\t\t const std::string &name);\n>\n> \tCamera(const Camera &) = delete;\n> \tvoid operator=(const Camera &) = delete;\n> @@ -23,9 +26,10 @@ public:\n> \tconst std::string &name() const;\n>\n> private:\n> -\texplicit Camera(const std::string &name);\n> +\tCamera(PipelineHandler *pipe, const std::string &name);\n> \t~Camera();\n>\n> +\tstd::shared_ptr pipe_;\n> \tstd::string name_;\n> };\n>\n> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h\n> index 9ade29d76692..45e72df0ef65 100644\n> --- a/include/libcamera/camera_manager.h\n> +++ b/include/libcamera/camera_manager.h\n> @@ -41,7 +41,7 @@ private:\n> \t~CameraManager();\n>\n> \tstd::unique_ptr enumerator_;\n> -\tstd::vector pipes_;\n> +\tstd::vector> pipes_;\n> \tstd::vector> cameras_;\n>\n> \tstd::unique_ptr dispatcher_;\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index acf912bee95c..3a531c7e4d8f 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -8,6 +8,7 @@\n> #include \n>\n> #include \"log.h\"\n> +#include \"pipeline_handler.h\"\n>\n> /**\n> * \\file camera.h\n> @@ -52,17 +53,20 @@ namespace libcamera {\n> /**\n> * \\brief Create a camera instance\n> * \\param[in] name The name of the camera device\n> + * \\param[in] pipe The pipeline handler responsible for the camera device\n> *\n> * The caller is responsible for guaranteeing unicity of the camera name.\n> *\n> * \\return A shared pointer to the newly created camera object\n> */\n> -std::shared_ptr Camera::create(const std::string &name)\n> +std::shared_ptr Camera::create(PipelineHandler *pipe,\n> +\t\t\t\t const std::string &name)\n> {\n> \tstruct Allocator : std::allocator {\n> -\t\tvoid construct(void *p, const std::string &name)\n> +\t\tvoid construct(void *p, PipelineHandler *pipe,\n> +\t\t\t const std::string &name)\n> \t\t{\n> -\t\t\t::new(p) Camera(name);\n> +\t\t\t::new(p) Camera(pipe, name);\n> \t\t}\n> \t\tvoid destroy(Camera *p)\n> \t\t{\n> @@ -70,7 +74,7 @@ std::shared_ptr Camera::create(const std::string &name)\n> \t\t}\n> \t};\n>\n> -\treturn std::allocate_shared(Allocator(), name);\n> +\treturn std::allocate_shared(Allocator(), pipe, name);\n> }\n>\n> /**\n> @@ -83,8 +87,8 @@ const std::string &Camera::name() const\n> \treturn name_;\n> }\n>\n> -Camera::Camera(const std::string &name)\n> -\t: name_(name)\n> +Camera::Camera(PipelineHandler *pipe, const std::string &name)\n> +\t: pipe_(pipe->shared_from_this()), name_(name)\n\nVery clever design. Am I wrong or it only works if the pipeline\nhandler is already managed by a shared_ptr<> ? (but that should be the\ncase, right?).\n\nAlso, are we copying it? Doesn't it increase the reference counting?\n\n\n> {\n> }\n>\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index 14410d4dcda7..3eccf20c4ce9 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -98,16 +98,14 @@ int CameraManager::start()\n> \t\t * all pipelines it can provide.\n> \t\t */\n> \t\twhile (1) {\n> -\t\t\tPipelineHandler *pipe = factory->create(this);\n> -\t\t\tif (!pipe->match(enumerator_.get())) {\n> -\t\t\t\tdelete pipe;\n> +\t\t\tstd::shared_ptr pipe = factory->create(this);\n> +\t\t\tif (!pipe->match(enumerator_.get()))\n> \t\t\t\tbreak;\n> -\t\t\t}\n>\n> \t\t\tLOG(Camera, Debug)\n> \t\t\t\t<< \"Pipeline handler \\\"\" << factory->name()\n> \t\t\t\t<< \"\\\" matched\";\n> -\t\t\tpipes_.push_back(pipe);\n> +\t\t\tpipes_.push_back(std::move(pipe));\n> \t\t}\n> \t}\n>\n> @@ -130,10 +128,13 @@ void CameraManager::stop()\n> {\n> \t/* TODO: unregister hot-plug callback here */\n>\n> -\tfor (PipelineHandler *pipe : pipes_)\n> -\t\tdelete pipe;\n> -\n> +\t/*\n> +\t * Release all references to cameras and pipeline handlers to ensure\n> +\t * they all get destroyed before the device enumerator deletes the\n> +\t * media devices.\n> +\t */\n> \tpipes_.clear();\n> +\tcameras_.clear();\n>\n> \tenumerator_.reset(nullptr);\n> }\n> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> index 1da6dc758ca6..e1d6369eb0c4 100644\n> --- a/src/libcamera/include/pipeline_handler.h\n> +++ b/src/libcamera/include/pipeline_handler.h\n> @@ -8,6 +8,7 @@\n> #define __LIBCAMERA_PIPELINE_HANDLER_H__\n>\n> #include \n> +#include \n> #include \n> #include \n>\n> @@ -16,7 +17,7 @@ namespace libcamera {\n> class CameraManager;\n> class DeviceEnumerator;\n>\n> -class PipelineHandler\n> +class PipelineHandler : public std::enable_shared_from_this\n> {\n> public:\n> \tPipelineHandler(CameraManager *manager);\n> @@ -34,7 +35,7 @@ public:\n> \tPipelineHandlerFactory(const char *name);\n> \tvirtual ~PipelineHandlerFactory() { };\n>\n> -\tvirtual PipelineHandler *create(CameraManager *manager) = 0;\n> +\tvirtual std::shared_ptr create(CameraManager *manager) = 0;\n>\n> \tconst std::string &name() const { return name_; }\n>\n> @@ -50,9 +51,9 @@ class handler##Factory final : public PipelineHandlerFactory\t\t\\\n> {\t\t\t\t\t\t\t\t\t\\\n> public:\t\t\t\t\t\t\t\t\t\\\n> \thandler##Factory() : PipelineHandlerFactory(#handler) {}\t\\\n> -\tPipelineHandler *create(CameraManager *manager) \t\t\\\n> +\tstd::shared_ptr create(CameraManager *manager)\t\\\n> \t{\t\t\t\t\t\t\t\t\\\n> -\t\treturn new handler(manager);\t\t\t\t\\\n> +\t\treturn std::make_shared(manager);\t\t\\\n> \t}\t\t\t\t\t\t\t\t\\\n> };\t\t\t\t\t\t\t\t\t\\\n> static handler##Factory global_##handler##Factory;\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 589b3078f301..13ff7da4c99e 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -171,7 +171,7 @@ void PipelineHandlerIPU3::registerCameras()\n> \t\t\tcontinue;\n>\n> \t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n> -\t\tstd::shared_ptr camera = Camera::create(cameraName);\n> +\t\tstd::shared_ptr camera = Camera::create(this, cameraName);\n> \t\tmanager_->addCamera(std::move(camera));\n>\n> \t\tLOG(IPU3, Info)\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index 92b23da73901..3ebc074093ab 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -48,7 +48,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n>\n> \tdev_->acquire();\n>\n> -\tstd::shared_ptr camera = Camera::create(dev_->model());\n> +\tstd::shared_ptr camera = Camera::create(this, dev_->model());\n> \tmanager_->addCamera(std::move(camera));\n>\n> \treturn true;\n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index f12d007cd956..68bfe9b12ab6 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -57,14 +57,7 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)\n>\n> \tdev_->acquire();\n>\n> -\t/*\n> -\t * NOTE: A more complete Camera implementation could\n> -\t * be passed the MediaDevice(s) it controls here or\n> -\t * a reference to the PipelineHandler. Which method\n> -\t * will be chosen depends on how the Camera\n> -\t * object is modeled.\n> -\t */\n> -\tstd::shared_ptr camera = Camera::create(\"Dummy VIMC Camera\");\n> +\tstd::shared_ptr camera = Camera::create(this, \"Dummy VIMC Camera\");\n> \tmanager_->addCamera(std::move(camera));\n>\n> \treturn true;\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 45788487b5c0..3850ea8fadb5 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -32,11 +32,21 @@ LOG_DEFINE_CATEGORY(Pipeline)\n> *\n> * The PipelineHandler matches the media devices provided by a DeviceEnumerator\n> * with the pipelines it supports and creates corresponding Camera devices.\n> + *\n> + * Pipeline handler instances are reference-counted through std::shared_ptr<>.\n> + * They implement std::enable_shared_from_this<> in order to create new\n> + * std::shared_ptr<> in code paths originating from member functions of the\n> + * PipelineHandler class where only the 'this' pointer is available.\n> */\n>\n> /**\n> * \\brief Construct a PipelineHandler instance\n> * \\param[in] manager The camera manager\n> + *\n> + * In order to honour the std::enable_shared_from_this<> contract,\n> + * PipelineHandler instances shall never be constructed manually, but always\n> + * through the PipelineHandlerFactory::create() method implemented by the\n> + * respective factories.\n> */\n> PipelineHandler::PipelineHandler(CameraManager *manager)\n> \t: manager_(manager)\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net\n\t[217.70.183.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F241D60C82\n\tfor ;\n\tThu, 24 Jan 2019 23:40:43 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay3-d.mail.gandi.net (Postfix) with ESMTPSA id 651A160003;\n\tThu, 24 Jan 2019 22:40:43 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Thu, 24 Jan 2019 23:40:56 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190124224056.ldd7wr6ubgzans66@uno.localdomain>","References":"<20190124101651.9993-1-laurent.pinchart@ideasonboard.com>\n\t<20190124101651.9993-4-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"lglxikk5es74oy3t\"","Content-Disposition":"inline","In-Reply-To":"<20190124101651.9993-4-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 03/10] libcamera: camera: Associate\n\tcameras with their pipeline handler","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Thu, 24 Jan 2019 22:40:44 -0000"}},{"id":572,"web_url":"https://patchwork.libcamera.org/comment/572/","msgid":"<20190124233359.GA4399@pendragon.ideasonboard.com>","date":"2019-01-24T23:33:59","subject":"Re: [libcamera-devel] [PATCH 03/10] libcamera: camera: Associate\n\tcameras with their pipeline handler","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Thu, Jan 24, 2019 at 11:40:56PM +0100, Jacopo Mondi wrote:\n> On Thu, Jan 24, 2019 at 12:16:44PM +0200, Laurent Pinchart wrote:\n> > From: Niklas Söderlund \n> >\n> > The PipelineHandler which creates a Camera is responsible for serving\n> > any operation requested by the user. In order forward the public API\n> > calls, the camera needs to store a reference to its pipeline handler.\n> >\n> > Signed-off-by: Niklas Söderlund \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > Changes since v1:\n> >\n> > - Create pipeline handlers is shared pointers, make them inherit from\n> > std::enable_shared_from_this<> and stored them in shared pointers.\n> > ---\n> > include/libcamera/camera.h | 8 ++++++--\n> > include/libcamera/camera_manager.h | 2 +-\n> > src/libcamera/camera.cpp | 16 ++++++++++------\n> > src/libcamera/camera_manager.cpp | 17 +++++++++--------\n> > src/libcamera/include/pipeline_handler.h | 9 +++++----\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +-\n> > src/libcamera/pipeline/uvcvideo.cpp | 2 +-\n> > src/libcamera/pipeline/vimc.cpp | 9 +--------\n> > src/libcamera/pipeline_handler.cpp | 10 ++++++++++\n> > 9 files changed, 44 insertions(+), 31 deletions(-)\n> >\n> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > index 2ea1a6883311..efafb9e28c56 100644\n> > --- a/include/libcamera/camera.h\n> > +++ b/include/libcamera/camera.h\n> > @@ -12,10 +12,13 @@\n> >\n> > namespace libcamera {\n> >\n> > +class PipelineHandler;\n> > +\n> > class Camera final\n> > {\n> > public:\n> > -\tstatic std::shared_ptr create(const std::string &name);\n> > +\tstatic std::shared_ptr create(PipelineHandler *pipe,\n> > +\t\t\t\t\t const std::string &name);\n> >\n> > \tCamera(const Camera &) = delete;\n> > \tvoid operator=(const Camera &) = delete;\n> > @@ -23,9 +26,10 @@ public:\n> > \tconst std::string &name() const;\n> >\n> > private:\n> > -\texplicit Camera(const std::string &name);\n> > +\tCamera(PipelineHandler *pipe, const std::string &name);\n> > \t~Camera();\n> >\n> > +\tstd::shared_ptr pipe_;\n> > \tstd::string name_;\n> > };\n> >\n> > diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h\n> > index 9ade29d76692..45e72df0ef65 100644\n> > --- a/include/libcamera/camera_manager.h\n> > +++ b/include/libcamera/camera_manager.h\n> > @@ -41,7 +41,7 @@ private:\n> > \t~CameraManager();\n> >\n> > \tstd::unique_ptr enumerator_;\n> > -\tstd::vector pipes_;\n> > +\tstd::vector> pipes_;\n> > \tstd::vector> cameras_;\n> >\n> > \tstd::unique_ptr dispatcher_;\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index acf912bee95c..3a531c7e4d8f 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -8,6 +8,7 @@\n> > #include \n> >\n> > #include \"log.h\"\n> > +#include \"pipeline_handler.h\"\n> >\n> > /**\n> > * \\file camera.h\n> > @@ -52,17 +53,20 @@ namespace libcamera {\n> > /**\n> > * \\brief Create a camera instance\n> > * \\param[in] name The name of the camera device\n> > + * \\param[in] pipe The pipeline handler responsible for the camera device\n> > *\n> > * The caller is responsible for guaranteeing unicity of the camera name.\n> > *\n> > * \\return A shared pointer to the newly created camera object\n> > */\n> > -std::shared_ptr Camera::create(const std::string &name)\n> > +std::shared_ptr Camera::create(PipelineHandler *pipe,\n> > +\t\t\t\t const std::string &name)\n> > {\n> > \tstruct Allocator : std::allocator {\n> > -\t\tvoid construct(void *p, const std::string &name)\n> > +\t\tvoid construct(void *p, PipelineHandler *pipe,\n> > +\t\t\t const std::string &name)\n> > \t\t{\n> > -\t\t\t::new(p) Camera(name);\n> > +\t\t\t::new(p) Camera(pipe, name);\n> > \t\t}\n> > \t\tvoid destroy(Camera *p)\n> > \t\t{\n> > @@ -70,7 +74,7 @@ std::shared_ptr Camera::create(const std::string &name)\n> > \t\t}\n> > \t};\n> >\n> > -\treturn std::allocate_shared(Allocator(), name);\n> > +\treturn std::allocate_shared(Allocator(), pipe, name);\n> > }\n> >\n> > /**\n> > @@ -83,8 +87,8 @@ const std::string &Camera::name() const\n> > \treturn name_;\n> > }\n> >\n> > -Camera::Camera(const std::string &name)\n> > -\t: name_(name)\n> > +Camera::Camera(PipelineHandler *pipe, const std::string &name)\n> > +\t: pipe_(pipe->shared_from_this()), name_(name)\n> \n> Very clever design. Am I wrong or it only works if the pipeline\n> handler is already managed by a shared_ptr<> ? (but that should be the\n> case, right?).\n\nThat's correct, shared_from_this can only be used if the object instance\nwas created with make_shared or allocate_shared. Otherwise the results\nare undefined in C++11, and in more recent standards an exception is\nraised. This shouldn't be a problem as all pipeline handler instances\nare created by their respective factory, using make_shared.\n\n> Also, are we copying it? Doesn't it increase the reference counting?\n\nWe create a new reference, and that's by design, as Camera holds a\nreference to the pipeline handler.\n\n> > {\n> > }\n> >\n> > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > index 14410d4dcda7..3eccf20c4ce9 100644\n> > --- a/src/libcamera/camera_manager.cpp\n> > +++ b/src/libcamera/camera_manager.cpp\n> > @@ -98,16 +98,14 @@ int CameraManager::start()\n> > \t\t * all pipelines it can provide.\n> > \t\t */\n> > \t\twhile (1) {\n> > -\t\t\tPipelineHandler *pipe = factory->create(this);\n> > -\t\t\tif (!pipe->match(enumerator_.get())) {\n> > -\t\t\t\tdelete pipe;\n> > +\t\t\tstd::shared_ptr pipe = factory->create(this);\n> > +\t\t\tif (!pipe->match(enumerator_.get()))\n> > \t\t\t\tbreak;\n> > -\t\t\t}\n> >\n> > \t\t\tLOG(Camera, Debug)\n> > \t\t\t\t<< \"Pipeline handler \\\"\" << factory->name()\n> > \t\t\t\t<< \"\\\" matched\";\n> > -\t\t\tpipes_.push_back(pipe);\n> > +\t\t\tpipes_.push_back(std::move(pipe));\n> > \t\t}\n> > \t}\n> >\n> > @@ -130,10 +128,13 @@ void CameraManager::stop()\n> > {\n> > \t/* TODO: unregister hot-plug callback here */\n> >\n> > -\tfor (PipelineHandler *pipe : pipes_)\n> > -\t\tdelete pipe;\n> > -\n> > +\t/*\n> > +\t * Release all references to cameras and pipeline handlers to ensure\n> > +\t * they all get destroyed before the device enumerator deletes the\n> > +\t * media devices.\n> > +\t */\n> > \tpipes_.clear();\n> > +\tcameras_.clear();\n> >\n> > \tenumerator_.reset(nullptr);\n> > }\n> > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> > index 1da6dc758ca6..e1d6369eb0c4 100644\n> > --- a/src/libcamera/include/pipeline_handler.h\n> > +++ b/src/libcamera/include/pipeline_handler.h\n> > @@ -8,6 +8,7 @@\n> > #define __LIBCAMERA_PIPELINE_HANDLER_H__\n> >\n> > #include \n> > +#include \n> > #include \n> > #include \n> >\n> > @@ -16,7 +17,7 @@ namespace libcamera {\n> > class CameraManager;\n> > class DeviceEnumerator;\n> >\n> > -class PipelineHandler\n> > +class PipelineHandler : public std::enable_shared_from_this\n> > {\n> > public:\n> > \tPipelineHandler(CameraManager *manager);\n> > @@ -34,7 +35,7 @@ public:\n> > \tPipelineHandlerFactory(const char *name);\n> > \tvirtual ~PipelineHandlerFactory() { };\n> >\n> > -\tvirtual PipelineHandler *create(CameraManager *manager) = 0;\n> > +\tvirtual std::shared_ptr create(CameraManager *manager) = 0;\n> >\n> > \tconst std::string &name() const { return name_; }\n> >\n> > @@ -50,9 +51,9 @@ class handler##Factory final : public PipelineHandlerFactory\t\t\\\n> > {\t\t\t\t\t\t\t\t\t\\\n> > public:\t\t\t\t\t\t\t\t\t\\\n> > \thandler##Factory() : PipelineHandlerFactory(#handler) {}\t\\\n> > -\tPipelineHandler *create(CameraManager *manager) \t\t\\\n> > +\tstd::shared_ptr create(CameraManager *manager)\t\\\n> > \t{\t\t\t\t\t\t\t\t\\\n> > -\t\treturn new handler(manager);\t\t\t\t\\\n> > +\t\treturn std::make_shared(manager);\t\t\\\n> > \t}\t\t\t\t\t\t\t\t\\\n> > };\t\t\t\t\t\t\t\t\t\\\n> > static handler##Factory global_##handler##Factory;\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 589b3078f301..13ff7da4c99e 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -171,7 +171,7 @@ void PipelineHandlerIPU3::registerCameras()\n> > \t\t\tcontinue;\n> >\n> > \t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n> > -\t\tstd::shared_ptr camera = Camera::create(cameraName);\n> > +\t\tstd::shared_ptr camera = Camera::create(this, cameraName);\n> > \t\tmanager_->addCamera(std::move(camera));\n> >\n> > \t\tLOG(IPU3, Info)\n> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> > index 92b23da73901..3ebc074093ab 100644\n> > --- a/src/libcamera/pipeline/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> > @@ -48,7 +48,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> >\n> > \tdev_->acquire();\n> >\n> > -\tstd::shared_ptr camera = Camera::create(dev_->model());\n> > +\tstd::shared_ptr camera = Camera::create(this, dev_->model());\n> > \tmanager_->addCamera(std::move(camera));\n> >\n> > \treturn true;\n> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> > index f12d007cd956..68bfe9b12ab6 100644\n> > --- a/src/libcamera/pipeline/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc.cpp\n> > @@ -57,14 +57,7 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)\n> >\n> > \tdev_->acquire();\n> >\n> > -\t/*\n> > -\t * NOTE: A more complete Camera implementation could\n> > -\t * be passed the MediaDevice(s) it controls here or\n> > -\t * a reference to the PipelineHandler. Which method\n> > -\t * will be chosen depends on how the Camera\n> > -\t * object is modeled.\n> > -\t */\n> > -\tstd::shared_ptr camera = Camera::create(\"Dummy VIMC Camera\");\n> > +\tstd::shared_ptr camera = Camera::create(this, \"Dummy VIMC Camera\");\n> > \tmanager_->addCamera(std::move(camera));\n> >\n> > \treturn true;\n> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > index 45788487b5c0..3850ea8fadb5 100644\n> > --- a/src/libcamera/pipeline_handler.cpp\n> > +++ b/src/libcamera/pipeline_handler.cpp\n> > @@ -32,11 +32,21 @@ LOG_DEFINE_CATEGORY(Pipeline)\n> > *\n> > * The PipelineHandler matches the media devices provided by a DeviceEnumerator\n> > * with the pipelines it supports and creates corresponding Camera devices.\n> > + *\n> > + * Pipeline handler instances are reference-counted through std::shared_ptr<>.\n> > + * They implement std::enable_shared_from_this<> in order to create new\n> > + * std::shared_ptr<> in code paths originating from member functions of the\n> > + * PipelineHandler class where only the 'this' pointer is available.\n> > */\n> >\n> > /**\n> > * \\brief Construct a PipelineHandler instance\n> > * \\param[in] manager The camera manager\n> > + *\n> > + * In order to honour the std::enable_shared_from_this<> contract,\n> > + * PipelineHandler instances shall never be constructed manually, but always\n> > + * through the PipelineHandlerFactory::create() method implemented by the\n> > + * respective factories.\n> > */\n> > PipelineHandler::PipelineHandler(CameraManager *manager)\n> > \t: manager_(manager)","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5A4C860C82\n\tfor ;\n\tFri, 25 Jan 2019 00:34:00 +0100 (CET)","from pendragon.ideasonboard.com (unknown\n\t[IPv6:2a02:a03f:4499:2700:1060:1d4c:d6a:8e80])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D16432F6;\n\tFri, 25 Jan 2019 00:33:59 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548372839;\n\tbh=RcE7o+BavpMfHh2fiZEq2XhUe6xbD7kY1PmgSt8rG2M=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=njw8INGiyoaOBsP+e6wzzHTYaNfH0oLFzZzdiSUehQ4z4ohaelTkGLZTuqgqwKJTB\n\tGRmjjB5Psu8T/PS4Ehqg1L0VbET755pxiPTPkVYb9U9LhVEBMVxTZraYARb2yINs+9\n\tdKijCNBNvD4se+9aMz8FHVg9qnuqk+hjTyKLmVGY=","Date":"Fri, 25 Jan 2019 01:33:59 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190124233359.GA4399@pendragon.ideasonboard.com>","References":"<20190124101651.9993-1-laurent.pinchart@ideasonboard.com>\n\t<20190124101651.9993-4-laurent.pinchart@ideasonboard.com>\n\t<20190124224056.ldd7wr6ubgzans66@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190124224056.ldd7wr6ubgzans66@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 03/10] libcamera: camera: Associate\n\tcameras with their pipeline handler","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Thu, 24 Jan 2019 23:34:00 -0000"}}]