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

[Laurent Pinchart]

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 at ragnatech.se>
> Reviewed-by: Jacopo Mondi <jacopo at 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. */

-- 
Regards,

Laurent Pinchart