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;
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
