[{"id":11701,"web_url":"https://patchwork.libcamera.org/comment/11701/","msgid":"<2743b579-9dd8-ed51-8b5f-a623153c6fde@ideasonboard.com>","date":"2020-07-29T10:54:40","subject":"Re: [libcamera-devel] [PATCH v4 4/5] libcamera: pipeline: uvcvideo:\n\tGenerate unique camera names","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Niklas,\n\nOn 29/07/2020 10:21, 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> \n\nI don't see any update to cam or qcam in this series.\n\nIs that really what we want to present to the users in this new-world-order?\n\nUnique 'yes'\nConsistent 'yes'\n\nAllows the 'user' to pick the right camera .... 'nope'.\n\n\n\n> Signed-off-by: Niklas Söderlund \n> Reviewed-by: Jacopo Mondi \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 \n> +#include \n> #include \n> #include \n> #include \n> @@ -81,6 +82,8 @@ public:\n> \tbool match(DeviceEnumerator *enumerator) override;\n> \n> private:\n> +\tstd::string generateName(const UVCCameraData *data);\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 files\n> +\t\t= { \"idVendor\", \"idProduct\", \"busnum\", \"devnum\" };\n> +\tstd::string path = data->video_->devicePath();\n> +\tstd::vector 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 streams{ &data->stream_ };\n> -\tstd::shared_ptr camera = Camera::create(this, media->model(), streams);\n> +\tstd::shared_ptr camera = Camera::create(this, name, streams);\n> \tregisterCamera(std::move(camera), std::move(data));\n> \n> \t/* Enable hot-unplug notifications. */\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 27BA1BD878\n\tfor ;\n\tWed, 29 Jul 2020 10:54:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9A5EB613C6;\n\tWed, 29 Jul 2020 12:54:44 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 44E6C6039D\n\tfor ;\n\tWed, 29 Jul 2020 12:54:43 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B07DF31F;\n\tWed, 29 Jul 2020 12:54:42 +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=\"CFGA2zxO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1596020082;\n\tbh=bYr0HZn8tY5VGbfPKljKwM9RYrZgYLDWn1NRpJ289yI=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=CFGA2zxOQi9Y3gZuCoJKz2kEFbY6J1Tb15/kVuU7NF52266B64QB6g1N/Rulmsvop\n\tDrhyACq+UC51OlKZfy7x+7jyhU/eT+lfGxmBJ3fc4ZdV8xGwWLCfvoDeadjJ/YoT//\n\tBj0x7PEgXSMWKw378j2xuK5X8SrOP7btTRCZoGi8=","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20200729092122.3765539-1-niklas.soderlund@ragnatech.se>\n\t<20200729092122.3765539-5-niklas.soderlund@ragnatech.se>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<2743b579-9dd8-ed51-8b5f-a623153c6fde@ideasonboard.com>","Date":"Wed, 29 Jul 2020 11:54:40 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20200729092122.3765539-5-niklas.soderlund@ragnatech.se>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v4 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11705,"web_url":"https://patchwork.libcamera.org/comment/11705/","msgid":"<20200729113908.GD322953@oden.dyn.berto.se>","date":"2020-07-29T11:39:08","subject":"Re: [libcamera-devel] [PATCH v4 4/5] libcamera: pipeline: uvcvideo:\n\tGenerate unique camera names","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Kieran,\n\nThanks for your feedback.\n\nOn 2020-07-29 11:54:40 +0100, Kieran Bingham wrote:\n> Hi Niklas,\n> \n> On 29/07/2020 10:21, 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> > \n> \n> I don't see any update to cam or qcam in this series.\n> \n> Is that really what we want to present to the users in this new-world-order?\n> \n> Unique 'yes'\n> Consistent 'yes'\n> \n> Allows the 'user' to pick the right camera .... 'nope'.\n\nAs discussed on previous versions of this series and irc the name of \ncameras is not suitable to be presented to users expect for \ndebugging/development tools. Examples of badly formatted UVC model names \nand serial numbers where brought up as examples on why they should not \nbe part of the camera name.\n\nThe use-case for the name is for applications to be able to have a \nidentifier that is unique and persistent.\n\nTo address your concern about a user-friendly string applications should \nconstruct one themself using informations and controls exposed by the \nCamera. The string could contain information about where in the system a \ncamera is located and the string might need to be localized and other \nsuch concerns.\n\nI agree work is needed on cam and qcam to support this. That is however \na different problem this series do not address. I hope to have shot at \nworking on that as soon as we have this work merged.\n\n> \n> \n> \n> > Signed-off-by: Niklas Söderlund \n> > Reviewed-by: Jacopo Mondi \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 \n> > +#include \n> > #include \n> > #include \n> > #include \n> > @@ -81,6 +82,8 @@ public:\n> > \tbool match(DeviceEnumerator *enumerator) override;\n> > \n> > private:\n> > +\tstd::string generateName(const UVCCameraData *data);\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 files\n> > +\t\t= { \"idVendor\", \"idProduct\", \"busnum\", \"devnum\" };\n> > +\tstd::string path = data->video_->devicePath();\n> > +\tstd::vector 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 streams{ &data->stream_ };\n> > -\tstd::shared_ptr camera = Camera::create(this, media->model(), streams);\n> > +\tstd::shared_ptr camera = Camera::create(this, name, streams);\n> > \tregisterCamera(std::move(camera), std::move(data));\n> > \n> > \t/* Enable hot-unplug notifications. */\n> > \n> \n> -- \n> Regards\n> --\n> Kieran","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 668C3BD86F\n\tfor ;\n\tWed, 29 Jul 2020 11:39:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D615B613C6;\n\tWed, 29 Jul 2020 13:39:12 +0200 (CEST)","from mail-lj1-x242.google.com (mail-lj1-x242.google.com\n\t[IPv6:2a00:1450:4864:20::242])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 20C7F6039D\n\tfor ;\n\tWed, 29 Jul 2020 13:39:11 +0200 (CEST)","by mail-lj1-x242.google.com with SMTP id f5so24657282ljj.10\n\tfor ;\n\tWed, 29 Jul 2020 04:39:11 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tt13sm401778lfg.53.2020.07.29.04.39.09\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 29 Jul 2020 04:39:09 -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=\"umrbioFl\"; 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=VVPHDD6sAUgqOpWqIAX8ui4SR5nCjFzCOeAWyVemubc=;\n\tb=umrbioFlhna7G7hrbpFbTNiE5mOuUgXbBwkontgHWls8BcdZ87a/F6Ah5S4gQzN6G9\n\tPnts/eLONKqZTG7v5RVtNgXewU1645/CfVOG7q4cLxpHJVtkMpVWrS4o+ZWQplFl4fNN\n\t0jpeMmclLMRoStMIfhDSS4WSruDa628EGHTn5UAgw8lFkn7P0aQJUAbokts8Pogv2+cK\n\thgUmhXwVMr5lXO0uERTKpYNNNRV8bCwTZzLvEGWpoUVStuTJubNbbhQCxUtwIwMvLBTn\n\tbpUqQMWLp2hI2rIHncnLstOnxASumAmYSaFc5hKIkYYVc7AMrP0qjaP5f46Vi+NXBp6y\n\tfcTQ==","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=VVPHDD6sAUgqOpWqIAX8ui4SR5nCjFzCOeAWyVemubc=;\n\tb=ATKPP5S3lXWDLk3xpXOIlIIuaz4ucATeLOCsY+cmuuXL3MQy47ChYtCMtsgVLW46eT\n\tu0d5pMCOoW+ycIEF41O+O2YUWTA2oYeeKLFDiN2xrlwX1vGwnKV1q4OTmMuIFrlbv4TI\n\tnqvQL/zEJY1ca8eeSS/Iauds5diHOvGdocYCPishar25m16ku2dIsQs8JfqmVLJivHPL\n\tZA7K/+U2jQs8rerqZ3Ilvm6Wv5YFcugcz+fKBCUeMDlWtrtkcBGEmwDzvyOp03UEGRZH\n\tBORLkZQ9SyNqJR1Ji28CaGPdQN/0tZEs+0RWnP9AiZU/cKWJWS5CN3GQQhJCsjYirvG9\n\tSPRw==","X-Gm-Message-State":"AOAM530bQT6gswi7eVtRI3g4Crphlrf+YWMHBqEWK3uBYdJBRiSFSGNQ\n\tTEDrXyFsPuLYN+ivVWgC8/06I2aAm0c=","X-Google-Smtp-Source":"ABdhPJyC64OOxQtiyW0SXq9vzkPwJuZd+gNzl4vs9nam+HWeFyp2sQbiDT5X/lgUnG23mk9CY/1AbA==","X-Received":"by 2002:a2e:240e:: with SMTP id\n\tk14mr13898759ljk.71.1596022750385; \n\tWed, 29 Jul 2020 04:39:10 -0700 (PDT)","Date":"Wed, 29 Jul 2020 13:39:08 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Kieran Bingham ","Message-ID":"<20200729113908.GD322953@oden.dyn.berto.se>","References":"<20200729092122.3765539-1-niklas.soderlund@ragnatech.se>\n\t<20200729092122.3765539-5-niklas.soderlund@ragnatech.se>\n\t<2743b579-9dd8-ed51-8b5f-a623153c6fde@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<2743b579-9dd8-ed51-8b5f-a623153c6fde@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 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":"","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=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11972,"web_url":"https://patchwork.libcamera.org/comment/11972/","msgid":"","date":"2020-08-10T16:07:54","subject":"Re: [libcamera-devel] [PATCH v4 4/5] libcamera: pipeline: uvcvideo:\n\tGenerate unique camera names","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Le mercredi 29 juillet 2020 à 13:39 +0200, Niklas Söderlund a écrit :\n> Hi Kieran,\n> \n> Thanks for your feedback.\n> \n> On 2020-07-29 11:54:40 +0100, Kieran Bingham wrote:\n> > Hi Niklas,\n> > \n> > On 29/07/2020 10:21, 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> > > \n> > \n> > I don't see any update to cam or qcam in this series.\n> > \n> > Is that really what we want to present to the users in this new-world-order?\n> > \n> > Unique 'yes'\n> > Consistent 'yes'\n> > \n> > Allows the 'user' to pick the right camera .... 'nope'.\n> \n> As discussed on previous versions of this series and irc the name of \n> cameras is not suitable to be presented to users expect for \n> debugging/development tools. Examples of badly formatted UVC model names \n> and serial numbers where brought up as examples on why they should not \n> be part of the camera name.\n> \n> The use-case for the name is for applications to be able to have a \n> identifier that is unique and persistent.\n> \n> To address your concern about a user-friendly string applications should \n> construct one themself using informations and controls exposed by the \n> Camera. The string could contain information about where in the system a \n> camera is located and the string might need to be localized and other \n> such concerns.\n> \n> I agree work is needed on cam and qcam to support this. That is however \n> a different problem this series do not address. I hope to have shot at \n> working on that as soon as we have this work merged.\n\nI believe the fact it's called name() makes that API dedicated to\nsomething human readable. I was expecting a new API called id() or\nidentifier(). At least all other systems seems to make an effort to\nkeep it human recognizable, perhaps you should have a look at\npulseaudio unique name scheme ?\n\nI don't think it's great to wave away the ability to display something\nslightly human readable, even for debugging having just string of\nhexadecimal (UVC cases) will make things much harder.\n\nI also dislike this waving away as it will make it looks like this\nbasic information is optional, but it should not be.\n\n> \n> > \n> > \n> > > Signed-off-by: Niklas Söderlund \n> > > Reviewed-by: Jacopo Mondi \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 \n> > > +#include \n> > > #include \n> > > #include \n> > > #include \n> > > @@ -81,6 +82,8 @@ public:\n> > > \tbool match(DeviceEnumerator *enumerator) override;\n> > > \n> > > private:\n> > > +\tstd::string generateName(const UVCCameraData *data);\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 files\n> > > +\t\t= { \"idVendor\", \"idProduct\", \"busnum\", \"devnum\" };\n> > > +\tstd::string path = data->video_->devicePath();\n> > > +\tstd::vector 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 streams{ &data->stream_ };\n> > > -\tstd::shared_ptr camera = Camera::create(this, media->model(), streams);\n> > > +\tstd::shared_ptr camera = Camera::create(this, name, streams);\n> > > \tregisterCamera(std::move(camera), std::move(data));\n> > > \n> > > \t/* Enable hot-unplug notifications. */\n> > > \n> > \n> > -- \n> > Regards\n> > --\n> > Kieran","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 29B7FBD87C\n\tfor ;\n\tMon, 10 Aug 2020 16:07:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ADDB261012;\n\tMon, 10 Aug 2020 18:07:57 +0200 (CEST)","from mail-qk1-x744.google.com (mail-qk1-x744.google.com\n\t[IPv6:2607:f8b0:4864:20::744])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DE6BB6038F\n\tfor ;\n\tMon, 10 Aug 2020 18:07:56 +0200 (CEST)","by mail-qk1-x744.google.com with SMTP id l64so8801691qkb.8\n\tfor ;\n\tMon, 10 Aug 2020 09:07:56 -0700 (PDT)","from skullcanyon ([192.222.193.21])\n\tby smtp.gmail.com with ESMTPSA id\n\tq13sm15504866qkn.85.2020.08.10.09.07.54\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 10 Aug 2020 09:07:54 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ndufresne-ca.20150623.gappssmtp.com\n\theader.i=@ndufresne-ca.20150623.gappssmtp.com\n\theader.b=\"aMNPctEj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20150623.gappssmtp.com; s=20150623;\n\th=message-id:subject:from:to:cc:date:in-reply-to:references\n\t:user-agent:mime-version:content-transfer-encoding;\n\tbh=DoczDrOk4ZbqBySh1V+BVGfihGlRvsubaZB9POYOHZ4=;\n\tb=aMNPctEjpqr57VXav//zjOpb8er/UpIUpKAHkIjHvJckYiUD38CeWMxAdzoweQ4DBr\n\tmnwSAS2SWowRnvpHmn+zGfH8Q1h9cGJ1QJJ0xOzXnKox8kI2U7brbc4IiF7rlGEPimAO\n\t+f1k+jIN/6huZ9aVXrl5yOSdckf0ysAcljU6IqmsBtISY+GGwRF2bQ5U7Hzbt81An7dJ\n\tXz/WvzUMDHRmA8Y/SPE0dcHxjLCiJp/xYW6DkObSgnrfeUfTGfTkTqjgHPhGmPiTP/k0\n\t72VHIP2gdi59CwTTisS39fBZI48Xt9CS90+0fpADB9L/gIBaOWGY2j3RnlFNO+3tTXFe\n\tjxxA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to\n\t:references:user-agent:mime-version:content-transfer-encoding;\n\tbh=DoczDrOk4ZbqBySh1V+BVGfihGlRvsubaZB9POYOHZ4=;\n\tb=OJqS9sMIn1TrQUkYC12rvROzihhiWjVExu4q98tpHz2BT6iHhzRYXnabKYXUO3GYZa\n\tikfoRlhy+APAtPLJND3aGWwPWtqndxyk9fyx52tf9YCSoZMCa4f6yMgjoq3mCqFZrO+p\n\tWK63KmsILO2SbfLxN47T5g5ceXG6nf0m0f8faxhqvqActg6AWZrNGD5rDS3Myk31pSDA\n\tKJN4m9ReaCSVRPtbJ+v2EprOBDd7QGWLYvmQlf1dNb5ShrcW5r7hrJTHbVB69qmIapYC\n\t8kLmvK77HUDahKpDMaCLgqefzrXnwGYpTn96D7FCTaCVjJZwAJxMDhJyI2Urjy0rjnk6\n\t0xRw==","X-Gm-Message-State":"AOAM533HNmOmGfqqMl8OpCQib4SYJqdjd5O9atusAB0uMeWqrozO2gFj\n\t5tIOhmc14v+Srt2Bkp6OgxdpLlWXoGiNRQ==","X-Google-Smtp-Source":"ABdhPJyvAys8U+T23l8eHF/dJjZeKe59Ak+398BROiel4f+YdZw4kFVd5BYG+SeLFVyb1ZgtKngoAA==","X-Received":"by 2002:a05:620a:1256:: with SMTP id\n\ta22mr26230870qkl.95.1597075675703; \n\tMon, 10 Aug 2020 09:07:55 -0700 (PDT)","Message-ID":"","From":"Nicolas Dufresne ","To":"Niklas =?iso-8859-1?q?S=F6derlund?= ,\n\tKieran Bingham ","Date":"Mon, 10 Aug 2020 12:07:54 -0400","In-Reply-To":"<20200729113908.GD322953@oden.dyn.berto.se>","References":"<20200729092122.3765539-1-niklas.soderlund@ragnatech.se>\n\t<20200729092122.3765539-5-niklas.soderlund@ragnatech.se>\n\t<2743b579-9dd8-ed51-8b5f-a623153c6fde@ideasonboard.com>\n\t<20200729113908.GD322953@oden.dyn.berto.se>","User-Agent":"Evolution 3.36.4 (3.36.4-1.fc32) ","MIME-Version":"1.0","Subject":"Re: [libcamera-devel] [PATCH v4 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":"","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\" "}},{"id":11974,"web_url":"https://patchwork.libcamera.org/comment/11974/","msgid":"<20200810162523.GI6056@pendragon.ideasonboard.com>","date":"2020-08-10T16:25:23","subject":"Re: [libcamera-devel] [PATCH v4 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 Nicolas,\n\nOn Mon, Aug 10, 2020 at 12:07:54PM -0400, Nicolas Dufresne wrote:\n> Le mercredi 29 juillet 2020 à 13:39 +0200, Niklas Söderlund a écrit :\n> > On 2020-07-29 11:54:40 +0100, Kieran Bingham wrote:\n> > > On 29/07/2020 10:21, 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> > > \n> > > I don't see any update to cam or qcam in this series.\n> > > \n> > > Is that really what we want to present to the users in this new-world-order?\n> > > \n> > > Unique 'yes'\n> > > Consistent 'yes'\n> > > \n> > > Allows the 'user' to pick the right camera .... 'nope'.\n> > \n> > As discussed on previous versions of this series and irc the name of \n> > cameras is not suitable to be presented to users expect for \n> > debugging/development tools. Examples of badly formatted UVC model names \n> > and serial numbers where brought up as examples on why they should not \n> > be part of the camera name.\n> > \n> > The use-case for the name is for applications to be able to have a \n> > identifier that is unique and persistent.\n> > \n> > To address your concern about a user-friendly string applications should \n> > construct one themself using informations and controls exposed by the \n> > Camera. The string could contain information about where in the system a \n> > camera is located and the string might need to be localized and other \n> > such concerns.\n> > \n> > I agree work is needed on cam and qcam to support this. That is however \n> > a different problem this series do not address. I hope to have shot at \n> > working on that as soon as we have this work merged.\n> \n> I believe the fact it's called name() makes that API dedicated to\n> something human readable. I was expecting a new API called id() or\n> identifier().\n\nFully agreed. v4 isn't the latest version of this series. The code that\nhas been merged calls this id().\n\n> At least all other systems seems to make an effort to\n> keep it human recognizable, perhaps you should have a look at\n> pulseaudio unique name scheme ?\n\nIt's a good idea. Any pointer ?\n\n> I don't think it's great to wave away the ability to display something\n> slightly human readable, even for debugging having just string of\n> hexadecimal (UVC cases) will make things much harder.\n> \n> I also dislike this waving away as it will make it looks like this\n> basic information is optional, but it should not be.\n\nNiklas has submitted another series aimed at solving the human-readable\nname issue, in a properly designed way this time. The overall idea is to\nexpose all the information needed to construct a name, and let\napplications construct it. I think we should also have a helper function\nto provide a default constructed name.\n\n> > > > Signed-off-by: Niklas Söderlund \n> > > > Reviewed-by: Jacopo Mondi \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 \n> > > > +#include \n> > > > #include \n> > > > #include \n> > > > #include \n> > > > @@ -81,6 +82,8 @@ public:\n> > > > \tbool match(DeviceEnumerator *enumerator) override;\n> > > > \n> > > > private:\n> > > > +\tstd::string generateName(const UVCCameraData *data);\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 files\n> > > > +\t\t= { \"idVendor\", \"idProduct\", \"busnum\", \"devnum\" };\n> > > > +\tstd::string path = data->video_->devicePath();\n> > > > +\tstd::vector 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 streams{ &data->stream_ };\n> > > > -\tstd::shared_ptr camera = Camera::create(this, media->model(), streams);\n> > > > +\tstd::shared_ptr camera = Camera::create(this, name, streams);\n> > > > \tregisterCamera(std::move(camera), std::move(data));\n> > > > \n> > > > \t/* Enable hot-unplug notifications. */","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 0B824BD87C\n\tfor ;\n\tMon, 10 Aug 2020 16:25:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8DA036038F;\n\tMon, 10 Aug 2020 18:25:38 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D2D9E6038F\n\tfor ;\n\tMon, 10 Aug 2020 18:25:36 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 071CE548;\n\tMon, 10 Aug 2020 18:25:35 +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=\"iQlknzUT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1597076736;\n\tbh=gxXDuTYOYWFIcIFcen9UQyhGyHx6UzpYtFAYSt0YnV4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=iQlknzUTj9VBrfE/jSV9FQKk17S9cGtt4cl8GD0IfCXiHjhpmfN+udzCpPmFFDGMH\n\tyfkJge7U6upAFVtZUk3WBLU4Bvg0vLd4aBawMl3bmGcLV5JjcHDhM3LEH4HxOrpk57\n\t7FCe9AWL2u1cCLk0rQ6x4AOv05wmy/bcT8pOWPBU=","Date":"Mon, 10 Aug 2020 19:25:23 +0300","From":"Laurent Pinchart ","To":"Nicolas Dufresne ","Message-ID":"<20200810162523.GI6056@pendragon.ideasonboard.com>","References":"<20200729092122.3765539-1-niklas.soderlund@ragnatech.se>\n\t<20200729092122.3765539-5-niklas.soderlund@ragnatech.se>\n\t<2743b579-9dd8-ed51-8b5f-a623153c6fde@ideasonboard.com>\n\t<20200729113908.GD322953@oden.dyn.berto.se>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v4 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":"","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\" "}},{"id":11975,"web_url":"https://patchwork.libcamera.org/comment/11975/","msgid":"<20200810163641.GA767913@oden.dyn.berto.se>","date":"2020-08-10T16:36:41","subject":"Re: [libcamera-devel] [PATCH v4 4/5] libcamera: pipeline: uvcvideo:\n\tGenerate unique camera names","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Nicolas,\n\nThanks for your feedback.\n\nOn 2020-08-10 12:07:54 -0400, Nicolas Dufresne wrote:\n> Le mercredi 29 juillet 2020 à 13:39 +0200, Niklas Söderlund a écrit :\n> > Hi Kieran,\n> > \n> > Thanks for your feedback.\n> > \n> > On 2020-07-29 11:54:40 +0100, Kieran Bingham wrote:\n> > > Hi Niklas,\n> > > \n> > > On 29/07/2020 10:21, 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> > > > \n> > > \n> > > I don't see any update to cam or qcam in this series.\n> > > \n> > > Is that really what we want to present to the users in this new-world-order?\n> > > \n> > > Unique 'yes'\n> > > Consistent 'yes'\n> > > \n> > > Allows the 'user' to pick the right camera .... 'nope'.\n> > \n> > As discussed on previous versions of this series and irc the name of \n> > cameras is not suitable to be presented to users expect for \n> > debugging/development tools. Examples of badly formatted UVC model names \n> > and serial numbers where brought up as examples on why they should not \n> > be part of the camera name.\n> > \n> > The use-case for the name is for applications to be able to have a \n> > identifier that is unique and persistent.\n> > \n> > To address your concern about a user-friendly string applications should \n> > construct one themself using informations and controls exposed by the \n> > Camera. The string could contain information about where in the system a \n> > camera is located and the string might need to be localized and other \n> > such concerns.\n> > \n> > I agree work is needed on cam and qcam to support this. That is however \n> > a different problem this series do not address. I hope to have shot at \n> > working on that as soon as we have this work merged.\n> \n> I believe the fact it's called name() makes that API dedicated to\n> something human readable. I was expecting a new API called id() or\n> identifier(). At least all other systems seems to make an effort to\n> keep it human recognizable, perhaps you should have a look at\n> pulseaudio unique name scheme ?\n\nThis is a old version of this series v8 is the one that got merged and \nthe structure of the ID have changed a bit since this. Also the \ninterface is changed to id() instead of name(), as you point out it \nmakes more sens. The Camera::name() is however removed as its true \nintent was better described by the id().\n\n> \n> I don't think it's great to wave away the ability to display something\n> slightly human readable, even for debugging having just string of\n> hexadecimal (UVC cases) will make things much harder.\n\nAgreed, we discussed this in length during this work and came to the \nconclusion that different use-cases have different needs for the \nuser-friendly name. Some cases the location of the camera (front, back) \nmakes more sens then the sensor model name for example. We also thought \nabout the need for users of the API to be able to localize the \nuser-friendly name.\n\nThe idea is to allow applications to build a user-friendly name which \nmakes sens for it's use-case. For example we have a pending series that \ndoes this for cam already [1].\n\n> \n> I also dislike this waving away as it will make it looks like this\n> basic information is optional, but it should not be.\n\nI understand your point. The idea has been for a long while to work on \nthe old Camera::name() interface to grantee the name is unique and \nconstant in the system and then use other properties to construct \nuser-friendly names. Maybe we could have done it the other way around \ntho, first constructing new user-friendly names from properties and then \nmade the Camera::name() -> Camera::id() change.\n\n1. [PATCH 0/7] libcamera: Allow for user-friendly names in applications\n\n> \n> > \n> > > \n> > > \n> > > > Signed-off-by: Niklas Söderlund \n> > > > Reviewed-by: Jacopo Mondi \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 \n> > > > +#include \n> > > > #include \n> > > > #include \n> > > > #include \n> > > > @@ -81,6 +82,8 @@ public:\n> > > > \tbool match(DeviceEnumerator *enumerator) override;\n> > > > \n> > > > private:\n> > > > +\tstd::string generateName(const UVCCameraData *data);\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 files\n> > > > +\t\t= { \"idVendor\", \"idProduct\", \"busnum\", \"devnum\" };\n> > > > +\tstd::string path = data->video_->devicePath();\n> > > > +\tstd::vector 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 streams{ &data->stream_ };\n> > > > -\tstd::shared_ptr camera = Camera::create(this, media->model(), streams);\n> > > > +\tstd::shared_ptr camera = Camera::create(this, name, streams);\n> > > > \tregisterCamera(std::move(camera), std::move(data));\n> > > > \n> > > > \t/* Enable hot-unplug notifications. */\n> > > > \n> > > \n> > > -- \n> > > Regards\n> > > --\n> > > Kieran\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2D362BD87D\n\tfor ;\n\tMon, 10 Aug 2020 16:36:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A5CA460DCA;\n\tMon, 10 Aug 2020 18:36:44 +0200 (CEST)","from mail-lj1-x242.google.com (mail-lj1-x242.google.com\n\t[IPv6:2a00:1450:4864:20::242])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8B2406038F\n\tfor ;\n\tMon, 10 Aug 2020 18:36:43 +0200 (CEST)","by mail-lj1-x242.google.com with SMTP id w14so10302377ljj.4\n\tfor ;\n\tMon, 10 Aug 2020 09:36:43 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tv5sm9408745ljh.73.2020.08.10.09.36.41\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 10 Aug 2020 09:36:41 -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=\"fh0kMkAq\"; 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=ss5x/SFAM+QI60AZYPV+7Qvsw8wIfi4eyBLZk21Mgiw=;\n\tb=fh0kMkAq/gxpgd/qHDYwPCNDQCmFWVjn/V/kpL6dJhhtPOOIdGSkSxlzLzYzwUh99h\n\tzJvjoehdgx80+vglX70NmneCV1iYCRebrNMjRa7yJ6RNQy4wiRkXd+hii4KEhfzbicVC\n\tQ40hBS+MuOsR8kLshTcsAwAavTJh56o8Ow6O/hxg5dBiDz1LMVkvd8AwPT00EhIPOfq9\n\tv8iuUN6MPVIydedWXzVNeBjXUWbS+MnMqZYhTim+RNfCAFOy5xRfiQZnYcoMuuurnwZ3\n\trsnB8YBqptcWDO6VUHSx+Ts5z+MJbDojB26WniWmtwZLP2vVht7myIPskldm9QRHtCxJ\n\tdsFg==","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=ss5x/SFAM+QI60AZYPV+7Qvsw8wIfi4eyBLZk21Mgiw=;\n\tb=OV3Xt0qVSpkWD+U/UNi7pe0+WJc84NoheulWj9lE5VBGVUGPgQoSOeA/kFUsLZtGja\n\t7PoC/nK22+hvZ0k1UVUBZGYFSkwaGFWi0CBmPUq1b7PER1efV4nfYnRiarYdqVGFyahM\n\tSHtriiIk0b+Hny8/b1AgHpom5VtPOo3qye0weGWKk2plvgUMy66Fjzibi60bdMLUq0VI\n\tlQMcQKPqK551WyoYOv3E8O6f5hGrYtxlq4qjnW72VTzMiirvE3Zot69wSdi0dx76HlGo\n\t1Vp+RVLf2tlmAhGAysDiuvIG2K3LMgGIVqrdK9JDZBD6tuNENI7Bx/g/pyIblgO30iu8\n\twW0g==","X-Gm-Message-State":"AOAM530Ds9GGcX9lJmkSvLtydtifAMPYWlA35jbj1RDB0hiLtAA/bQcf\n\tpiaHNtpBUZblmLOohrAghbjkQg==","X-Google-Smtp-Source":"ABdhPJy+/FEyIad1/pYFdwunoAd9kmcGfTUcMf+GOfFQFsm9ca3Fw9y5rmyxZhQCR35w5c59QSuwAQ==","X-Received":"by 2002:a2e:3817:: with SMTP id f23mr930772lja.118.1597077402578;\n\tMon, 10 Aug 2020 09:36:42 -0700 (PDT)","Date":"Mon, 10 Aug 2020 18:36:41 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Nicolas Dufresne ","Message-ID":"<20200810163641.GA767913@oden.dyn.berto.se>","References":"<20200729092122.3765539-1-niklas.soderlund@ragnatech.se>\n\t<20200729092122.3765539-5-niklas.soderlund@ragnatech.se>\n\t<2743b579-9dd8-ed51-8b5f-a623153c6fde@ideasonboard.com>\n\t<20200729113908.GD322953@oden.dyn.berto.se>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v4 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":"","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=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]