[{"id":27368,"web_url":"https://patchwork.libcamera.org/comment/27368/","msgid":"<20230616140916.GE14547@pendragon.ideasonboard.com>","date":"2023-06-16T14:09:16","subject":"Re: [libcamera-devel] [PATCH 1/2] v4l2: Use SystemDevices\n\tproperties to identify cameras","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 Thu, Jun 15, 2023 at 11:51:32PM +0100, Kieran Bingham via libcamera-devel wrote:\n> The CameraManager->get(dev_t) helper was implemented only to support the\n> V4L2 Adaptation layer, and has been deprecated now that a new camera\n> property - SystemDevices has been introduced.\n> \n> Rework the implementation of getCameraIndex() to use the SystemDevices\n> property and remove reliance on the now deprecated call.\n> \n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>  src/v4l2/v4l2_compat_manager.cpp | 28 +++++++++++++++++++++++-----\n>  1 file changed, 23 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp\n> index 0f7575c54b22..c49124290b42 100644\n> --- a/src/v4l2/v4l2_compat_manager.cpp\n> +++ b/src/v4l2/v4l2_compat_manager.cpp\n> @@ -24,6 +24,7 @@\n>  \n>  #include <libcamera/camera.h>\n>  #include <libcamera/camera_manager.h>\n> +#include <libcamera/property_ids.h>\n>  \n>  #include \"v4l2_camera_file.h\"\n>  \n> @@ -113,14 +114,31 @@ int V4L2CompatManager::getCameraIndex(int fd)\n>  \tif (ret < 0)\n>  \t\treturn -1;\n>  \n> -\tstd::shared_ptr<Camera> target = cm_->get(statbuf.st_rdev);\n> -\tif (!target)\n> -\t\treturn -1;\n> +\tconst dev_t devnum = statbuf.st_rdev;\n\n\tconst int64_t devnum = static_cast<int64_t>(statbuf.st_rdev);\n\nwill save you a static_cast<> in the loop below. Up to you.\n\n>  \n> +\t/*\n> +\t * Iterate each known camera and identify if it reports this nodes\n> +\t * device number in its list of SystemDevices.\n> +\t */\n>  \tauto cameras = cm_->cameras();\n>  \tfor (auto [index, camera] : utils::enumerate(cameras)) {\n> -\t\tif (camera == target)\n> -\t\t\treturn index;\n> +\t\tconst auto devices = camera->properties()\n> +\t\t\t\t\t     .get(properties::SystemDevices)\n> +\t\t\t\t\t     .value_or(std::vector<int64_t>{});\n\nCan we be explicit ?\n\n\t\tconst std::vector<int64_t> devices = camera->properties()\n\t\t\t\t\t.get(properties::SystemDevices)\n\t\t\t\t\t.value_or(std::vector<int64_t>{});\n\n> +\n> +\t\t/*\n> +\t\t * While there may be multiple Cameras that could reference the\n> +\t\t * same device node, we take a first match as a best effort for\n> +\t\t * now.\n> +\t\t *\n> +\t\t * \\todo Consider reworking the V4L2 adaptation layer to stop\n> +\t\t * trying to map video nodes directly to a camera and instead\n> +\t\t * hide all devices that may be used by libcamera and expose a\n> +\t\t * consistent 1:1 mapping with each Camera instance.\n\nI'm not sure what you mean here, could you elaborate ?\n\n> +\t\t */\n> +\t\tfor (const int64_t dev : devices)\n> +\t\t\tif (dev == static_cast<int64_t>(devnum))\n> +\t\t\t\treturn index;\n\n\t\tfor (const int64_t dev : devices) {\n\t\t\tif (dev == static_cast<int64_t>(devnum))\n\t\t\t\treturn index;\n\t\t}\n\n>  \t}\n>  \n>  \treturn -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 3BB4AC322E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 16 Jun 2023 14:09:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A36F8628AE;\n\tFri, 16 Jun 2023 16:09:19 +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 6F7B4628AD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 16 Jun 2023 16:09:17 +0200 (CEST)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 045A1C67;\n\tFri, 16 Jun 2023 16:08:44 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686924559;\n\tbh=CtH3hKO+37J0xfqqn/k2ckt9jEKF2xjpuaq2aumzcEU=;\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=hEWUQtXlYakyXOPXBkJtIiAHttJEi0Kzeb+iPL+orxCgK7lTeOzQ01UBMfOY6a0dH\n\t+PyzZnX6JH0kW1DeKxkbCUmG3RITtjRr47Q5UHTGjktsl/z/jQOLYLriG7tYIRShXH\n\tHWqiGCVibKHRvIOSU834VrzJHwyWFh598Kzc76n4sQhY+QPRhImv/T7GG3rx+hUL4S\n\tCAHUADByzYhVK+2wxIKTgd8CW8Qz2+W9XcpMzQo0rF3Zj3rJk2fdchfvZA97z0Dfzj\n\trpSMECGOFD6WenX8tyaqLhLkRu5fFFZEnf5pL4nTy6hfF2JpwNxlP16wM294CaznOx\n\tTrEuFn5MlI+nA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686924525;\n\tbh=CtH3hKO+37J0xfqqn/k2ckt9jEKF2xjpuaq2aumzcEU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=XFVCIvB+M7z8UYoc8Ssq5HkEOdFlOvZzCgJMpHaOcpAv2DLJryoGi9C+e1LH0YN9O\n\tSeEMq65UNdmcYq5KGhljkbnmX2iEpLKmIeGCnRMoBQ8W8vyuQatIvotrQbs30fQaqP\n\t4SiIkBQF5Brof90KJm9x7Q/BwdnsJhX2JgcwE1NM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"XFVCIvB+\"; dkim-atps=neutral","Date":"Fri, 16 Jun 2023 17:09:16 +0300","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20230616140916.GE14547@pendragon.ideasonboard.com>","References":"<20230615225133.622612-1-kieran.bingham@ideasonboard.com>\n\t<20230615225133.622612-2-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230615225133.622612-2-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/2] v4l2: Use SystemDevices\n\tproperties to identify cameras","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":27370,"web_url":"https://patchwork.libcamera.org/comment/27370/","msgid":"<56CsHuWOcCsnfM7sf2O5yxNcZMLuvB92V1iSKt2YXMHnSHIRnK_mWLftryctHtk-fTuD4L7IhkZm7tYYjREtsCt4cHWoz5Oi58svjZ76PTc=@protonmail.com>","date":"2023-06-16T14:27:27","subject":"Re: [libcamera-devel] [PATCH 1/2] v4l2: Use SystemDevices\n\tproperties to identify cameras","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2023. június 16., péntek 0:51 keltezéssel, Kieran Bingham via libcamera-devel <libcamera-devel@lists.libcamera.org> írta:\n\n> The CameraManager->get(dev_t) helper was implemented only to support the\n> V4L2 Adaptation layer, and has been deprecated now that a new camera\n> property - SystemDevices has been introduced.\n> \n> Rework the implementation of getCameraIndex() to use the SystemDevices\n> property and remove reliance on the now deprecated call.\n> \n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>  src/v4l2/v4l2_compat_manager.cpp | 28 +++++++++++++++++++++++-----\n>  1 file changed, 23 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp\n> index 0f7575c54b22..c49124290b42 100644\n> --- a/src/v4l2/v4l2_compat_manager.cpp\n> +++ b/src/v4l2/v4l2_compat_manager.cpp\n> @@ -24,6 +24,7 @@\n> \n>  #include <libcamera/camera.h>\n>  #include <libcamera/camera_manager.h>\n> +#include <libcamera/property_ids.h>\n> \n>  #include \"v4l2_camera_file.h\"\n> \n> @@ -113,14 +114,31 @@ int V4L2CompatManager::getCameraIndex(int fd)\n>  \tif (ret < 0)\n>  \t\treturn -1;\n> \n> -\tstd::shared_ptr<Camera> target = cm_->get(statbuf.st_rdev);\n> -\tif (!target)\n> -\t\treturn -1;\n> +\tconst dev_t devnum = statbuf.st_rdev;\n> \n> +\t/*\n> +\t * Iterate each known camera and identify if it reports this nodes\n> +\t * device number in its list of SystemDevices.\n> +\t */\n>  \tauto cameras = cm_->cameras();\n>  \tfor (auto [index, camera] : utils::enumerate(cameras)) {\n> -\t\tif (camera == target)\n> -\t\t\treturn index;\n> +\t\tconst auto devices = camera->properties()\n> +\t\t\t\t\t     .get(properties::SystemDevices)\n> +\t\t\t\t\t     .value_or(std::vector<int64_t>{});\n\nWhy a vector? Why not `Span<int64_t>{}`?\n\n\n> +\n> +\t\t/*\n> +\t\t * While there may be multiple Cameras that could reference the\n> +\t\t * same device node, we take a first match as a best effort for\n> +\t\t * now.\n> +\t\t *\n> +\t\t * \\todo Consider reworking the V4L2 adaptation layer to stop\n> +\t\t * trying to map video nodes directly to a camera and instead\n> +\t\t * hide all devices that may be used by libcamera and expose a\n> +\t\t * consistent 1:1 mapping with each Camera instance.\n> +\t\t */\n> +\t\tfor (const int64_t dev : devices)\n> +\t\t\tif (dev == static_cast<int64_t>(devnum))\n> +\t\t\t\treturn index;\n>  \t}\n> \n>  \treturn -1;\n> --\n> 2.34.1\n> \n\n\nRegards,\nBarnabás Pőcze","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 67373BD78E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 16 Jun 2023 14:27:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E1EE6628AE;\n\tFri, 16 Jun 2023 16:27:33 +0200 (CEST)","from mail-40134.protonmail.ch (mail-40134.protonmail.ch\n\t[185.70.40.134])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 427A3628AD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 16 Jun 2023 16:27:32 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686925653;\n\tbh=QkIlrFdzMuLbw4BXLMlz+tkRmzKt0cfH8hQOAY0OY6I=;\n\th=Date:To:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=Lg3MUCM49N66Sr9WA8RZ/YhdqsmORAMIHeU/wVNNBiqdp4Cs0ZIb2PEkhsH0XE4Z0\n\tTfnNBcLglpNXRZZOxHt+XZOucPui1x8YvZ0kY2caKS0ahSte4KmMtg2/r1MWvY2bHe\n\t0ACw28RGahjVE7sSg7tVWMvVORytiZLmrmsdXXl/D42zBgeYgoVO/NXmmEJGHMukrw\n\t6NUSxDN0rXYWG06EJycf92MsFlRaIZGoVn2xgaMfGqZboL11poET3OwvMxp+eKW5vf\n\tkUikIo3gncciYU26oK1Fc9mdfMZ2WCAQMlagvrHO2+jP0SgvzVW4vClq6/7vHNAjeJ\n\tN/ua9Us4Wh6Vw==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1686925651; x=1687184851;\n\tbh=lflmkAqMj3sk2/qwAqEEsxTJowBgL7lBAdsR0EEdgRA=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=x7joeOIxg2/BWMHtOf12wY8LPeg2wteZEOXS3xy5V0QkJqJqcd+ceCrBHnQb6wMsp\n\tUHkq512PQTnSA4m/SHX8bj8YYXg0NtSj5KWCiB+p2THvViL10bytJSdrxqXErkjoU0\n\tILb6wPe8dlMiO3TDbVN0VY1kI35bWUK50jFnJgSooUM6laaibTI/IlpCdS28XEPDI8\n\tNsy1wmDsWkmtI/8WIu2uIERkUvj9nXmQMRmz8DOqjv4Z7eD5WsORp3Ua4XPTh/UkWc\n\tafeJCgrhGyUNHmbEw5eGs88DMNY86kgltI6Zu5Q+c5czYJWlAuMNkzBfHGlM3oZQSq\n\tYToXGqH7PjJCw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=protonmail.com\n\theader.i=@protonmail.com\n\theader.b=\"x7joeOIx\"; dkim-atps=neutral","Date":"Fri, 16 Jun 2023 14:27:27 +0000","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<56CsHuWOcCsnfM7sf2O5yxNcZMLuvB92V1iSKt2YXMHnSHIRnK_mWLftryctHtk-fTuD4L7IhkZm7tYYjREtsCt4cHWoz5Oi58svjZ76PTc=@protonmail.com>","In-Reply-To":"<20230615225133.622612-2-kieran.bingham@ideasonboard.com>","References":"<20230615225133.622612-1-kieran.bingham@ideasonboard.com>\n\t<20230615225133.622612-2-kieran.bingham@ideasonboard.com>","Feedback-ID":"20568564:user:proton","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH 1/2] v4l2: Use SystemDevices\n\tproperties to identify cameras","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":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze_via_libcamera-devel?=\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.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":27376,"web_url":"https://patchwork.libcamera.org/comment/27376/","msgid":"<168703983599.2319555.15200605218940033889@Monstersaurus>","date":"2023-06-17T22:10:35","subject":"Re: [libcamera-devel] [PATCH 1/2] v4l2: Use SystemDevices\n\tproperties to identify cameras","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2023-06-16 15:27:27)\n> Hi\n> \n> \n> 2023. június 16., péntek 0:51 keltezéssel, Kieran Bingham via libcamera-devel <libcamera-devel@lists.libcamera.org> írta:\n> \n> > The CameraManager->get(dev_t) helper was implemented only to support the\n> > V4L2 Adaptation layer, and has been deprecated now that a new camera\n> > property - SystemDevices has been introduced.\n> > \n> > Rework the implementation of getCameraIndex() to use the SystemDevices\n> > property and remove reliance on the now deprecated call.\n> > \n> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > ---\n> >  src/v4l2/v4l2_compat_manager.cpp | 28 +++++++++++++++++++++++-----\n> >  1 file changed, 23 insertions(+), 5 deletions(-)\n> > \n> > diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp\n> > index 0f7575c54b22..c49124290b42 100644\n> > --- a/src/v4l2/v4l2_compat_manager.cpp\n> > +++ b/src/v4l2/v4l2_compat_manager.cpp\n> > @@ -24,6 +24,7 @@\n> > \n> >  #include <libcamera/camera.h>\n> >  #include <libcamera/camera_manager.h>\n> > +#include <libcamera/property_ids.h>\n> > \n> >  #include \"v4l2_camera_file.h\"\n> > \n> > @@ -113,14 +114,31 @@ int V4L2CompatManager::getCameraIndex(int fd)\n> >       if (ret < 0)\n> >               return -1;\n> > \n> > -     std::shared_ptr<Camera> target = cm_->get(statbuf.st_rdev);\n> > -     if (!target)\n> > -             return -1;\n> > +     const dev_t devnum = statbuf.st_rdev;\n> > \n> > +     /*\n> > +      * Iterate each known camera and identify if it reports this nodes\n> > +      * device number in its list of SystemDevices.\n> > +      */\n> >       auto cameras = cm_->cameras();\n> >       for (auto [index, camera] : utils::enumerate(cameras)) {\n> > -             if (camera == target)\n> > -                     return index;\n> > +             const auto devices = camera->properties()\n> > +                                          .get(properties::SystemDevices)\n> > +                                          .value_or(std::vector<int64_t>{});\n> \n> Why a vector? Why not `Span<int64_t>{}`?\n\nI think I just used vector because that's the control type. But I think\nyou're right, a Span could potentially work here.\n\nI'll try it and if it works I'll use that.\n\n--\nKieran\n\n\n> \n> \n> > +\n> > +             /*\n> > +              * While there may be multiple Cameras that could reference the\n> > +              * same device node, we take a first match as a best effort for\n> > +              * now.\n> > +              *\n> > +              * \\todo Consider reworking the V4L2 adaptation layer to stop\n> > +              * trying to map video nodes directly to a camera and instead\n> > +              * hide all devices that may be used by libcamera and expose a\n> > +              * consistent 1:1 mapping with each Camera instance.\n> > +              */\n> > +             for (const int64_t dev : devices)\n> > +                     if (dev == static_cast<int64_t>(devnum))\n> > +                             return index;\n> >       }\n> > \n> >       return -1;\n> > --\n> > 2.34.1\n> > \n> \n> \n> Regards,\n> Barnabás Pőcze","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 750F3C322E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 17 Jun 2023 22:10:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 82EA8628B7;\n\tSun, 18 Jun 2023 00:10:41 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 87AF6628AD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 18 Jun 2023 00:10:39 +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 5AEAF440;\n\tSun, 18 Jun 2023 00:10:06 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1687039841;\n\tbh=njZGmQJMXv3FQyubnYhe1uE/tuH7lmIJ3iJD9Esfkd8=;\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=L5ZflUO3azIZ9J1qyGZ9EprCnk43rpRQrnocrnXO4+f+Slunigns7BhPpXOCoNp5o\n\t+C4QWC7M0W36rk7K3MM0ThSgufN89PggeW4aXyjAfkiibDhwyJXJHJwyURjAH0Rt9A\n\t2BDdkc55ks9aFDtk53WwI4G4XZKtwN1daVITN2cfdbO8OGiSTu31t2V1Jtptt2dWgy\n\tbW5QyR8uAhxk/jDmx4nicByHXd/bWRLBGdzhm/9bwJ1bMWASa8f9KEsExhWcfHIj6r\n\txRlkwFKRkEWG4+wm6qPNrzI0YI4M2ZOpNVbanTljg5dWA5lrpMKvxUXts1QT3RT44o\n\t3XERjCcriU0Mw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1687039806;\n\tbh=njZGmQJMXv3FQyubnYhe1uE/tuH7lmIJ3iJD9Esfkd8=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=Wj3ZBy34RK//5tiia+5wEceRyJGa0Wztc/0UAKNCrT8TJ5+v32kbvc80xzaE4wMQ4\n\tiNYVdx/F63QX9UNK+ZfXsyBMGLXxaOz1yzAOvoMF8ahDhE1v8KIV6Z9Nk5zSe/J4ew\n\tXwqBSJ0zOU7J4QF7MAMujaVudTMY2ehYOPf/3cMw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Wj3ZBy34\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<56CsHuWOcCsnfM7sf2O5yxNcZMLuvB92V1iSKt2YXMHnSHIRnK_mWLftryctHtk-fTuD4L7IhkZm7tYYjREtsCt4cHWoz5Oi58svjZ76PTc=@protonmail.com>","References":"<20230615225133.622612-1-kieran.bingham@ideasonboard.com>\n\t<20230615225133.622612-2-kieran.bingham@ideasonboard.com>\n\t<56CsHuWOcCsnfM7sf2O5yxNcZMLuvB92V1iSKt2YXMHnSHIRnK_mWLftryctHtk-fTuD4L7IhkZm7tYYjREtsCt4cHWoz5Oi58svjZ76PTc=@protonmail.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>","Date":"Sat, 17 Jun 2023 23:10:35 +0100","Message-ID":"<168703983599.2319555.15200605218940033889@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH 1/2] v4l2: Use SystemDevices\n\tproperties to identify cameras","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":27377,"web_url":"https://patchwork.libcamera.org/comment/27377/","msgid":"<168704020725.2319555.5426709393704287761@Monstersaurus>","date":"2023-06-17T22:16:47","subject":"Re: [libcamera-devel] [PATCH 1/2] v4l2: Use SystemDevices\n\tproperties to identify cameras","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-16 15:09:16)\n> Hi Kieran,\n> \n> Thank you for the patch.\n> \n> On Thu, Jun 15, 2023 at 11:51:32PM +0100, Kieran Bingham via libcamera-devel wrote:\n> > The CameraManager->get(dev_t) helper was implemented only to support the\n> > V4L2 Adaptation layer, and has been deprecated now that a new camera\n> > property - SystemDevices has been introduced.\n> > \n> > Rework the implementation of getCameraIndex() to use the SystemDevices\n> > property and remove reliance on the now deprecated call.\n> > \n> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > ---\n> >  src/v4l2/v4l2_compat_manager.cpp | 28 +++++++++++++++++++++++-----\n> >  1 file changed, 23 insertions(+), 5 deletions(-)\n> > \n> > diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp\n> > index 0f7575c54b22..c49124290b42 100644\n> > --- a/src/v4l2/v4l2_compat_manager.cpp\n> > +++ b/src/v4l2/v4l2_compat_manager.cpp\n> > @@ -24,6 +24,7 @@\n> >  \n> >  #include <libcamera/camera.h>\n> >  #include <libcamera/camera_manager.h>\n> > +#include <libcamera/property_ids.h>\n> >  \n> >  #include \"v4l2_camera_file.h\"\n> >  \n> > @@ -113,14 +114,31 @@ int V4L2CompatManager::getCameraIndex(int fd)\n> >       if (ret < 0)\n> >               return -1;\n> >  \n> > -     std::shared_ptr<Camera> target = cm_->get(statbuf.st_rdev);\n> > -     if (!target)\n> > -             return -1;\n> > +     const dev_t devnum = statbuf.st_rdev;\n> \n>         const int64_t devnum = static_cast<int64_t>(statbuf.st_rdev);\n> \n> will save you a static_cast<> in the loop below. Up to you.\n> \n> >  \n> > +     /*\n> > +      * Iterate each known camera and identify if it reports this nodes\n> > +      * device number in its list of SystemDevices.\n> > +      */\n> >       auto cameras = cm_->cameras();\n> >       for (auto [index, camera] : utils::enumerate(cameras)) {\n> > -             if (camera == target)\n> > -                     return index;\n> > +             const auto devices = camera->properties()\n> > +                                          .get(properties::SystemDevices)\n> > +                                          .value_or(std::vector<int64_t>{});\n> \n> Can we be explicit ?\n> \n>                 const std::vector<int64_t> devices = camera->properties()\n>                                         .get(properties::SystemDevices)\n>                                         .value_or(std::vector<int64_t>{});\n> \n> > +\n> > +             /*\n> > +              * While there may be multiple Cameras that could reference the\n> > +              * same device node, we take a first match as a best effort for\n> > +              * now.\n> > +              *\n> > +              * \\todo Consider reworking the V4L2 adaptation layer to stop\n> > +              * trying to map video nodes directly to a camera and instead\n> > +              * hide all devices that may be used by libcamera and expose a\n> > +              * consistent 1:1 mapping with each Camera instance.\n> \n> I'm not sure what you mean here, could you elaborate ?\n\nWhat I mean is I think this could all be improved. In particular to stop\nthe issue where if mulitple cameras reference the same video node\n(unicam for instance) they would only ever work for the first one.\n\nIf there were time on this I would investigate making this layer\nprovides a better mapping where each camera is /dev/videoN directly.\n\n\n\n> > +              */\n> > +             for (const int64_t dev : devices)\n> > +                     if (dev == static_cast<int64_t>(devnum))\n> > +                             return index;\n> \n>                 for (const int64_t dev : devices) {\n>                         if (dev == static_cast<int64_t>(devnum))\n>                                 return index;\n>                 }\n> \n> >       }\n> >  \n> >       return -1;\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 6E1B9BD78E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 17 Jun 2023 22:16:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B11A2628AE;\n\tSun, 18 Jun 2023 00:16:52 +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 E8397628AD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 18 Jun 2023 00:16:50 +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 B0C5E922;\n\tSun, 18 Jun 2023 00:16:17 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1687040212;\n\tbh=GwUyJCBRh55uS//eNdymQgf+7SCWYI+97tzJKGy8jLo=;\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=Ql5fqMMtQuai0QAvvjdHeKOTHp7wOHyB5VOO+/XKgRRn2GaF/vG0Nwxq2Q4K6HSYq\n\t5hgatFTYPhJoz6mh9lH8E2WwpRxMV+MRuCbFksK0X/i4bDTzUhhol7HcXCwoFnKEZA\n\tc/UOSuIeIH/ICb9xGCwn2EZbNTztQY9oHSdora2+ZYSEiywf6W74RKxKIpppNVup1f\n\t5VSFXHEu6YDqlz/zmz+jxcYSM3H24RIp89b1Ug+n0LoQgODA854K0nKnGEEOws8POe\n\ta/NwhC7t9GNGFH73SWIuRSe326l2ADqnO2VqwFEiDBZesKbgxsUYD3KkPixywUs9FL\n\tZFtR6Quj+dVhg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1687040177;\n\tbh=GwUyJCBRh55uS//eNdymQgf+7SCWYI+97tzJKGy8jLo=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=Xt/gfulstBsHNY06NAusTmtGAk9LkdzNpmpgVumnTpf9pBz0Y8T5vzoSWpYYUHcOM\n\tnqCJq0f6njiOG6SCK3dsus60diE9dgk0sJjRld0ZaZjrnbozYY31PYyDtaYAD8S4o0\n\tlnRrGgta/6MGcCS3TdorIHZHw80LV4Tc84v4BLzQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Xt/gfuls\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230616140916.GE14547@pendragon.ideasonboard.com>","References":"<20230615225133.622612-1-kieran.bingham@ideasonboard.com>\n\t<20230615225133.622612-2-kieran.bingham@ideasonboard.com>\n\t<20230616140916.GE14547@pendragon.ideasonboard.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Date":"Sat, 17 Jun 2023 23:16:47 +0100","Message-ID":"<168704020725.2319555.5426709393704287761@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH 1/2] v4l2: Use SystemDevices\n\tproperties to identify cameras","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":27379,"web_url":"https://patchwork.libcamera.org/comment/27379/","msgid":"<lcRpxqXRU5V3xxixzYBzvQXbT-6Bh7-o_qwheXqJCZAERq2qUIaLMkHInNP3D_2uZZqVGoAouqMw29Hj6c5gYWqkML28fAaRBVP9RHMpQr8=@protonmail.com>","date":"2023-06-17T22:22:45","subject":"Re: [libcamera-devel] [PATCH 1/2] v4l2: Use SystemDevices\n\tproperties to identify cameras","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Hi\n\n\n2023. június 18., vasárnap 0:10 keltezéssel, Kieran Bingham <kieran.bingham@ideasonboard.com> írta:\n\n> Quoting Barnabás Pőcze (2023-06-16 15:27:27)\n> > Hi\n> >\n> >\n> > 2023. június 16., péntek 0:51 keltezéssel, Kieran Bingham via libcamera-devel <libcamera-devel@lists.libcamera.org> írta:\n> >\n> > > The CameraManager->get(dev_t) helper was implemented only to support the\n> > > V4L2 Adaptation layer, and has been deprecated now that a new camera\n> > > property - SystemDevices has been introduced.\n> > >\n> > > Rework the implementation of getCameraIndex() to use the SystemDevices\n> > > property and remove reliance on the now deprecated call.\n> > >\n> > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > > ---\n> > >  src/v4l2/v4l2_compat_manager.cpp | 28 +++++++++++++++++++++++-----\n> > >  1 file changed, 23 insertions(+), 5 deletions(-)\n> > >\n> > > diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp\n> > > index 0f7575c54b22..c49124290b42 100644\n> > > --- a/src/v4l2/v4l2_compat_manager.cpp\n> > > +++ b/src/v4l2/v4l2_compat_manager.cpp\n> > > @@ -24,6 +24,7 @@\n> > >\n> > >  #include <libcamera/camera.h>\n> > >  #include <libcamera/camera_manager.h>\n> > > +#include <libcamera/property_ids.h>\n> > >\n> > >  #include \"v4l2_camera_file.h\"\n> > >\n> > > @@ -113,14 +114,31 @@ int V4L2CompatManager::getCameraIndex(int fd)\n> > >       if (ret < 0)\n> > >               return -1;\n> > >\n> > > -     std::shared_ptr<Camera> target = cm_->get(statbuf.st_rdev);\n> > > -     if (!target)\n> > > -             return -1;\n> > > +     const dev_t devnum = statbuf.st_rdev;\n> > >\n> > > +     /*\n> > > +      * Iterate each known camera and identify if it reports this nodes\n> > > +      * device number in its list of SystemDevices.\n> > > +      */\n> > >       auto cameras = cm_->cameras();\n> > >       for (auto [index, camera] : utils::enumerate(cameras)) {\n> > > -             if (camera == target)\n> > > -                     return index;\n> > > +             const auto devices = camera->properties()\n> > > +                                          .get(properties::SystemDevices)\n> > > +                                          .value_or(std::vector<int64_t>{});\n> >\n> > Why a vector? Why not `Span<int64_t>{}`?\n> \n> I think I just used vector because that's the control type. But I think\n> you're right, a Span could potentially work here.\n> \n> I'll try it and if it works I'll use that.\n> [...]\n\nAs far as I can see, array-like controls have type `Control<Span<T>>`, so `.get(...)`\nreturns a span. That's the reason I asked, it seemed a bit odd that you used a vector there.\n\n\nRegards,\nBarnabás Pőcze","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 F3353C322E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 17 Jun 2023 22:22:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 61FF0628B6;\n\tSun, 18 Jun 2023 00:22:54 +0200 (CEST)","from mail-40134.protonmail.ch (mail-40134.protonmail.ch\n\t[185.70.40.134])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D8E99628AE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 18 Jun 2023 00:22:51 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1687040574;\n\tbh=TAEDGmd+GTM4F44v5CVQtsoqJVyHMirhXdTy6qd98/w=;\n\th=Date:To:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=c1KZPbvsa7Lozyy0xOhWPNKni75h/+OdqWQOpX2MqFOEkTIrtKX0Wg+H3JkZ1YoD+\n\tHM7H4aDcKjZ/7zs1gRolhUQ6w4RNd/SEGEm05vhkueqUhzAoQvFQ3Vz3BG3cj4dkOf\n\tlAUQZiaQ3mf2izW5muJEZEIUb5VaGeyVPIJF/KFOmr+cY16VNuYOrWO2IpEEur3lZw\n\tmOhbx1ujzAGF/oQCyZzC087h1Q7a/Y74YZMuVP9g7FKyq94v+RnFoimW0GmOlwatDa\n\tzl8PzZ4kTLiEuSHQ7XZzSe8o0FjXLKzx3knuDP3Af4FXmrpDdAFzpkdJEC1RJZ6UDc\n\tisTCb3koMF0Rw==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1687040570; x=1687299770;\n\tbh=Pf+mLKU43VWH2JWhNtg+vroXroG0aHFvMgpKdu7n8Yg=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=IkBRTafLB+bcZZb9SSiK7dOmr+//n6rwiCPukDgyjZhQooT/xni6Ul/TQSfa9cH4E\n\tnpbky5clolfppsVblMbAKMVms8Qzz07AEizJhUW1Za+EKF8BRgpC6h03x3ECnJAY+9\n\tNzHYcU0XMahNB1yeI/eVYl3MNmRj+cVlgF+8B5w0VT12+E4EGslYtUi8r9gVlOzPqD\n\tyKIwtygl0dAwI/bmOtwFBZETE9SHd+RpRCbx90HJKTTT7Yb1jhlyWClbn43F1gI7OQ\n\t7Yt09zykFNt051yglEB3NiVeuJkp7ejsp+JAvivIGPxYun7M8L2PMBiGp2g15ZlpRe\n\tkFIitZL0uemOw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=protonmail.com\n\theader.i=@protonmail.com\n\theader.b=\"IkBRTafL\"; dkim-atps=neutral","Date":"Sat, 17 Jun 2023 22:22:45 +0000","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<lcRpxqXRU5V3xxixzYBzvQXbT-6Bh7-o_qwheXqJCZAERq2qUIaLMkHInNP3D_2uZZqVGoAouqMw29Hj6c5gYWqkML28fAaRBVP9RHMpQr8=@protonmail.com>","In-Reply-To":"<168703983599.2319555.15200605218940033889@Monstersaurus>","References":"<20230615225133.622612-1-kieran.bingham@ideasonboard.com>\n\t<20230615225133.622612-2-kieran.bingham@ideasonboard.com>\n\t<56CsHuWOcCsnfM7sf2O5yxNcZMLuvB92V1iSKt2YXMHnSHIRnK_mWLftryctHtk-fTuD4L7IhkZm7tYYjREtsCt4cHWoz5Oi58svjZ76PTc=@protonmail.com>\n\t<168703983599.2319555.15200605218940033889@Monstersaurus>","Feedback-ID":"20568564:user:proton","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH 1/2] v4l2: Use SystemDevices\n\tproperties to identify cameras","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":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze_via_libcamera-devel?=\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.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":27382,"web_url":"https://patchwork.libcamera.org/comment/27382/","msgid":"<168704159046.2319555.8417760664892944530@Monstersaurus>","date":"2023-06-17T22:39:50","subject":"Re: [libcamera-devel] [PATCH 1/2] v4l2: Use SystemDevices\n\tproperties to identify cameras","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-17 23:16:47)\n> Quoting Laurent Pinchart (2023-06-16 15:09:16)\n> > Hi Kieran,\n> > \n> > Thank you for the patch.\n> > \n> > On Thu, Jun 15, 2023 at 11:51:32PM +0100, Kieran Bingham via libcamera-devel wrote:\n> > > The CameraManager->get(dev_t) helper was implemented only to support the\n> > > V4L2 Adaptation layer, and has been deprecated now that a new camera\n> > > property - SystemDevices has been introduced.\n> > > \n> > > Rework the implementation of getCameraIndex() to use the SystemDevices\n> > > property and remove reliance on the now deprecated call.\n> > > \n> > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > > ---\n> > >  src/v4l2/v4l2_compat_manager.cpp | 28 +++++++++++++++++++++++-----\n> > >  1 file changed, 23 insertions(+), 5 deletions(-)\n> > > \n> > > diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp\n> > > index 0f7575c54b22..c49124290b42 100644\n> > > --- a/src/v4l2/v4l2_compat_manager.cpp\n> > > +++ b/src/v4l2/v4l2_compat_manager.cpp\n> > > @@ -24,6 +24,7 @@\n> > >  \n> > >  #include <libcamera/camera.h>\n> > >  #include <libcamera/camera_manager.h>\n> > > +#include <libcamera/property_ids.h>\n> > >  \n> > >  #include \"v4l2_camera_file.h\"\n> > >  \n> > > @@ -113,14 +114,31 @@ int V4L2CompatManager::getCameraIndex(int fd)\n> > >       if (ret < 0)\n> > >               return -1;\n> > >  \n> > > -     std::shared_ptr<Camera> target = cm_->get(statbuf.st_rdev);\n> > > -     if (!target)\n> > > -             return -1;\n> > > +     const dev_t devnum = statbuf.st_rdev;\n> > \n> >         const int64_t devnum = static_cast<int64_t>(statbuf.st_rdev);\n> > \n> > will save you a static_cast<> in the loop below. Up to you.\n\nI'd rather keep it clear that statbuf.st_rdev is a dev_t. There's not a\ngreat deal of difference in a cast here or a cast below otherwise.\n\n> > \n> > >  \n> > > +     /*\n> > > +      * Iterate each known camera and identify if it reports this nodes\n> > > +      * device number in its list of SystemDevices.\n> > > +      */\n> > >       auto cameras = cm_->cameras();\n> > >       for (auto [index, camera] : utils::enumerate(cameras)) {\n> > > -             if (camera == target)\n> > > -                     return index;\n> > > +             const auto devices = camera->properties()\n> > > +                                          .get(properties::SystemDevices)\n> > > +                                          .value_or(std::vector<int64_t>{});\n> > \n> > Can we be explicit ?\n> > \n> >                 const std::vector<int64_t> devices = camera->properties()\n> >                                         .get(properties::SystemDevices)\n> >                                         .value_or(std::vector<int64_t>{});\n\nI'll use the following (Thanks to Barnabás)\n\ndiff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp\nindex c49124290b42..fa3cb01bb798 100644\n--- a/src/v4l2/v4l2_compat_manager.cpp\n+++ b/src/v4l2/v4l2_compat_manager.cpp\n@@ -122,9 +122,9 @@ int V4L2CompatManager::getCameraIndex(int fd)\n \t */\n \tauto cameras = cm_->cameras();\n \tfor (auto [index, camera] : utils::enumerate(cameras)) {\n-\t\tconst auto devices = camera->properties()\n+\t\tSpan<const int64_t> devices = camera->properties()\n \t\t\t\t\t     .get(properties::SystemDevices)\n-\t\t\t\t\t     .value_or(std::vector<int64_t>{});\n+\t\t\t\t\t     .value_or(Span<int64_t>{});\n\n \t\t/*\n \t\t * While there may be multiple Cameras that could reference the\n@@ -136,9 +136,10 @@ int V4L2CompatManager::getCameraIndex(int fd)\n \t\t * hide all devices that may be used by libcamera and expose a\n \t\t * consistent 1:1 mapping with each Camera instance.\n \t\t */\n-\t\tfor (const int64_t dev : devices)\n+\t\tfor (const int64_t dev : devices) {\n \t\t\tif (dev == static_cast<int64_t>(devnum))\n \t\t\t\treturn index;\n+\t\t}\n \t}\n\n \treturn -1;\n\n\n> > \n> > > +\n> > > +             /*\n> > > +              * While there may be multiple Cameras that could reference the\n> > > +              * same device node, we take a first match as a best effort for\n> > > +              * now.\n> > > +              *\n> > > +              * \\todo Consider reworking the V4L2 adaptation layer to stop\n> > > +              * trying to map video nodes directly to a camera and instead\n> > > +              * hide all devices that may be used by libcamera and expose a\n> > > +              * consistent 1:1 mapping with each Camera instance.\n> > \n> > I'm not sure what you mean here, could you elaborate ?\n> \n> What I mean is I think this could all be improved. In particular to stop\n> the issue where if mulitple cameras reference the same video node\n> (unicam for instance) they would only ever work for the first one.\n> \n> If there were time on this I would investigate making this layer\n> provides a better mapping where each camera is /dev/videoN directly.\n> \n> \n> \n> > > +              */\n> > > +             for (const int64_t dev : devices)\n> > > +                     if (dev == static_cast<int64_t>(devnum))\n> > > +                             return index;\n> > \n> >                 for (const int64_t dev : devices) {\n> >                         if (dev == static_cast<int64_t>(devnum))\n> >                                 return index;\n> >                 }\n> > \n> > >       }\n> > >  \n> > >       return -1;\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 B5973BD78E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 17 Jun 2023 22:39:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 266AB628B8;\n\tSun, 18 Jun 2023 00:39:55 +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 6AA8C628AE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 18 Jun 2023 00:39:53 +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 5EA0C922;\n\tSun, 18 Jun 2023 00:39:20 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1687041595;\n\tbh=SOlvp8pPUP3cmGpzN7R/Gt2r5sVbY7D45fL2pllPbg4=;\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=M1AFc1GKpLoGh29xRIv6/YH8s0Dh23sgUSt1VkERLICLzSlWGEs3dvmn0MPuNSZbS\n\tytKYjxCXG11/ONf7DM/N1uH98ttkiSKGWvqEWijUMD31B3DaTrTpjmK4GhBxYKvjo5\n\t1qFugL2c5V7Y8a5en+iCQzhkK3OM0mqgHJcOMJpFMI+mjvSrDnrCXczXZvufhZtiXA\n\tdw7v/YateFOmuitAis19BgVMQYqZ8OgTv5gZ+WPaSsgrC6TiI06VG7xW+PIJKKZRk6\n\t47dKdKAWJqtNiO/u0BrXNRLmetvvnqnzUXkoeYzDssAVVNRLePCxWUx28/UCilF4IP\n\tsMWiu5vj3jj/w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1687041560;\n\tbh=SOlvp8pPUP3cmGpzN7R/Gt2r5sVbY7D45fL2pllPbg4=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=c2CI1bN8LsAuXHwsYS/bSx/ma/EAmEQE/GvfkpIcfiCDGs0KI6DM/Q7WjZ7iBAm42\n\tPRUCzzQgZrpP8xVdnQp0Q9ZyUp/stT2uMfNkLoDthw2RTggtEuS/9DVRpYnIjnU52K\n\tcMBIOfnnDYUhRTCNMWi+xG8FH7lcfgi47a7dICBQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"c2CI1bN8\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<168704020725.2319555.5426709393704287761@Monstersaurus>","References":"<20230615225133.622612-1-kieran.bingham@ideasonboard.com>\n\t<20230615225133.622612-2-kieran.bingham@ideasonboard.com>\n\t<20230616140916.GE14547@pendragon.ideasonboard.com>\n\t<168704020725.2319555.5426709393704287761@Monstersaurus>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Date":"Sat, 17 Jun 2023 23:39:50 +0100","Message-ID":"<168704159046.2319555.8417760664892944530@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH 1/2] v4l2: Use SystemDevices\n\tproperties to identify cameras","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>"}}]