Hi Hans On Thu, 17 Oct 2024 at 18:25, Hans Verkuil <hverkuil@xxxxxxxxx> wrote: > > The video device names (also used as entity names) are identical > for the video capture and metadata capture devices. > > This series ensures that the video device names are unique. > > It also fixes a bug where a metadata device would be created > for video output devices, but that's for video capture devices > only. You probably want to split that fix in a separate patch, but be aware that there might be userspace depending on the extra metadata device :( > > This fixes a compliance failure when running > 'v4l2-compliance -M /dev/mediaX'. > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx> > --- > RFC for now. > > Based on the code I assume one UVC device can have multiple > capture and/or output devices, so I added an instance index. I believe we tried something similar and we ended up reverting it :( https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f66dcb32af19faf49cc4a9222c3152b10c6ec84a Maybe, as you suggest, dropping the Cap prefix for the first inst is enough, if my memory is correct I think I also tried with that but we ended up abandoning the patch https://lore.kernel.org/linux-media/20220920-resend-meta-v5-0-3385df697370@xxxxxxxxxxxx/ https://lore.kernel.org/linux-media/Y6OP%2Fqz9R4BgXi4o@xxxxxxxxxxxxxxxxxxxxxxxxxx/ > > But I may be wrong about that. > > I also assume that there is no metadata for UVC video outputs. > > It might also be safer to drop the 'Cap0 ' prefix if inst == 0, > just in case someone relies on the name. > --- > drivers/media/usb/uvc/uvc_driver.c | 34 +++++++++++++++++++--------- > drivers/media/usb/uvc/uvc_metadata.c | 4 ++-- > drivers/media/usb/uvc/uvcvideo.h | 3 ++- > 3 files changed, 27 insertions(+), 14 deletions(-) > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > index ab9cdb50e74e..59e5769166f2 100644 > --- a/drivers/media/usb/uvc/uvc_driver.c > +++ b/drivers/media/usb/uvc/uvc_driver.c > @@ -1956,6 +1956,7 @@ static void uvc_unregister_video(struct uvc_device *dev) > > int uvc_register_video_device(struct uvc_device *dev, > struct uvc_streaming *stream, > + unsigned int inst, > struct video_device *vdev, > struct uvc_video_queue *queue, > enum v4l2_buf_type type, > @@ -1990,17 +1991,18 @@ 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; > + snprintf(vdev->name, sizeof(vdev->name), "Cap%u %s", inst, dev->name); > break; > case V4L2_BUF_TYPE_VIDEO_OUTPUT: > vdev->device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING; > + snprintf(vdev->name, sizeof(vdev->name), "Out%u %s", inst, dev->name); > break; > case V4L2_BUF_TYPE_META_CAPTURE: > vdev->device_caps = V4L2_CAP_META_CAPTURE | V4L2_CAP_STREAMING; > + snprintf(vdev->name, sizeof(vdev->name), "Meta%u %s", inst, dev->name); > break; > } > > - strscpy(vdev->name, dev->name, sizeof(vdev->name)); > - > /* > * Set the driver data before calling video_register_device, otherwise > * the file open() handler might race us. > @@ -2020,7 +2022,7 @@ int uvc_register_video_device(struct uvc_device *dev, > } > > static int uvc_register_video(struct uvc_device *dev, > - struct uvc_streaming *stream) > + struct uvc_streaming *stream, unsigned int inst) > { > int ret; > > @@ -2041,7 +2043,7 @@ static int uvc_register_video(struct uvc_device *dev, > uvc_debugfs_init_stream(stream); > > /* Register the device with V4L. */ > - return uvc_register_video_device(dev, stream, &stream->vdev, > + return uvc_register_video_device(dev, stream, inst, &stream->vdev, > &stream->queue, stream->type, > &uvc_fops, &uvc_ioctl_ops); > } > @@ -2054,9 +2056,13 @@ static int uvc_register_terms(struct uvc_device *dev, > { > struct uvc_streaming *stream; > struct uvc_entity *term; > + unsigned int inst_cap = 0; > + unsigned int inst_out = 0; > int ret; > > list_for_each_entry(term, &chain->entities, chain) { > + bool is_capture; > + > if (UVC_ENTITY_TYPE(term) != UVC_TT_STREAMING) > continue; > > @@ -2069,16 +2075,22 @@ static int uvc_register_terms(struct uvc_device *dev, > } > > stream->chain = chain; > - ret = uvc_register_video(dev, stream); > + is_capture = stream->type == V4L2_BUF_TYPE_VIDEO_CAPTURE; > + ret = uvc_register_video(dev, stream, > + is_capture ? inst_cap : inst_out); > if (ret < 0) > return ret; > > - /* > - * Register a metadata node, but ignore a possible failure, > - * complete registration of video nodes anyway. > - */ > - uvc_meta_register(stream); > - > + if (is_capture) { > + /* > + * Register a metadata node, but ignore a possible failure, > + * complete registration of video nodes anyway. > + */ > + uvc_meta_register(stream, inst_cap); > + inst_cap++; > + } else { > + inst_out++; > + } > term->vdev = &stream->vdev; > } > > diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c > index 82de7781f5b6..685182302602 100644 > --- a/drivers/media/usb/uvc/uvc_metadata.c > +++ b/drivers/media/usb/uvc/uvc_metadata.c > @@ -156,7 +156,7 @@ static const struct v4l2_file_operations uvc_meta_fops = { > .mmap = vb2_fop_mmap, > }; > > -int uvc_meta_register(struct uvc_streaming *stream) > +int uvc_meta_register(struct uvc_streaming *stream, unsigned int inst) > { > struct uvc_device *dev = stream->dev; > struct video_device *vdev = &stream->meta.vdev; > @@ -170,7 +170,7 @@ int uvc_meta_register(struct uvc_streaming *stream) > */ > vdev->queue = &queue->queue; > > - return uvc_register_video_device(dev, stream, vdev, queue, > + return uvc_register_video_device(dev, stream, inst, vdev, queue, > V4L2_BUF_TYPE_META_CAPTURE, > &uvc_meta_fops, &uvc_meta_ioctl_ops); > } > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h > index 07f9921d83f2..92c72ef6ea49 100644 > --- a/drivers/media/usb/uvc/uvcvideo.h > +++ b/drivers/media/usb/uvc/uvcvideo.h > @@ -738,10 +738,11 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit, > void uvc_video_clock_update(struct uvc_streaming *stream, > struct vb2_v4l2_buffer *vbuf, > struct uvc_buffer *buf); > -int uvc_meta_register(struct uvc_streaming *stream); > +int uvc_meta_register(struct uvc_streaming *stream, unsigned int inst); > > int uvc_register_video_device(struct uvc_device *dev, > struct uvc_streaming *stream, > + unsigned int inst, > struct video_device *vdev, > struct uvc_video_queue *queue, > enum v4l2_buf_type type, > -- > 2.43.0 > > -- Ricardo Ribalda
