lkml.org 
[lkml]   [2022]   [Dec]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH v4 1/3] media: v4l2-dev.c: Add Meta: to the name of metadata devices
Hi Mauro,

On Wed, Dec 21, 2022 at 10:16:01PM +0000, Mauro Carvalho Chehab wrote:
> Em Fri, 02 Dec 2022 18:08:17 +0100 Ricardo Ribalda escreveu:
>
> > Devices with Metadata output (like uvc), create two video devices, one
> > for the data itself and another one for the metadata.
> >
> > Add a "Meta: " to the beginning of the device name, as suggested by Mauro,
> > to avoid having multiple devices with the same name.
>
> Hmm... I don't remember when I suggested it, but I suspect that such
> change could cause regressions on userspace.
>
> I double-checked: the two apps I know that have code to detect meta
> devices (Camorama and ZBar) won't be affected by name changes, as they
> always open v4l2 devnodes before start seeing them as capture devices,
> and run VIDIOC_QUERYCAP to check for V4L2_CAP_VIDEO_CAPTURE:
>
> if (first_device < 0) {
> fd = open(file, O_RDWR);
> if (fd < 0) {
> devices[n_devices].is_valid = FALSE;
> } else {
> if (ioctl(fd, VIDIOC_QUERYCAP, &vid_cap) == -1) {
> devices[n_devices].is_valid = FALSE;
> } else if (!(vid_cap.device_caps & V4L2_CAP_VIDEO_CAPTURE)) {
> devices[n_devices].is_valid = FALSE;
> } else {
> n_valid_devices++;
> devices[n_devices].is_valid = TRUE;
> }
> }
>
> close(fd);
> } else {
> devices[n_devices].is_valid = devices[first_device].is_valid;
> }
>
> You need to double-check if this won't cause userspace regressions, specially
> on browsers, as they have more complex logic, and seem to have already some
> logic to detect and remove meta files.

As commented separately in the same mail thread, this change would break
libcamera. For drivers that have well-known names for entities, we look
up some entities by name.

For the uvcvideo driver, which is affected by the v4l2-compliance
failure, a prefix (or suffix, or anything else) would be fine, but we
can't do that unconditionally for all 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
> >
> > Suggested-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> > drivers/media/v4l2-core/v4l2-dev.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> > index 397d553177fa..5c2c9ebb6b96 100644
> > --- a/drivers/media/v4l2-core/v4l2-dev.c
> > +++ b/drivers/media/v4l2-core/v4l2-dev.c
> > @@ -901,6 +901,15 @@ int __video_register_device(struct video_device *vdev,
> > if (WARN_ON(type != VFL_TYPE_SUBDEV && !vdev->device_caps))
> > return -EINVAL;
> >
> > + /* Add Meta: to metadata device names */
> > + if (vdev->device_caps &
> > + (V4L2_CAP_META_CAPTURE | V4L2_CAP_META_OUTPUT)) {
> > + char aux[sizeof(vdev->name)];
> > +
> > + snprintf(aux, sizeof(aux), "Meta: %s", vdev->name);
> > + strscpy(vdev->name, aux, sizeof(aux));
> > + }
> > +
> > /* v4l2_fh support */
> > spin_lock_init(&vdev->fh_lock);
> > INIT_LIST_HEAD(&vdev->fh_list);

--
Regards,

Laurent Pinchart

\
 
 \ /
  Last update: 2023-03-26 23:17    [W:0.087 / U:0.724 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site