Le mardi 03 juin 2014 à 08:38 +0200, Hans Verkuil a écrit : > Hi Nicholas, > > On 06/02/2014 09:47 PM, Nicolas Dufresne wrote: > > Hi everyone, > > > > Recently in GStreamer we notice that we where not handling the POLLERR > > flag at all. Though we found that what the code do, and what the doc > > says is slightly ambiguous. > > > > "When the application did not call VIDIOC_QBUF or > > VIDIOC_STREAMON yet the poll() function succeeds, but sets the > > POLLERR flag in the revents field." > > > > In our case, we first seen this error with a capture device. How things > > worked is that we first en-queue all allocated buffers. Our > > interpretation was that this would avoid not calling "VIDIOC_QBUF [...] > > yet", and only then we would call VIDIOC_STREAMON. This way, in our > > interpretation we would never get that error. > > > > Though, this is not what the code does. Looking at videobuf2, if simply > > return this error when the queue is empty. > > > > /* > > * There is nothing to wait for if no buffers have already been queued. > > */ > > if (list_empty(&q->queued_list)) > > return res | POLLERR; > > > > So basically, we endup in this situation where as soon as all existing > > buffers has been dequeued, we can't rely on the driver to wait for a > > buffer to be queued and then filled again. This basically forces us into > > adding a new user-space mechanism, to wait for buffer to come back. We > > are wandering if this is a bug. If not, maybe it would be nice to > > improve the documentation. > > Just for my understanding: I assume that gstreamer polls in one process or > thread and does the queuing/dequeuing in a different process/thread, is that > correct? Yes, in this particular case (polling with queues/thread downstream), the streaming thread do the polling, and then push the buffers. The buffer reach a queue element, which will queued and return. Polling restart at this point. The queue will later pass it downstream from the next streaming thread, and eventually the buffer will be released. For capture device, QBUF will be called upon release. It is assumed that this call to QBUF should take a finite amount of time. Though, libv4l2 makes this assumption wrong by inter-locking DQBUF and QBUF, clearly a bug, but not strictly related to this thread. Also, as we try and avoid blocking in the DQBUF ioctl, it should not be a problem for us. > > If it was all in one process, then it would make no sense polling for a > buffer to become available if you never queued one. Not exactly true, the driver may take some time before the buffer we have queued back is filled and available again. The poll/select FD set also have a control pipe, so we can stop the process at any moment. Not polling would mean blocking on an ioctl() which cannot be canceled. But, without downstream queues (thread), the size of the queue will be negotiated so that the buffer will be released before we go back polling. The queue will never be empty in this case. > > That is probably the reasoning behind what poll does today. That said, I've > always thought it was wrong and that it should be replaced by something like: > > if (!vb2_is_streaming(q)) > return res | POLLERR; > > I.e.: only return an error if we're not streaming. I think it would be easier for user space and closer to what the doc says. Though, it's not just about changing that check, there is some more work involved from what I've seen. cheers, Nicolas
Attachment:
signature.asc
Description: This is a digitally signed message part
