Re: [PATCHv2 1/3] vb2: fix VBI/poll regression

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Hans,

On Sunday 21 September 2014 11:34:26 Hans Verkuil wrote:
> On 09/21/2014 11:30 AM, Laurent Pinchart wrote:
> > 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@cisco.com>
> >>>> 
> >>>> 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@cisco.com>
> >>>> ---
> >>>> 
> >>>>  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.
> 
> That's not an option as it is called only once at probe time.

I know :-)

> And this flag needs to be set every time you setup the queue for streaming,
> when you stop streaming and when you queue a buffer.

I was thinking it would be enough to set the flag when stopping the stream, as 
it would then be set for the next REQBUFS or CREATE_BUFS operation, but that 
would break if an application calls QBUF - close() - open() - ...

Let's proceed with your patch.

-- 
Regards,

Laurent Pinchart