[{"id":11439,"web_url":"https://patchwork.libcamera.org/comment/11439/","msgid":"<20200720130707.wteidbf6ecjdwrvh@uno.localdomain>","date":"2020-07-20T13:07:07","subject":"Re: [libcamera-devel] [PATCH 0/9] libcamera: camera: Add camera ID","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Sat, Jul 18, 2020 at 03:23:15PM +0200, Niklas Söderlund wrote:\n> Hello,\n>\n\nminors apart that I will reply to patch-by-patch, I have two questions\non the series in general\n\n1) The series introduce and 'id' to be used alongside and\nalternatively to the camera 'name'. This might just be a matter of\nterminology, but I find this a bit confusing. Ideally, the 'name'\nshould be the unique part, to which a 'model' (to mimic the API we\nhave for camera sensors) could be added.\n\n2) The API and the cam implementation allow to use 'name' and 'id'\ninterchangibily. Is this a good thing ? Applications should always use\n'id' when interfacing to libcamera, and ideally 'name' should be a\nshortcut for users, to easily select a camera (provided it is unique).\nIf I'm not mistaken it is currently possible to ask libcamera for a\n'camera' name, should we drop this and implement that part in the\napplication ? 'cam' and alike can and should use mnemonic names to\nusers, but should libcamera do the same? Do we want an API that allows\nselecting camera with a name which is not guaranteed to be unique and\nconsistent ? I would say we don't...\n\nWhat do you think ?\n\n> This series aims to add a ID to each camera in addition to it's more\n> user-friendly name. The ID is unique and persistent between reboots of\n> the same system. The use-case for this is to create a single\n> machine-friendly ID that can be stored and used to always resolve to the\n> same camera.\n>\n> The idea on how to generate a ID is to take the sysfs path of the sensor\n> device which is part of each camera pipeline. As the path describes the\n> location of the sensor hardware it is persistent across reboots and as\n> the path is read from sysfs it's guaranteed to be unique in the system.\n>\n> For pipelines that do not have a sensor (UVC) the sysfs path of the main\n> video device is used instead. That path resolves to the USB device and\n> includes the USB bus information so it satisfy the ID requirements.\n>\n> While working with this problem it became apparent that two pipelines\n> diverge from the others on how they name their cameras, raspberrypi and\n> vimc. This series aligns these two and adds a helper to avoid such\n> situations in the future. Unfortunately this means the user-friendly\n> name of the sensor changes but this proves the need for a\n> machine-friendly ID which luckily this series also adds :-)\n>\n> Before this series camera user-friendly names on different systems\n> looked like this (I do not have access to a simple pipeline device):\n>\n> - ipu3\n>         ov13858 8-0010\n>         ov5670 10-0036\n> - raspberrypi\n>         imx219\n> - rkisp1\n>         ov5695 7-0036\n>         ov2685 7-003c\n> - uvcvideo\n>         Logitech Webcam C930e\n> - vimc\n>         VIMC Sensor B\n>\n> With this series applied the user-friendly names machine-friendly ID on\n> the same systems look like this:\n>\n> The format is:\n>     <user-friendly name> (<machine-friendly ID>)\n>\n> - ipu3\n>         ov13858 8-0010 (pci0000:00/0000:00:15.2/i2c_designware.2/i2c-8/i2c-OVTID858:00)\n>         ov5670 10-0036 (pci0000:00/0000:00:19.2/i2c_designware.5/i2c-10/i2c-INT3479:00)\n> - raspberrypi\n>         imx219 10-0010 (platform/soc/3f205000.i2c/i2c-11/i2c-10/10-0010)\n> - rkisp1\n>         ov5695 7-0036 (platform/ff160000.i2c/i2c-7/7-0036)\n>         ov2685 7-003c (platform/ff160000.i2c/i2c-7/7-003c)\n> - uvcvideo\n>         Logitech Webcam C930e (pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0)\n> - vimc\n>         Sensor B (platform/vimc.0)\n>\n> Where it previously where possible to select a camera by its\n> user-friendly name its now possible to also select it using its\n> machine-friendly one. The following is therefor two equivalent\n> commands:\n>\n>     $ cam -c \"Logitech Webcam C930e\" -C\n>     $ cam -c \"pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0\" -C\n>\n> Niklas Söderlund (9):\n>   libcamera: v4l2_device: Add method to lookup device path\n>   libcamera: camera_sensor: Expose a sensor ID\n>   libcamera: camera: Add camera ID\n>   libcamera: camera_manager: Enforce unique camera IDs\n>   libcamera: camera_manager: Try to match camera IDs first\n>   libcamera: pipeline: vimc: Align camera name\n>   libcamera: pipeline: raspberrypi: Align camera name\n>   libcamera: camera: Add create() that operates on CameraSensor\n>   cam: Print camera IDs when listing cameras\n>\n>  include/libcamera/camera.h                    | 11 +++-\n>  include/libcamera/internal/camera_sensor.h    |  2 +\n>  include/libcamera/internal/v4l2_device.h      |  1 +\n>  src/cam/main.cpp                              |  3 +-\n>  src/libcamera/camera.cpp                      | 54 +++++++++++++++----\n>  src/libcamera/camera_manager.cpp              | 13 +++++\n>  src/libcamera/camera_sensor.cpp               | 17 ++++++\n>  src/libcamera/pipeline/ipu3/ipu3.cpp          |  9 ++--\n>  .../pipeline/raspberrypi/raspberrypi.cpp      |  3 +-\n>  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  2 +-\n>  src/libcamera/pipeline/simple/simple.cpp      |  3 +-\n>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  7 ++-\n>  src/libcamera/pipeline/vimc/vimc.cpp          |  4 +-\n>  src/libcamera/v4l2_device.cpp                 | 27 ++++++++++\n>  test/camera/buffer_import.cpp                 |  2 +-\n>  test/camera/capture.cpp                       |  2 +-\n>  test/camera/configuration_default.cpp         |  2 +-\n>  test/camera/configuration_set.cpp             |  2 +-\n>  test/camera/statemachine.cpp                  |  2 +-\n>  test/controls/control_info_map.cpp            |  2 +-\n>  test/controls/control_list.cpp                |  2 +-\n>  21 files changed, 138 insertions(+), 32 deletions(-)\n>\n> --\n> 2.27.0\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 01C20BD792\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 20 Jul 2020 13:03:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 95A8C60537;\n\tMon, 20 Jul 2020 15:03:34 +0200 (CEST)","from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A2BA36039F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 20 Jul 2020 15:03:33 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 1968EE0008;\n\tMon, 20 Jul 2020 13:03:32 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Mon, 20 Jul 2020 15:07:07 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<20200720130707.wteidbf6ecjdwrvh@uno.localdomain>","References":"<20200718132324.867815-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200718132324.867815-1-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH 0/9] libcamera: camera: Add camera ID","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11441,"web_url":"https://patchwork.libcamera.org/comment/11441/","msgid":"<20200720144826.bxtbag22ki67wkoj@uno.localdomain>","date":"2020-07-20T14:48:26","subject":"Re: [libcamera-devel] [PATCH 0/9] libcamera: camera: Add camera ID","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Sat, Jul 18, 2020 at 03:23:15PM +0200, Niklas Söderlund wrote:\n> Hello,\n>\n> This series aims to add a ID to each camera in addition to it's more\n> user-friendly name. The ID is unique and persistent between reboots of\n> the same system. The use-case for this is to create a single\n> machine-friendly ID that can be stored and used to always resolve to the\n> same camera.\n>\n> The idea on how to generate a ID is to take the sysfs path of the sensor\n> device which is part of each camera pipeline. As the path describes the\n> location of the sensor hardware it is persistent across reboots and as\n> the path is read from sysfs it's guaranteed to be unique in the system.\n>\n> For pipelines that do not have a sensor (UVC) the sysfs path of the main\n> video device is used instead. That path resolves to the USB device and\n> includes the USB bus information so it satisfy the ID requirements.\n>\n> While working with this problem it became apparent that two pipelines\n> diverge from the others on how they name their cameras, raspberrypi and\n> vimc. This series aligns these two and adds a helper to avoid such\n> situations in the future. Unfortunately this means the user-friendly\n> name of the sensor changes but this proves the need for a\n> machine-friendly ID which luckily this series also adds :-)\n>\n> Before this series camera user-friendly names on different systems\n> looked like this (I do not have access to a simple pipeline device):\n>\n> - ipu3\n>         ov13858 8-0010\n>         ov5670 10-0036\n> - raspberrypi\n>         imx219\n> - rkisp1\n>         ov5695 7-0036\n>         ov2685 7-003c\n> - uvcvideo\n>         Logitech Webcam C930e\n> - vimc\n>         VIMC Sensor B\n>\n> With this series applied the user-friendly names machine-friendly ID on\n> the same systems look like this:\n>\n> The format is:\n>     <user-friendly name> (<machine-friendly ID>)\n>\n> - ipu3\n>         ov13858 8-0010 (pci0000:00/0000:00:15.2/i2c_designware.2/i2c-8/i2c-OVTID858:00)\n>         ov5670 10-0036 (pci0000:00/0000:00:19.2/i2c_designware.5/i2c-10/i2c-INT3479:00)\n> - raspberrypi\n>         imx219 10-0010 (platform/soc/3f205000.i2c/i2c-11/i2c-10/10-0010)\n> - rkisp1\n>         ov5695 7-0036 (platform/ff160000.i2c/i2c-7/7-0036)\n>         ov2685 7-003c (platform/ff160000.i2c/i2c-7/7-003c)\n> - uvcvideo\n>         Logitech Webcam C930e (pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0)\n> - vimc\n>         Sensor B (platform/vimc.0)\n\nReally an open question, are those name really useful ?\n\nI've spent some time going through systemd predictable interfaces\nnames and udevadm test-builtin path_id and indeed they use the same\ninformation you have here collected, but they produce a name without /\nand with a well defined naming scheme (systemd at least) that could be\nguessed by knowing how the devices are integrated in the system.\n\nI don't think we need to go as far as making our camera names\npredictable, at least I don't see a urgent use case for that, but\ntaking the raw sysfs path seems a bit too simplicistic, especially\nwhen you then have to deal with ids like:\n        pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0\n\nwhich seems a bit long to me.\n\nI know this is not expected to be used by users directly, but as per\nmy previous comment I think we should not expose an API in libcamera\nthat works on names which are not guranteed to be unique (althoug\napplication should be able to provide shortcut, maybe using friendly\nnames as reported by pipeline handlers.\n\nTrying to list which kind of sensor we should deal with, their number\nis fairly limited for my understanding: usb and i2c. Wouldn't coupling\nthe bus identifier with the sensor identifier enough to provide a\nunique and almost-as-friendly-as-a-name id ?\n\nUsing the use example you have above provided:\n- ipu3\n        i2c-8_i2c-OVTID858:00\n        i2c-10_i2c-INT3479:00\n- raspberrypi\n        i2c-10_10-0010\n- rkisp1\n        i2c-7_7-0036\n        i2c-7_7-003c\n- uvcvideo\n        usb3_3-2_3-2.4_3-2.4:1.0\n- vimc\n        vimc.0\n\nI2c is trivial, use the device identifier and walk the path back until\nthe i2c parent identifier is found, then couple the 2.\n\nusb seems a bit more tricky as a full name like\n        usb3_3-2_3-2.4_3-2.4:1.0\ncould probably be reduced to\n        usb3_3-2.4:1.0\n\n(however, udevadm for my UVC camera gives me:\nudevadm test-builtin path_id /sys/devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8:1.0\nID_PATH_TAG=pci-0000_00_14_0-usb-0_8_1_0\nwhile I would have expected usb1_1-8:1.0\n\nI know nothing about USB, but it might be worth reading why udev\nthinks that path tag should in that form\nhttps://github.com/systemd/systemd/blob/master/src/udev/udev-builtin-path_id.c\n\nVimc is kind of special, you cannot have 2 vimc instances in your\nsystem loaded at the same time, so I guess the device name should be\nenough.\n\nWhat options have you considered in place of the full sysfs path ? Or\ndo you think there's no need to and a string like\n        pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0\ncould be easily managed by applications ?\n\nThanks\n  j\n\n>\n> Where it previously where possible to select a camera by its\n> user-friendly name its now possible to also select it using its\n> machine-friendly one. The following is therefor two equivalent\n> commands:\n>\n>     $ cam -c \"Logitech Webcam C930e\" -C\n>     $ cam -c \"pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0\" -C\n>\n> Niklas Söderlund (9):\n>   libcamera: v4l2_device: Add method to lookup device path\n>   libcamera: camera_sensor: Expose a sensor ID\n>   libcamera: camera: Add camera ID\n>   libcamera: camera_manager: Enforce unique camera IDs\n>   libcamera: camera_manager: Try to match camera IDs first\n>   libcamera: pipeline: vimc: Align camera name\n>   libcamera: pipeline: raspberrypi: Align camera name\n>   libcamera: camera: Add create() that operates on CameraSensor\n>   cam: Print camera IDs when listing cameras\n>\n>  include/libcamera/camera.h                    | 11 +++-\n>  include/libcamera/internal/camera_sensor.h    |  2 +\n>  include/libcamera/internal/v4l2_device.h      |  1 +\n>  src/cam/main.cpp                              |  3 +-\n>  src/libcamera/camera.cpp                      | 54 +++++++++++++++----\n>  src/libcamera/camera_manager.cpp              | 13 +++++\n>  src/libcamera/camera_sensor.cpp               | 17 ++++++\n>  src/libcamera/pipeline/ipu3/ipu3.cpp          |  9 ++--\n>  .../pipeline/raspberrypi/raspberrypi.cpp      |  3 +-\n>  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  2 +-\n>  src/libcamera/pipeline/simple/simple.cpp      |  3 +-\n>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  7 ++-\n>  src/libcamera/pipeline/vimc/vimc.cpp          |  4 +-\n>  src/libcamera/v4l2_device.cpp                 | 27 ++++++++++\n>  test/camera/buffer_import.cpp                 |  2 +-\n>  test/camera/capture.cpp                       |  2 +-\n>  test/camera/configuration_default.cpp         |  2 +-\n>  test/camera/configuration_set.cpp             |  2 +-\n>  test/camera/statemachine.cpp                  |  2 +-\n>  test/controls/control_info_map.cpp            |  2 +-\n>  test/controls/control_list.cpp                |  2 +-\n>  21 files changed, 138 insertions(+), 32 deletions(-)\n>\n> --\n> 2.27.0\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 3603BBD792\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 20 Jul 2020 14:44:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9F642605BF;\n\tMon, 20 Jul 2020 16:44:53 +0200 (CEST)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A2B5C60491\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 20 Jul 2020 16:44:52 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id 17003100004;\n\tMon, 20 Jul 2020 14:44:51 +0000 (UTC)"],"Date":"Mon, 20 Jul 2020 16:48:26 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<20200720144826.bxtbag22ki67wkoj@uno.localdomain>","References":"<20200718132324.867815-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200718132324.867815-1-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH 0/9] libcamera: camera: Add camera ID","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11545,"web_url":"https://patchwork.libcamera.org/comment/11545/","msgid":"<20200724100835.GD2420270@oden.dyn.berto.se>","date":"2020-07-24T10:08:35","subject":"Re: [libcamera-devel] [PATCH 0/9] libcamera: camera: Add camera ID","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your feedback.\n\nOn 2020-07-20 15:07:07 +0200, Jacopo Mondi wrote:\n> Hi Niklas,\n> \n> On Sat, Jul 18, 2020 at 03:23:15PM +0200, Niklas Söderlund wrote:\n> > Hello,\n> >\n> \n> minors apart that I will reply to patch-by-patch, I have two questions\n> on the series in general\n> \n> 1) The series introduce and 'id' to be used alongside and\n> alternatively to the camera 'name'. This might just be a matter of\n> terminology, but I find this a bit confusing. Ideally, the 'name'\n> should be the unique part, to which a 'model' (to mimic the API we\n> have for camera sensors) could be added.\n\nI'm not against this change. But maybe removing the name() and adding a \nmodel() could be done on top as this series main goal is to add a id() \nfield?\n\n> \n> 2) The API and the cam implementation allow to use 'name' and 'id'\n> interchangibily. Is this a good thing ? Applications should always use\n> 'id' when interfacing to libcamera, and ideally 'name' should be a\n> shortcut for users, to easily select a camera (provided it is unique).\n> If I'm not mistaken it is currently possible to ask libcamera for a\n> 'camera' name, should we drop this and implement that part in the\n> application ? 'cam' and alike can and should use mnemonic names to\n> users, but should libcamera do the same? Do we want an API that allows\n> selecting camera with a name which is not guaranteed to be unique and\n> consistent ? I would say we don't...\n\nAlso I'm not against this change and I think if we all agree this series \ncan be modified to only allow selecting cameras based on id() instead of \nid() or name() as this version of this series allows.\n\n> \n> What do you think ?\n> \n> > This series aims to add a ID to each camera in addition to it's more\n> > user-friendly name. The ID is unique and persistent between reboots of\n> > the same system. The use-case for this is to create a single\n> > machine-friendly ID that can be stored and used to always resolve to the\n> > same camera.\n> >\n> > The idea on how to generate a ID is to take the sysfs path of the sensor\n> > device which is part of each camera pipeline. As the path describes the\n> > location of the sensor hardware it is persistent across reboots and as\n> > the path is read from sysfs it's guaranteed to be unique in the system.\n> >\n> > For pipelines that do not have a sensor (UVC) the sysfs path of the main\n> > video device is used instead. That path resolves to the USB device and\n> > includes the USB bus information so it satisfy the ID requirements.\n> >\n> > While working with this problem it became apparent that two pipelines\n> > diverge from the others on how they name their cameras, raspberrypi and\n> > vimc. This series aligns these two and adds a helper to avoid such\n> > situations in the future. Unfortunately this means the user-friendly\n> > name of the sensor changes but this proves the need for a\n> > machine-friendly ID which luckily this series also adds :-)\n> >\n> > Before this series camera user-friendly names on different systems\n> > looked like this (I do not have access to a simple pipeline device):\n> >\n> > - ipu3\n> >         ov13858 8-0010\n> >         ov5670 10-0036\n> > - raspberrypi\n> >         imx219\n> > - rkisp1\n> >         ov5695 7-0036\n> >         ov2685 7-003c\n> > - uvcvideo\n> >         Logitech Webcam C930e\n> > - vimc\n> >         VIMC Sensor B\n> >\n> > With this series applied the user-friendly names machine-friendly ID on\n> > the same systems look like this:\n> >\n> > The format is:\n> >     <user-friendly name> (<machine-friendly ID>)\n> >\n> > - ipu3\n> >         ov13858 8-0010 (pci0000:00/0000:00:15.2/i2c_designware.2/i2c-8/i2c-OVTID858:00)\n> >         ov5670 10-0036 (pci0000:00/0000:00:19.2/i2c_designware.5/i2c-10/i2c-INT3479:00)\n> > - raspberrypi\n> >         imx219 10-0010 (platform/soc/3f205000.i2c/i2c-11/i2c-10/10-0010)\n> > - rkisp1\n> >         ov5695 7-0036 (platform/ff160000.i2c/i2c-7/7-0036)\n> >         ov2685 7-003c (platform/ff160000.i2c/i2c-7/7-003c)\n> > - uvcvideo\n> >         Logitech Webcam C930e (pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0)\n> > - vimc\n> >         Sensor B (platform/vimc.0)\n> >\n> > Where it previously where possible to select a camera by its\n> > user-friendly name its now possible to also select it using its\n> > machine-friendly one. The following is therefor two equivalent\n> > commands:\n> >\n> >     $ cam -c \"Logitech Webcam C930e\" -C\n> >     $ cam -c \"pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0\" -C\n> >\n> > Niklas Söderlund (9):\n> >   libcamera: v4l2_device: Add method to lookup device path\n> >   libcamera: camera_sensor: Expose a sensor ID\n> >   libcamera: camera: Add camera ID\n> >   libcamera: camera_manager: Enforce unique camera IDs\n> >   libcamera: camera_manager: Try to match camera IDs first\n> >   libcamera: pipeline: vimc: Align camera name\n> >   libcamera: pipeline: raspberrypi: Align camera name\n> >   libcamera: camera: Add create() that operates on CameraSensor\n> >   cam: Print camera IDs when listing cameras\n> >\n> >  include/libcamera/camera.h                    | 11 +++-\n> >  include/libcamera/internal/camera_sensor.h    |  2 +\n> >  include/libcamera/internal/v4l2_device.h      |  1 +\n> >  src/cam/main.cpp                              |  3 +-\n> >  src/libcamera/camera.cpp                      | 54 +++++++++++++++----\n> >  src/libcamera/camera_manager.cpp              | 13 +++++\n> >  src/libcamera/camera_sensor.cpp               | 17 ++++++\n> >  src/libcamera/pipeline/ipu3/ipu3.cpp          |  9 ++--\n> >  .../pipeline/raspberrypi/raspberrypi.cpp      |  3 +-\n> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  2 +-\n> >  src/libcamera/pipeline/simple/simple.cpp      |  3 +-\n> >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  7 ++-\n> >  src/libcamera/pipeline/vimc/vimc.cpp          |  4 +-\n> >  src/libcamera/v4l2_device.cpp                 | 27 ++++++++++\n> >  test/camera/buffer_import.cpp                 |  2 +-\n> >  test/camera/capture.cpp                       |  2 +-\n> >  test/camera/configuration_default.cpp         |  2 +-\n> >  test/camera/configuration_set.cpp             |  2 +-\n> >  test/camera/statemachine.cpp                  |  2 +-\n> >  test/controls/control_info_map.cpp            |  2 +-\n> >  test/controls/control_list.cpp                |  2 +-\n> >  21 files changed, 138 insertions(+), 32 deletions(-)\n> >\n> > --\n> > 2.27.0\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel","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 7870CBD878\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 24 Jul 2020 10:08:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F03CC611AA;\n\tFri, 24 Jul 2020 12:08:38 +0200 (CEST)","from mail-lj1-x235.google.com (mail-lj1-x235.google.com\n\t[IPv6:2a00:1450:4864:20::235])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 125B36039B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 24 Jul 2020 12:08:38 +0200 (CEST)","by mail-lj1-x235.google.com with SMTP id s9so9370945ljm.11\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 24 Jul 2020 03:08:38 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\te24sm157020ljg.18.2020.07.24.03.08.35\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 24 Jul 2020 03:08:36 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"QkdhgWPb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=jWkLUWlvNttWjCl1OGgUyXGajge3R784ePw66xfoBuE=;\n\tb=QkdhgWPbwxPxkEVLuw+lA15Fgwfhb4yi1eDhdmXaM7F5z2odf3nuav0smDEymXcv3u\n\tFKx+52hp1RUqbI/66U2+Iu/+i0M+mrmLD1GRrumTDs803Ap91R/vC5fLSqk/MhhqmpF2\n\tCj1pZ8c9vFeKqhNDXIFxTdgy+SmmSv6LdawR4PwooxSQ3GWfqOkV0n3jLpKd7uLOBkH6\n\tHheZN+lf6Jgl6Y0piClT+iNRjwgRBM1koZECXL1j3jpeMePrmFYoh0aIX0hOJA8TVQuH\n\teXF+IhyfKNWlgN+32SYZySTW77nbuDP2msFYmEibxxP4pSpVdEpiisuNkkXhP2X5Q0ga\n\tPTDA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=jWkLUWlvNttWjCl1OGgUyXGajge3R784ePw66xfoBuE=;\n\tb=hTXksJrKGZmON5n078DR/7mHV/eLFu/Piqm/9SfdzgVDKaxO8JMOBfwm4kGto9PkoF\n\tIzbVv59spojmpRyRHrf2Zo1eTEZ95P8j/RJcHkdmIvwhATOXYVmkskcJlAIJmxRIXCFz\n\tYewyBH/QI8C2f4GD4+MGbZ295C7ohCCuy/CG/9mEDol4b36BgLotFWCqrJYXWOo3ilUO\n\t7VZ6jRw4SjNH5vDG24c+FdREl90JosttLi2RsTzmB1+lgDQ2cjsQXXe6wqdg2Cal8MYM\n\tgL7cpwusnTBhtonUZGpU7wBAOmZlas1rd4A/tXdYf71CZ6YIC2P2iw7Y+PjSKoRlcIss\n\txsWA==","X-Gm-Message-State":"AOAM531BXl9PJH/pOfr7ujS5lv5a+voT/Edg2hZh40FesaySsRoVmgIs\n\tCImn63iYHd5tEqZOR1kT3VpnSg+FpGY=","X-Google-Smtp-Source":"ABdhPJzla3TQtzGLxKC6+kGlS7vCOc1r/TBAh+DsOGMpjx0kbCWFaYyd6MGvBJCrTslwVa4XuHJMBg==","X-Received":"by 2002:a2e:9417:: with SMTP id\n\ti23mr3951321ljh.237.1595585317234; \n\tFri, 24 Jul 2020 03:08:37 -0700 (PDT)","Date":"Fri, 24 Jul 2020 12:08:35 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<20200724100835.GD2420270@oden.dyn.berto.se>","References":"<20200718132324.867815-1-niklas.soderlund@ragnatech.se>\n\t<20200720130707.wteidbf6ecjdwrvh@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200720130707.wteidbf6ecjdwrvh@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 0/9] libcamera: camera: Add camera ID","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11546,"web_url":"https://patchwork.libcamera.org/comment/11546/","msgid":"<20200724101245.GE2420270@oden.dyn.berto.se>","date":"2020-07-24T10:12:45","subject":"Re: [libcamera-devel] [PATCH 0/9] libcamera: camera: Add camera ID","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your feedback.\n\nOn 2020-07-20 16:48:26 +0200, Jacopo Mondi wrote:\n> Hi Niklas,\n> \n> On Sat, Jul 18, 2020 at 03:23:15PM +0200, Niklas Söderlund wrote:\n> > Hello,\n> >\n> > This series aims to add a ID to each camera in addition to it's more\n> > user-friendly name. The ID is unique and persistent between reboots of\n> > the same system. The use-case for this is to create a single\n> > machine-friendly ID that can be stored and used to always resolve to the\n> > same camera.\n> >\n> > The idea on how to generate a ID is to take the sysfs path of the sensor\n> > device which is part of each camera pipeline. As the path describes the\n> > location of the sensor hardware it is persistent across reboots and as\n> > the path is read from sysfs it's guaranteed to be unique in the system.\n> >\n> > For pipelines that do not have a sensor (UVC) the sysfs path of the main\n> > video device is used instead. That path resolves to the USB device and\n> > includes the USB bus information so it satisfy the ID requirements.\n> >\n> > While working with this problem it became apparent that two pipelines\n> > diverge from the others on how they name their cameras, raspberrypi and\n> > vimc. This series aligns these two and adds a helper to avoid such\n> > situations in the future. Unfortunately this means the user-friendly\n> > name of the sensor changes but this proves the need for a\n> > machine-friendly ID which luckily this series also adds :-)\n> >\n> > Before this series camera user-friendly names on different systems\n> > looked like this (I do not have access to a simple pipeline device):\n> >\n> > - ipu3\n> >         ov13858 8-0010\n> >         ov5670 10-0036\n> > - raspberrypi\n> >         imx219\n> > - rkisp1\n> >         ov5695 7-0036\n> >         ov2685 7-003c\n> > - uvcvideo\n> >         Logitech Webcam C930e\n> > - vimc\n> >         VIMC Sensor B\n> >\n> > With this series applied the user-friendly names machine-friendly ID on\n> > the same systems look like this:\n> >\n> > The format is:\n> >     <user-friendly name> (<machine-friendly ID>)\n> >\n> > - ipu3\n> >         ov13858 8-0010 (pci0000:00/0000:00:15.2/i2c_designware.2/i2c-8/i2c-OVTID858:00)\n> >         ov5670 10-0036 (pci0000:00/0000:00:19.2/i2c_designware.5/i2c-10/i2c-INT3479:00)\n> > - raspberrypi\n> >         imx219 10-0010 (platform/soc/3f205000.i2c/i2c-11/i2c-10/10-0010)\n> > - rkisp1\n> >         ov5695 7-0036 (platform/ff160000.i2c/i2c-7/7-0036)\n> >         ov2685 7-003c (platform/ff160000.i2c/i2c-7/7-003c)\n> > - uvcvideo\n> >         Logitech Webcam C930e (pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0)\n> > - vimc\n> >         Sensor B (platform/vimc.0)\n> \n> Really an open question, are those name really useful ?\n> \n> I've spent some time going through systemd predictable interfaces\n> names and udevadm test-builtin path_id and indeed they use the same\n> information you have here collected, but they produce a name without /\n> and with a well defined naming scheme (systemd at least) that could be\n> guessed by knowing how the devices are integrated in the system.\n> \n> I don't think we need to go as far as making our camera names\n> predictable, at least I don't see a urgent use case for that, but\n> taking the raw sysfs path seems a bit too simplicistic, especially\n> when you then have to deal with ids like:\n>         pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0\n> \n> which seems a bit long to me.\n> \n> I know this is not expected to be used by users directly, but as per\n> my previous comment I think we should not expose an API in libcamera\n> that works on names which are not guranteed to be unique (althoug\n> application should be able to provide shortcut, maybe using friendly\n> names as reported by pipeline handlers.\n> \n> Trying to list which kind of sensor we should deal with, their number\n> is fairly limited for my understanding: usb and i2c. Wouldn't coupling\n> the bus identifier with the sensor identifier enough to provide a\n> unique and almost-as-friendly-as-a-name id ?\n> \n> Using the use example you have above provided:\n> - ipu3\n>         i2c-8_i2c-OVTID858:00\n>         i2c-10_i2c-INT3479:00\n> - raspberrypi\n>         i2c-10_10-0010\n> - rkisp1\n>         i2c-7_7-0036\n>         i2c-7_7-003c\n> - uvcvideo\n>         usb3_3-2_3-2.4_3-2.4:1.0\n> - vimc\n>         vimc.0\n> \n> I2c is trivial, use the device identifier and walk the path back until\n> the i2c parent identifier is found, then couple the 2.\n> \n> usb seems a bit more tricky as a full name like\n>         usb3_3-2_3-2.4_3-2.4:1.0\n> could probably be reduced to\n>         usb3_3-2.4:1.0\n> \n> (however, udevadm for my UVC camera gives me:\n> udevadm test-builtin path_id /sys/devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8:1.0\n> ID_PATH_TAG=pci-0000_00_14_0-usb-0_8_1_0\n> while I would have expected usb1_1-8:1.0\n> \n> I know nothing about USB, but it might be worth reading why udev\n> thinks that path tag should in that form\n> https://github.com/systemd/systemd/blob/master/src/udev/udev-builtin-path_id.c\n> \n> Vimc is kind of special, you cannot have 2 vimc instances in your\n> system loaded at the same time, so I guess the device name should be\n> enough.\n> \n> What options have you considered in place of the full sysfs path ? Or\n> do you think there's no need to and a string like\n>         pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0\n> could be easily managed by applications ?\n\nThanks for pointing out the udev possibility to this problem, I was not \naware of it. I kind of like it and would like to use it instead of using \nthe sysfs device path as this series does.\n\nThe only problem I see is that it would make udev a required dependency.  \nIn the past we have gone to great length of keeping udev optional, we \neven implemented a sysfs based device enumerator to allow for this. If \nwe with what we know now are willing to reevaluate this design idea I \nwould be all for using udev as you describe above. So before I set out \nto try this out how do we all fell about making udev mandatory?\n\n> \n> Thanks\n>   j\n> \n> >\n> > Where it previously where possible to select a camera by its\n> > user-friendly name its now possible to also select it using its\n> > machine-friendly one. The following is therefor two equivalent\n> > commands:\n> >\n> >     $ cam -c \"Logitech Webcam C930e\" -C\n> >     $ cam -c \"pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0\" -C\n> >\n> > Niklas Söderlund (9):\n> >   libcamera: v4l2_device: Add method to lookup device path\n> >   libcamera: camera_sensor: Expose a sensor ID\n> >   libcamera: camera: Add camera ID\n> >   libcamera: camera_manager: Enforce unique camera IDs\n> >   libcamera: camera_manager: Try to match camera IDs first\n> >   libcamera: pipeline: vimc: Align camera name\n> >   libcamera: pipeline: raspberrypi: Align camera name\n> >   libcamera: camera: Add create() that operates on CameraSensor\n> >   cam: Print camera IDs when listing cameras\n> >\n> >  include/libcamera/camera.h                    | 11 +++-\n> >  include/libcamera/internal/camera_sensor.h    |  2 +\n> >  include/libcamera/internal/v4l2_device.h      |  1 +\n> >  src/cam/main.cpp                              |  3 +-\n> >  src/libcamera/camera.cpp                      | 54 +++++++++++++++----\n> >  src/libcamera/camera_manager.cpp              | 13 +++++\n> >  src/libcamera/camera_sensor.cpp               | 17 ++++++\n> >  src/libcamera/pipeline/ipu3/ipu3.cpp          |  9 ++--\n> >  .../pipeline/raspberrypi/raspberrypi.cpp      |  3 +-\n> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  2 +-\n> >  src/libcamera/pipeline/simple/simple.cpp      |  3 +-\n> >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  7 ++-\n> >  src/libcamera/pipeline/vimc/vimc.cpp          |  4 +-\n> >  src/libcamera/v4l2_device.cpp                 | 27 ++++++++++\n> >  test/camera/buffer_import.cpp                 |  2 +-\n> >  test/camera/capture.cpp                       |  2 +-\n> >  test/camera/configuration_default.cpp         |  2 +-\n> >  test/camera/configuration_set.cpp             |  2 +-\n> >  test/camera/statemachine.cpp                  |  2 +-\n> >  test/controls/control_info_map.cpp            |  2 +-\n> >  test/controls/control_list.cpp                |  2 +-\n> >  21 files changed, 138 insertions(+), 32 deletions(-)\n> >\n> > --\n> > 2.27.0\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel","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 2C11ABD86F\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 24 Jul 2020 10:12:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A4487611AA;\n\tFri, 24 Jul 2020 12:12:48 +0200 (CEST)","from mail-lf1-x12b.google.com (mail-lf1-x12b.google.com\n\t[IPv6:2a00:1450:4864:20::12b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 19E506039B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 24 Jul 2020 12:12:47 +0200 (CEST)","by mail-lf1-x12b.google.com with SMTP id s9so4910154lfs.4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 24 Jul 2020 03:12:47 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tu5sm154904lfm.81.2020.07.24.03.12.45\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 24 Jul 2020 03:12:45 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"avYEVpCm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=/TxN0BlTdigv+JHfrDDk0G6PTSUYAIm7Zp/1hYatje8=;\n\tb=avYEVpCmYPrDZb+Zyt2enwSMmbzlluVkwD4Li3riffJ/BOPafBDZ0WL4QVpTPBsCPg\n\tc08VDEdiRiWu/7+56VudeLX4TawWqQVeGWHdlqTZqFWNsdlUQFmRUDt3LPTYQ6Ou1HjA\n\tHD7VTs0qqnZ4ooa+aPgE5MQQOLPBtiq1LLjPkyUIEO88ENe9aBsFocnij4zrXLfWcOgv\n\tbXua83FIVvDMSUxXw4VlwWaY4M5aE/WJ29TUAmFyW9FzVyusg2k7qc1evVppIIju/nuV\n\tnlE02OmHVk4XqOwt8UzTrZWNaRsuw9HwGUVm/sfDucWRZD8Y3leFx+DFkNzitEkD/LJP\n\tE01g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=/TxN0BlTdigv+JHfrDDk0G6PTSUYAIm7Zp/1hYatje8=;\n\tb=HEKFiPfACV6hXGJPtEEA6mhOX/Z9UW+Bl7r6jRNLzoJO0Z3qBlecdmAH5MMfiz3cH/\n\tg78HWbqQTtAZoECpP41REmD9Q5nzIgf1uQMYR7064LCn+BY7bgMX0JRoWUr17Neq2SMx\n\t/tGZARBB1IdqaYTnWRN2DXeqnVVMHrC4QYYxZsea1lcJDX6pC+E9QthrwKWX/Tyc/HOo\n\tLyZFabAOZZuN398SLTE4HHlrEpaRF4Rj6reXvQ4pLdsAa3HK98OHOx8TY6Utjvii6gJB\n\tsnKfDuc3Jjm0JBYnq2GiGAqvwV3eymjXSRLAFDUYuBSlNNKrw2AyZeLdGjhicbZ8uy2b\n\ty62w==","X-Gm-Message-State":"AOAM530N5r9G3YsyxsRnNefcyzK47PJBRBIs4a+poSrnLqpI0rTZnCSM\n\tWUC79rHU0xhJXK5dwsHl2PILvg==","X-Google-Smtp-Source":"ABdhPJyi4RAZc9z2MKFwV2BgNiRWRZh86sBg2NnWyk/VICZ7WE5lPHEdo6MK7WSLMj75AkaHSMv0zQ==","X-Received":"by 2002:a19:70c:: with SMTP id 12mr4687378lfh.207.1595585566291; \n\tFri, 24 Jul 2020 03:12:46 -0700 (PDT)","Date":"Fri, 24 Jul 2020 12:12:45 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<20200724101245.GE2420270@oden.dyn.berto.se>","References":"<20200718132324.867815-1-niklas.soderlund@ragnatech.se>\n\t<20200720144826.bxtbag22ki67wkoj@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200720144826.bxtbag22ki67wkoj@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 0/9] libcamera: camera: Add camera ID","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11591,"web_url":"https://patchwork.libcamera.org/comment/11591/","msgid":"<20200724234407.GL5921@pendragon.ideasonboard.com>","date":"2020-07-24T23:44:07","subject":"Re: [libcamera-devel] [PATCH 0/9] libcamera: camera: Add camera ID","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Fri, Jul 24, 2020 at 12:08:35PM +0200, Niklas Söderlund wrote:\n> On 2020-07-20 15:07:07 +0200, Jacopo Mondi wrote:\n> > On Sat, Jul 18, 2020 at 03:23:15PM +0200, Niklas Söderlund wrote:\n> > > Hello,\n> > \n> > minors apart that I will reply to patch-by-patch, I have two questions\n> > on the series in general\n> > \n> > 1) The series introduce and 'id' to be used alongside and\n> > alternatively to the camera 'name'. This might just be a matter of\n> > terminology, but I find this a bit confusing. Ideally, the 'name'\n> > should be the unique part, to which a 'model' (to mimic the API we\n> > have for camera sensors) could be added.\n> \n> I'm not against this change. But maybe removing the name() and adding a \n> model() could be done on top as this series main goal is to add a id() \n> field?\n\nWe're entering the bikeshedding territory a little bit, but I think it's\nan important question. We may want to drop \"name\" altogether, as it's a\nconfusing name (no pun intended). Using friendlyName() and uniqueName(),\nfor instance, would make the API clearer. We can continue bikeshedding\non the name, but I think whatever function names we pick, they should be\nfairly explicit.\n\n> > 2) The API and the cam implementation allow to use 'name' and 'id'\n> > interchangibily. Is this a good thing ? Applications should always use\n> > 'id' when interfacing to libcamera, and ideally 'name' should be a\n> > shortcut for users, to easily select a camera (provided it is unique).\n> > If I'm not mistaken it is currently possible to ask libcamera for a\n> > 'camera' name, should we drop this and implement that part in the\n> > application ? 'cam' and alike can and should use mnemonic names to\n> > users, but should libcamera do the same? Do we want an API that allows\n> > selecting camera with a name which is not guaranteed to be unique and\n> > consistent ? I would say we don't...\n> \n> Also I'm not against this change and I think if we all agree this series \n> can be modified to only allow selecting cameras based on id() instead of \n> id() or name() as this version of this series allows.\n\nI'm with Jacopo on this, I'd prefer if libcamera pushed applications to\nuse a safer API, while still offering the option of an application-side\nmanual implementation based on the friendly name.\n\n> > What do you think ?\n> > \n> > > This series aims to add a ID to each camera in addition to it's more\n> > > user-friendly name. The ID is unique and persistent between reboots of\n> > > the same system. The use-case for this is to create a single\n> > > machine-friendly ID that can be stored and used to always resolve to the\n> > > same camera.\n> > >\n> > > The idea on how to generate a ID is to take the sysfs path of the sensor\n> > > device which is part of each camera pipeline. As the path describes the\n> > > location of the sensor hardware it is persistent across reboots and as\n> > > the path is read from sysfs it's guaranteed to be unique in the system.\n> > >\n> > > For pipelines that do not have a sensor (UVC) the sysfs path of the main\n> > > video device is used instead. That path resolves to the USB device and\n> > > includes the USB bus information so it satisfy the ID requirements.\n> > >\n> > > While working with this problem it became apparent that two pipelines\n> > > diverge from the others on how they name their cameras, raspberrypi and\n> > > vimc. This series aligns these two and adds a helper to avoid such\n> > > situations in the future. Unfortunately this means the user-friendly\n> > > name of the sensor changes but this proves the need for a\n> > > machine-friendly ID which luckily this series also adds :-)\n> > >\n> > > Before this series camera user-friendly names on different systems\n> > > looked like this (I do not have access to a simple pipeline device):\n> > >\n> > > - ipu3\n> > >         ov13858 8-0010\n> > >         ov5670 10-0036\n> > > - raspberrypi\n> > >         imx219\n> > > - rkisp1\n> > >         ov5695 7-0036\n> > >         ov2685 7-003c\n> > > - uvcvideo\n> > >         Logitech Webcam C930e\n> > > - vimc\n> > >         VIMC Sensor B\n> > >\n> > > With this series applied the user-friendly names machine-friendly ID on\n> > > the same systems look like this:\n> > >\n> > > The format is:\n> > >     <user-friendly name> (<machine-friendly ID>)\n> > >\n> > > - ipu3\n> > >         ov13858 8-0010 (pci0000:00/0000:00:15.2/i2c_designware.2/i2c-8/i2c-OVTID858:00)\n> > >         ov5670 10-0036 (pci0000:00/0000:00:19.2/i2c_designware.5/i2c-10/i2c-INT3479:00)\n> > > - raspberrypi\n> > >         imx219 10-0010 (platform/soc/3f205000.i2c/i2c-11/i2c-10/10-0010)\n> > > - rkisp1\n> > >         ov5695 7-0036 (platform/ff160000.i2c/i2c-7/7-0036)\n> > >         ov2685 7-003c (platform/ff160000.i2c/i2c-7/7-003c)\n> > > - uvcvideo\n> > >         Logitech Webcam C930e (pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0)\n> > > - vimc\n> > >         Sensor B (platform/vimc.0)\n> > >\n> > > Where it previously where possible to select a camera by its\n> > > user-friendly name its now possible to also select it using its\n> > > machine-friendly one. The following is therefor two equivalent\n> > > commands:\n> > >\n> > >     $ cam -c \"Logitech Webcam C930e\" -C\n> > >     $ cam -c \"pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0\" -C\n> > >\n> > > Niklas Söderlund (9):\n> > >   libcamera: v4l2_device: Add method to lookup device path\n> > >   libcamera: camera_sensor: Expose a sensor ID\n> > >   libcamera: camera: Add camera ID\n> > >   libcamera: camera_manager: Enforce unique camera IDs\n> > >   libcamera: camera_manager: Try to match camera IDs first\n> > >   libcamera: pipeline: vimc: Align camera name\n> > >   libcamera: pipeline: raspberrypi: Align camera name\n> > >   libcamera: camera: Add create() that operates on CameraSensor\n> > >   cam: Print camera IDs when listing cameras\n> > >\n> > >  include/libcamera/camera.h                    | 11 +++-\n> > >  include/libcamera/internal/camera_sensor.h    |  2 +\n> > >  include/libcamera/internal/v4l2_device.h      |  1 +\n> > >  src/cam/main.cpp                              |  3 +-\n> > >  src/libcamera/camera.cpp                      | 54 +++++++++++++++----\n> > >  src/libcamera/camera_manager.cpp              | 13 +++++\n> > >  src/libcamera/camera_sensor.cpp               | 17 ++++++\n> > >  src/libcamera/pipeline/ipu3/ipu3.cpp          |  9 ++--\n> > >  .../pipeline/raspberrypi/raspberrypi.cpp      |  3 +-\n> > >  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  2 +-\n> > >  src/libcamera/pipeline/simple/simple.cpp      |  3 +-\n> > >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  7 ++-\n> > >  src/libcamera/pipeline/vimc/vimc.cpp          |  4 +-\n> > >  src/libcamera/v4l2_device.cpp                 | 27 ++++++++++\n> > >  test/camera/buffer_import.cpp                 |  2 +-\n> > >  test/camera/capture.cpp                       |  2 +-\n> > >  test/camera/configuration_default.cpp         |  2 +-\n> > >  test/camera/configuration_set.cpp             |  2 +-\n> > >  test/camera/statemachine.cpp                  |  2 +-\n> > >  test/controls/control_info_map.cpp            |  2 +-\n> > >  test/controls/control_list.cpp                |  2 +-\n> > >  21 files changed, 138 insertions(+), 32 deletions(-)","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 0CA8CBD86F\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 24 Jul 2020 23:44:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8511461223;\n\tSat, 25 Jul 2020 01:44:16 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E9BDA60496\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 25 Jul 2020 01:44:14 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4777E538;\n\tSat, 25 Jul 2020 01:44:14 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ZmJT2JDP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1595634254;\n\tbh=huCwQTeRofg3TP5zQwHXI38eRSi5pbTSfo25rnl7l8E=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZmJT2JDPUP5qZipRhIwRtQL/LgiIGF5Ir7uo4lPe0jkAJ+VUCAYjOpm1vg/IJ6jP0\n\t67Kdlx3lx7ODFMcW34fR13TPv+wElEbvpuRRu2kUlHC4jAGQrWSuyo83EADn4VRpHR\n\t4gxMVl25Ck5etckKj85SHmTxqWw/u2z1C7XhtbfE=","Date":"Sat, 25 Jul 2020 02:44:07 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<20200724234407.GL5921@pendragon.ideasonboard.com>","References":"<20200718132324.867815-1-niklas.soderlund@ragnatech.se>\n\t<20200720130707.wteidbf6ecjdwrvh@uno.localdomain>\n\t<20200724100835.GD2420270@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200724100835.GD2420270@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH 0/9] libcamera: camera: Add camera ID","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11592,"web_url":"https://patchwork.libcamera.org/comment/11592/","msgid":"<20200725000356.GM5921@pendragon.ideasonboard.com>","date":"2020-07-25T00:03:56","subject":"Re: [libcamera-devel] [PATCH 0/9] libcamera: camera: Add camera ID","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Fri, Jul 24, 2020 at 12:12:45PM +0200, Niklas Söderlund wrote:\n> On 2020-07-20 16:48:26 +0200, Jacopo Mondi wrote:\n> > On Sat, Jul 18, 2020 at 03:23:15PM +0200, Niklas Söderlund wrote:\n> > > Hello,\n> > >\n> > > This series aims to add a ID to each camera in addition to it's more\n> > > user-friendly name. The ID is unique and persistent between reboots of\n> > > the same system. The use-case for this is to create a single\n> > > machine-friendly ID that can be stored and used to always resolve to the\n> > > same camera.\n> > >\n> > > The idea on how to generate a ID is to take the sysfs path of the sensor\n> > > device which is part of each camera pipeline. As the path describes the\n> > > location of the sensor hardware it is persistent across reboots and as\n> > > the path is read from sysfs it's guaranteed to be unique in the system.\n> > >\n> > > For pipelines that do not have a sensor (UVC) the sysfs path of the main\n> > > video device is used instead. That path resolves to the USB device and\n> > > includes the USB bus information so it satisfy the ID requirements.\n> > >\n> > > While working with this problem it became apparent that two pipelines\n> > > diverge from the others on how they name their cameras, raspberrypi and\n> > > vimc. This series aligns these two and adds a helper to avoid such\n> > > situations in the future. Unfortunately this means the user-friendly\n> > > name of the sensor changes but this proves the need for a\n> > > machine-friendly ID which luckily this series also adds :-)\n> > >\n> > > Before this series camera user-friendly names on different systems\n> > > looked like this (I do not have access to a simple pipeline device):\n> > >\n> > > - ipu3\n> > >         ov13858 8-0010\n> > >         ov5670 10-0036\n> > > - raspberrypi\n> > >         imx219\n> > > - rkisp1\n> > >         ov5695 7-0036\n> > >         ov2685 7-003c\n> > > - uvcvideo\n> > >         Logitech Webcam C930e\n> > > - vimc\n> > >         VIMC Sensor B\n> > >\n> > > With this series applied the user-friendly names machine-friendly ID on\n> > > the same systems look like this:\n> > >\n> > > The format is:\n> > >     <user-friendly name> (<machine-friendly ID>)\n> > >\n> > > - ipu3\n> > >         ov13858 8-0010 (pci0000:00/0000:00:15.2/i2c_designware.2/i2c-8/i2c-OVTID858:00)\n> > >         ov5670 10-0036 (pci0000:00/0000:00:19.2/i2c_designware.5/i2c-10/i2c-INT3479:00)\n\nThe friendly name should really be \"Front camera\" and \"Back camera\" on a\nSoraka device. Same on a Scarlet device, but on a different rkisp1-based\nsystem, there may not be a front and a back camera. Imagine a digital\nmicroscope, it would be nice to have a friendly name along the lines of\n\"Microscope camera\". How to make this happen will likely be challenging,\nand I'm fine solving the friendly name issue separately from the unique\nname issue (the former being more complex than the latter), but I'd like\nto start thinking about it.\n\n> > > - raspberrypi\n> > >         imx219 10-0010 (platform/soc/3f205000.i2c/i2c-11/i2c-10/10-0010)\n> > > - rkisp1\n> > >         ov5695 7-0036 (platform/ff160000.i2c/i2c-7/7-0036)\n> > >         ov2685 7-003c (platform/ff160000.i2c/i2c-7/7-003c)\n> > > - uvcvideo\n> > >         Logitech Webcam C930e (pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0)\n\nAnd if we have two identical USB webcams, it would be nice to name them\n\"Logitech Webcam C930e (on port 1)\" and \"Logitech Webcam C930e (on port\n3)\" for example.\n\n> > > - vimc\n> > >         Sensor B (platform/vimc.0)\n\nThis one is easy, it's for testing purpose, so we can hardcode any name\nwe find friendly.\n\n> > \n> > Really an open question, are those name really useful ?\n> > \n> > I've spent some time going through systemd predictable interfaces\n> > names and udevadm test-builtin path_id and indeed they use the same\n> > information you have here collected, but they produce a name without /\n> > and with a well defined naming scheme (systemd at least) that could be\n> > guessed by knowing how the devices are integrated in the system.\n> > \n> > I don't think we need to go as far as making our camera names\n> > predictable, at least I don't see a urgent use case for that, but\n> > taking the raw sysfs path seems a bit too simplicistic, especially\n> > when you then have to deal with ids like:\n> >         pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0\n> > \n> > which seems a bit long to me.\n\nHow can is be too long and too simplistic at the same time ? :-)\n\nThis name should never be presented to the end-user (except by test\napplications, when the end-user is a developer), so I don't mind much if\nit's long. That being said, if there's a way to shorten it while still\nkeeping the name stable, I think that's better.\n\n> > I know this is not expected to be used by users directly, but as per\n> > my previous comment I think we should not expose an API in libcamera\n> > that works on names which are not guranteed to be unique (althoug\n> > application should be able to provide shortcut, maybe using friendly\n> > names as reported by pipeline handlers.\n> > \n> > Trying to list which kind of sensor we should deal with, their number\n> > is fairly limited for my understanding: usb and i2c. Wouldn't coupling\n> > the bus identifier with the sensor identifier enough to provide a\n> > unique and almost-as-friendly-as-a-name id ?\n> > \n> > Using the use example you have above provided:\n> > - ipu3\n> >         i2c-8_i2c-OVTID858:00\n> >         i2c-10_i2c-INT3479:00\n> > - raspberrypi\n> >         i2c-10_10-0010\n> > - rkisp1\n> >         i2c-7_7-0036\n> >         i2c-7_7-003c\n> > - uvcvideo\n> >         usb3_3-2_3-2.4_3-2.4:1.0\n> > - vimc\n> >         vimc.0\n\nShould we take into account the possibility of later implementing\nsupport for sensor A, we will have two cameras for the same vimc device.\nI think this should already be taken into account.\n\nLet's try to also consider other cases, even if we don't implement\nsupport for them now. One use case I can think of is cameras made of\nmultiple sensors (for instance a sensor with a wide angle lens and a\nsensor with a tele lens, with seamless switching between the two when\nzooming). This should however not be too complicated, we can probably\njust pick one of the two sensors to construct the camera name.\n\nBrainstorming mode turned on, what other use cases can we have ?\n\n> > I2c is trivial, use the device identifier and walk the path back until\n> > the i2c parent identifier is found, then couple the 2.\n\nIs the I2C bus number stable ? On DT system, for SoC I2C controllers, it\nshould be *if* you have I2C bus aliases in your device tree, but for\nanything behind a PCI device for instance, I don't think we have any\nsuch guarantee.\n\n> > usb seems a bit more tricky as a full name like\n> >         usb3_3-2_3-2.4_3-2.4:1.0\n> > could probably be reduced to\n> >         usb3_3-2.4:1.0\n> > \n> > (however, udevadm for my UVC camera gives me:\n> > udevadm test-builtin path_id /sys/devices/pci0000:00/0000:00:14.0/usb1/1-8/1-8:1.0\n> > ID_PATH_TAG=pci-0000_00_14_0-usb-0_8_1_0\n> > while I would have expected usb1_1-8:1.0\n> > \n> > I know nothing about USB, but it might be worth reading why udev\n> > thinks that path tag should in that form\n> > https://github.com/systemd/systemd/blob/master/src/udev/udev-builtin-path_id.c\n\nIt may also be worth looking at how device names are constructed in the\nkernel (usb_make_path() for instance). The USB devices are usually\nidentified by walking the USB tree from the root hub, keeping in mind\nthere can be multiple root hubs, and using port numbers along the way as\nthose are stable. The reason why the PCI device is included may be\nbecause USB bus numbers are assigned dynamically, and thus may not be\nstable across reboots.\n\n> > Vimc is kind of special, you cannot have 2 vimc instances in your\n> > system loaded at the same time, so I guess the device name should be\n> > enough.\n> > \n> > What options have you considered in place of the full sysfs path ? Or\n> > do you think there's no need to and a string like\n> >         pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0\n> > could be easily managed by applications ?\n> \n> Thanks for pointing out the udev possibility to this problem, I was not \n> aware of it. I kind of like it and would like to use it instead of using \n> the sysfs device path as this series does.\n> \n> The only problem I see is that it would make udev a required dependency.  \n> In the past we have gone to great length of keeping udev optional, we \n> even implemented a sysfs based device enumerator to allow for this. If \n> we with what we know now are willing to reevaluate this design idea I \n> would be all for using udev as you describe above. So before I set out \n> to try this out how do we all fell about making udev mandatory?\n\nI'd rather keep it optional if possible. As stated in a separate reply,\nif we make the device enumerator responsible for providing the name, we\ncan have different implementations for udev and udev-less systems, so I\ndon't see any issue in relying on udev when available.\n\n> > > Where it previously where possible to select a camera by its\n> > > user-friendly name its now possible to also select it using its\n> > > machine-friendly one. The following is therefor two equivalent\n> > > commands:\n> > >\n> > >     $ cam -c \"Logitech Webcam C930e\" -C\n> > >     $ cam -c \"pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0\" -C\n> > >\n> > > Niklas Söderlund (9):\n> > >   libcamera: v4l2_device: Add method to lookup device path\n> > >   libcamera: camera_sensor: Expose a sensor ID\n> > >   libcamera: camera: Add camera ID\n> > >   libcamera: camera_manager: Enforce unique camera IDs\n> > >   libcamera: camera_manager: Try to match camera IDs first\n> > >   libcamera: pipeline: vimc: Align camera name\n> > >   libcamera: pipeline: raspberrypi: Align camera name\n> > >   libcamera: camera: Add create() that operates on CameraSensor\n> > >   cam: Print camera IDs when listing cameras\n> > >\n> > >  include/libcamera/camera.h                    | 11 +++-\n> > >  include/libcamera/internal/camera_sensor.h    |  2 +\n> > >  include/libcamera/internal/v4l2_device.h      |  1 +\n> > >  src/cam/main.cpp                              |  3 +-\n> > >  src/libcamera/camera.cpp                      | 54 +++++++++++++++----\n> > >  src/libcamera/camera_manager.cpp              | 13 +++++\n> > >  src/libcamera/camera_sensor.cpp               | 17 ++++++\n> > >  src/libcamera/pipeline/ipu3/ipu3.cpp          |  9 ++--\n> > >  .../pipeline/raspberrypi/raspberrypi.cpp      |  3 +-\n> > >  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  2 +-\n> > >  src/libcamera/pipeline/simple/simple.cpp      |  3 +-\n> > >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  7 ++-\n> > >  src/libcamera/pipeline/vimc/vimc.cpp          |  4 +-\n> > >  src/libcamera/v4l2_device.cpp                 | 27 ++++++++++\n> > >  test/camera/buffer_import.cpp                 |  2 +-\n> > >  test/camera/capture.cpp                       |  2 +-\n> > >  test/camera/configuration_default.cpp         |  2 +-\n> > >  test/camera/configuration_set.cpp             |  2 +-\n> > >  test/camera/statemachine.cpp                  |  2 +-\n> > >  test/controls/control_info_map.cpp            |  2 +-\n> > >  test/controls/control_list.cpp                |  2 +-\n> > >  21 files changed, 138 insertions(+), 32 deletions(-)","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 255FBBD878\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 25 Jul 2020 00:04:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 94AD261247;\n\tSat, 25 Jul 2020 02:04:05 +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 F111360496\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 25 Jul 2020 02:04:03 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6D104538;\n\tSat, 25 Jul 2020 02:04:03 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"LzLWLd/L\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1595635443;\n\tbh=vUp6KuApWjtJ6AdXyYdRL8FtYJ9qJyFs5cyudpVFWh0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LzLWLd/LeoAPMWzb7pWXkvfCZZuvJRLCNs58MV1p+UbpBznvyhQVwZoNTRAqdxEYW\n\tagdnpPPQz0B8dNhAUUFpjra1QvdPmzFUnqPxRRU+w7aheQvc8NEigmA8Us3zFxoKxO\n\tPHl7jtnO6o6UNSX40AmQonz0qd0QZb1UMJM6FFfY=","Date":"Sat, 25 Jul 2020 03:03:56 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<20200725000356.GM5921@pendragon.ideasonboard.com>","References":"<20200718132324.867815-1-niklas.soderlund@ragnatech.se>\n\t<20200720144826.bxtbag22ki67wkoj@uno.localdomain>\n\t<20200724101245.GE2420270@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200724101245.GE2420270@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH 0/9] libcamera: camera: Add camera ID","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11597,"web_url":"https://patchwork.libcamera.org/comment/11597/","msgid":"<20200725083526.GH2729799@oden.dyn.berto.se>","date":"2020-07-25T08:35:26","subject":"Re: [libcamera-devel] [PATCH 0/9] libcamera: camera: Add camera ID","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hello,\n\nOn 2020-07-25 02:44:07 +0300, Laurent Pinchart wrote:\n> Hello,\n> \n> On Fri, Jul 24, 2020 at 12:08:35PM +0200, Niklas Söderlund wrote:\n> > On 2020-07-20 15:07:07 +0200, Jacopo Mondi wrote:\n> > > On Sat, Jul 18, 2020 at 03:23:15PM +0200, Niklas Söderlund wrote:\n> > > > Hello,\n> > > \n> > > minors apart that I will reply to patch-by-patch, I have two questions\n> > > on the series in general\n> > > \n> > > 1) The series introduce and 'id' to be used alongside and\n> > > alternatively to the camera 'name'. This might just be a matter of\n> > > terminology, but I find this a bit confusing. Ideally, the 'name'\n> > > should be the unique part, to which a 'model' (to mimic the API we\n> > > have for camera sensors) could be added.\n> > \n> > I'm not against this change. But maybe removing the name() and adding a \n> > model() could be done on top as this series main goal is to add a id() \n> > field?\n> \n> We're entering the bikeshedding territory a little bit, but I think it's\n> an important question. We may want to drop \"name\" altogether, as it's a\n> confusing name (no pun intended). Using friendlyName() and uniqueName(),\n> for instance, would make the API clearer. We can continue bikeshedding\n> on the name, but I think whatever function names we pick, they should be\n> fairly explicit.\n\nI want to avoid bikeshedding so I will only focus of the addition of the \nnew id()/uniequeName() in this series. Then once that is in we can think \nabout how we wish to evolve or remove name().\n\nSome bikeshedding is of course needed, in my first private incarnation \nof this series I had named the new property uniqueName(). It became very \ncumbersome in the documentation and felt none obvious, switching to id() \nfelt like the right thing. So while we hash this out I will keep id() as \na working name and then I will switch it to whatever the majority likes.\n\n> \n> > > 2) The API and the cam implementation allow to use 'name' and 'id'\n> > > interchangibily. Is this a good thing ? Applications should always use\n> > > 'id' when interfacing to libcamera, and ideally 'name' should be a\n> > > shortcut for users, to easily select a camera (provided it is unique).\n> > > If I'm not mistaken it is currently possible to ask libcamera for a\n> > > 'camera' name, should we drop this and implement that part in the\n> > > application ? 'cam' and alike can and should use mnemonic names to\n> > > users, but should libcamera do the same? Do we want an API that allows\n> > > selecting camera with a name which is not guaranteed to be unique and\n> > > consistent ? I would say we don't...\n> > \n> > Also I'm not against this change and I think if we all agree this series \n> > can be modified to only allow selecting cameras based on id() instead of \n> > id() or name() as this version of this series allows.\n> \n> I'm with Jacopo on this, I'd prefer if libcamera pushed applications to\n> use a safer API, while still offering the option of an application-side\n> manual implementation based on the friendly name.\n\nThen we all agree on this point and this series will modify the get() \nfunction to only operate on the new id() property.\n\n> \n> > > What do you think ?\n> > > \n> > > > This series aims to add a ID to each camera in addition to it's more\n> > > > user-friendly name. The ID is unique and persistent between reboots of\n> > > > the same system. The use-case for this is to create a single\n> > > > machine-friendly ID that can be stored and used to always resolve to the\n> > > > same camera.\n> > > >\n> > > > The idea on how to generate a ID is to take the sysfs path of the sensor\n> > > > device which is part of each camera pipeline. As the path describes the\n> > > > location of the sensor hardware it is persistent across reboots and as\n> > > > the path is read from sysfs it's guaranteed to be unique in the system.\n> > > >\n> > > > For pipelines that do not have a sensor (UVC) the sysfs path of the main\n> > > > video device is used instead. That path resolves to the USB device and\n> > > > includes the USB bus information so it satisfy the ID requirements.\n> > > >\n> > > > While working with this problem it became apparent that two pipelines\n> > > > diverge from the others on how they name their cameras, raspberrypi and\n> > > > vimc. This series aligns these two and adds a helper to avoid such\n> > > > situations in the future. Unfortunately this means the user-friendly\n> > > > name of the sensor changes but this proves the need for a\n> > > > machine-friendly ID which luckily this series also adds :-)\n> > > >\n> > > > Before this series camera user-friendly names on different systems\n> > > > looked like this (I do not have access to a simple pipeline device):\n> > > >\n> > > > - ipu3\n> > > >         ov13858 8-0010\n> > > >         ov5670 10-0036\n> > > > - raspberrypi\n> > > >         imx219\n> > > > - rkisp1\n> > > >         ov5695 7-0036\n> > > >         ov2685 7-003c\n> > > > - uvcvideo\n> > > >         Logitech Webcam C930e\n> > > > - vimc\n> > > >         VIMC Sensor B\n> > > >\n> > > > With this series applied the user-friendly names machine-friendly ID on\n> > > > the same systems look like this:\n> > > >\n> > > > The format is:\n> > > >     <user-friendly name> (<machine-friendly ID>)\n> > > >\n> > > > - ipu3\n> > > >         ov13858 8-0010 (pci0000:00/0000:00:15.2/i2c_designware.2/i2c-8/i2c-OVTID858:00)\n> > > >         ov5670 10-0036 (pci0000:00/0000:00:19.2/i2c_designware.5/i2c-10/i2c-INT3479:00)\n> > > > - raspberrypi\n> > > >         imx219 10-0010 (platform/soc/3f205000.i2c/i2c-11/i2c-10/10-0010)\n> > > > - rkisp1\n> > > >         ov5695 7-0036 (platform/ff160000.i2c/i2c-7/7-0036)\n> > > >         ov2685 7-003c (platform/ff160000.i2c/i2c-7/7-003c)\n> > > > - uvcvideo\n> > > >         Logitech Webcam C930e (pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0)\n> > > > - vimc\n> > > >         Sensor B (platform/vimc.0)\n> > > >\n> > > > Where it previously where possible to select a camera by its\n> > > > user-friendly name its now possible to also select it using its\n> > > > machine-friendly one. The following is therefor two equivalent\n> > > > commands:\n> > > >\n> > > >     $ cam -c \"Logitech Webcam C930e\" -C\n> > > >     $ cam -c \"pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0\" -C\n> > > >\n> > > > Niklas Söderlund (9):\n> > > >   libcamera: v4l2_device: Add method to lookup device path\n> > > >   libcamera: camera_sensor: Expose a sensor ID\n> > > >   libcamera: camera: Add camera ID\n> > > >   libcamera: camera_manager: Enforce unique camera IDs\n> > > >   libcamera: camera_manager: Try to match camera IDs first\n> > > >   libcamera: pipeline: vimc: Align camera name\n> > > >   libcamera: pipeline: raspberrypi: Align camera name\n> > > >   libcamera: camera: Add create() that operates on CameraSensor\n> > > >   cam: Print camera IDs when listing cameras\n> > > >\n> > > >  include/libcamera/camera.h                    | 11 +++-\n> > > >  include/libcamera/internal/camera_sensor.h    |  2 +\n> > > >  include/libcamera/internal/v4l2_device.h      |  1 +\n> > > >  src/cam/main.cpp                              |  3 +-\n> > > >  src/libcamera/camera.cpp                      | 54 +++++++++++++++----\n> > > >  src/libcamera/camera_manager.cpp              | 13 +++++\n> > > >  src/libcamera/camera_sensor.cpp               | 17 ++++++\n> > > >  src/libcamera/pipeline/ipu3/ipu3.cpp          |  9 ++--\n> > > >  .../pipeline/raspberrypi/raspberrypi.cpp      |  3 +-\n> > > >  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  2 +-\n> > > >  src/libcamera/pipeline/simple/simple.cpp      |  3 +-\n> > > >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  7 ++-\n> > > >  src/libcamera/pipeline/vimc/vimc.cpp          |  4 +-\n> > > >  src/libcamera/v4l2_device.cpp                 | 27 ++++++++++\n> > > >  test/camera/buffer_import.cpp                 |  2 +-\n> > > >  test/camera/capture.cpp                       |  2 +-\n> > > >  test/camera/configuration_default.cpp         |  2 +-\n> > > >  test/camera/configuration_set.cpp             |  2 +-\n> > > >  test/camera/statemachine.cpp                  |  2 +-\n> > > >  test/controls/control_info_map.cpp            |  2 +-\n> > > >  test/controls/control_list.cpp                |  2 +-\n> > > >  21 files changed, 138 insertions(+), 32 deletions(-)\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 51F19BD86F\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 25 Jul 2020 08:35:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D97B66124D;\n\tSat, 25 Jul 2020 10:35:30 +0200 (CEST)","from mail-lj1-x235.google.com (mail-lj1-x235.google.com\n\t[IPv6:2a00:1450:4864:20::235])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 91DE36053C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 25 Jul 2020 10:35:29 +0200 (CEST)","by mail-lj1-x235.google.com with SMTP id f5so12248289ljj.10\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 25 Jul 2020 01:35:29 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\td25sm850446ljc.103.2020.07.25.01.35.27\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 25 Jul 2020 01:35:27 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"LEiWKwOv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=aaIsRzEE0njrp7REZUkPuW4b1IH+ZCe++x2ZbBnT5mk=;\n\tb=LEiWKwOv20zdfeYsQ3ShSavD0gSF4YaBc/baodhqafYxmWKrXiu1XPllQ2ItjfL2M+\n\tnE5/OgOSrHoVSVh7/WvvnnGTNtk31HNVK8/57Z4Q7S0h4uCltMG0q+QFts67hvcIGnwX\n\td6i1EYrzRET6y0C5+3a4G+VMbXaIhzm5yqUbD07GY1+9Nk6oSqzTQ04N1WRECR5hVXkR\n\tnMXkw+aptD2NJ06/HmCMqeX2upQOG/zl+mXqv6BEkHAhrATBlGr49hUaPW1I3Kp2SogO\n\tiyhEHbksaoy6SHYAtaxgTWGJ8KtDrO6mPpMEYsgm/eJ1IiNYd1g6rDidlcWrzy7Er+Um\n\tPXlA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=aaIsRzEE0njrp7REZUkPuW4b1IH+ZCe++x2ZbBnT5mk=;\n\tb=AdEWdic2Qthks3xBn5Xu522au0ltkSF3XAYanskOXm+4lStEfrLDL2M9mx/oQHrvzV\n\tynI7Wl8bRJqx/xXNZ853HlFaudieE1c0CdGVBqTXHzaJos+jYw5/3cDKdTelziBmrHAi\n\t83B0mrU5KviYgoKkDaofXFRxfEWXfwSNKbGsiiVNujVaKxtkga5rogFVtX9SyK6rqnwq\n\twHgRiiTKPR0D5qgQFss2mefpn0wyp0RkalLOkpnKeiS5c1fbxeldZmgPwnwtQldNUZay\n\tcO9LZcXGfogm2ie9pSgPrYLYJBGkonBWPHUqLkYSDmUyuwT5QTAps1TBvHfRxiZ7uguq\n\tMAdQ==","X-Gm-Message-State":"AOAM5304Vw7amgVVRPrB/PlXDI8yIF4l3Q92Y5wioxur8mrhK3B4rDLQ\n\tUzvF828ERo8j6IO3MIDuSnEUIg==","X-Google-Smtp-Source":"ABdhPJzCMThjHLvUTOkAZEJiNeI5rj+Aw3yjao7/qURVCaOac68noU1NqGV3YcMHsusW9BI7fjBrXw==","X-Received":"by 2002:a2e:3211:: with SMTP id\n\ty17mr6155687ljy.340.1595666128527; \n\tSat, 25 Jul 2020 01:35:28 -0700 (PDT)","Date":"Sat, 25 Jul 2020 10:35:26 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20200725083526.GH2729799@oden.dyn.berto.se>","References":"<20200718132324.867815-1-niklas.soderlund@ragnatech.se>\n\t<20200720130707.wteidbf6ecjdwrvh@uno.localdomain>\n\t<20200724100835.GD2420270@oden.dyn.berto.se>\n\t<20200724234407.GL5921@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200724234407.GL5921@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 0/9] libcamera: camera: Add camera ID","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11613,"web_url":"https://patchwork.libcamera.org/comment/11613/","msgid":"<20200727003450.GQ28704@pendragon.ideasonboard.com>","date":"2020-07-27T00:34:50","subject":"Re: [libcamera-devel] [PATCH 0/9] libcamera: camera: Add camera ID","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Sat, Jul 25, 2020 at 10:35:26AM +0200, Niklas Söderlund wrote:\n> On 2020-07-25 02:44:07 +0300, Laurent Pinchart wrote:\n> > On Fri, Jul 24, 2020 at 12:08:35PM +0200, Niklas Söderlund wrote:\n> >> On 2020-07-20 15:07:07 +0200, Jacopo Mondi wrote:\n> >>> On Sat, Jul 18, 2020 at 03:23:15PM +0200, Niklas Söderlund wrote:\n> >>>> Hello,\n> >>> \n> >>> minors apart that I will reply to patch-by-patch, I have two questions\n> >>> on the series in general\n> >>> \n> >>> 1) The series introduce and 'id' to be used alongside and\n> >>> alternatively to the camera 'name'. This might just be a matter of\n> >>> terminology, but I find this a bit confusing. Ideally, the 'name'\n> >>> should be the unique part, to which a 'model' (to mimic the API we\n> >>> have for camera sensors) could be added.\n> >> \n> >> I'm not against this change. But maybe removing the name() and adding a \n> >> model() could be done on top as this series main goal is to add a id() \n> >> field?\n> > \n> > We're entering the bikeshedding territory a little bit, but I think it's\n> > an important question. We may want to drop \"name\" altogether, as it's a\n> > confusing name (no pun intended). Using friendlyName() and uniqueName(),\n> > for instance, would make the API clearer. We can continue bikeshedding\n> > on the name, but I think whatever function names we pick, they should be\n> > fairly explicit.\n> \n> I want to avoid bikeshedding so I will only focus of the addition of the \n> new id()/uniequeName() in this series. Then once that is in we can think \n> about how we wish to evolve or remove name().\n> \n> Some bikeshedding is of course needed, in my first private incarnation \n> of this series I had named the new property uniqueName(). It became very \n> cumbersome in the documentation and felt none obvious, switching to id() \n> felt like the right thing. So while we hash this out I will keep id() as \n> a working name and then I will switch it to whatever the majority likes.\n\nI think this is important bikeshedding :-) I'm fine continuing to\ndevelop the patch series without a rename until we reach a consensus,\nbut I think we need to sort out the naming before the final version.\nThis is also not just about the function names, but also about making\nsure we have a solid scheme for camera identification. Not everything\nneeds to be implemented as part of this patch series, but the design\nneeds to cover the whole problem space.\n\n> >>> 2) The API and the cam implementation allow to use 'name' and 'id'\n> >>> interchangibily. Is this a good thing ? Applications should always use\n> >>> 'id' when interfacing to libcamera, and ideally 'name' should be a\n> >>> shortcut for users, to easily select a camera (provided it is unique).\n> >>> If I'm not mistaken it is currently possible to ask libcamera for a\n> >>> 'camera' name, should we drop this and implement that part in the\n> >>> application ? 'cam' and alike can and should use mnemonic names to\n> >>> users, but should libcamera do the same? Do we want an API that allows\n> >>> selecting camera with a name which is not guaranteed to be unique and\n> >>> consistent ? I would say we don't...\n> >> \n> >> Also I'm not against this change and I think if we all agree this series \n> >> can be modified to only allow selecting cameras based on id() instead of \n> >> id() or name() as this version of this series allows.\n> > \n> > I'm with Jacopo on this, I'd prefer if libcamera pushed applications to\n> > use a safer API, while still offering the option of an application-side\n> > manual implementation based on the friendly name.\n> \n> Then we all agree on this point and this series will modify the get() \n> function to only operate on the new id() property.\n> \n> >>> What do you think ?\n> >>> \n> >>>> This series aims to add a ID to each camera in addition to it's more\n> >>>> user-friendly name. The ID is unique and persistent between reboots of\n> >>>> the same system. The use-case for this is to create a single\n> >>>> machine-friendly ID that can be stored and used to always resolve to the\n> >>>> same camera.\n> >>>>\n> >>>> The idea on how to generate a ID is to take the sysfs path of the sensor\n> >>>> device which is part of each camera pipeline. As the path describes the\n> >>>> location of the sensor hardware it is persistent across reboots and as\n> >>>> the path is read from sysfs it's guaranteed to be unique in the system.\n> >>>>\n> >>>> For pipelines that do not have a sensor (UVC) the sysfs path of the main\n> >>>> video device is used instead. That path resolves to the USB device and\n> >>>> includes the USB bus information so it satisfy the ID requirements.\n> >>>>\n> >>>> While working with this problem it became apparent that two pipelines\n> >>>> diverge from the others on how they name their cameras, raspberrypi and\n> >>>> vimc. This series aligns these two and adds a helper to avoid such\n> >>>> situations in the future. Unfortunately this means the user-friendly\n> >>>> name of the sensor changes but this proves the need for a\n> >>>> machine-friendly ID which luckily this series also adds :-)\n> >>>>\n> >>>> Before this series camera user-friendly names on different systems\n> >>>> looked like this (I do not have access to a simple pipeline device):\n> >>>>\n> >>>> - ipu3\n> >>>>         ov13858 8-0010\n> >>>>         ov5670 10-0036\n> >>>> - raspberrypi\n> >>>>         imx219\n> >>>> - rkisp1\n> >>>>         ov5695 7-0036\n> >>>>         ov2685 7-003c\n> >>>> - uvcvideo\n> >>>>         Logitech Webcam C930e\n> >>>> - vimc\n> >>>>         VIMC Sensor B\n> >>>>\n> >>>> With this series applied the user-friendly names machine-friendly ID on\n> >>>> the same systems look like this:\n> >>>>\n> >>>> The format is:\n> >>>>     <user-friendly name> (<machine-friendly ID>)\n> >>>>\n> >>>> - ipu3\n> >>>>         ov13858 8-0010 (pci0000:00/0000:00:15.2/i2c_designware.2/i2c-8/i2c-OVTID858:00)\n> >>>>         ov5670 10-0036 (pci0000:00/0000:00:19.2/i2c_designware.5/i2c-10/i2c-INT3479:00)\n> >>>> - raspberrypi\n> >>>>         imx219 10-0010 (platform/soc/3f205000.i2c/i2c-11/i2c-10/10-0010)\n> >>>> - rkisp1\n> >>>>         ov5695 7-0036 (platform/ff160000.i2c/i2c-7/7-0036)\n> >>>>         ov2685 7-003c (platform/ff160000.i2c/i2c-7/7-003c)\n> >>>> - uvcvideo\n> >>>>         Logitech Webcam C930e (pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0)\n> >>>> - vimc\n> >>>>         Sensor B (platform/vimc.0)\n> >>>>\n> >>>> Where it previously where possible to select a camera by its\n> >>>> user-friendly name its now possible to also select it using its\n> >>>> machine-friendly one. The following is therefor two equivalent\n> >>>> commands:\n> >>>>\n> >>>>     $ cam -c \"Logitech Webcam C930e\" -C\n> >>>>     $ cam -c \"pci0000:00/0000:00:1c.4/0000:08:00.0/0000:09:02.0/0000:25:00.0/usb3/3-2/3-2.4/3-2.4:1.0\" -C\n> >>>>\n> >>>> Niklas Söderlund (9):\n> >>>>   libcamera: v4l2_device: Add method to lookup device path\n> >>>>   libcamera: camera_sensor: Expose a sensor ID\n> >>>>   libcamera: camera: Add camera ID\n> >>>>   libcamera: camera_manager: Enforce unique camera IDs\n> >>>>   libcamera: camera_manager: Try to match camera IDs first\n> >>>>   libcamera: pipeline: vimc: Align camera name\n> >>>>   libcamera: pipeline: raspberrypi: Align camera name\n> >>>>   libcamera: camera: Add create() that operates on CameraSensor\n> >>>>   cam: Print camera IDs when listing cameras\n> >>>>\n> >>>>  include/libcamera/camera.h                    | 11 +++-\n> >>>>  include/libcamera/internal/camera_sensor.h    |  2 +\n> >>>>  include/libcamera/internal/v4l2_device.h      |  1 +\n> >>>>  src/cam/main.cpp                              |  3 +-\n> >>>>  src/libcamera/camera.cpp                      | 54 +++++++++++++++----\n> >>>>  src/libcamera/camera_manager.cpp              | 13 +++++\n> >>>>  src/libcamera/camera_sensor.cpp               | 17 ++++++\n> >>>>  src/libcamera/pipeline/ipu3/ipu3.cpp          |  9 ++--\n> >>>>  .../pipeline/raspberrypi/raspberrypi.cpp      |  3 +-\n> >>>>  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  2 +-\n> >>>>  src/libcamera/pipeline/simple/simple.cpp      |  3 +-\n> >>>>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |  7 ++-\n> >>>>  src/libcamera/pipeline/vimc/vimc.cpp          |  4 +-\n> >>>>  src/libcamera/v4l2_device.cpp                 | 27 ++++++++++\n> >>>>  test/camera/buffer_import.cpp                 |  2 +-\n> >>>>  test/camera/capture.cpp                       |  2 +-\n> >>>>  test/camera/configuration_default.cpp         |  2 +-\n> >>>>  test/camera/configuration_set.cpp             |  2 +-\n> >>>>  test/camera/statemachine.cpp                  |  2 +-\n> >>>>  test/controls/control_info_map.cpp            |  2 +-\n> >>>>  test/controls/control_list.cpp                |  2 +-\n> >>>>  21 files changed, 138 insertions(+), 32 deletions(-)","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 1A2A7BD878\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 27 Jul 2020 00:35:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A61C161223;\n\tMon, 27 Jul 2020 02:34: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 7061760399\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Jul 2020 02:34:58 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BB3EB304;\n\tMon, 27 Jul 2020 02:34:57 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"WXxDDvUG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1595810097;\n\tbh=v8cz4AFYG1FP3N/zyWYwmfGmUhotxN/IAOL58e6Hjdg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WXxDDvUGh4rH7aMC4JCjIiiS7GbN6o64+mTtCuUvJHMnCrmxl5qP19qaGzd3aX/02\n\tBO24qQcamNF/PdlBNjryroB51/TS8wI1MToqalnYHySO2jU1FTW4HhzK+9g/KVBVmp\n\tYuR6gtEQotaOinAOyfSTIOYK6agLUWfaPnLvLZG8=","Date":"Mon, 27 Jul 2020 03:34:50 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<20200727003450.GQ28704@pendragon.ideasonboard.com>","References":"<20200718132324.867815-1-niklas.soderlund@ragnatech.se>\n\t<20200720130707.wteidbf6ecjdwrvh@uno.localdomain>\n\t<20200724100835.GD2420270@oden.dyn.berto.se>\n\t<20200724234407.GL5921@pendragon.ideasonboard.com>\n\t<20200725083526.GH2729799@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200725083526.GH2729799@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH 0/9] libcamera: camera: Add camera ID","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]