From: Hans Verkuil <hverkuil@xs4all.nl>
To: Nicolas Dufresne <nicolas.dufresne@collabora.com>,
linux-media@vger.kernel.org
Subject: Re: Poll and empty queues
Date: Tue, 03 Jun 2014 08:38:57 +0200 [thread overview]
Message-ID: <538D6D81.3000001@xs4all.nl> (raw)
In-Reply-To: <1401738463.2304.15.camel@nicolas-tpx230>
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?
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.
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.
Regards,
Hans
next prev parent reply other threads:[~2014-06-03 6:39 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-02 19:47 Poll and empty queues Nicolas Dufresne
2014-06-03 6:38 ` Hans Verkuil [this message]
2014-06-03 14:37 ` Nicolas Dufresne
2014-06-03 16:11 ` Laurent Pinchart
2014-06-03 17:39 ` Nicolas Dufresne
2014-06-03 23:46 ` Laurent Pinchart
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=538D6D81.3000001@xs4all.nl \
--to=hverkuil@xs4all.nl \
--cc=linux-media@vger.kernel.org \
--cc=nicolas.dufresne@collabora.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox