From: Hans Verkuil <hverkuil@xs4all.nl>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: linux-media@vger.kernel.org, Hans Verkuil <hans.verkuil@cisco.com>
Subject: Re: [PATCH for v3.17 1/2] videobuf2-core.h: fix comment
Date: Mon, 04 Aug 2014 13:04:24 +0200 [thread overview]
Message-ID: <53DF68B8.2030308@xs4all.nl> (raw)
In-Reply-To: <1677917.ZvIcNnnNOx@avalon>
On 08/04/2014 12:40 PM, Laurent Pinchart wrote:
> Hi Hans,
>
> Thank you for the patch.
>
> On Monday 04 August 2014 12:27:11 Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> The comment for start_streaming that tells the developer with which vb2
>> state buffers should be returned to vb2 gave the wrong state. Very
>> confusing.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> I wonder whether we couldn't simplify drivers by moving this into vb2 though.
> A failed start_streaming requires drivers to dequeue all buffers internally,
> but the call to vb2_buffer_done() could be handled inside vb2. On the other
> hand it would make the vb2 warning go away, and drivers that fail to dequeue
> buffers internally would not be caught as easily, so I won't push for that
> change.
The driver owns the buffers at that point. So if I just dequeue them internally
then I have no idea what sort of driver-internal data structures I am corrupting.
Most drivers use a linked list of some sort, so that is typically the one that
gets messed up (and that actually happens). By far the best approach is to
require that drivers just hand over the buffers themselves, that way everything
happens in a controlled manner.
Regards,
Hans
>
>> ---
>> include/media/videobuf2-core.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
>> index fc910a6..80fa725 100644
>> --- a/include/media/videobuf2-core.h
>> +++ b/include/media/videobuf2-core.h
>> @@ -295,7 +295,7 @@ struct vb2_buffer {
>> * can return an error if hardware fails, in that case all
>> * buffers that have been already given by the @buf_queue
>> * callback are to be returned by the driver by calling
>> - * @vb2_buffer_done(VB2_BUF_STATE_DEQUEUED).
>> + * @vb2_buffer_done(VB2_BUF_STATE_QUEUED).
>> * If you need a minimum number of buffers before you can
>> * start streaming, then set @min_buffers_needed in the
>> * vb2_queue structure. If that is non-zero then
>
next prev parent reply other threads:[~2014-08-04 11:04 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-04 10:27 [PATCH for v3.17 0/2] vb2 fixes Hans Verkuil
2014-08-04 10:27 ` [PATCH for v3.17 1/2] videobuf2-core.h: fix comment Hans Verkuil
2014-08-04 10:40 ` Laurent Pinchart
2014-08-04 11:04 ` Hans Verkuil [this message]
2014-08-04 10:27 ` [PATCH for v3.17 2/2] vb2: fix vb2 state check when start_streaming fails Hans Verkuil
2014-08-04 10:44 ` Laurent Pinchart
2014-08-04 11:06 ` Hans Verkuil
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53DF68B8.2030308@xs4all.nl \
--to=hverkuil@xs4all.nl \
--cc=hans.verkuil@cisco.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).