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

2018-03-04 Thread 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

2018-02-25 Thread 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

2018-02-25 Thread 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

2018-02-20 Thread 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

2018-02-20 Thread 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

2018-02-19 Thread 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

2018-02-19 Thread 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

2018-02-19 Thread 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

2018-02-19 Thread 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

2018-02-19 Thread 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

2018-02-17 Thread 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

2016-04-10 Thread 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

2012-08-03 Thread 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


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

2012-08-03 Thread 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

2012-08-01 Thread 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: Query regarding the support and testing of MJPEG frame type in the UVC webcam gadget

2012-08-01 Thread 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


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

2012-07-31 Thread 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: [PATCH 4/5] usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework

2012-07-25 Thread 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_

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

2012-07-16 Thread 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 
>>>> 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 
>>>> Signed-off-by: Mauro Carvalho Chehab 
>>> 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

2012-07-15 Thread 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 
> >> 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 
> >> Signed-off-by: Mauro Carvalho Chehab 
> > 
> > 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: [Regression 3.1->3.2, bisected] UVC-webcam: kernel panic when starting capturing

2012-07-15 Thread 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 
>> 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 
>> Signed-off-by: Mauro Carvalho Chehab 
> 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

2012-07-15 Thread 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 
> 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 
> Signed-off-by: Mauro Carvalho Chehab 

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


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

2012-07-12 Thread Frank Schäfer
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 
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 
Signed-off-by: Mauro Carvalho Chehab 



Regards,
Frank Schäfer



Output of lsusb -vvv:

device 2232:1005 ("WebCam SCB-0385N")

Bus 001 Device 004: ID 2232:1005 
Device Descriptor:
  bLength18
  bDescriptorType 1
  bcdUSB   2.00
  bDeviceClass  239 Miscellaneous Device
  bDeviceSubClass 2 ?
  bDeviceProtocol 1 Interface Association
  bMaxPacketSize064
  idVendor   0x2232
  idProduct  0x1005
  bcdDevice0.07
  iManufacturer   1 Image Processor
  iProduct2 WebCam SCB-0385N
  iSerial 0
  bNumConfigurations  1
  Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength  445
bNumInterfaces  2
bConfigurationValue 1
iConfiguration  0
bmAttributes 0x80
  (Bus Powered)
MaxPower  500mA
Interface Association:
  bLength 8
  bDescriptorType11
  bFirstInterface 0
  bInterfaceCount 2
  bFunctionClass 14 Video
  bFunctionSubClass   3 Video Interface Collection
  bFunctionProtocol   0
  iFunction   0
Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber0
  bAlternateSetting   0
  bNumEndpoints   1
  bInterfaceClass14 Video
  bInterfaceSubClass  1 Video Control
  bInterfaceProtocol  0
  iInterface  0
  VideoControl Interface Descriptor:
bLength13
bDescriptorType36
bDescriptorSubtype  1 (HEADER)
bcdUVC   1.00
wTotalLength   77
dwClockFrequency   30.00MHz
bInCollection   1
baInterfaceNr( 0)   1
  VideoControl Interface Descriptor:
bLength18
bDescriptorType36
bDescriptorSubtype  2 (INPUT_TERMINAL)
bTerminalID 1
wTerminalType  0x0201 Camera Sensor
bAssocTerminal  0
iTerminal   0
wObjectiveFocalLengthMin  0
wObjectiveFocalLengthMax  0
wOcularFocalLength0
bControlSize  3
bmControls   0x
  VideoControl Interface Descriptor:
bLength26
bDescriptorType36
bDescriptorSubtype  6 (EXTENSION_UNIT)
bUnitID 2
guidExtensionCode {92423946-d10c-e34a-8783-3133f9eaaa3b}
bNumControl 3
bNrPins 1
baSourceID( 0)  1
bControlSize1
bmControls( 0)   0xff
iExtension  0
  VideoControl Interface Descriptor:
bLength11
bDescriptorType36
bDescriptorSubtype  5 (PROCESSING_UNIT)
  Warning: Descriptor too short
bUnitID 3
bSourceID   2
wMaxMultiplier  0
bControlSize2
bmControls 0x043f
  Brightness
  Contrast
  Hue
  Saturation
  Sharpness
  Gamma
  Power Line Frequency
iProcessing 0
bmVideoStandards 0x 9
  None
  SECAM - 625/50
  VideoControl Interface Descriptor:
bLength 9
bDescriptorType36
bDescriptorSubtype  3 (OUTPUT_TERMINAL)
bTerminalID 4
wTerminalType  0x0101 USB Streaming
bAssocTerminal  0
bSourceID   3
iTerminal   0
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83  EP 3 IN

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

2012-07-07 Thread 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(stru

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

2012-07-03 Thread 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 
> > ---
> >  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

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

2012-06-18 Thread Laurent Pinchart
Hi Bhupesh,

On Monday 18 June 2012 18:14:16 Bhupesh SHARMA wrote:
> Hi Laurent,
> 
> Can you please review this patch and let me know if any modifications
> are required..

Done. I'm sorry for not having done this before, I've been really busy lately. 
I'll review 5/5 tomorrow.

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

2012-06-18 Thread 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 
> ---
>  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(!(

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

2012-06-18 Thread 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 
> ---
>  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 
>  #include 
>  #include 
>  #include 
> @@ -18,7 +19,8 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +
> +#include 
>
>  #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
> -

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

2012-06-15 Thread 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 
> 
> [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

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

2012-06-11 Thread 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 

[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 suppor

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

2012-06-08 Thread 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 
> > ---
> >  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 @

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

2012-06-08 Thread 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 
> ---
>  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,
> +
> + .b

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

2012-06-04 Thread 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 
> > > > 
> > > > 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


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

2012-06-04 Thread 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 
> > > 
> > > 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

2012-06-04 Thread 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 
> > > > >
> > > > > 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 som

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

2012-06-04 Thread 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 
> > > >
> > > > 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->irq

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

2012-06-04 Thread 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 
> > >
> > > 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 
> > Acked-by: Laurent Pinchart 
> 
> 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

--- Begin Message ---
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 
> Acked-by: Laurent Pinchart 

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

2012-06-04 Thread 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 
> > 
> > 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

2012-06-04 Thread 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 

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

2012-06-04 Thread 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 
> 
> 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


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

2012-06-01 Thread 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 
---
 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 
 #include 
 #include 
 #include 
@@ -18,7 +19,8 @@
 #include 
 #include 
 #include 
-#include 
+
+#include 
 
 #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 to protect the IRQ queue that holds the buffers to be processed by
+

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

2012-06-01 Thread 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 
---
 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 s

[PATCH 0/5] UVC webcam gadget related changes

2012-06-01 Thread 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


Re: Using UVC webcam gadget with a real v4l2 device

2012-05-02 Thread 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

2012-04-30 Thread 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

2012-04-30 Thread 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

2012-04-29 Thread 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-came

RE: Using UVC webcam gadget with a real v4l2 device

2012-04-25 Thread 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.

  

RE: Using UVC webcam gadget with a real v4l2 device

2012-04-25 Thread 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

2012-04-24 Thread 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

2012-04-23 Thread Peter Chen
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.

-- 
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: Using UVC webcam gadget with a real v4l2 device

2012-04-23 Thread 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

2012-04-23 Thread 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

2012-04-23 Thread 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


Using UVC webcam gadget with a real v4l2 device

2012-04-22 Thread Bhupesh SHARMA
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.

Any idea is much appreciated.

Thanks for your help,
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: UVC Webcam

2010-05-05 Thread 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

2010-04-30 Thread 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