[{"id":11662,"web_url":"https://patchwork.libcamera.org/comment/11662/","msgid":"<20200728072422.xiqpvndoodm7pt7i@uno.localdomain>","date":"2020-07-28T07:24:22","subject":"Re: [libcamera-devel] [PATCH v2 0/6] libcamera: Generate unique and\n\tstable camera names","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Tue, Jul 28, 2020 at 02:30:52AM +0200, Niklas Söderlund wrote:\n> Hello,\n>\n> This series aims to make and enforce unique camera names that are static\n> between system resets while keeping them user-friendly and adding more\n> information describing where the cameras are located in the system.\n> This v2 is a complete rewrite of v1 (libcamera: camera: Add camera ID)\n> of this series that centered around bus information instead of this v2\n> that focus more on user friendly names.\n>\n> The weakness in this series is that not a lot of platforms describe the\n> rather new location and rotation properties in their devicetree/ACPI.\n> This leads to issues that can be observed below where both cameras on\n> the ipu3 and rkisp1 platforms are reported to be located on the front\n> while in reality one is located on the front and the other on the back.\n> This is not a shortcoming of this series however and will solve itself\n> once the platforms gets update firmware or when CameraSensor learns to\n> read this information from configuration files (or by some other means).\n>\n> Patch 3/6 also needs more work to query the device enumerator for the\n> sysfs path instead of building a path that assumes the standard naming\n> schema. This is a implementation detail and I think it's more important\n> to get speedy feedback on the over all approach of the series.\n>\n> Before this series camera names on different systems looked like this (I\n> 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> Venus USB2.0 Camera: Venus USB2\n> Logitech Webcam C930e\n> - vimc\n> VIMC Sensor B\n>\n> With this series applied camera names on the same systems:\n>\n> - ipu3\n> ov13858 location front (PipelineHandlerIPU3)\n> ov5670 location front (PipelineHandlerIPU3)\n> - raspberrypi\n> imx219 location front (PipelineHandlerRPi)\n> - rkisp1\n> ov5695 location front (PipelineHandlerRkISP1)\n> ov2685 location front (PipelineHandlerRkISP1)\n> - uvcvideo\n> Venus USB2.0 Camera: Venus USB2 on bus 3:9 (PipelineHandlerUVC)\n> Logitech Webcam C930e on bus 3:4 serial 9F8F445E (PipelineHandlerUVC)\n> - vimc\n> Sensor B location front (PipelineHandlerVimc)\n\nI'm sorry if I sound negative, but I have comments on the general\nnaming scheme this series builds on. I'm sorry you went down to a full\nimplementation, discussing the names provided in that cover letter\nwould have been enough to get feedbacks imho.\n\nThat said, a few comments on the naming scheme\n1) location: it is reported through camera properties, I don't see\nmuch value having it in the name, maybe in the \"friendly\" one only,\nbut if I'm not mistaken this series makes the \"friendly\" name unique.\nI expect most applications (most, a lot..) to select cameras based on\ntheir location, and they will get it through the property, not by\nparsing the name.\n\n1.a) In a system like a RPi, where cameras are connected through flex\ncables, cameras will be marked as 'external'. If you can connect two\nsensors named \"amazingsensor\" you would get two identical camera names\n'amazingsensor location external'\n\n2) pipeline handler name: I can see UVC co-existing with other\npipeline handlers, but the UVC camera names won't collide with\nbuilt-in camera names, so I don't see much value in reporting the\npipeline handler that has created the camera I'm afraid.\nIf we need to convey this information, this should be a camera\nproperty imho.\n\nWe've ruled out -a lot- of information we could use to generate camera\nnames: video device numbers are not stable, sysfs paths are not\nbecause of that, the bus identifiers might be re-assigned between\nreboots if behing a PCI bus... there's not much in the system we can\nrely on to construct a stable and unique name if we consider all\nsystems to be equal. I however still think it should be useful to\ndefine a set of use cases we support and for each of them identify if\nthere's some information we can use.\n\nIn example, on a generic DT-based embedded system it would be enough\nto rely on\n1) the media entity name\n2) alphabetical order\n\nCombine the two and you have an ordering\n1 ov13858-2-0047\n2 ov5670-2-0036\n\nWorks for system with the same sensor, as they could not live on the\nsame bus\n\n1 ov5670-2-0036\n2 ov5670-4-0036\n\nFor non-PCI system, the i2c bus number is stable, hence, the camera on\nthe first bus comes first.\n\nFor PCI, is there a PCI id or something -stable- we can rely on ? UVC\nyou have built the vid/pid identifier here, right ? that seems stable\nto me but is it identical between two instances of the same device ?\n\nI think we really need to sort out this question before going into\ncoding. That's why I have pointed you to udev, and systemd predictable\nnames, because even if in a different context they do what we should\naim to do: define a naming scheme, and I see there reported\ninformation we might found useful about \"PCI slot number\" and \"USB\nport number chain\". Knowing how they build those part could help us\nsolve this issue, but it needs to be researched first.\n\nThanks\n j\n\n>\n> Niklas Söderlund (6):\n> libcamera: camera: Append pipeline name to camera name\n> libcamera: camera: Generate camera name from a CameraSensor\n> libcamera: v4l2_device: Add method to lookup device path\n> libcamera: media_device: Expose media device serial number\n> libcamera: pipeline: uvcvideo: Generate unique camera names\n> libcamera: camera_manager: Enforce unique camera names\n>\n> include/libcamera/camera.h | 5 +++\n> include/libcamera/internal/media_device.h | 2 +\n> include/libcamera/internal/v4l2_device.h | 1 +\n> src/libcamera/camera.cpp | 42 ++++++++++++++++++-\n> src/libcamera/camera_manager.cpp | 6 +--\n> src/libcamera/media_device.cpp | 7 ++++\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 12 +++---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 3 +-\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-\n> src/libcamera/pipeline/simple/simple.cpp | 2 +-\n> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 41 +++++++++++++++++-\n> src/libcamera/pipeline/vimc/vimc.cpp | 4 +-\n> src/libcamera/v4l2_device.cpp | 24 +++++++++++\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> test/serialization/serialization_test.h | 2 +-\n> 21 files changed, 142 insertions(+), 25 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":"","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 3FBFABD878\n\tfor ;\n\tTue, 28 Jul 2020 07:20:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C521860923;\n\tTue, 28 Jul 2020 09:20:45 +0200 (CEST)","from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 953DF6074F\n\tfor ;\n\tTue, 28 Jul 2020 09:20:43 +0200 (CEST)","from uno.localdomain (host-79-26-24-120.retail.telecomitalia.it\n\t[79.26.24.120]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 98A6140005;\n\tTue, 28 Jul 2020 07:20:42 +0000 (UTC)"],"X-Originating-IP":"79.26.24.120","Date":"Tue, 28 Jul 2020 09:24:22 +0200","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20200728072422.xiqpvndoodm7pt7i@uno.localdomain>","References":"<20200728003058.2871461-1-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200728003058.2871461-1-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH v2 0/6] libcamera: Generate unique and\n\tstable camera names","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","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\" "}}]