[{"id":27088,"web_url":"https://patchwork.libcamera.org/comment/27088/","msgid":"<20230513150754.jnkhuzr3enjsvpq3@lati>","date":"2023-05-13T15:07:54","subject":"Re: [libcamera-devel] [PATCH v2 4/4] 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 Thu, May 11, 2023 at 11:58:33PM +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\nmore than duplication isn't this about concurrent access to the same\nresources ?\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 \n> ---\n> include/libcamera/internal/camera_manager.h | 3 +--\n> src/libcamera/camera_manager.cpp | 13 ++++++++-----\n> src/libcamera/pipeline_handler.cpp | 12 ++++++++++--\n> 3 files changed, 19 insertions(+), 9 deletions(-)\n>\n> diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h\n> index 8386e90e0f60..061d2c5083b0 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,\n> -\t\t const std::vector &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> +\tvoid addCamera(std::shared_ptr camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> \tvoid removeCamera(std::shared_ptr 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 34f09a4d181b..df5fdede85da 100644\n> --- a/src/libcamera/camera_manager.cpp\n> +++ b/src/libcamera/camera_manager.cpp\n> @@ -15,7 +15,9 @@\n> #include \n>\n> #include \n> +#include \n>\n> +#include \"libcamera/internal/camera.h\"\n> #include \"libcamera/internal/camera_manager.h\"\n> #include \"libcamera/internal/device_enumerator.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> @@ -151,19 +153,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 properties::Devices\n\n> + * layer to map V4L2 device nodes to Camera instances.\n\nI would also add here what you have reported in the commit message\nabout higher level frameworks\n\n> *\n> * \\context This function shall be called from the CameraManager thread.\n> */\n> -void CameraManager::Private::addCamera(std::shared_ptr camera,\n> -\t\t\t\t const std::vector &devnums)\n> +void CameraManager::Private::addCamera(std::shared_ptr camera)\n> {\n> \tASSERT(Thread::current() == this);\n>\n> @@ -178,6 +178,9 @@ void CameraManager::Private::addCamera(std::shared_ptr camera,\n> \t\t}\n> \t}\n>\n> +\tCamera::Private *data = camera->_d();\n> +\tauto devnums = data->properties_.get(properties::Devices).value_or(std::vector{});\n\nNo need to get a pointer to the Private implementation to access the\nCamera properties.\n\n auto devnums = camera->properties().get...\n\nI would also have expected\n .value_or(Span{});\n\ndoes this work because of an implicit type conversion ?\n\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 \n> #include \n> +#include \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)\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 devnums;\n> +\tstd::vector devnums;\n> \tfor (const std::shared_ptr &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)\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\nnice\n\n> }\n>\n> /**\n> --\n> 2.34.1\n>","headers":{"Return-Path":"","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 3D0B3BE08B\n\tfor ;\n\tSat, 13 May 2023 15:08:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BEC07633B5;\n\tSat, 13 May 2023 17:07:59 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 83594627DE\n\tfor ;\n\tSat, 13 May 2023 17:07:58 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 005AB6CA;\n\tSat, 13 May 2023 17:07:48 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1683990479;\n\tbh=Jxqd9vGPRQU80Tyimm+ieGqs3X6zCJmISNQhaprGnNM=;\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=gKT04xt5kyNuONQuSM/GLUof5fRelgTxJEIrZnuMbCNmtKQJKNrYn2mN475IuvOJ1\n\trAkPZArZYVrGlKKIjWHhpABm/6D8LuEgmpU/mtZCnaLaV5agngm6H/pj96Tgxsknxo\n\tbYpbRie/WT/jsh1yBW21CZXRWi8HkewvMUuvmzyuOG62kNn1zAmPQ3bYua+qUNqyCu\n\tqnspNGaZgafM5F/FXdeGnU63G1kDjZzqboiRA17YfaRX44IaLT5MlaSFiQTzg9Yqwb\n\tGQKrdZasgasoC5l/FHMA0HPBElvHZhgxSIsHBXwIyti8WdVF1Mi5s/nG8UOHBx1eFY\n\tLcALg4SyQgImQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1683990469;\n\tbh=Jxqd9vGPRQU80Tyimm+ieGqs3X6zCJmISNQhaprGnNM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=H0HPm6LL2XTCPUq1FBfiqkVVT08jvVcJ/LsXu6CpQwdhhttxEhyjR2PZhYibPsWi9\n\ttQnK6ukc2VjlsVGYgKVfU1y/9WId8cYygDj7hI3PriZblCEXIP7kiicukjDurH7hf2\n\ti5pdVFkbBXLHMQyqV8d5tBh3jBggj5jzm+Oi5vns="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"H0HPm6LL\"; dkim-atps=neutral","Date":"Sat, 13 May 2023 17:07:54 +0200","To":"Kieran Bingham ","Message-ID":"<20230513150754.jnkhuzr3enjsvpq3@lati>","References":"<20230511225833.361699-1-kieran.bingham@ideasonboard.com>\n\t<20230511225833.361699-5-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230511225833.361699-5-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 4/4] 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"George Kiagiadakis ,\n\tlibcamera devel ,\n\tRobert Mader ,\n\tWim Taymans ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27092,"web_url":"https://patchwork.libcamera.org/comment/27092/","msgid":"<168399264412.1830511.13431479241942247249@Monstersaurus>","date":"2023-05-13T15:44:04","subject":"Re: [libcamera-devel] [PATCH v2 4/4] 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-05-13 16:07:54)\n> Hi Kieran\n> \n> On Thu, May 11, 2023 at 11:58:33PM +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> more than duplication isn't this about concurrent access to the same\n> resources ?\n\nNo not specifically concurrent. It's (in the feature request) so that\npipewire doesn't create a video object for the same UVC camera through\nboth libcamera and V4L2 for instance. They don't have to be used\nconcurrently even - it's the fact that pipewire doesn't know if the\ndevice is managed by libcamera - and it can be managed through both V4L2\nand libcamera - so pipewire ends up showing two camera objects that are\nactually the same device.\n\nTechnically both could be used (individually) but this lets pipewire\ndecide to only show one (and it's up to PW to decide which one it\nselects between libcamera and v4l2 directly).\n\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 \n> > ---\n> > include/libcamera/internal/camera_manager.h | 3 +--\n> > src/libcamera/camera_manager.cpp | 13 ++++++++-----\n> > src/libcamera/pipeline_handler.cpp | 12 ++++++++++--\n> > 3 files changed, 19 insertions(+), 9 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h\n> > index 8386e90e0f60..061d2c5083b0 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,\n> > - const std::vector &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> > + void addCamera(std::shared_ptr camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> > void removeCamera(std::shared_ptr camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> >\n> > /*\n> > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > index 34f09a4d181b..df5fdede85da 100644\n> > --- a/src/libcamera/camera_manager.cpp\n> > +++ b/src/libcamera/camera_manager.cpp\n> > @@ -15,7 +15,9 @@\n> > #include \n> >\n> > #include \n> > +#include \n> >\n> > +#include \"libcamera/internal/camera.h\"\n> > #include \"libcamera/internal/camera_manager.h\"\n> > #include \"libcamera/internal/device_enumerator.h\"\n> > #include \"libcamera/internal/pipeline_handler.h\"\n> > @@ -151,19 +153,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> properties::Devices\n> \n> > + * layer to map V4L2 device nodes to Camera instances.\n> \n> I would also add here what you have reported in the commit message\n> about higher level frameworks\n> \n> > *\n> > * \\context This function shall be called from the CameraManager thread.\n> > */\n> > -void CameraManager::Private::addCamera(std::shared_ptr camera,\n> > - const std::vector &devnums)\n> > +void CameraManager::Private::addCamera(std::shared_ptr camera)\n> > {\n> > ASSERT(Thread::current() == this);\n> >\n> > @@ -178,6 +178,9 @@ void CameraManager::Private::addCamera(std::shared_ptr camera,\n> > }\n> > }\n> >\n> > + Camera::Private *data = camera->_d();\n> > + auto devnums = data->properties_.get(properties::Devices).value_or(std::vector{});\n> \n> No need to get a pointer to the Private implementation to access the\n> Camera properties.\n> \n> auto devnums = camera->properties().get...\n\nOh yeah ! That's easier/cleaner ;-)\n\n> \n> I would also have expected\n> .value_or(Span{});\n> \n> does this work because of an implicit type conversion ?\n\nI expect so! I wish we had Laurents' default emtpy constructors to be\nable to do \n\t.value_or({}); directly :-(\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 \n> > #include \n> > +#include \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)\n> > * Walk the entity list and map the devnums of all capture video nodes\n> > * to the camera.\n> > */\n> > - std::vector devnums;\n> > + std::vector devnums;\n> > for (const std::shared_ptr &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)\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> nice\n\nYes, this bit worked out nicely indeed.\n\n> \n> > }\n> >\n> > /**\n> > --\n> > 2.34.1\n> >","headers":{"Return-Path":"","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 D20C5BD16B\n\tfor ;\n\tSat, 13 May 2023 15:44:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 351AE633B5;\n\tSat, 13 May 2023 17:44:08 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 12AA2627DE\n\tfor ;\n\tSat, 13 May 2023 17:44:07 +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 89DAC6CA;\n\tSat, 13 May 2023 17:43:57 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1683992648;\n\tbh=jfgsGfC2aNEFLqWZFe8W9hSvsA/zoSTBogY9TRl6RO4=;\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=3CzzxC93TCBrGVa8NPtK6EtdClFId11erOB3/o6lkBvaJh/9R+9exT4pxJrohzkxO\n\t1S30TS+yumgK5BnYNti4VXoRNxpm/oJBg0y8B9a+q17nbSnRxAkVNFqu9yO7cLdyhc\n\tfzLPC2XBF0wTE/9FFSXMS8xzkWfSNNaRcBujFvbWPrJpnlWHaWkyf+bAWDGEIHh5OH\n\tC6XPrKyWvUuubImKkbxv6on2y10zp9OqGM53wQlD6UDqwyorHV5fTpFbGS1YSaHZCo\n\taiEGurD6+hCGvXhWno27f+PpG982ikWR+eXXrIcfP/fLUxjhH+jeC5bU7zXcjaB5iq\n\tyoiBkQyZOQENg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1683992637;\n\tbh=jfgsGfC2aNEFLqWZFe8W9hSvsA/zoSTBogY9TRl6RO4=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=guSpKoxz33tAOWMWlK7e3w5e2jOHDgGxN0rcTVN57mtjIcLqsvxIxqjRnxH2xzNSG\n\tN1SuKvBKAmof3/c7WncIX4V1sBPPA3UuZil+wOD6k5woVBVXzhYPuf/NsFOFT5Nv7q\n\tqVlPrH2LzRTWQJBT4Zzkd7XAGuykVRRB0ktqpbYA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"guSpKoxz\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230513150754.jnkhuzr3enjsvpq3@lati>","References":"<20230511225833.361699-1-kieran.bingham@ideasonboard.com>\n\t<20230511225833.361699-5-kieran.bingham@ideasonboard.com>\n\t<20230513150754.jnkhuzr3enjsvpq3@lati>","To":"Jacopo Mondi ","Date":"Sat, 13 May 2023 16:44:04 +0100","Message-ID":"<168399264412.1830511.13431479241942247249@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 4/4] 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"George Kiagiadakis ,\n\tlibcamera devel ,\n\tRobert Mader ,\n\tWim Taymans ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27098,"web_url":"https://patchwork.libcamera.org/comment/27098/","msgid":"<168415434659.3378917.510419833984127080@Monstersaurus>","date":"2023-05-15T12:39:06","subject":"Re: [libcamera-devel] [PATCH v2 4/4] 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-05-13 16:44:04)\n> Quoting Jacopo Mondi (2023-05-13 16:07:54)\n> > Hi Kieran\n> > \n> > On Thu, May 11, 2023 at 11:58:33PM +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> > more than duplication isn't this about concurrent access to the same\n> > resources ?\n> \n> No not specifically concurrent. It's (in the feature request) so that\n> pipewire doesn't create a video object for the same UVC camera through\n> both libcamera and V4L2 for instance. They don't have to be used\n> concurrently even - it's the fact that pipewire doesn't know if the\n> device is managed by libcamera - and it can be managed through both V4L2\n> and libcamera - so pipewire ends up showing two camera objects that are\n> actually the same device.\n> \n> Technically both could be used (individually) but this lets pipewire\n> decide to only show one (and it's up to PW to decide which one it\n> selects between libcamera and v4l2 directly).\n> \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 \n> > > ---\n> > > include/libcamera/internal/camera_manager.h | 3 +--\n> > > src/libcamera/camera_manager.cpp | 13 ++++++++-----\n> > > src/libcamera/pipeline_handler.cpp | 12 ++++++++++--\n> > > 3 files changed, 19 insertions(+), 9 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h\n> > > index 8386e90e0f60..061d2c5083b0 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,\n> > > - const std::vector &devnums) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> > > + void addCamera(std::shared_ptr camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> > > void removeCamera(std::shared_ptr camera) LIBCAMERA_TSA_EXCLUDES(mutex_);\n> > >\n> > > /*\n> > > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > > index 34f09a4d181b..df5fdede85da 100644\n> > > --- a/src/libcamera/camera_manager.cpp\n> > > +++ b/src/libcamera/camera_manager.cpp\n> > > @@ -15,7 +15,9 @@\n> > > #include \n> > >\n> > > #include \n> > > +#include \n> > >\n> > > +#include \"libcamera/internal/camera.h\"\n> > > #include \"libcamera/internal/camera_manager.h\"\n> > > #include \"libcamera/internal/device_enumerator.h\"\n> > > #include \"libcamera/internal/pipeline_handler.h\"\n> > > @@ -151,19 +153,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> > properties::Devices\n> > \n> > > + * layer to map V4L2 device nodes to Camera instances.\n> > \n> > I would also add here what you have reported in the commit message\n> > about higher level frameworks\n\nI'm not going to do this because:\n\n - A) It's not relevant here. Only the V4L2 layer uses the\n camerasByDevnum_ parts. The registration of devnums into\n Properties::Devices is separate.\n\n B) I'd probably like to work out how we remove the camerasByDevnum_\n and update the V4L2 layer to use the newly available\n Properties::Devices and remove these extra mappings internally.\n\n> > \n> > > *\n> > > * \\context This function shall be called from the CameraManager thread.\n> > > */\n> > > -void CameraManager::Private::addCamera(std::shared_ptr camera,\n> > > - const std::vector &devnums)\n> > > +void CameraManager::Private::addCamera(std::shared_ptr camera)\n> > > {\n> > > ASSERT(Thread::current() == this);\n> > >\n> > > @@ -178,6 +178,9 @@ void CameraManager::Private::addCamera(std::shared_ptr camera,\n> > > }\n> > > }\n> > >\n> > > + Camera::Private *data = camera->_d();\n> > > + auto devnums = data->properties_.get(properties::Devices).value_or(std::vector{});\n> > \n> > No need to get a pointer to the Private implementation to access the\n> > Camera properties.\n> > \n> > auto devnums = camera->properties().get...\n> \n> Oh yeah ! That's easier/cleaner ;-)\n\nDone.\n\n\n> \n> > \n> > I would also have expected\n> > .value_or(Span{});\n> > \n> > does this work because of an implicit type conversion ?\n> \n> I expect so! I wish we had Laurents' default emtpy constructors to be\n> able to do \n> .value_or({}); directly :-(\n\nChanged to Span<>\n\n\nV3 imminent.\n\n--\nKieran\n\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 \n> > > #include \n> > > +#include \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)\n> > > * Walk the entity list and map the devnums of all capture video nodes\n> > > * to the camera.\n> > > */\n> > > - std::vector devnums;\n> > > + std::vector devnums;\n> > > for (const std::shared_ptr &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)\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> > nice\n> \n> Yes, this bit worked out nicely indeed.\n> \n> > \n> > > }\n> > >\n> > > /**\n> > > --\n> > > 2.34.1\n> > >","headers":{"Return-Path":"","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 A1E93C3284\n\tfor ;\n\tMon, 15 May 2023 12:39:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0438B627DE;\n\tMon, 15 May 2023 14:39:12 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CA1376039F\n\tfor ;\n\tMon, 15 May 2023 14:39:09 +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 EBD744DB;\n\tMon, 15 May 2023 14:38:58 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1684154352;\n\tbh=J8P0OkXwHId1Eg5VHDj0Eu/JUO5hb5Q7kfhKu2tH8bA=;\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=SPfepiwKG0xmsYcWLG8r9BuFQ8KOn3iu825WjPyZuqT+DW4wZdsPGwslSK8Y8THrI\n\tzXtiK4cM4xPshTJZVQ+yHku35ew6oVEFfJ7PAGd7KnqaVeJNVj14UIyD3hS/Rk8qMB\n\tok0rNz8vVk7vRnRablWj34EokuVrKP2XGsKYXAyunREheOwwJaeIiI7bONOLYDhttV\n\tB5TXeB5QvExcO0FAC0c2JVIZhvfb2i9VJhAodKwsPfzjBZxqJF+uvA5q0ZBvmHq1m/\n\tpHkWtoz4hjNzqWrxwXFGAdOWKgAzSJZ3qQmoqlF16ebkMVRJ0l1qK0C6sIfkj8kx1f\n\tRsdN2bUmxRygg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1684154339;\n\tbh=J8P0OkXwHId1Eg5VHDj0Eu/JUO5hb5Q7kfhKu2tH8bA=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=rgk4FJBov1E0YBkroyqEnzQYLed3am4S5bI6hBEsYlVLutlqYbth+kNsb15ApJhFt\n\tswCVCS8/WQiGuqhkjzBQdIgLRqteEO2kjuaoFEuFvjzcDeeeLr49h2qCxoazUiTfZ0\n\t/8+P38B2PXoRAOMyd3PJ/UoVz3bubevn/ONS+qjk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"rgk4FJBo\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<168399264412.1830511.13431479241942247249@Monstersaurus>","References":"<20230511225833.361699-1-kieran.bingham@ideasonboard.com>\n\t<20230511225833.361699-5-kieran.bingham@ideasonboard.com>\n\t<20230513150754.jnkhuzr3enjsvpq3@lati>\n\t<168399264412.1830511.13431479241942247249@Monstersaurus>","To":"Jacopo Mondi ","Date":"Mon, 15 May 2023 13:39:06 +0100","Message-ID":"<168415434659.3378917.510419833984127080@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 4/4] 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"George Kiagiadakis ,\n\tlibcamera devel ,\n\tRobert Mader ,\n\tWim Taymans ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]