Hi Laurent, Thank you for the review. On Wed, Dec 28, 2022 at 03:50:47AM +0200, Laurent Pinchart wrote:
On Thu, Dec 15, 2022 at 11:45:14PM +0100, Michael Grzeschik wrote:Since we have the helper function v4l2_fill_fmtdesc, we can use this to get the corresponding descriptive string for the pixelformat and set the compressed flag. This patch is removing the redundant name field in uvc_format_desc and makes use of v4l2_fill_fmtdesc instead.I really like that.Reviewed-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx> Tested-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx> Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx> --- v1 -> v2: - added reviewed and tested tags --- drivers/media/common/uvc.c | 37 -------------------------- drivers/media/usb/uvc/uvc_driver.c | 8 +++++- drivers/usb/gadget/function/uvc_v4l2.c | 6 +---- include/linux/usb/uvc.h | 1 - 4 files changed, 8 insertions(+), 44 deletions(-) diff --git a/drivers/media/common/uvc.c b/drivers/media/common/uvc.c index a9d587490de8d5..ab2637b9b39b2a 100644 --- a/drivers/media/common/uvc.c +++ b/drivers/media/common/uvc.c @@ -13,187 +13,150 @@ static const 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 = "H.265", .guid = UVC_GUID_FORMAT_H265, .fcc = V4L2_PIX_FMT_HEVC, }, { - .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 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, }, diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 12b6ad0966d94a..af92e730bde7c7 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -251,7 +251,13 @@ static int uvc_parse_format(struct uvc_device *dev, fmtdesc = uvc_format_by_guid(&buffer[5]); if (fmtdesc != NULL) { - strscpy(format->name, fmtdesc->name, + struct v4l2_fmtdesc fmt; + + fmt.pixelformat = fmtdesc->fcc; + + v4l2_fill_fmtdesc(&fmt); + + strscpy(format->name, fmt.description, sizeof(format->name)); format->fcc = fmtdesc->fcc; } else {I've just sent "[PATCH v1] media: uvcvideo: Remove format descriptions" which drops usage of the name in the uvcvideo driver without having to call v4l2_fill_fmtdesc(). With that merged, changes to uvc_driver.c can be dropped in this patch.
Right.
diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c index 21e573e628f4e7..6e46fa1695f212 100644 --- a/drivers/usb/gadget/function/uvc_v4l2.c +++ b/drivers/usb/gadget/function/uvc_v4l2.c @@ -374,14 +374,10 @@ uvc_v4l2_enum_format(struct file *file, void *fh, struct v4l2_fmtdesc *f) if (!uformat) return -EINVAL; - if (uformat->type != UVCG_UNCOMPRESSED) - f->flags |= V4L2_FMT_FLAG_COMPRESSED; - fmtdesc = to_uvc_format(uformat); f->pixelformat = fmtdesc->fcc; - strscpy(f->description, fmtdesc->name, sizeof(f->description)); - f->description[strlen(fmtdesc->name) - 1] = 0; + v4l2_fill_fmtdesc(f);v4l_fill_fmtdesc() is actually called by v4l_enum_fmt() after calling the driver's .vidioc_enum_fmt_vid_out() operation, so you don't have to call it manually here. By dropping the manual calls to v4l_fill_fmtdesc(), you can also drop patch 4/5 in the series.
This is a good point. I have dropped patch 4/5 in the stack.
This creates a dependency between the patch I've just sent and this series. As I don't want to introduce any further delay, I'll create a stable branch based on v6.2-rc1 as soon as my patch gets reviewed, you you can then base the next version of this series on top of it. An alternative would be to merge this series through the media tree if Greg is OK with that.
I found your v3 patch. I sure can rebase my work on your patch if your stable branch is available.
return 0; } diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h index 227a03f252a5c0..e407a7b8a91c70 100644 --- a/include/linux/usb/uvc.h +++ b/include/linux/usb/uvc.h @@ -146,7 +146,6 @@ 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71} struct uvc_format_desc { - char *name; u8 guid[16]; u32 fcc; };-- Regards, Laurent Pinchart
-- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Attachment:
signature.asc
Description: PGP signature
