{"id":975,"url":"https://patchwork.libcamera.org/api/patches/975/?format=json","web_url":"https://patchwork.libcamera.org/patch/975/","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":"<20190414013506.10515-7-niklas.soderlund@ragnatech.se>","date":"2019-04-14T01:35:01","name":"[libcamera-devel,RFC,06/11] libcamera: pipeline_handler: Keep track of MediaDevice","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"94e295caf99078b96a210a074ac73cfb89974557","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/?format=json","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/975/mbox/","series":[{"id":249,"url":"https://patchwork.libcamera.org/api/series/249/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=249","date":"2019-04-14T01:34:55","name":"libcamerea: Add support for exclusive access to cameras between processes","version":1,"mbox":"https://patchwork.libcamera.org/series/249/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/975/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/975/checks/","tags":{},"headers":{"Return-Path":"","Received":["from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net\n\t[195.74.38.228])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 99C1860DBF\n\tfor ;\n\tSun, 14 Apr 2019 03:35:18 +0200 (CEST)","from bismarck.berto.se (unknown [89.233.230.99])\n\tby bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA\n\tid 8ee54c97-5e55-11e9-8e2c-005056917f90;\n\tSun, 14 Apr 2019 03:35:17 +0200 (CEST)"],"X-Halon-ID":"8ee54c97-5e55-11e9-8e2c-005056917f90","Authorized-sender":"niklas@soderlund.pp.se","From":"=?utf-8?q?Niklas_S=C3=B6derlund?= ","To":"libcamera-devel@lists.libcamera.org","Date":"Sun, 14 Apr 2019 03:35:01 +0200","Message-Id":"<20190414013506.10515-7-niklas.soderlund@ragnatech.se>","X-Mailer":"git-send-email 2.21.0","In-Reply-To":"<20190414013506.10515-1-niklas.soderlund@ragnatech.se>","References":"<20190414013506.10515-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [RFC 06/11] libcamera: pipeline_handler: Keep\n\ttrack of MediaDevice","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":"Sun, 14 Apr 2019 01:35:19 -0000"},"content":"Instead of requiring each pipeline handle implementation to keep track\nof calling release() on its media devices upon deletion keep track of\nthem in the base class. Add a helper which pipeline handlers shall use\nto acquire a media device instead of directly interacting with the\nDeviceEnumerator.\n\nCentrally keeping track of media devices will also be beneficial\nimplementing pipeline exclusive access across processes.\n\nSigned-off-by: Niklas Söderlund \n---\n src/libcamera/include/pipeline_handler.h | 4 ++++\n src/libcamera/pipeline/ipu3/ipu3.cpp | 20 ++------------------\n src/libcamera/pipeline/uvcvideo.cpp | 24 +++++++-----------------\n src/libcamera/pipeline/vimc.cpp | 22 ++++++----------------\n src/libcamera/pipeline_handler.cpp | 22 ++++++++++++++++++++++\n 5 files changed, 41 insertions(+), 51 deletions(-)","diff":"diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\nindex b6cbd3bae51b08dc..d995a7e56d90f706 100644\n--- a/src/libcamera/include/pipeline_handler.h\n+++ b/src/libcamera/include/pipeline_handler.h\n@@ -21,6 +21,7 @@ class Camera;\n class CameraConfiguration;\n class CameraManager;\n class DeviceEnumerator;\n+class DeviceMatch;\n class MediaDevice;\n class PipelineHandler;\n class Request;\n@@ -52,6 +53,8 @@ public:\n \tvirtual ~PipelineHandler();\n \n \tvirtual bool match(DeviceEnumerator *enumerator) = 0;\n+\tstd::shared_ptr tryAcquire(DeviceEnumerator *enumerator,\n+\t\t\t\t\t\tconst DeviceMatch &dm);\n \n \tvirtual CameraConfiguration\n \tstreamConfiguration(Camera *camera, const std::vector &usages) = 0;\n@@ -81,6 +84,7 @@ private:\n \tvoid mediaDeviceDisconnected(MediaDevice *media);\n \tvirtual void disconnect();\n \n+\tstd::vector> mediaDevices_;\n \tstd::vector> cameras_;\n \tstd::map> cameraData_;\n };\ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex a87eff8dfec6d143..77d14533e6bceb80 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -137,7 +137,6 @@ class PipelineHandlerIPU3 : public PipelineHandler\n {\n public:\n \tPipelineHandlerIPU3(CameraManager *manager);\n-\t~PipelineHandlerIPU3();\n \n \tCameraConfiguration\n \tstreamConfiguration(Camera *camera,\n@@ -195,15 +194,6 @@ PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager)\n {\n }\n \n-PipelineHandlerIPU3::~PipelineHandlerIPU3()\n-{\n-\tif (cio2MediaDev_)\n-\t\tcio2MediaDev_->release();\n-\n-\tif (imguMediaDev_)\n-\t\timguMediaDev_->release();\n-}\n-\n CameraConfiguration\n PipelineHandlerIPU3::streamConfiguration(Camera *camera,\n \t\t\t\t\t const std::vector &usages)\n@@ -451,20 +441,14 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n \timgu_dm.add(\"ipu3-imgu 1 viewfinder\");\n \timgu_dm.add(\"ipu3-imgu 1 3a stat\");\n \n-\tcio2MediaDev_ = enumerator->search(cio2_dm);\n+\tcio2MediaDev_ = tryAcquire(enumerator, cio2_dm);\n \tif (!cio2MediaDev_)\n \t\treturn false;\n \n-\tif (!cio2MediaDev_->acquire())\n-\t\treturn false;\n-\n-\timguMediaDev_ = enumerator->search(imgu_dm);\n+\timguMediaDev_ = tryAcquire(enumerator, imgu_dm);\n \tif (!imguMediaDev_)\n \t\treturn false;\n \n-\tif (!imguMediaDev_->acquire())\n-\t\treturn false;\n-\n \t/*\n \t * Disable all links that are enabled by default on CIO2, as camera\n \t * creation enables all valid links it finds.\ndiff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\nindex 5214bfd3097b8217..43a6d59670299376 100644\n--- a/src/libcamera/pipeline/uvcvideo.cpp\n+++ b/src/libcamera/pipeline/uvcvideo.cpp\n@@ -24,7 +24,6 @@ class PipelineHandlerUVC : public PipelineHandler\n {\n public:\n \tPipelineHandlerUVC(CameraManager *manager);\n-\t~PipelineHandlerUVC();\n \n \tCameraConfiguration\n \tstreamConfiguration(Camera *camera,\n@@ -68,20 +67,13 @@ private:\n \t\t\tPipelineHandler::cameraData(camera));\n \t}\n \n-\tstd::shared_ptr media_;\n };\n \n PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)\n-\t: PipelineHandler(manager), media_(nullptr)\n+\t: PipelineHandler(manager)\n {\n }\n \n-PipelineHandlerUVC::~PipelineHandlerUVC()\n-{\n-\tif (media_)\n-\t\tmedia_->release();\n-}\n-\n CameraConfiguration\n PipelineHandlerUVC::streamConfiguration(Camera *camera,\n \t\t\t\t\tconst std::vector &usages)\n@@ -178,19 +170,17 @@ int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request)\n \n bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n {\n+\tstd::shared_ptr media;\n \tDeviceMatch dm(\"uvcvideo\");\n \n-\tmedia_ = enumerator->search(dm);\n-\tif (!media_)\n-\t\treturn false;\n-\n-\tif (!media_->acquire())\n+\tmedia = tryAcquire(enumerator, dm);\n+\tif (!media)\n \t\treturn false;\n \n \tstd::unique_ptr data = utils::make_unique(this);\n \n \t/* Locate and open the default video node. */\n-\tfor (MediaEntity *entity : media_->entities()) {\n+\tfor (MediaEntity *entity : media->entities()) {\n \t\tif (entity->flags() & MEDIA_ENT_FL_DEFAULT) {\n \t\t\tdata->video_ = new V4L2Device(entity);\n \t\t\tbreak;\n@@ -209,11 +199,11 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n \n \t/* Create and register the camera. */\n \tstd::set streams{ &data->stream_ };\n-\tstd::shared_ptr camera = Camera::create(this, media_->model(), streams);\n+\tstd::shared_ptr camera = Camera::create(this, media->model(), streams);\n \tregisterCamera(std::move(camera), std::move(data));\n \n \t/* Enable hot-unplug notifications. */\n-\thotplugMediaDevice(media_.get());\n+\thotplugMediaDevice(media.get());\n \n \treturn true;\n }\ndiff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\nindex e5e78ccedd59ae66..a80331e1cd4d1e46 100644\n--- a/src/libcamera/pipeline/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc.cpp\n@@ -24,7 +24,6 @@ class PipelineHandlerVimc : public PipelineHandler\n {\n public:\n \tPipelineHandlerVimc(CameraManager *manager);\n-\t~PipelineHandlerVimc();\n \n \tCameraConfiguration\n \tstreamConfiguration(Camera *camera,\n@@ -67,21 +66,13 @@ private:\n \t\treturn static_cast(\n \t\t\tPipelineHandler::cameraData(camera));\n \t}\n-\n-\tstd::shared_ptr media_;\n };\n \n PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager)\n-\t: PipelineHandler(manager), media_(nullptr)\n+\t: PipelineHandler(manager)\n {\n }\n \n-PipelineHandlerVimc::~PipelineHandlerVimc()\n-{\n-\tif (media_)\n-\t\tmedia_->release();\n-}\n-\n CameraConfiguration\n PipelineHandlerVimc::streamConfiguration(Camera *camera,\n \t\t\t\t\t const std::vector &usages)\n@@ -178,6 +169,8 @@ int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request)\n \n bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n {\n+\tstd::shared_ptr media;\n+\n \tDeviceMatch dm(\"vimc\");\n \n \tdm.add(\"Raw Capture 0\");\n@@ -190,17 +183,14 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n \tdm.add(\"RGB/YUV Input\");\n \tdm.add(\"Scaler\");\n \n-\tmedia_ = enumerator->search(dm);\n-\tif (!media_)\n-\t\treturn false;\n-\n-\tif (!media_->acquire())\n+\tmedia = tryAcquire(enumerator, dm);\n+\tif (!media)\n \t\treturn false;\n \n \tstd::unique_ptr data = utils::make_unique(this);\n \n \t/* Locate and open the capture video node. */\n-\tdata->video_ = new V4L2Device(media_->getEntityByName(\"Raw Capture 1\"));\n+\tdata->video_ = new V4L2Device(media->getEntityByName(\"Raw Capture 1\"));\n \tif (data->video_->open())\n \t\treturn false;\n \ndiff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\nindex 43550c0e0210b7b3..7f4035c008f95f91 100644\n--- a/src/libcamera/pipeline_handler.cpp\n+++ b/src/libcamera/pipeline_handler.cpp\n@@ -9,6 +9,7 @@\n #include \n #include \n \n+#include \"device_enumerator.h\"\n #include \"log.h\"\n #include \"media_device.h\"\n #include \"pipeline_handler.h\"\n@@ -115,6 +116,8 @@ PipelineHandler::PipelineHandler(CameraManager *manager)\n \n PipelineHandler::~PipelineHandler()\n {\n+\tfor (std::shared_ptr media : mediaDevices_)\n+\t\tmedia->release();\n };\n \n /**\n@@ -148,6 +151,25 @@ PipelineHandler::~PipelineHandler()\n * created, or false otherwise\n */\n \n+std::shared_ptr\n+PipelineHandler::tryAcquire(DeviceEnumerator *enumerator, const DeviceMatch &dm)\n+{\n+\tstd::shared_ptr media;\n+\n+\tmedia = enumerator->search(dm);\n+\tif (!media)\n+\t\tgoto out;\n+\n+\tif (!media->acquire()) {\n+\t\tmedia.reset();\n+\t\tgoto out;\n+\t}\n+\n+\tmediaDevices_.push_back(media);\n+out:\n+\treturn media;\n+}\n+\n /**\n * \\fn PipelineHandler::streamConfiguration()\n * \\brief Retrieve a group of stream configurations for a specified camera\n","prefixes":["libcamera-devel","RFC","06/11"]}