[v2,1/2] media: uvcvideo: Remove format descriptions

Message ID 20230104111944.962-2-laurent.pinchart@ideasonboard.com (mailing list archive)
State Accepted
Delegated to: Laurent Pinchart
Headers
Series media: uvcvideo: Drop format descriptions |

Commit Message

Laurent Pinchart Jan. 4, 2023, 11:19 a.m. UTC
  The V4L2 core fills format description on its own 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.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v1:

- Don't replace %pUl with %p4cc when the format is unknown
---
 drivers/media/usb/uvc/uvc_driver.c | 25 ++++++++++++-------------
 drivers/media/usb/uvc/uvc_v4l2.c   |  2 --
 drivers/media/usb/uvc/uvcvideo.h   |  2 --
 3 files changed, 12 insertions(+), 17 deletions(-)
  

Comments

Ricardo Ribalda Jan. 4, 2023, 11:48 a.m. UTC | #1
Hi Laurent

On Wed, 4 Jan 2023 at 12:19, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> The V4L2 core fills format description on its own in v4l_fill_fmtdesc(),
can we s/fills/overwrites/ ?
> 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.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
With or without my nits:
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> Changes since v1:
>
> - Don't replace %pUl with %p4cc when the format is unknown
> ---
>  drivers/media/usb/uvc/uvc_driver.c | 25 ++++++++++++-------------
>  drivers/media/usb/uvc/uvc_v4l2.c   |  2 --
>  drivers/media/usb/uvc/uvcvideo.h   |  2 --
>  3 files changed, 12 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index 72c025d8e20b..9852d6f63ae8 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -228,6 +228,7 @@ static int uvc_parse_format(struct uvc_device *dev,
>         struct uvc_format_desc *fmtdesc;
>         struct uvc_frame *frame;
>         const unsigned char *start = buffer;
> +       char fmtname[12] = { 0, };
>         unsigned int width_multiplier = 1;
>         unsigned int interval;
>         unsigned int i, n;
> @@ -252,14 +253,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 +268,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 +308,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,13 +325,13 @@ static int uvc_parse_format(struct uvc_device *dev,
>
>                 switch (buffer[8] & 0x7f) {
>                 case 0:
> -                       strscpy(format->name, "SD-DV", sizeof(format->name));
> +                       strscpy(fmtname, "SD-DV", sizeof(fmtname));
>                         break;
>                 case 1:
> -                       strscpy(format->name, "SDL-DV", sizeof(format->name));
> +                       strscpy(fmtname, "SDL-DV", sizeof(fmtname));
>                         break;
>                 case 2:
> -                       strscpy(format->name, "HD-DV", sizeof(format->name));
> +                       strscpy(fmtname, "HD-DV", sizeof(fmtname));
>                         break;
>                 default:
>                         uvc_dbg(dev, DESCR,
> @@ -347,8 +341,8 @@ static int uvc_parse_format(struct uvc_device *dev,
>                         return -EINVAL;
>                 }
>
> -               strlcat(format->name, buffer[8] & (1 << 7) ? " 60Hz" : " 50Hz",
> -                       sizeof(format->name));
> +               strlcat(fmtname, buffer[8] & (1 << 7) ? " 60Hz" : " 50Hz",
> +                       sizeof(fmtname));
>
>                 format->fcc = V4L2_PIX_FMT_DV;
>                 format->flags = UVC_FMT_FLAG_COMPRESSED | UVC_FMT_FLAG_STREAM;
> @@ -376,7 +370,12 @@ static int uvc_parse_format(struct uvc_device *dev,
>                 return -EINVAL;
>         }
>
> -       uvc_dbg(dev, DESCR, "Found format %s\n", format->name);
> +       if (format->fcc) {
> +               if (fmtname[0])
> +                       uvc_dbg(dev, DESCR, "Found format %s\n", fmtname);
> +               else
> +                       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 ed2525e7e2a5..727fafb47c1f 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -661,8 +661,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 ae0066eceffd..cb504b9d7ec9 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -262,8 +262,6 @@ struct uvc_format {
>         u32 fcc;
>         u32 flags;
>
> -       char name[32];
> -
>         unsigned int nframes;
>         struct uvc_frame *frame;
>  };
> --
> Regards,
>
> Laurent Pinchart
>
  

Patch

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 72c025d8e20b..9852d6f63ae8 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -228,6 +228,7 @@  static int uvc_parse_format(struct uvc_device *dev,
 	struct uvc_format_desc *fmtdesc;
 	struct uvc_frame *frame;
 	const unsigned char *start = buffer;
+	char fmtname[12] = { 0, };
 	unsigned int width_multiplier = 1;
 	unsigned int interval;
 	unsigned int i, n;
@@ -252,14 +253,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 +268,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 +308,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,13 +325,13 @@  static int uvc_parse_format(struct uvc_device *dev,
 
 		switch (buffer[8] & 0x7f) {
 		case 0:
-			strscpy(format->name, "SD-DV", sizeof(format->name));
+			strscpy(fmtname, "SD-DV", sizeof(fmtname));
 			break;
 		case 1:
-			strscpy(format->name, "SDL-DV", sizeof(format->name));
+			strscpy(fmtname, "SDL-DV", sizeof(fmtname));
 			break;
 		case 2:
-			strscpy(format->name, "HD-DV", sizeof(format->name));
+			strscpy(fmtname, "HD-DV", sizeof(fmtname));
 			break;
 		default:
 			uvc_dbg(dev, DESCR,
@@ -347,8 +341,8 @@  static int uvc_parse_format(struct uvc_device *dev,
 			return -EINVAL;
 		}
 
-		strlcat(format->name, buffer[8] & (1 << 7) ? " 60Hz" : " 50Hz",
-			sizeof(format->name));
+		strlcat(fmtname, buffer[8] & (1 << 7) ? " 60Hz" : " 50Hz",
+			sizeof(fmtname));
 
 		format->fcc = V4L2_PIX_FMT_DV;
 		format->flags = UVC_FMT_FLAG_COMPRESSED | UVC_FMT_FLAG_STREAM;
@@ -376,7 +370,12 @@  static int uvc_parse_format(struct uvc_device *dev,
 		return -EINVAL;
 	}
 
-	uvc_dbg(dev, DESCR, "Found format %s\n", format->name);
+	if (format->fcc) {
+		if (fmtname[0])
+			uvc_dbg(dev, DESCR, "Found format %s\n", fmtname);
+		else
+			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 ed2525e7e2a5..727fafb47c1f 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -661,8 +661,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 ae0066eceffd..cb504b9d7ec9 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -262,8 +262,6 @@  struct uvc_format {
 	u32 fcc;
 	u32 flags;
 
-	char name[32];
-
 	unsigned int nframes;
 	struct uvc_frame *frame;
 };