Re: [GIT PULL FOR 2.6.37] uvcvideo: BKL removal

Re: [GIT PULL FOR 2.6.37] uvcvideo: BKL removal

[Laurent Pinchart]

Hi Mauro,

Please don't forget this pull request for 2.6.37.

On Monday 29 November 2010 11:15:10 Laurent Pinchart wrote:
> Hi Mauro,
> 
> The following changes since commit
> c796e203229c8c08250f9d372ae4e10c466b1787:
> 
>   [media] kconfig: add an option to determine a menu's visibility
> (2010-11-22 10:37:56 -0200)
> 
> are available in the git repository at:
>   git://linuxtv.org/pinchartl/uvcvideo.git uvcvideo-stable
> 
> They complete the BKL removal from the uvcvideo driver. Feedback received
> from Hans during review has been integrated.
> 
> Laurent Pinchart (5):
>       uvcvideo: Lock controls mutex when querying menus
>       uvcvideo: Move mutex lock/unlock inside uvc_free_buffers
>       uvcvideo: Move mmap() handler to uvc_queue.c
>       uvcvideo: Lock stream mutex when accessing format-related information
>       uvcvideo: Convert to unlocked_ioctl
> 
>  drivers/media/video/uvc/uvc_ctrl.c  |   48 +++++++++-
>  drivers/media/video/uvc/uvc_queue.c |  133 +++++++++++++++++++++-----
>  drivers/media/video/uvc/uvc_v4l2.c  |  185
> +++++++++++----------------------- drivers/media/video/uvc/uvc_video.c |  
>  3 -
>  drivers/media/video/uvc/uvcvideo.h  |   10 ++-
>  5 files changed, 222 insertions(+), 157 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [GIT PULL FOR 2.6.37] uvcvideo: BKL removal

[Mauro Carvalho Chehab]

Laurent,

Em 14-12-2010 08:55, Laurent Pinchart escreveu:
> Hi Mauro,
> 
> Please don't forget this pull request for 2.6.37.

Pull request for upstream sent today. 

I didn't find any regressions at the BKL removal patches, but I noticed a few 
issues with qv4l2, not all related to uvcvideo. The remaining of this email is an
attempt to document them for later fixes.

They don't seem to be regressions caused by BKL removal, but the better would be 
to fix them later.

- with uvcvideo and two video apps, if qv4l2 is started first, the second application 
doesn't start/capture. I suspect that REQBUFS (used by qv4l2 to probe mmap/userptr
capabilities) create some resource locking at uvcvideo. The proper way is to lock
the resources only if the driver is streaming, as other drivers and videobuf do.

- with saa7134 and qv4l2 (and after a fix for input capabilities): saa7134 and/or
qv4l2 doesn't seem to work fine if video format is changed to a 60HZ format (NTSC or
PAL/M). It keeps trying to use 576 lines, but the driver only works with 480 lines
for those formats. So, if qv4l2 tries to capture with STD/M, it fails, except if the
number of lines is manually fixed by the user.

- at least with the saa7134 board I used for test, video capture fails on some
conditions. This is not related to BKL patches. I suspect it may be some initialization
failure with the tuner (tda8275/tda8290), but I didn't have time to dig into it, nor
to test with a simpler saa7134 device. The device I used was an Avermedia m135.

> 
> On Monday 29 November 2010 11:15:10 Laurent Pinchart wrote:
>> Hi Mauro,
>>
>> The following changes since commit
>> c796e203229c8c08250f9d372ae4e10c466b1787:
>>
>>   [media] kconfig: add an option to determine a menu's visibility
>> (2010-11-22 10:37:56 -0200)
>>
>> are available in the git repository at:
>>   git://linuxtv.org/pinchartl/uvcvideo.git uvcvideo-stable
>>
>> They complete the BKL removal from the uvcvideo driver. Feedback received
>> from Hans during review has been integrated.



>>
>> Laurent Pinchart (5):
>>       uvcvideo: Lock controls mutex when querying menus
>>       uvcvideo: Move mutex lock/unlock inside uvc_free_buffers
>>       uvcvideo: Move mmap() handler to uvc_queue.c
>>       uvcvideo: Lock stream mutex when accessing format-related information
>>       uvcvideo: Convert to unlocked_ioctl
>>
>>  drivers/media/video/uvc/uvc_ctrl.c  |   48 +++++++++-
>>  drivers/media/video/uvc/uvc_queue.c |  133 +++++++++++++++++++++-----
>>  drivers/media/video/uvc/uvc_v4l2.c  |  185
>> +++++++++++----------------------- drivers/media/video/uvc/uvc_video.c |  
>>  3 -
>>  drivers/media/video/uvc/uvcvideo.h  |   10 ++-
>>  5 files changed, 222 insertions(+), 157 deletions(-)
> 

Re: [GIT PULL FOR 2.6.37] uvcvideo: BKL removal

[Laurent Pinchart]

Hi Mauro,

On Friday 17 December 2010 18:09:39 Mauro Carvalho Chehab wrote:
> Em 14-12-2010 08:55, Laurent Pinchart escreveu:
> > Hi Mauro,
> > 
> > Please don't forget this pull request for 2.6.37.
> 
> Pull request for upstream sent today.

Thank you.

> I didn't find any regressions at the BKL removal patches, but I noticed a
> few issues with qv4l2, not all related to uvcvideo. The remaining of this
> email is an attempt to document them for later fixes.
> 
> They don't seem to be regressions caused by BKL removal, but the better
> would be to fix them later.
> 
> - with uvcvideo and two video apps, if qv4l2 is started first, the second
> application doesn't start/capture. I suspect that REQBUFS (used by qv4l2
> to probe mmap/userptr capabilities) create some resource locking at
> uvcvideo. The proper way is to lock the resources only if the driver is
> streaming, as other drivers and videobuf do.

I don't agree with that. The uvcvideo driver has one buffer queue per device, 
so if an application requests buffers on one file handle it will lock other 
applications out. If the driver didn't it would be subject to race conditions.

> - with saa7134 and qv4l2 (and after a fix for input capabilities): saa7134
> and/or qv4l2 doesn't seem to work fine if video format is changed to a
> 60HZ format (NTSC or PAL/M). It keeps trying to use 576 lines, but the
> driver only works with 480 lines for those formats. So, if qv4l2 tries to
> capture with STD/M, it fails, except if the number of lines is manually
> fixed by the user.
> 
> - at least with the saa7134 board I used for test, video capture fails on
> some conditions. This is not related to BKL patches. I suspect it may be
> some initialization failure with the tuner (tda8275/tda8290), but I didn't
> have time to dig into it, nor to test with a simpler saa7134 device. The
> device I used was an Avermedia m135.

-- 
Regards,

Laurent Pinchart

Re: [GIT PULL FOR 2.6.37] uvcvideo: BKL removal

[Hans Verkuil]

On Saturday, December 18, 2010 01:54:41 Laurent Pinchart wrote:
> Hi Mauro,
> 
> On Friday 17 December 2010 18:09:39 Mauro Carvalho Chehab wrote:
> > Em 14-12-2010 08:55, Laurent Pinchart escreveu:
> > > Hi Mauro,
> > > 
> > > Please don't forget this pull request for 2.6.37.
> > 
> > Pull request for upstream sent today.
> 
> Thank you.
> 
> > I didn't find any regressions at the BKL removal patches, but I noticed a
> > few issues with qv4l2, not all related to uvcvideo. The remaining of this
> > email is an attempt to document them for later fixes.
> > 
> > They don't seem to be regressions caused by BKL removal, but the better
> > would be to fix them later.
> > 
> > - with uvcvideo and two video apps, if qv4l2 is started first, the second
> > application doesn't start/capture. I suspect that REQBUFS (used by qv4l2
> > to probe mmap/userptr capabilities) create some resource locking at
> > uvcvideo. The proper way is to lock the resources only if the driver is
> > streaming, as other drivers and videobuf do.
> 
> I don't agree with that. The uvcvideo driver has one buffer queue per device, 
> so if an application requests buffers on one file handle it will lock other 
> applications out. If the driver didn't it would be subject to race conditions.

I agree with Laurent. Once an application calls REQBUFS with non-zero count,
then it should lock the resources needed for streaming. The reason behind that
is that REQBUFS also locks the current selected format in place, since the
format determines the amount of memory needed for the buffers.

The reason a lot of drivers don't do this is partially because for many TV
capture drivers it is highly unlikely that the buffer size will change after
calling REQBUFS (there are basically only two formats: 720x480 or 720x576 and
users will normally never change between the two). However, this is much more
likely to happen for webcams and embedded systems supporting HDTV.

The other reason is probably because driver developers simple do not realize
they need to lock the resources on REQBUFS. I'm sure many existing drivers will
fail miserably if you change the format after calling REQBUFS (particularly
with mmap streaming mode).

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

Re: [GIT PULL FOR 2.6.37] uvcvideo: BKL removal

[Mauro Carvalho Chehab]

Em 18-12-2010 08:45, Hans Verkuil escreveu:
> On Saturday, December 18, 2010 01:54:41 Laurent Pinchart wrote:
>> Hi Mauro,
>>
>> On Friday 17 December 2010 18:09:39 Mauro Carvalho Chehab wrote:
>>> Em 14-12-2010 08:55, Laurent Pinchart escreveu:
>>>> Hi Mauro,
>>>>
>>>> Please don't forget this pull request for 2.6.37.
>>>
>>> Pull request for upstream sent today.
>>
>> Thank you.
>>
>>> I didn't find any regressions at the BKL removal patches, but I noticed a
>>> few issues with qv4l2, not all related to uvcvideo. The remaining of this
>>> email is an attempt to document them for later fixes.
>>>
>>> They don't seem to be regressions caused by BKL removal, but the better
>>> would be to fix them later.
>>>
>>> - with uvcvideo and two video apps, if qv4l2 is started first, the second
>>> application doesn't start/capture. I suspect that REQBUFS (used by qv4l2
>>> to probe mmap/userptr capabilities) create some resource locking at
>>> uvcvideo. The proper way is to lock the resources only if the driver is
>>> streaming, as other drivers and videobuf do.
>>
>> I don't agree with that. The uvcvideo driver has one buffer queue per device, 
>> so if an application requests buffers on one file handle it will lock other 
>> applications out. If the driver didn't it would be subject to race conditions.
> 
> I agree with Laurent. Once an application calls REQBUFS with non-zero count,
> then it should lock the resources needed for streaming. The reason behind that
> is that REQBUFS also locks the current selected format in place, since the
> format determines the amount of memory needed for the buffers.

qv4l2 calls REQBUFS(1), then REQBUFS(0). Well, this is currently wrong, as most
drivers will only release buffers at VIDIOC_STREAMOFF. Anyway, even replacing 
REQBUFS(0) with VIDIOC_STREAMOFF at qv4l2 won't help with uvcvideo. It seems that,
once buffers are requested at uvcvideo, they will release only at close().

One consequence on the way uvcvideo is currently doing it is that, if you use qv4l2,
it is impossible to change the video size, as it returns -EBUSY, if you ask it to
select a different format (even without streaming):
	$ ./qv4l2
	Set Capture Format: Device or resource busy

> The reason a lot of drivers don't do this is partially because for many TV
> capture drivers it is highly unlikely that the buffer size will change after
> calling REQBUFS (there are basically only two formats: 720x480 or 720x576 and
> users will normally never change between the two). However, this is much more
> likely to happen for webcams and embedded systems supporting HDTV.

What applications do, when they need to change the formats, is to call REQBUFS again.

> The other reason is probably because driver developers simple do not realize
> they need to lock the resources on REQBUFS. I'm sure many existing drivers will
> fail miserably if you change the format after calling REQBUFS (particularly
> with mmap streaming mode).

I didn't make any test, but I don't think they'll fail (at least, on the drivers
that use videobuf), as streaming format will be stored at the videobuf handling 
(at buffer_prepare callback).

So, if you change the format, the change will be applied only at the next call
to REQBUFS.

Cheers,
Mauro

Re: [GIT PULL FOR 2.6.37] uvcvideo: BKL removal

[Hans Verkuil]

On Monday, December 20, 2010 13:10:32 Mauro Carvalho Chehab wrote:
> Em 18-12-2010 08:45, Hans Verkuil escreveu:
> > On Saturday, December 18, 2010 01:54:41 Laurent Pinchart wrote:
> >> Hi Mauro,
> >>
> >> On Friday 17 December 2010 18:09:39 Mauro Carvalho Chehab wrote:
> >>> Em 14-12-2010 08:55, Laurent Pinchart escreveu:
> >>>> Hi Mauro,
> >>>>
> >>>> Please don't forget this pull request for 2.6.37.
> >>>
> >>> Pull request for upstream sent today.
> >>
> >> Thank you.
> >>
> >>> I didn't find any regressions at the BKL removal patches, but I noticed a
> >>> few issues with qv4l2, not all related to uvcvideo. The remaining of this
> >>> email is an attempt to document them for later fixes.
> >>>
> >>> They don't seem to be regressions caused by BKL removal, but the better
> >>> would be to fix them later.
> >>>
> >>> - with uvcvideo and two video apps, if qv4l2 is started first, the second
> >>> application doesn't start/capture. I suspect that REQBUFS (used by qv4l2
> >>> to probe mmap/userptr capabilities) create some resource locking at
> >>> uvcvideo. The proper way is to lock the resources only if the driver is
> >>> streaming, as other drivers and videobuf do.
> >>
> >> I don't agree with that. The uvcvideo driver has one buffer queue per device, 
> >> so if an application requests buffers on one file handle it will lock other 
> >> applications out. If the driver didn't it would be subject to race conditions.
> > 
> > I agree with Laurent. Once an application calls REQBUFS with non-zero count,
> > then it should lock the resources needed for streaming. The reason behind that
> > is that REQBUFS also locks the current selected format in place, since the
> > format determines the amount of memory needed for the buffers.
> 
> qv4l2 calls REQBUFS(1), then REQBUFS(0). Well, this is currently wrong, as most
> drivers will only release buffers at VIDIOC_STREAMOFF.

qv4l2 first calls STREAMOFF, then REQBUFS(1), then REQBUFS(0). In the hope that one
of these will actually free any buffers. It's random at the moment when drivers
release buffers, one of the reasons for using vb2.

> Anyway, even replacing 
> REQBUFS(0) with VIDIOC_STREAMOFF at qv4l2 won't help with uvcvideo. It seems that,
> once buffers are requested at uvcvideo, they will release only at close().
> 
> One consequence on the way uvcvideo is currently doing it is that, if you use qv4l2,
> it is impossible to change the video size, as it returns -EBUSY, if you ask it to
> select a different format (even without streaming):
> 	$ ./qv4l2
> 	Set Capture Format: Device or resource busy
> 
> > The reason a lot of drivers don't do this is partially because for many TV
> > capture drivers it is highly unlikely that the buffer size will change after
> > calling REQBUFS (there are basically only two formats: 720x480 or 720x576 and
> > users will normally never change between the two). However, this is much more
> > likely to happen for webcams and embedded systems supporting HDTV.
> 
> What applications do, when they need to change the formats, is to call REQBUFS again.
> 
> > The other reason is probably because driver developers simple do not realize
> > they need to lock the resources on REQBUFS. I'm sure many existing drivers will
> > fail miserably if you change the format after calling REQBUFS (particularly
> > with mmap streaming mode).
> 
> I didn't make any test, but I don't think they'll fail (at least, on the drivers
> that use videobuf), as streaming format will be stored at the videobuf handling 
> (at buffer_prepare callback).
> 
> So, if you change the format, the change will be applied only at the next call
> to REQBUFS.

This behavior isn't according to the spec. G/S_FMT relate to the current format,
not to some future format. Most non-videobuf drivers will not support this
behavior.

It should be simple, really:

STREAMOFF
REQBUFS(0)

That's all that should be needed to stop streaming and return all buffers to the
app. This is what uvc should also support (and I actually thought it did).

Attempts to change formats while buffers have been requested should be blocked
with EBUSY. It's all perfectly reasonable. Well, perhaps next year we might
succeed in having all drivers behave consistently...

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

Re: [GIT PULL FOR 2.6.37] uvcvideo: BKL removal

[Laurent Pinchart]

Hi Hans,

On Monday 20 December 2010 13:28:06 Hans Verkuil wrote:
> On Monday, December 20, 2010 13:10:32 Mauro Carvalho Chehab wrote:
> > Em 18-12-2010 08:45, Hans Verkuil escreveu:
> > > On Saturday, December 18, 2010 01:54:41 Laurent Pinchart wrote:
> > >> On Friday 17 December 2010 18:09:39 Mauro Carvalho Chehab wrote:
> > >>>
> > >>> I didn't find any regressions at the BKL removal patches, but I
> > >>> noticed a few issues with qv4l2, not all related to uvcvideo. The
> > >>> remaining of this email is an attempt to document them for later
> > >>> fixes.
> > >>> 
> > >>> They don't seem to be regressions caused by BKL removal, but the
> > >>> better would be to fix them later.
> > >>> 
> > >>> - with uvcvideo and two video apps, if qv4l2 is started first, the
> > >>> second application doesn't start/capture. I suspect that REQBUFS
> > >>> (used by qv4l2 to probe mmap/userptr capabilities) create some
> > >>> resource locking at uvcvideo. The proper way is to lock the
> > >>> resources only if the driver is streaming, as other drivers and
> > >>> videobuf do.
> > >> 
> > >> I don't agree with that. The uvcvideo driver has one buffer queue per
> > >> device, so if an application requests buffers on one file handle it
> > >> will lock other applications out. If the driver didn't it would be
> > >> subject to race conditions.
> > > 
> > > I agree with Laurent. Once an application calls REQBUFS with non-zero
> > > count, then it should lock the resources needed for streaming. The
> > > reason behind that is that REQBUFS also locks the current selected
> > > format in place, since the format determines the amount of memory
> > > needed for the buffers.
> > 
> > qv4l2 calls REQBUFS(1), then REQBUFS(0). Well, this is currently wrong,
> > as most drivers will only release buffers at VIDIOC_STREAMOFF.
> 
> qv4l2 first calls STREAMOFF, then REQBUFS(1), then REQBUFS(0). In the hope
> that one of these will actually free any buffers. It's random at the
> moment when drivers release buffers, one of the reasons for using vb2.
> 
> > Anyway, even replacing
> > REQBUFS(0) with VIDIOC_STREAMOFF at qv4l2 won't help with uvcvideo. It
> > seems that, once buffers are requested at uvcvideo, they will release
> > only at close().

That's not correct. Buffers are released when calling REQBUFS(0). However, the 
file handle is still marked as owning the device for streaming purpose, so 
other applications can't change the format or request buffers.

> > One consequence on the way uvcvideo is currently doing it is that, if you
> > use qv4l2, it is impossible to change the video size, as it returns
> > -EBUSY, if you ask it to
> > 
> > select a different format (even without streaming):
> > 	$ ./qv4l2
> > 	Set Capture Format: Device or resource busy
> > 	
> > > The reason a lot of drivers don't do this is partially because for many
> > > TV capture drivers it is highly unlikely that the buffer size will
> > > change after calling REQBUFS (there are basically only two formats:
> > > 720x480 or 720x576 and users will normally never change between the
> > > two). However, this is much more likely to happen for webcams and
> > > embedded systems supporting HDTV.
> > 
> > What applications do, when they need to change the formats, is to call
> > REQBUFS again.
> > 
> > > The other reason is probably because driver developers simple do not
> > > realize they need to lock the resources on REQBUFS. I'm sure many
> > > existing drivers will fail miserably if you change the format after
> > > calling REQBUFS (particularly with mmap streaming mode).
> > 
> > I didn't make any test, but I don't think they'll fail (at least, on the
> > drivers that use videobuf), as streaming format will be stored at the
> > videobuf handling (at buffer_prepare callback).
> > 
> > So, if you change the format, the change will be applied only at the next
> > call to REQBUFS.
> 
> This behavior isn't according to the spec. G/S_FMT relate to the current
> format, not to some future format. Most non-videobuf drivers will not
> support this behavior.
> 
> It should be simple, really:
> 
> STREAMOFF
> REQBUFS(0)
> 
> That's all that should be needed to stop streaming and return all buffers
> to the app. This is what uvc should also support (and I actually thought
> it did).

That's what uvcvideo does.

> Attempts to change formats while buffers have been requested should be
> blocked with EBUSY. It's all perfectly reasonable. Well, perhaps next year
> we might succeed in having all drivers behave consistently...

-- 
Regards,

Laurent Pinchart

Re: [GIT PULL FOR 2.6.37] uvcvideo: BKL removal

[Hans Verkuil]

On Monday, December 20, 2010 13:35:28 Laurent Pinchart wrote:
> Hi Hans,
> 
> On Monday 20 December 2010 13:28:06 Hans Verkuil wrote:
> > On Monday, December 20, 2010 13:10:32 Mauro Carvalho Chehab wrote:
> > > Em 18-12-2010 08:45, Hans Verkuil escreveu:
> > > > On Saturday, December 18, 2010 01:54:41 Laurent Pinchart wrote:
> > > >> On Friday 17 December 2010 18:09:39 Mauro Carvalho Chehab wrote:
> > > >>>
> > > >>> I didn't find any regressions at the BKL removal patches, but I
> > > >>> noticed a few issues with qv4l2, not all related to uvcvideo. The
> > > >>> remaining of this email is an attempt to document them for later
> > > >>> fixes.
> > > >>> 
> > > >>> They don't seem to be regressions caused by BKL removal, but the
> > > >>> better would be to fix them later.
> > > >>> 
> > > >>> - with uvcvideo and two video apps, if qv4l2 is started first, the
> > > >>> second application doesn't start/capture. I suspect that REQBUFS
> > > >>> (used by qv4l2 to probe mmap/userptr capabilities) create some
> > > >>> resource locking at uvcvideo. The proper way is to lock the
> > > >>> resources only if the driver is streaming, as other drivers and
> > > >>> videobuf do.
> > > >> 
> > > >> I don't agree with that. The uvcvideo driver has one buffer queue per
> > > >> device, so if an application requests buffers on one file handle it
> > > >> will lock other applications out. If the driver didn't it would be
> > > >> subject to race conditions.
> > > > 
> > > > I agree with Laurent. Once an application calls REQBUFS with non-zero
> > > > count, then it should lock the resources needed for streaming. The
> > > > reason behind that is that REQBUFS also locks the current selected
> > > > format in place, since the format determines the amount of memory
> > > > needed for the buffers.
> > > 
> > > qv4l2 calls REQBUFS(1), then REQBUFS(0). Well, this is currently wrong,
> > > as most drivers will only release buffers at VIDIOC_STREAMOFF.
> > 
> > qv4l2 first calls STREAMOFF, then REQBUFS(1), then REQBUFS(0). In the hope
> > that one of these will actually free any buffers. It's random at the
> > moment when drivers release buffers, one of the reasons for using vb2.
> > 
> > > Anyway, even replacing
> > > REQBUFS(0) with VIDIOC_STREAMOFF at qv4l2 won't help with uvcvideo. It
> > > seems that, once buffers are requested at uvcvideo, they will release
> > > only at close().
> 
> That's not correct. Buffers are released when calling REQBUFS(0). However, the 
> file handle is still marked as owning the device for streaming purpose, so 
> other applications can't change the format or request buffers.

Why? After REQBUFS(0) any filehandle ownership should be dropped.

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

Re: [GIT PULL FOR 2.6.37] uvcvideo: BKL removal

[Laurent Pinchart]

Hi Hans,

On Monday 20 December 2010 13:45:45 Hans Verkuil wrote:
> On Monday, December 20, 2010 13:35:28 Laurent Pinchart wrote:
> > On Monday 20 December 2010 13:28:06 Hans Verkuil wrote:
> > > On Monday, December 20, 2010 13:10:32 Mauro Carvalho Chehab wrote:
> > > > Em 18-12-2010 08:45, Hans Verkuil escreveu:
> > > > > On Saturday, December 18, 2010 01:54:41 Laurent Pinchart wrote:
> > > > >> On Friday 17 December 2010 18:09:39 Mauro Carvalho Chehab wrote:
> > > > >>> I didn't find any regressions at the BKL removal patches, but I
> > > > >>> noticed a few issues with qv4l2, not all related to uvcvideo. The
> > > > >>> remaining of this email is an attempt to document them for later
> > > > >>> fixes.
> > > > >>> 
> > > > >>> They don't seem to be regressions caused by BKL removal, but the
> > > > >>> better would be to fix them later.
> > > > >>> 
> > > > >>> - with uvcvideo and two video apps, if qv4l2 is started first,
> > > > >>> the second application doesn't start/capture. I suspect that
> > > > >>> REQBUFS (used by qv4l2 to probe mmap/userptr capabilities)
> > > > >>> create some resource locking at uvcvideo. The proper way is to
> > > > >>> lock the resources only if the driver is streaming, as other
> > > > >>> drivers and videobuf do.
> > > > >> 
> > > > >> I don't agree with that. The uvcvideo driver has one buffer queue
> > > > >> per device, so if an application requests buffers on one file
> > > > >> handle it will lock other applications out. If the driver didn't
> > > > >> it would be subject to race conditions.
> > > > > 
> > > > > I agree with Laurent. Once an application calls REQBUFS with
> > > > > non-zero count, then it should lock the resources needed for
> > > > > streaming. The reason behind that is that REQBUFS also locks the
> > > > > current selected format in place, since the format determines the
> > > > > amount of memory needed for the buffers.
> > > > 
> > > > qv4l2 calls REQBUFS(1), then REQBUFS(0). Well, this is currently
> > > > wrong, as most drivers will only release buffers at
> > > > VIDIOC_STREAMOFF.
> > > 
> > > qv4l2 first calls STREAMOFF, then REQBUFS(1), then REQBUFS(0). In the
> > > hope that one of these will actually free any buffers. It's random at
> > > the moment when drivers release buffers, one of the reasons for using
> > > vb2.
> > > 
> > > > Anyway, even replacing
> > > > REQBUFS(0) with VIDIOC_STREAMOFF at qv4l2 won't help with uvcvideo.
> > > > It seems that, once buffers are requested at uvcvideo, they will
> > > > release only at close().
> > 
> > That's not correct. Buffers are released when calling REQBUFS(0).
> > However, the file handle is still marked as owning the device for
> > streaming purpose, so other applications can't change the format or
> > request buffers.
> 
> Why? After REQBUFS(0) any filehandle ownership should be dropped.

What if the application wants to change the resolution during capture ? It 
will have to stop capture, call REQBUFS(0), change the format, request buffers 
and restart capture. If filehandle ownership is dropped after REQBUFS(0) that 
will open the door to a race condition.

-- 
Regards,

Laurent Pinchart

Re: [GIT PULL FOR 2.6.37] uvcvideo: BKL removal

[Hans Verkuil]

On Monday, December 20, 2010 13:48:51 Laurent Pinchart wrote:
> Hi Hans,
> 
> On Monday 20 December 2010 13:45:45 Hans Verkuil wrote:
> > On Monday, December 20, 2010 13:35:28 Laurent Pinchart wrote:
> > > On Monday 20 December 2010 13:28:06 Hans Verkuil wrote:
> > > > On Monday, December 20, 2010 13:10:32 Mauro Carvalho Chehab wrote:
> > > > > Em 18-12-2010 08:45, Hans Verkuil escreveu:
> > > > > > On Saturday, December 18, 2010 01:54:41 Laurent Pinchart wrote:
> > > > > >> On Friday 17 December 2010 18:09:39 Mauro Carvalho Chehab wrote:
> > > > > >>> I didn't find any regressions at the BKL removal patches, but I
> > > > > >>> noticed a few issues with qv4l2, not all related to uvcvideo. The
> > > > > >>> remaining of this email is an attempt to document them for later
> > > > > >>> fixes.
> > > > > >>> 
> > > > > >>> They don't seem to be regressions caused by BKL removal, but the
> > > > > >>> better would be to fix them later.
> > > > > >>> 
> > > > > >>> - with uvcvideo and two video apps, if qv4l2 is started first,
> > > > > >>> the second application doesn't start/capture. I suspect that
> > > > > >>> REQBUFS (used by qv4l2 to probe mmap/userptr capabilities)
> > > > > >>> create some resource locking at uvcvideo. The proper way is to
> > > > > >>> lock the resources only if the driver is streaming, as other
> > > > > >>> drivers and videobuf do.
> > > > > >> 
> > > > > >> I don't agree with that. The uvcvideo driver has one buffer queue
> > > > > >> per device, so if an application requests buffers on one file
> > > > > >> handle it will lock other applications out. If the driver didn't
> > > > > >> it would be subject to race conditions.
> > > > > > 
> > > > > > I agree with Laurent. Once an application calls REQBUFS with
> > > > > > non-zero count, then it should lock the resources needed for
> > > > > > streaming. The reason behind that is that REQBUFS also locks the
> > > > > > current selected format in place, since the format determines the
> > > > > > amount of memory needed for the buffers.
> > > > > 
> > > > > qv4l2 calls REQBUFS(1), then REQBUFS(0). Well, this is currently
> > > > > wrong, as most drivers will only release buffers at
> > > > > VIDIOC_STREAMOFF.
> > > > 
> > > > qv4l2 first calls STREAMOFF, then REQBUFS(1), then REQBUFS(0). In the
> > > > hope that one of these will actually free any buffers. It's random at
> > > > the moment when drivers release buffers, one of the reasons for using
> > > > vb2.
> > > > 
> > > > > Anyway, even replacing
> > > > > REQBUFS(0) with VIDIOC_STREAMOFF at qv4l2 won't help with uvcvideo.
> > > > > It seems that, once buffers are requested at uvcvideo, they will
> > > > > release only at close().
> > > 
> > > That's not correct. Buffers are released when calling REQBUFS(0).
> > > However, the file handle is still marked as owning the device for
> > > streaming purpose, so other applications can't change the format or
> > > request buffers.
> > 
> > Why? After REQBUFS(0) any filehandle ownership should be dropped.
> 
> What if the application wants to change the resolution during capture ? It 
> will have to stop capture, call REQBUFS(0), change the format, request buffers 
> and restart capture. If filehandle ownership is dropped after REQBUFS(0) that 
> will open the door to a race condition.

That's why S_PRIORITY was invented.

One of the nice properties of V4L2 is the ability to allow multiple processes to
access the same device at the same time and even make modifications where possible.

To prevent unwanted modifications the priority scheme was created. Unfortunately,
too many drivers do not support it. It really should be core functionality. I did
work on it earlier in the year, but never finished it, although it shouldn't be
hard to do.

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

Re: [GIT PULL FOR 2.6.37] uvcvideo: BKL removal

[Laurent Pinchart]

Hi Hans,

On Monday 20 December 2010 14:09:40 Hans Verkuil wrote:
> On Monday, December 20, 2010 13:48:51 Laurent Pinchart wrote:
> >
> > What if the application wants to change the resolution during capture ?
> > It will have to stop capture, call REQBUFS(0), change the format,
> > request buffers and restart capture. If filehandle ownership is dropped
> > after REQBUFS(0) that will open the door to a race condition.
> 
> That's why S_PRIORITY was invented.

Right, I should implement that. I think the documentation isn't clear though. 
What is the background priority for exactly ? And the "unset" priority ? Are 
other applications allowed to change controls when an application has the 
record priority ?

In general I find the priority ioctls underspecified, that's why I haven't 
implemented them yet.

On a side note, I've just tested the latest uvcvideo driver, and I've 
successfully captured video using a second application after calling 
REQBUFS(0) in a first application.

-- 
Regards,

Laurent Pinchart

Re: [GIT PULL FOR 2.6.37] uvcvideo: BKL removal

[Hans Verkuil]

On Thursday, December 23, 2010 10:02:33 Laurent Pinchart wrote:
> Hi Hans,
> 
> On Monday 20 December 2010 14:09:40 Hans Verkuil wrote:
> > On Monday, December 20, 2010 13:48:51 Laurent Pinchart wrote:
> > >
> > > What if the application wants to change the resolution during capture ?
> > > It will have to stop capture, call REQBUFS(0), change the format,
> > > request buffers and restart capture. If filehandle ownership is dropped
> > > after REQBUFS(0) that will open the door to a race condition.
> > 
> > That's why S_PRIORITY was invented.
> 
> Right, I should implement that. I think the documentation isn't clear though. 
> What is the background priority for exactly ?

As the documentation mentions, it can be used for background processes monitoring
VBI (e.g. teletext) transmissions. I'm not aware of any such applications, though.

PRIORITY_DEFAULT and PRIORITY_RECORD are the only two relevant prios in practice.

> And the "unset" priority ?

Internal prio only. I think it's the value when no file handle is open.

> Are 
> other applications allowed to change controls when an application has the 
> record priority ?

No. Only read-only ioctls can be executed.

> In general I find the priority ioctls underspecified, that's why I haven't 
> implemented them yet.

Use the prio support functions in v4l2-common. They are easy to use and do the
job.

Regards,

	Hans

> On a side note, I've just tested the latest uvcvideo driver, and I've 
> successfully captured video using a second application after calling 
> REQBUFS(0) in a first application.
> 
> 

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

Re: [GIT PULL FOR 2.6.37] uvcvideo: BKL removal

[Laurent Pinchart]

Hi Hans,

On Thursday 23 December 2010 10:20:17 Hans Verkuil wrote:
> On Thursday, December 23, 2010 10:02:33 Laurent Pinchart wrote:
> > On Monday 20 December 2010 14:09:40 Hans Verkuil wrote:
> > > On Monday, December 20, 2010 13:48:51 Laurent Pinchart wrote:
> > > > What if the application wants to change the resolution during capture
> > > > ? It will have to stop capture, call REQBUFS(0), change the format,
> > > > request buffers and restart capture. If filehandle ownership is
> > > > dropped after REQBUFS(0) that will open the door to a race
> > > > condition.
> > > 
> > > That's why S_PRIORITY was invented.
> > 
> > Right, I should implement that. I think the documentation isn't clear
> > though. What is the background priority for exactly ?
> 
> As the documentation mentions, it can be used for background processes
> monitoring VBI (e.g. teletext) transmissions. I'm not aware of any such
> applications, though.
> 
> PRIORITY_DEFAULT and PRIORITY_RECORD are the only two relevant prios in
> practice.
> 
> > And the "unset" priority ?
> 
> Internal prio only. I think it's the value when no file handle is open.

Aren't priorities associated with file handles ?

> > Are other applications allowed to change controls when an application has
> > the record priority ?
> 
> No. Only read-only ioctls can be executed.

Then we got an issue here. I want an application to be able to acquire 
exclusive streaming rights on the device (so that there won't be race 
conditions when changing the resolution), but still allow other applications 
to change controls.

> > In general I find the priority ioctls underspecified, that's why I
> > haven't implemented them yet.
> 
> Use the prio support functions in v4l2-common. They are easy to use and do
> the job.

-- 
Regards,

Laurent Pinchart

Re: [GIT PULL FOR 2.6.37] uvcvideo: BKL removal

[Hans Verkuil]

On Thursday, December 23, 2010 10:27:25 Laurent Pinchart wrote:
> Hi Hans,
> 
> On Thursday 23 December 2010 10:20:17 Hans Verkuil wrote:
> > On Thursday, December 23, 2010 10:02:33 Laurent Pinchart wrote:
> > > On Monday 20 December 2010 14:09:40 Hans Verkuil wrote:
> > > > On Monday, December 20, 2010 13:48:51 Laurent Pinchart wrote:
> > > > > What if the application wants to change the resolution during capture
> > > > > ? It will have to stop capture, call REQBUFS(0), change the format,
> > > > > request buffers and restart capture. If filehandle ownership is
> > > > > dropped after REQBUFS(0) that will open the door to a race
> > > > > condition.
> > > > 
> > > > That's why S_PRIORITY was invented.
> > > 
> > > Right, I should implement that. I think the documentation isn't clear
> > > though. What is the background priority for exactly ?
> > 
> > As the documentation mentions, it can be used for background processes
> > monitoring VBI (e.g. teletext) transmissions. I'm not aware of any such
> > applications, though.
> > 
> > PRIORITY_DEFAULT and PRIORITY_RECORD are the only two relevant prios in
> > practice.
> > 
> > > And the "unset" priority ?
> > 
> > Internal prio only. I think it's the value when no file handle is open.
> 
> Aren't priorities associated with file handles ?

Yes, but there is also a global prio.

> > > Are other applications allowed to change controls when an application has
> > > the record priority ?
> > 
> > No. Only read-only ioctls can be executed.
> 
> Then we got an issue here. I want an application to be able to acquire 
> exclusive streaming rights on the device (so that there won't be race 
> conditions when changing the resolution), but still allow other applications 
> to change controls.

Why? The whole idea of prios is that once you are PRIO_RECORD no one else
can mess with settings. Allowing other apps access to controls will make it
possible to e.g. change the contrast to some crappy value. Not acceptable.

If the only thing you want to use PRIO_RECORD for in apps is to prevent this
'race condition' (I don't really see this as a real race, to be honest), then
the app can raise the prio to RECORD just before STREAMOFF, change the resolution,
start streaming again and lower it to the default prio.

Regards,

	Hans

> > > In general I find the priority ioctls underspecified, that's why I
> > > haven't implemented them yet.
> > 
> > Use the prio support functions in v4l2-common. They are easy to use and do
> > the job.
> 
> 

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

Re: [GIT PULL FOR 2.6.37] uvcvideo: BKL removal

[Andy Walls]

On Thu, 2010-12-23 at 10:02 +0100, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Monday 20 December 2010 14:09:40 Hans Verkuil wrote:
> > On Monday, December 20, 2010 13:48:51 Laurent Pinchart wrote:
> > >
> > > What if the application wants to change the resolution during capture ?
> > > It will have to stop capture, call REQBUFS(0), change the format,
> > > request buffers and restart capture. If filehandle ownership is dropped
> > > after REQBUFS(0) that will open the door to a race condition.
> > 
> > That's why S_PRIORITY was invented.
> 
> Right, I should implement that. I think the documentation isn't clear though. 
> What is the background priority for exactly ? 

http://linuxtv.org/downloads/v4l-dvb-apis/app-pri.html

"Another objective is to permit low priority applications working in
background, which can be preempted by user controlled applications and
automatically regain control of the device at a later time."

<contrived examples>
A use case would be for a daemon that does background channel scanning
or VBI data collection when the user isn't doing something else with the
video device.

For a camera, maybe it would be an application that does device health
monitoring, some sort of continuous calibration, or motion detection
when the device isn't in use by the user for viewing.
</contrived examples>

> And the "unset" priority ?

When checking for the maximum priority in use, it indicates that no
nodes have any priorities. See v4l2_prio_max().  For a driver that
supports priorities, it means no device nodes are opened, since any
device node being open would get a priority of
V4L2_PRIORITY_INTERACTIVE/_DEFAULT by default in v4l2_prio_open().

V4L2_PRIORITY_UNSET is also used to indicate to the v4l2_prio_*()
functions that a struct v4l2_prio_state hasn't been initialized (i.e.
has just been kzalloc()'ed).

> Are 
> other applications allowed to change controls when an application has the 
> record priority ?

According to the spec, no:

"V4L2_PRIORITY_RECORD 3 Highest priority. Only one file descriptor can
have this priority, it blocks any other fd from changing device
properties."

Once an automated, scheduled-recording process is recording, one really
wouldn't want another process changing them.  I personally would not
like unpredictable volume level variations on my TV show recordings.


In cx18-controls.c one can find an implementation example for the old
control framework:

        int cx18_s_ext_ctrls(struct file *file, void *fh, struct v4l2_ext_controls *c)
        {       
                struct cx18_open_id *id = fh;   
                struct cx18 *cx = id->cx;       
                int ret;
                struct v4l2_control ctrl;       
        
                ret = v4l2_prio_check(&cx->prio, id->prio);
                if (ret)
                        return ret;
    [...]

However, the new control framework in v4l2-ctrl.c seems to be missing
any v4l2_prio_check() calls. The ioctl() handling in v4l2-ioctl.c
doesn't have any either.


> In general I find the priority ioctls underspecified, that's why I haven't 
> implemented them yet.

Yes, it appears some of the details should be gleaned from the existing
driver implementations before they are/were converted to the new control
framework.

Regards,
Andy

Re: [GIT PULL FOR 2.6.37] uvcvideo: BKL removal

[Hans Verkuil]

On Thursday, December 23, 2010 14:04:38 Andy Walls wrote:
> On Thu, 2010-12-23 at 10:02 +0100, Laurent Pinchart wrote:
> > Hi Hans,
> > 
> > On Monday 20 December 2010 14:09:40 Hans Verkuil wrote:
> > > On Monday, December 20, 2010 13:48:51 Laurent Pinchart wrote:
> > > >
> > > > What if the application wants to change the resolution during capture ?
> > > > It will have to stop capture, call REQBUFS(0), change the format,
> > > > request buffers and restart capture. If filehandle ownership is dropped
> > > > after REQBUFS(0) that will open the door to a race condition.
> > > 
> > > That's why S_PRIORITY was invented.
> > 
> > Right, I should implement that. I think the documentation isn't clear though. 
> > What is the background priority for exactly ? 
> 
> http://linuxtv.org/downloads/v4l-dvb-apis/app-pri.html
> 
> "Another objective is to permit low priority applications working in
> background, which can be preempted by user controlled applications and
> automatically regain control of the device at a later time."
> 
> <contrived examples>
> A use case would be for a daemon that does background channel scanning
> or VBI data collection when the user isn't doing something else with the
> video device.
> 
> For a camera, maybe it would be an application that does device health
> monitoring, some sort of continuous calibration, or motion detection
> when the device isn't in use by the user for viewing.
> </contrived examples>
> 
> > And the "unset" priority ?
> 
> When checking for the maximum priority in use, it indicates that no
> nodes have any priorities. See v4l2_prio_max().  For a driver that
> supports priorities, it means no device nodes are opened, since any
> device node being open would get a priority of
> V4L2_PRIORITY_INTERACTIVE/_DEFAULT by default in v4l2_prio_open().
> 
> V4L2_PRIORITY_UNSET is also used to indicate to the v4l2_prio_*()
> functions that a struct v4l2_prio_state hasn't been initialized (i.e.
> has just been kzalloc()'ed).
> 
> > Are 
> > other applications allowed to change controls when an application has the 
> > record priority ?
> 
> According to the spec, no:
> 
> "V4L2_PRIORITY_RECORD 3 Highest priority. Only one file descriptor can
> have this priority, it blocks any other fd from changing device
> properties."
> 
> Once an automated, scheduled-recording process is recording, one really
> wouldn't want another process changing them.  I personally would not
> like unpredictable volume level variations on my TV show recordings.
> 
> 
> In cx18-controls.c one can find an implementation example for the old
> control framework:
> 
>         int cx18_s_ext_ctrls(struct file *file, void *fh, struct v4l2_ext_controls *c)
>         {       
>                 struct cx18_open_id *id = fh;   
>                 struct cx18 *cx = id->cx;       
>                 int ret;
>                 struct v4l2_control ctrl;       
>         
>                 ret = v4l2_prio_check(&cx->prio, id->prio);
>                 if (ret)
>                         return ret;
>     [...]
> 
> However, the new control framework in v4l2-ctrl.c seems to be missing
> any v4l2_prio_check() calls. The ioctl() handling in v4l2-ioctl.c
> doesn't have any either.

Oops, I'd forgotten about that. The priority support really should be
moved into the v4l2 core framework where it belongs. I'll see if I can spend
some time on that after Christmas.

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by Cisco

Re: [GIT PULL FOR 2.6.37] uvcvideo: BKL removal

[Mauro Carvalho Chehab]

Em 20-12-2010 10:28, Hans Verkuil escreveu:
> On Monday, December 20, 2010 13:10:32 Mauro Carvalho Chehab wrote:
>> Em 18-12-2010 08:45, Hans Verkuil escreveu:
>>> On Saturday, December 18, 2010 01:54:41 Laurent Pinchart wrote:
>>>> Hi Mauro,
>>>>
>>>> On Friday 17 December 2010 18:09:39 Mauro Carvalho Chehab wrote:
>>>>> Em 14-12-2010 08:55, Laurent Pinchart escreveu:
>>>>>> Hi Mauro,
>>>>>>
>>>>>> Please don't forget this pull request for 2.6.37.
>>>>>
>>>>> Pull request for upstream sent today.
>>>>
>>>> Thank you.
>>>>
>>>>> I didn't find any regressions at the BKL removal patches, but I noticed a
>>>>> few issues with qv4l2, not all related to uvcvideo. The remaining of this
>>>>> email is an attempt to document them for later fixes.
>>>>>
>>>>> They don't seem to be regressions caused by BKL removal, but the better
>>>>> would be to fix them later.
>>>>>
>>>>> - with uvcvideo and two video apps, if qv4l2 is started first, the second
>>>>> application doesn't start/capture. I suspect that REQBUFS (used by qv4l2
>>>>> to probe mmap/userptr capabilities) create some resource locking at
>>>>> uvcvideo. The proper way is to lock the resources only if the driver is
>>>>> streaming, as other drivers and videobuf do.
>>>>
>>>> I don't agree with that. The uvcvideo driver has one buffer queue per device, 
>>>> so if an application requests buffers on one file handle it will lock other 
>>>> applications out. If the driver didn't it would be subject to race conditions.
>>>
>>> I agree with Laurent. Once an application calls REQBUFS with non-zero count,
>>> then it should lock the resources needed for streaming. The reason behind that
>>> is that REQBUFS also locks the current selected format in place, since the
>>> format determines the amount of memory needed for the buffers.
>>
>> qv4l2 calls REQBUFS(1), then REQBUFS(0). Well, this is currently wrong, as most
>> drivers will only release buffers at VIDIOC_STREAMOFF.
> 
> qv4l2 first calls STREAMOFF, then REQBUFS(1), then REQBUFS(0). In the hope that one
> of these will actually free any buffers. It's random at the moment when drivers
> release buffers, one of the reasons for using vb2.

This won't free buffers. REQBUFS(0) is not honored. so, what you're doing is really
STREAMOFF/REQBUFS(1).

It would work fine if you change the order to:
REQBUFS(1)
REQBUFS(0)
STREAMOFF

>> Anyway, even replacing 
>> REQBUFS(0) with VIDIOC_STREAMOFF at qv4l2 won't help with uvcvideo. It seems that,
>> once buffers are requested at uvcvideo, they will release only at close().
>>
>> One consequence on the way uvcvideo is currently doing it is that, if you use qv4l2,
>> it is impossible to change the video size, as it returns -EBUSY, if you ask it to
>> select a different format (even without streaming):
>> 	$ ./qv4l2
>> 	Set Capture Format: Device or resource busy
>>
>>> The reason a lot of drivers don't do this is partially because for many TV
>>> capture drivers it is highly unlikely that the buffer size will change after
>>> calling REQBUFS (there are basically only two formats: 720x480 or 720x576 and
>>> users will normally never change between the two). However, this is much more
>>> likely to happen for webcams and embedded systems supporting HDTV.
>>
>> What applications do, when they need to change the formats, is to call REQBUFS again.
>>
>>> The other reason is probably because driver developers simple do not realize
>>> they need to lock the resources on REQBUFS. I'm sure many existing drivers will
>>> fail miserably if you change the format after calling REQBUFS (particularly
>>> with mmap streaming mode).
>>
>> I didn't make any test, but I don't think they'll fail (at least, on the drivers
>> that use videobuf), as streaming format will be stored at the videobuf handling 
>> (at buffer_prepare callback).
>>
>> So, if you change the format, the change will be applied only at the next call
>> to REQBUFS.
> 
> This behavior isn't according to the spec. G/S_FMT relate to the current format,
> not to some future format. Most non-videobuf drivers will not support this
> behavior.
> 
> It should be simple, really:
> 
> STREAMOFF
> REQBUFS(0)
> 
> That's all that should be needed to stop streaming and return all buffers to the
> app. This is what uvc should also support (and I actually thought it did).
> 
> Attempts to change formats while buffers have been requested should be blocked
> with EBUSY. It's all perfectly reasonable. Well, perhaps next year we might
> succeed in having all drivers behave consistently...

You didn't understand me: uvcvideo is returning -EBUSY to format changes with
buffers freed.

If I understood well, during probe time, qv4l2 calls REQBUFS(1)/REQBUFS(0) for both
userptr and mmap, in order to detect what the device supports. This takes some locking
schema inside uvcvideo, but, as uvcvideo is only releasing such lock at close(),
it is impossible to change the video format. I even added an extra STREAMOFF at qv4l2
code, but this didn't solve the issue.

Well, maybe better than explaining it, you should try to change the video image size
or format with qv4l2 and an uvc camera ;)

Cheers,
Mauro

Re: [GIT PULL FOR 2.6.37] uvcvideo: BKL removal

[Laurent Pinchart]

Hi Mauro,

On Tuesday 21 December 2010 12:10:18 Mauro Carvalho Chehab wrote:
>
> You didn't understand me: uvcvideo is returning -EBUSY to format changes
> with buffers freed.

As explained in my answer to Hans, that's on purpose.

The uvcvideo driver releases buffers when calling REQBUFS(0). However, the 
file handle is still marked as owning the device for streaming purpose, so 
other applications can't change the format or request buffers.

The reason for that is to avoid race conditions when an application wants to 
change the resolution during capture. As the application has to stop capture, 
call REQBUFS(0), change the format, request buffers  and restart capture, it 
prevents another application from racing it after REQBUFS(0).

As Hans correctly pointed out, this should be implemented using the priority 
ioctls. I will fix that.

-- 
Regards,

Laurent Pinchart

Re: [GIT PULL FOR 2.6.37] uvcvideo: BKL removal

[Laurent Pinchart]

On Thursday 23 December 2010 09:34:27 Laurent Pinchart wrote:
> Hi Mauro,
> 
> On Tuesday 21 December 2010 12:10:18 Mauro Carvalho Chehab wrote:
> > You didn't understand me: uvcvideo is returning -EBUSY to format changes
> > with buffers freed.
> 
> As explained in my answer to Hans, that's on purpose.
> 
> The uvcvideo driver releases buffers when calling REQBUFS(0). However, the
> file handle is still marked as owning the device for streaming purpose, so
> other applications can't change the format or request buffers.
> 
> The reason for that is to avoid race conditions when an application wants
> to change the resolution during capture. As the application has to stop
> capture, call REQBUFS(0), change the format, request buffers  and restart
> capture, it prevents another application from racing it after REQBUFS(0).

And it's actually not correct. I've just tested the latest uvcvideo version, 
and I can start capture in a second application after calling REQBUFS(0) in 
the first one.

> As Hans correctly pointed out, this should be implemented using the
> priority ioctls. I will fix that.

-- 
Regards,

Laurent Pinchart