Re: Bug: Two device nodes created in /dev for a single UVC webcam
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
> -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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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