Hi Pavel,
On Thursday 22 April 2010 11:35:35 Pawel Osciak wrote:
> >Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> >On Wednesday 21 April 2010 13:39:44 Pawel Osciak wrote:
> >> @@ -679,23 +682,20 @@ int videobuf_dqbuf(struct videobuf_queue *q,
> >>
> >> switch (buf->state) {
> >>
> >> case VIDEOBUF_ERROR:
> >> dprintk(1, "dqbuf: state is error\n");
> >>
> >> - retval = -EIO;
> >> - CALL(q, sync, q, buf);
> >> - buf->state = VIDEOBUF_IDLE;
> >>
> >> break;
> >>
> >> case VIDEOBUF_DONE:
> >> dprintk(1, "dqbuf: state is done\n");
> >>
> >> - CALL(q, sync, q, buf);
> >> - buf->state = VIDEOBUF_IDLE;
> >>
> >> break;
> >>
> >> default:
> >> dprintk(1, "dqbuf: state invalid\n");
> >> retval = -EINVAL;
> >> goto done;
> >>
> >> }
> >>
> >> - list_del(&buf->stream);
> >> - memset(b, 0, sizeof(*b));
> >> + CALL(q, sync, q, buf);
> >>
> >> videobuf_status(q, b, buf, q->type);
> >>
> >> + list_del(&buf->stream);
> >> + buf->state = VIDEOBUF_IDLE;
> >> + b->flags &= ~V4L2_BUF_FLAG_DONE;
> >
> >We do you clear the done flag here ?
>
> The DONE flag is supposed to be cleared when dequeuing, but should
> be set when querying:
>
> "When this flag is set, the buffer is currently on the outgoing queue,
> ready to be dequeued from the driver. Drivers set or clear this flag
> when the VIDIOC_QUERYBUF ioctl is called. After calling the VIDIOC_QBUF
> or VIDIOC_DQBUF it is always cleared."
>
> videobuf_status() is used for both QUERYBUF and DQBUF and making both
> work properly is not very straightforward without losing
> VIDEOBUF_DONE/VIDEOBUF_ERROR distinction (it becomes more clear when you
> analyze both cases).
>
> My previous patch was doing it the other way around, but Hans' version
> seemed shorter and cleaner.
Thanks for pointing this out.
I have no more objection then,
Acked-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
--
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
