Re: Bug: Two device nodes created in /dev for a single UVC webcam

[Alexandre-Xavier Labonté-Lamoureux]

Hi Laurent,

I see why. `make install` installed my newly built modules into
`/lib/modules/4.9.0-5-amd64`, but I was running version 4.9.0-6 of the
kernel. Each kernel version has its own folder with its own modules.

I tried to revert the four commits, but they created conflicts. I
wasn't able to resolve them easily and, to be honest, I don't have a
lot of time to spend on this.

Regards,
Alexandre-Xavier

On Sun, Feb 25, 2018 at 6:41 AM, Laurent Pinchart
 wrote:
> Hi Alexandre-Xavier,
>
> On Sunday, 25 February 2018 10:19:51 EET Alexandre-Xavier Labonté-Lamoureux
> wrote:
>> Hi Laurent,
>>
>> Sorry for the late reply.
>>
>> I've been trying to reproduce the issue again. I cloned the entire
>> media repository later during the week and I haven't been able to
>> reproduce the issue after I installed the modules. A metadata node is
>> no longer created for my webcam. The four commits that you've
>> mentioned are still in the commit log, so it seems that they didn't
>> break anything.
>
> Now that's weird. I would expect a metadata video node to be created if the
> patches I mentioned are applied. Are you sure you have loaded the modules
> corresponding to the compiled sources ?
>
>> I'm not sure what could have changed that would have caused it to work
>> fine this time. I believe that I'm in the correct branch.
>>
>> $ git status
>> On branch media_tree/master
>> Your branch is up-to-date with 'r_media_tree/master'.
>>
>> I probably did `./build` instead of `./build --main-git` the first time.
>>
>> On Mon, Feb 19, 2018 at 2:10 PM, Laurent Pinchart wrote:
>> > On Monday, 19 February 2018 19:29:24 EET Alexandre-Xavier
>> > Labonté-Lamoureux wrote:
>> >> Hi Kieran,
>> >>
>> >> This is how I built the drivers:
>> >>
>> >> $ git clone --depth=1 git://linuxtv.org/media_build.git
>> >> $ cd media_build
>> >> $ ./build --main-git
>> >>
>> >> I then installed the newly built kernel modules:
>> >>
>> >> $ sudo make install
>> >>
>> >> Once the modules were updated, I restarted my computer to make sure
>> >> every module got reloaded. I didn't make any changes to the code and I
>> >> found the issues after trying each of those programs individually
>> >> after I restarted my computer.
>> >>
>> >> This was the latest commit when I cloned the repo:
>> >>
>> >> commit d144cfe4b3c37ece55ae27778c99765d4943c4fa
>> >> Author: Jasmin Jessich 
>> >> Date:   Fri Feb 16 22:40:49 2018 +0100
>> >>
>> >> Re-generated v3.12_kfifo_in.patch
>> >>
>> >> My version of VLC is 2.2.6. Here's a copy of the relevant data of
>> >> VLC's log file in case it can help: https://paste.debian.net/1011025/
>> >> In this case, I tried to open /dev/video0 first and /dev/video1 second.
>> >>
>> >> I can also try with ffplay:
>> >> $ ffplay /dev/video0
>> >>
>> >> I get this: [video4linux2,v4l2 @ 0x7f216920]
>> >> ioctl(VIDIOC_STREAMON): Message too long
>> >> /dev/video0: Message too long
>> >>
>> >> A new message appears in dmesg: uvcvideo: Failed to submit URB 0 (-90).
>> >
>> > That's interesting, and possibly unrelated to the patch series that added
>> > metadata capture support. Would you be able to revert that patch series
>> > and see if the problem still occurs ? The four commits to be reverted are
>> >
>> > 088ead25524583e2200aa99111bea2f66a86545a
>> > 3bc85817d7982ed53fbc9b150b0205beff68ca5c
>> > 94c53e26dc74744cc4f9a8ddc593b7aef96ba764
>> > 31a96f4c872e8fb953c853630f69d5de6ec961c9
>> >
>> > And if you could bisect the issue it would be even better :-)
>> >
>> > Could you also send me the output of lsusb -v for your camera (you can
>> > restrict it to the camera with -d VID:PID), running as root if possible ?
>> >
>> >> $ ffplay /dev/video1
>> >>
>> >> I get this:
>> >> [video4linux2,v4l2 @ 0x7f00ec000920] ioctl(VIDIOC_G_INPUT):
>> >> Inappropriate ioctl for device
>> >> /dev/video1: Inappropriate ioctl for device
>> >>
>> >> Like Guennadi said, /dev/video1 is a metadata node, so I don't expect
>> >> it to work. In the case of /dev/video0, I can't tell what could be
>> >> wrong from the error message.
>
> --
> Regards,
>
> Laurent Pinchart
>

Re: Bug: Two device nodes created in /dev for a single UVC webcam

[Laurent Pinchart]

Hi Alexandre-Xavier,

On Sunday, 25 February 2018 10:19:51 EET Alexandre-Xavier Labonté-Lamoureux 
wrote:
> Hi Laurent,
> 
> Sorry for the late reply.
> 
> I've been trying to reproduce the issue again. I cloned the entire
> media repository later during the week and I haven't been able to
> reproduce the issue after I installed the modules. A metadata node is
> no longer created for my webcam. The four commits that you've
> mentioned are still in the commit log, so it seems that they didn't
> break anything.

Now that's weird. I would expect a metadata video node to be created if the 
patches I mentioned are applied. Are you sure you have loaded the modules 
corresponding to the compiled sources ?

> I'm not sure what could have changed that would have caused it to work
> fine this time. I believe that I'm in the correct branch.
> 
> $ git status
> On branch media_tree/master
> Your branch is up-to-date with 'r_media_tree/master'.
> 
> I probably did `./build` instead of `./build --main-git` the first time.
> 
> On Mon, Feb 19, 2018 at 2:10 PM, Laurent Pinchart wrote:
> > On Monday, 19 February 2018 19:29:24 EET Alexandre-Xavier
> > Labonté-Lamoureux wrote:
> >> Hi Kieran,
> >> 
> >> This is how I built the drivers:
> >> 
> >> $ git clone --depth=1 git://linuxtv.org/media_build.git
> >> $ cd media_build
> >> $ ./build --main-git
> >> 
> >> I then installed the newly built kernel modules:
> >> 
> >> $ sudo make install
> >> 
> >> Once the modules were updated, I restarted my computer to make sure
> >> every module got reloaded. I didn't make any changes to the code and I
> >> found the issues after trying each of those programs individually
> >> after I restarted my computer.
> >> 
> >> This was the latest commit when I cloned the repo:
> >> 
> >> commit d144cfe4b3c37ece55ae27778c99765d4943c4fa
> >> Author: Jasmin Jessich 
> >> Date:   Fri Feb 16 22:40:49 2018 +0100
> >> 
> >> Re-generated v3.12_kfifo_in.patch
> >> 
> >> My version of VLC is 2.2.6. Here's a copy of the relevant data of
> >> VLC's log file in case it can help: https://paste.debian.net/1011025/
> >> In this case, I tried to open /dev/video0 first and /dev/video1 second.
> >> 
> >> I can also try with ffplay:
> >> $ ffplay /dev/video0
> >> 
> >> I get this: [video4linux2,v4l2 @ 0x7f216920]
> >> ioctl(VIDIOC_STREAMON): Message too long
> >> /dev/video0: Message too long
> >> 
> >> A new message appears in dmesg: uvcvideo: Failed to submit URB 0 (-90).
> > 
> > That's interesting, and possibly unrelated to the patch series that added
> > metadata capture support. Would you be able to revert that patch series
> > and see if the problem still occurs ? The four commits to be reverted are
> > 
> > 088ead25524583e2200aa99111bea2f66a86545a
> > 3bc85817d7982ed53fbc9b150b0205beff68ca5c
> > 94c53e26dc74744cc4f9a8ddc593b7aef96ba764
> > 31a96f4c872e8fb953c853630f69d5de6ec961c9
> > 
> > And if you could bisect the issue it would be even better :-)
> > 
> > Could you also send me the output of lsusb -v for your camera (you can
> > restrict it to the camera with -d VID:PID), running as root if possible ?
> > 
> >> $ ffplay /dev/video1
> >> 
> >> I get this:
> >> [video4linux2,v4l2 @ 0x7f00ec000920] ioctl(VIDIOC_G_INPUT):
> >> Inappropriate ioctl for device
> >> /dev/video1: Inappropriate ioctl for device
> >> 
> >> Like Guennadi said, /dev/video1 is a metadata node, so I don't expect
> >> it to work. In the case of /dev/video0, I can't tell what could be
> >> wrong from the error message.

-- 
Regards,

Laurent Pinchart

Re: Bug: Two device nodes created in /dev for a single UVC webcam

[Alexandre-Xavier Labonté-Lamoureux]

Hi Laurent,

Sorry for the late reply.

I've been trying to reproduce the issue again. I cloned the entire
media repository later during the week and I haven't been able to
reproduce the issue after I installed the modules. A metadata node is
no longer created for my webcam. The four commits that you've
mentioned are still in the commit log, so it seems that they didn't
break anything.

I'm not sure what could have changed that would have caused it to work
fine this time. I believe that I'm in the correct branch.

$ git status
On branch media_tree/master
Your branch is up-to-date with 'r_media_tree/master'.

I probably did `./build` instead of `./build --main-git` the first time.

Thank you,
Alexandre-Xavier

On Mon, Feb 19, 2018 at 2:10 PM, Laurent Pinchart
 wrote:
> Hi Alexandre-Xavier,
>
> On Monday, 19 February 2018 19:29:24 EET Alexandre-Xavier Labonté-Lamoureux
> wrote:
>> Hi Kieran,
>>
>> This is how I built the drivers:
>>
>> $ git clone --depth=1 git://linuxtv.org/media_build.git
>> $ cd media_build
>> $ ./build --main-git
>>
>> I then installed the newly built kernel modules:
>>
>> $ sudo make install
>>
>> Once the modules were updated, I restarted my computer to make sure
>> every module got reloaded. I didn't make any changes to the code and I
>> found the issues after trying each of those programs individually
>> after I restarted my computer.
>>
>> This was the latest commit when I cloned the repo:
>>
>> commit d144cfe4b3c37ece55ae27778c99765d4943c4fa
>> Author: Jasmin Jessich 
>> Date:   Fri Feb 16 22:40:49 2018 +0100
>> Re-generated v3.12_kfifo_in.patch
>>
>> My version of VLC is 2.2.6. Here's a copy of the relevant data of
>> VLC's log file in case it can help: https://paste.debian.net/1011025/
>> In this case, I tried to open /dev/video0 first and /dev/video1 second.
>>
>> I can also try with ffplay:
>> $ ffplay /dev/video0
>>
>> I get this: [video4linux2,v4l2 @ 0x7f216920]
>> ioctl(VIDIOC_STREAMON): Message too long
>> /dev/video0: Message too long
>>
>> A new message appears in dmesg: uvcvideo: Failed to submit URB 0 (-90).
>
> That's interesting, and possibly unrelated to the patch series that added
> metadata capture support. Would you be able to revert that patch series and
> see if the problem still occurs ? The four commits to be reverted are
>
> 088ead25524583e2200aa99111bea2f66a86545a
> 3bc85817d7982ed53fbc9b150b0205beff68ca5c
> 94c53e26dc74744cc4f9a8ddc593b7aef96ba764
> 31a96f4c872e8fb953c853630f69d5de6ec961c9
>
> And if you could bisect the issue it would be even better :-)
>
> Could you also send me the output of lsusb -v for your camera (you can
> restrict it to the camera with -d VID:PID), running as root if possible ?
>
>> $ ffplay /dev/video1
>>
>> I get this:
>> [video4linux2,v4l2 @ 0x7f00ec000920] ioctl(VIDIOC_G_INPUT):
>> Inappropriate ioctl for device
>> /dev/video1: Inappropriate ioctl for device
>>
>> Like Guennadi said, /dev/video1 is a metadata node, so I don't expect
>> it to work. In the case of /dev/video0, I can't tell what could be
>> wrong from the error message.
>
> --
> Regards,
>
> Laurent Pinchart
>

Re: Bug: Two device nodes created in /dev for a single UVC webcam

[Laurent Pinchart]

Hi Devin,

On Tuesday, 20 February 2018 20:18:16 EET Devin Heitmueller wrote:
> On Mon, Feb 19, 2018 at 11:19 AM, Laurent Pinchart wrote:
> > I've tested VLC (2.2.8) and haven't noticed any issue. If a program is
> > directed to the metadata video node and tries to capture video from it it
> > will obviously fail. That being said, software that work today should
> > continue working, otherwise it's a regression, and we'll have to handle
> > that.
> 
> Perhaps it shouldn't be a video node then (as we do with VBI devices).
> Would something like /dev/videometadataX would be more appropriate?

We've thought about it, and the initial implementation created a metadata 
device node instead of a video device node. This has been rejected, see 
https://www.mail-archive.com/linux-media@vger.kernel.org/msg97454.html and 
https://www.mail-archive.com/linux-media@vger.kernel.org/msg97446.html.

> People have for years operated under the expectation that /dev/videoX
> nodes are video nodes.  If we're going to be creating things with that
> name which aren't video nodes then that is going to cause considerable
> confusion as well as messing up all sorts of existing applications
> which operate under that expectation.
> 
> I know that some of the older PCI boards have always exposed a bunch
> of video nodes for various things (i.e. raw video vs. mpeg, etc), but
> because USB devices have traditionally been simpler they generally
> expose only one node of each type (i.e. one /dev/videoX, /dev/vbiX
> /dev/radioX).  I've already gotten an email from a customer who has a
> ton of scripts which depend on this behavior, so please seriously
> consider the implications of this design decision.

While I can't speak about other USB devices as I'm not too familiar with them, 
please note that the UVC driver already exposes multiple video nodes related 
to video capture (or video output) for some devices, and will posssibly do so 
increasingly in the future when we add support for UVC 1.5. We can reconsider 
the decision of exposing metadata through a video node, but adding new video 
nodes to expose additional compressed video streams for UVC 1.5 support is 
something that userspace has to live with the same way it already has to live 
with multiple video nodes for older PCI boards.

> It's easy to brush this off as "all the existing applications will
> eventually be updated", but you're talking about changing the basic
> behavior of how these device nodes have been presented for over a
> decade.

That's not what I meant, I might have not expressed myself correctly. Updating 
applications is something we should strive for when we want to get rid of an 
undesired userspace behaviour, but that's in no way an excuse for breaking 
anything. Regarding the issue reported by Alexandre-Xavier, it looks to me 
like he might be suffering from another problem, possibly part of the same 
patch series, but not caused by the extra video device node. That's why I 
asked for more information before taking any decision.

-- 
Regards,

Laurent Pinchart

Re: Bug: Two device nodes created in /dev for a single UVC webcam

[Devin Heitmueller]

Hi Laurent,

On Mon, Feb 19, 2018 at 11:19 AM, Laurent Pinchart
 wrote:
> I've tested VLC (2.2.8) and haven't noticed any issue. If a program is
> directed to the metadata video node and tries to capture video from it it will
> obviously fail. That being said, software that work today should continue
> working, otherwise it's a regression, and we'll have to handle that.

Perhaps it shouldn't be a video node then (as we do with VBI devices).
Would something like /dev/videometadataX would be more appropriate?

People have for years operated under the expectation that /dev/videoX
nodes are video nodes.  If we're going to be creating things with that
name which aren't video nodes then that is going to cause considerable
confusion as well as messing up all sorts of existing applications
which operate under that expectation.

I know that some of the older PCI boards have always exposed a bunch
of video nodes for various things (i.e. raw video vs. mpeg, etc), but
because USB devices have traditionally been simpler they generally
expose only one node of each type (i.e. one /dev/videoX, /dev/vbiX
/dev/radioX).  I've already gotten an email from a customer who has a
ton of scripts which depend on this behavior, so please seriously
consider the implications of this design decision.

It's easy to brush this off as "all the existing applications will
eventually be updated", but you're talking about changing the basic
behavior of how these device nodes have been presented for over a
decade.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: Bug: Two device nodes created in /dev for a single UVC webcam

[Laurent Pinchart]

Hi Alexandre-Xavier,

On Monday, 19 February 2018 19:29:24 EET Alexandre-Xavier Labonté-Lamoureux 
wrote:
> Hi Kieran,
> 
> This is how I built the drivers:
> 
> $ git clone --depth=1 git://linuxtv.org/media_build.git
> $ cd media_build
> $ ./build --main-git
> 
> I then installed the newly built kernel modules:
> 
> $ sudo make install
> 
> Once the modules were updated, I restarted my computer to make sure
> every module got reloaded. I didn't make any changes to the code and I
> found the issues after trying each of those programs individually
> after I restarted my computer.
> 
> This was the latest commit when I cloned the repo:
> 
> commit d144cfe4b3c37ece55ae27778c99765d4943c4fa
> Author: Jasmin Jessich 
> Date:   Fri Feb 16 22:40:49 2018 +0100
> Re-generated v3.12_kfifo_in.patch
> 
> My version of VLC is 2.2.6. Here's a copy of the relevant data of
> VLC's log file in case it can help: https://paste.debian.net/1011025/
> In this case, I tried to open /dev/video0 first and /dev/video1 second.
> 
> I can also try with ffplay:
> $ ffplay /dev/video0
> 
> I get this: [video4linux2,v4l2 @ 0x7f216920]
> ioctl(VIDIOC_STREAMON): Message too long
> /dev/video0: Message too long
> 
> A new message appears in dmesg: uvcvideo: Failed to submit URB 0 (-90).

That's interesting, and possibly unrelated to the patch series that added 
metadata capture support. Would you be able to revert that patch series and 
see if the problem still occurs ? The four commits to be reverted are

088ead25524583e2200aa99111bea2f66a86545a
3bc85817d7982ed53fbc9b150b0205beff68ca5c
94c53e26dc74744cc4f9a8ddc593b7aef96ba764
31a96f4c872e8fb953c853630f69d5de6ec961c9

And if you could bisect the issue it would be even better :-)

Could you also send me the output of lsusb -v for your camera (you can 
restrict it to the camera with -d VID:PID), running as root if possible ?

> $ ffplay /dev/video1
> 
> I get this:
> [video4linux2,v4l2 @ 0x7f00ec000920] ioctl(VIDIOC_G_INPUT):
> Inappropriate ioctl for device
> /dev/video1: Inappropriate ioctl for device
> 
> Like Guennadi said, /dev/video1 is a metadata node, so I don't expect
> it to work. In the case of /dev/video0, I can't tell what could be
> wrong from the error message.

-- 
Regards,

Laurent Pinchart

Re: Bug: Two device nodes created in /dev for a single UVC webcam

[Alexandre-Xavier Labonté-Lamoureux]

Hi Kieran,

This is how I built the drivers:

$ git clone --depth=1 git://linuxtv.org/media_build.git
$ cd media_build
$ ./build --main-git

I then installed the newly built kernel modules:

$ sudo make install

Once the modules were updated, I restarted my computer to make sure
every module got reloaded. I didn't make any changes to the code and I
found the issues after trying each of those programs individually
after I restarted my computer.

This was the latest commit when I cloned the repo:

commit d144cfe4b3c37ece55ae27778c99765d4943c4fa
Author: Jasmin Jessich 
Date:   Fri Feb 16 22:40:49 2018 +0100
Re-generated v3.12_kfifo_in.patch

My version of VLC is 2.2.6. Here's a copy of the relevant data of
VLC's log file in case it can help: https://paste.debian.net/1011025/
In this case, I tried to open /dev/video0 first and /dev/video1 second.

I can also try with ffplay:
$ ffplay /dev/video0

I get this: [video4linux2,v4l2 @ 0x7f216920]
ioctl(VIDIOC_STREAMON): Message too long
/dev/video0: Message too long

A new message appears in dmesg: uvcvideo: Failed to submit URB 0 (-90).

$ ffplay /dev/video1

I get this:
[video4linux2,v4l2 @ 0x7f00ec000920] ioctl(VIDIOC_G_INPUT):
Inappropriate ioctl for device
/dev/video1: Inappropriate ioctl for device

Like Guennadi said, /dev/video1 is a metadata node, so I don't expect
it to work. In the case of /dev/video0, I can't tell what could be
wrong from the error message.

Alexandre-Xavier

On Mon, Feb 19, 2018 at 8:52 AM, Kieran Bingham
 wrote:
> Hi Alexandre,
>
> Thankyou for your bug report,
>
> On 17/02/18 20:47, Alexandre-Xavier Labonté-Lamoureux wrote:
>> Hi,
>>
>> I'm running Linux 4.9.0-5-amd64 on Debian. I built the drivers from
>> the latest git and installed the modules.
>
> Could you please be specific here?
>
> Are you referring to linux-media/master branch or such? The latest from 
> Linus' tree?
>
> Please also detail the steps you have taken to reproduce this issue - and of
> course - if you have made any code changes to make the latest UVC module 
> compile
> against a v4.9 kernel...
>
> Building the latest git tree and installing as a module on a v4.9 kernel is
> quite a leap... I wouldn't have expected that to work.
>
> The code would have to be compiled against a v4.9 kernel directly, and I 
> didn't
> think compiling the UVC driver against older kernels worked.
>
> (at least it didn't work cleanly when I tried to compile v4.15 against a v4.14
> kernel last month)
>
>> Now, two device nodes are
>> created for my webcam. This is not normal as it has never happened to
>> me before. If I connect another webcam to my laptop, two more device
>> nodes will be created for this webcam. So two new device nodes are
>> created for a single webcam.
>
> I believe Guennadi's latest work for handling meta-data (in the latest 
> v4.16-rc1
> I think) will create two device nodes.
>
>
>> The name of my webcam appears twice in the device comobox in Guvcview
>> because of this. One of them will not work if I select it.
>
> It would be expected that only the device with video functions as a streaming
> camera device, while the other would not.
>
>
>> My webcam has completely stopped working with Cheese and VLC.
>
> This part is of particular concern however.
>
> Guennadi - Have you tested Cheese/VLC with your series?
>
> Are there any known issues that need looking at ?
>
>>> v4l2-ctl --list-devices
>> Laptop_Integrated_Webcam_E4HD:  (usb-:00:1a.0-1.5):
>> /dev/video0
>> /dev/video1
>>
>>> ls /dev/video*
>> /dev/video0  /dev/video1
>>
>> Have a nice day,
>> Alexandre-Xavier
>
> Regards
>
> Kieran Bingham
>
>

Re: Bug: Two device nodes created in /dev for a single UVC webcam

[Laurent Pinchart]

Hello,

On Monday, 19 February 2018 15:58:40 EET Guennadi Liakhovetski wrote:
> On Mon, 19 Feb 2018, Kieran Bingham wrote:
> > On 17/02/18 20:47, Alexandre-Xavier Labonté-Lamoureux wrote:
> >> Hi,
> >> 
> >> I'm running Linux 4.9.0-5-amd64 on Debian. I built the drivers from
> >> the latest git and installed the modules.
> > 
> > Could you please be specific here?
> > 
> > Are you referring to linux-media/master branch or such? The latest from
> > Linus' tree?
> > 
> > Please also detail the steps you have taken to reproduce this issue - and
> > of course - if you have made any code changes to make the latest UVC
> > module compile against a v4.9 kernel...
> > 
> > Building the latest git tree and installing as a module on a v4.9 kernel
> > is quite a leap... I wouldn't have expected that to work.
> > 
> > The code would have to be compiled against a v4.9 kernel directly, and I
> > didn't think compiling the UVC driver against older kernels worked.
> > 
> > (at least it didn't work cleanly when I tried to compile v4.15 against a
> > v4.14 kernel last month)
> > 
> >> Now, two device nodes are created for my webcam. This is not normal as
> >> it has never happened to me before. If I connect another webcam to my
> >> laptop, two more device nodes will be created for this webcam. So two
> >> new device nodes are created for a single webcam.
> > 
> > I believe Guennadi's latest work for handling meta-data (in the latest
> > v4.16-rc1 I think) will create two device nodes.
> 
> That's correct. The lower index node (/dev/video0) is a video node, the
> higher videoo node (/dev/video1) is a metadata node.
> 
> > > The name of my webcam appears twice in the device comobox in Guvcview
> > > because of this. One of them will not work if I select it.
> > 
> > It would be expected that only the device with video functions as a
> > streaming camera device, while the other would not.
> 
> Exactly.
> 
> > > My webcam has completely stopped working with Cheese and VLC.
> > 
> > This part is of particular concern however.
> > 
> > Guennadi - Have you tested Cheese/VLC with your series?
> 
> Sure, with cheese you can specify which camera you need by using its
> --device= parameter. Eventually it's expected, that those programs will be
> updated to recognise metadata nodes and not attempt to use them.

I've tested VLC (2.2.8) and haven't noticed any issue. If a program is 
directed to the metadata video node and tries to capture video from it it will 
obviously fail. That being said, software that work today should continue 
working, otherwise it's a regression, and we'll have to handle that.

> > Are there any known issues that need looking at ?
> > 
> >>> v4l2-ctl --list-devices
> >> 
> >> Laptop_Integrated_Webcam_E4HD:  (usb-:00:1a.0-1.5):
> >> /dev/video0
> >> /dev/video1
> >>> 
> >>> ls /dev/video*
> >> 
> >> /dev/video0  /dev/video1

-- 
Regards,

Laurent Pinchart

Re: Bug: Two device nodes created in /dev for a single UVC webcam

[Guennadi Liakhovetski]

Hi Kieran,

On Mon, 19 Feb 2018, Kieran Bingham wrote:

> Hi Alexandre,
> 
> Thankyou for your bug report,
> 
> On 17/02/18 20:47, Alexandre-Xavier Labonté-Lamoureux wrote:
> > Hi,
> > 
> > I'm running Linux 4.9.0-5-amd64 on Debian. I built the drivers from
> > the latest git and installed the modules.
> 
> Could you please be specific here?
> 
> Are you referring to linux-media/master branch or such? The latest from 
> Linus' tree?
> 
> Please also detail the steps you have taken to reproduce this issue - and of
> course - if you have made any code changes to make the latest UVC module 
> compile
> against a v4.9 kernel...
> 
> Building the latest git tree and installing as a module on a v4.9 kernel is
> quite a leap... I wouldn't have expected that to work.
> 
> The code would have to be compiled against a v4.9 kernel directly, and I 
> didn't
> think compiling the UVC driver against older kernels worked.
> 
> (at least it didn't work cleanly when I tried to compile v4.15 against a v4.14
> kernel last month)
> 
> > Now, two device nodes are
> > created for my webcam. This is not normal as it has never happened to
> > me before. If I connect another webcam to my laptop, two more device
> > nodes will be created for this webcam. So two new device nodes are
> > created for a single webcam.
> 
> I believe Guennadi's latest work for handling meta-data (in the latest 
> v4.16-rc1
> I think) will create two device nodes.

That's correct. The lower index node (/dev/video0) is a video node, the 
higher videoo node (/dev/video1) is a metadata node.

> > The name of my webcam appears twice in the device comobox in Guvcview
> > because of this. One of them will not work if I select it.
> 
> It would be expected that only the device with video functions as a streaming
> camera device, while the other would not.

Exactly.

> > My webcam has completely stopped working with Cheese and VLC.
> 
> This part is of particular concern however.
> 
> Guennadi - Have you tested Cheese/VLC with your series?

Sure, with cheese you can specify which camera you need by using its 
--device= parameter. Eventually it's expected, that those programs will be 
updated to recognise metadata nodes and not attempt to use them.

Thanks
Guennadi

> Are there any known issues that need looking at ?
> 
> >> v4l2-ctl --list-devices
> > Laptop_Integrated_Webcam_E4HD:  (usb-:00:1a.0-1.5):
> > /dev/video0
> > /dev/video1
> > 
> >> ls /dev/video*
> > /dev/video0  /dev/video1
> >
> > Have a nice day,
> > Alexandre-Xavier
> 
> Regards
> 
> Kieran Bingham
> 
> 

Re: Bug: Two device nodes created in /dev for a single UVC webcam

[Kieran Bingham]

Hi Alexandre,

Thankyou for your bug report,

On 17/02/18 20:47, Alexandre-Xavier Labonté-Lamoureux wrote:
> Hi,
> 
> I'm running Linux 4.9.0-5-amd64 on Debian. I built the drivers from
> the latest git and installed the modules.

Could you please be specific here?

Are you referring to linux-media/master branch or such? The latest from Linus' 
tree?

Please also detail the steps you have taken to reproduce this issue - and of
course - if you have made any code changes to make the latest UVC module compile
against a v4.9 kernel...

Building the latest git tree and installing as a module on a v4.9 kernel is
quite a leap... I wouldn't have expected that to work.

The code would have to be compiled against a v4.9 kernel directly, and I didn't
think compiling the UVC driver against older kernels worked.

(at least it didn't work cleanly when I tried to compile v4.15 against a v4.14
kernel last month)

> Now, two device nodes are
> created for my webcam. This is not normal as it has never happened to
> me before. If I connect another webcam to my laptop, two more device
> nodes will be created for this webcam. So two new device nodes are
> created for a single webcam.

I believe Guennadi's latest work for handling meta-data (in the latest v4.16-rc1
I think) will create two device nodes.


> The name of my webcam appears twice in the device comobox in Guvcview
> because of this. One of them will not work if I select it.

It would be expected that only the device with video functions as a streaming
camera device, while the other would not.


> My webcam has completely stopped working with Cheese and VLC.

This part is of particular concern however.

Guennadi - Have you tested Cheese/VLC with your series?

Are there any known issues that need looking at ?

>> v4l2-ctl --list-devices
> Laptop_Integrated_Webcam_E4HD:  (usb-:00:1a.0-1.5):
> /dev/video0
> /dev/video1
> 
>> ls /dev/video*
> /dev/video0  /dev/video1
>
> Have a nice day,
> Alexandre-Xavier

Regards

Kieran Bingham

Bug: Two device nodes created in /dev for a single UVC webcam

[Alexandre-Xavier Labonté-Lamoureux]

Hi,

I'm running Linux 4.9.0-5-amd64 on Debian. I built the drivers from
the latest git and installed the modules. Now, two device nodes are
created for my webcam. This is not normal as it has never happened to
me before. If I connect another webcam to my laptop, two more device
nodes will be created for this webcam. So two new device nodes are
created for a single webcam.

The name of my webcam appears twice in the device comobox in Guvcview
because of this. One of them will not work if I select it. My webcam
has completely stopped working with Cheese and VLC.

> v4l2-ctl --list-devices
Laptop_Integrated_Webcam_E4HD:  (usb-:00:1a.0-1.5):
/dev/video0
/dev/video1

> ls /dev/video*
/dev/video0  /dev/video1

Have a nice day,
Alexandre-Xavier

segfault with UVC webcam

[Yan Seiner]

Hi everyone:

I am trying to use a Samsung branded webcam and it's resulting in a 
segfault.  This happens with both 3.13 and 4.3.6 kernels.  Googling 
around doesn't reveal anything useful.


[  110.605327] usb 2-1.2: new high-speed USB device number 5 using ehci-pci
[  110.698450] usb 2-1.2: New USB device found, idVendor=04e8, 
idProduct=2061
[  110.698458] usb 2-1.2: New USB device strings: Mfr=1, Product=2, 
SerialNumber=0

[  110.698462] usb 2-1.2: Product: USB2.0 UVC HQ WebCam
[  110.698465] usb 2-1.2: Manufacturer: Alpha Imaging Tech. Corp.
[  110.698889] uvcvideo: Found UVC 1.00 device USB2.0 UVC HQ WebCam 
(04e8:2061)
[  110.699547] input: USB2.0 UVC HQ WebCam as 
/devices/pci:00/:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.0/input/input16

[  136.030151] vmalloc: allocation failure: 0 bytes
[  136.030156] video_source:sr: page allocation failure: order:0, 
mode:0x80d2
[  136.030160] CPU: 3 PID: 10533 Comm: video_source:sr Not tainted 
4.3.6-040306-generic #201602191831
[  136.030162] Hardware name: LENOVO 243852U/243852U, BIOS G5ET98WW 
(2.58 ) 04/01/2014
[  136.030164]   5823f19b 8805a5ee3a20 
813c2e44
[  136.030167]  80d2 8805a5ee3ab0 8118a4d7 
81acc738
[  136.030170]  8805a5ee3a40 88050018 8805a5ee3ac0 
8805a5ee3a60

[  136.030173] Call Trace:
[  136.030181]  [] dump_stack+0x44/0x60
[  136.030187]  [] warn_alloc_failed+0xf7/0x150
[  136.030193]  [] ? usb_start_wait_urb+0xab/0x170
[  136.030196]  [] ? usb_control_msg+0xeb/0x130
[  136.030200]  [] __vmalloc_node_range+0x20b/0x290
[  136.030203]  [] ? usb_control_msg+0xeb/0x130
[  136.030205]  [] vmalloc_user+0x55/0x90
[  136.030211]  [] ? vb2_vmalloc_alloc+0x46/0xc0 
[videobuf2_vmalloc]
[  136.030214]  [] vb2_vmalloc_alloc+0x46/0xc0 
[videobuf2_vmalloc]
[  136.030220]  [] __vb2_queue_alloc+0x164/0x530 
[videobuf2_core]
[  136.030224]  [] __reqbufs.isra.20+0x1f8/0x360 
[videobuf2_core]

[  136.030228]  [] vb2_reqbufs+0x30/0x40 [videobuf2_core]
[  136.030232]  [] uvc_request_buffers+0x2e/0x50 
[uvcvideo]

[  136.030236]  [] uvc_ioctl_reqbufs+0x4a/0xa0 [uvcvideo]
[  136.030244]  [] v4l_reqbufs+0x43/0x50 [videodev]
[  136.030249]  [] __video_do_ioctl+0x291/0x310 [videodev]
[  136.030256]  [] video_usercopy+0x33e/0x550 [videodev]
[  136.030261]  [] ? video_ioctl2+0x20/0x20 [videodev]
[  136.030265]  [] ? sock_write_iter+0x85/0xf0
[  136.030271]  [] video_ioctl2+0x15/0x20 [videodev]
[  136.030276]  [] v4l2_ioctl+0xd3/0xe0 [videodev]
[  136.030280]  [] do_vfs_ioctl+0x295/0x480
[  136.030284]  [] ? vfs_write+0x146/0x1a0
[  136.030287]  [] SyS_ioctl+0x79/0x90
[  136.030290]  [] entry_SYSCALL_64_fastpath+0x16/0x75
[  136.030292] Mem-Info:
[  136.030297] active_anon:321167 inactive_anon:28083 isolated_anon:0
[  136.030297]  active_file:69108 inactive_file:202742 isolated_file:0
[  136.030297]  unevictable:27 dirty:119 writeback:0 unstable:0
[  136.030297]  slab_reclaimable:11597 slab_unreclaimable:9590
[  136.030297]  mapped:62552 shmem:28410 pagetables:9020 bounce:0
[  136.030297]  free:5376384 free_pcp:2637 free_cma:0
[  136.030302] Node 0 DMA free:15884kB min:12kB low:12kB high:16kB 
active_anon:0kB inactive_anon:0kB active_file:0kB inactive_file:0kB 
unevictable:0kB isolated(anon):0kB isolated(file):0kB present:15984kB 
managed:15900kB mlocked:0kB dirty:0kB writeback:0kB mapped:0kB shmem:0kB 
slab_reclaimable:0kB slab_unreclaimable:16kB kernel_stack:0kB 
pagetables:0kB unstable:0kB bounce:0kB free_pcp:0kB local_pcp:0kB 
free_cma:0kB writeback_tmp:0kB pages_scanned:0 all_unreclaimable? yes

[  136.030308] lowmem_reserve[]: 0 2550 23648 23648
[  136.030311] Node 0 DMA32 free:2224980kB min:2116kB low:2644kB 
high:3172kB active_anon:222304kB inactive_anon:13472kB 
active_file:31616kB inactive_file:97440kB unevictable:0kB 
isolated(anon):0kB isolated(file):0kB present:2693612kB 
managed:2613408kB mlocked:0kB dirty:36kB writeback:0kB mapped:28924kB 
shmem:13592kB slab_reclaimable:4936kB slab_unreclaimable:3792kB 
kernel_stack:992kB pagetables:4044kB unstable:0kB bounce:0kB 
free_pcp:5472kB local_pcp:696kB free_cma:0kB writeback_tmp:0kB 
pages_scanned:0 all_unreclaimable? no

[  136.030317] lowmem_reserve[]: 0 0 21098 21098
[  136.030320] Node 0 Normal free:19264672kB min:17516kB low:21892kB 
high:26272kB active_anon:1062364kB inactive_anon:98860kB 
active_file:244816kB inactive_file:713528kB unevictable:108kB 
isolated(anon):0kB isolated(file):0kB present:21993472kB 
managed:21604496kB mlocked:108kB dirty:440kB writeback:0kB 
mapped:221284kB shmem:100048kB slab_reclaimable:41452kB 
slab_unreclaimable:34552kB kernel_stack:8240kB pagetables:32036kB 
unstable:0kB bounce:0kB free_pcp:5076kB local_pcp:632kB free_cma:0kB 
writeback_tmp:0kB pages_scanned:0 all_unreclaimable? no

[  136.030326] lowmem_reserve[]: 0 0 0 0
[  136.030329] Node 0 DMA: 1*4kB (U) 1*8kB (U) 2*16kB (U) 1*32kB (U) 
1*64kB (U) 1*128kB (U) 1*256kB (U) 0*512kB 1*1024kB (U) 1*2048kB (M) 
3*4096kB (M) = 15884kB
[ 

Re: Query regarding the support and testing of MJPEG frame type in the UVC webcam gadget

[Laurent Pinchart]

Hi Bhupesh,

On Wednesday 01 August 2012 21:29:30 Bhupesh SHARMA wrote:
 On Wednesday, August 01, 2012 6:46 PM Laurent Pinchart wrote:
  On Wednesday 01 August 2012 14:26:33 Bhupesh SHARMA wrote:
   Hi Laurent,
   
   I have a query for you regarding the support and testing of MJPEG
   frame type in the UVC webcam gadget.
   
   I see that in the webcam.c gadget, the 720p and VGA MJPEG uvc formats
   are supported. I was trying the same out and got confused because the
   data arriving from a real video capture video supporting JPEG will have
   no fixed size. We will have the JPEG defined Start-of-Frame and End-of-
   Frame markers defining the boundary of the JPEG frame.
   
   But for almost all JPEG video capture devices even if we have kept a
   frame size of VGA initially, the final frame size will be a compressed
   version (with the compression depending on the nature of the scene, so a
   flat scene will have high compression and hence less frame size) of VGA
   and will not be equal to 640 * 480.
   
   So I couldn't exactly get why the dwMaxVideoFrameBufferSize is kept
   as 614400 in webcam.c (see [1]).
  
  The dwMaxVideoFrameBufferSize value must be larger than or equal to the
  largest MJPEG frame size. As I have no idea what that value is, I've
  kept the same size as for uncompressed frames, which should be big enough
  (and most probably too big).
 
 .. Yes, so that means that the user-space application should set the length
 of the buffer being queued at the UVC side equal to the length of the buffer
 dequeued from the V4L2 side, to ensure that varying length JPEG frames are
 correctly handled.

You should copy the bytesused field from the captured v4l2_buffer to the 
output v4l2_buffer. The length field stores the total buffer size, not the 
number of bytes used.

 I will try with these changes in the user-space daemon.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: Query regarding the support and testing of MJPEG frame type in the UVC webcam gadget

[Bhupesh SHARMA]

Hi Laurent,

 -Original Message-
 From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com]
 Sent: Friday, August 03, 2012 1:45 PM
 To: Bhupesh SHARMA
 Cc: linux-...@vger.kernel.org; linux-media@vger.kernel.org
 Subject: Re: Query regarding the support and testing of MJPEG frame
 type in the UVC webcam gadget
 
 Hi Bhupesh,
 
 On Wednesday 01 August 2012 21:29:30 Bhupesh SHARMA wrote:
  On Wednesday, August 01, 2012 6:46 PM Laurent Pinchart wrote:
   On Wednesday 01 August 2012 14:26:33 Bhupesh SHARMA wrote:
Hi Laurent,
   
I have a query for you regarding the support and testing of MJPEG
frame type in the UVC webcam gadget.
   
I see that in the webcam.c gadget, the 720p and VGA MJPEG uvc
 formats
are supported. I was trying the same out and got confused because
 the
data arriving from a real video capture video supporting JPEG
 will have
no fixed size. We will have the JPEG defined Start-of-Frame and
 End-of-
Frame markers defining the boundary of the JPEG frame.
   
But for almost all JPEG video capture devices even if we have
 kept a
frame size of VGA initially, the final frame size will be a
 compressed
version (with the compression depending on the nature of the
 scene, so a
flat scene will have high compression and hence less frame size)
 of VGA
and will not be equal to 640 * 480.
   
So I couldn't exactly get why the dwMaxVideoFrameBufferSize is
 kept
as 614400 in webcam.c (see [1]).
  
   The dwMaxVideoFrameBufferSize value must be larger than or equal to
 the
   largest MJPEG frame size. As I have no idea what that value is,
 I've
   kept the same size as for uncompressed frames, which should be big
 enough
   (and most probably too big).
 
  .. Yes, so that means that the user-space application should set the
 length
  of the buffer being queued at the UVC side equal to the length of the
 buffer
  dequeued from the V4L2 side, to ensure that varying length JPEG
 frames are
  correctly handled.
 
 You should copy the bytesused field from the captured v4l2_buffer to
 the
 output v4l2_buffer. The length field stores the total buffer size, not
 the
 number of bytes used.
 

Yes, you are right. It should be bytesused field instead of the length field.

Regards,
Bhupesh
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Query regarding the support and testing of MJPEG frame type in the UVC webcam gadget

[Bhupesh SHARMA]

Hi Laurent,

I have a query for you regarding the support and testing of MJPEG frame type in 
the UVC webcam gadget.

I see that in the webcam.c gadget, the 720p and VGA MJPEG uvc formats are 
supported. I was trying the same
out and got confused because the data arriving from a real video capture video 
supporting JPEG will have no
fixed size. We will have the JPEG defined Start-of-Frame and End-of-Frame 
markers defining the boundary
of the JPEG frame.

But for almost all JPEG video capture devices even if we have kept a frame size 
of VGA initially, the final
frame size will be a compressed version (with the compression depending on the 
nature of the scene, so a flat
scene will have high compression and hence less frame size) of VGA and will not 
be equal to 640 * 480.

So I couldn't exactly get why the dwMaxVideoFrameBufferSize is kept as 614400 
in webcam.c (see [1]).

Can you please let me know your opinions and how you tested the UVC gadget's 
MJPEG frame format.

[1] http://lxr.linux.no/linux+v3.5/drivers/usb/gadget/webcam.c#L232

Thanks,
Bhupesh
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Query regarding the support and testing of MJPEG frame type in the UVC webcam gadget

[Laurent Pinchart]

Hi Bhupesh,

On Wednesday 01 August 2012 14:26:33 Bhupesh SHARMA wrote:
 Hi Laurent,
 
 I have a query for you regarding the support and testing of MJPEG frame type
 in the UVC webcam gadget.
 
 I see that in the webcam.c gadget, the 720p and VGA MJPEG uvc formats are
 supported. I was trying the same out and got confused because the data
 arriving from a real video capture video supporting JPEG will have no fixed
 size. We will have the JPEG defined Start-of-Frame and End-of-Frame markers
 defining the boundary of the JPEG frame.
 
 But for almost all JPEG video capture devices even if we have kept a frame
 size of VGA initially, the final frame size will be a compressed version
 (with the compression depending on the nature of the scene, so a flat scene
 will have high compression and hence less frame size) of VGA and will not
 be equal to 640 * 480.
 
 So I couldn't exactly get why the dwMaxVideoFrameBufferSize is kept as
 614400 in webcam.c (see [1]).

The dwMaxVideoFrameBufferSize value must be larger than or equal to the 
largest MJPEG frame size. As I have no idea what that value is, I've kept the 
same size as for uncompressed frames, which should be big enough (and most 
probably too big).

 Can you please let me know your opinions and how you tested the UVC gadget's
 MJPEG frame format.
 
 [1] http://lxr.linux.no/linux+v3.5/drivers/usb/gadget/webcam.c#L232

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: Query regarding the support and testing of MJPEG frame type in the UVC webcam gadget

[Bhupesh SHARMA]

Hi Laurent,

Thanks for clearing this doubt..

 -Original Message-
 From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com]
 Sent: Wednesday, August 01, 2012 6:46 PM
 To: Bhupesh SHARMA
 Cc: linux-...@vger.kernel.org; linux-media@vger.kernel.org
 Subject: Re: Query regarding the support and testing of MJPEG frame
 type in the UVC webcam gadget
 
 Hi Bhupesh,
 
 On Wednesday 01 August 2012 14:26:33 Bhupesh SHARMA wrote:
  Hi Laurent,
 
  I have a query for you regarding the support and testing of MJPEG
 frame type
  in the UVC webcam gadget.
 
  I see that in the webcam.c gadget, the 720p and VGA MJPEG uvc formats
 are
  supported. I was trying the same out and got confused because the
 data
  arriving from a real video capture video supporting JPEG will have no
 fixed
  size. We will have the JPEG defined Start-of-Frame and End-of-Frame
 markers
  defining the boundary of the JPEG frame.
 
  But for almost all JPEG video capture devices even if we have kept a
 frame
  size of VGA initially, the final frame size will be a compressed
 version
  (with the compression depending on the nature of the scene, so a flat
 scene
  will have high compression and hence less frame size) of VGA and will
 not
  be equal to 640 * 480.
 
  So I couldn't exactly get why the dwMaxVideoFrameBufferSize is kept
 as
  614400 in webcam.c (see [1]).
 
 The dwMaxVideoFrameBufferSize value must be larger than or equal to the
 largest MJPEG frame size. As I have no idea what that value is, I've
 kept the
 same size as for uncompressed frames, which should be big enough (and
 most
 probably too big).

.. Yes, so that means that the user-space application should set the length
of the buffer being queued at the UVC side equal to the length of the buffer
dequeued from the V4L2 side, to ensure that varying length JPEG frames are
correctly handled.

I will try with these changes in the user-space daemon.

Thanks for your help,
Regards,
Bhupesh

  Can you please let me know your opinions and how you tested the UVC
 gadget's
  MJPEG frame format.
 
  [1] http://lxr.linux.no/linux+v3.5/drivers/usb/gadget/webcam.c#L232
 
 --
 Regards,
 
 Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

[Bhupesh SHARMA]

Hi Laurent,

Sorry for the delayed reply. I thought of performing some extensive tests
before replying to your comments.

 -Original Message-
 From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com]
 Sent: Saturday, July 07, 2012 5:28 PM
 To: Bhupesh SHARMA
 Cc: linux-...@vger.kernel.org; ba...@ti.com; linux-
 me...@vger.kernel.org; gre...@linuxfoundation.org
 Subject: Re: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use
 videobuf2 framework
 
 Hi Bhupesh,
 
 On Tuesday 03 July 2012 23:42:59 Bhupesh SHARMA wrote:
  Hi Laurent,
 
  Thanks for your review and sorry for being late with my replies.
  I have a lot on my plate these days :)
 
 No worries, I'm no less busy anyway :-)

:)

  On Tuesday, June 19, 2012 4:19 AM Laurent Pinchart wrote:
   On Friday 01 June 2012 15:08:57 Bhupesh Sharma wrote:
 
 [snip]
 
diff --git a/drivers/usb/gadget/uvc_queue.c
b/drivers/usb/gadget/uvc_queue.c
index 0cdf89d..907ece8 100644
--- a/drivers/usb/gadget/uvc_queue.c
+++ b/drivers/usb/gadget/uvc_queue.c
 
 [snip]
 
+static int uvc_buffer_prepare(struct vb2_buffer *vb)
 {
 
 [snip]
 
+   buf-state = UVC_BUF_STATE_QUEUED;
+   buf-mem = vb2_plane_vaddr(vb, 0);
+   buf-length = vb2_plane_size(vb, 0);
+   if (vb-v4l2_buf.type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
+   buf-bytesused = 0;
+   else
+   buf-bytesused = vb2_get_plane_payload(vb, 0);
  
   The driver doesn't support the capture type at the moment so this
 might be
   a bit overkill, but I think it's a good idea to support capture in
 the
   queue imeplementation. I plan to try and merge the uvcvideo and
 uvcgadget
   queue implementations at some point.
 
  I am thinking now whether we really need to support UVC as a capture
 type
  video device. The use cases that I can think of now, UVC always seems
 to be
  a output device.
 
  Any use-case where you think UVC can be a capture device?
 
 It could be useful for video output devices. I know of at least one UVC
 output
 device (albeit not Linux-based), which I used to implement and test
 video
 output in the UVC host driver. As the host driver supports video output
 devices, supporting them in the gadget driver can be useful as well.

Ok.

+   return 0;
+}
 
 [snip]
 
 static struct uvc_buffer *
 uvc_queue_next_buffer(struct uvc_video_queue *queue, struct
 uvc_buffer
*buf)
 
 [snip]
 
-   buf-buf.sequence = queue-sequence++;
-   do_gettimeofday(buf-buf.timestamp);
  
   videobuf2 doesn't fill the sequence number or timestamp fields, so
 you
   either need to keep this here or move it to the caller.
 
  Yes I think these fields are only valid for video capture devices.
  As my use-case was only an output UVC video device, I didn't add the
 same.
 
  Please let me know your views on the same.
 
 Good point. The spec isn't clear about this, so I'd rather keep these
 two
 lines for now.

Ok, sure.

+   vb2_set_plane_payload(buf-buf, 0, buf-bytesused);
+   vb2_buffer_done(buf-buf, VB2_BUF_STATE_DONE);
   
-   wake_up(buf-wait);
   
return nextbuf;
 }
 
 [snip]
 
diff --git a/drivers/usb/gadget/uvc_v4l2.c
   b/drivers/usb/gadget/uvc_v4l2.c
index f6e083b..9c2b45b 100644
--- a/drivers/usb/gadget/uvc_v4l2.c
+++ b/drivers/usb/gadget/uvc_v4l2.c
@@ -144,20 +144,23 @@ uvc_v4l2_release(struct file *file)
   
struct uvc_device *uvc = video_get_drvdata(vdev);
struct uvc_file_handle *handle = to_uvc_file_handle(file-
   
   private_data);
   
struct uvc_video *video = handle-device;
   
+   int ret;
   
uvc_function_disconnect(uvc);
   
-   uvc_video_enable(video, 0);
-   mutex_lock(video-queue.mutex);
-   if (uvc_free_buffers(video-queue)  0)
-   printk(KERN_ERR uvc_v4l2_release: Unable to free 
-   buffers.\n);
-   mutex_unlock(video-queue.mutex);
+   ret = uvc_video_enable(video, 0);
+   if (ret  0) {
+   printk(KERN_ERR uvc_v4l2_release: uvc video disable
  
   failed\n);
  
+   return ret;
+   }
  
   This shouldn't prevent uvc_v4l2_release() from succeeding. In
 practive
   uvc_video_enable(0) will never fail, so you can remove the error
 check.
 
  To be honest, I saw once the 'uvc_video_enable(0)' failing that's why
 I
  added this check. I don't remember the exact instance of the failure,
 but
  I can try to check again and then will come back on the same.
 
 The only reason I see for uvc_video_enable(video, 0) to fail is if the
 video
 endpoint hasn't been allocated. As the V4L2 device node is registered
 after
 allocating the endpoint, I'm surprised to hear that you saw it failing.
 If you
 can reproduce the problem I'd be curious to have more information.

Yes. I tested and tested again, but could not reproduce this issue.
Perhaps it was some initial incorrect test finding.

+
+   uvc_free_buffers(video-queue

Re: [Regression 3.1-3.2, bisected] UVC-webcam: kernel panic when starting capturing

[Frank Schäfer]

Am 16.07.2012 01:24, schrieb Laurent Pinchart:
 Hi Frank,

 On Sunday 15 July 2012 21:39:47 Frank Schäfer wrote:
 Am 15.07.2012 14:07, schrieb Laurent Pinchart:
 On Thursday 12 July 2012 21:07:56 Frank Schäfer wrote:
 Hi,

 when I start capturing from the UVC-webcam 2232:1005 (WebCam
 SCB-0385N) of my netbook, I get a kernel panic.
 You can find a screenshot of the backtrace here:

 http://imageshack.us/photo/my-images/9/img125km.jpg/

 This is a regression which has been introduced between kernel 3.2-rc2
 and 3.2-rc3 with the following commit:

 3afedb95858bcc117b207a7c0a6767fe891bdfe9 is the first bad commit
 commit 3afedb95858bcc117b207a7c0a6767fe891bdfe9
 Author: Laurent Pinchart laurent.pinch...@ideasonboard.com
 Date:   Thu Nov 3 07:24:34 2011 -0300

 [media] uvcvideo: Don't skip erroneous payloads
 
 Instead of skipping the payload completely, which would make the
 resulting image corrupted anyway, store the payload normally and mark
 the buffer as erroneous. If the no_drop module parameter is set to 1
 the buffer will then be passed to userspace, and tt will then be up
 to the application to decide what to do with the buffer.

 Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
 Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
 I'm puzzled. Your screenshot shows the uvc_video_stats_decode() function
 in the stack trace, but that function wasn't present in
 3afedb95858bcc117b207a7c0a6767fe891bdfe9. Could you please send me a stack
 trace corresponding to 3afedb95858bcc117b207a7c0a6767fe891bdfe9 ?

 Your stack trace looks similar to the problem reported in
 https://bugzilla.redhat.com/show_bug.cgi?id=836742.
 3afedb95858bcc117b207a7c0a6767fe891bdfe9 might have introduced a different
 bug, possibly fixed in a later commit.
 Hmm... you're right.
 The screenshot I've sent to you was made during the bisection process at
 a commit somewhere between 3.2-rc7 and 3.2-rc8.
 It seems that this one is slightly different from the others.

 This one is made at commit 3afedb95858bcc117b207a7c0a6767fe891bdfe9 (the
 first bad commit):

 http://imageshack.us/photo/my-images/811/img130hv.jpg

 and this one is made at 3.5.rc6+:

 http://imageshack.us/photo/my-images/440/img127u.jpg
 Thank you. Could you please try the patch I've attached to 
 https://bugzilla.redhat.com/show_bug.cgi?id=836742 ?


Thank you Laurent, I can confirm that this patch fixes the bug !
Don't forget to add CC-stable (and a comment that this should be applied
to all kernels =3.2 ?).

Regards,
Frank Schäfer




--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Regression 3.1-3.2, bisected] UVC-webcam: kernel panic when starting capturing

[Laurent Pinchart]

Hi Frank,

Thanks for the report.

On Thursday 12 July 2012 21:07:56 Frank Schäfer wrote:
 Hi,
 
 when I start capturing from the UVC-webcam 2232:1005 (WebCam
 SCB-0385N) of my netbook, I get a kernel panic.
 You can find a screenshot of the backtrace here:
 
 http://imageshack.us/photo/my-images/9/img125km.jpg/
 
 
 This is a regression which has been introduced between kernel 3.2-rc2
 and 3.2-rc3 with the following commit:
 
 
 3afedb95858bcc117b207a7c0a6767fe891bdfe9 is the first bad commit
 commit 3afedb95858bcc117b207a7c0a6767fe891bdfe9
 Author: Laurent Pinchart laurent.pinch...@ideasonboard.com
 Date:   Thu Nov 3 07:24:34 2011 -0300
 
 [media] uvcvideo: Don't skip erroneous payloads
 
 Instead of skipping the payload completely, which would make the
 resulting image corrupted anyway, store the payload normally and mark
 the buffer as erroneous. If the no_drop module parameter is set to 1 the
 buffer will then be passed to userspace, and tt will then be up to the
 application to decide what to do with the buffer.
 
 Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
 Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com

I'm puzzled. Your screenshot shows the uvc_video_stats_decode() function in 
the stack trace, but that function wasn't present in 
3afedb95858bcc117b207a7c0a6767fe891bdfe9. Could you please send me a stack 
trace corresponding to 3afedb95858bcc117b207a7c0a6767fe891bdfe9 ?

Your stack trace looks similar to the problem reported in 
https://bugzilla.redhat.com/show_bug.cgi?id=836742. 
3afedb95858bcc117b207a7c0a6767fe891bdfe9 might have introduced a different 
bug, possibly fixed in a later commit.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Regression 3.1-3.2, bisected] UVC-webcam: kernel panic when starting capturing

[Frank Schäfer]

Am 15.07.2012 14:07, schrieb Laurent Pinchart:
 Hi Frank,

 Thanks for the report.

 On Thursday 12 July 2012 21:07:56 Frank Schäfer wrote:
 Hi,

 when I start capturing from the UVC-webcam 2232:1005 (WebCam
 SCB-0385N) of my netbook, I get a kernel panic.
 You can find a screenshot of the backtrace here:

 http://imageshack.us/photo/my-images/9/img125km.jpg/


 This is a regression which has been introduced between kernel 3.2-rc2
 and 3.2-rc3 with the following commit:


 3afedb95858bcc117b207a7c0a6767fe891bdfe9 is the first bad commit
 commit 3afedb95858bcc117b207a7c0a6767fe891bdfe9
 Author: Laurent Pinchart laurent.pinch...@ideasonboard.com
 Date:   Thu Nov 3 07:24:34 2011 -0300

 [media] uvcvideo: Don't skip erroneous payloads

 Instead of skipping the payload completely, which would make the
 resulting image corrupted anyway, store the payload normally and mark
 the buffer as erroneous. If the no_drop module parameter is set to 1 the
 buffer will then be passed to userspace, and tt will then be up to the
 application to decide what to do with the buffer.

 Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
 Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
 I'm puzzled. Your screenshot shows the uvc_video_stats_decode() function in 
 the stack trace, but that function wasn't present in 
 3afedb95858bcc117b207a7c0a6767fe891bdfe9. Could you please send me a stack 
 trace corresponding to 3afedb95858bcc117b207a7c0a6767fe891bdfe9 ?

 Your stack trace looks similar to the problem reported in 
 https://bugzilla.redhat.com/show_bug.cgi?id=836742. 
 3afedb95858bcc117b207a7c0a6767fe891bdfe9 might have introduced a different 
 bug, possibly fixed in a later commit.
Hmm... you're right.
The screenshot I've sent to you was made during the bisection process at
a commit somewhere between 3.2-rc7 and 3.2-rc8.
It seems that this one is slightly different from the others.


This one is made at commit 3afedb95858bcc117b207a7c0a6767fe891bdfe9 (the
first bad commit):

http://imageshack.us/photo/my-images/811/img130hv.jpg


and this one is made at 3.5.rc6+:

http://imageshack.us/photo/my-images/440/img127u.jpg


Regards,
Frank Schäfer
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Regression 3.1-3.2, bisected] UVC-webcam: kernel panic when starting capturing

[Laurent Pinchart]

Hi Frank,

On Sunday 15 July 2012 21:39:47 Frank Schäfer wrote:
 Am 15.07.2012 14:07, schrieb Laurent Pinchart:
  On Thursday 12 July 2012 21:07:56 Frank Schäfer wrote:
  Hi,
  
  when I start capturing from the UVC-webcam 2232:1005 (WebCam
  SCB-0385N) of my netbook, I get a kernel panic.
  You can find a screenshot of the backtrace here:
  
  http://imageshack.us/photo/my-images/9/img125km.jpg/
  
  This is a regression which has been introduced between kernel 3.2-rc2
  and 3.2-rc3 with the following commit:
  
  3afedb95858bcc117b207a7c0a6767fe891bdfe9 is the first bad commit
  commit 3afedb95858bcc117b207a7c0a6767fe891bdfe9
  Author: Laurent Pinchart laurent.pinch...@ideasonboard.com
  Date:   Thu Nov 3 07:24:34 2011 -0300
  
  [media] uvcvideo: Don't skip erroneous payloads
  
  Instead of skipping the payload completely, which would make the
  resulting image corrupted anyway, store the payload normally and mark
  the buffer as erroneous. If the no_drop module parameter is set to 1
  the buffer will then be passed to userspace, and tt will then be up
  to the application to decide what to do with the buffer.
  
  Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
  Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
  
  I'm puzzled. Your screenshot shows the uvc_video_stats_decode() function
  in the stack trace, but that function wasn't present in
  3afedb95858bcc117b207a7c0a6767fe891bdfe9. Could you please send me a stack
  trace corresponding to 3afedb95858bcc117b207a7c0a6767fe891bdfe9 ?
  
  Your stack trace looks similar to the problem reported in
  https://bugzilla.redhat.com/show_bug.cgi?id=836742.
  3afedb95858bcc117b207a7c0a6767fe891bdfe9 might have introduced a different
  bug, possibly fixed in a later commit.
 
 Hmm... you're right.
 The screenshot I've sent to you was made during the bisection process at
 a commit somewhere between 3.2-rc7 and 3.2-rc8.
 It seems that this one is slightly different from the others.
 
 This one is made at commit 3afedb95858bcc117b207a7c0a6767fe891bdfe9 (the
 first bad commit):
 
 http://imageshack.us/photo/my-images/811/img130hv.jpg
 
 and this one is made at 3.5.rc6+:
 
 http://imageshack.us/photo/my-images/440/img127u.jpg

Thank you. Could you please try the patch I've attached to 
https://bugzilla.redhat.com/show_bug.cgi?id=836742 ?

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

[Laurent Pinchart]

Hi Bhupesh,

On Tuesday 03 July 2012 23:42:59 Bhupesh SHARMA wrote:
 Hi Laurent,
 
 Thanks for your review and sorry for being late with my replies.
 I have a lot on my plate these days :)

No worries, I'm no less busy anyway :-)

 On Tuesday, June 19, 2012 4:19 AM Laurent Pinchart wrote:
  On Friday 01 June 2012 15:08:57 Bhupesh Sharma wrote:

[snip]

   diff --git a/drivers/usb/gadget/uvc_queue.c
   b/drivers/usb/gadget/uvc_queue.c
   index 0cdf89d..907ece8 100644
   --- a/drivers/usb/gadget/uvc_queue.c
   +++ b/drivers/usb/gadget/uvc_queue.c

[snip]

   +static int uvc_buffer_prepare(struct vb2_buffer *vb)
{

[snip]

   +   buf-state = UVC_BUF_STATE_QUEUED;
   +   buf-mem = vb2_plane_vaddr(vb, 0);
   +   buf-length = vb2_plane_size(vb, 0);
   +   if (vb-v4l2_buf.type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
   +   buf-bytesused = 0;
   +   else
   +   buf-bytesused = vb2_get_plane_payload(vb, 0);
  
  The driver doesn't support the capture type at the moment so this might be
  a bit overkill, but I think it's a good idea to support capture in the
  queue imeplementation. I plan to try and merge the uvcvideo and uvcgadget
  queue implementations at some point.
 
 I am thinking now whether we really need to support UVC as a capture type
 video device. The use cases that I can think of now, UVC always seems to be
 a output device.
 
 Any use-case where you think UVC can be a capture device?

It could be useful for video output devices. I know of at least one UVC output 
device (albeit not Linux-based), which I used to implement and test video 
output in the UVC host driver. As the host driver supports video output 
devices, supporting them in the gadget driver can be useful as well.

   +   return 0;
   +}

[snip]

static struct uvc_buffer *
uvc_queue_next_buffer(struct uvc_video_queue *queue, struct uvc_buffer
   *buf)

[snip]

   -   buf-buf.sequence = queue-sequence++;
   -   do_gettimeofday(buf-buf.timestamp);
  
  videobuf2 doesn't fill the sequence number or timestamp fields, so you
  either need to keep this here or move it to the caller.
 
 Yes I think these fields are only valid for video capture devices.
 As my use-case was only an output UVC video device, I didn't add the same.
 
 Please let me know your views on the same.

Good point. The spec isn't clear about this, so I'd rather keep these two 
lines for now.

   +   vb2_set_plane_payload(buf-buf, 0, buf-bytesused);
   +   vb2_buffer_done(buf-buf, VB2_BUF_STATE_DONE);
   
   -   wake_up(buf-wait);
   
   return nextbuf;
}

[snip]

   diff --git a/drivers/usb/gadget/uvc_v4l2.c
  b/drivers/usb/gadget/uvc_v4l2.c
   index f6e083b..9c2b45b 100644
   --- a/drivers/usb/gadget/uvc_v4l2.c
   +++ b/drivers/usb/gadget/uvc_v4l2.c
   @@ -144,20 +144,23 @@ uvc_v4l2_release(struct file *file)
   
   struct uvc_device *uvc = video_get_drvdata(vdev);
   struct uvc_file_handle *handle = to_uvc_file_handle(file-
  
  private_data);
  
   struct uvc_video *video = handle-device;
   
   +   int ret;
   
   uvc_function_disconnect(uvc);
   
   -   uvc_video_enable(video, 0);
   -   mutex_lock(video-queue.mutex);
   -   if (uvc_free_buffers(video-queue)  0)
   -   printk(KERN_ERR uvc_v4l2_release: Unable to free 
   -   buffers.\n);
   -   mutex_unlock(video-queue.mutex);
   +   ret = uvc_video_enable(video, 0);
   +   if (ret  0) {
   +   printk(KERN_ERR uvc_v4l2_release: uvc video disable
  
  failed\n);
  
   +   return ret;
   +   }
  
  This shouldn't prevent uvc_v4l2_release() from succeeding. In practive
  uvc_video_enable(0) will never fail, so you can remove the error check.
 
 To be honest, I saw once the 'uvc_video_enable(0)' failing that's why I
 added this check. I don't remember the exact instance of the failure, but
 I can try to check again and then will come back on the same.

The only reason I see for uvc_video_enable(video, 0) to fail is if the video 
endpoint hasn't been allocated. As the V4L2 device node is registered after 
allocating the endpoint, I'm surprised to hear that you saw it failing. If you 
can reproduce the problem I'd be curious to have more information.

   +
   +   uvc_free_buffers(video-queue);
   
   file-private_data = NULL;
   v4l2_fh_del(handle-vfh);
   v4l2_fh_exit(handle-vfh);
   kfree(handle);
   +
   return 0;
}

[snip]

   diff --git a/drivers/usb/gadget/uvc_video.c
   b/drivers/usb/gadget/uvc_video.c
   index b0e53a8..195bbb6 100644
   --- a/drivers/usb/gadget/uvc_video.c
   +++ b/drivers/usb/gadget/uvc_video.c
  
  [snip]
  
   @@ -161,6 +161,7 @@ static void
   
uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
{
   struct uvc_video *video = req-context;
   +   struct uvc_video_queue *queue = video-queue;
   struct uvc_buffer *buf;
   unsigned long flags;
   int ret;
   @@ -169,13 +170,15 @@ uvc_video_complete(struct usb_ep *ep, struct
   usb_request *req) case 0:
 

RE: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

[Bhupesh SHARMA]

Hi Laurent,

Thanks for your review and sorry for being late with my replies.
I have a lot on my plate these days :)

 -Original Message-
 From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com]
 Sent: Tuesday, June 19, 2012 4:19 AM
 To: Bhupesh SHARMA
 Cc: linux-...@vger.kernel.org; ba...@ti.com; linux-
 me...@vger.kernel.org; gre...@linuxfoundation.org
 Subject: Re: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use
 videobuf2 framework

 Hi Bhupesh,

 Thanks for the patch. It looks quite good, please see below for various
 small
 comments.

 On Friday 01 June 2012 15:08:57 Bhupesh Sharma wrote:
  This patch reworks the videobuffer management logic present in the
 UVC
  webcam gadget and ports it to use the more apt videobuf2 framework
 for
  video buffer management.
 
  To support routing video data captured from a real V4L2 video capture
  device with a zero copy operation on videobuffers (as they pass
 from the
  V4L2 domain to UVC domain via a user-space application), we need to
 support
  USER_PTR IO method at the UVC gadget side.
 
  So the V4L2 capture device driver can still continue to use MMAO IO
 method

 s/MMAO/MMAP/

Oops. Ok.

  and now the user-space application can just pass a pointer to the
 video
  buffers being DeQueued from the V4L2 device side while Queueing them
 at the

 I don't think dequeued and queueing need capitals :-)

:)
Yes, it seems, I always end up writing them in Camel case.
Will correct this.

  UVC gadget end. This ensures that we have a zero-copy design as the
  videobuffers pass from the V4L2 capture device to the UVC gadget.
 
  Note that there will still be a need to apply UVC specific payload
 headers
  on top of each UVC payload data, which will still require a copy
 operation
  to be performed in the 'encode' routines of the UVC gadget.
 
  Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
  ---
   drivers/usb/gadget/Kconfig |1 +
   drivers/usb/gadget/uvc_queue.c |  524 ++
 ---
   drivers/usb/gadget/uvc_queue.h |   25 +--
   drivers/usb/gadget/uvc_v4l2.c  |   35 ++--
   drivers/usb/gadget/uvc_video.c |   17 +-
   5 files changed, 184 insertions(+), 418 deletions(-)

 [snip]

  diff --git a/drivers/usb/gadget/uvc_queue.c
 b/drivers/usb/gadget/uvc_queue.c
  index 0cdf89d..907ece8 100644
  --- a/drivers/usb/gadget/uvc_queue.c
  +++ b/drivers/usb/gadget/uvc_queue.c

 [snip]

 This part is a bit difficult to review, as git tried too hard to create
 a
 universal diff where your patch really replaces the code. I'll remove
 the -
 lines to make the comments as readable as possible.

  +/*
  -
 --
  --
  + * videobuf2 queue operations
*/
  +
  +static int uvc_queue_setup(struct vb2_queue *vq, const struct
 v4l2_format
  *fmt,
  +   unsigned int *nbuffers, unsigned int *nplanes,
  +   unsigned int sizes[], void *alloc_ctxs[])
   {
  +   struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
  +   struct uvc_video *video =
  +   container_of(queue, struct uvc_video, queue);

 No need for a line split.

Ok.

 
  +   if (*nbuffers  UVC_MAX_VIDEO_BUFFERS)
  +   *nbuffers = UVC_MAX_VIDEO_BUFFERS;
 
  +   *nplanes = 1;
  +
  +   sizes[0] = video-imagesize;
 
  return 0;
   }
 
  +static int uvc_buffer_prepare(struct vb2_buffer *vb)
   {
  +   struct uvc_video_queue *queue = vb2_get_drv_priv(vb-vb2_queue);
  +   struct uvc_buffer *buf = container_of(vb, struct uvc_buffer,
 buf);
 
  +   if (vb-v4l2_buf.type == V4L2_BUF_TYPE_VIDEO_OUTPUT 
  +   vb2_get_plane_payload(vb, 0)  vb2_plane_size(vb, 0))
 {

 Please align vb2 with the vb- on the previous line.

Ok.


 Have you by any chance found some inspiration in
 drivers/media/video/uvc/uvc_queue.c ? :-) It would probably make sense
 to move
 this check to vb2 core, but that's outside of the scope of this patch.


Yes :)

  +   uvc_trace(UVC_TRACE_CAPTURE, [E] Bytes used out of
 bounds.\n);
  +   return -EINVAL;
  +   }
 
  +   if (unlikely(queue-flags  UVC_QUEUE_DISCONNECTED))
  +   return -ENODEV;
 
  +   buf-state = UVC_BUF_STATE_QUEUED;
  +   buf-mem = vb2_plane_vaddr(vb, 0);
  +   buf-length = vb2_plane_size(vb, 0);
  +   if (vb-v4l2_buf.type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
  +   buf-bytesused = 0;
  +   else
  +   buf-bytesused = vb2_get_plane_payload(vb, 0);

 The driver doesn't support the capture type at the moment so this might
 be a
 bit overkill, but I think it's a good idea to support capture in the
 queue
 imeplementation. I plan to try and merge the uvcvideo and uvcgadget
 queue
 implementations at some point.

I am thinking now whether we really need to support UVC as a capture type
video device. The use cases that I can think of now, UVC always seems to be
a output device.

Any use-case where you think UVC can be a capture device?

 
  +   return

RE: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

[Bhupesh SHARMA]

Hi Laurent,

Can you please review this patch and let me know if any modifications
are required..

 -Original Message-
 From: Bhupesh SHARMA
 Sent: Friday, June 01, 2012 3:09 PM
 To: laurent.pinch...@ideasonboard.com; linux-...@vger.kernel.org
 Cc: ba...@ti.com; linux-media@vger.kernel.org;
 gre...@linuxfoundation.org; Bhupesh SHARMA
 Subject: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use
 videobuf2 framework

 This patch reworks the videobuffer management logic present in the UVC
 webcam gadget and ports it to use the more apt videobuf2 framework
 for
 video buffer management.

 To support routing video data captured from a real V4L2 video capture
 device with a zero copy operation on videobuffers (as they pass from
 the V4L2
 domain to UVC domain via a user-space application), we need to support
 USER_PTR
 IO method at the UVC gadget side.

 So the V4L2 capture device driver can still continue to use MMAO IO
 method
 and now the user-space application can just pass a pointer to the video
 buffers
 being DeQueued from the V4L2 device side while Queueing them at the UVC
 gadget
 end. This ensures that we have a zero-copy design as the videobuffers
 pass
 from the V4L2 capture device to the UVC gadget.

 Note that there will still be a need to apply UVC specific payload
 headers
 on top of each UVC payload data, which will still require a copy
 operation
 to be performed in the 'encode' routines of the UVC gadget.

 Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
 ---
  drivers/usb/gadget/Kconfig |1 +
  drivers/usb/gadget/uvc_queue.c |  524 +++-
 
  drivers/usb/gadget/uvc_queue.h |   25 +--
  drivers/usb/gadget/uvc_v4l2.c  |   35 ++--
  drivers/usb/gadget/uvc_video.c |   17 +-
  5 files changed, 184 insertions(+), 418 deletions(-)

 diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
 index 1f93861..5a351f8 100644
 --- a/drivers/usb/gadget/Kconfig
 +++ b/drivers/usb/gadget/Kconfig
 @@ -967,6 +967,7 @@ endif
  config USB_G_WEBCAM
   tristate USB Webcam Gadget
   depends on VIDEO_DEV
 + select VIDEOBUF2_VMALLOC
   help
 The Webcam Gadget acts as a composite USB Audio and Video Class
 device. It provides a userspace API to process UVC control
 requests
 diff --git a/drivers/usb/gadget/uvc_queue.c
 b/drivers/usb/gadget/uvc_queue.c
 index 0cdf89d..907ece8 100644
 --- a/drivers/usb/gadget/uvc_queue.c
 +++ b/drivers/usb/gadget/uvc_queue.c
 @@ -10,6 +10,7 @@
   *   (at your option) any later version.
   */

 +#include linux/atomic.h
  #include linux/kernel.h
  #include linux/mm.h
  #include linux/list.h
 @@ -18,7 +19,8 @@
  #include linux/videodev2.h
  #include linux/vmalloc.h
  #include linux/wait.h
 -#include linux/atomic.h
 +
 +#include media/videobuf2-vmalloc.h

  #include uvc.h

 @@ -28,271 +30,156 @@
   * Video queues is initialized by uvc_queue_init(). The function
 performs
   * basic initialization of the uvc_video_queue struct and never fails.
   *
 - * Video buffer allocation and freeing are performed by
 uvc_alloc_buffers and
 - * uvc_free_buffers respectively. The former acquires the video queue
 lock,
 - * while the later must be called with the lock held (so that
 allocation can
 - * free previously allocated buffers). Trying to free buffers that are
 mapped
 - * to user space will return -EBUSY.
 - *
 - * Video buffers are managed using two queues. However, unlike most
 USB video
 - * drivers that use an in queue and an out queue, we use a main queue
 to hold
 - * all queued buffers (both 'empty' and 'done' buffers), and an irq
 queue to
 - * hold empty buffers. This design (copied from video-buf) minimizes
 locking
 - * in interrupt, as only one queue is shared between interrupt and
 user
 - * contexts.
 - *
 - * Use cases
 - * -
 - *
 - * Unless stated otherwise, all operations that modify the irq buffers
 queue
 - * are protected by the irq spinlock.
 - *
 - * 1. The user queues the buffers, starts streaming and dequeues a
 buffer.
 - *
 - *The buffers are added to the main and irq queues. Both
 operations are
 - *protected by the queue lock, and the later is protected by the
 irq
 - *spinlock as well.
 - *
 - *The completion handler fetches a buffer from the irq queue and
 fills it
 - *with video data. If no buffer is available (irq queue empty),
 the handler
 - *returns immediately.
 - *
 - *When the buffer is full, the completion handler removes it from
 the irq
 - *queue, marks it as ready (UVC_BUF_STATE_DONE) and wakes its wait
 queue.
 - *At that point, any process waiting on the buffer will be woken
 up. If a
 - *process tries to dequeue a buffer after it has been marked
 ready, the
 - *dequeing will succeed immediately.
 - *
 - * 2. Buffers are queued, user is waiting on a buffer and the device
 gets
 - *disconnected.
 - *
 - *When the device is disconnected, the kernel calls the completion
 handler

Re: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

[Laurent Pinchart]

Hi Bhupesh,

Thanks for the patch. It looks quite good, please see below for various small 
comments.

On Friday 01 June 2012 15:08:57 Bhupesh Sharma wrote:
 This patch reworks the videobuffer management logic present in the UVC
 webcam gadget and ports it to use the more apt videobuf2 framework for
 video buffer management.
 
 To support routing video data captured from a real V4L2 video capture
 device with a zero copy operation on videobuffers (as they pass from the
 V4L2 domain to UVC domain via a user-space application), we need to support
 USER_PTR IO method at the UVC gadget side.
 
 So the V4L2 capture device driver can still continue to use MMAO IO method

s/MMAO/MMAP/

 and now the user-space application can just pass a pointer to the video
 buffers being DeQueued from the V4L2 device side while Queueing them at the

I don't think dequeued and queueing need capitals :-)

 UVC gadget end. This ensures that we have a zero-copy design as the
 videobuffers pass from the V4L2 capture device to the UVC gadget.
 
 Note that there will still be a need to apply UVC specific payload headers
 on top of each UVC payload data, which will still require a copy operation
 to be performed in the 'encode' routines of the UVC gadget.
 
 Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
 ---
  drivers/usb/gadget/Kconfig |1 +
  drivers/usb/gadget/uvc_queue.c |  524 ++---
  drivers/usb/gadget/uvc_queue.h |   25 +--
  drivers/usb/gadget/uvc_v4l2.c  |   35 ++--
  drivers/usb/gadget/uvc_video.c |   17 +-
  5 files changed, 184 insertions(+), 418 deletions(-)

[snip]

 diff --git a/drivers/usb/gadget/uvc_queue.c b/drivers/usb/gadget/uvc_queue.c
 index 0cdf89d..907ece8 100644
 --- a/drivers/usb/gadget/uvc_queue.c
 +++ b/drivers/usb/gadget/uvc_queue.c

[snip]

This part is a bit difficult to review, as git tried too hard to create a 
universal diff where your patch really replaces the code. I'll remove the - 
lines to make the comments as readable as possible.

 +/*
 ---
 --
 + * videobuf2 queue operations
   */
 +
 +static int uvc_queue_setup(struct vb2_queue *vq, const struct v4l2_format
 *fmt,
 + unsigned int *nbuffers, unsigned int *nplanes,
 + unsigned int sizes[], void *alloc_ctxs[])
  {
 + struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
 + struct uvc_video *video =
 + container_of(queue, struct uvc_video, queue);

No need for a line split.

 
 + if (*nbuffers  UVC_MAX_VIDEO_BUFFERS)
 + *nbuffers = UVC_MAX_VIDEO_BUFFERS;
 
 + *nplanes = 1;
 +
 + sizes[0] = video-imagesize;
 
   return 0;
  }
 
 +static int uvc_buffer_prepare(struct vb2_buffer *vb)
  {
 + struct uvc_video_queue *queue = vb2_get_drv_priv(vb-vb2_queue);
 + struct uvc_buffer *buf = container_of(vb, struct uvc_buffer, buf);
 
 + if (vb-v4l2_buf.type == V4L2_BUF_TYPE_VIDEO_OUTPUT 
 + vb2_get_plane_payload(vb, 0)  vb2_plane_size(vb, 0)) {

Please align vb2 with the vb- on the previous line.

Have you by any chance found some inspiration in 
drivers/media/video/uvc/uvc_queue.c ? :-) It would probably make sense to move 
this check to vb2 core, but that's outside of the scope of this patch.

 + uvc_trace(UVC_TRACE_CAPTURE, [E] Bytes used out of bounds.\n);
 + return -EINVAL;
 + }
 
 + if (unlikely(queue-flags  UVC_QUEUE_DISCONNECTED))
 + return -ENODEV;
 
 + buf-state = UVC_BUF_STATE_QUEUED;
 + buf-mem = vb2_plane_vaddr(vb, 0);
 + buf-length = vb2_plane_size(vb, 0);
 + if (vb-v4l2_buf.type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
 + buf-bytesused = 0;
 + else
 + buf-bytesused = vb2_get_plane_payload(vb, 0);

The driver doesn't support the capture type at the moment so this might be a 
bit overkill, but I think it's a good idea to support capture in the queue 
imeplementation. I plan to try and merge the uvcvideo and uvcgadget queue 
implementations at some point.

 
 + return 0;
 +}
 
 +static void uvc_buffer_queue(struct vb2_buffer *vb)
 +{
 + struct uvc_video_queue *queue = vb2_get_drv_priv(vb-vb2_queue);
 + struct uvc_buffer *buf = container_of(vb, struct uvc_buffer, buf);
 + unsigned long flags;
 
 + spin_lock_irqsave(queue-irqlock, flags);
 
 + if (likely(!(queue-flags  UVC_QUEUE_DISCONNECTED))) {
 + list_add_tail(buf-queue, queue-irqqueue);
 + } else {
 + /* If the device is disconnected return the buffer to userspace
 +  * directly. The next QBUF call will fail with -ENODEV.
 +  */
 + buf-state = UVC_BUF_STATE_ERROR;
 + vb2_buffer_done(buf-buf, VB2_BUF_STATE_ERROR);
   }
 
 + spin_unlock_irqrestore(queue-irqlock, flags);
  }
 
 +static struct vb2_ops uvc_queue_qops = {
 + .queue_setup = uvc_queue_setup

RE: [PATCH 3/5] usb: gadget/uvc: Add super-speed support to UVC webcam gadget

[Bhupesh SHARMA]

Hi Laurent,

 -Original Message-
 From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com]
 Sent: Monday, June 11, 2012 12:55 PM
 To: Bhupesh SHARMA
 Cc: linux-...@vger.kernel.org; ba...@ti.com; linux-
 me...@vger.kernel.org
 Subject: Re: [PATCH 3/5] usb: gadget/uvc: Add super-speed support to
 UVC webcam gadget
 
 Hi Bhupesh,
 
 (Dropping Greg from the CC list, I think he gets enough e-mails already
 without our help :-))

Ofcourse :)

 On Saturday 09 June 2012 13:39:34 Bhupesh SHARMA wrote:
  Hi Laurent,
 
  Thanks for your review comments.
 
  Please find my comments inline:
   As Felipe has already applied the patch to his public tree, I'll
 send
   incremental cleanup patches. Here's my review nonetheless, with a
 question
   which I'd like to know your opinion about to write the cleanup
 patches.
 
  Not to worry, I can send incremental patches.
 
 I've already prepared incremental patches so I'll send them.

Ok.

  Anyways I need to ensure 4/5 and 5/5 patches of the series also
 cleanly
  apply on Felipe's tree as he was facing issues while applying these
  patches.
 
  Please review 4/5 and 5/5 patches of this patch-set as well and then
 I can
  re-circulate patches for re-review and incorporation in gadget-next.
 
 I will review 4/5 and 5/5 and ask you to post a new version. I'll send
 incremental patches for 1/5 to 3/5 myself.

Sure.

 
   On Friday 01 June 2012 15:08:56 Bhupesh Sharma wrote:
This patch adds super-speed support to UVC webcam gadget.
   
Also in this patch:
- We add the configurability to pass bInterval, bMaxBurst,
 mult
  factors for video streaming endpoint (ISOC IN) through
 module
  parameters.
   
- We use config_ep_by_speed helper routine to configure video
  streaming endpoint.
   
Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
 
 [snip]
 
+static unsigned streaming_interval = 1;
+module_param(streaming_interval, uint, S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(streaming_interval, 1 - 16);
+
+static unsigned streaming_maxpacket = 1024;
  
   unsigned int please.
 
  Ok.
 
+module_param(streaming_maxpacket, uint, S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(streaming_maxpacket, 0 - 1023 (fs), 0 - 1024
(hs/ss));
+
+static unsigned streaming_mult;
+module_param(streaming_mult, uint, S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(streaming_mult, 0 - 2 (hs/ss only));
  
   I'd rather like to integrate this into the streaming_maxpacket
 parameter,
   and compute the multiplier at runtime. This shouldn't be difficult
 for
   high speed, the multiplier for max packet sizes between 1 and 1024
 is 1,
   between 1025 and 2048 is 2 and between 2049 and 3072 is 3.
 
  There is a specific purpose for keeping these three module parameters
  separate, with xHCI hosts and USB3 device-side controllers still in
  stabilization phase, it is easy for a person switching from USB2.0 to
  USB3.0 to understand these module parameters as the major difference
 points
  in respect to ISOC IN endpoints.
 
  A similar methodology is already used in the reference USB gadget
 zero
  (see here [1]) and I have tried to keep the same methodology here as
 well.
 
  [1]
 
 http://git.kernel.org/?p=linux/kernel/git/balbi/usb.git;a=blob;f=driver
 s/us
  b/gadget/f_sourcesink.c
 
 Having another driver implementing the same doesn't automatically make
 it
 right ;-)
 
 I still think the maxpacket and mult values should be combined into a
 single
 parameter. There's a single way to split a given number of bytes into
 maxpacket and mult values. Felipe (and others), any opinion on this ?

Ok. I will ask Felipe and others to pitch in as well before we
finalize our approach.

 
+static unsigned streaming_maxburst;
+module_param(streaming_maxburst, uint, S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(streaming_maxburst, 0 - 15 (ss only));
  
   Do you think maxburst could also be integrated into the
   streaming_maxpacket parameter ? Put it another way, can we computer
 the
   multiplier and the burst value from a single maximum number of
 bytes per
   service interval, or do they have to be specified independently ?
 If using
   more than one burst, the wMaxPacketSize value must be 1024 for HS.
 Only
   multiples of 1024 higher than 1024 can thus be achieved through
 different
   multipler/burst settings.
 
  Pls see above..
 
 I'll keep this parameter separate. When maxburst is non-zero the
 maxpacket
 value must be a multiple of 1024. If the user-specified value is
 incorrect the
 driver should error out.
 
 I forgot to mention that S_IWUSR looks unsafe to me. If we allow
 changing the
 mackpacket, mult and maxburst values at runtime, the function could be
 bound
 after one of the values have been changed but not the others.
 
 [snip]
 
+
+/* super speed support */
+static struct usb_endpoint_descriptor uvc_ss_control_ep
 __initdata =
   {
+   .bLength =  USB_DT_ENDPOINT_SIZE

Re: [PATCH 3/5] usb: gadget/uvc: Add super-speed support to UVC webcam gadget

[Laurent Pinchart]

Hi Bhupesh,

(Dropping Greg from the CC list, I think he gets enough e-mails already 
without our help :-))

On Saturday 09 June 2012 13:39:34 Bhupesh SHARMA wrote:
 Hi Laurent,
 
 Thanks for your review comments.
 
 Please find my comments inline:
  As Felipe has already applied the patch to his public tree, I'll send
  incremental cleanup patches. Here's my review nonetheless, with a question
  which I'd like to know your opinion about to write the cleanup patches.
 
 Not to worry, I can send incremental patches.

I've already prepared incremental patches so I'll send them.

 Anyways I need to ensure 4/5 and 5/5 patches of the series also cleanly
 apply on Felipe's tree as he was facing issues while applying these
 patches.
 
 Please review 4/5 and 5/5 patches of this patch-set as well and then I can
 re-circulate patches for re-review and incorporation in gadget-next.

I will review 4/5 and 5/5 and ask you to post a new version. I'll send 
incremental patches for 1/5 to 3/5 myself.

  On Friday 01 June 2012 15:08:56 Bhupesh Sharma wrote:
   This patch adds super-speed support to UVC webcam gadget.
   
   Also in this patch:
   - We add the configurability to pass bInterval, bMaxBurst, mult
 factors for video streaming endpoint (ISOC IN) through module
 parameters.
   
   - We use config_ep_by_speed helper routine to configure video
 streaming endpoint.
   
   Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com

[snip]

   +static unsigned streaming_interval = 1;
   +module_param(streaming_interval, uint, S_IRUGO|S_IWUSR);
   +MODULE_PARM_DESC(streaming_interval, 1 - 16);
   +
   +static unsigned streaming_maxpacket = 1024;
  
  unsigned int please.
 
 Ok.
 
   +module_param(streaming_maxpacket, uint, S_IRUGO|S_IWUSR);
   +MODULE_PARM_DESC(streaming_maxpacket, 0 - 1023 (fs), 0 - 1024
   (hs/ss));
   +
   +static unsigned streaming_mult;
   +module_param(streaming_mult, uint, S_IRUGO|S_IWUSR);
   +MODULE_PARM_DESC(streaming_mult, 0 - 2 (hs/ss only));
  
  I'd rather like to integrate this into the streaming_maxpacket parameter,
  and compute the multiplier at runtime. This shouldn't be difficult for
  high speed, the multiplier for max packet sizes between 1 and 1024 is 1,
  between 1025 and 2048 is 2 and between 2049 and 3072 is 3.
 
 There is a specific purpose for keeping these three module parameters
 separate, with xHCI hosts and USB3 device-side controllers still in
 stabilization phase, it is easy for a person switching from USB2.0 to
 USB3.0 to understand these module parameters as the major difference points
 in respect to ISOC IN endpoints.
 
 A similar methodology is already used in the reference USB gadget zero
 (see here [1]) and I have tried to keep the same methodology here as well.
 
 [1]
 http://git.kernel.org/?p=linux/kernel/git/balbi/usb.git;a=blob;f=drivers/us
 b/gadget/f_sourcesink.c

Having another driver implementing the same doesn't automatically make it 
right ;-)

I still think the maxpacket and mult values should be combined into a single 
parameter. There's a single way to split a given number of bytes into 
maxpacket and mult values. Felipe (and others), any opinion on this ?

   +static unsigned streaming_maxburst;
   +module_param(streaming_maxburst, uint, S_IRUGO|S_IWUSR);
   +MODULE_PARM_DESC(streaming_maxburst, 0 - 15 (ss only));
  
  Do you think maxburst could also be integrated into the
  streaming_maxpacket parameter ? Put it another way, can we computer the
  multiplier and the burst value from a single maximum number of bytes per
  service interval, or do they have to be specified independently ? If using
  more than one burst, the wMaxPacketSize value must be 1024 for HS. Only
  multiples of 1024 higher than 1024 can thus be achieved through different
  multipler/burst settings.
 
 Pls see above..

I'll keep this parameter separate. When maxburst is non-zero the maxpacket 
value must be a multiple of 1024. If the user-specified value is incorrect the 
driver should error out.

I forgot to mention that S_IWUSR looks unsafe to me. If we allow changing the 
mackpacket, mult and maxburst values at runtime, the function could be bound 
after one of the values have been changed but not the others.

[snip]

   +
   +/* super speed support */
   +static struct usb_endpoint_descriptor uvc_ss_control_ep __initdata =
  {
   +   .bLength =  USB_DT_ENDPOINT_SIZE,
   +   .bDescriptorType =  USB_DT_ENDPOINT,
   +
   +   .bEndpointAddress = USB_DIR_IN,
   +   .bmAttributes = USB_ENDPOINT_XFER_INT,
   +   .wMaxPacketSize =   cpu_to_le16(STATUS_BYTECOUNT),
   +   .bInterval =8,
   +};
  
  The FS/HS/SS control endpoint descriptors are identical, there's no need
  to define separate descriptors. You also don't set the SS endpoint number
  to the FS endpoint number. As you don't call usb_ep_autoconfig() on the SS
  endpoint, I doubt this will work in SS. Have you tested SS support ?
 
 Yes. I have tested

Re: [PATCH 3/5] usb: gadget/uvc: Add super-speed support to UVC webcam gadget

[Laurent Pinchart]

Hi Bhupesh,

Thanks for the patch.

As Felipe has already applied the patch to his public tree, I'll send 
incremental cleanup patches. Here's my review nonetheless, with a question 
which I'd like to know your opinion about to write the cleanup patches.

On Friday 01 June 2012 15:08:56 Bhupesh Sharma wrote:
 This patch adds super-speed support to UVC webcam gadget.
 
 Also in this patch:
   - We add the configurability to pass bInterval, bMaxBurst, mult
 factors for video streaming endpoint (ISOC IN) through module
 parameters.
 
   - We use config_ep_by_speed helper routine to configure video
 streaming endpoint.
 
 Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
 ---
  drivers/usb/gadget/f_uvc.c  |  241 +++-
  drivers/usb/gadget/f_uvc.h  |8 +-
  drivers/usb/gadget/uvc.h|4 +-
  drivers/usb/gadget/webcam.c |   29 +-
  4 files changed, 247 insertions(+), 35 deletions(-)
 
 diff --git a/drivers/usb/gadget/f_uvc.c b/drivers/usb/gadget/f_uvc.c
 index dd7d7a9..2a8bf06 100644
 --- a/drivers/usb/gadget/f_uvc.c
 +++ b/drivers/usb/gadget/f_uvc.c
 @@ -29,6 +29,25 @@
 
  unsigned int uvc_gadget_trace_param;
 
 +/*-
 */ +
 +/* module parameters specific to the Video streaming endpoint */
 +static unsigned streaming_interval = 1;
 +module_param(streaming_interval, uint, S_IRUGO|S_IWUSR);
 +MODULE_PARM_DESC(streaming_interval, 1 - 16);
 +
 +static unsigned streaming_maxpacket = 1024;

unsigned int please.

 +module_param(streaming_maxpacket, uint, S_IRUGO|S_IWUSR);
 +MODULE_PARM_DESC(streaming_maxpacket, 0 - 1023 (fs), 0 - 1024 (hs/ss));
 +
 +static unsigned streaming_mult;
 +module_param(streaming_mult, uint, S_IRUGO|S_IWUSR);
 +MODULE_PARM_DESC(streaming_mult, 0 - 2 (hs/ss only));

I'd rather like to integrate this into the streaming_maxpacket parameter, and 
compute the multiplier at runtime. This shouldn't be difficult for high speed, 
the multiplier for max packet sizes between 1 and 1024 is 1, between 1025 and 
2048 is 2 and between 2049 and 3072 is 3.

 +static unsigned streaming_maxburst;
 +module_param(streaming_maxburst, uint, S_IRUGO|S_IWUSR);
 +MODULE_PARM_DESC(streaming_maxburst, 0 - 15 (ss only));

Do you think maxburst could also be integrated into the streaming_maxpacket 
parameter ? Put it another way, can we computer the multiplier and the burst 
value from a single maximum number of bytes per service interval, or do they 
have to be specified independently ? If using more than one burst, the 
wMaxPacketSize value must be 1024 for HS. Only multiples of 1024 higher than 
1024 can thus be achieved through different multipler/burst settings.

 +
  /*
 --
 * Function descriptors
   */
 @@ -84,7 +103,7 @@ static struct usb_interface_descriptor uvc_control_intf
 __initdata = { .iInterface= 0,
  };
 
 -static struct usb_endpoint_descriptor uvc_control_ep __initdata = {
 +static struct usb_endpoint_descriptor uvc_fs_control_ep __initdata = {
   .bLength= USB_DT_ENDPOINT_SIZE,
   .bDescriptorType= USB_DT_ENDPOINT,
   .bEndpointAddress   = USB_DIR_IN,
 @@ -124,7 +143,7 @@ static struct usb_interface_descriptor
 uvc_streaming_intf_alt1 __initdata = { .iInterface= 0,
  };
 
 -static struct usb_endpoint_descriptor uvc_streaming_ep = {
 +static struct usb_endpoint_descriptor uvc_fs_streaming_ep = {
   .bLength= USB_DT_ENDPOINT_SIZE,
   .bDescriptorType= USB_DT_ENDPOINT,
   .bEndpointAddress   = USB_DIR_IN,
 @@ -133,15 +152,72 @@ static struct usb_endpoint_descriptor uvc_streaming_ep
 = { .bInterval= 1,
  };
 
 +static struct usb_endpoint_descriptor uvc_hs_streaming_ep = {
 + .bLength= USB_DT_ENDPOINT_SIZE,
 + .bDescriptorType= USB_DT_ENDPOINT,
 + .bEndpointAddress   = USB_DIR_IN,
 + .bmAttributes   = USB_ENDPOINT_XFER_ISOC,
 + .wMaxPacketSize = cpu_to_le16(1024),
 + .bInterval  = 1,

wMaxPacketSize and bInterval are now initialized from module parameters, so 
I'd leave it to zero and add a comment about it.

 +};

Please keep the indentation style consistent with the rest of the file.

 +
 +/* super speed support */
 +static struct usb_endpoint_descriptor uvc_ss_control_ep __initdata = {
 + .bLength =  USB_DT_ENDPOINT_SIZE,
 + .bDescriptorType =  USB_DT_ENDPOINT,
 +
 + .bEndpointAddress = USB_DIR_IN,
 + .bmAttributes = USB_ENDPOINT_XFER_INT,
 + .wMaxPacketSize =   cpu_to_le16(STATUS_BYTECOUNT),
 + .bInterval =8,
 +};

The FS/HS/SS control endpoint descriptors are identical, there's no need to 
define separate descriptors. You also don't set the SS endpoint number to the 
FS endpoint number. As you don't call

RE: [PATCH 3/5] usb: gadget/uvc: Add super-speed support to UVC webcam gadget

[Bhupesh SHARMA]

Hi Laurent,

Thanks for your review comments.
Please find my comments inline:

 As Felipe has already applied the patch to his public tree, I'll send
 incremental cleanup patches. Here's my review nonetheless, with a
 question
 which I'd like to know your opinion about to write the cleanup patches.

Not to worry, I can send incremental patches.
Anyways I need to ensure 4/5 and 5/5 patches of the series also cleanly apply
on Felipe's tree as he was facing issues while applying these patches.

Please review 4/5 and 5/5 patches of this patch-set as well and then I can
re-circulate patches for re-review and incorporation in gadget-next.

 On Friday 01 June 2012 15:08:56 Bhupesh Sharma wrote:
  This patch adds super-speed support to UVC webcam gadget.
 
  Also in this patch:
  - We add the configurability to pass bInterval, bMaxBurst, mult
factors for video streaming endpoint (ISOC IN) through module
parameters.
 
  - We use config_ep_by_speed helper routine to configure video
streaming endpoint.
 
  Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
  ---
   drivers/usb/gadget/f_uvc.c  |  241
 +++-
   drivers/usb/gadget/f_uvc.h  |8 +-
   drivers/usb/gadget/uvc.h|4 +-
   drivers/usb/gadget/webcam.c |   29 +-
   4 files changed, 247 insertions(+), 35 deletions(-)
 
  diff --git a/drivers/usb/gadget/f_uvc.c b/drivers/usb/gadget/f_uvc.c
  index dd7d7a9..2a8bf06 100644
  --- a/drivers/usb/gadget/f_uvc.c
  +++ b/drivers/usb/gadget/f_uvc.c
  @@ -29,6 +29,25 @@
 
   unsigned int uvc_gadget_trace_param;
 
  +/*--
 ---
  */ +
  +/* module parameters specific to the Video streaming endpoint */
  +static unsigned streaming_interval = 1;
  +module_param(streaming_interval, uint, S_IRUGO|S_IWUSR);
  +MODULE_PARM_DESC(streaming_interval, 1 - 16);
  +
  +static unsigned streaming_maxpacket = 1024;

 unsigned int please.

Ok.

  +module_param(streaming_maxpacket, uint, S_IRUGO|S_IWUSR);
  +MODULE_PARM_DESC(streaming_maxpacket, 0 - 1023 (fs), 0 - 1024
 (hs/ss));
  +
  +static unsigned streaming_mult;
  +module_param(streaming_mult, uint, S_IRUGO|S_IWUSR);
  +MODULE_PARM_DESC(streaming_mult, 0 - 2 (hs/ss only));

 I'd rather like to integrate this into the streaming_maxpacket
 parameter, and
 compute the multiplier at runtime. This shouldn't be difficult for high
 speed,
 the multiplier for max packet sizes between 1 and 1024 is 1, between
 1025 and
 2048 is 2 and between 2049 and 3072 is 3.

There is a specific purpose for keeping these three module parameters separate,
with xHCI hosts and USB3 device-side controllers still in stabilization phase, 
it
is easy for a person switching from USB2.0 to USB3.0 to understand these module
parameters as the major difference points in respect to ISOC IN endpoints.

A similar methodology is already used in the reference USB gadget zero (see 
here [1])
and I have tried to keep the same methodology here as well.

[1] 
http://git.kernel.org/?p=linux/kernel/git/balbi/usb.git;a=blob;f=drivers/usb/gadget/f_sourcesink.c

  +static unsigned streaming_maxburst;
  +module_param(streaming_maxburst, uint, S_IRUGO|S_IWUSR);
  +MODULE_PARM_DESC(streaming_maxburst, 0 - 15 (ss only));

 Do you think maxburst could also be integrated into the
 streaming_maxpacket
 parameter ? Put it another way, can we computer the multiplier and the
 burst
 value from a single maximum number of bytes per service interval, or do
 they
 have to be specified independently ? If using more than one burst, the
 wMaxPacketSize value must be 1024 for HS. Only multiples of 1024 higher
 than
 1024 can thus be achieved through different multipler/burst settings.

Pls see above..

  +
   /*
  -
 -
  * Function descriptors
*/
  @@ -84,7 +103,7 @@ static struct usb_interface_descriptor
 uvc_control_intf
  __initdata = { .iInterface  = 0,
   };
 
  -static struct usb_endpoint_descriptor uvc_control_ep __initdata = {
  +static struct usb_endpoint_descriptor uvc_fs_control_ep __initdata =
 {
  .bLength= USB_DT_ENDPOINT_SIZE,
  .bDescriptorType= USB_DT_ENDPOINT,
  .bEndpointAddress   = USB_DIR_IN,
  @@ -124,7 +143,7 @@ static struct usb_interface_descriptor
  uvc_streaming_intf_alt1 __initdata = { .iInterface  = 0,
   };
 
  -static struct usb_endpoint_descriptor uvc_streaming_ep = {
  +static struct usb_endpoint_descriptor uvc_fs_streaming_ep = {
  .bLength= USB_DT_ENDPOINT_SIZE,
  .bDescriptorType= USB_DT_ENDPOINT,
  .bEndpointAddress   = USB_DIR_IN,
  @@ -133,15 +152,72 @@ static struct usb_endpoint_descriptor
 uvc_streaming_ep
  = { .bInterval  = 1,
   };
 
  +static struct usb_endpoint_descriptor uvc_hs_streaming_ep = {
  +   .bLength= USB_DT_ENDPOINT_SIZE,
  +   .bDescriptorType

RE: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

[Bhupesh SHARMA]

Hi Felipe,

 -Original Message-
 From: Felipe Balbi [mailto:ba...@ti.com]
 Sent: Monday, June 04, 2012 8:44 PM
 To: Bhupesh SHARMA
 Cc: laurent.pinch...@ideasonboard.com; linux-...@vger.kernel.org;
 ba...@ti.com; linux-media@vger.kernel.org; gre...@linuxfoundation.org
 Subject: Re: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use
 videobuf2 framework
 
 On Fri, Jun 01, 2012 at 03:08:57PM +0530, Bhupesh Sharma wrote:
  This patch reworks the videobuffer management logic present in the
 UVC
  webcam gadget and ports it to use the more apt videobuf2 framework
  for video buffer management.
 
  To support routing video data captured from a real V4L2 video capture
  device with a zero copy operation on videobuffers (as they pass
 from
  the V4L2 domain to UVC domain via a user-space application), we need
  to support USER_PTR IO method at the UVC gadget side.
 
  So the V4L2 capture device driver can still continue to use MMAO IO
  method and now the user-space application can just pass a pointer to
  the video buffers being DeQueued from the V4L2 device side while
  Queueing them at the UVC gadget end. This ensures that we have a
  zero-copy design as the videobuffers pass from the V4L2 capture
 device to the UVC gadget.
 
  Note that there will still be a need to apply UVC specific payload
  headers on top of each UVC payload data, which will still require a
  copy operation to be performed in the 'encode' routines of the UVC
 gadget.
 
  Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
 
 this patch doesn't apply. Please refresh on top of v3.5-rc1 or my
 gadget branch which I will update in a while.
 

I rebased and submitted my changes on your gadget-for-v3.5 tag.
Should I now refresh my patches on top of your v3.5-rc1 branch ?

I am a bit confused on what is the latest gadget branch to be used now.
Thanks for helping out.

Regards,
Bhupesh
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

[Felipe Balbi]

On Fri, Jun 01, 2012 at 03:08:57PM +0530, Bhupesh Sharma wrote:
 This patch reworks the videobuffer management logic present in the UVC
 webcam gadget and ports it to use the more apt videobuf2 framework for
 video buffer management.
 
 To support routing video data captured from a real V4L2 video capture
 device with a zero copy operation on videobuffers (as they pass from the 
 V4L2
 domain to UVC domain via a user-space application), we need to support 
 USER_PTR
 IO method at the UVC gadget side.
 
 So the V4L2 capture device driver can still continue to use MMAO IO method
 and now the user-space application can just pass a pointer to the video 
 buffers
 being DeQueued from the V4L2 device side while Queueing them at the UVC gadget
 end. This ensures that we have a zero-copy design as the videobuffers pass
 from the V4L2 capture device to the UVC gadget.
 
 Note that there will still be a need to apply UVC specific payload headers
 on top of each UVC payload data, which will still require a copy operation
 to be performed in the 'encode' routines of the UVC gadget.
 
 Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com

this patch doesn't apply. Please refresh on top of v3.5-rc1 or my gadget
branch which I will update in a while.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

[Felipe Balbi]

On Mon, Jun 04, 2012 at 11:21:13PM +0800, Bhupesh SHARMA wrote:
 Hi Felipe,
 
  -Original Message-
  From: Felipe Balbi [mailto:ba...@ti.com]
  Sent: Monday, June 04, 2012 8:44 PM
  To: Bhupesh SHARMA
  Cc: laurent.pinch...@ideasonboard.com; linux-...@vger.kernel.org;
  ba...@ti.com; linux-media@vger.kernel.org; gre...@linuxfoundation.org
  Subject: Re: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use
  videobuf2 framework
  
  On Fri, Jun 01, 2012 at 03:08:57PM +0530, Bhupesh Sharma wrote:
   This patch reworks the videobuffer management logic present in the
  UVC
   webcam gadget and ports it to use the more apt videobuf2 framework
   for video buffer management.
  
   To support routing video data captured from a real V4L2 video capture
   device with a zero copy operation on videobuffers (as they pass
  from
   the V4L2 domain to UVC domain via a user-space application), we need
   to support USER_PTR IO method at the UVC gadget side.
  
   So the V4L2 capture device driver can still continue to use MMAO IO
   method and now the user-space application can just pass a pointer to
   the video buffers being DeQueued from the V4L2 device side while
   Queueing them at the UVC gadget end. This ensures that we have a
   zero-copy design as the videobuffers pass from the V4L2 capture
  device to the UVC gadget.
  
   Note that there will still be a need to apply UVC specific payload
   headers on top of each UVC payload data, which will still require a
   copy operation to be performed in the 'encode' routines of the UVC
  gadget.
  
   Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
  
  this patch doesn't apply. Please refresh on top of v3.5-rc1 or my
  gadget branch which I will update in a while.
  
 
 I rebased and submitted my changes on your gadget-for-v3.5 tag.
 Should I now refresh my patches on top of your v3.5-rc1 branch ?
 
 I am a bit confused on what is the latest gadget branch to be used now.
 Thanks for helping out.

The gadget branch is the branch called gadget on my kernel.org tree. For
some reason this didn't apply. Probably some patches on
drivers/usb/gadget/*uvc* went into v3.5 without my knowledge. Possibly
because I was out for quite a while and asked Greg to help me out during
the merge window.

Anyway, I just pushed gadget with a bunch of new patches and part of
your series.

-- 
balbi


signature.asc
Description: Digital signature

RE: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

[Bhupesh SHARMA]

 -Original Message-
 From: Felipe Balbi [mailto:ba...@ti.com]
 Sent: Monday, June 04, 2012 8:59 PM
 To: Bhupesh SHARMA
 Cc: ba...@ti.com; laurent.pinch...@ideasonboard.com; linux-
 u...@vger.kernel.org; linux-media@vger.kernel.org;
 gre...@linuxfoundation.org
 Subject: Re: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use
 videobuf2 framework
 
 On Mon, Jun 04, 2012 at 11:21:13PM +0800, Bhupesh SHARMA wrote:
  Hi Felipe,
 
   -Original Message-
   From: Felipe Balbi [mailto:ba...@ti.com]
   Sent: Monday, June 04, 2012 8:44 PM
   To: Bhupesh SHARMA
   Cc: laurent.pinch...@ideasonboard.com; linux-...@vger.kernel.org;
   ba...@ti.com; linux-media@vger.kernel.org;
   gre...@linuxfoundation.org
   Subject: Re: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to
   use
   videobuf2 framework
  
   On Fri, Jun 01, 2012 at 03:08:57PM +0530, Bhupesh Sharma wrote:
This patch reworks the videobuffer management logic present in
 the
   UVC
webcam gadget and ports it to use the more apt videobuf2
framework for video buffer management.
   
To support routing video data captured from a real V4L2 video
capture device with a zero copy operation on videobuffers (as
they pass
   from
the V4L2 domain to UVC domain via a user-space application), we
need to support USER_PTR IO method at the UVC gadget side.
   
So the V4L2 capture device driver can still continue to use MMAO
IO method and now the user-space application can just pass a
pointer to the video buffers being DeQueued from the V4L2 device
side while Queueing them at the UVC gadget end. This ensures that
we have a zero-copy design as the videobuffers pass from the
V4L2 capture
   device to the UVC gadget.
   
Note that there will still be a need to apply UVC specific
 payload
headers on top of each UVC payload data, which will still require
a copy operation to be performed in the 'encode' routines of the
UVC
   gadget.
   
Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
  
   this patch doesn't apply. Please refresh on top of v3.5-rc1 or my
   gadget branch which I will update in a while.
  
 
  I rebased and submitted my changes on your gadget-for-v3.5 tag.
  Should I now refresh my patches on top of your v3.5-rc1 branch ?
 
  I am a bit confused on what is the latest gadget branch to be used
 now.
  Thanks for helping out.
 
 The gadget branch is the branch called gadget on my kernel.org tree.
 For some reason this didn't apply. Probably some patches on
 drivers/usb/gadget/*uvc* went into v3.5 without my knowledge. Possibly
 because I was out for quite a while and asked Greg to help me out
 during the merge window.
 
 Anyway, I just pushed gadget with a bunch of new patches and part of
 your series.
 

Yes. I had sent two patches some time ago for drivers/usb/gadget/*uvc*.
For one of them I received an *applied* message from you:

  usb: gadget/uvc: Remove non-required locking from 'uvc_queue_next_buffer' 
  routine

  This patch removes the non-required spinlock acquire/release calls on
  'queue-irqlock' from 'uvc_queue_next_buffer' routine.
 
  This routine is called from 'video-encode' function (which
 translates
  to either 'uvc_video_encode_bulk' or 'uvc_video_encode_isoc') in
 'uvc_video.c'.
  As, the 'video-encode' routines are called with 'queue-irqlock'
  already held, so acquiring a 'queue-irqlock' again in
  'uvc_queue_next_buffer' routine causes a spin lock recursion.
 
  Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
  Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
 
 applied, thanks

Not sure, if that can cause the merge conflict issue.
So now, should I send a clean patchset on top of your 3.5-rc1 branch to ensure
the entire new patchset for drivers/usb/gadget/*uvc* is pulled properly?

Thanks,
Bhupesh

---BeginMessage---
On Fri, Mar 23, 2012 at 10:23:13PM +0530, Bhupesh Sharma wrote:
 This patch removes the non-required spinlock acquire/release calls on
 'queue-irqlock' from 'uvc_queue_next_buffer' routine.
 
 This routine is called from 'video-encode' function (which translates to 
 either
 'uvc_video_encode_bulk' or 'uvc_video_encode_isoc') in 'uvc_video.c'.
 As, the 'video-encode' routines are called with 'queue-irqlock' already 
 held,
 so acquiring a 'queue-irqlock' again in 'uvc_queue_next_buffer' routine 
 causes
 a spin lock recursion.
 
 Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
 Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com

applied, thanks

-- 
balbi


signature.asc
Description: Digital signature
---End Message---

Re: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

[Felipe Balbi]

Hi,

On Mon, Jun 04, 2012 at 11:37:59PM +0800, Bhupesh SHARMA wrote:
  -Original Message-
  From: Felipe Balbi [mailto:ba...@ti.com]
  Sent: Monday, June 04, 2012 8:59 PM
  To: Bhupesh SHARMA
  Cc: ba...@ti.com; laurent.pinch...@ideasonboard.com; linux-
  u...@vger.kernel.org; linux-media@vger.kernel.org;
  gre...@linuxfoundation.org
  Subject: Re: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use
  videobuf2 framework
  
  On Mon, Jun 04, 2012 at 11:21:13PM +0800, Bhupesh SHARMA wrote:
   Hi Felipe,
  
-Original Message-
From: Felipe Balbi [mailto:ba...@ti.com]
Sent: Monday, June 04, 2012 8:44 PM
To: Bhupesh SHARMA
Cc: laurent.pinch...@ideasonboard.com; linux-...@vger.kernel.org;
ba...@ti.com; linux-media@vger.kernel.org;
gre...@linuxfoundation.org
Subject: Re: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to
use
videobuf2 framework
   
On Fri, Jun 01, 2012 at 03:08:57PM +0530, Bhupesh Sharma wrote:
 This patch reworks the videobuffer management logic present in
  the
UVC
 webcam gadget and ports it to use the more apt videobuf2
 framework for video buffer management.

 To support routing video data captured from a real V4L2 video
 capture device with a zero copy operation on videobuffers (as
 they pass
from
 the V4L2 domain to UVC domain via a user-space application), we
 need to support USER_PTR IO method at the UVC gadget side.

 So the V4L2 capture device driver can still continue to use MMAO
 IO method and now the user-space application can just pass a
 pointer to the video buffers being DeQueued from the V4L2 device
 side while Queueing them at the UVC gadget end. This ensures that
 we have a zero-copy design as the videobuffers pass from the
 V4L2 capture
device to the UVC gadget.

 Note that there will still be a need to apply UVC specific
  payload
 headers on top of each UVC payload data, which will still require
 a copy operation to be performed in the 'encode' routines of the
 UVC
gadget.

 Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
   
this patch doesn't apply. Please refresh on top of v3.5-rc1 or my
gadget branch which I will update in a while.
   
  
   I rebased and submitted my changes on your gadget-for-v3.5 tag.
   Should I now refresh my patches on top of your v3.5-rc1 branch ?
  
   I am a bit confused on what is the latest gadget branch to be used
  now.
   Thanks for helping out.
  
  The gadget branch is the branch called gadget on my kernel.org tree.
  For some reason this didn't apply. Probably some patches on
  drivers/usb/gadget/*uvc* went into v3.5 without my knowledge. Possibly
  because I was out for quite a while and asked Greg to help me out
  during the merge window.
  
  Anyway, I just pushed gadget with a bunch of new patches and part of
  your series.
  
 
 Yes. I had sent two patches some time ago for drivers/usb/gadget/*uvc*.
 For one of them I received an *applied* message from you:

that was already applied long ago. ;-)

 
   usb: gadget/uvc: Remove non-required locking from 'uvc_queue_next_buffer' 
   routine
 
   This patch removes the non-required spinlock acquire/release calls on
   'queue-irqlock' from 'uvc_queue_next_buffer' routine.
  
   This routine is called from 'video-encode' function (which
  translates
   to either 'uvc_video_encode_bulk' or 'uvc_video_encode_isoc') in
  'uvc_video.c'.
   As, the 'video-encode' routines are called with 'queue-irqlock'
   already held, so acquiring a 'queue-irqlock' again in
   'uvc_queue_next_buffer' routine causes a spin lock recursion.
  
   Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
   Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
  
  applied, thanks
 
 Not sure, if that can cause the merge conflict issue.
 So now, should I send a clean patchset on top of your 3.5-rc1 branch to ensure
 the entire new patchset for drivers/usb/gadget/*uvc* is pulled properly?

Yes please, just give kernel.org about 20 minutes to sync all git
servers.

Just so you know, head on my gadget branch is:

commit fbcaba0e3dcec8451cccdc1fa92fcddbde2bc3f2
Author: Bhupesh Sharma bhupesh.sha...@st.com
Date:   Fri Jun 1 15:08:56 2012 +0530

usb: gadget: uvc: Add super-speed support to UVC webcam gadget

This patch adds super-speed support to UVC webcam gadget.

Also in this patch:
- We add the configurability to pass bInterval, bMaxBurst, mult
  factors for video streaming endpoint (ISOC IN) through module
  parameters.

- We use config_ep_by_speed helper routine to configure video
  streaming endpoint.

Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
Signed-off-by: Felipe Balbi ba...@ti.com

-- 
balbi


signature.asc
Description: Digital signature

RE: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

[Bhupesh SHARMA]

Hi Felipe,

 -Original Message-
 From: Felipe Balbi [mailto:ba...@ti.com]
 Sent: Monday, June 04, 2012 9:11 PM
 To: Bhupesh SHARMA
 Cc: ba...@ti.com; laurent.pinch...@ideasonboard.com; linux-
 u...@vger.kernel.org; linux-media@vger.kernel.org;
 gre...@linuxfoundation.org
 Subject: Re: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use
 videobuf2 framework
 
 Hi,
 
 On Mon, Jun 04, 2012 at 11:37:59PM +0800, Bhupesh SHARMA wrote:
   -Original Message-
   From: Felipe Balbi [mailto:ba...@ti.com]
   Sent: Monday, June 04, 2012 8:59 PM
   To: Bhupesh SHARMA
   Cc: ba...@ti.com; laurent.pinch...@ideasonboard.com; linux-
   u...@vger.kernel.org; linux-media@vger.kernel.org;
   gre...@linuxfoundation.org
   Subject: Re: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to
   use
   videobuf2 framework
  
   On Mon, Jun 04, 2012 at 11:21:13PM +0800, Bhupesh SHARMA wrote:
Hi Felipe,
   
 -Original Message-
 From: Felipe Balbi [mailto:ba...@ti.com]
 Sent: Monday, June 04, 2012 8:44 PM
 To: Bhupesh SHARMA
 Cc: laurent.pinch...@ideasonboard.com;
 linux-...@vger.kernel.org; ba...@ti.com;
 linux-media@vger.kernel.org; gre...@linuxfoundation.org
 Subject: Re: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam
 gadget
 to use
 videobuf2 framework

 On Fri, Jun 01, 2012 at 03:08:57PM +0530, Bhupesh Sharma wrote:
  This patch reworks the videobuffer management logic present
 in
   the
 UVC
  webcam gadget and ports it to use the more apt videobuf2
  framework for video buffer management.
 
  To support routing video data captured from a real V4L2 video
  capture device with a zero copy operation on videobuffers
  (as they pass
 from
  the V4L2 domain to UVC domain via a user-space application),
  we need to support USER_PTR IO method at the UVC gadget side.
 
  So the V4L2 capture device driver can still continue to use
  MMAO IO method and now the user-space application can just
  pass a pointer to the video buffers being DeQueued from the
  V4L2 device side while Queueing them at the UVC gadget end.
  This ensures that we have a zero-copy design as the
  videobuffers pass from the
  V4L2 capture
 device to the UVC gadget.
 
  Note that there will still be a need to apply UVC specific
   payload
  headers on top of each UVC payload data, which will still
  require a copy operation to be performed in the 'encode'
  routines of the UVC
 gadget.
 
  Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com

 this patch doesn't apply. Please refresh on top of v3.5-rc1 or
 my gadget branch which I will update in a while.

   
I rebased and submitted my changes on your gadget-for-v3.5 tag.
Should I now refresh my patches on top of your v3.5-rc1 branch
 ?
   
I am a bit confused on what is the latest gadget branch to be
 used
   now.
Thanks for helping out.
  
   The gadget branch is the branch called gadget on my kernel.org
 tree.
   For some reason this didn't apply. Probably some patches on
   drivers/usb/gadget/*uvc* went into v3.5 without my knowledge.
   Possibly because I was out for quite a while and asked Greg to help
   me out during the merge window.
  
   Anyway, I just pushed gadget with a bunch of new patches and part
 of
   your series.
  
 
  Yes. I had sent two patches some time ago for
 drivers/usb/gadget/*uvc*.
  For one of them I received an *applied* message from you:
 
 that was already applied long ago. ;-)
 
 
usb: gadget/uvc: Remove non-required locking from
'uvc_queue_next_buffer' routine
 
This patch removes the non-required spinlock acquire/release
 calls
on 'queue-irqlock' from 'uvc_queue_next_buffer' routine.
   
This routine is called from 'video-encode' function (which
   translates
to either 'uvc_video_encode_bulk' or 'uvc_video_encode_isoc') in
   'uvc_video.c'.
As, the 'video-encode' routines are called with 'queue-irqlock'
already held, so acquiring a 'queue-irqlock' again in
'uvc_queue_next_buffer' routine causes a spin lock recursion.
   
Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
  
   applied, thanks
 
  Not sure, if that can cause the merge conflict issue.
  So now, should I send a clean patchset on top of your 3.5-rc1 branch
  to ensure the entire new patchset for drivers/usb/gadget/*uvc* is
 pulled properly?
 
 Yes please, just give kernel.org about 20 minutes to sync all git
 servers.
 
 Just so you know, head on my gadget branch is:
 
 commit fbcaba0e3dcec8451cccdc1fa92fcddbde2bc3f2
 Author: Bhupesh Sharma bhupesh.sha...@st.com
 Date:   Fri Jun 1 15:08:56 2012 +0530
 
 usb: gadget: uvc: Add super-speed support to UVC webcam gadget
 
 This patch adds super-speed support to UVC webcam gadget.
 
 Also in this patch:
 - We add

Re: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

[Laurent Pinchart]

Hi Felipe,

On Monday 04 June 2012 18:28:33 Felipe Balbi wrote:
 On Mon, Jun 04, 2012 at 11:21:13PM +0800, Bhupesh SHARMA wrote:
  On Monday, June 04, 2012 8:44 PM Felipe Balbi wrote:
   On Fri, Jun 01, 2012 at 03:08:57PM +0530, Bhupesh Sharma wrote:
This patch reworks the videobuffer management logic present in the
UVC webcam gadget and ports it to use the more apt videobuf2
framework for video buffer management.

To support routing video data captured from a real V4L2 video capture
device with a zero copy operation on videobuffers (as they pass from
the V4L2 domain to UVC domain via a user-space application), we need
to support USER_PTR IO method at the UVC gadget side.

So the V4L2 capture device driver can still continue to use MMAO IO
method and now the user-space application can just pass a pointer to
the video buffers being DeQueued from the V4L2 device side while
Queueing them at the UVC gadget end. This ensures that we have a
zero-copy design as the videobuffers pass from the V4L2 capture
device to the UVC gadget.
   
Note that there will still be a need to apply UVC specific payload
headers on top of each UVC payload data, which will still require a
copy operation to be performed in the 'encode' routines of the UVC
gadget.
   
Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
   
   this patch doesn't apply. Please refresh on top of v3.5-rc1 or my gadget
   branch which I will update in a while.
  
  I rebased and submitted my changes on your gadget-for-v3.5 tag.
  Should I now refresh my patches on top of your v3.5-rc1 branch ?
  
  I am a bit confused on what is the latest gadget branch to be used now.
  Thanks for helping out.
 
 The gadget branch is the branch called gadget on my kernel.org tree. For
 some reason this didn't apply. Probably some patches on
 drivers/usb/gadget/*uvc* went into v3.5 without my knowledge. Possibly
 because I was out for quite a while and asked Greg to help me out during
 the merge window.
 
 Anyway, I just pushed gadget with a bunch of new patches and part of
 your series.

I would have appreciated an occasion to review them first (especially 3/5 
which should *really* have been split into several patches) :-( Have they been 
pushed to mainline yet ?

I'm currently traveling to Japan for LinuxCon so I won't have time to look 
into this before next week. I'll send incremental patches to fix issues with 
the already applied patches, *please* don't apply 4/5 and 5/5 before I can 
review them.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

[Felipe Balbi]

On Mon, Jun 04, 2012 at 06:40:46PM +0200, Laurent Pinchart wrote:
 Hi Felipe,
 
 On Monday 04 June 2012 18:28:33 Felipe Balbi wrote:
  On Mon, Jun 04, 2012 at 11:21:13PM +0800, Bhupesh SHARMA wrote:
   On Monday, June 04, 2012 8:44 PM Felipe Balbi wrote:
On Fri, Jun 01, 2012 at 03:08:57PM +0530, Bhupesh Sharma wrote:
 This patch reworks the videobuffer management logic present in the
 UVC webcam gadget and ports it to use the more apt videobuf2
 framework for video buffer management.
 
 To support routing video data captured from a real V4L2 video capture
 device with a zero copy operation on videobuffers (as they pass from
 the V4L2 domain to UVC domain via a user-space application), we need
 to support USER_PTR IO method at the UVC gadget side.
 
 So the V4L2 capture device driver can still continue to use MMAO IO
 method and now the user-space application can just pass a pointer to
 the video buffers being DeQueued from the V4L2 device side while
 Queueing them at the UVC gadget end. This ensures that we have a
 zero-copy design as the videobuffers pass from the V4L2 capture
 device to the UVC gadget.

 Note that there will still be a need to apply UVC specific payload
 headers on top of each UVC payload data, which will still require a
 copy operation to be performed in the 'encode' routines of the UVC
 gadget.

 Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com

this patch doesn't apply. Please refresh on top of v3.5-rc1 or my gadget
branch which I will update in a while.
   
   I rebased and submitted my changes on your gadget-for-v3.5 tag.
   Should I now refresh my patches on top of your v3.5-rc1 branch ?
   
   I am a bit confused on what is the latest gadget branch to be used now.
   Thanks for helping out.
  
  The gadget branch is the branch called gadget on my kernel.org tree. For
  some reason this didn't apply. Probably some patches on
  drivers/usb/gadget/*uvc* went into v3.5 without my knowledge. Possibly
  because I was out for quite a while and asked Greg to help me out during
  the merge window.
  
  Anyway, I just pushed gadget with a bunch of new patches and part of
  your series.
 
 I would have appreciated an occasion to review them first (especially 3/5 
 which should *really* have been split into several patches) :-( Have they 
 been 
 pushed to mainline yet ?

on my branch only, but I don't plan to rebase as that would screw up my
git objects.

 I'm currently traveling to Japan for LinuxCon so I won't have time to look 
 into this before next week. I'll send incremental patches to fix issues with 
 the already applied patches, *please* don't apply 4/5 and 5/5 before I can 
 review them.

sure, no problem... Will wait.

-- 
balbi


signature.asc
Description: Digital signature

[PATCH 0/5] UVC webcam gadget related changes

[Bhupesh Sharma]

This patchset tries to take the UVC webcam gadget one step
closer to being used with a real V4L2 video capture device (via
a user-space application which is responsible for ensuring correct sequence of
operations being performed on both UVC gadget and V4L2 capture device
end).

A major change introduced by this patchset is to port UVC webcam gadget
to use videobuf2 framework for videobuffer managment and exposes
USER_PTR IO method at the UVC gadget side to ensure zero-copy of
video data as it passes from V4L2 capture driver domain to UVC gadget domain.
(Thanks to Laurent Pinchart for suggesting this design change).

I have tested this patchset on a super-speed compliant USB device
controller, with the VIVI capture device acting as a dummy source
of video data and I have modified the 'uvc-gadget' application written
by Laurent (original application available
here: http://git.ideasonboard.org/uvc-gadget.git) for testing the
complete flow from V4L2 to UVC domain and vice versa.

Bhupesh Sharma (5):
  usb: gadget/uvc: Fix string descriptor STALL issue when multiple uvc
functions are added to a configuration
  usb: gadget/uvc: Use macro for interrupt endpoint status size instead
of using a MAGIC number
  usb: gadget/uvc: Add super-speed support to UVC webcam gadget
  usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework
  usb: gadget/uvc: Add support for 'USB_GADGET_DELAYED_STATUS' response
for a set_intf(alt-set 1) command

 drivers/usb/gadget/Kconfig |1 +
 drivers/usb/gadget/f_uvc.c |  304 +++
 drivers/usb/gadget/f_uvc.h |8 +-
 drivers/usb/gadget/uvc.h   |7 +-
 drivers/usb/gadget/uvc_queue.c |  524 +++-
 drivers/usb/gadget/uvc_queue.h |   25 +--
 drivers/usb/gadget/uvc_v4l2.c  |   73 --
 drivers/usb/gadget/uvc_video.c |   17 +-
 drivers/usb/gadget/webcam.c|   29 ++-
 9 files changed, 509 insertions(+), 479 deletions(-)

-- 
1.7.2.2

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/5] usb: gadget/uvc: Add super-speed support to UVC webcam gadget

[Bhupesh Sharma]

This patch adds super-speed support to UVC webcam gadget.

Also in this patch:
- We add the configurability to pass bInterval, bMaxBurst, mult
  factors for video streaming endpoint (ISOC IN) through module
  parameters.

- We use config_ep_by_speed helper routine to configure video
  streaming endpoint.

Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
---
 drivers/usb/gadget/f_uvc.c  |  241 ++-
 drivers/usb/gadget/f_uvc.h  |8 +-
 drivers/usb/gadget/uvc.h|4 +-
 drivers/usb/gadget/webcam.c |   29 +-
 4 files changed, 247 insertions(+), 35 deletions(-)

diff --git a/drivers/usb/gadget/f_uvc.c b/drivers/usb/gadget/f_uvc.c
index dd7d7a9..2a8bf06 100644
--- a/drivers/usb/gadget/f_uvc.c
+++ b/drivers/usb/gadget/f_uvc.c
@@ -29,6 +29,25 @@
 
 unsigned int uvc_gadget_trace_param;
 
+/*-*/
+
+/* module parameters specific to the Video streaming endpoint */
+static unsigned streaming_interval = 1;
+module_param(streaming_interval, uint, S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(streaming_interval, 1 - 16);
+
+static unsigned streaming_maxpacket = 1024;
+module_param(streaming_maxpacket, uint, S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(streaming_maxpacket, 0 - 1023 (fs), 0 - 1024 (hs/ss));
+
+static unsigned streaming_mult;
+module_param(streaming_mult, uint, S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(streaming_mult, 0 - 2 (hs/ss only));
+
+static unsigned streaming_maxburst;
+module_param(streaming_maxburst, uint, S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(streaming_maxburst, 0 - 15 (ss only));
+
 /* --
  * Function descriptors
  */
@@ -84,7 +103,7 @@ static struct usb_interface_descriptor uvc_control_intf 
__initdata = {
.iInterface = 0,
 };
 
-static struct usb_endpoint_descriptor uvc_control_ep __initdata = {
+static struct usb_endpoint_descriptor uvc_fs_control_ep __initdata = {
.bLength= USB_DT_ENDPOINT_SIZE,
.bDescriptorType= USB_DT_ENDPOINT,
.bEndpointAddress   = USB_DIR_IN,
@@ -124,7 +143,7 @@ static struct usb_interface_descriptor 
uvc_streaming_intf_alt1 __initdata = {
.iInterface = 0,
 };
 
-static struct usb_endpoint_descriptor uvc_streaming_ep = {
+static struct usb_endpoint_descriptor uvc_fs_streaming_ep = {
.bLength= USB_DT_ENDPOINT_SIZE,
.bDescriptorType= USB_DT_ENDPOINT,
.bEndpointAddress   = USB_DIR_IN,
@@ -133,15 +152,72 @@ static struct usb_endpoint_descriptor uvc_streaming_ep = {
.bInterval  = 1,
 };
 
+static struct usb_endpoint_descriptor uvc_hs_streaming_ep = {
+   .bLength= USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType= USB_DT_ENDPOINT,
+   .bEndpointAddress   = USB_DIR_IN,
+   .bmAttributes   = USB_ENDPOINT_XFER_ISOC,
+   .wMaxPacketSize = cpu_to_le16(1024),
+   .bInterval  = 1,
+};
+
+/* super speed support */
+static struct usb_endpoint_descriptor uvc_ss_control_ep __initdata = {
+   .bLength =  USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType =  USB_DT_ENDPOINT,
+
+   .bEndpointAddress = USB_DIR_IN,
+   .bmAttributes = USB_ENDPOINT_XFER_INT,
+   .wMaxPacketSize =   cpu_to_le16(STATUS_BYTECOUNT),
+   .bInterval =8,
+};
+
+static struct usb_ss_ep_comp_descriptor uvc_ss_control_comp __initdata = {
+   .bLength =  sizeof uvc_ss_control_comp,
+   .bDescriptorType =  USB_DT_SS_ENDPOINT_COMP,
+
+   /* the following 3 values can be tweaked if necessary */
+   /* .bMaxBurst = 0, */
+   /* .bmAttributes =  0, */
+   .wBytesPerInterval =cpu_to_le16(STATUS_BYTECOUNT),
+};
+
+static struct usb_endpoint_descriptor uvc_ss_streaming_ep __initdata = {
+   .bLength =  USB_DT_ENDPOINT_SIZE,
+   .bDescriptorType =  USB_DT_ENDPOINT,
+
+   .bEndpointAddress = USB_DIR_IN,
+   .bmAttributes = USB_ENDPOINT_XFER_ISOC,
+   .wMaxPacketSize =   cpu_to_le16(1024),
+   .bInterval =4,
+};
+
+static struct usb_ss_ep_comp_descriptor uvc_ss_streaming_comp = {
+   .bLength =  sizeof uvc_ss_streaming_comp,
+   .bDescriptorType =  USB_DT_SS_ENDPOINT_COMP,
+
+   /* the following 3 values can be tweaked if necessary */
+   .bMaxBurst =0,
+   .bmAttributes = 0,
+   .wBytesPerInterval =cpu_to_le16(1024),
+};
+
 static const struct usb_descriptor_header * const uvc_fs_streaming[] = {
(struct usb_descriptor_header *) uvc_streaming_intf_alt1,
-   (struct usb_descriptor_header *) uvc_streaming_ep,
+   (struct usb_descriptor_header *) uvc_fs_streaming_ep,
NULL,
 };
 
 static const struct usb_descriptor_header * const

[PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

[Bhupesh Sharma]

This patch reworks the videobuffer management logic present in the UVC
webcam gadget and ports it to use the more apt videobuf2 framework for
video buffer management.

To support routing video data captured from a real V4L2 video capture
device with a zero copy operation on videobuffers (as they pass from the V4L2
domain to UVC domain via a user-space application), we need to support USER_PTR
IO method at the UVC gadget side.

So the V4L2 capture device driver can still continue to use MMAO IO method
and now the user-space application can just pass a pointer to the video buffers
being DeQueued from the V4L2 device side while Queueing them at the UVC gadget
end. This ensures that we have a zero-copy design as the videobuffers pass
from the V4L2 capture device to the UVC gadget.

Note that there will still be a need to apply UVC specific payload headers
on top of each UVC payload data, which will still require a copy operation
to be performed in the 'encode' routines of the UVC gadget.

Signed-off-by: Bhupesh Sharma bhupesh.sha...@st.com
---
 drivers/usb/gadget/Kconfig |1 +
 drivers/usb/gadget/uvc_queue.c |  524 +++-
 drivers/usb/gadget/uvc_queue.h |   25 +--
 drivers/usb/gadget/uvc_v4l2.c  |   35 ++--
 drivers/usb/gadget/uvc_video.c |   17 +-
 5 files changed, 184 insertions(+), 418 deletions(-)

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 1f93861..5a351f8 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -967,6 +967,7 @@ endif
 config USB_G_WEBCAM
tristate USB Webcam Gadget
depends on VIDEO_DEV
+   select VIDEOBUF2_VMALLOC
help
  The Webcam Gadget acts as a composite USB Audio and Video Class
  device. It provides a userspace API to process UVC control requests
diff --git a/drivers/usb/gadget/uvc_queue.c b/drivers/usb/gadget/uvc_queue.c
index 0cdf89d..907ece8 100644
--- a/drivers/usb/gadget/uvc_queue.c
+++ b/drivers/usb/gadget/uvc_queue.c
@@ -10,6 +10,7 @@
  * (at your option) any later version.
  */
 
+#include linux/atomic.h
 #include linux/kernel.h
 #include linux/mm.h
 #include linux/list.h
@@ -18,7 +19,8 @@
 #include linux/videodev2.h
 #include linux/vmalloc.h
 #include linux/wait.h
-#include linux/atomic.h
+
+#include media/videobuf2-vmalloc.h
 
 #include uvc.h
 
@@ -28,271 +30,156 @@
  * Video queues is initialized by uvc_queue_init(). The function performs
  * basic initialization of the uvc_video_queue struct and never fails.
  *
- * Video buffer allocation and freeing are performed by uvc_alloc_buffers and
- * uvc_free_buffers respectively. The former acquires the video queue lock,
- * while the later must be called with the lock held (so that allocation can
- * free previously allocated buffers). Trying to free buffers that are mapped
- * to user space will return -EBUSY.
- *
- * Video buffers are managed using two queues. However, unlike most USB video
- * drivers that use an in queue and an out queue, we use a main queue to hold
- * all queued buffers (both 'empty' and 'done' buffers), and an irq queue to
- * hold empty buffers. This design (copied from video-buf) minimizes locking
- * in interrupt, as only one queue is shared between interrupt and user
- * contexts.
- *
- * Use cases
- * -
- *
- * Unless stated otherwise, all operations that modify the irq buffers queue
- * are protected by the irq spinlock.
- *
- * 1. The user queues the buffers, starts streaming and dequeues a buffer.
- *
- *The buffers are added to the main and irq queues. Both operations are
- *protected by the queue lock, and the later is protected by the irq
- *spinlock as well.
- *
- *The completion handler fetches a buffer from the irq queue and fills it
- *with video data. If no buffer is available (irq queue empty), the handler
- *returns immediately.
- *
- *When the buffer is full, the completion handler removes it from the irq
- *queue, marks it as ready (UVC_BUF_STATE_DONE) and wakes its wait queue.
- *At that point, any process waiting on the buffer will be woken up. If a
- *process tries to dequeue a buffer after it has been marked ready, the
- *dequeing will succeed immediately.
- *
- * 2. Buffers are queued, user is waiting on a buffer and the device gets
- *disconnected.
- *
- *When the device is disconnected, the kernel calls the completion handler
- *with an appropriate status code. The handler marks all buffers in the
- *irq queue as being erroneous (UVC_BUF_STATE_ERROR) and wakes them up so
- *that any process waiting on a buffer gets woken up.
- *
- *Waking up up the first buffer on the irq list is not enough, as the
- *process waiting on the buffer might restart the dequeue operation
- *immediately.
- *
+ * Video buffers are managed by videobuf2. The driver uses a mutex to protect
+ * the videobuf2 queue operations by serializing calls to videobuf2 and a
+ * spinlock

Re: Using UVC webcam gadget with a real v4l2 device

[Laurent Pinchart]

Hi Bhupesh,

On Monday 30 April 2012 18:47:24 Bhupesh SHARMA wrote:
 On Monday, April 30, 2012 3:51 PM Laurent Pinchart wrote:
  On Thursday 26 April 2012 13:23:59 Bhupesh SHARMA wrote:
   Hi Laurent,
   
   Sorry to jump-in before your reply on my previous mail, but as I was
   studying the USERPTR stuff in more detail, I have a few more queries
   which I believe you can include in your reply as well..
  
  [snip]
  
   I am now a bit confused on how the entire system will work now:
   - Does USERPTR method needs to be supported both in UVC gadget and
   soc-camera side, or one can still support the MMAP method and the other
   can now be changed to support USERPTR method and we can achieve a ZERO
   buffer copy operation using this method?
  
  You need USERPTR support on one side only. In practice many (all?) soc-
  camera drivers require physically contiguous memory, so you will need to
  use MMAP on the soc-camera side and USERPTR on the UVC gadget side.
  DMABUF, when merged in the kernel, will be a better solution (but will
  require all drivers to use vb2).
 
 Perfect. So, I plan now to add vb2 support for uvc-gadget and leave soc-
 camera side to use the mmap stuff.
 
 Now, waiting for your pointers for managing the race-conditions in the UVC
 gadget and also avoiding the memcpy that is happening in the QBUF call on
 the UVC gadget, before I start the actual work.

The memcpy doesn't happen at QBUF time, but when filling the URBs. Avoiding it 
will be pretty difficult, as the driver needs to add packet headers. I would 
leave that out for now.

Regarding videobuf2 support, the main issue comes from race conditions between 
stream start, buffer queueing and URB completion. Unlike the UVC host driver 
where URBs can be resubmitted immediately, the gadget driver can only resubmit 
URBs (in uvc_video_complete()) when there is data to be sent. Otherwise the 
URB is put on a free URBs list (video-req_free) and enqueued in 
uvc_video_pump() the next time a buffer is queued. This requires taking 
various locks and must thus be considered with care. I'm pretty unhappy with 
calling video-encode with the queue irqlock held, I would like to change 
that, but I don't expect to to be an easy task.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Using UVC webcam gadget with a real v4l2 device

[Laurent Pinchart]

Hi Bhupesh,

On Thursday 26 April 2012 13:23:59 Bhupesh SHARMA wrote:
 Hi Laurent,
 
 Sorry to jump-in before your reply on my previous mail,
 but as I was studying the USERPTR stuff in more detail, I have a few more
 queries which I believe you can include in your reply as well..

[snip]

 I am now a bit confused on how the entire system will work now:
   - Does USERPTR method needs to be supported both in UVC gadget and
 soc-camera side, or one can still support the MMAP method and the other can
 now be changed to support USERPTR method and we can achieve a ZERO buffer
 copy operation using this method?

You need USERPTR support on one side only. In practice many (all?) soc-camera 
drivers require physically contiguous memory, so you will need to use MMAP on 
the soc-camera side and USERPTR on the UVC gadget side. DMABUF, when merged in 
the kernel, will be a better solution (but will require all drivers to use 
vb2).

   - More specifically, I would like to keep the soc-camera still using 
 MMAP
 (and hence still using video-buf) and make changes at the UVC gadget side
 to support USERPTR and videobuf2. Will this work?

Please see above :-)

   - At the application side how should we design the flow in case both
 support USERPTR, i.e. the buffer needs to be protected from simultaneous
 access from the UVC gadget driver and soc-camera driver (to ensure that a
 single buffer can be shared across them). Also in case we keep soc-camera
 still using MMAP and UVC gadget side supporting USERPTR, how can we share a
 common buffer across the UVC gadget and soc-camera driver.

That's easy. Request the same number of buffers on both sides with REQBUFS, 
mmap() them to userspace on the soc-camera side, and then use the user pointer 
to queue them with QBUF on the UVC side. You just need to ensure that a buffer 
is never enqueued to two drivers at the same time. Wait for buffers to be 
ready on both sides with select(), and when a buffer is ready dequeue it and 
requeue it on the other side.

   - In case of USERPTR method the camera capture hardware should be able 
 to
 DMA the received data to the user space buffers. Are there any specific
 requirements on the DMA capability of these use-space buffers
 (scatter-gather or contiguous?).

DMA to userspace is quite hackish. You should use the MMAP method on the soc-
camera side.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: Using UVC webcam gadget with a real v4l2 device

[Bhupesh SHARMA]

Hi Laurent,

 -Original Message-
 From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com]
 Sent: Monday, April 30, 2012 3:51 PM
 To: Bhupesh SHARMA
 Cc: linux-...@vger.kernel.org; linux-media@vger.kernel.org;
 ba...@ti.com; g.liakhovet...@gmx.de
 Subject: Re: Using UVC webcam gadget with a real v4l2 device
 
 Hi Bhupesh,
 
 On Thursday 26 April 2012 13:23:59 Bhupesh SHARMA wrote:
  Hi Laurent,
 
  Sorry to jump-in before your reply on my previous mail,
  but as I was studying the USERPTR stuff in more detail, I have a few
 more
  queries which I believe you can include in your reply as well..
 
 [snip]
 
  I am now a bit confused on how the entire system will work now:
  - Does USERPTR method needs to be supported both in UVC gadget
 and
  soc-camera side, or one can still support the MMAP method and the
 other can
  now be changed to support USERPTR method and we can achieve a ZERO
 buffer
  copy operation using this method?
 
 You need USERPTR support on one side only. In practice many (all?) soc-
 camera
 drivers require physically contiguous memory, so you will need to use
 MMAP on
 the soc-camera side and USERPTR on the UVC gadget side. DMABUF, when
 merged in
 the kernel, will be a better solution (but will require all drivers to
 use
 vb2).

Perfect. So, I plan now to add vb2 support for uvc-gadget and leave soc-camera
side to use the mmap stuff.

Now, waiting for your pointers for managing the race-conditions in the UVC 
gadget
and also avoiding the memcpy that is happening in the QBUF call on the UVC 
gadget,
before I start the actual work.

Thanks for your help.

Regards,
Bhupesh

  - More specifically, I would like to keep the soc-camera still
 using MMAP
  (and hence still using video-buf) and make changes at the UVC gadget
 side
  to support USERPTR and videobuf2. Will this work?
 
 Please see above :-)
 
  - At the application side how should we design the flow in case
 both
  support USERPTR, i.e. the buffer needs to be protected from
 simultaneous
  access from the UVC gadget driver and soc-camera driver (to ensure
 that a
  single buffer can be shared across them). Also in case we keep soc-
 camera
  still using MMAP and UVC gadget side supporting USERPTR, how can we
 share a
  common buffer across the UVC gadget and soc-camera driver.
 
 That's easy. Request the same number of buffers on both sides with
 REQBUFS,
 mmap() them to userspace on the soc-camera side, and then use the user
 pointer
 to queue them with QBUF on the UVC side. You just need to ensure that a
 buffer
 is never enqueued to two drivers at the same time. Wait for buffers to
 be
 ready on both sides with select(), and when a buffer is ready dequeue
 it and
 requeue it on the other side.
 
  - In case of USERPTR method the camera capture hardware should be
 able to
  DMA the received data to the user space buffers. Are there any
 specific
  requirements on the DMA capability of these use-space buffers
  (scatter-gather or contiguous?).
 
 DMA to userspace is quite hackish. You should use the MMAP method on
 the soc-
 camera side.
 
 --
 Regards,
 
 Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: Using UVC webcam gadget with a real v4l2 device

[Bhupesh SHARMA]

Hi Laurent,

 -Original Message-
 From: Bhupesh SHARMA
 Sent: Thursday, April 26, 2012 10:54 AM
 To: 'Laurent Pinchart'
 Cc: 'linux-...@vger.kernel.org'; 'linux-media@vger.kernel.org';
 'ba...@ti.com'; 'g.liakhovet...@gmx.de'
 Subject: RE: Using UVC webcam gadget with a real v4l2 device
 
 Hi Laurent,
 
 Sorry to jump-in before your reply on my previous mail,
 but as I was studying the USERPTR stuff in more detail, I have a few
 more
 queries which I believe you can include in your reply as well..
 
  -Original Message-
  From: Bhupesh SHARMA
  Sent: Wednesday, April 25, 2012 8:37 PM
  To: 'Laurent Pinchart'
  Cc: linux-...@vger.kernel.org; linux-media@vger.kernel.org;
  ba...@ti.com; g.liakhovet...@gmx.de
  Subject: RE: Using UVC webcam gadget with a real v4l2 device
 
  Hi Laurent,
 
   -Original Message-
   From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com]
   Sent: Tuesday, April 24, 2012 2:26 AM
   To: Bhupesh SHARMA
   Cc: linux-...@vger.kernel.org; linux-media@vger.kernel.org;
   ba...@ti.com; g.liakhovet...@gmx.de
   Subject: Re: Using UVC webcam gadget with a real v4l2 device
  
   Hi Bhupesh,
  
   On Tuesday 24 April 2012 02:46:22 Bhupesh SHARMA wrote:
On Monday, April 23, 2012 7:47 PM Laurent Pinchart wrote:
 On Monday 23 April 2012 02:24:53 Bhupesh SHARMA wrote:
  Hi Laurent,
 
  I have been doing some experimentation with the UVC webcam
  gadget
   along
  with the UVC user-space application which you have written.
 
  The UVC webcam gadget works fine with the user space
  application
  handling the CONTROL events and providing DATA events. Now, I
   wish to
  interface a real v4l2 device, for e.g. VIVI or more
  particularly
   a
  soc_camera based host and subdev pair.
 
  Now, I see that I can achieve this by opening the UVC and
 V4L2
   devices
  and doing MMAP - REQBUF - QBUF - DQBUF calls on both the
   devices per
  the UVC control event received. But this will involve copying
  the
   video
  buffer in the user-space application from v4l2 (_CAPTURE) to
  uvc
  (_OUTPUT) domains, which will significantly reduce the video
   capture
  performance.
 
  Is there a better solution to this issue? Maybe doing
 something
   like a
  RNDIS gadget does with the help of u_ether.c like helper
   routines. But
  if I remember well it also requires the BRCTL (Bridge Control
   Utility)
  in userspace to route data arriving on usb0 to eth0 and vice-
   versa. Not
  sure though, if it does copying of a skb buffer from ethernet
  to
   usb
  domain and vice-versa.

 To avoid copying data between the two devices you should use
   USERPTR
 instead of MMAP on at least one of the two V4L2 devices. The
 UVC
   gadget
 driver doesn't support USERPTR yet though. This shouldn't be
 too
   difficult
 to fix, we need toreplace the custom buffers queue
 implementation
   with
 videobuf2, as has been done in the uvcvideo driver.
   
I was thinking of using the USERPTR method too, but I realized
 that
currently neither UVC webcam gadget nor soc-camera subsystem
  supports
   this
IO method. They support only MMAP IO as of now :(
  
   Both soc-camera and the UVC gadget driver should be ported to
  videobuf2
   to fix
   the problem.
 
 I am now a bit confused on how the entire system will work now:
   - Does USERPTR method needs to be supported both in UVC gadget
 and soc-camera side,
 or one can still support the MMAP method and the other can now
 be changed to support USERPTR method
 and we can achieve a ZERO buffer copy operation using this
 method?
 
   - More specifically, I would like to keep the soc-camera still
 using MMAP (and hence still using video-buf)
 and make changes at the UVC gadget side to support USERPTR and
 videobuf2. Will this work?
 
   - At the application side how should we design the flow in case
 both support USERPTR, i.e. the buffer needs
 to be protected from simultaneous access from the UVC gadget
 driver and soc-camera driver (to ensure that
 a single buffer can be shared across them). Also in case we
 keep soc-camera still using MMAP and UVC gadget
 side supporting USERPTR, how can we share a common buffer
 across the UVC gadget and soc-camera driver.
 
   - In case of USERPTR method the camera capture hardware should be
 able to DMA the received data to the user
 space buffers. Are there any specific requirements on the DMA
 capability of these use-space buffers
 (scatter-gather or contiguous?).


Can you help me with these queries.

Thanks,
Bhupesh
 
 Regards,
 Bhupesh
 
 I'll try to implement this. Would you then be able to test
  patches
   ?
   
For sure, I can test your patches on my setup.
  
   I had a quick look, but there's a bit more work than expected. The
  UVC
   gadget
   driver locking scheme needs

RE: Using UVC webcam gadget with a real v4l2 device

[Bhupesh SHARMA]

Hi Laurent,

 -Original Message-
 From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com]
 Sent: Tuesday, April 24, 2012 2:26 AM
 To: Bhupesh SHARMA
 Cc: linux-...@vger.kernel.org; linux-media@vger.kernel.org;
 ba...@ti.com; g.liakhovet...@gmx.de
 Subject: Re: Using UVC webcam gadget with a real v4l2 device
 
 Hi Bhupesh,
 
 On Tuesday 24 April 2012 02:46:22 Bhupesh SHARMA wrote:
  On Monday, April 23, 2012 7:47 PM Laurent Pinchart wrote:
   On Monday 23 April 2012 02:24:53 Bhupesh SHARMA wrote:
Hi Laurent,
   
I have been doing some experimentation with the UVC webcam gadget
 along
with the UVC user-space application which you have written.
   
The UVC webcam gadget works fine with the user space application
handling the CONTROL events and providing DATA events. Now, I
 wish to
interface a real v4l2 device, for e.g. VIVI or more particularly
 a
soc_camera based host and subdev pair.
   
Now, I see that I can achieve this by opening the UVC and V4L2
 devices
and doing MMAP - REQBUF - QBUF - DQBUF calls on both the
 devices per
the UVC control event received. But this will involve copying the
 video
buffer in the user-space application from v4l2 (_CAPTURE) to uvc
(_OUTPUT) domains, which will significantly reduce the video
 capture
performance.
   
Is there a better solution to this issue? Maybe doing something
 like a
RNDIS gadget does with the help of u_ether.c like helper
 routines. But
if I remember well it also requires the BRCTL (Bridge Control
 Utility)
in userspace to route data arriving on usb0 to eth0 and vice-
 versa. Not
sure though, if it does copying of a skb buffer from ethernet to
 usb
domain and vice-versa.
  
   To avoid copying data between the two devices you should use
 USERPTR
   instead of MMAP on at least one of the two V4L2 devices. The UVC
 gadget
   driver doesn't support USERPTR yet though. This shouldn't be too
 difficult
   to fix, we need toreplace the custom buffers queue implementation
 with
   videobuf2, as has been done in the uvcvideo driver.
 
  I was thinking of using the USERPTR method too, but I realized that
  currently neither UVC webcam gadget nor soc-camera subsystem supports
 this
  IO method. They support only MMAP IO as of now :(
 
 Both soc-camera and the UVC gadget driver should be ported to videobuf2
 to fix
 the problem.
 
   I'll try to implement this. Would you then be able to test patches
 ?
 
  For sure, I can test your patches on my setup.
 
 I had a quick look, but there's a bit more work than expected. The UVC
 gadget
 driver locking scheme needs to be revisited. I unfortunately won't have
 time
 to work on that in the next couple of weeks, and very probably not
 before end
 of June. Sorry.

 If you want to give it a try, I can provide you with some pointers.

It's  a pity. You are the best person to do it as you have in-depth know
-how of both v4l2 and UVC webcam gadget. But I can give it a try if you
can provide me some pointers..

 
  BTW, I was exploring GSTREAMER to use the data arriving from soc-
 camera
  (v4l2) capture device '/dev/video1' via 'v4l2src' plugin and routing
 the
  same to the UVC gadget '/dev/video0' via the 'v4l2sink' plugin.
 
  Don't know if this can work cleanly in my setup and whether GSTREAMER
  actually performs a buffer copy internally. But I will at-least give
 it a
  try :)
 
 There will definitely be a buffer copy (and actually two copies, as the
 UVC
 gadget driver performs a second copy internally) if you don't use
 USERPTR.

That's what I was afraid of. But can you let me know where the gadget driver
performs a second copy internally, so that I can also start exploring the
USERPTR method using the pointers provided by you..

Regards,
Bhupesh
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: Using UVC webcam gadget with a real v4l2 device

[Bhupesh SHARMA]

Hi Laurent,

Sorry to jump-in before your reply on my previous mail,
but as I was studying the USERPTR stuff in more detail, I have a few more
queries which I believe you can include in your reply as well..

 -Original Message-
 From: Bhupesh SHARMA
 Sent: Wednesday, April 25, 2012 8:37 PM
 To: 'Laurent Pinchart'
 Cc: linux-...@vger.kernel.org; linux-media@vger.kernel.org;
 ba...@ti.com; g.liakhovet...@gmx.de
 Subject: RE: Using UVC webcam gadget with a real v4l2 device
 
 Hi Laurent,
 
  -Original Message-
  From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com]
  Sent: Tuesday, April 24, 2012 2:26 AM
  To: Bhupesh SHARMA
  Cc: linux-...@vger.kernel.org; linux-media@vger.kernel.org;
  ba...@ti.com; g.liakhovet...@gmx.de
  Subject: Re: Using UVC webcam gadget with a real v4l2 device
 
  Hi Bhupesh,
 
  On Tuesday 24 April 2012 02:46:22 Bhupesh SHARMA wrote:
   On Monday, April 23, 2012 7:47 PM Laurent Pinchart wrote:
On Monday 23 April 2012 02:24:53 Bhupesh SHARMA wrote:
 Hi Laurent,

 I have been doing some experimentation with the UVC webcam
 gadget
  along
 with the UVC user-space application which you have written.

 The UVC webcam gadget works fine with the user space
 application
 handling the CONTROL events and providing DATA events. Now, I
  wish to
 interface a real v4l2 device, for e.g. VIVI or more
 particularly
  a
 soc_camera based host and subdev pair.

 Now, I see that I can achieve this by opening the UVC and V4L2
  devices
 and doing MMAP - REQBUF - QBUF - DQBUF calls on both the
  devices per
 the UVC control event received. But this will involve copying
 the
  video
 buffer in the user-space application from v4l2 (_CAPTURE) to
 uvc
 (_OUTPUT) domains, which will significantly reduce the video
  capture
 performance.

 Is there a better solution to this issue? Maybe doing something
  like a
 RNDIS gadget does with the help of u_ether.c like helper
  routines. But
 if I remember well it also requires the BRCTL (Bridge Control
  Utility)
 in userspace to route data arriving on usb0 to eth0 and vice-
  versa. Not
 sure though, if it does copying of a skb buffer from ethernet
 to
  usb
 domain and vice-versa.
   
To avoid copying data between the two devices you should use
  USERPTR
instead of MMAP on at least one of the two V4L2 devices. The UVC
  gadget
driver doesn't support USERPTR yet though. This shouldn't be too
  difficult
to fix, we need toreplace the custom buffers queue implementation
  with
videobuf2, as has been done in the uvcvideo driver.
  
   I was thinking of using the USERPTR method too, but I realized that
   currently neither UVC webcam gadget nor soc-camera subsystem
 supports
  this
   IO method. They support only MMAP IO as of now :(
 
  Both soc-camera and the UVC gadget driver should be ported to
 videobuf2
  to fix
  the problem.

I am now a bit confused on how the entire system will work now:
- Does USERPTR method needs to be supported both in UVC gadget and 
soc-camera side,
  or one can still support the MMAP method and the other can now be 
changed to support USERPTR method
  and we can achieve a ZERO buffer copy operation using this method?

- More specifically, I would like to keep the soc-camera still using 
MMAP (and hence still using video-buf)
  and make changes at the UVC gadget side to support USERPTR and 
videobuf2. Will this work?

- At the application side how should we design the flow in case both 
support USERPTR, i.e. the buffer needs
  to be protected from simultaneous access from the UVC gadget driver 
and soc-camera driver (to ensure that
  a single buffer can be shared across them). Also in case we keep 
soc-camera still using MMAP and UVC gadget
  side supporting USERPTR, how can we share a common buffer across the 
UVC gadget and soc-camera driver.

- In case of USERPTR method the camera capture hardware should be able 
to DMA the received data to the user
  space buffers. Are there any specific requirements on the DMA 
capability of these use-space buffers
  (scatter-gather or contiguous?).

Regards,
Bhupesh

I'll try to implement this. Would you then be able to test
 patches
  ?
  
   For sure, I can test your patches on my setup.
 
  I had a quick look, but there's a bit more work than expected. The
 UVC
  gadget
  driver locking scheme needs to be revisited. I unfortunately won't
 have
  time
  to work on that in the next couple of weeks, and very probably not
  before end
  of June. Sorry.
 
  If you want to give it a try, I can provide you with some pointers.
 
 It's  a pity. You are the best person to do it as you have in-depth
 know
 -how of both v4l2 and UVC webcam gadget. But I can give it a try if you
 can provide me some pointers..
 
 
   BTW, I was exploring GSTREAMER to use the data

Re: Using UVC webcam gadget with a real v4l2 device

[Laurent Pinchart]

On Tuesday 24 April 2012 08:36:34 Peter Chen wrote:
 On Mon, Apr 23, 2012 at 2:24 AM, Bhupesh SHARMA wrote:
  Hi Laurent,
  
  I have been doing some experimentation with the UVC webcam gadget along
  with the UVC user-space application which you have written.
 
 I have tried UVC webcam gadget at Freescale i.mx platform, unfortunately, 
 It can't work properly with my demo application. Would you tell me where I
 can get Laurent's user-space application? Thanks.

Here you go: http://git.ideasonboard.org/uvc-gadget.git

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Using UVC webcam gadget with a real v4l2 device

[Laurent Pinchart]

Hi Bhupesh,

On Monday 23 April 2012 02:24:53 Bhupesh SHARMA wrote:
 Hi Laurent,
 
 I have been doing some experimentation with the UVC webcam gadget along with
 the UVC user-space application which you have written.
 
 The UVC webcam gadget works fine with the user space application handling
 the CONTROL events and providing DATA events. Now, I wish to interface a
 real v4l2 device, for e.g. VIVI or more particularly a soc_camera based
 host and subdev pair.
 
 Now, I see that I can achieve this by opening the UVC and V4L2 devices and
 doing MMAP - REQBUF - QBUF - DQBUF calls on both the devices per the UVC
 control event received. But this will involve copying the video buffer in
 the user-space application from v4l2 (_CAPTURE) to uvc (_OUTPUT) domains,
 which will significantly reduce the video capture performance.
 
 Is there a better solution to this issue? Maybe doing something like a RNDIS
 gadget does with the help of u_ether.c like helper routines. But if I
 remember well it also requires the BRCTL (Bridge Control Utility) in
 userspace to route data arriving on usb0 to eth0 and vice-versa. Not sure
 though, if it does copying of a skb buffer from ethernet to usb domain and
 vice-versa.

To avoid copying data between the two devices you should use USERPTR instead 
of MMAP on at least one of the two V4L2 devices. The UVC gadget driver doesn't 
support USERPTR yet though. This shouldn't be too difficult to fix, we need to 
replace the custom buffers queue implementation with videobuf2, as has been 
done in the uvcvideo driver.

I'll try to implement this. Would you then be able to test patches ?

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: Using UVC webcam gadget with a real v4l2 device

[Bhupesh SHARMA]

Hi Laurent,

 -Original Message-
 From: Laurent Pinchart [mailto:laurent.pinch...@ideasonboard.com]
 Sent: Monday, April 23, 2012 7:47 PM
 To: Bhupesh SHARMA
 Cc: linux-...@vger.kernel.org; linux-media@vger.kernel.org;
 ba...@ti.com; g.liakhovet...@gmx.de
 Subject: Re: Using UVC webcam gadget with a real v4l2 device
 
 Hi Bhupesh,
 
 On Monday 23 April 2012 02:24:53 Bhupesh SHARMA wrote:
  Hi Laurent,
 
  I have been doing some experimentation with the UVC webcam gadget
 along with
  the UVC user-space application which you have written.
 
  The UVC webcam gadget works fine with the user space application
 handling
  the CONTROL events and providing DATA events. Now, I wish to
 interface a
  real v4l2 device, for e.g. VIVI or more particularly a soc_camera
 based
  host and subdev pair.
 
  Now, I see that I can achieve this by opening the UVC and V4L2
 devices and
  doing MMAP - REQBUF - QBUF - DQBUF calls on both the devices per
 the UVC
  control event received. But this will involve copying the video
 buffer in
  the user-space application from v4l2 (_CAPTURE) to uvc (_OUTPUT)
 domains,
  which will significantly reduce the video capture performance.
 
  Is there a better solution to this issue? Maybe doing something like
 a RNDIS
  gadget does with the help of u_ether.c like helper routines. But if I
  remember well it also requires the BRCTL (Bridge Control Utility) in
  userspace to route data arriving on usb0 to eth0 and vice-versa. Not
 sure
  though, if it does copying of a skb buffer from ethernet to usb
 domain and
  vice-versa.
 
 To avoid copying data between the two devices you should use USERPTR
 instead
 of MMAP on at least one of the two V4L2 devices. The UVC gadget driver
 doesn't
 support USERPTR yet though. This shouldn't be too difficult to fix, we
 need to
 replace the custom buffers queue implementation with videobuf2, as has
 been
 done in the uvcvideo driver.

I was thinking of using the USERPTR method too, but I realized that currently
neither UVC webcam gadget nor soc-camera subsystem supports this IO method.
They support only MMAP IO as of now :(

 
 I'll try to implement this. Would you then be able to test patches ?

For sure, I can test your patches on my setup.

BTW, I was exploring GSTREAMER to use the data arriving from soc-camera (v4l2)
capture device '/dev/video1' via 'v4l2src' plugin and routing the same to
the UVC gadget '/dev/video0' via the 'v4l2sink' plugin.

Don't know if this can work cleanly in my setup and whether GSTREAMER actually
performs a buffer copy internally. But I will at-least give it a try :)

Regards,
Bhupesh
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Using UVC webcam gadget with a real v4l2 device

[Laurent Pinchart]

Hi Bhupesh,

On Tuesday 24 April 2012 02:46:22 Bhupesh SHARMA wrote:
 On Monday, April 23, 2012 7:47 PM Laurent Pinchart wrote:
  On Monday 23 April 2012 02:24:53 Bhupesh SHARMA wrote:
   Hi Laurent,
   
   I have been doing some experimentation with the UVC webcam gadget along
   with the UVC user-space application which you have written.
   
   The UVC webcam gadget works fine with the user space application
   handling the CONTROL events and providing DATA events. Now, I wish to
   interface a real v4l2 device, for e.g. VIVI or more particularly a
   soc_camera based host and subdev pair.
   
   Now, I see that I can achieve this by opening the UVC and V4L2 devices
   and doing MMAP - REQBUF - QBUF - DQBUF calls on both the devices per
   the UVC control event received. But this will involve copying the video
   buffer in the user-space application from v4l2 (_CAPTURE) to uvc
   (_OUTPUT) domains, which will significantly reduce the video capture
   performance.
   
   Is there a better solution to this issue? Maybe doing something like a
   RNDIS gadget does with the help of u_ether.c like helper routines. But
   if I remember well it also requires the BRCTL (Bridge Control Utility)
   in userspace to route data arriving on usb0 to eth0 and vice-versa. Not
   sure though, if it does copying of a skb buffer from ethernet to usb
   domain and vice-versa.
  
  To avoid copying data between the two devices you should use USERPTR
  instead of MMAP on at least one of the two V4L2 devices. The UVC gadget
  driver doesn't support USERPTR yet though. This shouldn't be too difficult
  to fix, we need toreplace the custom buffers queue implementation with
  videobuf2, as has been done in the uvcvideo driver.
 
 I was thinking of using the USERPTR method too, but I realized that
 currently neither UVC webcam gadget nor soc-camera subsystem supports this
 IO method. They support only MMAP IO as of now :(

Both soc-camera and the UVC gadget driver should be ported to videobuf2 to fix 
the problem.

  I'll try to implement this. Would you then be able to test patches ?
 
 For sure, I can test your patches on my setup.

I had a quick look, but there's a bit more work than expected. The UVC gadget 
driver locking scheme needs to be revisited. I unfortunately won't have time 
to work on that in the next couple of weeks, and very probably not before end 
of June. Sorry.

If you want to give it a try, I can provide you with some pointers.

 BTW, I was exploring GSTREAMER to use the data arriving from soc-camera
 (v4l2) capture device '/dev/video1' via 'v4l2src' plugin and routing the
 same to the UVC gadget '/dev/video0' via the 'v4l2sink' plugin.
 
 Don't know if this can work cleanly in my setup and whether GSTREAMER
 actually performs a buffer copy internally. But I will at-least give it a
 try :)

There will definitely be a buffer copy (and actually two copies, as the UVC 
gadget driver performs a second copy internally) if you don't use USERPTR.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Using UVC webcam gadget with a real v4l2 device

[Peter Chen]

On Mon, Apr 23, 2012 at 2:24 AM, Bhupesh SHARMA bhupesh.sha...@st.com wrote:
 Hi Laurent,

 I have been doing some experimentation with the UVC webcam gadget along with 
 the UVC user-space
 application which you have written.

I have tried UVC webcam gadget at Freescale i.mx platform,
unfortunately,  It can't work
properly with my demo application. Would you tell me where I can get
Laurent's user-space
application? Thanks.

-- 
BR,
Peter Chen
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: UVC Webcam

[Laurent Pinchart]

Hi Gijo,

On Thursday 29 April 2010 12:54:16 Gijo Prems wrote:
 Hello,
 
 I have some queries related to linux uvc client driver(uvcvideo) and
 general uvc webcam functionality.
 
 1. There is a wDelay (during probe-commit) parameter which camera
 exposes to the host signifying the delay (Latency) inside the camera.
 Does the UVC driver on Linux Host expose this parameter to the
 application if they require it?

No it doesn't.

 And what would be the use case of this parameter?

I don't know, and that's exactly why the parameter isn't exposed :-)

 2. How the audio and video sync (lipsync) would happen on host side?

There's no sync at the moment. UVC supports timestamping the packets sent to 
the host, but the driver ignores the timestamps.

 3. How buffers are allocated on the host side?
 Which parameter from camera needs to be set to signify the correct
 buffer allocation?

There are two sets of buffers, the USB buffers and the V4L2 buffers.

The uvcvideo driver allocates one USB buffer per URB (the number of URBs is 
hardcoded to 5). The USB buffer size is set to the payload size returned by 
the device, bounded to a maximum value of 32 times the endpoint max packet 
size.

For V4L2 buffers, the uvcvideo driver uses the V4L2 MMAP streaming mode. 
Applications are supposed to first set the format (VIDIOC_S_FMT), and then ask 
the driver to allocate buffers (VIDIOC_REQBUFS).

 4. Are there any parameters in USB audio class which allocate the
 buffers and handles the latency at host?

I don't know much about the USB audio class, sorry.

 It would be great if someone could put some thoughts on these.

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

UVC Webcam

[Gijo Prems]

Hello,

I have some queries related to linux uvc client driver(uvcvideo) and
general uvc webcam functionality.

1. There is a wDelay (during probe-commit) parameter which camera
exposes to the host signifying the delay (Latency) inside the camera.
Does the UVC driver on Linux Host expose this parameter to the
application if they require it?
And what would be the use case of this parameter?

2. How the audio and video sync (lipsync) would happen on host side?

3. How buffers are allocated on the host side?
Which parameter from camera needs to be set to signify the correct
buffer allocation?

4. Are there any parameters in USB audio class which allocate the
buffers and handles the latency at host?

It would be great if someone could put some thoughts on these.

-Gijo
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html