On 17/10/2024 16:48, Ricardo Ribalda wrote: > 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. Right, the suggestion was to try to just fix the entity name, so I'll see if I can figure out how to do this. Regards, Hans >> --- >> 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 >> >> > >
