Message ID | 20230106235242.9536-1-laurent.pinchart@ideasonboard.com |
---|---|
State | Accepted |
Commit | 50459f103edfe47c9a599d766a850ef6014936c5 |
Headers | show |
Series | [v3] media: uvcvideo: Remove format descriptions | expand |
On Sat, Jan 07, 2023 at 01:52:42AM +0200, Laurent Pinchart wrote: >The V4L2 core overwrites format descriptions in v4l_fill_fmtdesc(), >there's no need to manually set the descriptions in the driver. This >prepares for removal of the format descriptions from the uvc_fmts table. > >Unlike V4L2, UVC makes a distinction between the SD-DV, SDL-DV and HD-DV >formats. It also indicates whether the DV format uses 50Hz or 60Hz. This >information is parsed by the driver to construct a format name string >that is printed in a debug message, but serves no other purpose as V4L2 >has a single V4L2_PIX_FMT_DV pixel format that covers all those cases. > >As the information is available in the UVC descriptors, and thus >accessible to users with lsusb if they really care, don't log it in a >debug message and drop the format name string to simplify the code. > >Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >Reviewed-by: Ricardo Ribalda <ribalda@chromium.org> >--- >Changes since v2: > >- Squash "media: uvcvideo: Remove format descriptions" and "media: > uvcvideo: Drop custom format names for DV formats" >- Don't replace %pUl with %p4cc when the format is unknown >- Print debug message even if fcc is 0 >--- > drivers/media/usb/uvc/uvc_driver.c | 18 ++---------------- > drivers/media/usb/uvc/uvc_v4l2.c | 2 -- > drivers/media/usb/uvc/uvcvideo.h | 2 -- > 3 files changed, 2 insertions(+), 20 deletions(-) > >diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c >index 57c948d47bbf..3318ec8ae7ef 100644 >--- a/drivers/media/usb/uvc/uvc_driver.c >+++ b/drivers/media/usb/uvc/uvc_driver.c >@@ -252,14 +252,10 @@ static int uvc_parse_format(struct uvc_device *dev, > fmtdesc = uvc_format_by_guid(&buffer[5]); > > if (fmtdesc != NULL) { >- strscpy(format->name, fmtdesc->name, >- sizeof(format->name)); > format->fcc = fmtdesc->fcc; > } else { > dev_info(&streaming->intf->dev, > "Unknown video format %pUl\n", &buffer[5]); >- snprintf(format->name, sizeof(format->name), "%pUl\n", >- &buffer[5]); > format->fcc = 0; > } > >@@ -271,8 +267,6 @@ static int uvc_parse_format(struct uvc_device *dev, > */ > if (dev->quirks & UVC_QUIRK_FORCE_Y8) { > if (format->fcc == V4L2_PIX_FMT_YUYV) { >- strscpy(format->name, "Greyscale 8-bit (Y8 )", >- sizeof(format->name)); > format->fcc = V4L2_PIX_FMT_GREY; > format->bpp = 8; > width_multiplier = 2; >@@ -313,7 +307,6 @@ static int uvc_parse_format(struct uvc_device *dev, > return -EINVAL; > } > >- strscpy(format->name, "MJPEG", sizeof(format->name)); > format->fcc = V4L2_PIX_FMT_MJPEG; > format->flags = UVC_FMT_FLAG_COMPRESSED; > format->bpp = 0; >@@ -331,14 +324,10 @@ static int uvc_parse_format(struct uvc_device *dev, > > switch (buffer[8] & 0x7f) { > case 0: >- strscpy(format->name, "SD-DV", sizeof(format->name)); >- break; > case 1: >- strscpy(format->name, "SDL-DV", sizeof(format->name)); >- break; > case 2: >- strscpy(format->name, "HD-DV", sizeof(format->name)); > break; >+ > default: Couldn't this become an simple if instead of this odd looking switch? if (buffer[8] & 0x7f) > 2) With that change you can add: Reviewed-by: Michael Grzeschik <m.grzeschik@pengutronix.de> > uvc_dbg(dev, DESCR, > "device %d videostreaming interface %d: unknown DV format %u\n", >@@ -347,9 +336,6 @@ static int uvc_parse_format(struct uvc_device *dev, > return -EINVAL; > } > >- strlcat(format->name, buffer[8] & (1 << 7) ? " 60Hz" : " 50Hz", >- sizeof(format->name)); >- > format->fcc = V4L2_PIX_FMT_DV; > format->flags = UVC_FMT_FLAG_COMPRESSED | UVC_FMT_FLAG_STREAM; > format->bpp = 0; >@@ -376,7 +362,7 @@ static int uvc_parse_format(struct uvc_device *dev, > return -EINVAL; > } > >- uvc_dbg(dev, DESCR, "Found format %s\n", format->name); >+ uvc_dbg(dev, DESCR, "Found format %p4cc", &format->fcc); > > buflen -= buffer[0]; > buffer += buffer[0]; >diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c >index 0f4a39324062..35453f81c1d9 100644 >--- a/drivers/media/usb/uvc/uvc_v4l2.c >+++ b/drivers/media/usb/uvc/uvc_v4l2.c >@@ -712,8 +712,6 @@ static int uvc_ioctl_enum_fmt(struct uvc_streaming *stream, > fmt->flags = 0; > if (format->flags & UVC_FMT_FLAG_COMPRESSED) > fmt->flags |= V4L2_FMT_FLAG_COMPRESSED; >- strscpy(fmt->description, format->name, sizeof(fmt->description)); >- fmt->description[sizeof(fmt->description) - 1] = 0; > fmt->pixelformat = format->fcc; > return 0; > } >diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h >index c5a1c1c9d49e..e85df8deb965 100644 >--- a/drivers/media/usb/uvc/uvcvideo.h >+++ b/drivers/media/usb/uvc/uvcvideo.h >@@ -263,8 +263,6 @@ struct uvc_format { > u32 fcc; > u32 flags; > >- char name[32]; >- > unsigned int nframes; > struct uvc_frame *frame; > }; > >base-commit: d3428667a95be621bfffe70e5bf2e607bbf7e049 >-- >Regards, > >Laurent Pinchart > >
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 57c948d47bbf..3318ec8ae7ef 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -252,14 +252,10 @@ static int uvc_parse_format(struct uvc_device *dev, fmtdesc = uvc_format_by_guid(&buffer[5]); if (fmtdesc != NULL) { - strscpy(format->name, fmtdesc->name, - sizeof(format->name)); format->fcc = fmtdesc->fcc; } else { dev_info(&streaming->intf->dev, "Unknown video format %pUl\n", &buffer[5]); - snprintf(format->name, sizeof(format->name), "%pUl\n", - &buffer[5]); format->fcc = 0; } @@ -271,8 +267,6 @@ static int uvc_parse_format(struct uvc_device *dev, */ if (dev->quirks & UVC_QUIRK_FORCE_Y8) { if (format->fcc == V4L2_PIX_FMT_YUYV) { - strscpy(format->name, "Greyscale 8-bit (Y8 )", - sizeof(format->name)); format->fcc = V4L2_PIX_FMT_GREY; format->bpp = 8; width_multiplier = 2; @@ -313,7 +307,6 @@ static int uvc_parse_format(struct uvc_device *dev, return -EINVAL; } - strscpy(format->name, "MJPEG", sizeof(format->name)); format->fcc = V4L2_PIX_FMT_MJPEG; format->flags = UVC_FMT_FLAG_COMPRESSED; format->bpp = 0; @@ -331,14 +324,10 @@ static int uvc_parse_format(struct uvc_device *dev, switch (buffer[8] & 0x7f) { case 0: - strscpy(format->name, "SD-DV", sizeof(format->name)); - break; case 1: - strscpy(format->name, "SDL-DV", sizeof(format->name)); - break; case 2: - strscpy(format->name, "HD-DV", sizeof(format->name)); break; + default: uvc_dbg(dev, DESCR, "device %d videostreaming interface %d: unknown DV format %u\n", @@ -347,9 +336,6 @@ static int uvc_parse_format(struct uvc_device *dev, return -EINVAL; } - strlcat(format->name, buffer[8] & (1 << 7) ? " 60Hz" : " 50Hz", - sizeof(format->name)); - format->fcc = V4L2_PIX_FMT_DV; format->flags = UVC_FMT_FLAG_COMPRESSED | UVC_FMT_FLAG_STREAM; format->bpp = 0; @@ -376,7 +362,7 @@ static int uvc_parse_format(struct uvc_device *dev, return -EINVAL; } - uvc_dbg(dev, DESCR, "Found format %s\n", format->name); + uvc_dbg(dev, DESCR, "Found format %p4cc", &format->fcc); buflen -= buffer[0]; buffer += buffer[0]; diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c index 0f4a39324062..35453f81c1d9 100644 --- a/drivers/media/usb/uvc/uvc_v4l2.c +++ b/drivers/media/usb/uvc/uvc_v4l2.c @@ -712,8 +712,6 @@ static int uvc_ioctl_enum_fmt(struct uvc_streaming *stream, fmt->flags = 0; if (format->flags & UVC_FMT_FLAG_COMPRESSED) fmt->flags |= V4L2_FMT_FLAG_COMPRESSED; - strscpy(fmt->description, format->name, sizeof(fmt->description)); - fmt->description[sizeof(fmt->description) - 1] = 0; fmt->pixelformat = format->fcc; return 0; } diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index c5a1c1c9d49e..e85df8deb965 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -263,8 +263,6 @@ struct uvc_format { u32 fcc; u32 flags; - char name[32]; - unsigned int nframes; struct uvc_frame *frame; };