| Message ID | 4b1bc0d5-808b-816d-d7de-5baa8851e74f@xs4all.nl (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Laurent Pinchart |
| Headers |
Received: from vger.kernel.org ([23.128.96.18])
by www.linuxtv.org with esmtp (Exim 4.92)
(envelope-from <linux-media-owner@vger.kernel.org>)
id 1phUvJ-0098kN-Bh; Wed, 29 Mar 2023 12:29:05 +0000
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
id S229827AbjC2M3D (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other);
Wed, 29 Mar 2023 08:29:03 -0400
Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40284 "EHLO
lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
with ESMTP id S229807AbjC2M3C (ORCPT
<rfc822;linux-media@vger.kernel.org>);
Wed, 29 Mar 2023 08:29:02 -0400
Received: from ams.source.kernel.org (ams.source.kernel.org
[IPv6:2604:1380:4601:e00::1])
by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 06BEE4229
for <linux-media@vger.kernel.org>;
Wed, 29 Mar 2023 05:28:58 -0700 (PDT)
Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(No client certificate requested)
by ams.source.kernel.org (Postfix) with ESMTPS id 9DC80B822EF
for <linux-media@vger.kernel.org>;
Wed, 29 Mar 2023 12:28:56 +0000 (UTC)
Received: by smtp.kernel.org (Postfix) with ESMTPSA id 786CAC433D2;
Wed, 29 Mar 2023 12:28:54 +0000 (UTC)
Message-ID: <4b1bc0d5-808b-816d-d7de-5baa8851e74f@xs4all.nl>
Date: Wed, 29 Mar 2023 14:28:52 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
Thunderbird/102.8.0
Content-Language: en-US
To: Linux Media Mailing List <linux-media@vger.kernel.org>
Cc: regressions@lists.linux.dev,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Hans de Goede <hdegoede@redhat.com>,
"Linux regression tracking (Thorsten Leemhuis)"
<regressions@leemhuis.info>
From: Hans Verkuil <hverkuil@xs4all.nl>
Subject: [PATCH] media: usb: uvc: fill in description for unknown pixelformats
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
X-Spam-Status: No,
score=-2.0 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS,
RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_PASS autolearn=unavailable
autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-media.vger.kernel.org>
X-Mailing-List: linux-media@vger.kernel.org
X-LSpam-Score: -2.4 (--)
X-LSpam-Report: No,
score=-2.4 required=5.0 tests=BAYES_00=-1.9,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1
autolearn=ham autolearn_force=no
|
| Series |
media: usb: uvc: fill in description for unknown pixelformats
|
|
Commit Message
Hans Verkuil
March 29, 2023, 12:28 p.m. UTC
If the fcc is 0 (indicating an unknown GUID format), then fill in the
description field in ENUM_FMT. Otherwise the V4L2 core will WARN.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Fixes: 50459f103edf ("media: uvcvideo: Remove format descriptions")
---
Comments
Hi Hans Thanks for the patch. I believe the user can fetch this info from lsusb, so this is kind of duplicated info, and this is why it was removed. Is there an app that uses this unknown format code ? Or the only complaint is that WARN() is too loud for the user? Regards! On Wed, 29 Mar 2023 at 14:39, Hans Verkuil <hverkuil@xs4all.nl> wrote: > > If the fcc is 0 (indicating an unknown GUID format), then fill in the > description field in ENUM_FMT. Otherwise the V4L2 core will WARN. > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > Fixes: 50459f103edf ("media: uvcvideo: Remove format descriptions") > --- > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > index 7aefa76a42b3..2f1ced1212cd 100644 > --- a/drivers/media/usb/uvc/uvc_driver.c > +++ b/drivers/media/usb/uvc/uvc_driver.c > @@ -256,6 +256,9 @@ static int uvc_parse_format(struct uvc_device *dev, > } else { > dev_info(&streaming->intf->dev, > "Unknown video format %pUl\n", &buffer[5]); > + snprintf(format->name, sizeof(format->name), "%pUl\n", > + &buffer[5]); Don't we need at least 38 chars for this? https://docs.kernel.org/core-api/printk-formats.html#uuid-guid-addresses > + > format->fcc = 0; > } > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > index 35453f81c1d9..fc6f9e7d8506 100644 > --- a/drivers/media/usb/uvc/uvc_v4l2.c > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > @@ -713,6 +713,10 @@ static int uvc_ioctl_enum_fmt(struct uvc_streaming *stream, > if (format->flags & UVC_FMT_FLAG_COMPRESSED) > fmt->flags |= V4L2_FMT_FLAG_COMPRESSED; > fmt->pixelformat = format->fcc; > + if (format->name[0]) > + strscpy(fmt->description, format->name, > + sizeof(fmt->description)); > + > return 0; > } > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h > index 9a596c8d894a..22656755a801 100644 > --- a/drivers/media/usb/uvc/uvcvideo.h > +++ b/drivers/media/usb/uvc/uvcvideo.h > @@ -264,6 +264,8 @@ struct uvc_format { > u32 fcc; > u32 flags; > > + char name[32]; > + > unsigned int nframes; > struct uvc_frame *frame; > }; > -- Ricardo Ribalda
On 29/03/2023 18:05, Ricardo Ribalda wrote: > Hi Hans > > Thanks for the patch. > > I believe the user can fetch this info from lsusb, so this is kind of > duplicated info, and this is why it was removed. You got to set some description, so using the GUID this seems best. > Is there an app that uses this unknown format code ? Or the only > complaint is that WARN() is too loud for the user? Normally drivers do not pass on unknown formats, but if a driver does, then I want a WARN. If a driver does this legitimately (and I understand that's the case for UVC), then the driver should fill in the description to avoid this WARN. > > Regards! > > On Wed, 29 Mar 2023 at 14:39, Hans Verkuil <hverkuil@xs4all.nl> wrote: >> >> If the fcc is 0 (indicating an unknown GUID format), then fill in the >> description field in ENUM_FMT. Otherwise the V4L2 core will WARN. >> >> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> >> Fixes: 50459f103edf ("media: uvcvideo: Remove format descriptions") >> --- >> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c >> index 7aefa76a42b3..2f1ced1212cd 100644 >> --- a/drivers/media/usb/uvc/uvc_driver.c >> +++ b/drivers/media/usb/uvc/uvc_driver.c >> @@ -256,6 +256,9 @@ static int uvc_parse_format(struct uvc_device *dev, >> } else { >> dev_info(&streaming->intf->dev, >> "Unknown video format %pUl\n", &buffer[5]); >> + snprintf(format->name, sizeof(format->name), "%pUl\n", >> + &buffer[5]); > Don't we need at least 38 chars for this? Yes. But all we have is 31 chars, so we take what we can :-) This is what uvc did before this was removed. Regards, Hans > > https://docs.kernel.org/core-api/printk-formats.html#uuid-guid-addresses > > >> + >> format->fcc = 0; >> } >> >> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c >> index 35453f81c1d9..fc6f9e7d8506 100644 >> --- a/drivers/media/usb/uvc/uvc_v4l2.c >> +++ b/drivers/media/usb/uvc/uvc_v4l2.c >> @@ -713,6 +713,10 @@ static int uvc_ioctl_enum_fmt(struct uvc_streaming *stream, >> if (format->flags & UVC_FMT_FLAG_COMPRESSED) >> fmt->flags |= V4L2_FMT_FLAG_COMPRESSED; >> fmt->pixelformat = format->fcc; >> + if (format->name[0]) >> + strscpy(fmt->description, format->name, >> + sizeof(fmt->description)); >> + >> return 0; >> } >> >> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h >> index 9a596c8d894a..22656755a801 100644 >> --- a/drivers/media/usb/uvc/uvcvideo.h >> +++ b/drivers/media/usb/uvc/uvcvideo.h >> @@ -264,6 +264,8 @@ struct uvc_format { >> u32 fcc; >> u32 flags; >> >> + char name[32]; >> + >> unsigned int nframes; >> struct uvc_frame *frame; >> }; >> > > > -- > Ricardo Ribalda
Hi, On 3/29/23 14:28, Hans Verkuil wrote: > If the fcc is 0 (indicating an unknown GUID format), then fill in the > description field in ENUM_FMT. Otherwise the V4L2 core will WARN. > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > Fixes: 50459f103edf ("media: uvcvideo: Remove format descriptions") Thanks, patch looks good to me: FWIW: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > --- > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > index 7aefa76a42b3..2f1ced1212cd 100644 > --- a/drivers/media/usb/uvc/uvc_driver.c > +++ b/drivers/media/usb/uvc/uvc_driver.c > @@ -256,6 +256,9 @@ static int uvc_parse_format(struct uvc_device *dev, > } 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; > } > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > index 35453f81c1d9..fc6f9e7d8506 100644 > --- a/drivers/media/usb/uvc/uvc_v4l2.c > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > @@ -713,6 +713,10 @@ static int uvc_ioctl_enum_fmt(struct uvc_streaming *stream, > if (format->flags & UVC_FMT_FLAG_COMPRESSED) > fmt->flags |= V4L2_FMT_FLAG_COMPRESSED; > fmt->pixelformat = format->fcc; > + if (format->name[0]) > + strscpy(fmt->description, format->name, > + sizeof(fmt->description)); > + > return 0; > } > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h > index 9a596c8d894a..22656755a801 100644 > --- a/drivers/media/usb/uvc/uvcvideo.h > +++ b/drivers/media/usb/uvc/uvcvideo.h > @@ -264,6 +264,8 @@ struct uvc_format { > u32 fcc; > u32 flags; > > + char name[32]; > + > unsigned int nframes; > struct uvc_frame *frame; > }; >
Hello, On Wed, Mar 29, 2023 at 07:20:59PM +0200, Hans Verkuil wrote: > On 29/03/2023 18:05, Ricardo Ribalda wrote: > > Hi Hans > > > > Thanks for the patch. > > > > I believe the user can fetch this info from lsusb, so this is kind of > > duplicated info, and this is why it was removed. > > You got to set some description, so using the GUID this seems best. > > > Is there an app that uses this unknown format code ? Or the only > > complaint is that WARN() is too loud for the user? > > Normally drivers do not pass on unknown formats, but if a driver does, > then I want a WARN. If a driver does this legitimately (and I understand > that's the case for UVC), then the driver should fill in the description > to avoid this WARN. In hindsight we shouldn't have added a text description to formats :-) > > On Wed, 29 Mar 2023 at 14:39, Hans Verkuil wrote: > >> > >> If the fcc is 0 (indicating an unknown GUID format), then fill in the > >> description field in ENUM_FMT. Otherwise the V4L2 core will WARN. > >> > >> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > >> Fixes: 50459f103edf ("media: uvcvideo: Remove format descriptions") > >> --- > >> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > >> index 7aefa76a42b3..2f1ced1212cd 100644 > >> --- a/drivers/media/usb/uvc/uvc_driver.c > >> +++ b/drivers/media/usb/uvc/uvc_driver.c > >> @@ -256,6 +256,9 @@ static int uvc_parse_format(struct uvc_device *dev, > >> } else { > >> dev_info(&streaming->intf->dev, > >> "Unknown video format %pUl\n", &buffer[5]); > >> + snprintf(format->name, sizeof(format->name), "%pUl\n", > >> + &buffer[5]); > > Don't we need at least 38 chars for this? > > Yes. But all we have is 31 chars, so we take what we can :-) > > This is what uvc did before this was removed. > > Regards, > > Hans > > > https://docs.kernel.org/core-api/printk-formats.html#uuid-guid-addresses > > > >> + > >> format->fcc = 0; > >> } > >> > >> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > >> index 35453f81c1d9..fc6f9e7d8506 100644 > >> --- a/drivers/media/usb/uvc/uvc_v4l2.c > >> +++ b/drivers/media/usb/uvc/uvc_v4l2.c > >> @@ -713,6 +713,10 @@ static int uvc_ioctl_enum_fmt(struct uvc_streaming *stream, > >> if (format->flags & UVC_FMT_FLAG_COMPRESSED) > >> fmt->flags |= V4L2_FMT_FLAG_COMPRESSED; > >> fmt->pixelformat = format->fcc; > >> + if (format->name[0]) > >> + strscpy(fmt->description, format->name, > >> + sizeof(fmt->description)); > >> + > >> return 0; > >> } > >> > >> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h > >> index 9a596c8d894a..22656755a801 100644 > >> --- a/drivers/media/usb/uvc/uvcvideo.h > >> +++ b/drivers/media/usb/uvc/uvcvideo.h > >> @@ -264,6 +264,8 @@ struct uvc_format { > >> u32 fcc; > >> u32 flags; > >> > >> + char name[32]; > >> + I'd not really nice to have to store the name for every format, when we know it will very rarely be used. One alternative option would be to store the GUID, which would halve the amount of memory. Another option would be to stop reporting those formats to userspace in uvc_ioctl_enum_fmt(). They can't be selected anyway, they have no unique 4CC. > >> unsigned int nframes; > >> struct uvc_frame *frame; > >> };
Linux regression tracking (Thorsten Leemhuis)
April 11, 2023, 11:49 a.m. UTC |
#5
Addressed
Unaddressed
On 29.03.23 14:28, Hans Verkuil wrote: > If the fcc is 0 (indicating an unknown GUID format), then fill in the > description field in ENUM_FMT. Otherwise the V4L2 core will WARN. > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > Fixes: 50459f103edf ("media: uvcvideo: Remove format descriptions") Thx for working on this. Would be good to have Reported-by and Link tags for any reports about the issue; I'm aware of the following two, maybe there were more: https://bugzilla.kernel.org/show_bug.cgi?id=217252 https://bugzilla.redhat.com/show_bug.cgi?id=2180107 And a Cc: <stable@vger.kernel.org> # 5.15.x would likely be good as well, as the culprit was backported. But I write for a different reason: how urgent is this fix? Is this "just" fixing a kernel warning, or do users notice this as some apps crash? The bugzilla.redhat.com ticket's subject indicates it's the latter; and I think I saw someone else mentioning that this leads to crashes, but maybe I'm mixing things up. Because if this fixes a crash, it afaics would be good to get this fixed rather sooner than later in mainline, so that it can be fixed in stable as well. Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page. > --- > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > index 7aefa76a42b3..2f1ced1212cd 100644 > --- a/drivers/media/usb/uvc/uvc_driver.c > +++ b/drivers/media/usb/uvc/uvc_driver.c > @@ -256,6 +256,9 @@ static int uvc_parse_format(struct uvc_device *dev, > } 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; > } > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > index 35453f81c1d9..fc6f9e7d8506 100644 > --- a/drivers/media/usb/uvc/uvc_v4l2.c > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > @@ -713,6 +713,10 @@ static int uvc_ioctl_enum_fmt(struct uvc_streaming *stream, > if (format->flags & UVC_FMT_FLAG_COMPRESSED) > fmt->flags |= V4L2_FMT_FLAG_COMPRESSED; > fmt->pixelformat = format->fcc; > + if (format->name[0]) > + strscpy(fmt->description, format->name, > + sizeof(fmt->description)); > + > return 0; > } > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h > index 9a596c8d894a..22656755a801 100644 > --- a/drivers/media/usb/uvc/uvcvideo.h > +++ b/drivers/media/usb/uvc/uvcvideo.h > @@ -264,6 +264,8 @@ struct uvc_format { > u32 fcc; > u32 flags; > > + char name[32]; > + > unsigned int nframes; > struct uvc_frame *frame; > }; > > >
Hi Thorsten, On Tue, Apr 11, 2023 at 01:49:03PM +0200, Thorsten Leemhuis wrote: > On 29.03.23 14:28, Hans Verkuil wrote: > > If the fcc is 0 (indicating an unknown GUID format), then fill in the > > description field in ENUM_FMT. Otherwise the V4L2 core will WARN. > > > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > > Fixes: 50459f103edf ("media: uvcvideo: Remove format descriptions") > > Thx for working on this. > > Would be good to have Reported-by and Link tags for any reports about > the issue; I'm aware of the following two, maybe there were more: > > https://bugzilla.kernel.org/show_bug.cgi?id=217252 > https://bugzilla.redhat.com/show_bug.cgi?id=2180107 > > And a Cc: <stable@vger.kernel.org> # 5.15.x would likely be good as > well, as the culprit was backported. > > But I write for a different reason: how urgent is this fix? Is this > "just" fixing a kernel warning, or do users notice this as some apps > crash? The bugzilla.redhat.com ticket's subject indicates it's the > latter; and I think I saw someone else mentioning that this leads to > crashes, but maybe I'm mixing things up. > > Because if this fixes a crash, it afaics would be good to get this fixed > rather sooner than later in mainline, so that it can be fixed in stable > as well. As far as I understand, it fixes a WARN_ON(). That's technically not a crash, but it looks like a big scary one to anyone watching the kernel log. > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) > -- > Everything you wanna know about Linux kernel regression tracking: > https://linux-regtracking.leemhuis.info/about/#tldr > If I did something stupid, please tell me, as explained on that page. > > > --- > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > > index 7aefa76a42b3..2f1ced1212cd 100644 > > --- a/drivers/media/usb/uvc/uvc_driver.c > > +++ b/drivers/media/usb/uvc/uvc_driver.c > > @@ -256,6 +256,9 @@ static int uvc_parse_format(struct uvc_device *dev, > > } 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; > > } > > > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > > index 35453f81c1d9..fc6f9e7d8506 100644 > > --- a/drivers/media/usb/uvc/uvc_v4l2.c > > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > > @@ -713,6 +713,10 @@ static int uvc_ioctl_enum_fmt(struct uvc_streaming *stream, > > if (format->flags & UVC_FMT_FLAG_COMPRESSED) > > fmt->flags |= V4L2_FMT_FLAG_COMPRESSED; > > fmt->pixelformat = format->fcc; > > + if (format->name[0]) > > + strscpy(fmt->description, format->name, > > + sizeof(fmt->description)); > > + > > return 0; > > } > > > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h > > index 9a596c8d894a..22656755a801 100644 > > --- a/drivers/media/usb/uvc/uvcvideo.h > > +++ b/drivers/media/usb/uvc/uvcvideo.h > > @@ -264,6 +264,8 @@ struct uvc_format { > > u32 fcc; > > u32 flags; > > > > + char name[32]; > > + > > unsigned int nframes; > > struct uvc_frame *frame; > > };
Hi Thorsten On Tue, 11 Apr 2023 at 13:51, Thorsten Leemhuis <regressions@leemhuis.info> wrote: > > On 29.03.23 14:28, Hans Verkuil wrote: > > If the fcc is 0 (indicating an unknown GUID format), then fill in the > > description field in ENUM_FMT. Otherwise the V4L2 core will WARN. > > > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > > Fixes: 50459f103edf ("media: uvcvideo: Remove format descriptions") > > Thx for working on this. > > Would be good to have Reported-by and Link tags for any reports about > the issue; I'm aware of the following two, maybe there were more: > > https://bugzilla.kernel.org/show_bug.cgi?id=217252 > https://bugzilla.redhat.com/show_bug.cgi?id=2180107 > > And a Cc: <stable@vger.kernel.org> # 5.15.x would likely be good as > well, as the culprit was backported. > > But I write for a different reason: how urgent is this fix? Is this > "just" fixing a kernel warning, or do users notice this as some apps > crash? The bugzilla.redhat.com ticket's subject indicates it's the > latter; and I think I saw someone else mentioning that this leads to > crashes, but maybe I'm mixing things up. I think it only a crash if panic_on_warn is enabled. > > Because if this fixes a crash, it afaics would be good to get this fixed > rather sooner than later in mainline, so that it can be fixed in stable > as well. > > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) > -- > Everything you wanna know about Linux kernel regression tracking: > https://linux-regtracking.leemhuis.info/about/#tldr > If I did something stupid, please tell me, as explained on that page. > > > > --- > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > > index 7aefa76a42b3..2f1ced1212cd 100644 > > --- a/drivers/media/usb/uvc/uvc_driver.c > > +++ b/drivers/media/usb/uvc/uvc_driver.c > > @@ -256,6 +256,9 @@ static int uvc_parse_format(struct uvc_device *dev, > > } 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; > > } > > > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > > index 35453f81c1d9..fc6f9e7d8506 100644 > > --- a/drivers/media/usb/uvc/uvc_v4l2.c > > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > > @@ -713,6 +713,10 @@ static int uvc_ioctl_enum_fmt(struct uvc_streaming *stream, > > if (format->flags & UVC_FMT_FLAG_COMPRESSED) > > fmt->flags |= V4L2_FMT_FLAG_COMPRESSED; > > fmt->pixelformat = format->fcc; > > + if (format->name[0]) > > + strscpy(fmt->description, format->name, > > + sizeof(fmt->description)); > > + > > return 0; > > } > > > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h > > index 9a596c8d894a..22656755a801 100644 > > --- a/drivers/media/usb/uvc/uvcvideo.h > > +++ b/drivers/media/usb/uvc/uvcvideo.h > > @@ -264,6 +264,8 @@ struct uvc_format { > > u32 fcc; > > u32 flags; > > > > + char name[32]; > > + > > unsigned int nframes; > > struct uvc_frame *frame; > > }; > > > > > >
Linux regression tracking (Thorsten Leemhuis)
April 11, 2023, 12:26 p.m. UTC |
#8
Addressed
Unaddressed
On 11.04.23 14:06, Laurent Pinchart wrote: > Hi Thorsten, > > On Tue, Apr 11, 2023 at 01:49:03PM +0200, Thorsten Leemhuis wrote: >> On 29.03.23 14:28, Hans Verkuil wrote: >>> If the fcc is 0 (indicating an unknown GUID format), then fill in the >>> description field in ENUM_FMT. Otherwise the V4L2 core will WARN. >>> >>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> >>> Fixes: 50459f103edf ("media: uvcvideo: Remove format descriptions") >> >> Thx for working on this. >> >> Would be good to have Reported-by and Link tags for any reports about >> the issue; I'm aware of the following two, maybe there were more: >> >> https://bugzilla.kernel.org/show_bug.cgi?id=217252 >> https://bugzilla.redhat.com/show_bug.cgi?id=2180107 >> >> And a Cc: <stable@vger.kernel.org> # 5.15.x would likely be good as >> well, as the culprit was backported. >> >> But I write for a different reason: how urgent is this fix? Is this >> "just" fixing a kernel warning, or do users notice this as some apps >> crash? The bugzilla.redhat.com ticket's subject indicates it's the >> latter; and I think I saw someone else mentioning that this leads to >> crashes, but maybe I'm mixing things up. >> >> Because if this fixes a crash, it afaics would be good to get this fixed >> rather sooner than later in mainline, so that it can be fixed in stable >> as well. > > As far as I understand, it fixes a WARN_ON(). That's technically not a > crash, but it looks like a big scary one to anyone watching the kernel > log. Thx for clarifying. I know what WARN_ON() does, but for some reason was under the strong impression that one of the reports mentioned the video app was crashing in parallel to the kernel warning. Well, guess I mixed this up with some other regression. Sorry for the noise! Ciao, Thorsten >>> --- >>> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c >>> index 7aefa76a42b3..2f1ced1212cd 100644 >>> --- a/drivers/media/usb/uvc/uvc_driver.c >>> +++ b/drivers/media/usb/uvc/uvc_driver.c >>> @@ -256,6 +256,9 @@ static int uvc_parse_format(struct uvc_device *dev, >>> } 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; >>> } >>> >>> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c >>> index 35453f81c1d9..fc6f9e7d8506 100644 >>> --- a/drivers/media/usb/uvc/uvc_v4l2.c >>> +++ b/drivers/media/usb/uvc/uvc_v4l2.c >>> @@ -713,6 +713,10 @@ static int uvc_ioctl_enum_fmt(struct uvc_streaming *stream, >>> if (format->flags & UVC_FMT_FLAG_COMPRESSED) >>> fmt->flags |= V4L2_FMT_FLAG_COMPRESSED; >>> fmt->pixelformat = format->fcc; >>> + if (format->name[0]) >>> + strscpy(fmt->description, format->name, >>> + sizeof(fmt->description)); >>> + >>> return 0; >>> } >>> >>> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h >>> index 9a596c8d894a..22656755a801 100644 >>> --- a/drivers/media/usb/uvc/uvcvideo.h >>> +++ b/drivers/media/usb/uvc/uvcvideo.h >>> @@ -264,6 +264,8 @@ struct uvc_format { >>> u32 fcc; >>> u32 flags; >>> >>> + char name[32]; >>> + >>> unsigned int nframes; >>> struct uvc_frame *frame; >>> }; >
Hi Hans, On Wed, Apr 05, 2023 at 09:40:20AM +0300, Laurent Pinchart wrote: > On Wed, Mar 29, 2023 at 07:20:59PM +0200, Hans Verkuil wrote: > > On 29/03/2023 18:05, Ricardo Ribalda wrote: > > > Hi Hans > > > > > > Thanks for the patch. > > > > > > I believe the user can fetch this info from lsusb, so this is kind of > > > duplicated info, and this is why it was removed. > > > > You got to set some description, so using the GUID this seems best. > > > > > Is there an app that uses this unknown format code ? Or the only > > > complaint is that WARN() is too loud for the user? > > > > Normally drivers do not pass on unknown formats, but if a driver does, > > then I want a WARN. If a driver does this legitimately (and I understand > > that's the case for UVC), then the driver should fill in the description > > to avoid this WARN. > > In hindsight we shouldn't have added a text description to formats :-) > > > > On Wed, 29 Mar 2023 at 14:39, Hans Verkuil wrote: > > >> > > >> If the fcc is 0 (indicating an unknown GUID format), then fill in the > > >> description field in ENUM_FMT. Otherwise the V4L2 core will WARN. > > >> > > >> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > > >> Fixes: 50459f103edf ("media: uvcvideo: Remove format descriptions") > > >> --- > > >> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > > >> index 7aefa76a42b3..2f1ced1212cd 100644 > > >> --- a/drivers/media/usb/uvc/uvc_driver.c > > >> +++ b/drivers/media/usb/uvc/uvc_driver.c > > >> @@ -256,6 +256,9 @@ static int uvc_parse_format(struct uvc_device *dev, > > >> } else { > > >> dev_info(&streaming->intf->dev, > > >> "Unknown video format %pUl\n", &buffer[5]); > > >> + snprintf(format->name, sizeof(format->name), "%pUl\n", > > >> + &buffer[5]); > > > Don't we need at least 38 chars for this? > > > > Yes. But all we have is 31 chars, so we take what we can :-) > > > > This is what uvc did before this was removed. > > > > Regards, > > > > Hans > > > > > https://docs.kernel.org/core-api/printk-formats.html#uuid-guid-addresses > > > > > >> + > > >> format->fcc = 0; > > >> } > > >> > > >> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > > >> index 35453f81c1d9..fc6f9e7d8506 100644 > > >> --- a/drivers/media/usb/uvc/uvc_v4l2.c > > >> +++ b/drivers/media/usb/uvc/uvc_v4l2.c > > >> @@ -713,6 +713,10 @@ static int uvc_ioctl_enum_fmt(struct uvc_streaming *stream, > > >> if (format->flags & UVC_FMT_FLAG_COMPRESSED) > > >> fmt->flags |= V4L2_FMT_FLAG_COMPRESSED; > > >> fmt->pixelformat = format->fcc; > > >> + if (format->name[0]) > > >> + strscpy(fmt->description, format->name, > > >> + sizeof(fmt->description)); > > >> + > > >> return 0; > > >> } > > >> > > >> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h > > >> index 9a596c8d894a..22656755a801 100644 > > >> --- a/drivers/media/usb/uvc/uvcvideo.h > > >> +++ b/drivers/media/usb/uvc/uvcvideo.h > > >> @@ -264,6 +264,8 @@ struct uvc_format { > > >> u32 fcc; > > >> u32 flags; > > >> > > >> + char name[32]; > > >> + > > I'd not really nice to have to store the name for every format, when we > know it will very rarely be used. > > One alternative option would be to store the GUID, which would halve the > amount of memory. Another option would be to stop reporting those > formats to userspace in uvc_ioctl_enum_fmt(). They can't be selected > anyway, they have no unique 4CC. Any opinion on this ? I'm increasingly tempted by not reporting unsupported formats to userspace. > > >> unsigned int nframes; > > >> struct uvc_frame *frame; > > >> };
On 19/04/2023 08:23, Laurent Pinchart wrote: > Hi Hans, > > On Wed, Apr 05, 2023 at 09:40:20AM +0300, Laurent Pinchart wrote: >> On Wed, Mar 29, 2023 at 07:20:59PM +0200, Hans Verkuil wrote: >>> On 29/03/2023 18:05, Ricardo Ribalda wrote: >>>> Hi Hans >>>> >>>> Thanks for the patch. >>>> >>>> I believe the user can fetch this info from lsusb, so this is kind of >>>> duplicated info, and this is why it was removed. >>> >>> You got to set some description, so using the GUID this seems best. >>> >>>> Is there an app that uses this unknown format code ? Or the only >>>> complaint is that WARN() is too loud for the user? >>> >>> Normally drivers do not pass on unknown formats, but if a driver does, >>> then I want a WARN. If a driver does this legitimately (and I understand >>> that's the case for UVC), then the driver should fill in the description >>> to avoid this WARN. >> >> In hindsight we shouldn't have added a text description to formats :-) >> >>>> On Wed, 29 Mar 2023 at 14:39, Hans Verkuil wrote: >>>>> >>>>> If the fcc is 0 (indicating an unknown GUID format), then fill in the >>>>> description field in ENUM_FMT. Otherwise the V4L2 core will WARN. >>>>> >>>>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> >>>>> Fixes: 50459f103edf ("media: uvcvideo: Remove format descriptions") >>>>> --- >>>>> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c >>>>> index 7aefa76a42b3..2f1ced1212cd 100644 >>>>> --- a/drivers/media/usb/uvc/uvc_driver.c >>>>> +++ b/drivers/media/usb/uvc/uvc_driver.c >>>>> @@ -256,6 +256,9 @@ static int uvc_parse_format(struct uvc_device *dev, >>>>> } else { >>>>> dev_info(&streaming->intf->dev, >>>>> "Unknown video format %pUl\n", &buffer[5]); >>>>> + snprintf(format->name, sizeof(format->name), "%pUl\n", >>>>> + &buffer[5]); >>>> Don't we need at least 38 chars for this? >>> >>> Yes. But all we have is 31 chars, so we take what we can :-) >>> >>> This is what uvc did before this was removed. >>> >>> Regards, >>> >>> Hans >>> >>>> https://docs.kernel.org/core-api/printk-formats.html#uuid-guid-addresses >>>> >>>>> + >>>>> format->fcc = 0; >>>>> } >>>>> >>>>> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c >>>>> index 35453f81c1d9..fc6f9e7d8506 100644 >>>>> --- a/drivers/media/usb/uvc/uvc_v4l2.c >>>>> +++ b/drivers/media/usb/uvc/uvc_v4l2.c >>>>> @@ -713,6 +713,10 @@ static int uvc_ioctl_enum_fmt(struct uvc_streaming *stream, >>>>> if (format->flags & UVC_FMT_FLAG_COMPRESSED) >>>>> fmt->flags |= V4L2_FMT_FLAG_COMPRESSED; >>>>> fmt->pixelformat = format->fcc; >>>>> + if (format->name[0]) >>>>> + strscpy(fmt->description, format->name, >>>>> + sizeof(fmt->description)); >>>>> + >>>>> return 0; >>>>> } >>>>> >>>>> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h >>>>> index 9a596c8d894a..22656755a801 100644 >>>>> --- a/drivers/media/usb/uvc/uvcvideo.h >>>>> +++ b/drivers/media/usb/uvc/uvcvideo.h >>>>> @@ -264,6 +264,8 @@ struct uvc_format { >>>>> u32 fcc; >>>>> u32 flags; >>>>> >>>>> + char name[32]; >>>>> + >> >> I'd not really nice to have to store the name for every format, when we >> know it will very rarely be used. >> >> One alternative option would be to store the GUID, which would halve the >> amount of memory. Another option would be to stop reporting those >> formats to userspace in uvc_ioctl_enum_fmt(). They can't be selected >> anyway, they have no unique 4CC. > > Any opinion on this ? I'm increasingly tempted by not reporting > unsupported formats to userspace. Since they cannot be selected, I think that is the best approach. Perhaps only show them in the kernel log if debugging is enabled. Regards, Hans > >>>>> unsigned int nframes; >>>>> struct uvc_frame *frame; >>>>> }; >
Linux regression tracking (Thorsten Leemhuis)
May 31, 2023, 11:48 a.m. UTC |
#11
Addressed
Unaddressed
On 29.03.23 14:28, Hans Verkuil wrote: > If the fcc is 0 (indicating an unknown GUID format), then fill in the > description field in ENUM_FMT. Otherwise the V4L2 core will WARN. What happened to this? It seems this fall through the cracks. BTW: > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> Afaics it might be good to have these in here: Link: https://bugzilla.kernel.org/show_bug.cgi?id=217252 Link: https://bugzilla.redhat.com/show_bug.cgi?id=2180107 A comment in the former is what brought me here. Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page. #regzbot ^backmonitor: https://lore.kernel.org/lkml/dc8e5276-ef88-648f-9f0d-10151ea62c90@leemhuis.info/ #regzbot poke > Fixes: 50459f103edf ("media: uvcvideo: Remove format descriptions") > --- > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > index 7aefa76a42b3..2f1ced1212cd 100644 > --- a/drivers/media/usb/uvc/uvc_driver.c > +++ b/drivers/media/usb/uvc/uvc_driver.c > @@ -256,6 +256,9 @@ static int uvc_parse_format(struct uvc_device *dev, > } 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; > } > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > index 35453f81c1d9..fc6f9e7d8506 100644 > --- a/drivers/media/usb/uvc/uvc_v4l2.c > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > @@ -713,6 +713,10 @@ static int uvc_ioctl_enum_fmt(struct uvc_streaming *stream, > if (format->flags & UVC_FMT_FLAG_COMPRESSED) > fmt->flags |= V4L2_FMT_FLAG_COMPRESSED; > fmt->pixelformat = format->fcc; > + if (format->name[0]) > + strscpy(fmt->description, format->name, > + sizeof(fmt->description)); > + > return 0; > } > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h > index 9a596c8d894a..22656755a801 100644 > --- a/drivers/media/usb/uvc/uvcvideo.h > +++ b/drivers/media/usb/uvc/uvcvideo.h > @@ -264,6 +264,8 @@ struct uvc_format { > u32 fcc; > u32 flags; > > + char name[32]; > + > unsigned int nframes; > struct uvc_frame *frame; > }; >
Hi Thorsten, On Wed, May 31, 2023 at 01:48:51PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote: > On 29.03.23 14:28, Hans Verkuil wrote: > > If the fcc is 0 (indicating an unknown GUID format), then fill in the > > description field in ENUM_FMT. Otherwise the V4L2 core will WARN. > > What happened to this? It seems this fall through the cracks. I've posted a better fix, see https://lore.kernel.org/linux-media/20230506065809.24645-1-laurent.pinchart@ideasonboard.com/. Of course, fate had it that it got reviewed exactly on the day when I started by holidays :-S I'm now back, and will send a pull request. > BTW: > > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > > Afaics it might be good to have these in here: > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=217252 > Link: https://bugzilla.redhat.com/show_bug.cgi?id=2180107 I'll add those two links. > A comment in the former is what brought me here. > > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) > -- > Everything you wanna know about Linux kernel regression tracking: > https://linux-regtracking.leemhuis.info/about/#tldr > If I did something stupid, please tell me, as explained on that page. > > #regzbot ^backmonitor: > https://lore.kernel.org/lkml/dc8e5276-ef88-648f-9f0d-10151ea62c90@leemhuis.info/ > #regzbot poke > > > Fixes: 50459f103edf ("media: uvcvideo: Remove format descriptions") > > --- > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > > index 7aefa76a42b3..2f1ced1212cd 100644 > > --- a/drivers/media/usb/uvc/uvc_driver.c > > +++ b/drivers/media/usb/uvc/uvc_driver.c > > @@ -256,6 +256,9 @@ static int uvc_parse_format(struct uvc_device *dev, > > } 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; > > } > > > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c > > index 35453f81c1d9..fc6f9e7d8506 100644 > > --- a/drivers/media/usb/uvc/uvc_v4l2.c > > +++ b/drivers/media/usb/uvc/uvc_v4l2.c > > @@ -713,6 +713,10 @@ static int uvc_ioctl_enum_fmt(struct uvc_streaming *stream, > > if (format->flags & UVC_FMT_FLAG_COMPRESSED) > > fmt->flags |= V4L2_FMT_FLAG_COMPRESSED; > > fmt->pixelformat = format->fcc; > > + if (format->name[0]) > > + strscpy(fmt->description, format->name, > > + sizeof(fmt->description)); > > + > > return 0; > > } > > > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h > > index 9a596c8d894a..22656755a801 100644 > > --- a/drivers/media/usb/uvc/uvcvideo.h > > +++ b/drivers/media/usb/uvc/uvcvideo.h > > @@ -264,6 +264,8 @@ struct uvc_format { > > u32 fcc; > > u32 flags; > > > > + char name[32]; > > + > > unsigned int nframes; > > struct uvc_frame *frame; > > };
Linux regression tracking (Thorsten Leemhuis)
May 31, 2023, 12:54 p.m. UTC |
#13
Addressed
Unaddressed
On 31.05.23 14:44, Laurent Pinchart wrote: > Hi Thorsten, > > On Wed, May 31, 2023 at 01:48:51PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote: >> On 29.03.23 14:28, Hans Verkuil wrote: >>> If the fcc is 0 (indicating an unknown GUID format), then fill in the >>> description field in ENUM_FMT. Otherwise the V4L2 core will WARN. >> >> What happened to this? It seems this fall through the cracks. > > I've posted a better fix, You mean an even better fix, as this one already was a better fix for a earlier fix ;) > see > https://lore.kernel.org/linux-media/20230506065809.24645-1-laurent.pinchart@ideasonboard.com/. > Of course, fate had it that it got reviewed exactly on the day when I started by holidays :-S :-D > I'm now back, and will send a pull request. In that case: welcome back. >> BTW: >> >>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> >> >> Afaics it might be good to have these in here: >> >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217252 >> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2180107 > > I'll add those two links. Great, many thx! Ciao, Thorsten >> A comment in the former is what brought me here. >> >> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) >> -- >> Everything you wanna know about Linux kernel regression tracking: >> https://linux-regtracking.leemhuis.info/about/#tldr >> If I did something stupid, please tell me, as explained on that page. >> >> #regzbot ^backmonitor: >> https://lore.kernel.org/lkml/dc8e5276-ef88-648f-9f0d-10151ea62c90@leemhuis.info/ >> #regzbot poke >> >>> Fixes: 50459f103edf ("media: uvcvideo: Remove format descriptions") >>> --- >>> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c >>> index 7aefa76a42b3..2f1ced1212cd 100644 >>> --- a/drivers/media/usb/uvc/uvc_driver.c >>> +++ b/drivers/media/usb/uvc/uvc_driver.c >>> @@ -256,6 +256,9 @@ static int uvc_parse_format(struct uvc_device *dev, >>> } 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; >>> } >>> >>> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c >>> index 35453f81c1d9..fc6f9e7d8506 100644 >>> --- a/drivers/media/usb/uvc/uvc_v4l2.c >>> +++ b/drivers/media/usb/uvc/uvc_v4l2.c >>> @@ -713,6 +713,10 @@ static int uvc_ioctl_enum_fmt(struct uvc_streaming *stream, >>> if (format->flags & UVC_FMT_FLAG_COMPRESSED) >>> fmt->flags |= V4L2_FMT_FLAG_COMPRESSED; >>> fmt->pixelformat = format->fcc; >>> + if (format->name[0]) >>> + strscpy(fmt->description, format->name, >>> + sizeof(fmt->description)); >>> + >>> return 0; >>> } >>> >>> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h >>> index 9a596c8d894a..22656755a801 100644 >>> --- a/drivers/media/usb/uvc/uvcvideo.h >>> +++ b/drivers/media/usb/uvc/uvcvideo.h >>> @@ -264,6 +264,8 @@ struct uvc_format { >>> u32 fcc; >>> u32 flags; >>> >>> + char name[32]; >>> + >>> unsigned int nframes; >>> struct uvc_frame *frame; >>> }; >
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 7aefa76a42b3..2f1ced1212cd 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -256,6 +256,9 @@ static int uvc_parse_format(struct uvc_device *dev, } 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; } diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c index 35453f81c1d9..fc6f9e7d8506 100644 --- a/drivers/media/usb/uvc/uvc_v4l2.c +++ b/drivers/media/usb/uvc/uvc_v4l2.c @@ -713,6 +713,10 @@ static int uvc_ioctl_enum_fmt(struct uvc_streaming *stream, if (format->flags & UVC_FMT_FLAG_COMPRESSED) fmt->flags |= V4L2_FMT_FLAG_COMPRESSED; fmt->pixelformat = format->fcc; + if (format->name[0]) + strscpy(fmt->description, format->name, + sizeof(fmt->description)); + return 0; } diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index 9a596c8d894a..22656755a801 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -264,6 +264,8 @@ struct uvc_format { u32 fcc; u32 flags; + char name[32]; + unsigned int nframes; struct uvc_frame *frame; };