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