Re: [PATCH 5/5] usb: uvc: use v4l2_fill_fmtdesc instead of open coded format name

Dan Scally <dan.scally@xxxxxxxxxxxxxxxx> · Wed, 14 Dec 2022 10:30:49 +0000

Hi all

On 14/12/2022 10:25, Laurent Pinchart wrote:
On Wed, Dec 14, 2022 at 11:20:20AM +0100, Ricardo Ribalda wrote:
On Wed, 14 Dec 2022 at 10:48, Dan Scally wrote:
On 12/12/2022 19:47, 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.

Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
---
The only thing that makes me wary about this one is that it will change
the format names reported by the uvcvideo driver to userspace, since
those returned by v4l2_fill_fmtdesc() are not the same as the ones being
dropped from uvc_format_desc[]...are we sure that's not going to matter?
I would expect apps using the fourcc, not the string to select one
format or the other.
I also believe it shouldn't be an issue. The format description is
likely used only for the purpose of displaying it directly to the user,
if used at all (we shouldn't have a format description in the kernel API
in the first place, but that's another issue). It should thus be fine.


OK - sounds like we're confident then; worries calmed :)


   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 a6787f1999becd..02de0dcad0f088 100644
--- a/drivers/media/common/uvc.c
+++ b/drivers/media/common/uvc.c
@@ -11,187 +11,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 {
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);

       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;
   };


--
Ricardo Ribalda






