On Tue 29 May 2012 14:05:08 Ezequiel Garcia wrote: > On Mon, May 28, 2012 at 7:22 AM, Hans Verkuil <hverkuil@xxxxxxxxx> wrote: > > > > In practice it seems that the easiest approach is not to clean up anything in the > > disconnect, just take the lock, do the bare minimum necessary for the disconnect, > > unregister the video nodes, unlock and end with v4l2_device_put(v4l2_dev). > > > > It's a suggestion only, but experience has shown that it works well. And as I said, > > when you get multiple device nodes, then this is the only workable approach. > > I'm convinced: it's both cleaner and more logical to use > v4l2_release instead of video_device release to the final cleanup. > > > > > OK, the general rule is as follows (many drivers do not follow this correctly, BTW, > > but this is what should happen): > > > > - the filehandle that calls REQBUFS owns the buffers and is the only one that can > > start/stop streaming and queue/dequeue buffers. > > and read, poll, etc right? Read yes, but anyone can poll. > > > This is until REQBUFS with count == 0 > > is called, or until the filehandle is closed. > > Okey. But currently videobuf2 doesn't notify the driver > when reqbufs with zero count has been called. > > So, I have to "assume" it (aka trouble ahead) or "capture" the zero > count case before/after calling vb2_reqbufs (aka ugly). You just check the count after calling vb2_reqbufs. Nothing ugly about it. > I humbly think that, if we wan't to enforce this behavior > (as part of v4l2 driver semantics) > then we should have videobuf2 tell the driver when reqbufs has been > called with zero count. > > You can take a look at pwc which only drops owner on filehandle close, That's a bug. REQBUFS(0) must drop the owner: you've just released all resources related to streaming, so there is no reason to prevent others from using them. But I suspect 90% of all drivers do this wrong. Not to mention that the old videobuf doesn't handle a count of 0 at all, which is a complete violation of the spec. So an application never knows whether a count of 0 will work. Lovely... The qv4l2 test tool has some really ugly workaround for this :-) At some point I'm going to add a test for this to v4l2-compliance... > or uvc which captures this from vb2_reqbufs. > > After looking at uvc, now I wonder is it really ugly? or perhaps > it's just ok. It would be nice if this was somehow integrated into videobuf2, but not everyone liked the idea of vb2 using filehandle information from what I remember. But I didn't push for it very hard. > > > > v4l2_device is a top-level struct, video_device represents a single device node. > > For cleanup purposes there isn't much difference between the two if you have > > only one device node. When you have more, then those differences are much more > > important. > > Yes, it's cleaner now. > > Thanks! > Ezequiel. > Your welcome! Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
