Re: Regression at gspca core affecting SXGA mode on sn9c201 driver
Hi, On 12/09/2010 01:08 PM, Jean-Francois Moine wrote: On Wed, 08 Dec 2010 14:06:06 -0200 Mauro Carvalho Chehab wrote: Drivers should provide a wmaxpacketsize range which they need / can deal with for a given resolution. This way we can fix your does not need highest alt setting at lower resolutions scenario (which is a very valid one), while still allowing fallback to a lower alt setting if the driver can deal with lower settings too, be it by lowering framerate or by increasing compression (decreasing the image quality). It seems a good strategy. Hi, That seems fine also to me. p.s. If you look at my recent stv06xx changes you will see that this (having an allowable maxpacketsize per resolution) is already used there. You could use this to model more generic code for this after. Also, as a quicker hack, the 'audio' flag could be set only when USB is 1.1. Could that fix the problems found with sn9c20x and stv06xx? That won't help for the stv06xx, more over the skipping of the alt currently happens each time alt_xfer gets called. So lets say we have a came with alt 0-8, before the whole audio hack patch gspca_main would try: 8, 7, 6, ... Now it will try: 7, 5, 3, ... As it skips one alt because of the audio hack each time it tries another alt setting. Regards, Hans -- 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 at gspca core affecting SXGA mode on sn9c201 driver
Hi, On 12/09/2010 01:08 PM, Jean-Francois Moine wrote: On Wed, 08 Dec 2010 14:06:06 -0200 Mauro Carvalho Chehab wrote: Drivers should provide a wmaxpacketsize range which they need / can deal with for a given resolution. This way we can fix your does not need highest alt setting at lower resolutions scenario (which is a very valid one), while still allowing fallback to a lower alt setting if the driver can deal with lower settings too, be it by lowering framerate or by increasing compression (decreasing the image quality). It seems a good strategy. Hi, That seems fine also to me. Ok, so one of us needs to write a patch for this (and I'm rather busy atm). If you do write a patch for this, it would also be good to change the code which searches for the next lower alt setting to look at wmaxpacketsize, then the reverse_alts flag can go away, and more importantly then we will also work properly with cameras which do not have their alt settings sorted by size at all (I have at least one such cam). Regards, Hans -- 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 at gspca core affecting SXGA mode on sn9c201 driver
Em 09-12-2010 10:08, Jean-Francois Moine escreveu: > On Wed, 08 Dec 2010 14:06:06 -0200 > Mauro Carvalho Chehab wrote: > >>> Drivers should provide a wmaxpacketsize range which they need / can >>> deal with for a given resolution. This way we can fix your does not >>> need highest alt setting at lower resolutions scenario (which is a >>> very valid one), while still allowing fallback to a lower alt >>> setting if the driver can deal with lower settings too, be it by >>> lowering framerate or by increasing compression (decreasing the >>> image quality). >> >> It seems a good strategy. > > Hi, > > That seems fine also to me. > > Mauro, about your patch, there is no need to add the variable > 'no_audio_hack': just resetting the flag 'audio' in the subdrivers > should be enough. > > Also, as a quicker hack, the 'audio' flag could be set only when USB is > 1.1. Could that fix the problems found with sn9c20x and stv06xx? It probably will fix for sn9c20x and stv06xx, but, as Hans pointed, the patch logic is broken. I prefer to just revert the patch and apply another one with the right fix. Could you please cook such patch? Thanks, Mauro -- 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 at gspca core affecting SXGA mode on sn9c201 driver
On Wed, 08 Dec 2010 14:06:06 -0200 Mauro Carvalho Chehab wrote: > > Drivers should provide a wmaxpacketsize range which they need / can > > deal with for a given resolution. This way we can fix your does not > > need highest alt setting at lower resolutions scenario (which is a > > very valid one), while still allowing fallback to a lower alt > > setting if the driver can deal with lower settings too, be it by > > lowering framerate or by increasing compression (decreasing the > > image quality). > > It seems a good strategy. Hi, That seems fine also to me. Mauro, about your patch, there is no need to add the variable 'no_audio_hack': just resetting the flag 'audio' in the subdrivers should be enough. Also, as a quicker hack, the 'audio' flag could be set only when USB is 1.1. Could that fix the problems found with sn9c20x and stv06xx? Regards. -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ -- 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 at gspca core affecting SXGA mode on sn9c201 driver
Em 08-12-2010 12:11, Hans de Goede escreveu: > Hi, > > On 12/08/2010 01:54 PM, Mauro Carvalho Chehab wrote: >> Em 08-12-2010 08:01, Hans de Goede escreveu: >>> Hi, >>> >>> On 12/07/2010 09:01 PM, Mauro Carvalho Chehab wrote: Hi Jean-Fronçois, git commit 35680baa6822df98a6ed602e2380aa0a04e18b07 (see enclosed) caused not only a regression at PS/3 Eye webcam (git commit f43402fa55bf5e7e190c176343015122f694857c), but also at sn9c201 driver, when used on SXGA mode. What happens is that only the highest alternate mode is enough for the maximum resolution. I suspect that other drivers broke due to that change. So, IMO, the better is to revert and work on another alternative. >>> >>> I have to agree with Mauro here, dropping back to a lower alt setting for >>> all devices with a usb >>> audio interface is the wrong thing to do. >> >> >> Btw, I tested here with other hardware. It completely broke stv06xx driver >> for devices with audio >> (046d:08f6 Logitech, Inc. QuickCam Messenger Plus), as there's just one >> alternate config: >> >> gspca: no transfer endpoint found >> >> My proposal, for now, is to just revert the logic, enabling the hack only in >> the specific case where >> doing alt-1 work (see enclosed patch). I tested the enclosed patch and it >> fixed for PS/3, Quickcam >> and Gigaware (the 3 cams where I noticed the breakage). >> > > Hmm, looking at your patch I notice that the current patch is completely > wrong. Not only does it > lower the endpoint to use wrongly in many cases, it also does the lowering > *each* time we try > to start streaming, so if we have a cam with alt settings 0 - 8 and there is > not enough bandwidth > we used to try: 8, 7, 6, ... and now we will try 7, 5, 3, ... Notice the > skipping of an alt setting > each time. > > Before this broken, should be reverted, patch in some cases we used to lower > the nbalt setting in some > subdrivers where the highest alt setting causes issues for audio, which would > result in trying 7, 6, 5 > ... in the example, iow work as intended where as 7, 5, 3, ... clearly is not > work as intended. Agreed. Let's then just revert the broken patch. >> Of course, it will break the specific model(s) where the audio hack work, so >> we need to do: >> gspca_dev->audio_hack = 1; >> On the specific model(s) tested with the hack. >> >>> For example I know multiple devices where the highest alt setting does not >>> have a wMaxPacketSize >>> of 1023, but use something lower instead, to make sure there is enough >>> bandwidth left for usb audio >>> even if the highest alt setting get used. >>> Btw, no matter what resolution is used, sn9c201 is setting the same alternate for all modes, spending more USB bandwidth than needed. Why gspca is just getting the highest value for wMaxPacketSize? It should, instead, seek for the minimum packet size that is needed for a given resolution. >>> >>> There are 2 reasons for this: >>> 1) We do not know the minimum packetsize for a given resolution, unlike >>> with uvc where this >>> is arranged in the protocol, for proprietary apps we can only guess >> >> Ok, but, during the driver development, someone could test it. >> > > Right, but many of the drivers come from other sources (old v4l1 drivers) and > often we only > have one model of many supported models when porting these drivers to the new > gspca framework. Ok, so, for some drivers, we might never be able to have a proper fix, due to the lack of resources, but we should try to fix it where possible. >>> 2) Almost all devices supported by gspca are usb-1.1 and thus the framerate >>> is bandwidth >>> limited. We want to deliver the highest framerate possible, and thus >>> get as high an alt >>> setting as possible. >> >> This is a bogus argument. Core shouldn't assume that all devices are usb 1.1. > > You're right about the should not assume part, but that does not make this a > bogus argument > usb-1.1 webcams are bandwidth limited in their framerate, people don't like > low framerates > so clearly trying to get the highest alt setting and thus allow for the > highest framerate > is the right thing to do for 1.1 cameras. Allowing the highest framerate/resolution is the right thing to do also for 2.0 cameras. The point is that, if the user is selecting a lower resolution/framerate, drivers should use lower sizes, to save bandwidth. >> In the specific case of sn9c20x, it has 8 alternates. This probably means >> that all those alternates >> could be used, depending on the bandwidth requirements. >> >> At the maximum packet size (alt 8) means to spend 60% of the USB 2.0 traffic >> for isoc, being >> required for SXGA. Alt 7 spends 36% and it is enough for VGA resolution. >> >> If someone wants to have 2 webcams at VGA resolution, wasting bandwith using >> alt8 would prevent it, >> as core don't provide 120%
Re: Regression at gspca core affecting SXGA mode on sn9c201 driver
Hi, On 12/08/2010 01:54 PM, Mauro Carvalho Chehab wrote: Em 08-12-2010 08:01, Hans de Goede escreveu: Hi, On 12/07/2010 09:01 PM, Mauro Carvalho Chehab wrote: Hi Jean-Fronçois, git commit 35680baa6822df98a6ed602e2380aa0a04e18b07 (see enclosed) caused not only a regression at PS/3 Eye webcam (git commit f43402fa55bf5e7e190c176343015122f694857c), but also at sn9c201 driver, when used on SXGA mode. What happens is that only the highest alternate mode is enough for the maximum resolution. I suspect that other drivers broke due to that change. So, IMO, the better is to revert and work on another alternative. I have to agree with Mauro here, dropping back to a lower alt setting for all devices with a usb audio interface is the wrong thing to do. Btw, I tested here with other hardware. It completely broke stv06xx driver for devices with audio (046d:08f6 Logitech, Inc. QuickCam Messenger Plus), as there's just one alternate config: gspca: no transfer endpoint found My proposal, for now, is to just revert the logic, enabling the hack only in the specific case where doing alt-1 work (see enclosed patch). I tested the enclosed patch and it fixed for PS/3, Quickcam and Gigaware (the 3 cams where I noticed the breakage). Hmm, looking at your patch I notice that the current patch is completely wrong. Not only does it lower the endpoint to use wrongly in many cases, it also does the lowering *each* time we try to start streaming, so if we have a cam with alt settings 0 - 8 and there is not enough bandwidth we used to try: 8, 7, 6, ... and now we will try 7, 5, 3, ... Notice the skipping of an alt setting each time. Before this broken, should be reverted, patch in some cases we used to lower the nbalt setting in some subdrivers where the highest alt setting causes issues for audio, which would result in trying 7, 6, 5 ... in the example, iow work as intended where as 7, 5, 3, ... clearly is not work as intended. Of course, it will break the specific model(s) where the audio hack work, so we need to do: gspca_dev->audio_hack = 1; On the specific model(s) tested with the hack. For example I know multiple devices where the highest alt setting does not have a wMaxPacketSize of 1023, but use something lower instead, to make sure there is enough bandwidth left for usb audio even if the highest alt setting get used. Btw, no matter what resolution is used, sn9c201 is setting the same alternate for all modes, spending more USB bandwidth than needed. Why gspca is just getting the highest value for wMaxPacketSize? It should, instead, seek for the minimum packet size that is needed for a given resolution. There are 2 reasons for this: 1) We do not know the minimum packetsize for a given resolution, unlike with uvc where this is arranged in the protocol, for proprietary apps we can only guess Ok, but, during the driver development, someone could test it. Right, but many of the drivers come from other sources (old v4l1 drivers) and often we only have one model of many supported models when porting these drivers to the new gspca framework. 2) Almost all devices supported by gspca are usb-1.1 and thus the framerate is bandwidth limited. We want to deliver the highest framerate possible, and thus get as high an alt setting as possible. This is a bogus argument. Core shouldn't assume that all devices are usb 1.1. You're right about the should not assume part, but that does not make this a bogus argument usb-1.1 webcams are bandwidth limited in their framerate, people don't like low framerates so clearly trying to get the highest alt setting and thus allow for the highest framerate is the right thing to do for 1.1 cameras. In the specific case of sn9c20x, it has 8 alternates. This probably means that all those alternates could be used, depending on the bandwidth requirements. At the maximum packet size (alt 8) means to spend 60% of the USB 2.0 traffic for isoc, being required for SXGA. Alt 7 spends 36% and it is enough for VGA resolution. If someone wants to have 2 webcams at VGA resolution, wasting bandwith using alt8 would prevent it, as core don't provide 120% bandwidth ;) I agree that the highest setting should not be used when not needed. The alternate selection should be based on the desired resolution. It is a function of the imagesize, the frame rate, and the line width. On em28xx, we've made a function that estimates the needed alternate. This works pretty well. When the framerate is a fixed number this works well, but it is not a fixed number with webcams. More over some 1.1 webcams use "dynamic" compression algorithms where the bridge changes the compression quality setting based on available bandwidth. So here we may be able to do the same framerate at a lower alt setting, but this will result in a lower quality picture. I most note that many gspca subdrivers are defective wrt how this is all handled though, in
Re: Regression at gspca core affecting SXGA mode on sn9c201 driver
Em 08-12-2010 08:01, Hans de Goede escreveu:
> Hi,
>
> On 12/07/2010 09:01 PM, Mauro Carvalho Chehab wrote:
>> Hi Jean-Fronçois,
>>
>> git commit 35680baa6822df98a6ed602e2380aa0a04e18b07 (see enclosed) caused
>> not only a regression
>> at PS/3 Eye webcam (git commit f43402fa55bf5e7e190c176343015122f694857c),
>> but also at sn9c201 driver,
>> when used on SXGA mode. What happens is that only the highest alternate mode
>> is enough for the
>> maximum resolution.
>>
>> I suspect that other drivers broke due to that change. So, IMO, the better
>> is to revert and work
>> on another alternative.
>>
>
> I have to agree with Mauro here, dropping back to a lower alt setting for all
> devices with a usb
> audio interface is the wrong thing to do.
Btw, I tested here with other hardware. It completely broke stv06xx driver for
devices with audio
(046d:08f6 Logitech, Inc. QuickCam Messenger Plus), as there's just one
alternate config:
gspca: no transfer endpoint found
My proposal, for now, is to just revert the logic, enabling the hack only in
the specific case where
doing alt-1 work (see enclosed patch). I tested the enclosed patch and it fixed
for PS/3, Quickcam
and Gigaware (the 3 cams where I noticed the breakage).
Of course, it will break the specific model(s) where the audio hack work, so we
need to do:
gspca_dev->audio_hack = 1;
On the specific model(s) tested with the hack.
> For example I know multiple devices where the highest alt setting does not
> have a wMaxPacketSize
> of 1023, but use something lower instead, to make sure there is enough
> bandwidth left for usb audio
> even if the highest alt setting get used.
>
>> Btw, no matter what resolution is used, sn9c201 is setting the same
>> alternate for all modes,
>> spending more USB bandwidth than needed. Why gspca is just getting the
>> highest value for
>> wMaxPacketSize? It should, instead, seek for the minimum packet size that is
>> needed for a
>> given resolution.
>
> There are 2 reasons for this:
> 1) We do not know the minimum packetsize for a given resolution, unlike with
> uvc where this
>is arranged in the protocol, for proprietary apps we can only guess
Ok, but, during the driver development, someone could test it.
> 2) Almost all devices supported by gspca are usb-1.1 and thus the framerate
> is bandwidth
>limited. We want to deliver the highest framerate possible, and thus get
> as high an alt
>setting as possible.
This is a bogus argument. Core shouldn't assume that all devices are usb 1.1.
In the specific case of sn9c20x, it has 8 alternates. This probably means that
all those alternates
could be used, depending on the bandwidth requirements.
At the maximum packet size (alt 8) means to spend 60% of the USB 2.0 traffic
for isoc, being
required for SXGA. Alt 7 spends 36% and it is enough for VGA resolution.
If someone wants to have 2 webcams at VGA resolution, wasting bandwith using
alt8 would prevent it,
as core don't provide 120% bandwidth ;)
The alternate selection should be based on the desired resolution. It is a
function of
the imagesize, the frame rate, and the line width. On em28xx, we've made a
function that estimates
the needed alternate. This works pretty well.
> I most note that many gspca subdrivers are defective wrt how this is all
> handled though, in
> that they don't vary the framerate when the available bandwidth changes. So
> in many cases if
> the gspca core cannot get the highest alt setting things may not function at
> all (because the
> subdriver fails to select a lower framerate, resulting in parts of frames
> getting dropped
> due to bandwidth limitations).
Ok, but gspca shouldn't assume that subdrivers won't do it. I did a "grep alt"
and discovered that
most drivers just force the number of alternates to something else, like:
drivers/media/video/gspca/spca561.c: gspca_dev->nbalt = 7 + 1;
/* choose alternate 7 first */
Despite the comment, what the above does is to change the alternates count to 8
(0 to 7), instead of
asking the core to use alternate 7. Ah, and, due to the audio hack, the above
hack won't work anymore,
if the device has audio, as the core will decrement it to 6.
The only driver that seems to have a logic to work with different alts
per-resolution is xirlink_cit.c,
but it is based on a try-and-error approach.
Drivers shouldn't be allowed to change the number of alternates. Instead, the
core should provide a standard
way to use an alternate number provided by the sub-driver, or, otherwise,
fallback to an auto-select
mode. Something like:
if (gspca_dev->force_alt && gspca_dev->force_alt < gspca_dev->nbalt)
gspca_dev->alt = gspca_dev->force_alt;
else {
/* Current way */
}
> Fixing this requires:
> 1) Figuring out how to change the framerate for each sensor + bridge combo
> 2) Testing to see what framerate will still work at which alt setting
Re: Regression at gspca core affecting SXGA mode on sn9c201 driver
Hi, On 12/07/2010 09:01 PM, Mauro Carvalho Chehab wrote: Hi Jean-Fronçois, git commit 35680baa6822df98a6ed602e2380aa0a04e18b07 (see enclosed) caused not only a regression at PS/3 Eye webcam (git commit f43402fa55bf5e7e190c176343015122f694857c), but also at sn9c201 driver, when used on SXGA mode. What happens is that only the highest alternate mode is enough for the maximum resolution. I suspect that other drivers broke due to that change. So, IMO, the better is to revert and work on another alternative. I have to agree with Mauro here, dropping back to a lower alt setting for all devices with a usb audio interface is the wrong thing to do. For example I know multiple devices where the highest alt setting does not have a wMaxPacketSize of 1023, but use something lower instead, to make sure there is enough bandwidth left for usb audio even if the highest alt setting get used. Btw, no matter what resolution is used, sn9c201 is setting the same alternate for all modes, spending more USB bandwidth than needed. Why gspca is just getting the highest value for wMaxPacketSize? It should, instead, seek for the minimum packet size that is needed for a given resolution. There are 2 reasons for this: 1) We do not know the minimum packetsize for a given resolution, unlike with uvc where this is arranged in the protocol, for proprietary apps we can only guess 2) Almost all devices supported by gspca are usb-1.1 and thus the framerate is bandwidth limited. We want to deliver the highest framerate possible, and thus get as high an alt setting as possible. I most note that many gspca subdrivers are defective wrt how this is all handled though, in that they don't vary the framerate when the available bandwidth changes. So in many cases if the gspca core cannot get the highest alt setting things may not function at all (because the subdriver fails to select a lower framerate, resulting in parts of frames getting dropped due to bandwidth limitations). Fixing this requires: 1) Figuring out how to change the framerate for each sensor + bridge combo 2) Testing to see what framerate will still work at which alt setting Regards, Hans -- 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 at gspca core affecting SXGA mode on sn9c201 driver
Hi Jean-Fronçois, git commit 35680baa6822df98a6ed602e2380aa0a04e18b07 (see enclosed) caused not only a regression at PS/3 Eye webcam (git commit f43402fa55bf5e7e190c176343015122f694857c), but also at sn9c201 driver, when used on SXGA mode. What happens is that only the highest alternate mode is enough for the maximum resolution. I suspect that other drivers broke due to that change. So, IMO, the better is to revert and work on another alternative. Btw, no matter what resolution is used, sn9c201 is setting the same alternate for all modes, spending more USB bandwidth than needed. Why gspca is just getting the highest value for wMaxPacketSize? It should, instead, seek for the minimum packet size that is needed for a given resolution. I'll try to play with it, to fix the max res at least for sn9c201 driver. Cheers, Mauro -- 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
