vb2: Update buffer state flags after __vb2_dqbuf

Previously we where updating the buffer state using __fill_v4l2_buffer
before the state transition was completed through __vb2_dqbuf. This
would cause the V4L2_BUF_FLAG_DONE to be set, which would mean it still
queued. The spec says the dqbuf should clean the DONE flag, right not it
alway set it.

Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 drivers/media/v4l2-core/videobuf2-core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
  

Hi Nicolas,

On Wednesday 16 April 2014 15:34:06 Nicolas Dufresne wrote:
> Previously we where updating the buffer state using __fill_v4l2_buffer
> before the state transition was completed through __vb2_dqbuf. This
> would cause the V4L2_BUF_FLAG_DONE to be set, which would mean it still
> queued. The spec says the dqbuf should clean the DONE flag, right not it
> alway set it.
> 
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

This looks fine to me, thanks for catching and fixing the problem.

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/v4l2-core/videobuf2-core.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> b/drivers/media/v4l2-core/videobuf2-core.c index f9059bb..ac5026a 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -1943,14 +1943,15 @@ static int vb2_internal_dqbuf(struct vb2_queue *q,
> struct v4l2_buffer *b, bool n
> 
>  	call_vb_qop(vb, buf_finish, vb);
> 
> -	/* Fill buffer information for the userspace */
> -	__fill_v4l2_buffer(vb, b);
>  	/* Remove from videobuf queue */
>  	list_del(&vb->queued_entry);
>  	q->queued_count--;
>  	/* go back to dequeued state */
>  	__vb2_dqbuf(vb);
> 
> +	/* Fill buffer information for the userspace */
> +	__fill_v4l2_buffer(vb, b);
> +
>  	dprintk(1, "dqbuf of buffer %d, with state %d\n",
>  			vb->v4l2_buf.index, vb->state);
  

On 04/16/2014 09:34 PM, Nicolas Dufresne wrote:
> 
> Previously we where updating the buffer state using __fill_v4l2_buffer
> before the state transition was completed through __vb2_dqbuf. This
> would cause the V4L2_BUF_FLAG_DONE to be set, which would mean it still
> queued. The spec says the dqbuf should clean the DONE flag, right not it
> alway set it.
> 
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

Nack: this will break the V4L2_BUF_FLAG_ERROR support.

I would recommend just clearing the DONE flag explicitly.

Looking at the code I believe the flags are also set incorrectly
after QBUF (and PREPARE_BUF might have problems as well).

This needs some careful checking and I need to add v4l2-compliance
checks for this.

I don't really have time to look into this at the moment, though.

Regards,

	Hans

> ---
>  drivers/media/v4l2-core/videobuf2-core.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index f9059bb..ac5026a 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -1943,14 +1943,15 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
>  
>  	call_vb_qop(vb, buf_finish, vb);
>  
> -	/* Fill buffer information for the userspace */
> -	__fill_v4l2_buffer(vb, b);
>  	/* Remove from videobuf queue */
>  	list_del(&vb->queued_entry);
>  	q->queued_count--;
>  	/* go back to dequeued state */
>  	__vb2_dqbuf(vb);
>  
> +	/* Fill buffer information for the userspace */
> +	__fill_v4l2_buffer(vb, b);
> +
>  	dprintk(1, "dqbuf of buffer %d, with state %d\n",
>  			vb->v4l2_buf.index, vb->state);
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html