[{"id":11776,"web_url":"https://patchwork.libcamera.org/comment/11776/","msgid":"<20200802211717.GF23801@pendragon.ideasonboard.com>","date":"2020-08-02T21:17:17","subject":"Re: [libcamera-devel] [PATCH v5 4/5] libcamera: pipeline: uvcvideo:\n\tGenerate unique camera names","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Wed, Jul 29, 2020 at 01:50:54PM +0200, Niklas Söderlund wrote:\n> Generate camera names that are unique and persistent between system\n> resets. The name is constructed from the USB vendor and product\n> information that is stored on USB device itself and the USB bus and\n> device numbers where the hardware is plugged in.\n> \n> Before this change example of camera names:\n> \n> Venus USB2.0 Camera: Venus USB2\n> Logitech Webcam C930e\n> \n> After this change the same cameras are:\n> \n> 0ac8:3420:3:10\n> 046d:0843:3:4\n\nIs it worth trying not to include the bus number, as that is not\nguaranteed to be stable ? And more importantle, the device number is\neven less stable, it's the device address and is increased every time a\ndevice is unplugged and replugged.\n\nI'm thinking about something along the lines of\n\n  id = controller id, \"-\", ports, \":\", config, \".\", interface, \"-\", vendor id, \":\", product id ;\n  ports = port, [ \".\", ports ] ;\n  port = number;\n  config = number ;\n  interface = number\n\nwhere controller id is the DT or ACPI ID of the controller (retrieved\nthrough sysfs as for the camera sensor).\n\nWith my external webcam plugged into a hub, that would be\n\n  \\_SB_.PCI0.RP05.PXSX.TBL3.TBTU.RHUB.UB21-1.4.1:1.0-046d:080a\n\nIt would start with path = data->video_->devicePath(), and\n\n- usb_path=$(echo $(basename $path) | sed 's/^[0-9]\\+$//')\n- path=$path/..\n- id_vendor=$(cat $path/idVendor)\n- id_product=$(cat $path/idProduct)\n- while [ ! is_root($path) ] ; do\n    if [ -f $path/firmware_node ] ; then\n      controller_id=$(cat $path/firmware_node/path)\n      break\n    fi\n    if [ -f $path/of_node ] ; then\n      controller_id=$(cat $path/of_node/path)\n      break\n    fi\n    path=$path/..\n  done\n\nBonus points if this code lives in a central location, not in the UVC\npipeline handler.\n\n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n> * Changes since v3\n> - Switch argument to generateName() to UVCCameraData pointer.\n> ---\n>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 35 +++++++++++++++++++-\n>  1 file changed, 34 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 93e3dc17e3a7105e..f51529ea519f5aee 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -6,6 +6,7 @@\n>   */\n>  \n>  #include <algorithm>\n> +#include <fstream>\n>  #include <iomanip>\n>  #include <math.h>\n>  #include <tuple>\n> @@ -81,6 +82,8 @@ public:\n>  \tbool match(DeviceEnumerator *enumerator) override;\n>  \n>  private:\n> +\tstd::string generateName(const UVCCameraData *data);\n\ngenerateId() ?\n\n> +\n>  \tint processControl(ControlList *controls, unsigned int id,\n>  \t\t\t   const ControlValue &value);\n>  \tint processControls(UVCCameraData *data, Request *request);\n> @@ -379,6 +382,30 @@ int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request)\n>  \treturn 0;\n>  }\n>  \n> +std::string PipelineHandlerUVC::generateName(const UVCCameraData *data)\n> +{\n> +\n> +\tstatic const std::vector<std::string> files\n> +\t\t= { \"idVendor\", \"idProduct\", \"busnum\", \"devnum\" };\n> +\tstd::string path = data->video_->devicePath();\n> +\tstd::vector<std::string> values;\n> +\tstd::string value;\n> +\n> +\tfor (const std::string &name : files) {\n> +\t\tstd::ifstream file(path + \"/../\" + name);\n> +\n> +\t\tif (!file.is_open())\n> +\t\t\treturn \"\";\n> +\n> +\t\tstd::getline(file, value);\n> +\t\tfile.close();\n> +\n> +\t\tvalues.push_back(value);\n> +\t}\n> +\n> +\treturn utils::join(values, \":\");\n> +}\n> +\n>  bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n>  {\n>  \tMediaDevice *media;\n> @@ -405,8 +432,14 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)\n>  \t\treturn false;\n>  \n>  \t/* Create and register the camera. */\n> +\tstd::string name = generateName(data.get());\n> +\tif (name.empty()) {\n> +\t\tLOG(UVC, Error) << \"Failed to generate camera name\";\n> +\t\treturn false;\n> +\t}\n> +\n>  \tstd::set<Stream *> streams{ &data->stream_ };\n> -\tstd::shared_ptr<Camera> camera = Camera::create(this, media->model(), streams);\n> +\tstd::shared_ptr<Camera> camera = Camera::create(this, name, streams);\n>  \tregisterCamera(std::move(camera), std::move(data));\n>  \n>  \t/* Enable hot-unplug notifications. */","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 C1FE8BD87A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun,  2 Aug 2020 21:17:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4D47A60396;\n\tSun,  2 Aug 2020 23:17:31 +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 1BADB60393\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun,  2 Aug 2020 23:17:29 +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 88C99296;\n\tSun,  2 Aug 2020 23:17:28 +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=\"ZUKE/Poc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1596403048;\n\tbh=CISK0emzK9Sl2HdSmCiJZwrY89p3r8tys9XdguVRG6o=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZUKE/PocX4g7a6EqownDBFRTTww9RUxE8e7XgYfffpl5u4ZWqcSS8sk6VEWQ0qhiV\n\tw6f1nsClU1qvCy0RnDD29WRx6pVWWa1EbqNOWp7IHhKGWgm5jObBtHcClorW3PBq93\n\tvDIIekCQHIrX8XcsSOZlxdjaONecFQTf52pzlW54=","Date":"Mon, 3 Aug 2020 00:17:17 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<20200802211717.GF23801@pendragon.ideasonboard.com>","References":"<20200729115055.3840110-1-niklas.soderlund@ragnatech.se>\n\t<20200729115055.3840110-5-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200729115055.3840110-5-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH v5 4/5] libcamera: pipeline: uvcvideo:\n\tGenerate unique camera names","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>"}}]