{"id":1194,"url":"https://patchwork.libcamera.org/api/1.1/patches/1194/?format=json","web_url":"https://patchwork.libcamera.org/patch/1194/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/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":"<20190511091907.10050-10-niklas.soderlund@ragnatech.se>","date":"2019-05-11T09:19:05","name":"[libcamera-devel,v3,09/11] libcamera: pipeline_handler: Keep track of MediaDevice","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"5e5443e1505564e51760629fe5c98892c54efe29","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/1.1/people/5/?format=json","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/1194/mbox/","series":[{"id":303,"url":"https://patchwork.libcamera.org/api/1.1/series/303/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=303","date":"2019-05-11T09:18:56","name":"libcamerea: Add support for exclusive access to cameras between processes","version":3,"mbox":"https://patchwork.libcamera.org/series/303/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/1194/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/1194/checks/","tags":{},"headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net\n\t[195.74.38.229])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DAE5A60E6C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 11 May 2019 11:20:00 +0200 (CEST)","from localhost.localdomain (unknown [185.224.57.161])\n\tby bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA\n\tid f162c222-73cd-11e9-8e2f-005056917f90;\n\tSat, 11 May 2019 11:19:57 +0200 (CEST)"],"X-Halon-ID":"f162c222-73cd-11e9-8e2f-005056917f90","Authorized-sender":"niklas@soderlund.pp.se","From":"=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","To":"libcamera-devel@lists.libcamera.org","Date":"Sat, 11 May 2019 11:19:05 +0200","Message-Id":"<20190511091907.10050-10-niklas.soderlund@ragnatech.se>","X-Mailer":"git-send-email 2.21.0","In-Reply-To":"<20190511091907.10050-1-niklas.soderlund@ragnatech.se>","References":"<20190511091907.10050-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v3 09/11] libcamera: pipeline_handler:\n\tKeep track of MediaDevice","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Sat, 11 May 2019 09:20:01 -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\nThis also means that pipeline handler implementations do no need to keep\na shared_ptr<> reference to the media devices it stores locally as the\nPipelineHandler base class will keep a shared_ptr<> reference to all\nmedia devices consumed for the entire lifetime of the pipeline handler\nimplementation.\n\nCentrally keeping track of media devices will also be beneficial\nimplementing pipeline exclusive access across processes.\n\nSigned-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n---\n src/libcamera/include/pipeline_handler.h |  4 +++\n src/libcamera/pipeline/ipu3/ipu3.cpp     | 30 +++++----------------\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 15 ++++-------\n src/libcamera/pipeline/uvcvideo.cpp      | 24 +++++------------\n src/libcamera/pipeline/vimc.cpp          | 22 +++++-----------\n src/libcamera/pipeline_handler.cpp       | 33 ++++++++++++++++++++++++\n 6 files changed, 62 insertions(+), 66 deletions(-)","diff":"diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\nindex 5830e53108faa105..4c13496246c2251c 100644\n--- a/src/libcamera/include/pipeline_handler.h\n+++ b/src/libcamera/include/pipeline_handler.h\n@@ -22,6 +22,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@@ -53,6 +54,8 @@ public:\n \tvirtual ~PipelineHandler();\n \n \tvirtual bool match(DeviceEnumerator *enumerator) = 0;\n+\tMediaDevice *tryAcquire(DeviceEnumerator *enumerator,\n+\t\t\t\tconst DeviceMatch &dm);\n \n \tvirtual CameraConfiguration\n \tstreamConfiguration(Camera *camera, const std::vector<StreamUsage> &usages) = 0;\n@@ -84,6 +87,7 @@ private:\n \tvoid mediaDeviceDisconnected(MediaDevice *media);\n \tvirtual void disconnect();\n \n+\tstd::vector<std::shared_ptr<MediaDevice>> mediaDevices_;\n \tstd::vector<std::weak_ptr<Camera>> cameras_;\n \tstd::map<const Camera *, std::unique_ptr<CameraData>> cameraData_;\n };\ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex 8a6a0e2721955d15..f1e04db1707a0335 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -149,7 +149,6 @@ class PipelineHandlerIPU3 : public PipelineHandler\n {\n public:\n \tPipelineHandlerIPU3(CameraManager *manager);\n-\t~PipelineHandlerIPU3();\n \n \tCameraConfiguration\n \tstreamConfiguration(Camera *camera,\n@@ -201,8 +200,8 @@ private:\n \n \tImgUDevice imgu0_;\n \tImgUDevice imgu1_;\n-\tstd::shared_ptr<MediaDevice> cio2MediaDev_;\n-\tstd::shared_ptr<MediaDevice> imguMediaDev_;\n+\tMediaDevice *cio2MediaDev_;\n+\tMediaDevice *imguMediaDev_;\n };\n \n PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager)\n@@ -210,15 +209,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<StreamUsage> &usages)\n@@ -614,20 +604,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.\n@@ -682,11 +666,11 @@ int PipelineHandlerIPU3::registerCameras()\n {\n \tint ret;\n \n-\tret = imgu0_.init(imguMediaDev_.get(), 0);\n+\tret = imgu0_.init(imguMediaDev_, 0);\n \tif (ret)\n \t\treturn ret;\n \n-\tret = imgu1_.init(imguMediaDev_.get(), 1);\n+\tret = imgu1_.init(imguMediaDev_, 1);\n \tif (ret)\n \t\treturn ret;\n \n@@ -705,7 +689,7 @@ int PipelineHandlerIPU3::registerCameras()\n \t\t};\n \t\tCIO2Device *cio2 = &data->cio2_;\n \n-\t\tret = cio2->init(cio2MediaDev_.get(), id);\n+\t\tret = cio2->init(cio2MediaDev_, id);\n \t\tif (ret)\n \t\t\tcontinue;\n \ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex b94d742dd6ec33a4..761d144f514e110f 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -81,7 +81,7 @@ private:\n \tint createCamera(MediaEntity *sensor);\n \tvoid bufferReady(Buffer *buffer);\n \n-\tstd::shared_ptr<MediaDevice> media_;\n+\tMediaDevice *media_;\n \tV4L2Subdevice *dphy_;\n \tV4L2Subdevice *isp_;\n \tV4L2Device *video_;\n@@ -100,9 +100,6 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1()\n \tdelete video_;\n \tdelete isp_;\n \tdelete dphy_;\n-\n-\tif (media_)\n-\t\tmedia_->release();\n }\n \n /* -----------------------------------------------------------------------------\n@@ -355,24 +352,22 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n \tdm.add(\"rkisp1-input-params\");\n \tdm.add(\"rockchip-sy-mipi-dphy\");\n \n-\tmedia_ = enumerator->search(dm);\n+\tmedia_ = tryAcquire(enumerator, dm);\n \tif (!media_)\n \t\treturn false;\n \n-\tmedia_->acquire();\n-\n \t/* Create the V4L2 subdevices we will need. */\n-\tdphy_ = V4L2Subdevice::fromEntityName(media_.get(),\n+\tdphy_ = V4L2Subdevice::fromEntityName(media_,\n \t\t\t\t\t      \"rockchip-sy-mipi-dphy\");\n \tif (dphy_->open() < 0)\n \t\treturn false;\n \n-\tisp_ = V4L2Subdevice::fromEntityName(media_.get(), \"rkisp1-isp-subdev\");\n+\tisp_ = V4L2Subdevice::fromEntityName(media_, \"rkisp1-isp-subdev\");\n \tif (isp_->open() < 0)\n \t\treturn false;\n \n \t/* Locate and open the capture video node. */\n-\tvideo_ = V4L2Device::fromEntityName(media_.get(), \"rkisp1_mainpath\");\n+\tvideo_ = V4L2Device::fromEntityName(media_, \"rkisp1_mainpath\");\n \tif (video_->open() < 0)\n \t\treturn false;\n \ndiff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\nindex e40b052f4d877d5d..bc4b4ec76a2d869c 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@@ -70,20 +69,13 @@ private:\n \t\t\tPipelineHandler::cameraData(camera));\n \t}\n \n-\tstd::shared_ptr<MediaDevice> 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<StreamUsage> &usages)\n@@ -177,19 +169,17 @@ int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request)\n \n bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n {\n+\tMediaDevice *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<UVCCameraData> data = utils::make_unique<UVCCameraData>(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@@ -208,11 +198,11 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n \n \t/* Create and register the camera. */\n \tstd::set<Stream *> streams{ &data->stream_ };\n-\tstd::shared_ptr<Camera> camera = Camera::create(this, media_->model(), streams);\n+\tstd::shared_ptr<Camera> 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);\n \n \treturn true;\n }\ndiff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\nindex 7b6ebd4cc0a27e25..730a4c2829c10efe 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@@ -69,21 +68,13 @@ private:\n \t\treturn static_cast<VimcCameraData *>(\n \t\t\tPipelineHandler::cameraData(camera));\n \t}\n-\n-\tstd::shared_ptr<MediaDevice> 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<StreamUsage> &usages)\n@@ -177,6 +168,8 @@ int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request)\n \n bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n {\n+\tMediaDevice *media;\n+\n \tDeviceMatch dm(\"vimc\");\n \n \tdm.add(\"Raw Capture 0\");\n@@ -189,17 +182,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<VimcCameraData> data = utils::make_unique<VimcCameraData>(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 4ecd6c49efd8ca89..8f50ef51f0c23301 100644\n--- a/src/libcamera/pipeline_handler.cpp\n+++ b/src/libcamera/pipeline_handler.cpp\n@@ -11,6 +11,7 @@\n #include <libcamera/camera.h>\n #include <libcamera/camera_manager.h>\n \n+#include \"device_enumerator.h\"\n #include \"log.h\"\n #include \"media_device.h\"\n #include \"utils.h\"\n@@ -116,6 +117,8 @@ PipelineHandler::PipelineHandler(CameraManager *manager)\n \n PipelineHandler::~PipelineHandler()\n {\n+\tfor (std::shared_ptr<MediaDevice> media : mediaDevices_)\n+\t\tmedia->release();\n };\n \n /**\n@@ -149,6 +152,36 @@ PipelineHandler::~PipelineHandler()\n  * created, or false otherwise\n  */\n \n+/**\n+ * \\brief Try to acquire a MediaDevice from a device pattern\n+ * \\param[in] enumerator Enumerator containing all media devices in the system\n+ * \\param[in] dm Device match pattern\n+ *\n+ * Search in the enumerated media devices that are not already in use for a\n+ * match described in \\a dm. If a match is found acquire it and store it for\n+ * release when the pipeline handler is destroyed.\n+ *\n+ * \\return pointer to the matching MediaDevice, or nullptr if no match is found\n+ */\n+MediaDevice *PipelineHandler::tryAcquire(DeviceEnumerator *enumerator,\n+\t\t\t\t\t const DeviceMatch &dm)\n+{\n+\tstd::shared_ptr<MediaDevice> 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.get();\n+}\n+\n /**\n  * \\fn PipelineHandler::streamConfiguration()\n  * \\brief Retrieve a group of stream configurations for a specified camera\n","prefixes":["libcamera-devel","v3","09/11"]}