Hi Hans, Thank you for the patch. On Mon, Mar 22, 2021 at 04:46:58PM +0100, Hans Verkuil wrote: > Leave it to the V4L2 core to set the description. In fact, that was > already the case for a long time since v4l_fill_fmtdesc() overwrites > the description. > > So remove all description strings from the driver. > > uvc_ioctl_enum_fmt() was also cleaned up a bit since zeroed the > v4l2_fmtdesc struct, when all fields after 'type' are already zeroed > by the V4L2 core. I have written a very similar patch (which I think I've shared with Ricardo before). You can find it at https://git.linuxtv.org/pinchartl/media.git/commit/?h=uvc/dev&id=16a7d79d67cdd06a448d8c4c20e270d1c21828b1 It has two additions compared to yours: - Compacting the format array - Keeping the debug message in uvc_parse_format() (I'd like if the message could use a nicer format name than printing the raw 4CC) But it's missing the cleanup in uvc_ioctl_enum_fmt(). Would you have time to merge the two together and submit a v3 ? If so, we can get it merged in v5.13. > Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx> > --- > drivers/media/usb/uvc/uvc_driver.c | 57 ------------------------------ > drivers/media/usb/uvc/uvc_v4l2.c | 9 ----- > drivers/media/usb/uvc/uvcvideo.h | 3 -- > 3 files changed, 69 deletions(-) > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > index 7ecd26be6353..9bf066460699 100644 > --- a/drivers/media/usb/uvc/uvc_driver.c > +++ b/drivers/media/usb/uvc/uvc_driver.c > @@ -41,202 +41,162 @@ unsigned int uvc_timeout_param = UVC_CTRL_STREAMING_TIMEOUT; > > static struct uvc_format_desc uvc_fmts[] = { > { > - .name = "YUV 4:2:2 (YUYV)", > .guid = UVC_GUID_FORMAT_YUY2, > .fcc = V4L2_PIX_FMT_YUYV, > }, > { > - .name = "YUV 4:2:2 (YUYV)", > .guid = UVC_GUID_FORMAT_YUY2_ISIGHT, > .fcc = V4L2_PIX_FMT_YUYV, > }, > { > - .name = "YUV 4:2:0 (NV12)", > .guid = UVC_GUID_FORMAT_NV12, > .fcc = V4L2_PIX_FMT_NV12, > }, > { > - .name = "MJPEG", > .guid = UVC_GUID_FORMAT_MJPEG, > .fcc = V4L2_PIX_FMT_MJPEG, > }, > { > - .name = "YVU 4:2:0 (YV12)", > .guid = UVC_GUID_FORMAT_YV12, > .fcc = V4L2_PIX_FMT_YVU420, > }, > { > - .name = "YUV 4:2:0 (I420)", > .guid = UVC_GUID_FORMAT_I420, > .fcc = V4L2_PIX_FMT_YUV420, > }, > { > - .name = "YUV 4:2:0 (M420)", > .guid = UVC_GUID_FORMAT_M420, > .fcc = V4L2_PIX_FMT_M420, > }, > { > - .name = "YUV 4:2:2 (UYVY)", > .guid = UVC_GUID_FORMAT_UYVY, > .fcc = V4L2_PIX_FMT_UYVY, > }, > { > - .name = "Greyscale 8-bit (Y800)", > .guid = UVC_GUID_FORMAT_Y800, > .fcc = V4L2_PIX_FMT_GREY, > }, > { > - .name = "Greyscale 8-bit (Y8 )", > .guid = UVC_GUID_FORMAT_Y8, > .fcc = V4L2_PIX_FMT_GREY, > }, > { > - .name = "Greyscale 8-bit (D3DFMT_L8)", > .guid = UVC_GUID_FORMAT_D3DFMT_L8, > .fcc = V4L2_PIX_FMT_GREY, > }, > { > - .name = "IR 8-bit (L8_IR)", > .guid = UVC_GUID_FORMAT_KSMEDIA_L8_IR, > .fcc = V4L2_PIX_FMT_GREY, > }, > { > - .name = "Greyscale 10-bit (Y10 )", > .guid = UVC_GUID_FORMAT_Y10, > .fcc = V4L2_PIX_FMT_Y10, > }, > { > - .name = "Greyscale 12-bit (Y12 )", > .guid = UVC_GUID_FORMAT_Y12, > .fcc = V4L2_PIX_FMT_Y12, > }, > { > - .name = "Greyscale 16-bit (Y16 )", > .guid = UVC_GUID_FORMAT_Y16, > .fcc = V4L2_PIX_FMT_Y16, > }, > { > - .name = "BGGR Bayer (BY8 )", > .guid = UVC_GUID_FORMAT_BY8, > .fcc = V4L2_PIX_FMT_SBGGR8, > }, > { > - .name = "BGGR Bayer (BA81)", > .guid = UVC_GUID_FORMAT_BA81, > .fcc = V4L2_PIX_FMT_SBGGR8, > }, > { > - .name = "GBRG Bayer (GBRG)", > .guid = UVC_GUID_FORMAT_GBRG, > .fcc = V4L2_PIX_FMT_SGBRG8, > }, > { > - .name = "GRBG Bayer (GRBG)", > .guid = UVC_GUID_FORMAT_GRBG, > .fcc = V4L2_PIX_FMT_SGRBG8, > }, > { > - .name = "RGGB Bayer (RGGB)", > .guid = UVC_GUID_FORMAT_RGGB, > .fcc = V4L2_PIX_FMT_SRGGB8, > }, > { > - .name = "RGB565", > .guid = UVC_GUID_FORMAT_RGBP, > .fcc = V4L2_PIX_FMT_RGB565, > }, > { > - .name = "BGR 8:8:8 (BGR3)", > .guid = UVC_GUID_FORMAT_BGR3, > .fcc = V4L2_PIX_FMT_BGR24, > }, > { > - .name = "H.264", > .guid = UVC_GUID_FORMAT_H264, > .fcc = V4L2_PIX_FMT_H264, > }, > { > - .name = "Greyscale 8 L/R (Y8I)", > .guid = UVC_GUID_FORMAT_Y8I, > .fcc = V4L2_PIX_FMT_Y8I, > }, > { > - .name = "Greyscale 12 L/R (Y12I)", > .guid = UVC_GUID_FORMAT_Y12I, > .fcc = V4L2_PIX_FMT_Y12I, > }, > { > - .name = "Depth data 16-bit (Z16)", > .guid = UVC_GUID_FORMAT_Z16, > .fcc = V4L2_PIX_FMT_Z16, > }, > { > - .name = "Bayer 10-bit (SRGGB10P)", > .guid = UVC_GUID_FORMAT_RW10, > .fcc = V4L2_PIX_FMT_SRGGB10P, > }, > { > - .name = "Bayer 12-bit linear packed (SBGGR12LP)", > .guid = UVC_GUID_FORMAT_BGCP, > .fcc = V4L2_PIX_FMT_SBGGR12LP, > }, > { > - .name = "Bayer 12-bit linear packed (SGBRG12LP)", > .guid = UVC_GUID_FORMAT_GBCP, > .fcc = V4L2_PIX_FMT_SGBRG12LP, > }, > { > - .name = "Bayer 12-bit linear packed (SRGGB12LP)", > .guid = UVC_GUID_FORMAT_RGCP, > .fcc = V4L2_PIX_FMT_SRGGB12LP, > }, > { > - .name = "Bayer 12-bit linear packed (SGRBG12LP)", > .guid = UVC_GUID_FORMAT_GRCP, > .fcc = V4L2_PIX_FMT_SGRBG12LP, > }, > { > - .name = "Bayer 16-bit (SBGGR16)", > .guid = UVC_GUID_FORMAT_BG16, > .fcc = V4L2_PIX_FMT_SBGGR16, > }, > { > - .name = "Bayer 16-bit (SGBRG16)", > .guid = UVC_GUID_FORMAT_GB16, > .fcc = V4L2_PIX_FMT_SGBRG16, > }, > { > - .name = "Bayer 16-bit (SRGGB16)", > .guid = UVC_GUID_FORMAT_RG16, > .fcc = V4L2_PIX_FMT_SRGGB16, > }, > { > - .name = "Bayer 16-bit (SGRBG16)", > .guid = UVC_GUID_FORMAT_GR16, > .fcc = V4L2_PIX_FMT_SGRBG16, > }, > { > - .name = "Depth data 16-bit (Z16)", > .guid = UVC_GUID_FORMAT_INVZ, > .fcc = V4L2_PIX_FMT_Z16, > }, > { > - .name = "Greyscale 10-bit (Y10 )", > .guid = UVC_GUID_FORMAT_INVI, > .fcc = V4L2_PIX_FMT_Y10, > }, > { > - .name = "IR:Depth 26-bit (INZI)", > .guid = UVC_GUID_FORMAT_INZI, > .fcc = V4L2_PIX_FMT_INZI, > }, > { > - .name = "4-bit Depth Confidence (Packed)", > .guid = UVC_GUID_FORMAT_CNF4, > .fcc = V4L2_PIX_FMT_CNF4, > }, > { > - .name = "HEVC", > .guid = UVC_GUID_FORMAT_HEVC, > .fcc = V4L2_PIX_FMT_HEVC, > }, > @@ -551,14 +511,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; > } > > @@ -569,8 +525,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; > @@ -611,7 +565,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; > @@ -629,13 +582,8 @@ 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, > @@ -645,9 +593,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; > @@ -674,8 +619,6 @@ static int uvc_parse_format(struct uvc_device *dev, > return -EINVAL; > } > > - uvc_dbg(dev, DESCR, "Found format %s\n", format->name); > - > 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 252136cc885c..1cfd081c2004 100644 > --- a/drivers/media/usb/uvc/uvc_v4l2.c > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > @@ -632,22 +632,13 @@ static int uvc_ioctl_enum_fmt(struct uvc_streaming *stream, > struct v4l2_fmtdesc *fmt) > { > struct uvc_format *format; > - enum v4l2_buf_type type = fmt->type; > - u32 index = fmt->index; > > if (fmt->type != stream->type || fmt->index >= stream->nformats) > return -EINVAL; > > - memset(fmt, 0, sizeof(*fmt)); > - fmt->index = index; > - fmt->type = type; > - > format = &stream->format[fmt->index]; > - 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 51cda67946d5..8fff8b93def2 100644 > --- a/drivers/media/usb/uvc/uvcvideo.h > +++ b/drivers/media/usb/uvc/uvcvideo.h > @@ -292,7 +292,6 @@ struct uvc_control { > }; > > struct uvc_format_desc { > - char *name; > u8 guid[16]; > u32 fcc; > }; > @@ -416,8 +415,6 @@ struct uvc_format { > u32 fcc; > u32 flags; > > - char name[32]; > - > unsigned int nframes; > struct uvc_frame *frame; > }; -- Regards, Laurent Pinchart