Patch Detail
Show a patch.
GET /api/patches/1137/?format=api
{ "id": 1137, "url": "https://patchwork.libcamera.org/api/patches/1137/?format=api", "web_url": "https://patchwork.libcamera.org/patch/1137/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20190429191729.29697-10-niklas.soderlund@ragnatech.se>", "date": "2019-04-29T19:17:27", "name": "[libcamera-devel,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/people/5/?format=api", "name": "Niklas Söderlund", "email": "niklas.soderlund@ragnatech.se" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/1137/mbox/", "series": [ { "id": 284, "url": "https://patchwork.libcamera.org/api/series/284/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=284", "date": "2019-04-29T19:17:18", "name": "libcamerea: Add support for exclusive access to cameras between processes", "version": 1, "mbox": "https://patchwork.libcamera.org/series/284/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/1137/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/1137/checks/", "tags": {}, "headers": { "Return-Path": "<niklas.soderlund@ragnatech.se>", "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 2E298600EC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 29 Apr 2019 21:17:54 +0200 (CEST)", "from wyvern.station (unknown [37.182.44.227])\n\tby bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA\n\tid 7c18d849-6ab3-11e9-8e2c-005056917f90;\n\tMon, 29 Apr 2019 21:17:52 +0200 (CEST)" ], "X-Halon-ID": "7c18d849-6ab3-11e9-8e2c-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": "Mon, 29 Apr 2019 21:17:27 +0200", "Message-Id": "<20190429191729.29697-10-niklas.soderlund@ragnatech.se>", "X-Mailer": "git-send-email 2.21.0", "In-Reply-To": "<20190429191729.29697-1-niklas.soderlund@ragnatech.se>", "References": "<20190429191729.29697-1-niklas.soderlund@ragnatech.se>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=UTF-8", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH 09/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": "<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": "Mon, 29 Apr 2019 19:17:54 -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 c18bb0ec9fa6ba8a..4e3b210530ebd92d 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@@ -617,20 +607,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@@ -685,11 +669,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@@ -708,7 +692,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 b720abaa6043a95c..64cbe5d2eb594672 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@@ -359,24 +356,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 d7ef37a8171f0eac..c6ffa0e6b2aa9c77 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@@ -183,19 +175,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@@ -214,11 +204,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 25f0802c30d3d07b..03898f935925b006 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@@ -183,6 +174,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@@ -195,17 +188,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 345abca89ab83590..702b398a50a18085 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", "09/11" ] }