* Re: [PATCH v2 2/3] usb: gadget/uvc: also handle v4l2 ioctl ENUM_FMT
[not found] ` <1407512339-8433-3-git-send-email-m.grzeschik@pengutronix.de>
@ 2014-08-19 17:01 ` Laurent Pinchart
2014-08-20 2:06 ` Hans Verkuil
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Pinchart @ 2014-08-19 17:01 UTC (permalink / raw)
To: Michael Grzeschik; +Cc: linux-usb, balbi, kernel, linux-media, hans.verkuil
Hi Michael,
Thank you for the patch.
(CC'ing Hans Verkuil and the linux-media mailing list)
On Friday 08 August 2014 17:38:58 Michael Grzeschik wrote:
> This patch adds ENUM_FMT as possible ioctl to the uvc v4l2 device.
> That makes userspace applications with a generic IOCTL calling
> convention make also use of it.
>
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
> v1 -> v2:
> - changed first switch case to simple if
> - added separate function
> - added description field
> - bail out on array boundaries
>
> drivers/usb/gadget/uvc_v4l2.c | 30 ++++++++++++++++++++++++++++--
> 1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/uvc_v4l2.c b/drivers/usb/gadget/uvc_v4l2.c
> index ad48e81..58633bf 100644
> --- a/drivers/usb/gadget/uvc_v4l2.c
> +++ b/drivers/usb/gadget/uvc_v4l2.c
> @@ -55,14 +55,30 @@ struct uvc_format
> {
> u8 bpp;
> u32 fcc;
> + char *description;
> };
>
> static struct uvc_format uvc_formats[] = {
> - { 16, V4L2_PIX_FMT_YUYV },
> - { 0, V4L2_PIX_FMT_MJPEG },
> + { 16, V4L2_PIX_FMT_YUYV, "YUV 4:2:2" },
> + { 0, V4L2_PIX_FMT_MJPEG, "MJPEG" },
Format descriptions are currently duplicated in every driver, causing higher
memory usage and different descriptions for the same format depending on the
driver. Hans, should we try to fix this ?
> };
>
> static int
> +uvc_v4l2_enum_format(struct uvc_video *video, struct v4l2_fmtdesc *fmt)
> +{
> +
There's an extra blank line here.
> + int index = fmt->index;
You can use fmt->index directly below, there's no need for a local variable.
> + if (index >= ARRAY_SIZE(uvc_formats))
> + return -EINVAL;
> +
> + strcpy(fmt->description, uvc_formats[index].description);
How about strlcpy to make sure we don't overflow the buffer ?
> + fmt->pixelformat = uvc_formats[index].fcc;
> +
> + return 0;
> +}
> +
> +static int
> uvc_v4l2_get_format(struct uvc_video *video, struct v4l2_format *fmt)
> {
> fmt->fmt.pix.pixelformat = video->fcc;
> @@ -183,6 +199,16 @@ uvc_v4l2_do_ioctl(struct file *file, unsigned int cmd,
> void *arg) break;
> }
>
> + case VIDIOC_ENUM_FMT:
> + {
> + struct v4l2_fmtdesc *fmt = arg;
> +
> + if (fmt->type != video->queue.queue.type)
> + return -EINVAL;
> +
> + return uvc_v4l2_enum_format(video, fmt);
> + }
> +
> /* Get & Set format */
> case VIDIOC_G_FMT:
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/3] usb: gadget/uvc: also handle v4l2 ioctl ENUM_FMT
2014-08-19 17:01 ` [PATCH v2 2/3] usb: gadget/uvc: also handle v4l2 ioctl ENUM_FMT Laurent Pinchart
@ 2014-08-20 2:06 ` Hans Verkuil
2014-08-20 17:05 ` Laurent Pinchart
0 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2014-08-20 2:06 UTC (permalink / raw)
To: Laurent Pinchart, Michael Grzeschik
Cc: linux-usb, balbi, kernel, linux-media, hans.verkuil
On 08/19/2014 05:01 PM, Laurent Pinchart wrote:
> Hi Michael,
>
> Thank you for the patch.
>
> (CC'ing Hans Verkuil and the linux-media mailing list)
>
> On Friday 08 August 2014 17:38:58 Michael Grzeschik wrote:
>> This patch adds ENUM_FMT as possible ioctl to the uvc v4l2 device.
>> That makes userspace applications with a generic IOCTL calling
>> convention make also use of it.
>>
>> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>> ---
>> v1 -> v2:
>> - changed first switch case to simple if
>> - added separate function
>> - added description field
>> - bail out on array boundaries
>>
>> drivers/usb/gadget/uvc_v4l2.c | 30 ++++++++++++++++++++++++++++--
>> 1 file changed, 28 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/uvc_v4l2.c b/drivers/usb/gadget/uvc_v4l2.c
>> index ad48e81..58633bf 100644
>> --- a/drivers/usb/gadget/uvc_v4l2.c
>> +++ b/drivers/usb/gadget/uvc_v4l2.c
>> @@ -55,14 +55,30 @@ struct uvc_format
>> {
>> u8 bpp;
>> u32 fcc;
>> + char *description;
>> };
>>
>> static struct uvc_format uvc_formats[] = {
>> - { 16, V4L2_PIX_FMT_YUYV },
>> - { 0, V4L2_PIX_FMT_MJPEG },
>> + { 16, V4L2_PIX_FMT_YUYV, "YUV 4:2:2" },
>> + { 0, V4L2_PIX_FMT_MJPEG, "MJPEG" },
>
> Format descriptions are currently duplicated in every driver, causing higher
> memory usage and different descriptions for the same format depending on the
> driver. Hans, should we try to fix this ?
Yes, we should. It's been on my todo list for ages, but at a very low priority.
I'm not planning to work on this in the near future, but if someone else wants
to work on this, then just go ahead.
Regards,
Hans
>
>> };
>>
>> static int
>> +uvc_v4l2_enum_format(struct uvc_video *video, struct v4l2_fmtdesc *fmt)
>> +{
>> +
>
> There's an extra blank line here.
>
>> + int index = fmt->index;
>
> You can use fmt->index directly below, there's no need for a local variable.
>
>> + if (index >= ARRAY_SIZE(uvc_formats))
>> + return -EINVAL;
>> +
>> + strcpy(fmt->description, uvc_formats[index].description);
>
> How about strlcpy to make sure we don't overflow the buffer ?
>
>> + fmt->pixelformat = uvc_formats[index].fcc;
>> +
>> + return 0;
>> +}
>> +
>> +static int
>> uvc_v4l2_get_format(struct uvc_video *video, struct v4l2_format *fmt)
>> {
>> fmt->fmt.pix.pixelformat = video->fcc;
>> @@ -183,6 +199,16 @@ uvc_v4l2_do_ioctl(struct file *file, unsigned int cmd,
>> void *arg) break;
>> }
>>
>> + case VIDIOC_ENUM_FMT:
>> + {
>> + struct v4l2_fmtdesc *fmt = arg;
>> +
>> + if (fmt->type != video->queue.queue.type)
>> + return -EINVAL;
>> +
>> + return uvc_v4l2_enum_format(video, fmt);
>> + }
>> +
>> /* Get & Set format */
>> case VIDIOC_G_FMT:
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/3] usb: gadget/uvc: also handle v4l2 ioctl ENUM_FMT
2014-08-20 2:06 ` Hans Verkuil
@ 2014-08-20 17:05 ` Laurent Pinchart
2014-08-25 13:59 ` Michael Grzeschik
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Pinchart @ 2014-08-20 17:05 UTC (permalink / raw)
To: Hans Verkuil, Michael Grzeschik
Cc: linux-usb, balbi, kernel, linux-media, hans.verkuil
Hi Hans and Michael,
On Wednesday 20 August 2014 02:06:54 Hans Verkuil wrote:
> On 08/19/2014 05:01 PM, Laurent Pinchart wrote:
> > Hi Michael,
> >
> > Thank you for the patch.
> >
> > (CC'ing Hans Verkuil and the linux-media mailing list)
> >
> > On Friday 08 August 2014 17:38:58 Michael Grzeschik wrote:
> >> This patch adds ENUM_FMT as possible ioctl to the uvc v4l2 device.
> >> That makes userspace applications with a generic IOCTL calling
> >> convention make also use of it.
> >>
> >> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> >> ---
> >>
> >> v1 -> v2:
> >> - changed first switch case to simple if
> >> - added separate function
> >> - added description field
> >> - bail out on array boundaries
> >>
> >> drivers/usb/gadget/uvc_v4l2.c | 30 ++++++++++++++++++++++++++++--
> >> 1 file changed, 28 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/usb/gadget/uvc_v4l2.c
> >> b/drivers/usb/gadget/uvc_v4l2.c
> >> index ad48e81..58633bf 100644
> >> --- a/drivers/usb/gadget/uvc_v4l2.c
> >> +++ b/drivers/usb/gadget/uvc_v4l2.c
> >> @@ -55,14 +55,30 @@ struct uvc_format
> >> {
> >> u8 bpp;
> >> u32 fcc;
> >> + char *description;
> >> };
> >>
> >> static struct uvc_format uvc_formats[] = {
> >> - { 16, V4L2_PIX_FMT_YUYV },
> >> - { 0, V4L2_PIX_FMT_MJPEG },
> >> + { 16, V4L2_PIX_FMT_YUYV, "YUV 4:2:2" },
> >> + { 0, V4L2_PIX_FMT_MJPEG, "MJPEG" },
> >
> > Format descriptions are currently duplicated in every driver, causing
> > higher memory usage and different descriptions for the same format
> > depending on the driver. Hans, should we try to fix this ?
>
> Yes, we should. It's been on my todo list for ages, but at a very low
> priority. I'm not planning to work on this in the near future, but if
> someone else wants to work on this, then just go ahead.
Michael, would you like to give this a try, or should I do it ?
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/3] usb: gadget/uvc: also handle v4l2 ioctl ENUM_FMT
2014-08-20 17:05 ` Laurent Pinchart
@ 2014-08-25 13:59 ` Michael Grzeschik
2014-08-25 14:48 ` Laurent Pinchart
0 siblings, 1 reply; 6+ messages in thread
From: Michael Grzeschik @ 2014-08-25 13:59 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Hans Verkuil, Michael Grzeschik, linux-usb, balbi, kernel,
linux-media, hans.verkuil
Hi Laurent,
On Wed, Aug 20, 2014 at 07:05:30PM +0200, Laurent Pinchart wrote:
> Hi Hans and Michael,
>
> On Wednesday 20 August 2014 02:06:54 Hans Verkuil wrote:
> > On 08/19/2014 05:01 PM, Laurent Pinchart wrote:
> > > Hi Michael,
> > >
> > > Thank you for the patch.
> > >
> > > (CC'ing Hans Verkuil and the linux-media mailing list)
> > >
> > > On Friday 08 August 2014 17:38:58 Michael Grzeschik wrote:
> > >> This patch adds ENUM_FMT as possible ioctl to the uvc v4l2 device.
> > >> That makes userspace applications with a generic IOCTL calling
> > >> convention make also use of it.
> > >>
> > >> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> > >> ---
> > >>
> > >> v1 -> v2:
> > >> - changed first switch case to simple if
> > >> - added separate function
> > >> - added description field
> > >> - bail out on array boundaries
> > >>
> > >> drivers/usb/gadget/uvc_v4l2.c | 30 ++++++++++++++++++++++++++++--
> > >> 1 file changed, 28 insertions(+), 2 deletions(-)
> > >>
> > >> diff --git a/drivers/usb/gadget/uvc_v4l2.c
> > >> b/drivers/usb/gadget/uvc_v4l2.c
> > >> index ad48e81..58633bf 100644
> > >> --- a/drivers/usb/gadget/uvc_v4l2.c
> > >> +++ b/drivers/usb/gadget/uvc_v4l2.c
> > >> @@ -55,14 +55,30 @@ struct uvc_format
> > >> {
> > >> u8 bpp;
> > >> u32 fcc;
> > >> + char *description;
> > >> };
> > >>
> > >> static struct uvc_format uvc_formats[] = {
> > >> - { 16, V4L2_PIX_FMT_YUYV },
> > >> - { 0, V4L2_PIX_FMT_MJPEG },
> > >> + { 16, V4L2_PIX_FMT_YUYV, "YUV 4:2:2" },
> > >> + { 0, V4L2_PIX_FMT_MJPEG, "MJPEG" },
> > >
> > > Format descriptions are currently duplicated in every driver, causing
> > > higher memory usage and different descriptions for the same format
> > > depending on the driver. Hans, should we try to fix this ?
> >
> > Yes, we should. It's been on my todo list for ages, but at a very low
> > priority. I'm not planning to work on this in the near future, but if
> > someone else wants to work on this, then just go ahead.
>
> Michael, would you like to give this a try, or should I do it ?
It seems Philipp is already taking the chance! :)
Regards,
Michael
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/3] usb: gadget/uvc: also handle v4l2 ioctl ENUM_FMT
2014-08-25 13:59 ` Michael Grzeschik
@ 2014-08-25 14:48 ` Laurent Pinchart
2014-08-25 15:13 ` Philipp Zabel
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Pinchart @ 2014-08-25 14:48 UTC (permalink / raw)
To: Michael Grzeschik
Cc: Hans Verkuil, Michael Grzeschik, linux-usb, balbi, kernel,
linux-media, hans.verkuil
Hi Michael,
On Monday 25 August 2014 15:59:57 Michael Grzeschik wrote:
> On Wed, Aug 20, 2014 at 07:05:30PM +0200, Laurent Pinchart wrote:
> > On Wednesday 20 August 2014 02:06:54 Hans Verkuil wrote:
> > > On 08/19/2014 05:01 PM, Laurent Pinchart wrote:
> > > > Hi Michael,
> > > >
> > > > Thank you for the patch.
> > > >
> > > > (CC'ing Hans Verkuil and the linux-media mailing list)
> > > >
> > > > On Friday 08 August 2014 17:38:58 Michael Grzeschik wrote:
> > > >> This patch adds ENUM_FMT as possible ioctl to the uvc v4l2 device.
> > > >> That makes userspace applications with a generic IOCTL calling
> > > >> convention make also use of it.
> > > >>
> > > >> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> > > >> ---
> > > >>
> > > >> v1 -> v2:
> > > >> - changed first switch case to simple if
> > > >> - added separate function
> > > >> - added description field
> > > >> - bail out on array boundaries
> > > >>
> > > >> drivers/usb/gadget/uvc_v4l2.c | 30 ++++++++++++++++++++++++++++--
> > > >> 1 file changed, 28 insertions(+), 2 deletions(-)
> > > >>
> > > >> diff --git a/drivers/usb/gadget/uvc_v4l2.c
> > > >> b/drivers/usb/gadget/uvc_v4l2.c
> > > >> index ad48e81..58633bf 100644
> > > >> --- a/drivers/usb/gadget/uvc_v4l2.c
> > > >> +++ b/drivers/usb/gadget/uvc_v4l2.c
> > > >> @@ -55,14 +55,30 @@ struct uvc_format
> > > >> {
> > > >> u8 bpp;
> > > >> u32 fcc;
> > > >> + char *description;
> > > >> };
> > > >>
> > > >> static struct uvc_format uvc_formats[] = {
> > > >> - { 16, V4L2_PIX_FMT_YUYV },
> > > >> - { 0, V4L2_PIX_FMT_MJPEG },
> > > >> + { 16, V4L2_PIX_FMT_YUYV, "YUV 4:2:2" },
> > > >> + { 0, V4L2_PIX_FMT_MJPEG, "MJPEG" },
> > > >
> > > > Format descriptions are currently duplicated in every driver, causing
> > > > higher memory usage and different descriptions for the same format
> > > > depending on the driver. Hans, should we try to fix this ?
> > >
> > > Yes, we should. It's been on my todo list for ages, but at a very low
> > > priority. I'm not planning to work on this in the near future, but if
> > > someone else wants to work on this, then just go ahead.
> >
> > Michael, would you like to give this a try, or should I do it ?
>
> It seems Philipp is already taking the chance! :)
Perfect timing, I wonder if that's just a coincidence ;-)
I don't think this patch is very urgent, would you be fine with rebasing it on
top of Philipp's patch when it will be accepted ?
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/3] usb: gadget/uvc: also handle v4l2 ioctl ENUM_FMT
2014-08-25 14:48 ` Laurent Pinchart
@ 2014-08-25 15:13 ` Philipp Zabel
0 siblings, 0 replies; 6+ messages in thread
From: Philipp Zabel @ 2014-08-25 15:13 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Michael Grzeschik, Hans Verkuil, Michael Grzeschik, linux-usb,
balbi, kernel, linux-media, hans.verkuil
Hi Laurent,
Am Montag, den 25.08.2014, 16:48 +0200 schrieb Laurent Pinchart:
[...]
> > > > > Format descriptions are currently duplicated in every driver, causing
> > > > > higher memory usage and different descriptions for the same format
> > > > > depending on the driver. Hans, should we try to fix this ?
> > > >
> > > > Yes, we should. It's been on my todo list for ages, but at a very low
> > > > priority. I'm not planning to work on this in the near future, but if
> > > > someone else wants to work on this, then just go ahead.
> > >
> > > Michael, would you like to give this a try, or should I do it ?
> >
> > It seems Philipp is already taking the chance! :)
>
> Perfect timing, I wonder if that's just a coincidence ;-)
It felt like my own idea this weekend, but I strongly suspect that I
took up enough information to trigger it, when scanning mail last week.
I shouldn't be so fast to dismiss uvc/gadget mails as "Michael's
business"...
regards
Philipp
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-08-25 15:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1407512339-8433-1-git-send-email-m.grzeschik@pengutronix.de>
[not found] ` <1407512339-8433-3-git-send-email-m.grzeschik@pengutronix.de>
2014-08-19 17:01 ` [PATCH v2 2/3] usb: gadget/uvc: also handle v4l2 ioctl ENUM_FMT Laurent Pinchart
2014-08-20 2:06 ` Hans Verkuil
2014-08-20 17:05 ` Laurent Pinchart
2014-08-25 13:59 ` Michael Grzeschik
2014-08-25 14:48 ` Laurent Pinchart
2014-08-25 15:13 ` Philipp Zabel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).