gspca, audio and ov534: regression.

Message ID 20101006160441.6ee9583d.ospite@studenti.unina.it (mailing list archive)
State Superseded, archived
Headers

Commit Message

Antonio Ospite Oct. 6, 2010, 2:04 p.m. UTC
On Wed, 6 Oct 2010 13:48:55 +0200
Jean-Francois Moine <moinejf@free.fr> wrote:

> On Wed, 6 Oct 2010 12:33:21 +0200
> Antonio Ospite <ospite@studenti.unina.it> wrote:
> 
> > with 2.6.36-rc6 I can't use the ov534 gspca subdriver (with PS3 eye)
> > anymore, when I try to capture video in dmesg I get:
> > gspca: no transfer endpoint found
> > 
> > If I revert commit 35680ba I can make video capture work again but I
> > still don't get the audio device in pulseaudio, it shows up in
> > alsamixer but if I try to select it, on the console I get:
> > cannot load mixer controls: Invalid argument
> > 
[...]
> 
> I think I see why the commit prevents the webcam to work: as it is
> done, the choice of the alternate setting does not work with bulk
> transfer. A simple fix could be to also check bulk transfer when
> skipping an alt setting in the function get_ep().
>

Thanks, the following change fixes it, was this what you had in mind?



> About audio stream, I do not see how it can have been broken.
>

PS3 Eye audio is working with linux-2.6.33.7 it is broken in
linux-2.6.35.7 already, I'll try to further narrow down the interval.
Ah, alsamixer doesn't work even when the device is OK in pulseaudio...

> Might you send me the full USB information of your webcam?
>

You can find lsusb output attached.

Thanks,
   Antonio
  

Comments

Antonio Ospite Oct. 6, 2010, 2:53 p.m. UTC | #1
On Wed, 6 Oct 2010 16:04:41 +0200
Antonio Ospite <ospite@studenti.unina.it> wrote:

> On Wed, 6 Oct 2010 13:48:55 +0200
> Jean-Francois Moine <moinejf@free.fr> wrote:
> 
> > On Wed, 6 Oct 2010 12:33:21 +0200
> > Antonio Ospite <ospite@studenti.unina.it> wrote:
> > 
> > > with 2.6.36-rc6 I can't use the ov534 gspca subdriver (with PS3 eye)
> > > anymore, when I try to capture video in dmesg I get:
> > > gspca: no transfer endpoint found
> > > 
> > > If I revert commit 35680ba I can make video capture work again but I
> > > still don't get the audio device in pulseaudio, it shows up in
> > > alsamixer but if I try to select it, on the console I get:
> > > cannot load mixer controls: Invalid argument
> > > 
[...]
> > About audio stream, I do not see how it can have been broken.
> >
> 
> PS3 Eye audio is working with linux-2.6.33.7 it is broken in
> linux-2.6.35.7 already, I'll try to further narrow down the interval.
> Ah, alsamixer doesn't work even when the device is OK in pulseaudio...
> 

I was wrong, the audio part works even in 2.6.36-rc6 but _only_ when
the webcam is plugged in from boot, could this have to do with the order
gspca and snd-usb-audio are loaded?

Regards,
   Antonio
  
Jean-Francois Moine Oct. 6, 2010, 6:35 p.m. UTC | #2
On Wed, 6 Oct 2010 16:04:41 +0200
Antonio Ospite <ospite@studenti.unina.it> wrote:

> Thanks, the following change fixes it, was this what you had in mind?
> 
> diff --git a/drivers/media/video/gspca/gspca.c
> b/drivers/media/video/gspca/gspca.c index b984610..30e0b32 100644
> --- a/drivers/media/video/gspca/gspca.c
> +++ b/drivers/media/video/gspca/gspca.c
> @@ -651,7 +651,7 @@ static struct usb_host_endpoint *get_ep(struct
> gspca_dev *gspca_dev) : USB_ENDPOINT_XFER_ISOC;
>         i = gspca_dev->alt;                     /* previous alt
> setting */ if (gspca_dev->cam.reverse_alts) {
> -               if (gspca_dev->audio)
> +               if (gspca_dev->audio && !gspca_dev->cam.bulk)
>                         i++;
>                 while (++i < gspca_dev->nbalt) {
>                         ep = alt_xfer(&intf->altsetting[i], xfer);
> @@ -659,7 +659,7 @@ static struct usb_host_endpoint *get_ep(struct
> gspca_dev *gspca_dev) break;
>                 }
>         } else {
> -               if (gspca_dev->audio)
> +               if (gspca_dev->audio && !gspca_dev->cam.bulk)
>                         i--;
>                 while (--i >= 0) {
>                         ep = alt_xfer(&intf->altsetting[i], xfer);

Yes, but, after thought, as there is only one alternate setting, the
tests could be:
	if (gspca_dev->audio && i < gspca_dev->nbalt - 1)
and
	if (gspca_dev->audio && i > 0)

This should work also for isochronous transfers.

regards.
  
Jean-Francois Moine Oct. 7, 2010, 5:44 p.m. UTC | #3
On Wed, 6 Oct 2010 16:53:37 +0200
Antonio Ospite <ospite@studenti.unina.it> wrote:

> > PS3 Eye audio is working with linux-2.6.33.7 it is broken in
> > linux-2.6.35.7 already, I'll try to further narrow down the
> > interval. Ah, alsamixer doesn't work even when the device is OK in
> > pulseaudio... 
> 
> I was wrong, the audio part works even in 2.6.36-rc6 but _only_ when
> the webcam is plugged in from boot, could this have to do with the
> order gspca and snd-usb-audio are loaded?

Hi Antonio,

If you still have a kernel 2.6.33, may you try my test version (tarball
in my web page)? As it contain only the gspca stuff, this may tell if
the problem is in gspca or elsewhere in the kernel.

Best regards.
  
Antonio Ospite Oct. 10, 2010, 9:45 a.m. UTC | #4
On Thu, 7 Oct 2010 19:44:01 +0200
Jean-Francois Moine <moinejf@free.fr> wrote:

> On Wed, 6 Oct 2010 16:53:37 +0200
> Antonio Ospite <ospite@studenti.unina.it> wrote:
> 
> > > PS3 Eye audio is working with linux-2.6.33.7 it is broken in
> > > linux-2.6.35.7 already, I'll try to further narrow down the
> > > interval. Ah, alsamixer doesn't work even when the device is OK in
> > > pulseaudio... 
> > 
> > I was wrong, the audio part works even in 2.6.36-rc6 but _only_ when
> > the webcam is plugged in from boot, could this have to do with the
> > order gspca and snd-usb-audio are loaded?
> 
> Hi Antonio,
> 
> If you still have a kernel 2.6.33, may you try my test version (tarball
> in my web page)? As it contain only the gspca stuff, this may tell if
> the problem is in gspca or elsewhere in the kernel.
> 

JF I suspect the device not showing up in pulseaudio has nothing to do
with gspca at all. I can actually record audio using alsa even if
pulseaudio does not see the device, so it must be a pulseaudio issue.

Thanks,
   Antonio
  
Antonio Ospite Oct. 10, 2010, 10:02 a.m. UTC | #5
On Wed, 6 Oct 2010 20:35:53 +0200
Jean-Francois Moine <moinejf@free.fr> wrote:

> On Wed, 6 Oct 2010 16:04:41 +0200
> Antonio Ospite <ospite@studenti.unina.it> wrote:
> 
> > Thanks, the following change fixes it, was this what you had in mind?
> > 
> > diff --git a/drivers/media/video/gspca/gspca.c
[...]
> 
> Yes, but, after thought, as there is only one alternate setting, the
> tests could be:
> 	if (gspca_dev->audio && i < gspca_dev->nbalt - 1)
> and
> 	if (gspca_dev->audio && i > 0)
> 
> This should work also for isochronous transfers.

JF this change as is does not work for me, if I change the second check
to 
	if (gspca_dev->audio && i > 1)

it does, but I don't know if this breaks anything else.

In my case I have:

get_ep: gspca_dev->cam.reverse_alts: 0
get_ep: gspca_dev->alt: 1
get_ep: gspca_dev->nbalt: 1
get_ep: gspca_dev->audio: 1
get_ep: gspca_dev->cam.bulk: 1

Thanks,
   Antonio
  
Jean-Francois Moine Oct. 10, 2010, 10:21 a.m. UTC | #6
On Sun, 10 Oct 2010 12:02:50 +0200
Antonio Ospite <ospite@studenti.unina.it> wrote:

> JF this change as is does not work for me, if I change the second
> check to 
> 	if (gspca_dev->audio && i > 1)
> 
> it does, but I don't know if this breaks anything else.

Hi Antonio,

You are right, this is the way the test must be.

I'll try to have this in the kernel 2.6.36.

Regards.
  
Antonio Ospite Oct. 10, 2010, 11 a.m. UTC | #7
On Sun, 10 Oct 2010 12:21:29 +0200
Jean-Francois Moine <moinejf@free.fr> wrote:

> On Sun, 10 Oct 2010 12:02:50 +0200
> Antonio Ospite <ospite@studenti.unina.it> wrote:
> 
> > JF this change as is does not work for me, if I change the second
> > check to 
> > 	if (gspca_dev->audio && i > 1)
> > 
> > it does, but I don't know if this breaks anything else.
> 
> Hi Antonio,
> 
> You are right, this is the way the test must be.
> 
> I'll try to have this in the kernel 2.6.36.
> 

Thanks, feel free to add 

Reported-by: Antonio Ospite <ospite@studenti.unina.it>

or Tested-by or whatever-by you consider appropriate.

Regards,
  Antonio
  

Patch

diff --git a/drivers/media/video/gspca/gspca.c b/drivers/media/video/gspca/gspca.c
index b984610..30e0b32 100644
--- a/drivers/media/video/gspca/gspca.c
+++ b/drivers/media/video/gspca/gspca.c
@@ -651,7 +651,7 @@  static struct usb_host_endpoint *get_ep(struct gspca_dev *gspca_dev)
                                   : USB_ENDPOINT_XFER_ISOC;
        i = gspca_dev->alt;                     /* previous alt setting */
        if (gspca_dev->cam.reverse_alts) {
-               if (gspca_dev->audio)
+               if (gspca_dev->audio && !gspca_dev->cam.bulk)
                        i++;
                while (++i < gspca_dev->nbalt) {
                        ep = alt_xfer(&intf->altsetting[i], xfer);
@@ -659,7 +659,7 @@  static struct usb_host_endpoint *get_ep(struct gspca_dev *gspca_dev)
                                break;
                }
        } else {
-               if (gspca_dev->audio)
+               if (gspca_dev->audio && !gspca_dev->cam.bulk)
                        i--;
                while (--i >= 0) {
                        ep = alt_xfer(&intf->altsetting[i], xfer);