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.verkuil@xxxxxxxxx>
>
> 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 majordomo@xxxxxxxxxxxxxxx
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
