[{"id":571,"web_url":"https://patchwork.libcamera.org/comment/571/","msgid":"<20190124232449.6t6q6cpmv6udfkqo@uno.localdomain>","date":"2019-01-24T23:24:49","subject":"Re: [libcamera-devel] [PATCH 08/10] libcamera: pipeline_handler:\n\tAdd camera disconnection support","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent, Niklas\n a quite small things\n\nOn Thu, Jan 24, 2019 at 12:16:49PM +0200, Laurent Pinchart wrote:\n> From: Niklas Söderlund \n>\n> Pipeline handlers are responsible for creating camera instances, but\n> also for destroying them when devices are unplugged. As camera objects\n> are reference-counted this isn't a straightforward operation and\n> involves the camera manager and camera object itself. Add two helper\n> methods in the PipelineHandler base class to register a camera and to\n> register a media device with the pipeline handler.\n>\n> When registering a camera, the registerCamera() helper method will add\n> it to the camera manager. When registering a media device, the\n> registerMediaDevice() helper method will listen to device disconnection\n> events, and disconnect all cameras created by the pipeline handler as a\n> response.\n>\n> Under the hood the PipelineHandler class needs to keep track of\n> registered cameras in order to handle disconnection. They can't be\n> stored as shared pointers as this would create a circular dependency\n> (the Camera class owns a shared pointer to the pipeline handler). Store\n> them as weak pointers instead. This is safe as a reference to the camera\n> is stored in the camera manager, and doesn't get removed until the\n> camera is unregistered from the manager by the PipelineHandler.\n>\n> Signed-off-by: Niklas Söderlund \n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/include/pipeline_handler.h | 10 ++++\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 3 +-\n> src/libcamera/pipeline/uvcvideo.cpp | 3 +-\n> src/libcamera/pipeline/vimc.cpp | 3 +-\n> src/libcamera/pipeline_handler.cpp | 71 ++++++++++++++++++++++++\n> 5 files changed, 84 insertions(+), 6 deletions(-)\n>\n> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> index e1d6369eb0c4..804cce4807ee 100644\n> --- a/src/libcamera/include/pipeline_handler.h\n> +++ b/src/libcamera/include/pipeline_handler.h\n> @@ -16,6 +16,7 @@ namespace libcamera {\n>\n> class CameraManager;\n> class DeviceEnumerator;\n> +class MediaDevice;\n>\n> class PipelineHandler : public std::enable_shared_from_this\n> {\n> @@ -27,6 +28,15 @@ public:\n>\n> protected:\n> \tCameraManager *manager_;\n> +\n> +\tvoid registerCamera(std::shared_ptr camera);\n> +\tvoid hotplugMediaDevice(MediaDevice *media);\n> +\n> +private:\n> +\tvirtual void disconnect();\n> +\tvoid mediaDeviceDisconnected(MediaDevice *media);\n> +\n> +\tstd::vector> cameras_;\n> };\n>\n> class PipelineHandlerFactory\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 9831f74fe53f..3161e71420ed 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -9,7 +9,6 @@\n> #include \n>\n> #include \n> -#include \n>\n> #include \"device_enumerator.h\"\n> #include \"log.h\"\n> @@ -169,7 +168,7 @@ void PipelineHandlerIPU3::registerCameras()\n>\n> \t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n> \t\tstd::shared_ptr camera = Camera::create(this, cameraName);\n> -\t\tmanager_->addCamera(std::move(camera));\n> +\t\tregisterCamera(std::move(camera));\n>\n> \t\tLOG(IPU3, Info)\n> \t\t\t<< \"Registered Camera[\" << numCameras << \"] \\\"\"\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index 73bad6714bb4..c8f1bf553bfe 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -6,7 +6,6 @@\n> */\n>\n> #include \n> -#include \n>\n> #include \"device_enumerator.h\"\n> #include \"media_device.h\"\n> @@ -49,7 +48,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> \tdev_->acquire();\n>\n> \tstd::shared_ptr camera = Camera::create(this, dev_->model());\n> -\tmanager_->addCamera(std::move(camera));\n> +\tregisterCamera(std::move(camera));\n>\n> \treturn true;\n> }\n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index 521b20d3a120..b714a07688e9 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -6,7 +6,6 @@\n> */\n>\n> #include \n> -#include \n>\n> #include \"device_enumerator.h\"\n> #include \"media_device.h\"\n> @@ -58,7 +57,7 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)\n> \tdev_->acquire();\n>\n> \tstd::shared_ptr camera = Camera::create(this, \"Dummy VIMC Camera\");\n> -\tmanager_->addCamera(std::move(camera));\n> +\tregisterCamera(std::move(camera));\n>\n> \treturn true;\n> }\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 3850ea8fadb5..f0aa2f8022c2 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -5,7 +5,11 @@\n> * pipeline_handler.cpp - Pipeline handler infrastructure\n> */\n>\n> +#include \n> +#include \n> +\n> #include \"log.h\"\n> +#include \"media_device.h\"\n> #include \"pipeline_handler.h\"\n>\n> /**\n> @@ -97,6 +101,73 @@ PipelineHandler::~PipelineHandler()\n> * constant for the whole lifetime of the pipeline handler.\n> */\n>\n> +/**\n> + * \\brief Register a camera to the camera manager and pipeline handler\n> + * \\param[in] camera The camera to be added\n> + *\n> + * This function is called by pipeline handlers to register the cameras they\n> + * handle with the camera manager.\n> + */\n> +void PipelineHandler::registerCamera(std::shared_ptr camera)\n> +{\n> +\tcameras_.push_back(camera);\n> +\tmanager_->addCamera(std::move(camera));\n> +}\n> +\n> +/**\n> + * \\brief Handle hotplugging of a media device\n\n\"Handle\" here misleaded me. What about \"Enable\" ?\n\n> + * \\param[in] media The media device\n> + *\n> + * This function enables hotplug handling, and especially hot-unplug handling,\n> + * of the \\a media device. It shall be called by pipeline handlers for all the\n> + * media devices that can be disconnected.\n> + *\n> + * When a media device passed to this function is later unplugged, the pipeline\n> + * handler gets notified and automatically disconnects all the cameras it has\n> + * registered without requiring any manual intervention.\n> + */\n> +void PipelineHandler::hotplugMediaDevice(MediaDevice *media)\n> +{\n> +\tmedia->disconnected.connect(this, &PipelineHandler::mediaDeviceDisconnected);\n> +}\n> +\n> +/**\n> + * \\brief Device disconnection handler\n> + *\n> + * This virtual function is called to notify the pipeline handler that the\n> + * device it handles has been disconnected. It notifies all cameras created by\n> + * the pipeline handler that they have been disconnected, and unregisters them\n> + * from the camera manager.\n> + *\n> + * The function can be overloaded by pipeline handlers to perform custom\n> + * operations at disconnection time. Any overloaded version shall call the\n> + * PipelineHandler::disconnect() base function for proper hot-unplug operation.\n> + */\n> +void PipelineHandler::disconnect()\n> +{\n> +\tfor (std::weak_ptr ptr : cameras_) {\n> +\t\tstd::shared_ptr camera = ptr.lock();\n> +\t\tif (!camera)\n> +\t\t\tcontinue;\n> +\n\nI wonder if sub-classes need a disconnectCamera(*camera) to perform\nper-camera operations before disconnect...\n\n> +\t\tcamera->disconnect();\n> +\t\tmanager_->removeCamera(camera.get());\n> +\t}\n> +\n> +\tcameras_.clear();\n> +}\n> +\n> +/**\n> + * \\brief Slot for the MediaDevice disconnected signal\n> + */\n> +void PipelineHandler::mediaDeviceDisconnected(MediaDevice *media)\n> +{\n> +\tif (cameras_.empty())\n> +\t\treturn;\n> +\n> +\tdisconnect();\n> +}\n> +\n> /**\n> * \\class PipelineHandlerFactory\n> * \\brief Registration of PipelineHandler classes and creation of instances\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 relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2C47460C82\n\tfor ;\n\tFri, 25 Jan 2019 00:24:37 +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 relay4-d.mail.gandi.net (Postfix) with ESMTPSA id B085BE0002;\n\tThu, 24 Jan 2019 23:24:36 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Fri, 25 Jan 2019 00:24:49 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190124232449.6t6q6cpmv6udfkqo@uno.localdomain>","References":"<20190124101651.9993-1-laurent.pinchart@ideasonboard.com>\n\t<20190124101651.9993-9-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"uy6rwppvzptw3hc5\"","Content-Disposition":"inline","In-Reply-To":"<20190124101651.9993-9-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 08/10] libcamera: pipeline_handler:\n\tAdd camera disconnection support","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:24:37 -0000"}},{"id":574,"web_url":"https://patchwork.libcamera.org/comment/574/","msgid":"<20190124235903.GC4399@pendragon.ideasonboard.com>","date":"2019-01-24T23:59:03","subject":"Re: [libcamera-devel] [PATCH 08/10] libcamera: pipeline_handler:\n\tAdd camera disconnection support","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Fri, Jan 25, 2019 at 12:24:49AM +0100, Jacopo Mondi wrote:\n> On Thu, Jan 24, 2019 at 12:16:49PM +0200, Laurent Pinchart wrote:\n> > From: Niklas Söderlund \n> >\n> > Pipeline handlers are responsible for creating camera instances, but\n> > also for destroying them when devices are unplugged. As camera objects\n> > are reference-counted this isn't a straightforward operation and\n> > involves the camera manager and camera object itself. Add two helper\n> > methods in the PipelineHandler base class to register a camera and to\n> > register a media device with the pipeline handler.\n> >\n> > When registering a camera, the registerCamera() helper method will add\n> > it to the camera manager. When registering a media device, the\n> > registerMediaDevice() helper method will listen to device disconnection\n> > events, and disconnect all cameras created by the pipeline handler as a\n> > response.\n> >\n> > Under the hood the PipelineHandler class needs to keep track of\n> > registered cameras in order to handle disconnection. They can't be\n> > stored as shared pointers as this would create a circular dependency\n> > (the Camera class owns a shared pointer to the pipeline handler). Store\n> > them as weak pointers instead. This is safe as a reference to the camera\n> > is stored in the camera manager, and doesn't get removed until the\n> > camera is unregistered from the manager by the PipelineHandler.\n> >\n> > Signed-off-by: Niklas Söderlund \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/libcamera/include/pipeline_handler.h | 10 ++++\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 3 +-\n> > src/libcamera/pipeline/uvcvideo.cpp | 3 +-\n> > src/libcamera/pipeline/vimc.cpp | 3 +-\n> > src/libcamera/pipeline_handler.cpp | 71 ++++++++++++++++++++++++\n> > 5 files changed, 84 insertions(+), 6 deletions(-)\n> >\n> > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> > index e1d6369eb0c4..804cce4807ee 100644\n> > --- a/src/libcamera/include/pipeline_handler.h\n> > +++ b/src/libcamera/include/pipeline_handler.h\n> > @@ -16,6 +16,7 @@ namespace libcamera {\n> >\n> > class CameraManager;\n> > class DeviceEnumerator;\n> > +class MediaDevice;\n> >\n> > class PipelineHandler : public std::enable_shared_from_this\n> > {\n> > @@ -27,6 +28,15 @@ public:\n> >\n> > protected:\n> > \tCameraManager *manager_;\n> > +\n> > +\tvoid registerCamera(std::shared_ptr camera);\n> > +\tvoid hotplugMediaDevice(MediaDevice *media);\n> > +\n> > +private:\n> > +\tvirtual void disconnect();\n> > +\tvoid mediaDeviceDisconnected(MediaDevice *media);\n> > +\n> > +\tstd::vector> cameras_;\n> > };\n> >\n> > class PipelineHandlerFactory\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 9831f74fe53f..3161e71420ed 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -9,7 +9,6 @@\n> > #include \n> >\n> > #include \n> > -#include \n> >\n> > #include \"device_enumerator.h\"\n> > #include \"log.h\"\n> > @@ -169,7 +168,7 @@ void PipelineHandlerIPU3::registerCameras()\n> >\n> > \t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n> > \t\tstd::shared_ptr camera = Camera::create(this, cameraName);\n> > -\t\tmanager_->addCamera(std::move(camera));\n> > +\t\tregisterCamera(std::move(camera));\n> >\n> > \t\tLOG(IPU3, Info)\n> > \t\t\t<< \"Registered Camera[\" << numCameras << \"] \\\"\"\n> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> > index 73bad6714bb4..c8f1bf553bfe 100644\n> > --- a/src/libcamera/pipeline/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> > @@ -6,7 +6,6 @@\n> > */\n> >\n> > #include \n> > -#include \n> >\n> > #include \"device_enumerator.h\"\n> > #include \"media_device.h\"\n> > @@ -49,7 +48,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> > \tdev_->acquire();\n> >\n> > \tstd::shared_ptr camera = Camera::create(this, dev_->model());\n> > -\tmanager_->addCamera(std::move(camera));\n> > +\tregisterCamera(std::move(camera));\n> >\n> > \treturn true;\n> > }\n> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> > index 521b20d3a120..b714a07688e9 100644\n> > --- a/src/libcamera/pipeline/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc.cpp\n> > @@ -6,7 +6,6 @@\n> > */\n> >\n> > #include \n> > -#include \n> >\n> > #include \"device_enumerator.h\"\n> > #include \"media_device.h\"\n> > @@ -58,7 +57,7 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)\n> > \tdev_->acquire();\n> >\n> > \tstd::shared_ptr camera = Camera::create(this, \"Dummy VIMC Camera\");\n> > -\tmanager_->addCamera(std::move(camera));\n> > +\tregisterCamera(std::move(camera));\n> >\n> > \treturn true;\n> > }\n> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > index 3850ea8fadb5..f0aa2f8022c2 100644\n> > --- a/src/libcamera/pipeline_handler.cpp\n> > +++ b/src/libcamera/pipeline_handler.cpp\n> > @@ -5,7 +5,11 @@\n> > * pipeline_handler.cpp - Pipeline handler infrastructure\n> > */\n> >\n> > +#include \n> > +#include \n> > +\n> > #include \"log.h\"\n> > +#include \"media_device.h\"\n> > #include \"pipeline_handler.h\"\n> >\n> > /**\n> > @@ -97,6 +101,73 @@ PipelineHandler::~PipelineHandler()\n> > * constant for the whole lifetime of the pipeline handler.\n> > */\n> >\n> > +/**\n> > + * \\brief Register a camera to the camera manager and pipeline handler\n> > + * \\param[in] camera The camera to be added\n> > + *\n> > + * This function is called by pipeline handlers to register the cameras they\n> > + * handle with the camera manager.\n> > + */\n> > +void PipelineHandler::registerCamera(std::shared_ptr camera)\n> > +{\n> > +\tcameras_.push_back(camera);\n> > +\tmanager_->addCamera(std::move(camera));\n> > +}\n> > +\n> > +/**\n> > + * \\brief Handle hotplugging of a media device\n> \n> \"Handle\" here misleaded me. What about \"Enable\" ?\n\nI'll change this to \"Enable hotplug handling for a media device\".\n\n> > + * \\param[in] media The media device\n> > + *\n> > + * This function enables hotplug handling, and especially hot-unplug handling,\n> > + * of the \\a media device. It shall be called by pipeline handlers for all the\n> > + * media devices that can be disconnected.\n> > + *\n> > + * When a media device passed to this function is later unplugged, the pipeline\n> > + * handler gets notified and automatically disconnects all the cameras it has\n> > + * registered without requiring any manual intervention.\n> > + */\n> > +void PipelineHandler::hotplugMediaDevice(MediaDevice *media)\n> > +{\n> > +\tmedia->disconnected.connect(this, &PipelineHandler::mediaDeviceDisconnected);\n> > +}\n> > +\n> > +/**\n> > + * \\brief Device disconnection handler\n> > + *\n> > + * This virtual function is called to notify the pipeline handler that the\n> > + * device it handles has been disconnected. It notifies all cameras created by\n> > + * the pipeline handler that they have been disconnected, and unregisters them\n> > + * from the camera manager.\n> > + *\n> > + * The function can be overloaded by pipeline handlers to perform custom\n> > + * operations at disconnection time. Any overloaded version shall call the\n> > + * PipelineHandler::disconnect() base function for proper hot-unplug operation.\n> > + */\n> > +void PipelineHandler::disconnect()\n> > +{\n> > +\tfor (std::weak_ptr ptr : cameras_) {\n> > +\t\tstd::shared_ptr camera = ptr.lock();\n> > +\t\tif (!camera)\n> > +\t\t\tcontinue;\n> > +\n> \n> I wonder if sub-classes need a disconnectCamera(*camera) to perform\n> per-camera operations before disconnect...\n\nIt's a good question, and I don't have answers yet I'm afraid. I think\nwe should add that later if it turns out to be needed. Note that\nsubclasses could always connect a slot to the camera disconnected\nsignal, but that would probably be a bit cumbersome, a virtual function\nwould likely be better.\n\n> > +\t\tcamera->disconnect();\n> > +\t\tmanager_->removeCamera(camera.get());\n> > +\t}\n> > +\n> > +\tcameras_.clear();\n> > +}\n> > +\n> > +/**\n> > + * \\brief Slot for the MediaDevice disconnected signal\n> > + */\n> > +void PipelineHandler::mediaDeviceDisconnected(MediaDevice *media)\n> > +{\n> > +\tif (cameras_.empty())\n> > +\t\treturn;\n> > +\n> > +\tdisconnect();\n> > +}\n> > +\n> > /**\n> > * \\class PipelineHandlerFactory\n> > * \\brief Registration of PipelineHandler classes and creation of instances","headers":{"Return-Path":"","Received":["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 E17AA60C82\n\tfor ;\n\tFri, 25 Jan 2019 00:59:04 +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 555642F6;\n\tFri, 25 Jan 2019 00:59:04 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548374344;\n\tbh=LIJDmM/GI/AKL1lmLh6gN2RyIiDmIb3C0NM/sbQHhM0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hetGxlHJ1rabb9l2RtyNZmw7m+hMc1HaLt1KtJvtLlQ3RKZn1p7KH6NRq31+TmYjQ\n\tKIIkY3V6B8ivB1SAzyydgZUjoCAe/fINjY3KW3Hhzg1FvI4jmk0gaSGdMlKEjcJmE3\n\tG00XbpPP7BUF+Lim1YY+NSb2qKkF1iJIurySBM0E=","Date":"Fri, 25 Jan 2019 01:59:03 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190124235903.GC4399@pendragon.ideasonboard.com>","References":"<20190124101651.9993-1-laurent.pinchart@ideasonboard.com>\n\t<20190124101651.9993-9-laurent.pinchart@ideasonboard.com>\n\t<20190124232449.6t6q6cpmv6udfkqo@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190124232449.6t6q6cpmv6udfkqo@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 08/10] libcamera: pipeline_handler:\n\tAdd camera disconnection support","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:59:05 -0000"}},{"id":578,"web_url":"https://patchwork.libcamera.org/comment/578/","msgid":"","date":"2019-01-25T10:22:56","subject":"Re: [libcamera-devel] [PATCH 08/10] libcamera: pipeline_handler:\n\tAdd camera disconnection support","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi All,\n\nOn 24/01/2019 23:59, Laurent Pinchart wrote:\n> Hi Jacopo,\n> \n> On Fri, Jan 25, 2019 at 12:24:49AM +0100, Jacopo Mondi wrote:\n>> On Thu, Jan 24, 2019 at 12:16:49PM +0200, Laurent Pinchart wrote:\n>>> From: Niklas Söderlund \n>>>\n>>> Pipeline handlers are responsible for creating camera instances, but\n>>> also for destroying them when devices are unplugged. As camera objects\n>>> are reference-counted this isn't a straightforward operation and\n>>> involves the camera manager and camera object itself. Add two helper\n>>> methods in the PipelineHandler base class to register a camera and to\n>>> register a media device with the pipeline handler.\n>>>\n>>> When registering a camera, the registerCamera() helper method will add\n>>> it to the camera manager. When registering a media device, the\n>>> registerMediaDevice() helper method will listen to device disconnection\n>>> events, and disconnect all cameras created by the pipeline handler as a\n>>> response.\n>>>\n>>> Under the hood the PipelineHandler class needs to keep track of\n>>> registered cameras in order to handle disconnection. They can't be\n>>> stored as shared pointers as this would create a circular dependency\n>>> (the Camera class owns a shared pointer to the pipeline handler). Store\n>>> them as weak pointers instead. This is safe as a reference to the camera\n>>> is stored in the camera manager, and doesn't get removed until the\n>>> camera is unregistered from the manager by the PipelineHandler.\n>>>\n>>> Signed-off-by: Niklas Söderlund \n>>> Signed-off-by: Laurent Pinchart \n>>> ---\n>>> src/libcamera/include/pipeline_handler.h | 10 ++++\n>>> src/libcamera/pipeline/ipu3/ipu3.cpp | 3 +-\n>>> src/libcamera/pipeline/uvcvideo.cpp | 3 +-\n>>> src/libcamera/pipeline/vimc.cpp | 3 +-\n>>> src/libcamera/pipeline_handler.cpp | 71 ++++++++++++++++++++++++\n>>> 5 files changed, 84 insertions(+), 6 deletions(-)\n>>>\n>>> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n>>> index e1d6369eb0c4..804cce4807ee 100644\n>>> --- a/src/libcamera/include/pipeline_handler.h\n>>> +++ b/src/libcamera/include/pipeline_handler.h\n>>> @@ -16,6 +16,7 @@ namespace libcamera {\n>>>\n>>> class CameraManager;\n>>> class DeviceEnumerator;\n>>> +class MediaDevice;\n>>>\n>>> class PipelineHandler : public std::enable_shared_from_this\n>>> {\n>>> @@ -27,6 +28,15 @@ public:\n>>>\n>>> protected:\n>>> \tCameraManager *manager_;\n>>> +\n>>> +\tvoid registerCamera(std::shared_ptr camera);\n>>> +\tvoid hotplugMediaDevice(MediaDevice *media);\n>>> +\n>>> +private:\n>>> +\tvirtual void disconnect();\n>>> +\tvoid mediaDeviceDisconnected(MediaDevice *media);\n>>> +\n>>> +\tstd::vector> cameras_;\n>>> };\n>>>\n>>> class PipelineHandlerFactory\n>>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>> index 9831f74fe53f..3161e71420ed 100644\n>>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>> @@ -9,7 +9,6 @@\n>>> #include \n>>>\n>>> #include \n>>> -#include \n>>>\n>>> #include \"device_enumerator.h\"\n>>> #include \"log.h\"\n>>> @@ -169,7 +168,7 @@ void PipelineHandlerIPU3::registerCameras()\n>>>\n>>> \t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n>>> \t\tstd::shared_ptr camera = Camera::create(this, cameraName);\n>>> -\t\tmanager_->addCamera(std::move(camera));\n>>> +\t\tregisterCamera(std::move(camera));\n>>>\n>>> \t\tLOG(IPU3, Info)\n>>> \t\t\t<< \"Registered Camera[\" << numCameras << \"] \\\"\"\n>>> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n>>> index 73bad6714bb4..c8f1bf553bfe 100644\n>>> --- a/src/libcamera/pipeline/uvcvideo.cpp\n>>> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n>>> @@ -6,7 +6,6 @@\n>>> */\n>>>\n>>> #include \n>>> -#include \n>>>\n>>> #include \"device_enumerator.h\"\n>>> #include \"media_device.h\"\n>>> @@ -49,7 +48,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n>>> \tdev_->acquire();\n>>>\n>>> \tstd::shared_ptr camera = Camera::create(this, dev_->model());\n>>> -\tmanager_->addCamera(std::move(camera));\n>>> +\tregisterCamera(std::move(camera));\n>>>\n>>> \treturn true;\n>>> }\n>>> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n>>> index 521b20d3a120..b714a07688e9 100644\n>>> --- a/src/libcamera/pipeline/vimc.cpp\n>>> +++ b/src/libcamera/pipeline/vimc.cpp\n>>> @@ -6,7 +6,6 @@\n>>> */\n>>>\n>>> #include \n>>> -#include \n>>>\n>>> #include \"device_enumerator.h\"\n>>> #include \"media_device.h\"\n>>> @@ -58,7 +57,7 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)\n>>> \tdev_->acquire();\n>>>\n>>> \tstd::shared_ptr camera = Camera::create(this, \"Dummy VIMC Camera\");\n>>> -\tmanager_->addCamera(std::move(camera));\n>>> +\tregisterCamera(std::move(camera));\n>>>\n>>> \treturn true;\n>>> }\n>>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n>>> index 3850ea8fadb5..f0aa2f8022c2 100644\n>>> --- a/src/libcamera/pipeline_handler.cpp\n>>> +++ b/src/libcamera/pipeline_handler.cpp\n>>> @@ -5,7 +5,11 @@\n>>> * pipeline_handler.cpp - Pipeline handler infrastructure\n>>> */\n>>>\n>>> +#include \n>>> +#include \n>>> +\n>>> #include \"log.h\"\n>>> +#include \"media_device.h\"\n>>> #include \"pipeline_handler.h\"\n>>>\n>>> /**\n>>> @@ -97,6 +101,73 @@ PipelineHandler::~PipelineHandler()\n>>> * constant for the whole lifetime of the pipeline handler.\n>>> */\n>>>\n>>> +/**\n>>> + * \\brief Register a camera to the camera manager and pipeline handler\n>>> + * \\param[in] camera The camera to be added\n>>> + *\n>>> + * This function is called by pipeline handlers to register the cameras they\n>>> + * handle with the camera manager.\n>>> + */\n>>> +void PipelineHandler::registerCamera(std::shared_ptr camera)\n>>> +{\n>>> +\tcameras_.push_back(camera);\n>>> +\tmanager_->addCamera(std::move(camera));\n>>> +}\n>>> +\n>>> +/**\n>>> + * \\brief Handle hotplugging of a media device\n>>\n>> \"Handle\" here misleaded me. What about \"Enable\" ?\n> \n> I'll change this to \"Enable hotplug handling for a media device\".\n\nTo me 'hotplugging' means supporting both connect and disconnect.\n\nThis function doesn't do anything regarding connect - it's just\nsupporting device removal events, so the naming does seem further\nmisleading.\n\nhow about s/hotplugMediaDevice/registerDisconnect/ ? (and similar if\n'hotplug' is used on other objects too)\n\n\n\n>>> + * \\param[in] media The media device\n>>> + *\n>>> + * This function enables hotplug handling, and especially hot-unplug handling,\n>>> + * of the \\a media device. It shall be called by pipeline handlers for all the\n>>> + * media devices that can be disconnected.\n>>> + *\n>>> + * When a media device passed to this function is later unplugged, the pipeline\n>>> + * handler gets notified and automatically disconnects all the cameras it has\n>>> + * registered without requiring any manual intervention.\n>>> + */\n>>> +void PipelineHandler::hotplugMediaDevice(MediaDevice *media)\n>>> +{\n>>> +\tmedia->disconnected.connect(this, &PipelineHandler::mediaDeviceDisconnected);\n>>> +}\n>>> +\n>>> +/**\n>>> + * \\brief Device disconnection handler\n>>> + *\n>>> + * This virtual function is called to notify the pipeline handler that the\n>>> + * device it handles has been disconnected. It notifies all cameras created by\n>>> + * the pipeline handler that they have been disconnected, and unregisters them\n>>> + * from the camera manager.\n>>> + *\n>>> + * The function can be overloaded by pipeline handlers to perform custom\n>>> + * operations at disconnection time. Any overloaded version shall call the\n>>> + * PipelineHandler::disconnect() base function for proper hot-unplug operation.\n>>> + */\n>>> +void PipelineHandler::disconnect()\n>>> +{\n>>> +\tfor (std::weak_ptr ptr : cameras_) {\n>>> +\t\tstd::shared_ptr camera = ptr.lock();\n>>> +\t\tif (!camera)\n>>> +\t\t\tcontinue;\n>>> +\n>>\n>> I wonder if sub-classes need a disconnectCamera(*camera) to perform\n>> per-camera operations before disconnect...\n> \n> It's a good question, and I don't have answers yet I'm afraid. I think\n> we should add that later if it turns out to be needed. Note that\n> subclasses could always connect a slot to the camera disconnected\n> signal, but that would probably be a bit cumbersome, a virtual function\n> would likely be better.\n> \n>>> +\t\tcamera->disconnect();\n>>> +\t\tmanager_->removeCamera(camera.get());\n>>> +\t}\n>>> +\n>>> +\tcameras_.clear();\n>>> +}\n>>> +\n>>> +/**\n>>> + * \\brief Slot for the MediaDevice disconnected signal\n>>> + */\n>>> +void PipelineHandler::mediaDeviceDisconnected(MediaDevice *media)\n>>> +{\n>>> +\tif (cameras_.empty())\n>>> +\t\treturn;\n>>> +\n>>> +\tdisconnect();\n>>> +}\n>>> +\n>>> /**\n>>> * \\class PipelineHandlerFactory\n>>> * \\brief Registration of PipelineHandler classes and creation of instances\n>","headers":{"Return-Path":"","Received":["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 A370260C7F\n\tfor ;\n\tFri, 25 Jan 2019 11:22:59 +0100 (CET)","from [192.168.0.21]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E489E325;\n\tFri, 25 Jan 2019 11:22:58 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548411779;\n\tbh=82P1Rik1NfR+IqiUCbu4LYh95o717VdC/DGklx+VHMo=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=TlhZqgO4sn85ycet+a96dt+NeTS8IrUAdexfVgdfzwNW25jHk84AZknV/ySqbmGYx\n\tMY6DXXpejGeubNuSf9BdxXSxyKZTJQW/oB1SAV/QpnMXVbHX5YUyOL7AD5o4Vzb0Cd\n\tGxYcB3pVSlvLMSHdaXjzCpftOs+f+DqL2YQ/wEto=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart ,\n\tJacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20190124101651.9993-1-laurent.pinchart@ideasonboard.com>\n\t<20190124101651.9993-9-laurent.pinchart@ideasonboard.com>\n\t<20190124232449.6t6q6cpmv6udfkqo@uno.localdomain>\n\t<20190124235903.GC4399@pendragon.ideasonboard.com>","From":"Kieran Bingham ","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"","Date":"Fri, 25 Jan 2019 10:22:56 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.2.1","MIME-Version":"1.0","In-Reply-To":"<20190124235903.GC4399@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 08/10] libcamera: pipeline_handler:\n\tAdd camera disconnection support","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":"Fri, 25 Jan 2019 10:22:59 -0000"}},{"id":580,"web_url":"https://patchwork.libcamera.org/comment/580/","msgid":"<20190125104307.GB2934@pendragon.ideasonboard.com>","date":"2019-01-25T10:43:07","subject":"Re: [libcamera-devel] [PATCH 08/10] libcamera: pipeline_handler:\n\tAdd camera disconnection support","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Fri, Jan 25, 2019 at 10:22:56AM +0000, Kieran Bingham wrote:\n> On 24/01/2019 23:59, Laurent Pinchart wrote:\n> > On Fri, Jan 25, 2019 at 12:24:49AM +0100, Jacopo Mondi wrote:\n> >> On Thu, Jan 24, 2019 at 12:16:49PM +0200, Laurent Pinchart wrote:\n> >>> From: Niklas Söderlund \n> >>>\n> >>> Pipeline handlers are responsible for creating camera instances, but\n> >>> also for destroying them when devices are unplugged. As camera objects\n> >>> are reference-counted this isn't a straightforward operation and\n> >>> involves the camera manager and camera object itself. Add two helper\n> >>> methods in the PipelineHandler base class to register a camera and to\n> >>> register a media device with the pipeline handler.\n> >>>\n> >>> When registering a camera, the registerCamera() helper method will add\n> >>> it to the camera manager. When registering a media device, the\n> >>> registerMediaDevice() helper method will listen to device disconnection\n> >>> events, and disconnect all cameras created by the pipeline handler as a\n> >>> response.\n> >>>\n> >>> Under the hood the PipelineHandler class needs to keep track of\n> >>> registered cameras in order to handle disconnection. They can't be\n> >>> stored as shared pointers as this would create a circular dependency\n> >>> (the Camera class owns a shared pointer to the pipeline handler). Store\n> >>> them as weak pointers instead. This is safe as a reference to the camera\n> >>> is stored in the camera manager, and doesn't get removed until the\n> >>> camera is unregistered from the manager by the PipelineHandler.\n> >>>\n> >>> Signed-off-by: Niklas Söderlund \n> >>> Signed-off-by: Laurent Pinchart \n> >>> ---\n> >>> src/libcamera/include/pipeline_handler.h | 10 ++++\n> >>> src/libcamera/pipeline/ipu3/ipu3.cpp | 3 +-\n> >>> src/libcamera/pipeline/uvcvideo.cpp | 3 +-\n> >>> src/libcamera/pipeline/vimc.cpp | 3 +-\n> >>> src/libcamera/pipeline_handler.cpp | 71 ++++++++++++++++++++++++\n> >>> 5 files changed, 84 insertions(+), 6 deletions(-)\n> >>>\n> >>> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> >>> index e1d6369eb0c4..804cce4807ee 100644\n> >>> --- a/src/libcamera/include/pipeline_handler.h\n> >>> +++ b/src/libcamera/include/pipeline_handler.h\n> >>> @@ -16,6 +16,7 @@ namespace libcamera {\n> >>>\n> >>> class CameraManager;\n> >>> class DeviceEnumerator;\n> >>> +class MediaDevice;\n> >>>\n> >>> class PipelineHandler : public std::enable_shared_from_this\n> >>> {\n> >>> @@ -27,6 +28,15 @@ public:\n> >>>\n> >>> protected:\n> >>> \tCameraManager *manager_;\n> >>> +\n> >>> +\tvoid registerCamera(std::shared_ptr camera);\n> >>> +\tvoid hotplugMediaDevice(MediaDevice *media);\n> >>> +\n> >>> +private:\n> >>> +\tvirtual void disconnect();\n> >>> +\tvoid mediaDeviceDisconnected(MediaDevice *media);\n> >>> +\n> >>> +\tstd::vector> cameras_;\n> >>> };\n> >>>\n> >>> class PipelineHandlerFactory\n> >>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> >>> index 9831f74fe53f..3161e71420ed 100644\n> >>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> >>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> >>> @@ -9,7 +9,6 @@\n> >>> #include \n> >>>\n> >>> #include \n> >>> -#include \n> >>>\n> >>> #include \"device_enumerator.h\"\n> >>> #include \"log.h\"\n> >>> @@ -169,7 +168,7 @@ void PipelineHandlerIPU3::registerCameras()\n> >>>\n> >>> \t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n> >>> \t\tstd::shared_ptr camera = Camera::create(this, cameraName);\n> >>> -\t\tmanager_->addCamera(std::move(camera));\n> >>> +\t\tregisterCamera(std::move(camera));\n> >>>\n> >>> \t\tLOG(IPU3, Info)\n> >>> \t\t\t<< \"Registered Camera[\" << numCameras << \"] \\\"\"\n> >>> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> >>> index 73bad6714bb4..c8f1bf553bfe 100644\n> >>> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> >>> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> >>> @@ -6,7 +6,6 @@\n> >>> */\n> >>>\n> >>> #include \n> >>> -#include \n> >>>\n> >>> #include \"device_enumerator.h\"\n> >>> #include \"media_device.h\"\n> >>> @@ -49,7 +48,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n> >>> \tdev_->acquire();\n> >>>\n> >>> \tstd::shared_ptr camera = Camera::create(this, dev_->model());\n> >>> -\tmanager_->addCamera(std::move(camera));\n> >>> +\tregisterCamera(std::move(camera));\n> >>>\n> >>> \treturn true;\n> >>> }\n> >>> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> >>> index 521b20d3a120..b714a07688e9 100644\n> >>> --- a/src/libcamera/pipeline/vimc.cpp\n> >>> +++ b/src/libcamera/pipeline/vimc.cpp\n> >>> @@ -6,7 +6,6 @@\n> >>> */\n> >>>\n> >>> #include \n> >>> -#include \n> >>>\n> >>> #include \"device_enumerator.h\"\n> >>> #include \"media_device.h\"\n> >>> @@ -58,7 +57,7 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)\n> >>> \tdev_->acquire();\n> >>>\n> >>> \tstd::shared_ptr camera = Camera::create(this, \"Dummy VIMC Camera\");\n> >>> -\tmanager_->addCamera(std::move(camera));\n> >>> +\tregisterCamera(std::move(camera));\n> >>>\n> >>> \treturn true;\n> >>> }\n> >>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> >>> index 3850ea8fadb5..f0aa2f8022c2 100644\n> >>> --- a/src/libcamera/pipeline_handler.cpp\n> >>> +++ b/src/libcamera/pipeline_handler.cpp\n> >>> @@ -5,7 +5,11 @@\n> >>> * pipeline_handler.cpp - Pipeline handler infrastructure\n> >>> */\n> >>>\n> >>> +#include \n> >>> +#include \n> >>> +\n> >>> #include \"log.h\"\n> >>> +#include \"media_device.h\"\n> >>> #include \"pipeline_handler.h\"\n> >>>\n> >>> /**\n> >>> @@ -97,6 +101,73 @@ PipelineHandler::~PipelineHandler()\n> >>> * constant for the whole lifetime of the pipeline handler.\n> >>> */\n> >>>\n> >>> +/**\n> >>> + * \\brief Register a camera to the camera manager and pipeline handler\n> >>> + * \\param[in] camera The camera to be added\n> >>> + *\n> >>> + * This function is called by pipeline handlers to register the cameras they\n> >>> + * handle with the camera manager.\n> >>> + */\n> >>> +void PipelineHandler::registerCamera(std::shared_ptr camera)\n> >>> +{\n> >>> +\tcameras_.push_back(camera);\n> >>> +\tmanager_->addCamera(std::move(camera));\n> >>> +}\n> >>> +\n> >>> +/**\n> >>> + * \\brief Handle hotplugging of a media device\n> >>\n> >> \"Handle\" here misleaded me. What about \"Enable\" ?\n> > \n> > I'll change this to \"Enable hotplug handling for a media device\".\n> \n> To me 'hotplugging' means supporting both connect and disconnect.\n> \n> This function doesn't do anything regarding connect - it's just\n> supporting device removal events, so the naming does seem further\n> misleading.\n> \n> how about s/hotplugMediaDevice/registerDisconnect/ ? (and similar if\n> 'hotplug' is used on other objects too)\n\nI'm of two minds about this. I agree the naming isn't ideal, but\nregisterDisconnect sounds more cryptic to me. The name\nhotplugMediaDevice at least refers to the hotplug mechanism, and could\nalso be understood as registering a media device that has been\nhotplugged.\n\nI expect this API to evolve as I'd like the acquire/release of media\ndevices to be handled automatically (it is otherwise quite error-prone\nfor pipeline handler developers). How about revisiting the function then\n?\n\n> >>> + * \\param[in] media The media device\n> >>> + *\n> >>> + * This function enables hotplug handling, and especially hot-unplug handling,\n> >>> + * of the \\a media device. It shall be called by pipeline handlers for all the\n> >>> + * media devices that can be disconnected.\n> >>> + *\n> >>> + * When a media device passed to this function is later unplugged, the pipeline\n> >>> + * handler gets notified and automatically disconnects all the cameras it has\n> >>> + * registered without requiring any manual intervention.\n> >>> + */\n> >>> +void PipelineHandler::hotplugMediaDevice(MediaDevice *media)\n> >>> +{\n> >>> +\tmedia->disconnected.connect(this, &PipelineHandler::mediaDeviceDisconnected);\n> >>> +}\n> >>> +\n> >>> +/**\n> >>> + * \\brief Device disconnection handler\n> >>> + *\n> >>> + * This virtual function is called to notify the pipeline handler that the\n> >>> + * device it handles has been disconnected. It notifies all cameras created by\n> >>> + * the pipeline handler that they have been disconnected, and unregisters them\n> >>> + * from the camera manager.\n> >>> + *\n> >>> + * The function can be overloaded by pipeline handlers to perform custom\n> >>> + * operations at disconnection time. Any overloaded version shall call the\n> >>> + * PipelineHandler::disconnect() base function for proper hot-unplug operation.\n> >>> + */\n> >>> +void PipelineHandler::disconnect()\n> >>> +{\n> >>> +\tfor (std::weak_ptr ptr : cameras_) {\n> >>> +\t\tstd::shared_ptr camera = ptr.lock();\n> >>> +\t\tif (!camera)\n> >>> +\t\t\tcontinue;\n> >>> +\n> >>\n> >> I wonder if sub-classes need a disconnectCamera(*camera) to perform\n> >> per-camera operations before disconnect...\n> > \n> > It's a good question, and I don't have answers yet I'm afraid. I think\n> > we should add that later if it turns out to be needed. Note that\n> > subclasses could always connect a slot to the camera disconnected\n> > signal, but that would probably be a bit cumbersome, a virtual function\n> > would likely be better.\n> > \n> >>> +\t\tcamera->disconnect();\n> >>> +\t\tmanager_->removeCamera(camera.get());\n> >>> +\t}\n> >>> +\n> >>> +\tcameras_.clear();\n> >>> +}\n> >>> +\n> >>> +/**\n> >>> + * \\brief Slot for the MediaDevice disconnected signal\n> >>> + */\n> >>> +void PipelineHandler::mediaDeviceDisconnected(MediaDevice *media)\n> >>> +{\n> >>> +\tif (cameras_.empty())\n> >>> +\t\treturn;\n> >>> +\n> >>> +\tdisconnect();\n> >>> +}\n> >>> +\n> >>> /**\n> >>> * \\class PipelineHandlerFactory\n> >>> * \\brief Registration of PipelineHandler classes and creation of instances","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 F40F660C7F\n\tfor ;\n\tFri, 25 Jan 2019 11:43:08 +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 68BAC325;\n\tFri, 25 Jan 2019 11:43:08 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548412988;\n\tbh=kw+eEGiY0ZAKGzd1+unzzw/LXkMRC44IsjlIu6rKD3c=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ka3ljIsCmSmPxJvB5ptDZFl+4EzVl7BYZSQ0rrSYsowncO4xENCGfSxcU9yMpoRLl\n\tncFhjTJ25kbZUXJ8WzK2UyZ012c39MFdRbLKNLh6GdkbkRUaOumLQ5OltOH6Kly50U\n\tSMMmiJLNG150VgyYzXHm4SwK72eDXRd9SBKhxVXA=","Date":"Fri, 25 Jan 2019 12:43:07 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","Message-ID":"<20190125104307.GB2934@pendragon.ideasonboard.com>","References":"<20190124101651.9993-1-laurent.pinchart@ideasonboard.com>\n\t<20190124101651.9993-9-laurent.pinchart@ideasonboard.com>\n\t<20190124232449.6t6q6cpmv6udfkqo@uno.localdomain>\n\t<20190124235903.GC4399@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 08/10] libcamera: pipeline_handler:\n\tAdd camera disconnection support","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":"Fri, 25 Jan 2019 10:43:09 -0000"}},{"id":581,"web_url":"https://patchwork.libcamera.org/comment/581/","msgid":"<68a57e54-fe96-49b9-be64-d3d4b7a694c8@ideasonboard.com>","date":"2019-01-25T10:50:59","subject":"Re: [libcamera-devel] [PATCH 08/10] libcamera: pipeline_handler:\n\tAdd camera disconnection support","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 25/01/2019 10:43, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> On Fri, Jan 25, 2019 at 10:22:56AM +0000, Kieran Bingham wrote:\n>> On 24/01/2019 23:59, Laurent Pinchart wrote:\n>>> On Fri, Jan 25, 2019 at 12:24:49AM +0100, Jacopo Mondi wrote:\n>>>> On Thu, Jan 24, 2019 at 12:16:49PM +0200, Laurent Pinchart wrote:\n>>>>> From: Niklas Söderlund \n>>>>>\n>>>>> Pipeline handlers are responsible for creating camera instances, but\n>>>>> also for destroying them when devices are unplugged. As camera objects\n>>>>> are reference-counted this isn't a straightforward operation and\n>>>>> involves the camera manager and camera object itself. Add two helper\n>>>>> methods in the PipelineHandler base class to register a camera and to\n>>>>> register a media device with the pipeline handler.\n>>>>>\n>>>>> When registering a camera, the registerCamera() helper method will add\n>>>>> it to the camera manager. When registering a media device, the\n>>>>> registerMediaDevice() helper method will listen to device disconnection\n>>>>> events, and disconnect all cameras created by the pipeline handler as a\n>>>>> response.\n>>>>>\n>>>>> Under the hood the PipelineHandler class needs to keep track of\n>>>>> registered cameras in order to handle disconnection. They can't be\n>>>>> stored as shared pointers as this would create a circular dependency\n>>>>> (the Camera class owns a shared pointer to the pipeline handler). Store\n>>>>> them as weak pointers instead. This is safe as a reference to the camera\n>>>>> is stored in the camera manager, and doesn't get removed until the\n>>>>> camera is unregistered from the manager by the PipelineHandler.\n>>>>>\n>>>>> Signed-off-by: Niklas Söderlund \n>>>>> Signed-off-by: Laurent Pinchart \n>>>>> ---\n>>>>> src/libcamera/include/pipeline_handler.h | 10 ++++\n>>>>> src/libcamera/pipeline/ipu3/ipu3.cpp | 3 +-\n>>>>> src/libcamera/pipeline/uvcvideo.cpp | 3 +-\n>>>>> src/libcamera/pipeline/vimc.cpp | 3 +-\n>>>>> src/libcamera/pipeline_handler.cpp | 71 ++++++++++++++++++++++++\n>>>>> 5 files changed, 84 insertions(+), 6 deletions(-)\n>>>>>\n>>>>> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n>>>>> index e1d6369eb0c4..804cce4807ee 100644\n>>>>> --- a/src/libcamera/include/pipeline_handler.h\n>>>>> +++ b/src/libcamera/include/pipeline_handler.h\n>>>>> @@ -16,6 +16,7 @@ namespace libcamera {\n>>>>>\n>>>>> class CameraManager;\n>>>>> class DeviceEnumerator;\n>>>>> +class MediaDevice;\n>>>>>\n>>>>> class PipelineHandler : public std::enable_shared_from_this\n>>>>> {\n>>>>> @@ -27,6 +28,15 @@ public:\n>>>>>\n>>>>> protected:\n>>>>> \tCameraManager *manager_;\n>>>>> +\n>>>>> +\tvoid registerCamera(std::shared_ptr camera);\n>>>>> +\tvoid hotplugMediaDevice(MediaDevice *media);\n>>>>> +\n>>>>> +private:\n>>>>> +\tvirtual void disconnect();\n>>>>> +\tvoid mediaDeviceDisconnected(MediaDevice *media);\n>>>>> +\n>>>>> +\tstd::vector> cameras_;\n>>>>> };\n>>>>>\n>>>>> class PipelineHandlerFactory\n>>>>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>>>> index 9831f74fe53f..3161e71420ed 100644\n>>>>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>>>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>>>> @@ -9,7 +9,6 @@\n>>>>> #include \n>>>>>\n>>>>> #include \n>>>>> -#include \n>>>>>\n>>>>> #include \"device_enumerator.h\"\n>>>>> #include \"log.h\"\n>>>>> @@ -169,7 +168,7 @@ void PipelineHandlerIPU3::registerCameras()\n>>>>>\n>>>>> \t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n>>>>> \t\tstd::shared_ptr camera = Camera::create(this, cameraName);\n>>>>> -\t\tmanager_->addCamera(std::move(camera));\n>>>>> +\t\tregisterCamera(std::move(camera));\n>>>>>\n>>>>> \t\tLOG(IPU3, Info)\n>>>>> \t\t\t<< \"Registered Camera[\" << numCameras << \"] \\\"\"\n>>>>> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n>>>>> index 73bad6714bb4..c8f1bf553bfe 100644\n>>>>> --- a/src/libcamera/pipeline/uvcvideo.cpp\n>>>>> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n>>>>> @@ -6,7 +6,6 @@\n>>>>> */\n>>>>>\n>>>>> #include \n>>>>> -#include \n>>>>>\n>>>>> #include \"device_enumerator.h\"\n>>>>> #include \"media_device.h\"\n>>>>> @@ -49,7 +48,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n>>>>> \tdev_->acquire();\n>>>>>\n>>>>> \tstd::shared_ptr camera = Camera::create(this, dev_->model());\n>>>>> -\tmanager_->addCamera(std::move(camera));\n>>>>> +\tregisterCamera(std::move(camera));\n>>>>>\n>>>>> \treturn true;\n>>>>> }\n>>>>> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n>>>>> index 521b20d3a120..b714a07688e9 100644\n>>>>> --- a/src/libcamera/pipeline/vimc.cpp\n>>>>> +++ b/src/libcamera/pipeline/vimc.cpp\n>>>>> @@ -6,7 +6,6 @@\n>>>>> */\n>>>>>\n>>>>> #include \n>>>>> -#include \n>>>>>\n>>>>> #include \"device_enumerator.h\"\n>>>>> #include \"media_device.h\"\n>>>>> @@ -58,7 +57,7 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)\n>>>>> \tdev_->acquire();\n>>>>>\n>>>>> \tstd::shared_ptr camera = Camera::create(this, \"Dummy VIMC Camera\");\n>>>>> -\tmanager_->addCamera(std::move(camera));\n>>>>> +\tregisterCamera(std::move(camera));\n>>>>>\n>>>>> \treturn true;\n>>>>> }\n>>>>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n>>>>> index 3850ea8fadb5..f0aa2f8022c2 100644\n>>>>> --- a/src/libcamera/pipeline_handler.cpp\n>>>>> +++ b/src/libcamera/pipeline_handler.cpp\n>>>>> @@ -5,7 +5,11 @@\n>>>>> * pipeline_handler.cpp - Pipeline handler infrastructure\n>>>>> */\n>>>>>\n>>>>> +#include \n>>>>> +#include \n>>>>> +\n>>>>> #include \"log.h\"\n>>>>> +#include \"media_device.h\"\n>>>>> #include \"pipeline_handler.h\"\n>>>>>\n>>>>> /**\n>>>>> @@ -97,6 +101,73 @@ PipelineHandler::~PipelineHandler()\n>>>>> * constant for the whole lifetime of the pipeline handler.\n>>>>> */\n>>>>>\n>>>>> +/**\n>>>>> + * \\brief Register a camera to the camera manager and pipeline handler\n>>>>> + * \\param[in] camera The camera to be added\n>>>>> + *\n>>>>> + * This function is called by pipeline handlers to register the cameras they\n>>>>> + * handle with the camera manager.\n>>>>> + */\n>>>>> +void PipelineHandler::registerCamera(std::shared_ptr camera)\n>>>>> +{\n>>>>> +\tcameras_.push_back(camera);\n>>>>> +\tmanager_->addCamera(std::move(camera));\n>>>>> +}\n>>>>> +\n>>>>> +/**\n>>>>> + * \\brief Handle hotplugging of a media device\n>>>>\n>>>> \"Handle\" here misleaded me. What about \"Enable\" ?\n>>>\n>>> I'll change this to \"Enable hotplug handling for a media device\".\n>>\n>> To me 'hotplugging' means supporting both connect and disconnect.\n>>\n>> This function doesn't do anything regarding connect - it's just\n>> supporting device removal events, so the naming does seem further\n>> misleading.\n>>\n>> how about s/hotplugMediaDevice/registerDisconnect/ ? (and similar if\n>> 'hotplug' is used on other objects too)\n> \n> I'm of two minds about this. I agree the naming isn't ideal, but\n> registerDisconnect sounds more cryptic to me. The name\n> hotplugMediaDevice at least refers to the hotplug mechanism, and could\n> also be understood as registering a media device that has been\n> hotplugged.\n\nOtherwise, registerDisconnectHandler(MediaDevice)?\n\n> I expect this API to evolve as I'd like the acquire/release of media\n> devices to be handled automatically (it is otherwise quite error-prone\n> for pipeline handler developers). How about revisiting the function then\n> ?\n\nAs you wish,\n\n--\nKieran\n\n\n\n>>>>> + * \\param[in] media The media device\n>>>>> + *\n>>>>> + * This function enables hotplug handling, and especially hot-unplug handling,\n>>>>> + * of the \\a media device. It shall be called by pipeline handlers for all the\n>>>>> + * media devices that can be disconnected.\n>>>>> + *\n>>>>> + * When a media device passed to this function is later unplugged, the pipeline\n>>>>> + * handler gets notified and automatically disconnects all the cameras it has\n>>>>> + * registered without requiring any manual intervention.\n>>>>> + */\n>>>>> +void PipelineHandler::hotplugMediaDevice(MediaDevice *media)\n>>>>> +{\n>>>>> +\tmedia->disconnected.connect(this, &PipelineHandler::mediaDeviceDisconnected);\n>>>>> +}\n>>>>> +\n>>>>> +/**\n>>>>> + * \\brief Device disconnection handler\n>>>>> + *\n>>>>> + * This virtual function is called to notify the pipeline handler that the\n>>>>> + * device it handles has been disconnected. It notifies all cameras created by\n>>>>> + * the pipeline handler that they have been disconnected, and unregisters them\n>>>>> + * from the camera manager.\n>>>>> + *\n>>>>> + * The function can be overloaded by pipeline handlers to perform custom\n>>>>> + * operations at disconnection time. Any overloaded version shall call the\n>>>>> + * PipelineHandler::disconnect() base function for proper hot-unplug operation.\n>>>>> + */\n>>>>> +void PipelineHandler::disconnect()\n>>>>> +{\n>>>>> +\tfor (std::weak_ptr ptr : cameras_) {\n>>>>> +\t\tstd::shared_ptr camera = ptr.lock();\n>>>>> +\t\tif (!camera)\n>>>>> +\t\t\tcontinue;\n>>>>> +\n>>>>\n>>>> I wonder if sub-classes need a disconnectCamera(*camera) to perform\n>>>> per-camera operations before disconnect...\n>>>\n>>> It's a good question, and I don't have answers yet I'm afraid. I think\n>>> we should add that later if it turns out to be needed. Note that\n>>> subclasses could always connect a slot to the camera disconnected\n>>> signal, but that would probably be a bit cumbersome, a virtual function\n>>> would likely be better.\n>>>\n>>>>> +\t\tcamera->disconnect();\n>>>>> +\t\tmanager_->removeCamera(camera.get());\n>>>>> +\t}\n>>>>> +\n>>>>> +\tcameras_.clear();\n>>>>> +}\n>>>>> +\n>>>>> +/**\n>>>>> + * \\brief Slot for the MediaDevice disconnected signal\n>>>>> + */\n>>>>> +void PipelineHandler::mediaDeviceDisconnected(MediaDevice *media)\n>>>>> +{\n>>>>> +\tif (cameras_.empty())\n>>>>> +\t\treturn;\n>>>>> +\n>>>>> +\tdisconnect();\n>>>>> +}\n>>>>> +\n>>>>> /**\n>>>>> * \\class PipelineHandlerFactory\n>>>>> * \\brief Registration of PipelineHandler classes and creation of instances\n>","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 59DDE60B2D\n\tfor ;\n\tFri, 25 Jan 2019 11:51:04 +0100 (CET)","from [192.168.0.21]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 63934325;\n\tFri, 25 Jan 2019 11:51:03 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548413463;\n\tbh=fHGY4uBMM1r3FH24tzWUmtTUyrMxqWNRqO3TAK4IHPU=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=p86jBmWbaQ5RnXahGA6cJMeXHLZ3WBF+F2r6lgDFnqfuWLsZRpIuhTYYOQGnfjdkt\n\t7F9ToCV3nzTd4ZrAzbRBrwg9iMU7Hzb2nvBP2jBe+Oq5RpqLGzcMXmzHo6uDm1s9aF\n\tcDoUp2FrcoSJcx7igJ6gfzRDdvCEd8V05JtV7vJo=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart ","Cc":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","References":"<20190124101651.9993-1-laurent.pinchart@ideasonboard.com>\n\t<20190124101651.9993-9-laurent.pinchart@ideasonboard.com>\n\t<20190124232449.6t6q6cpmv6udfkqo@uno.localdomain>\n\t<20190124235903.GC4399@pendragon.ideasonboard.com>\n\t\n\t<20190125104307.GB2934@pendragon.ideasonboard.com>","From":"Kieran Bingham ","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<68a57e54-fe96-49b9-be64-d3d4b7a694c8@ideasonboard.com>","Date":"Fri, 25 Jan 2019 10:50:59 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.2.1","MIME-Version":"1.0","In-Reply-To":"<20190125104307.GB2934@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 08/10] libcamera: pipeline_handler:\n\tAdd camera disconnection support","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":"Fri, 25 Jan 2019 10:51:04 -0000"}}]