All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] media: uvcvideo: Remove format descriptions
@ 2023-01-06 23:52 Laurent Pinchart
  2023-01-09 21:41 ` Michael Grzeschik
  0 siblings, 1 reply; 2+ messages in thread
From: Laurent Pinchart @ 2023-01-06 23:52 UTC (permalink / raw)
  To: linux-media
  Cc: Ricardo Ribalda, Kieran Bingham, Daniel Scally, Michael Grzeschik

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:
 			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


^ permalink raw reply related	[flat|nested] 2+ messages in thread
* Re: [PATCH v3] media: uvcvideo: Remove format descriptions
  2023-01-06 23:52 [PATCH v3] media: uvcvideo: Remove format descriptions Laurent Pinchart
@ 2023-01-09 21:41 ` Michael Grzeschik
  0 siblings, 0 replies; 2+ messages in thread
From: Michael Grzeschik @ 2023-01-09 21:41 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-media, Ricardo Ribalda, Kieran Bingham, Daniel Scally

[-- Attachment #1: Type: text/plain, Size: 5450 bytes --]

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

-- 
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-01-09 21:42 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-06 23:52 [PATCH v3] media: uvcvideo: Remove format descriptions Laurent Pinchart
2023-01-09 21:41 ` Michael Grzeschik

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.