On 09/19/2014 06:15 PM, Hans Verkuil wrote: > After a long discussion on irc the decision was taken that poll() should: > > - return POLLERR when not streaming or when q->error is set > - return POLLERR when streaming from a capture queue, but no buffers have been > queued yet, and it is not part of an M2M device. > > The first rule is logical, the second less so. It emulates vb1 behavior that > some > applications might rely on. It is behavior that we don't want for output > devices > or M2M devices because calling STREAMON without QBUF makes a lot of sense for > output devices, and for M2M devices I want to avoid causing a regression by > potentially changing the behavior of M2M capture queues. We don't have legacy > apps > to support there, so let's make sure that those queue types remain unchanged. > > I do that by setting needs_buffers to false in v4l2_m2m_streamon. All M2M > drivers > use that function with the exception of s5p-mfc, but there STREAMON will > return > an error if not enough buffers are queued so it's not able to do STREAMON > without > a QBUF anyway. > > There will be a second version, since I need to update some comments in the > header > and adjust the spec, but I would like to get code reviews as soon as possible. > > Just explaining that second rule makes my head hurt, which is usually a bad > sign. > > Hans > > Signed-off-by: Hans Verkuil <hans.verk...@cisco.com> > > diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c > b/drivers/media/v4l2-core/v4l2-mem2mem.c > index 80c588f..c8d2b5b 100644 > --- a/drivers/media/v4l2-core/v4l2-mem2mem.c > +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c > @@ -463,6 +463,7 @@ int v4l2_m2m_streamon(struct file *file, struct > v4l2_m2m_ctx *m2m_ctx, > int ret; > > vq = v4l2_m2m_get_vq(m2m_ctx, type); > + vq->needs_buffers = false;
I'm going to drop this. Up to 3.16 all vb2 drivers would return POLLERR if no buffers were queued, so it was standard behavior. That simplifies the patch a bit. Hans > ret = vb2_streamon(vq, type); > if (!ret) > v4l2_m2m_try_schedule(m2m_ctx); > diff --git a/drivers/media/v4l2-core/videobuf2-core.c > b/drivers/media/v4l2-core/videobuf2-core.c > index 7e6aff6..efbf1ce 100644 > --- a/drivers/media/v4l2-core/videobuf2-core.c > +++ b/drivers/media/v4l2-core/videobuf2-core.c > @@ -977,6 +977,7 @@ static int __reqbufs(struct vb2_queue *q, struct > v4l2_requestbuffers *req) > * to the userspace. > */ > req->count = allocated_buffers; > + q->needs_buffers = !V4L2_TYPE_IS_OUTPUT(q->type); > > return 0; > } > @@ -1801,6 +1802,7 @@ static int vb2_internal_qbuf(struct vb2_queue *q, > struct v4l2_buffer *b) > */ > list_add_tail(&vb->queued_entry, &q->queued_list); > q->queued_count++; > + q->needs_buffers = false; > vb->state = VB2_BUF_STATE_QUEUED; > if (V4L2_TYPE_IS_OUTPUT(q->type)) { > /* > @@ -2583,10 +2585,18 @@ unsigned int vb2_poll(struct vb2_queue *q, struct > file *file, poll_table *wait) > } > > /* > - * There is nothing to wait for if no buffer has been queued and the > - * queue isn't streaming, or if the error flag is set. > + * There is nothing to wait for if the queue isn't streaming, or if the > + * error flag is set. > */ > - if ((list_empty(&q->queued_list) && !vb2_is_streaming(q)) || q->error) > + if (!vb2_is_streaming(q) || q->error) > + return res | POLLERR; > + /* > + * For compatibility with vb1: if QBUF hasn't been called yet, then > + * return POLLERR as well. This only affects capture queues, output > + * queues will always initialize needs_buffers to false. M2M devices > + * also set needs_buffers to false in v4l2_m2m_streamon(). > + */ > + if (q->needs_buffers) > return res | POLLERR; > > /* > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h > index 5a10d8d..1c218b1 100644 > --- a/include/media/videobuf2-core.h > +++ b/include/media/videobuf2-core.h > @@ -419,6 +419,7 @@ struct vb2_queue { > unsigned int streaming:1; > unsigned int start_streaming_called:1; > unsigned int error:1; > + unsigned int needs_buffers:1; > > struct vb2_fileio_data *fileio; > struct vb2_threadio_data *threadio; > -- > 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 > -- 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