[v10,11/21] media: uvcvideo: Set unique vdev name based in type

  All the entities must have a unique name. We can have a descriptive and
unique name by appending the function and the entity->id.

This is even resilent to multi chain devices.

Fixes v4l2-compliance:
Media Controller ioctls:
                fail: v4l2-test-media.cpp(205): v2_entity_names_set.find(key) != v2_entity_names_set.end()
        test MEDIA_IOC_G_TOPOLOGY: FAIL
                fail: v4l2-test-media.cpp(394): num_data_links != num_links
	test MEDIA_IOC_ENUM_ENTITIES/LINKS: FAIL

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/usb/uvc/uvc_driver.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
  

Hi Ricard, 

Le vendredi 18 juin 2021 à 14:29 +0200, Ricardo Ribalda a écrit :
> All the entities must have a unique name. We can have a descriptive and
> unique name by appending the function and the entity->id.

Thanks for your work. The only issue is that unfortunately this change cause an
important regression for users. All UVC cameras in all UIs seems to no longer
include any information about the camera. As an example, I have two cameras on
my system and Firefox, Chrome, Cheese, Zoom and MS Team all agree that my camera
are now:

  Video Capture 4
  Video Capture 5

Previously they would be shown as something like:

  StreamCam
  Integrated

We should probably revert this change quickly before it get deployed more
widely, I have notice the backport being sent for 5.4, 5.10 and 5.14. I'm using
5.15 shipped by Fedora team.

As a proper solution, maybe I could suggest to keep using dev->name, but trim it
enough to fit the " N" string to guaranty that you have enough space in this
limited 32 char string and use that instead ? This should fit the uniqueness
requirement without the sacrifice of the only possibly useful information we had
in that limited string.

regards,
Nicolas

> 
> This is even resilent to multi chain devices.
> 
> Fixes v4l2-compliance:
> Media Controller ioctls:
>                 fail: v4l2-test-media.cpp(205): v2_entity_names_set.find(key) != v2_entity_names_set.end()
>         test MEDIA_IOC_G_TOPOLOGY: FAIL
>                 fail: v4l2-test-media.cpp(394): num_data_links != num_links
> 	test MEDIA_IOC_ENUM_ENTITIES/LINKS: FAIL
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  drivers/media/usb/uvc/uvc_driver.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index 14b60792ffab..037bf80d1100 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -2194,6 +2194,7 @@ int uvc_register_video_device(struct uvc_device *dev,
>  			      const struct v4l2_file_operations *fops,
>  			      const struct v4l2_ioctl_ops *ioctl_ops)
>  {
> +	const char *name;
>  	int ret;
>  
>  	/* Initialize the video buffers queue. */
> @@ -2222,16 +2223,20 @@ int uvc_register_video_device(struct uvc_device *dev,
>  	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
>  	default:
>  		vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
> +		name = "Video Capture";
>  		break;
>  	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
>  		vdev->device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING;
> +		name = "Video Output";
>  		break;
>  	case V4L2_BUF_TYPE_META_CAPTURE:
>  		vdev->device_caps = V4L2_CAP_META_CAPTURE | V4L2_CAP_STREAMING;
> +		name = "Metadata";
>  		break;
>  	}
>  
> -	strscpy(vdev->name, dev->name, sizeof(vdev->name));
> +	snprintf(vdev->name, sizeof(vdev->name), "%s %u", name,
> +		 stream->header.bTerminalLink);
>  
>  	/*
>  	 * Set the driver data before calling video_register_device, otherwise
  

Hi Nicolas,

On Mon, Dec 06, 2021 at 02:05:06PM -0500, Nicolas Dufresne wrote:
> Le vendredi 18 juin 2021 à 14:29 +0200, Ricardo Ribalda a écrit :
> > All the entities must have a unique name. We can have a descriptive and
> > unique name by appending the function and the entity->id.
> 
> Thanks for your work. The only issue is that unfortunately this change cause an
> important regression for users. All UVC cameras in all UIs seems to no longer
> include any information about the camera. As an example, I have two cameras on
> my system and Firefox, Chrome, Cheese, Zoom and MS Team all agree that my camera
> are now:
> 
>   Video Capture 4
>   Video Capture 5
> 
> Previously they would be shown as something like:
> 
>   StreamCam
>   Integrated
> 
> We should probably revert this change quickly before it get deployed more
> widely, I have notice the backport being sent for 5.4, 5.10 and 5.14. I'm using
> 5.15 shipped by Fedora team.

Ack.

> As a proper solution, maybe I could suggest to keep using dev->name, but trim it
> enough to fit the " N" string to guaranty that you have enough space in this
> limited 32 char string and use that instead ? This should fit the uniqueness
> requirement without the sacrifice of the only possibly useful information we had
> in that limited string.

That would polute the device name a bit, which isn't very nice for
users. I wonder if we could instead decouple the entity name from the
video device name.

> > This is even resilent to multi chain devices.
> > 
> > Fixes v4l2-compliance:
> > Media Controller ioctls:
> >                 fail: v4l2-test-media.cpp(205): v2_entity_names_set.find(key) != v2_entity_names_set.end()
> >         test MEDIA_IOC_G_TOPOLOGY: FAIL
> >                 fail: v4l2-test-media.cpp(394): num_data_links != num_links
> > 	test MEDIA_IOC_ENUM_ENTITIES/LINKS: FAIL
> > 
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> > ---
> >  drivers/media/usb/uvc/uvc_driver.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > index 14b60792ffab..037bf80d1100 100644
> > --- a/drivers/media/usb/uvc/uvc_driver.c
> > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > @@ -2194,6 +2194,7 @@ int uvc_register_video_device(struct uvc_device *dev,
> >  			      const struct v4l2_file_operations *fops,
> >  			      const struct v4l2_ioctl_ops *ioctl_ops)
> >  {
> > +	const char *name;
> >  	int ret;
> >  
> >  	/* Initialize the video buffers queue. */
> > @@ -2222,16 +2223,20 @@ int uvc_register_video_device(struct uvc_device *dev,
> >  	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
> >  	default:
> >  		vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
> > +		name = "Video Capture";
> >  		break;
> >  	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
> >  		vdev->device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING;
> > +		name = "Video Output";
> >  		break;
> >  	case V4L2_BUF_TYPE_META_CAPTURE:
> >  		vdev->device_caps = V4L2_CAP_META_CAPTURE | V4L2_CAP_STREAMING;
> > +		name = "Metadata";
> >  		break;
> >  	}
> >  
> > -	strscpy(vdev->name, dev->name, sizeof(vdev->name));
> > +	snprintf(vdev->name, sizeof(vdev->name), "%s %u", name,
> > +		 stream->header.bTerminalLink);
> >  
> >  	/*
> >  	 * Set the driver data before calling video_register_device, otherwise