Hi Hans,
On Sunday 21 September 2014 11:00:02 Hans Verkuil wrote:
> On 09/20/2014 09:26 PM, Laurent Pinchart wrote:
> > On Saturday 20 September 2014 21:16:35 Hans Verkuil wrote:
> >> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> >>
> >> The recent conversion of saa7134 to vb2 unconvered a poll() bug that
> >> broke the teletext applications alevt and mtt. These applications
> >> expect that calling poll() without having called VIDIOC_STREAMON will
> >> cause poll() to return POLLERR. That did not happen in vb2.
> >>
> >> This patch fixes that behavior. It also fixes what should happen when
> >> poll() is called when STREAMON is called but no buffers have been
> >> queued. In that case poll() will also return POLLERR, but only for
> >> capture queues since output queues will always return POLLOUT
> >> anyway in that situation.
> >>
> >> This brings the vb2 behavior in line with the old videobuf behavior.
> >>
> >> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> >> ---
> >>
> >> drivers/media/v4l2-core/videobuf2-core.c | 17 ++++++++++++++---
> >> include/media/videobuf2-core.h | 4 ++++
> >> 2 files changed, 18 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> >> b/drivers/media/v4l2-core/videobuf2-core.c index 7e6aff6..a0aa694 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->waiting_for_buffers = !V4L2_TYPE_IS_OUTPUT(q->type);
> >> return 0;
> >> }
> >>
> >> @@ -1024,6 +1025,7 @@ static int __create_bufs(struct vb2_queue *q,
> >> struct
> >> v4l2_create_buffers *create memset(q->plane_sizes, 0,
> >> sizeof(q->plane_sizes));
> >> memset(q->alloc_ctx, 0, sizeof(q->alloc_ctx));
> >> q->memory = create->memory;
> >> + q->waiting_for_buffers = !V4L2_TYPE_IS_OUTPUT(q->type);
> >
> > Wouldn't it be easier to set the flag when creating the queue and in
> > vb2_internal_streamoff() instead of in __create_bufs and __reqbufs ? I'll
> > let you decide.
>
> Sorry, I don't follow. 'When creating the queue'? __create_bufs and
> __reqbufs are the functions that create the queue (i.e. allocate the
> buffers).
I meant vb2_queue_init.
> >> }
> >>
> >> num_buffers = min(create->count, VIDEO_MAX_FRAME - q->num_buffers);
> >>
> >> @@ -1801,6 +1803,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->waiting_for_buffers = false;
> >>
> >> vb->state = VB2_BUF_STATE_QUEUED;
> >> if (V4L2_TYPE_IS_OUTPUT(q->type)) {
> >>
> >> /*
> >>
> >> @@ -2261,6 +2264,7 @@ static int vb2_internal_streamoff(struct vb2_queue
> >> *q, enum v4l2_buf_type type) * their normal dequeued state.
> >>
> >> */
> >>
> >> __vb2_queue_cancel(q);
> >>
> >> + q->waiting_for_buffers = !V4L2_TYPE_IS_OUTPUT(q->type);
> >>
> >> dprintk(3, "successful\n");
> >> return 0;
> >>
> >> @@ -2583,10 +2587,17 @@ 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 waiting_for_buffers to false.
> >> + */
> >> + if (q->waiting_for_buffers)
> >>
> >> return res | POLLERR;
> >>
> >> /*
> >>
> >> diff --git a/include/media/videobuf2-core.h
> >> b/include/media/videobuf2-core.h index 5a10d8d..84f790c 100644
> >> --- a/include/media/videobuf2-core.h
> >> +++ b/include/media/videobuf2-core.h
> >> @@ -381,6 +381,9 @@ struct v4l2_fh;
> >> * @start_streaming_called: start_streaming() was called successfully
> >> and we
> >> * started streaming.
> >> * @error: a fatal error occurred on the queue
> >> + * @waiting_for_buffers: used in poll() to check if vb2 is still waiting
> >> for
> >> + * buffers. Only set for capture queues if qbuf has not yet been
> >> + * called since poll() needs to return POLLERR in that situation.
> >> * @fileio: file io emulator internal data, used only if emulator is
> >> active
> >> * @threadio: thread io internal data, used only if thread is active
> >> */
> >> @@ -419,6 +422,7 @@ struct vb2_queue {
> >> unsigned int streaming:1;
> >> unsigned int start_streaming_called:1;
> >> unsigned int error:1;
> >> + unsigned int waiting_for_buffers:1;
> >>
> >> struct vb2_fileio_data *fileio;
> >> struct vb2_threadio_data *threadio;
--
Regards,
Laurent Pinchart
--
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
