Hi Laurent
For what it's worth, I am pro squash :)
On Wed, 4 Jan 2023 at 12:19, Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxxxxxxxx>
With or without my nits
Reviewed-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
> ---
> drivers/media/usb/uvc/uvc_driver.c | 18 +++---------------
> 1 file changed, 3 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index 9852d6f63ae8..ba41f13a2491 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -228,7 +228,6 @@ 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;
> @@ -325,14 +324,10 @@ static int uvc_parse_format(struct uvc_device *dev,
>
> switch (buffer[8] & 0x7f) {
> case 0:
> - strscpy(fmtname, "SD-DV", sizeof(fmtname));
> - break;
> case 1:
> - strscpy(fmtname, "SDL-DV", sizeof(fmtname));
> - break;
> case 2:
> - strscpy(fmtname, "HD-DV", sizeof(fmtname));
> break;
> +
> default:
> uvc_dbg(dev, DESCR,
> "device %d videostreaming interface %d: unknown DV format %u\n",
> @@ -341,9 +336,6 @@ static int uvc_parse_format(struct uvc_device *dev,
> return -EINVAL;
> }
>
> - 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;
> format->bpp = 0;
> @@ -370,12 +362,8 @@ static int uvc_parse_format(struct uvc_device *dev,
> return -EINVAL;
> }
>
> - 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);
> - }
Maybe it is the way that I debug the issues. I run an OK execution
with a FAIl one and then I compare results. I tend to prefer that the
extra lines are errors and there is no missing lines.... but I if your
prefer it this way, I am ok with it ;)
> + if (format->fcc)
> + uvc_dbg(dev, DESCR, "Found format %p4cc", &format->fcc);
>
> buflen -= buffer[0];
> buffer += buffer[0];
> --
> Regards,
>
> Laurent Pinchart
>
--
Ricardo Ribalda
