[{"id":27275,"web_url":"https://patchwork.libcamera.org/comment/27275/","msgid":"<168c-647f1f00-b-2235d640@253258127>","date":"2023-06-06T11:56:22","subject":"Re: [libcamera-devel] =?utf-8?q?=5BPATCH_v3_5/5=5D_libcamera=3A_?=\n\t=?utf-8?q?pipeline=3A_Register_device_numbers_with_camera?=","submitter":{"id":165,"url":"https://patchwork.libcamera.org/api/people/165/","name":"Ashok Sidipotu","email":"ashok.sidipotu@collabora.com"},"content":"Tested-by: Ashok Sidipotu <ashok.sidipotu@collabora.com>\n\nOn Monday, May 15, 2023 18:15 IST, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote:\n Register the identified device numbers with each camera as the Devices\nproperty.\n\nThis facilitates camera daemons or other systems to identify which\ndevices are being managed by libcamera, and can prevent duplication of\ncamera resources.\n\nAs the Devices property now provides this list of devices, use it\ndirectly from within the CameraManager when adding a Camera rather than\npassing it through the internal API.\n\nSigned-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n---\ninclude/libcamera/internal/camera_manager.h | 3 +--\nsrc/libcamera/camera_manager.cpp | 14 +++++++++-----\nsrc/libcamera/pipeline_handler.cpp | 12 ++++++++++--\n3 files changed, 20 insertions(+), 9 deletions(-)\n\ndiff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h\nindex 885bb2825687..680e1f58cb02 100644\n--- a/include/libcamera/internal/camera_manager.h\n+++ b/include/libcamera/internal/camera_manager.h\n@@ -29,8 +29,7 @@ public:\nPrivate();\n\nint start();\n- void addCamera(std::shared_ptr<Camera> camera,\n- const std::vector<dev_t> &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);\n+ void addCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\nvoid removeCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n\n/*\ndiff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\nindex 70eb4e455e54..7e499def9ddc 100644\n--- a/src/libcamera/camera_manager.cpp\n+++ b/src/libcamera/camera_manager.cpp\n@@ -15,7 +15,9 @@\n#include <libcamera/base/utils.h>\n\n#include <libcamera/camera.h>\n+#include <libcamera/property_ids.h>\n\n+#include \"libcamera/internal/camera.h\"\n#include \"libcamera/internal/device_enumerator.h\"\n#include \"libcamera/internal/pipeline_handler.h\"\n\n@@ -150,19 +152,17 @@ void CameraManager::Private::cleanup()\n/**\n* \\brief Add a camera to the camera manager\n* \\param[in] camera The camera to be added\n- * \\param[in] devnums The device numbers to associate with \\a camera\n*\n* This function is called by pipeline handlers to register the cameras they\n* handle with the camera manager. Registered cameras are immediately made\n* available to the system.\n*\n- * \\a devnums are used by the V4L2 compatibility layer to map V4L2 device nodes\n- * to Camera instances.\n+ * Device numbers from the Devices property are used by the V4L2 compatibility\n+ * layer to map V4L2 device nodes to Camera instances.\n*\n* \\context This function shall be called from the CameraManager thread.\n*/\n-void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,\n- const std::vector<dev_t> &devnums)\n+void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera)\n{\nASSERT(Thread::current() == this);\n\n@@ -177,6 +177,10 @@ void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,\n}\n}\n\n+ auto devnums = camera->properties()\n+ .get(properties::Devices)\n+ .value_or(Span<int64_t>{});\n+\ncameras_.push_back(std::move(camera));\n\nunsigned int index = cameras_.size() - 1;\ndiff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\nindex 49092ea88a58..0b5fac7ad113 100644\n--- a/src/libcamera/pipeline_handler.cpp\n+++ b/src/libcamera/pipeline_handler.cpp\n@@ -17,6 +17,7 @@\n\n#include <libcamera/camera.h>\n#include <libcamera/framebuffer.h>\n+#include <libcamera/property_ids.h>\n\n#include \"libcamera/internal/camera.h\"\n#include \"libcamera/internal/camera_manager.h\"\n@@ -612,7 +613,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)\n* Walk the entity list and map the devnums of all capture video nodes\n* to the camera.\n*/\n- std::vector<dev_t> devnums;\n+ std::vector<int64_t> devnums;\nfor (const std::shared_ptr<MediaDevice> &media : mediaDevices_) {\nfor (const MediaEntity *entity : media->entities()) {\nif (entity->pads().size() == 1 &&\n@@ -624,7 +625,14 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)\n}\n}\n\n- manager_->_d()->addCamera(std::move(camera), devnums);\n+ /*\n+ * Store the associated devices as a property of the camera to allow\n+ * systems to identify which devices are managed by libcamera.\n+ */\n+ Camera::Private *data = camera->_d();\n+ data->properties_.set(properties::Devices, devnums);\n+\n+ manager_->_d()->addCamera(std::move(camera));\n}\n\n/**\n--\n2.34.1","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 01FFDC31E9\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  6 Jun 2023 12:03:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BEFBC628A6;\n\tTue,  6 Jun 2023 14:03:30 +0200 (CEST)","from madras.collabora.co.uk (madras.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e5ab])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B104862893\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  6 Jun 2023 13:56:22 +0200 (CEST)","from hamburger.collabora.co.uk (hamburger.collabora.co.uk\n\t[IPv6:2a01:4f8:1c1c:f269::1])\n\tby madras.collabora.co.uk (Postfix) with ESMTP id 6D8E86606EAD;\n\tTue,  6 Jun 2023 12:56:22 +0100 (BST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686053010;\n\tbh=mBGdkbM7nx0xl+XDlCUdTPwLNlgZQLKtZTYIULJmInY=;\n\th=In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:\n\tList-Post:List-Help:List-Subscribe:From:Reply-To:Cc:From;\n\tb=XQ33FzqGHJozngVRm1JbWrxmq7m6ePynHp4Dv06GDNvatQBkhH2MIoKL8/fMQCfso\n\tNfvpUnogARzL3rlp+3Ey0DKQeCCDGSw0an5Nck4/iY3Ms/qNe3jFkiDxA+GMRIRvcf\n\tKwauEJiPz/LIDX+yxVK8wCII5YrYvuWSPwUvK1X698R0+tU2XoA9P3eTeUAuLTxmxU\n\tPaBKIwV1vNMcg5I7AxzqoVTGqlAyq83AySSRSKN3BiiW591RdiERdM5TGjnMOyuVrZ\n\t7JjoSYiEKGYWmHp8JjFwaQ2NwYPso2vv3W5B4fM6j6g/677acWi4VXu5DArNfLiklY\n\tPGSwPq/ymYBRg==","In-Reply-To":"<20230515124550.3601128-6-kieran.bingham@ideasonboard.com>","Content-Type":"multipart/alternative;\n\tboundary=\"----=_=-_OpenGroupware_org_NGMime-5772-1686052582.191103-3------\"","X-Forward":"127.0.0.1","Date":"Tue, 06 Jun 2023 12:56:22 +0100","To":"\"Kieran Bingham\" <kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Message-ID":"<168c-647f1f00-b-2235d640@253258127>","User-Agent":"SOGoMail 5.8.0","X-Mailman-Approved-At":"Tue, 06 Jun 2023 14:03:27 +0200","Subject":"Re: [libcamera-devel] =?utf-8?q?=5BPATCH_v3_5/5=5D_libcamera=3A_?=\n\t=?utf-8?q?pipeline=3A_Register_device_numbers_with_camera?=","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","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>","From":"Ashok Sidipotu via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Ashok Sidipotu <ashok.sidipotu@collabora.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27291,"web_url":"https://patchwork.libcamera.org/comment/27291/","msgid":"<20230606162736.GG25679@pendragon.ideasonboard.com>","date":"2023-06-06T16:27:36","subject":"Re: [libcamera-devel] [PATCH v3 5/5] libcamera: pipeline: Register\n\tdevice numbers with camera","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Mon, May 15, 2023 at 01:45:50PM +0100, Kieran Bingham via libcamera-devel wrote:\n> Register the identified device numbers with each camera as the Devices\n> property.\n\nNow that I read this, I wonder if we should call the property\nSystemDevices. It seems clearer to me but I'm not sure why :-)\n\n> This facilitates camera daemons or other systems to identify which\n> devices are being managed by libcamera, and can prevent duplication of\n> camera resources.\n\nIs it really duplication of resources, or resource usage conflicts ?\n\n> As the Devices property now provides this list of devices, use it\n> directly from within the CameraManager when adding a Camera rather than\n> passing it through the internal API.\n> \n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> ---\n>  include/libcamera/internal/camera_manager.h |  3 +--\n>  src/libcamera/camera_manager.cpp            | 14 +++++++++-----\n>  src/libcamera/pipeline_handler.cpp          | 12 ++++++++++--\n>  3 files changed, 20 insertions(+), 9 deletions(-)\n> \n> diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h\n> index 885bb2825687..680e1f58cb02 100644\n> --- a/include/libcamera/internal/camera_manager.h\n> +++ b/include/libcamera/internal/camera_manager.h\n> @@ -29,8 +29,7 @@ public:\n>  \tPrivate();\n>  \n>  \tint start();\n> -\tvoid addCamera(std::shared_ptr<Camera> camera,\n> -\t\t       const std::vector<dev_t> &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> +\tvoid addCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n>  \tvoid removeCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n>  \n>  \t/*\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index 70eb4e455e54..7e499def9ddc 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -15,7 +15,9 @@\n>  #include <libcamera/base/utils.h>\n>  \n>  #include <libcamera/camera.h>\n> +#include <libcamera/property_ids.h>\n>  \n> +#include \"libcamera/internal/camera.h\"\n>  #include \"libcamera/internal/device_enumerator.h\"\n>  #include \"libcamera/internal/pipeline_handler.h\"\n>  \n> @@ -150,19 +152,17 @@ void CameraManager::Private::cleanup()\n>  /**\n>   * \\brief Add a camera to the camera manager\n>   * \\param[in] camera The camera to be added\n> - * \\param[in] devnums The device numbers to associate with \\a camera\n>   *\n>   * This function is called by pipeline handlers to register the cameras they\n>   * handle with the camera manager. Registered cameras are immediately made\n>   * available to the system.\n>   *\n> - * \\a devnums are used by the V4L2 compatibility layer to map V4L2 device nodes\n> - * to Camera instances.\n> + * Device numbers from the Devices property are used by the V4L2 compatibility\n> + * layer to map V4L2 device nodes to Camera instances.\n>   *\n>   * \\context This function shall be called from the CameraManager thread.\n>   */\n> -void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,\n> -\t\t\t\t       const std::vector<dev_t> &devnums)\n> +void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera)\n>  {\n>  \tASSERT(Thread::current() == this);\n>  \n> @@ -177,6 +177,10 @@ void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,\n>  \t\t}\n>  \t}\n>  \n> +\tauto devnums = camera->properties()\n> +\t\t\t       .get(properties::Devices)\n> +\t\t\t       .value_or(Span<int64_t>{});\n> +\n>  \tcameras_.push_back(std::move(camera));\n>  \n>  \tunsigned int index = cameras_.size() - 1;\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 49092ea88a58..0b5fac7ad113 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -17,6 +17,7 @@\n>  \n>  #include <libcamera/camera.h>\n>  #include <libcamera/framebuffer.h>\n> +#include <libcamera/property_ids.h>\n>  \n>  #include \"libcamera/internal/camera.h\"\n>  #include \"libcamera/internal/camera_manager.h\"\n> @@ -612,7 +613,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)\n>  \t * Walk the entity list and map the devnums of all capture video nodes\n>  \t * to the camera.\n>  \t */\n> -\tstd::vector<dev_t> devnums;\n> +\tstd::vector<int64_t> devnums;\n>  \tfor (const std::shared_ptr<MediaDevice> &media : mediaDevices_) {\n>  \t\tfor (const MediaEntity *entity : media->entities()) {\n>  \t\t\tif (entity->pads().size() == 1 &&\n> @@ -624,7 +625,14 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)\n>  \t\t}\n>  \t}\n>  \n> -\tmanager_->_d()->addCamera(std::move(camera), devnums);\n> +\t/*\n> +\t * Store the associated devices as a property of the camera to allow\n> +\t * systems to identify which devices are managed by libcamera.\n> +\t */\n> +\tCamera::Private *data = camera->_d();\n> +\tdata->properties_.set(properties::Devices, devnums);\n> +\n> +\tmanager_->_d()->addCamera(std::move(camera));\n>  }\n>  \n>  /**","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 42C09C3200\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  6 Jun 2023 16:27:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8842262722;\n\tTue,  6 Jun 2023 18:27:40 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4E8E862709\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  6 Jun 2023 18:27:39 +0200 (CEST)","from pendragon.ideasonboard.com (om126253223039.31.openmobile.ne.jp\n\t[126.253.223.39])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B7E6C283;\n\tTue,  6 Jun 2023 18:27:12 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686068860;\n\tbh=nad+nObDlPQFMQ1aIsqzWiHjrfd4aKnmxpB88Aq7CII=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=cjA8qDMXfh1I7MVAnhLeaC6/IT1vfZv7IQfTyP2BW8EPArQJrR7SqD9of/A+yOSGK\n\t4vaTnmskQw6LMTJoeZ9lEutHcl23cDxIC+XSi3sGIrJrJ2T+Yad9X34UbpimZ3Twwh\n\tnqlSv/ztbqBEt8qi4xbQnI/3FIAeBBZACEWFbGLRWs2kQ+s3yv8NdTw3Dce4FNZa8A\n\tQcUwwyJOruUTuqKDzaBzvB9UgqK4MJ1posyiiZJ85Nxd1BGYTmZSz+kkMEg/Ogy0xK\n\tSCCsxF4eNGwOW3KOmgkbFErphp0FACdmqWVGHjfJcif8RWLARfGOuAwkAyYOeT/o+i\n\tdTyT2UV7xg2Og==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686068833;\n\tbh=nad+nObDlPQFMQ1aIsqzWiHjrfd4aKnmxpB88Aq7CII=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=tj+a1pR2bbRGUxEnGVo1KFFfUBVbsHqWRByS6QSPni6wv5UaGoWLh7PTYswWZglGB\n\tHqM8ldPCc/SB1DpZJefxg5l+yISfocJMKQdyKMMWA+JgBp2ckKxQf/amA+NsB0vUKq\n\tvSZfEOpxqWuqsienEJq7l3LkSo+D5J/+gi0VPWuc="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"tj+a1pR2\"; dkim-atps=neutral","Date":"Tue, 6 Jun 2023 19:27:36 +0300","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20230606162736.GG25679@pendragon.ideasonboard.com>","References":"<20230515124550.3601128-1-kieran.bingham@ideasonboard.com>\n\t<20230515124550.3601128-6-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230515124550.3601128-6-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 5/5] libcamera: pipeline: Register\n\tdevice numbers with camera","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","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>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27302,"web_url":"https://patchwork.libcamera.org/comment/27302/","msgid":"<168614782811.2889415.9195196058890174630@Monstersaurus>","date":"2023-06-07T14:23:48","subject":"Re: [libcamera-devel] [PATCH v3 5/5] libcamera: pipeline: Register\n\tdevice numbers with camera","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2023-06-06 17:27:36)\n> Hi Kieran,\n> \n> Thank you for the patch.\n> \n> On Mon, May 15, 2023 at 01:45:50PM +0100, Kieran Bingham via libcamera-devel wrote:\n> > Register the identified device numbers with each camera as the Devices\n> > property.\n> \n> Now that I read this, I wonder if we should call the property\n> SystemDevices. It seems clearer to me but I'm not sure why :-)\n\nman dev_t:\n\ndev_t\nInclude: <sys/types.h>.  Alternatively, <sys/stat.h>.\n\nUsed for device IDs.  According to POSIX, it shall be an integer type.\nFor further details of this type, see makedev(3).\n\nConforming to: POSIX.1-2001 and later.\n\nSee also: mknod(2), stat(2)\n\n\nor\n\nman makedev:\n\nNAME\n       makedev, major, minor - manage a device number\nDESCRIPTION\n\n       A  device ID consists of two parts: a major ID, identifying the\n       class of the device, and a minor ID, identifying a specific\n       instance of a device in that class.  A device ID is represented\n       using the type dev_t.\n\n       Given major and minor device IDs, makedev() combines these to\n       produce a device ID, returned as the function result.  This\n       device ID can be given to mknod(2), for example.\n\n       The major() and minor() functions perform the converse task:\n       given a device ID, they return, respectively, the major and minor\n       components.  These macros can be useful to,  for  example,\n       decompose the device IDs in the structure returned by stat(2).\n\nThe value we store is only ever referenced as a 'device number'. They\nhelp identify device nodes. They are part of the 'system' but then so\nis everything else.\n\nIt may help if it clarifies these are 'internal (linux) system devices'\nbut what is a device in a linux system if it's not a system device?\n\nIOW : If you want to ask for this to be SystemDevices, then that's fine.\n- but it doesn't add anything to me..\n\n\n> \n> > This facilitates camera daemons or other systems to identify which\n> > devices are being managed by libcamera, and can prevent duplication of\n> > camera resources.\n> \n> Is it really duplication of resources, or resource usage conflicts ?\n\n\nWell maybe it's both. It's means that resources are duplicated to the\nuser. They don't conflict unless they are both accessed at the same\ntime.\n\n\n\n\n> > As the Devices property now provides this list of devices, use it\n> > directly from within the CameraManager when adding a Camera rather than\n> > passing it through the internal API.\n> > \n> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> \n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> > ---\n> >  include/libcamera/internal/camera_manager.h |  3 +--\n> >  src/libcamera/camera_manager.cpp            | 14 +++++++++-----\n> >  src/libcamera/pipeline_handler.cpp          | 12 ++++++++++--\n> >  3 files changed, 20 insertions(+), 9 deletions(-)\n> > \n> > diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h\n> > index 885bb2825687..680e1f58cb02 100644\n> > --- a/include/libcamera/internal/camera_manager.h\n> > +++ b/include/libcamera/internal/camera_manager.h\n> > @@ -29,8 +29,7 @@ public:\n> >       Private();\n> >  \n> >       int start();\n> > -     void addCamera(std::shared_ptr<Camera> camera,\n> > -                    const std::vector<dev_t> &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> > +     void addCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> >       void removeCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> >  \n> >       /*\n> > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > index 70eb4e455e54..7e499def9ddc 100644\n> > --- a/src/libcamera/camera_manager.cpp\n> > +++ b/src/libcamera/camera_manager.cpp\n> > @@ -15,7 +15,9 @@\n> >  #include <libcamera/base/utils.h>\n> >  \n> >  #include <libcamera/camera.h>\n> > +#include <libcamera/property_ids.h>\n> >  \n> > +#include \"libcamera/internal/camera.h\"\n> >  #include \"libcamera/internal/device_enumerator.h\"\n> >  #include \"libcamera/internal/pipeline_handler.h\"\n> >  \n> > @@ -150,19 +152,17 @@ void CameraManager::Private::cleanup()\n> >  /**\n> >   * \\brief Add a camera to the camera manager\n> >   * \\param[in] camera The camera to be added\n> > - * \\param[in] devnums The device numbers to associate with \\a camera\n> >   *\n> >   * This function is called by pipeline handlers to register the cameras they\n> >   * handle with the camera manager. Registered cameras are immediately made\n> >   * available to the system.\n> >   *\n> > - * \\a devnums are used by the V4L2 compatibility layer to map V4L2 device nodes\n> > - * to Camera instances.\n> > + * Device numbers from the Devices property are used by the V4L2 compatibility\n> > + * layer to map V4L2 device nodes to Camera instances.\n> >   *\n> >   * \\context This function shall be called from the CameraManager thread.\n> >   */\n> > -void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,\n> > -                                    const std::vector<dev_t> &devnums)\n> > +void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera)\n> >  {\n> >       ASSERT(Thread::current() == this);\n> >  \n> > @@ -177,6 +177,10 @@ void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,\n> >               }\n> >       }\n> >  \n> > +     auto devnums = camera->properties()\n> > +                            .get(properties::Devices)\n> > +                            .value_or(Span<int64_t>{});\n> > +\n> >       cameras_.push_back(std::move(camera));\n> >  \n> >       unsigned int index = cameras_.size() - 1;\n> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > index 49092ea88a58..0b5fac7ad113 100644\n> > --- a/src/libcamera/pipeline_handler.cpp\n> > +++ b/src/libcamera/pipeline_handler.cpp\n> > @@ -17,6 +17,7 @@\n> >  \n> >  #include <libcamera/camera.h>\n> >  #include <libcamera/framebuffer.h>\n> > +#include <libcamera/property_ids.h>\n> >  \n> >  #include \"libcamera/internal/camera.h\"\n> >  #include \"libcamera/internal/camera_manager.h\"\n> > @@ -612,7 +613,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)\n> >        * Walk the entity list and map the devnums of all capture video nodes\n> >        * to the camera.\n> >        */\n> > -     std::vector<dev_t> devnums;\n> > +     std::vector<int64_t> devnums;\n> >       for (const std::shared_ptr<MediaDevice> &media : mediaDevices_) {\n> >               for (const MediaEntity *entity : media->entities()) {\n> >                       if (entity->pads().size() == 1 &&\n> > @@ -624,7 +625,14 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)\n> >               }\n> >       }\n> >  \n> > -     manager_->_d()->addCamera(std::move(camera), devnums);\n> > +     /*\n> > +      * Store the associated devices as a property of the camera to allow\n> > +      * systems to identify which devices are managed by libcamera.\n> > +      */\n> > +     Camera::Private *data = camera->_d();\n> > +     data->properties_.set(properties::Devices, devnums);\n> > +\n> > +     manager_->_d()->addCamera(std::move(camera));\n> >  }\n> >  \n> >  /**\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 25A1CC31E9\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  7 Jun 2023 14:23:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 91D6762886;\n\tWed,  7 Jun 2023 16:23:52 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 559F660579\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  7 Jun 2023 16:23:51 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1E48E74C;\n\tWed,  7 Jun 2023 16:23:25 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686147832;\n\tbh=ilc9VMh8qBwxkqIIfl+GGfaKeHVY+DQ1rif5dzKvk7Y=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=OrKlWtYKJIQ6sa9AnCxy+Jg/TdvnXL+Xs1/MhnGLdbcNWzzl59/ZgA9QALul27JAy\n\tr0LJ41ZV8yFn1ttEQwHA+6l4ExWKlWJDI9A4ECFI4a8fVKUhCNfGqTemvd4+uvGIzu\n\tW+ArtwJx1QtF4NoJ6KxuCq8WNvLyAT/go9llbFxidKY8iecMz6JT5FshbHseZeyD+b\n\tzy60TsSV6BzaAVy8gvyoj2G/MeMBcwHxQ+pok5xIhHKJeM0lC21Wwmg4TKew6nLrhb\n\tYApGQJHuOB9erzSTn21CdyI2LDZdfarqpuVpBP4Vk+vuvRZfysgqbqeHLlJZYgJMRR\n\tM59+KKlVQmxWQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686147805;\n\tbh=ilc9VMh8qBwxkqIIfl+GGfaKeHVY+DQ1rif5dzKvk7Y=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=qHPagRCH8eoT0aP6NSBDG9B6DCE79tdG+y6zWRaKe13QBua2eE44WplGx3MCW5gHJ\n\tMayw1jNzkFCDxoHTG8fYgymEUhCQAFEnSw65bjgX4zUYrQhw3wUKZF93bZJ9S6fNv8\n\tgYn++asgwm2N24kB7wLLHZizMES1JrCj/h1s1u/c="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"qHPagRCH\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230606162736.GG25679@pendragon.ideasonboard.com>","References":"<20230515124550.3601128-1-kieran.bingham@ideasonboard.com>\n\t<20230515124550.3601128-6-kieran.bingham@ideasonboard.com>\n\t<20230606162736.GG25679@pendragon.ideasonboard.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Date":"Wed, 07 Jun 2023 15:23:48 +0100","Message-ID":"<168614782811.2889415.9195196058890174630@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v3 5/5] libcamera: pipeline: Register\n\tdevice numbers with camera","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","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>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27308,"web_url":"https://patchwork.libcamera.org/comment/27308/","msgid":"<20230607185304.GC26742@pendragon.ideasonboard.com>","date":"2023-06-07T18:53:04","subject":"Re: [libcamera-devel] [PATCH v3 5/5] libcamera: pipeline: Register\n\tdevice numbers with camera","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Wed, Jun 07, 2023 at 03:23:48PM +0100, Kieran Bingham wrote:\n> Quoting Laurent Pinchart (2023-06-06 17:27:36)\n> > On Mon, May 15, 2023 at 01:45:50PM +0100, Kieran Bingham via libcamera-devel wrote:\n> > > Register the identified device numbers with each camera as the Devices\n> > > property.\n> > \n> > Now that I read this, I wonder if we should call the property\n> > SystemDevices. It seems clearer to me but I'm not sure why :-)\n> \n> man dev_t:\n> \n> dev_t\n> Include: <sys/types.h>.  Alternatively, <sys/stat.h>.\n> \n> Used for device IDs.  According to POSIX, it shall be an integer type.\n> For further details of this type, see makedev(3).\n> \n> Conforming to: POSIX.1-2001 and later.\n> \n> See also: mknod(2), stat(2)\n> \n> \n> or\n> \n> man makedev:\n> \n> NAME\n>        makedev, major, minor - manage a device number\n> DESCRIPTION\n> \n>        A  device ID consists of two parts: a major ID, identifying the\n>        class of the device, and a minor ID, identifying a specific\n>        instance of a device in that class.  A device ID is represented\n>        using the type dev_t.\n> \n>        Given major and minor device IDs, makedev() combines these to\n>        produce a device ID, returned as the function result.  This\n>        device ID can be given to mknod(2), for example.\n> \n>        The major() and minor() functions perform the converse task:\n>        given a device ID, they return, respectively, the major and minor\n>        components.  These macros can be useful to,  for  example,\n>        decompose the device IDs in the structure returned by stat(2).\n> \n> The value we store is only ever referenced as a 'device number'. They\n> help identify device nodes. They are part of the 'system' but then so\n> is everything else.\n> \n> It may help if it clarifies these are 'internal (linux) system devices'\n> but what is a device in a linux system if it's not a system device?\n> \n> IOW : If you want to ask for this to be SystemDevices, then that's fine.\n> - but it doesn't add anything to me..\n\n\"device\" is one of those terms that I really dislike when unqualified,\nas it's very ambiguous. A camera is a device. A chip in the camera\nhardware pipeline is a device. A DT node is a device, and it results in\na device being created in the kernel. This in turn causes yet another\ntype of devices to be exposed to userspace through a device node.\n\nTry to forget most of the things you know about the kernel internals,\nand think about how an application developer who has no idea how the\nkernel and UAPIs work will guess what the property exposes just based on\nits name. Then add the property description. It's totally understandable\nto someone who already knows what this is about, but less so for other\nreaders.\n\nIt's not our job to turn the libcamera documentation into a Linux 101,\nbut I think the current property name and short documentation could be\nimproved.\n\n> > > This facilitates camera daemons or other systems to identify which\n> > > devices are being managed by libcamera, and can prevent duplication of\n> > > camera resources.\n> > \n> > Is it really duplication of resources, or resource usage conflicts ?\n> \n> Well maybe it's both. It's means that resources are duplicated to the\n> user. They don't conflict unless they are both accessed at the same\n> time.\n\nThis is \"just\" the commit message so I won't bikeshed too much at this\ntime of the night :-)\n\n> > > As the Devices property now provides this list of devices, use it\n> > > directly from within the CameraManager when adding a Camera rather than\n> > > passing it through the internal API.\n> > > \n> > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > \n> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > \n> > > ---\n> > >  include/libcamera/internal/camera_manager.h |  3 +--\n> > >  src/libcamera/camera_manager.cpp            | 14 +++++++++-----\n> > >  src/libcamera/pipeline_handler.cpp          | 12 ++++++++++--\n> > >  3 files changed, 20 insertions(+), 9 deletions(-)\n> > > \n> > > diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h\n> > > index 885bb2825687..680e1f58cb02 100644\n> > > --- a/include/libcamera/internal/camera_manager.h\n> > > +++ b/include/libcamera/internal/camera_manager.h\n> > > @@ -29,8 +29,7 @@ public:\n> > >       Private();\n> > >  \n> > >       int start();\n> > > -     void addCamera(std::shared_ptr<Camera> camera,\n> > > -                    const std::vector<dev_t> &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> > > +     void addCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> > >       void removeCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> > >  \n> > >       /*\n> > > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > > index 70eb4e455e54..7e499def9ddc 100644\n> > > --- a/src/libcamera/camera_manager.cpp\n> > > +++ b/src/libcamera/camera_manager.cpp\n> > > @@ -15,7 +15,9 @@\n> > >  #include <libcamera/base/utils.h>\n> > >  \n> > >  #include <libcamera/camera.h>\n> > > +#include <libcamera/property_ids.h>\n> > >  \n> > > +#include \"libcamera/internal/camera.h\"\n> > >  #include \"libcamera/internal/device_enumerator.h\"\n> > >  #include \"libcamera/internal/pipeline_handler.h\"\n> > >  \n> > > @@ -150,19 +152,17 @@ void CameraManager::Private::cleanup()\n> > >  /**\n> > >   * \\brief Add a camera to the camera manager\n> > >   * \\param[in] camera The camera to be added\n> > > - * \\param[in] devnums The device numbers to associate with \\a camera\n> > >   *\n> > >   * This function is called by pipeline handlers to register the cameras they\n> > >   * handle with the camera manager. Registered cameras are immediately made\n> > >   * available to the system.\n> > >   *\n> > > - * \\a devnums are used by the V4L2 compatibility layer to map V4L2 device nodes\n> > > - * to Camera instances.\n> > > + * Device numbers from the Devices property are used by the V4L2 compatibility\n> > > + * layer to map V4L2 device nodes to Camera instances.\n> > >   *\n> > >   * \\context This function shall be called from the CameraManager thread.\n> > >   */\n> > > -void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,\n> > > -                                    const std::vector<dev_t> &devnums)\n> > > +void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera)\n> > >  {\n> > >       ASSERT(Thread::current() == this);\n> > >  \n> > > @@ -177,6 +177,10 @@ void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,\n> > >               }\n> > >       }\n> > >  \n> > > +     auto devnums = camera->properties()\n> > > +                            .get(properties::Devices)\n> > > +                            .value_or(Span<int64_t>{});\n> > > +\n> > >       cameras_.push_back(std::move(camera));\n> > >  \n> > >       unsigned int index = cameras_.size() - 1;\n> > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > > index 49092ea88a58..0b5fac7ad113 100644\n> > > --- a/src/libcamera/pipeline_handler.cpp\n> > > +++ b/src/libcamera/pipeline_handler.cpp\n> > > @@ -17,6 +17,7 @@\n> > >  \n> > >  #include <libcamera/camera.h>\n> > >  #include <libcamera/framebuffer.h>\n> > > +#include <libcamera/property_ids.h>\n> > >  \n> > >  #include \"libcamera/internal/camera.h\"\n> > >  #include \"libcamera/internal/camera_manager.h\"\n> > > @@ -612,7 +613,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)\n> > >        * Walk the entity list and map the devnums of all capture video nodes\n> > >        * to the camera.\n> > >        */\n> > > -     std::vector<dev_t> devnums;\n> > > +     std::vector<int64_t> devnums;\n> > >       for (const std::shared_ptr<MediaDevice> &media : mediaDevices_) {\n> > >               for (const MediaEntity *entity : media->entities()) {\n> > >                       if (entity->pads().size() == 1 &&\n> > > @@ -624,7 +625,14 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)\n> > >               }\n> > >       }\n> > >  \n> > > -     manager_->_d()->addCamera(std::move(camera), devnums);\n> > > +     /*\n> > > +      * Store the associated devices as a property of the camera to allow\n> > > +      * systems to identify which devices are managed by libcamera.\n> > > +      */\n> > > +     Camera::Private *data = camera->_d();\n> > > +     data->properties_.set(properties::Devices, devnums);\n> > > +\n> > > +     manager_->_d()->addCamera(std::move(camera));\n> > >  }\n> > >  \n> > >  /**","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id BFEFAC3200\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  7 Jun 2023 18:53:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 00E54609FF;\n\tWed,  7 Jun 2023 20:53:10 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F2641609FF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  7 Jun 2023 20:53:07 +0200 (CEST)","from pendragon.ideasonboard.com (om126233170111.36.openmobile.ne.jp\n\t[126.233.170.111])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 881AC83F;\n\tWed,  7 Jun 2023 20:52:40 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686163990;\n\tbh=zVvfLeL/49chxDw9KKJAxJySe5dbgTGeTgtYuLJmzFY=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=GAPdI+9vn45bmD/6wrrCnoP2uVG3chijU0hxETWaJGry2auJ3jIcGyGOejyDCJDh8\n\tmcPsv175zugUg3Rwzfk259RQGrxsk0f5u0y4Rg0qRTLrw49swLbtfGH4ucNzGR1tLm\n\tNQF3Fl4Zyyi82PVt2j6wfuWw37lVTTycnf9Serhkm7LsHffyleS/Wjz4+WTC2tLyRF\n\t2pgeX0r3vw6lHg7lnMz1VpqQdRRS4AyKFKA7dHsWr0YU2X6AAaTKOzM8aUfEl5TC/m\n\t3FTdZDBZVkpIUvduNhGX9y3gzKFWufoDMAw4piyCWGTFTV5zYe2GVmIg7GZZ//2nIF\n\thOD1zZ5o6WoLA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686163961;\n\tbh=zVvfLeL/49chxDw9KKJAxJySe5dbgTGeTgtYuLJmzFY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=dJsp/Qwe4QiQo9X1vrRiRSiuhMfdRffVXE1I61GwQbS1c517CJ/QrM8rS1VHS7/3+\n\tg5oXXk828X896Dtt97KpbDEPXURQUP5I4XKnvlzGm+zsI+nGYWcXgcEQQ8Vt3g93B/\n\txufqVZZ0Y/YtYiIVm3128iYAmFLzt8Yd/+1yTZFU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"dJsp/Qwe\"; dkim-atps=neutral","Date":"Wed, 7 Jun 2023 21:53:04 +0300","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20230607185304.GC26742@pendragon.ideasonboard.com>","References":"<20230515124550.3601128-1-kieran.bingham@ideasonboard.com>\n\t<20230515124550.3601128-6-kieran.bingham@ideasonboard.com>\n\t<20230606162736.GG25679@pendragon.ideasonboard.com>\n\t<168614782811.2889415.9195196058890174630@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<168614782811.2889415.9195196058890174630@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v3 5/5] libcamera: pipeline: Register\n\tdevice numbers with camera","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","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>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27355,"web_url":"https://patchwork.libcamera.org/comment/27355/","msgid":"<g46ky7zkistobclbv5xx4ewqvvz6btcoyzxnjcbv36jrsbhmpo@bmbq7kbsvjnz>","date":"2023-06-15T11:40:36","subject":"Re: [libcamera-devel] [PATCH v3 5/5] libcamera: pipeline: Register\n\tdevice numbers with camera","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Kieran\n\nOn Mon, May 15, 2023 at 01:45:50PM +0100, Kieran Bingham via libcamera-devel wrote:\n> Register the identified device numbers with each camera as the Devices\n> property.\n>\n> This facilitates camera daemons or other systems to identify which\n> devices are being managed by libcamera, and can prevent duplication of\n> camera resources.\n>\n> As the Devices property now provides this list of devices, use it\n> directly from within the CameraManager when adding a Camera rather than\n> passing it through the internal API.\n>\n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>  include/libcamera/internal/camera_manager.h |  3 +--\n>  src/libcamera/camera_manager.cpp            | 14 +++++++++-----\n>  src/libcamera/pipeline_handler.cpp          | 12 ++++++++++--\n>  3 files changed, 20 insertions(+), 9 deletions(-)\n>\n> diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h\n> index 885bb2825687..680e1f58cb02 100644\n> --- a/include/libcamera/internal/camera_manager.h\n> +++ b/include/libcamera/internal/camera_manager.h\n> @@ -29,8 +29,7 @@ public:\n>  \tPrivate();\n>\n>  \tint start();\n> -\tvoid addCamera(std::shared_ptr<Camera> camera,\n> -\t\t       const std::vector<dev_t> &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> +\tvoid addCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n>  \tvoid removeCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n>\n>  \t/*\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index 70eb4e455e54..7e499def9ddc 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -15,7 +15,9 @@\n>  #include <libcamera/base/utils.h>\n>\n>  #include <libcamera/camera.h>\n> +#include <libcamera/property_ids.h>\n>\n> +#include \"libcamera/internal/camera.h\"\n\nThis already includes <libcamera/camera.h>\n\ndoesn't hurt though\n\n>  #include \"libcamera/internal/device_enumerator.h\"\n>  #include \"libcamera/internal/pipeline_handler.h\"\n>\n> @@ -150,19 +152,17 @@ void CameraManager::Private::cleanup()\n>  /**\n>   * \\brief Add a camera to the camera manager\n>   * \\param[in] camera The camera to be added\n> - * \\param[in] devnums The device numbers to associate with \\a camera\n>   *\n>   * This function is called by pipeline handlers to register the cameras they\n>   * handle with the camera manager. Registered cameras are immediately made\n>   * available to the system.\n>   *\n> - * \\a devnums are used by the V4L2 compatibility layer to map V4L2 device nodes\n> - * to Camera instances.\n> + * Device numbers from the Devices property are used by the V4L2 compatibility\n> + * layer to map V4L2 device nodes to Camera instances.\n>   *\n>   * \\context This function shall be called from the CameraManager thread.\n>   */\n> -void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,\n> -\t\t\t\t       const std::vector<dev_t> &devnums)\n> +void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera)\n>  {\n>  \tASSERT(Thread::current() == this);\n>\n> @@ -177,6 +177,10 @@ void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,\n>  \t\t}\n>  \t}\n>\n> +\tauto devnums = camera->properties()\n> +\t\t\t       .get(properties::Devices)\n> +\t\t\t       .value_or(Span<int64_t>{});\n> +\n\nWeird indent\n\n\tauto devnums = camera->properties().get(properties::Devices)\n\t\t\t                   .value_or(Span<int64_t>{});\n\n?\n\nI guess we don't need to make sure devnums is populated, right ?\n\n>  \tcameras_.push_back(std::move(camera));\n>\n>  \tunsigned int index = cameras_.size() - 1;\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 49092ea88a58..0b5fac7ad113 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -17,6 +17,7 @@\n>\n>  #include <libcamera/camera.h>\n>  #include <libcamera/framebuffer.h>\n> +#include <libcamera/property_ids.h>\n>\n>  #include \"libcamera/internal/camera.h\"\n>  #include \"libcamera/internal/camera_manager.h\"\n> @@ -612,7 +613,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)\n>  \t * Walk the entity list and map the devnums of all capture video nodes\n>  \t * to the camera.\n>  \t */\n> -\tstd::vector<dev_t> devnums;\n> +\tstd::vector<int64_t> devnums;\n>  \tfor (const std::shared_ptr<MediaDevice> &media : mediaDevices_) {\n>  \t\tfor (const MediaEntity *entity : media->entities()) {\n>  \t\t\tif (entity->pads().size() == 1 &&\n> @@ -624,7 +625,14 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)\n>  \t\t}\n>  \t}\n>\n> -\tmanager_->_d()->addCamera(std::move(camera), devnums);\n> +\t/*\n> +\t * Store the associated devices as a property of the camera to allow\n> +\t * systems to identify which devices are managed by libcamera.\n> +\t */\n> +\tCamera::Private *data = camera->_d();\n> +\tdata->properties_.set(properties::Devices, devnums);\n> +\n> +\tmanager_->_d()->addCamera(std::move(camera));\n\nThe rest looks good\nReviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n\nThanks\n  j\n\n>  }\n>\n>  /**\n> --\n> 2.34.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id F026ABD78E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 15 Jun 2023 11:40:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5FA9861E4F;\n\tThu, 15 Jun 2023 13:40:41 +0200 (CEST)","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 E7C1A614FE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 15 Jun 2023 13:40:39 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:5d2e:52c9:72c3:346:a663:c82d])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 621D6891;\n\tThu, 15 Jun 2023 13:40:08 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686829241;\n\tbh=vw6jTexT9cmfCCcWSS/5aX3OLQbxF8HtCl4Otj9w7No=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=FmqcRuYsEUPF1tO0TtR4OYXNzZoQ37kR3uM66nXE9s64fPp1kgRicRGR6Jn/rEJA5\n\tiD1UigzS7ioMGgI8AXvPc2Byl9mwAUreueibFktJkUAVZ4bqnJhPHXFTyMVHPHWUb8\n\tCaOLxFyv0CnJ61qp+U+FFdUhw+BnUwmQKfPs7oRBoVjxz4gbnVXQ0npUXWbkh7Fo1C\n\t4mZIdLWL1tUfm9jnKxQRHB2S5ThQ5z6F6S9m2sViqSZcNOqIzuj9oT3Z5AF4TkEam0\n\t6mTrPlna1xXMkQgtYer31hnnnO/GZ+ahQG+2BH1LvCFI1aKKYkMEIawov/nhNJDu1B\n\tydUSEWYaUb0fA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686829208;\n\tbh=vw6jTexT9cmfCCcWSS/5aX3OLQbxF8HtCl4Otj9w7No=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jcmjVRaITQI5Pla+vjUo3AzePvvrgEw7CJuHFq39k+DtnBkmDILY6FhoMvfQ5F1s/\n\tPO4B6uPDk/PN5vfItofyz3a66NX6vqje+KBp5x3Oa9xHOkf145YO9eU+vM4XU+8XBj\n\t+a45AataffFIa67/WBL7iXJrBaDuFoeJTQm1YVEM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"jcmjVRaI\"; dkim-atps=neutral","Date":"Thu, 15 Jun 2023 13:40:36 +0200","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<g46ky7zkistobclbv5xx4ewqvvz6btcoyzxnjcbv36jrsbhmpo@bmbq7kbsvjnz>","References":"<20230515124550.3601128-1-kieran.bingham@ideasonboard.com>\n\t<20230515124550.3601128-6-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230515124550.3601128-6-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 5/5] libcamera: pipeline: Register\n\tdevice numbers with camera","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","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>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27356,"web_url":"https://patchwork.libcamera.org/comment/27356/","msgid":"<168683287692.3585053.12436447464808527226@Monstersaurus>","date":"2023-06-15T12:41:16","subject":"Re: [libcamera-devel] [PATCH v3 5/5] libcamera: pipeline: Register\n\tdevice numbers with camera","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jacopo Mondi (2023-06-15 12:40:36)\n> Hi Kieran\n> \n> On Mon, May 15, 2023 at 01:45:50PM +0100, Kieran Bingham via libcamera-devel wrote:\n> > Register the identified device numbers with each camera as the Devices\n> > property.\n> >\n> > This facilitates camera daemons or other systems to identify which\n> > devices are being managed by libcamera, and can prevent duplication of\n> > camera resources.\n> >\n> > As the Devices property now provides this list of devices, use it\n> > directly from within the CameraManager when adding a Camera rather than\n> > passing it through the internal API.\n> >\n> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > ---\n> >  include/libcamera/internal/camera_manager.h |  3 +--\n> >  src/libcamera/camera_manager.cpp            | 14 +++++++++-----\n> >  src/libcamera/pipeline_handler.cpp          | 12 ++++++++++--\n> >  3 files changed, 20 insertions(+), 9 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h\n> > index 885bb2825687..680e1f58cb02 100644\n> > --- a/include/libcamera/internal/camera_manager.h\n> > +++ b/include/libcamera/internal/camera_manager.h\n> > @@ -29,8 +29,7 @@ public:\n> >       Private();\n> >\n> >       int start();\n> > -     void addCamera(std::shared_ptr<Camera> camera,\n> > -                    const std::vector<dev_t> &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> > +     void addCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> >       void removeCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> >\n> >       /*\n> > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > index 70eb4e455e54..7e499def9ddc 100644\n> > --- a/src/libcamera/camera_manager.cpp\n> > +++ b/src/libcamera/camera_manager.cpp\n> > @@ -15,7 +15,9 @@\n> >  #include <libcamera/base/utils.h>\n> >\n> >  #include <libcamera/camera.h>\n> > +#include <libcamera/property_ids.h>\n> >\n> > +#include \"libcamera/internal/camera.h\"\n> \n> This already includes <libcamera/camera.h>\n> \n> doesn't hurt though\n> \n> >  #include \"libcamera/internal/device_enumerator.h\"\n> >  #include \"libcamera/internal/pipeline_handler.h\"\n> >\n> > @@ -150,19 +152,17 @@ void CameraManager::Private::cleanup()\n> >  /**\n> >   * \\brief Add a camera to the camera manager\n> >   * \\param[in] camera The camera to be added\n> > - * \\param[in] devnums The device numbers to associate with \\a camera\n> >   *\n> >   * This function is called by pipeline handlers to register the cameras they\n> >   * handle with the camera manager. Registered cameras are immediately made\n> >   * available to the system.\n> >   *\n> > - * \\a devnums are used by the V4L2 compatibility layer to map V4L2 device nodes\n> > - * to Camera instances.\n> > + * Device numbers from the Devices property are used by the V4L2 compatibility\n> > + * layer to map V4L2 device nodes to Camera instances.\n> >   *\n> >   * \\context This function shall be called from the CameraManager thread.\n> >   */\n> > -void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,\n> > -                                    const std::vector<dev_t> &devnums)\n> > +void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera)\n> >  {\n> >       ASSERT(Thread::current() == this);\n> >\n> > @@ -177,6 +177,10 @@ void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,\n> >               }\n> >       }\n> >\n> > +     auto devnums = camera->properties()\n> > +                            .get(properties::Devices)\n> > +                            .value_or(Span<int64_t>{});\n> > +\n> \n> Weird indent\n> \n>         auto devnums = camera->properties().get(properties::Devices)\n>                                            .value_or(Span<int64_t>{});\n> \n> ?\n> \n> I guess we don't need to make sure devnums is populated, right ?\n\nNo - if we end up with Virtual pipeline handlers there wouldn't be any\ndevnums anyway.\n\nI'd expect we can later update the V4L2 Adaptation layer to use the\nProperty directly and clean up this 'extra registration' too.\n\nI wanted to get the core implementation handled first though.\n\n--\nKieran\n\n\n> \n> >       cameras_.push_back(std::move(camera));\n> >\n> >       unsigned int index = cameras_.size() - 1;\n> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > index 49092ea88a58..0b5fac7ad113 100644\n> > --- a/src/libcamera/pipeline_handler.cpp\n> > +++ b/src/libcamera/pipeline_handler.cpp\n> > @@ -17,6 +17,7 @@\n> >\n> >  #include <libcamera/camera.h>\n> >  #include <libcamera/framebuffer.h>\n> > +#include <libcamera/property_ids.h>\n> >\n> >  #include \"libcamera/internal/camera.h\"\n> >  #include \"libcamera/internal/camera_manager.h\"\n> > @@ -612,7 +613,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)\n> >        * Walk the entity list and map the devnums of all capture video nodes\n> >        * to the camera.\n> >        */\n> > -     std::vector<dev_t> devnums;\n> > +     std::vector<int64_t> devnums;\n> >       for (const std::shared_ptr<MediaDevice> &media : mediaDevices_) {\n> >               for (const MediaEntity *entity : media->entities()) {\n> >                       if (entity->pads().size() == 1 &&\n> > @@ -624,7 +625,14 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)\n> >               }\n> >       }\n> >\n> > -     manager_->_d()->addCamera(std::move(camera), devnums);\n> > +     /*\n> > +      * Store the associated devices as a property of the camera to allow\n> > +      * systems to identify which devices are managed by libcamera.\n> > +      */\n> > +     Camera::Private *data = camera->_d();\n> > +     data->properties_.set(properties::Devices, devnums);\n> > +\n> > +     manager_->_d()->addCamera(std::move(camera));\n> \n> The rest looks good\n> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> \n> Thanks\n>   j\n> \n> >  }\n> >\n> >  /**\n> > --\n> > 2.34.1\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D3209C322E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 15 Jun 2023 12:41:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2DF61628B5;\n\tThu, 15 Jun 2023 14:41:21 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 61B48614FE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 15 Jun 2023 14:41:20 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D3AED9CA;\n\tThu, 15 Jun 2023 14:40:48 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686832881;\n\tbh=q0hTyFFgTnxGfIMR5Ottlreel2pRq/1DpUcnaS0xYr8=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=DLoMkaKxeBN9rnPpQQ4wdP0oabe2pF4Tuk0XS6jjpX9WumjZo939B25Hjc+mNn0SU\n\t5zdeszX6DYvx9c+KMGl5yKrBQod+EyWX2tLvo7ubrrFH1tnn5VVpHan7uxdsRTe5U0\n\tOn/WCDhG27bC0aThIfkkkmPSvRmpsc/lNVw2jcSawSYNAy807hq4xKqqGCcJd/TAQz\n\tIwSGh6K19GG0FXLA/+OCu7xgxazO27evyDwXSODRgYL+nzHVjUaaPpNPQTekXgePzD\n\t3+jhNlQtlUiJpqwULHlGmc5Z7Ct1rLyONQZ+6RxKxJMU9dFiJnRQ7YzaTfoRBkq/RM\n\te+gGwTCvDwGnw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686832849;\n\tbh=q0hTyFFgTnxGfIMR5Ottlreel2pRq/1DpUcnaS0xYr8=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=YAP4itSxWvR3eteymH9/FFkk/l5ihMNSKAPiiSOiROS0O+PTaeVb1fOygt/2Stbxm\n\tLgdeD6iebCpIHDGBYAXuqNoht5G9p0W2ka4SpS9guOEmxyo7WH7K53+2/o52uiPWNx\n\tnzIMThmCQIcIeMMbgwQjp16WSSQFkYaAi0qmPjGU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"YAP4itSx\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<g46ky7zkistobclbv5xx4ewqvvz6btcoyzxnjcbv36jrsbhmpo@bmbq7kbsvjnz>","References":"<20230515124550.3601128-1-kieran.bingham@ideasonboard.com>\n\t<20230515124550.3601128-6-kieran.bingham@ideasonboard.com>\n\t<g46ky7zkistobclbv5xx4ewqvvz6btcoyzxnjcbv36jrsbhmpo@bmbq7kbsvjnz>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Date":"Thu, 15 Jun 2023 13:41:16 +0100","Message-ID":"<168683287692.3585053.12436447464808527226@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v3 5/5] libcamera: pipeline: Register\n\tdevice numbers with camera","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","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>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27357,"web_url":"https://patchwork.libcamera.org/comment/27357/","msgid":"<168684958322.3585053.16508711554350631989@Monstersaurus>","date":"2023-06-15T17:19:43","subject":"Re: [libcamera-devel] [PATCH v3 5/5] libcamera: pipeline: Register\n\tdevice numbers with camera","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Kieran Bingham (2023-06-15 13:41:16)\n> Quoting Jacopo Mondi (2023-06-15 12:40:36)\n> > Hi Kieran\n> > \n> > On Mon, May 15, 2023 at 01:45:50PM +0100, Kieran Bingham via libcamera-devel wrote:\n> > > Register the identified device numbers with each camera as the Devices\n> > > property.\n> > >\n> > > This facilitates camera daemons or other systems to identify which\n> > > devices are being managed by libcamera, and can prevent duplication of\n> > > camera resources.\n> > >\n> > > As the Devices property now provides this list of devices, use it\n> > > directly from within the CameraManager when adding a Camera rather than\n> > > passing it through the internal API.\n> > >\n> > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > > ---\n> > >  include/libcamera/internal/camera_manager.h |  3 +--\n> > >  src/libcamera/camera_manager.cpp            | 14 +++++++++-----\n> > >  src/libcamera/pipeline_handler.cpp          | 12 ++++++++++--\n> > >  3 files changed, 20 insertions(+), 9 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h\n> > > index 885bb2825687..680e1f58cb02 100644\n> > > --- a/include/libcamera/internal/camera_manager.h\n> > > +++ b/include/libcamera/internal/camera_manager.h\n> > > @@ -29,8 +29,7 @@ public:\n> > >       Private();\n> > >\n> > >       int start();\n> > > -     void addCamera(std::shared_ptr<Camera> camera,\n> > > -                    const std::vector<dev_t> &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> > > +     void addCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> > >       void removeCamera(std::shared_ptr<Camera> camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> > >\n> > >       /*\n> > > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > > index 70eb4e455e54..7e499def9ddc 100644\n> > > --- a/src/libcamera/camera_manager.cpp\n> > > +++ b/src/libcamera/camera_manager.cpp\n> > > @@ -15,7 +15,9 @@\n> > >  #include <libcamera/base/utils.h>\n> > >\n> > >  #include <libcamera/camera.h>\n> > > +#include <libcamera/property_ids.h>\n> > >\n> > > +#include \"libcamera/internal/camera.h\"\n> > \n> > This already includes <libcamera/camera.h>\n> > \n> > doesn't hurt though\n> > \n> > >  #include \"libcamera/internal/device_enumerator.h\"\n> > >  #include \"libcamera/internal/pipeline_handler.h\"\n> > >\n> > > @@ -150,19 +152,17 @@ void CameraManager::Private::cleanup()\n> > >  /**\n> > >   * \\brief Add a camera to the camera manager\n> > >   * \\param[in] camera The camera to be added\n> > > - * \\param[in] devnums The device numbers to associate with \\a camera\n> > >   *\n> > >   * This function is called by pipeline handlers to register the cameras they\n> > >   * handle with the camera manager. Registered cameras are immediately made\n> > >   * available to the system.\n> > >   *\n> > > - * \\a devnums are used by the V4L2 compatibility layer to map V4L2 device nodes\n> > > - * to Camera instances.\n> > > + * Device numbers from the Devices property are used by the V4L2 compatibility\n> > > + * layer to map V4L2 device nodes to Camera instances.\n> > >   *\n> > >   * \\context This function shall be called from the CameraManager thread.\n> > >   */\n> > > -void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,\n> > > -                                    const std::vector<dev_t> &devnums)\n> > > +void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera)\n> > >  {\n> > >       ASSERT(Thread::current() == this);\n> > >\n> > > @@ -177,6 +177,10 @@ void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera,\n> > >               }\n> > >       }\n> > >\n> > > +     auto devnums = camera->properties()\n> > > +                            .get(properties::Devices)\n> > > +                            .value_or(Span<int64_t>{});\n> > > +\n> > \n> > Weird indent\n\nThis gets through checkstyle, while\n\n> > \n> >         auto devnums = camera->properties().get(properties::Devices)\n> >                                            .value_or(Span<int64_t>{});\n\nIf I apply the above it causes:\n\n--- src/libcamera/camera_manager.cpp\n+++ src/libcamera/camera_manager.cpp\n@@ -178,8 +178,7 @@\n \t\t}\n \t}\n\n-\tauto devnums = camera->properties().get(properties::SystemDevices)\n-\t\t\t\t\t   .value_or(Span<int64_t>{});\n+\tauto devnums = camera->properties().get(properties::SystemDevices).value_or(Span<int64_t>{});\n\n \tcameras_.push_back(std::move(camera));\n\nTo fit all on one line which is longer than 80 chars.\n\nI prefer the hanging indent for the chained calls, so I'll leave it as\nit is in this patch.\n\n> > \n> > ?\n> > \n> > I guess we don't need to make sure devnums is populated, right ?\n> \n> No - if we end up with Virtual pipeline handlers there wouldn't be any\n> devnums anyway.\n> \n> I'd expect we can later update the V4L2 Adaptation layer to use the\n> Property directly and clean up this 'extra registration' too.\n> \n> I wanted to get the core implementation handled first though.\n> \n> --\n> Kieran\n> \n> \n> > \n> > >       cameras_.push_back(std::move(camera));\n> > >\n> > >       unsigned int index = cameras_.size() - 1;\n> > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > > index 49092ea88a58..0b5fac7ad113 100644\n> > > --- a/src/libcamera/pipeline_handler.cpp\n> > > +++ b/src/libcamera/pipeline_handler.cpp\n> > > @@ -17,6 +17,7 @@\n> > >\n> > >  #include <libcamera/camera.h>\n> > >  #include <libcamera/framebuffer.h>\n> > > +#include <libcamera/property_ids.h>\n> > >\n> > >  #include \"libcamera/internal/camera.h\"\n> > >  #include \"libcamera/internal/camera_manager.h\"\n> > > @@ -612,7 +613,7 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)\n> > >        * Walk the entity list and map the devnums of all capture video nodes\n> > >        * to the camera.\n> > >        */\n> > > -     std::vector<dev_t> devnums;\n> > > +     std::vector<int64_t> devnums;\n> > >       for (const std::shared_ptr<MediaDevice> &media : mediaDevices_) {\n> > >               for (const MediaEntity *entity : media->entities()) {\n> > >                       if (entity->pads().size() == 1 &&\n> > > @@ -624,7 +625,14 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera)\n> > >               }\n> > >       }\n> > >\n> > > -     manager_->_d()->addCamera(std::move(camera), devnums);\n> > > +     /*\n> > > +      * Store the associated devices as a property of the camera to allow\n> > > +      * systems to identify which devices are managed by libcamera.\n> > > +      */\n> > > +     Camera::Private *data = camera->_d();\n> > > +     data->properties_.set(properties::Devices, devnums);\n> > > +\n> > > +     manager_->_d()->addCamera(std::move(camera));\n> > \n> > The rest looks good\n> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> > \n> > Thanks\n> >   j\n> > \n> > >  }\n> > >\n> > >  /**\n> > > --\n> > > 2.34.1\n> > >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 745DCBD78E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 15 Jun 2023 17:19:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B6FA2614FE;\n\tThu, 15 Jun 2023 19:19:47 +0200 (CEST)","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 1D963614FE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 15 Jun 2023 19:19:46 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7F999E4;\n\tThu, 15 Jun 2023 19:19:14 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686849587;\n\tbh=xFFByhtsgYyn8DCJ+fWsUyywJlD0wQXsWlG1RB/Za2o=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=pl8BiOVqg95oxuwfUJQBAlT29hLPqhDWxMGBUEQ2Jd37Pt+kxt8FdpkAbFojbb//r\n\t1B5bFYUzD47aivqokock3bXZO13XiYCBnS1yoDxpIPGc76NWaPgraE6Xu1uMIjhUBs\n\tQgwr5xLr+gHBe6lHoS/LSAwYmPiqtMpWpxSqxUBt0xPuw8uGGV7wHrL8gukRxJQ8tK\n\tg11RdktuJQzjoATD3zrgiQvmElxcyFhqSzaMo1guVqU8IbQq38Df1FKeH2mY9SpnUo\n\tRmLvi17YrDoftX+gQDbqSVAoLck+Y11ikaTrxSsm+SdVHXbRW85d8zBByX3YHq3Z/z\n\tbw217gG+fly5w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686849554;\n\tbh=xFFByhtsgYyn8DCJ+fWsUyywJlD0wQXsWlG1RB/Za2o=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=GWxZlGuTIVCR3GxmrYujsZhZvlyuvJOJGG2BGDCnygHZ7PYGeuvHNDDggAHknr6Zf\n\tDA6WA/zLp4/OyaI9qiAhDnMcB5tF03tmDlBX++ttdtFlmfAImn7MWlYjFXE1n6aHFS\n\tzQB9N6wJ9x8oL6fNIn6q8VOvSWXBV6bSXLoA9+t0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"GWxZlGuT\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<168683287692.3585053.12436447464808527226@Monstersaurus>","References":"<20230515124550.3601128-1-kieran.bingham@ideasonboard.com>\n\t<20230515124550.3601128-6-kieran.bingham@ideasonboard.com>\n\t<g46ky7zkistobclbv5xx4ewqvvz6btcoyzxnjcbv36jrsbhmpo@bmbq7kbsvjnz>\n\t<168683287692.3585053.12436447464808527226@Monstersaurus>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Date":"Thu, 15 Jun 2023 18:19:43 +0100","Message-ID":"<168684958322.3585053.16508711554350631989@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v3 5/5] libcamera: pipeline: Register\n\tdevice numbers with camera","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","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>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]