Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220920-resend-meta-v4-0-3ac355b66723@chromium.org>
 <20220920-resend-meta-v4-3-3ac355b66723@chromium.org> <Y45tnp0naosOrYCY@pendragon.ideasonboard.com>
In-Reply-To: <Y45tnp0naosOrYCY@pendragon.ideasonboard.com>
From:   Ricardo Ribalda <ribalda@chromium.org>
Date:   Tue, 6 Dec 2022 00:02:22 +0100
Message-ID: <CANiDSCtRoVQ2+asPmOacarvC2VrJYTbU67+wKJq1ciuMrwguPg@mail.gmail.com>
Subject: Re: [PATCH v4 3/3] media: uvcvideo: Add a unique suffix to camera names
To:     Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc:     Yunke Cao <yunkec@chromium.org>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Sergey Senozhatsky <senozhatsky@chromium.org>,
        linux-media@vger.kernel.org,
        Mauro Carvalho Chehab <mchehab+huawei@kernel.org>,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Laurent

On Mon, 5 Dec 2022 at 23:16, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Ricardo,
>
> Thank you for the patch.
>
> On Fri, Dec 02, 2022 at 06:08:19PM +0100, Ricardo Ribalda wrote:
> > Some cameras have multiple data inputs (i.e. IR sensor and RGB sensor),
> > append a unique number to the device name.
> >
> > 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>
> > ---
> >  drivers/media/usb/uvc/uvc_driver.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > index 215fb483efb0..f4032ebb3689 100644
> > --- a/drivers/media/usb/uvc/uvc_driver.c
> > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > @@ -1963,7 +1963,8 @@ int uvc_register_video_device(struct uvc_device *dev,
> >               break;
> >       }
> >
> > -     strscpy(vdev->name, dev->name, sizeof(vdev->name));
> > +     snprintf(vdev->name, sizeof(vdev->name), "%s %u", dev->name,
> > +              stream->header.bTerminalLink);
>
> This won't be perfect as the string is not guaranteed to fit in
> vdev->name, but I suppose it will help as a quick fix for some devices.
> How about the other devices ? Won't they still exhibit the above
> v4l2-compliance failure ? Isn't that something that will still affect
> Chrome OS devices ?

We could place the id first... but that will look bad: Eg:

1- My favorite camera

Another option is to remove the last chars to fit the id. Eg:

My favorite came-1

If you prefer any of those options or have a better idea I can implement that.


>
> The change should not cause any regression as big as in patch 1/3.
> However, unless I'm mistaken users will notice a device name change,
> especially when selecting a device in their web browser. Could that be a
> problem ?

I think the only side effect is that the first time that the kernel
changes the naming convention, if there are more than one camera on
the system, the video conference might pick a different camera.
The good news is that the user will be presented with cameras with
different names. Now some cameras show very confusing names:

ribalda@alco:~/work/linux$ for a in /dev/video* ; do yavta -l $a| grep
"Dell Webcam"; done
Device `Dell Webcam WB7022' on `usb-0000:2d:00.0-1.2.3.1' (driver
'uvcvideo') supports video, capture, without mplanes.
Device `Dell Webcam WB7022' on `usb-0000:2d:00.0-1.2.3.1' (driver
'uvcvideo') supports meta-data, capture, without mplanes.
Device `Dell Webcam WB7022' on `usb-0000:2d:00.0-1.2.3.1' (driver
'uvcvideo') supports video, capture, without mplanes.
Device `Dell Webcam WB7022' on `usb-0000:2d:00.0-1.2.3.1' (driver
'uvcvideo') supports meta-data, capture, without mplanes.


>
> >       /*
> >        * Set the driver data before calling video_register_device, otherwise
> >
>
> --
> Regards,
>
> Laurent Pinchart


-- 
Ricardo Ribalda