* Hans Verkuil (hverkuil-cisco@xxxxxxxxx) wrote:
> The vb2 read support requests 1 buffer, leaving it to the driver
> to increase this number to something that works.
>
> Unfortunately, drivers do not deal with this reliably, and in fact
> this caused problems for the bttv driver and reading from /dev/vbiX,
> causing every other VBI frame to be all 0.
>
> Instead, request as the number of buffers whatever is the maximum of
> 2 and q->min_buffers_needed+1.
>
> In order to start streaming you need at least q->min_buffers_needed
> queued buffers, so add 1 buffer for processing. And if that field
> is 0, then choose 2 (again, one buffer is being filled while the
> other one is being processed).
>
> This certainly makes more sense than requesting just 1 buffer, and
> the VBI bttv support is now working again.
>
> It turns out that the old videobuf1 behavior of bttv was to allocate
> 8 (video) and 4 (vbi) buffers when used with read(). After the vb2
> conversion that changed to 2 for both. With this patch it is 3, which
> is really all you need.
>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
> Fixes: b7ec3212a73a ("media: bttv: convert to vb2")
This looks like it's working nicely; I've tested it with both
Alistair's test stream and a real signal, and I'm getting
a consistent 25fps out of the VBI with or without xawtv
grabbing, and the test stream looks good to me.
So,
Tested-by: Dr. David Alan Gilbert <dave@xxxxxxxxxxx>
Thanks for fixing this!
Dave
> ---
> drivers/media/common/videobuf2/videobuf2-core.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> index 8c1df829745b..40d89f29fa33 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -2735,9 +2735,14 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read)
> return -EBUSY;
>
> /*
> - * Start with count 1, driver can increase it in queue_setup()
> + * Start with q->min_buffers_needed + 1, driver can increase it in
> + * queue_setup()
> + *
> + * 'min_buffers_needed' buffers need to be queued up before you
> + * can start streaming, plus 1 for userspace (or in this case,
> + * kernelspace) processing.
> */
> - count = 1;
> + count = max(2, q->min_buffers_needed + 1);
>
> dprintk(q, 3, "setting up file io: mode %s, count %d, read_once %d, write_immediately %d\n",
> (read) ? "read" : "write", count, q->fileio_read_once,
> --
> 2.42.0
>
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
