[libcamera-devel,v5,4/5] libcamera: pipeline: uvcvideo: Generate unique camera names

9075 diff mbox series

Generate camera names that are unique and persistent between system
resets. The name is constructed from the USB vendor and product
information that is stored on USB device itself and the USB bus and
device numbers where the hardware is plugged in.

Before this change example of camera names:

Venus USB2.0 Camera: Venus USB2
Logitech Webcam C930e

After this change the same cameras are:

0ac8:3420:3:10
046d:0843:3:4

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
---
* Changes since v3
- Switch argument to generateName() to UVCCameraData pointer.
---
 src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 35 +++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

Hi Niklas,

Thank you for the patch.

On Wed, Jul 29, 2020 at 01:50:54PM +0200, Niklas Söderlund wrote:
> Generate camera names that are unique and persistent between system
> resets. The name is constructed from the USB vendor and product
> information that is stored on USB device itself and the USB bus and
> device numbers where the hardware is plugged in.
> 
> Before this change example of camera names:
> 
> Venus USB2.0 Camera: Venus USB2
> Logitech Webcam C930e
> 
> After this change the same cameras are:
> 
> 0ac8:3420:3:10
> 046d:0843:3:4

Is it worth trying not to include the bus number, as that is not
guaranteed to be stable ? And more importantle, the device number is
even less stable, it's the device address and is increased every time a
device is unplugged and replugged.

I'm thinking about something along the lines of

  id = controller id, "-", ports, ":", config, ".", interface, "-", vendor id, ":", product id ;
  ports = port, [ ".", ports ] ;
  port = number;
  config = number ;
  interface = number

where controller id is the DT or ACPI ID of the controller (retrieved
through sysfs as for the camera sensor).

With my external webcam plugged into a hub, that would be

  \_SB_.PCI0.RP05.PXSX.TBL3.TBTU.RHUB.UB21-1.4.1:1.0-046d:080a

It would start with path = data->video_->devicePath(), and

- usb_path=$(echo $(basename $path) | sed 's/^[0-9]\+$//')
- path=$path/..
- id_vendor=$(cat $path/idVendor)
- id_product=$(cat $path/idProduct)
- while [ ! is_root($path) ] ; do
    if [ -f $path/firmware_node ] ; then
      controller_id=$(cat $path/firmware_node/path)
      break
    fi
    if [ -f $path/of_node ] ; then
      controller_id=$(cat $path/of_node/path)
      break
    fi
    path=$path/..
  done

Bonus points if this code lives in a central location, not in the UVC
pipeline handler.

> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
> * Changes since v3
> - Switch argument to generateName() to UVCCameraData pointer.
> ---
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 35 +++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 93e3dc17e3a7105e..f51529ea519f5aee 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -6,6 +6,7 @@
>   */
>  
>  #include <algorithm>
> +#include <fstream>
>  #include <iomanip>
>  #include <math.h>
>  #include <tuple>
> @@ -81,6 +82,8 @@ public:
>  	bool match(DeviceEnumerator *enumerator) override;
>  
>  private:
> +	std::string generateName(const UVCCameraData *data);

generateId() ?

> +
>  	int processControl(ControlList *controls, unsigned int id,
>  			   const ControlValue &value);
>  	int processControls(UVCCameraData *data, Request *request);
> @@ -379,6 +382,30 @@ int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request)
>  	return 0;
>  }
>  
> +std::string PipelineHandlerUVC::generateName(const UVCCameraData *data)
> +{
> +
> +	static const std::vector<std::string> files
> +		= { "idVendor", "idProduct", "busnum", "devnum" };
> +	std::string path = data->video_->devicePath();
> +	std::vector<std::string> values;
> +	std::string value;
> +
> +	for (const std::string &name : files) {
> +		std::ifstream file(path + "/../" + name);
> +
> +		if (!file.is_open())
> +			return "";
> +
> +		std::getline(file, value);
> +		file.close();
> +
> +		values.push_back(value);
> +	}
> +
> +	return utils::join(values, ":");
> +}
> +
>  bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
>  {
>  	MediaDevice *media;
> @@ -405,8 +432,14 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
>  		return false;
>  
>  	/* Create and register the camera. */
> +	std::string name = generateName(data.get());
> +	if (name.empty()) {
> +		LOG(UVC, Error) << "Failed to generate camera name";
> +		return false;
> +	}
> +
>  	std::set<Stream *> streams{ &data->stream_ };
> -	std::shared_ptr<Camera> camera = Camera::create(this, media->model(), streams);
> +	std::shared_ptr<Camera> camera = Camera::create(this, name, streams);
>  	registerCamera(std::move(camera), std::move(data));
>  
>  	/* Enable hot-unplug notifications. */