vb2 queue setup skipped when playing bigger video

vb2 queue setup skipped when playing bigger video

[Uwe Kleine-König]

Hello,

I'm writing a driver for a video overlay device and have the problem
that if I first playback a video with a resolution of say 320x240 and
then another one with 640x400 the buffers allocated for the first
playback are too small, but my .queue_setup callback isn't called.

I think the culprit is the following line in vb2_reqbufs()
(drivers/media/video/videobuf2-core.c):

	/*
	 * If the same number of buffers and memory access method is requested
	 * then return immediately.
	 */
	if (q->memory == req->memory && req->count == q->num_buffers)
		return 0;

which exits vb2_reqbufs before

	ret = call_qop(q, queue_setup, q, &num_buffers, &num_planes,
		       plane_sizes, q->alloc_ctx);

Reading the vb1 code, this shortcut isn't implemented there.

As I'm quite new to all that v4l2 stuff, I'm not sure what to do.
Just removing the return 0 (i.e. reverting 31901a07) seems to do the
right thing for me.

Thoughts?

Thanks and best regards,
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Corrected address for Pawel [Was: vb2 queue setup skipped when playing bigger video]

[Uwe Kleine-König]

The address of Pawel at Samsung doesn't seem to work anymore. If you
want to reach him with your reply take care to fix his address to the
one used in this mail (which is the most recent in the kernel's history
and hopefully works).

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

RE: vb2 queue setup skipped when playing bigger video

[Marek Szyprowski]

Hello,

On Wednesday, June 08, 2011 10:54 AM Uwe Kleine-König wrote:

> I'm writing a driver for a video overlay device and have the problem
> that if I first playback a video with a resolution of say 320x240 and
> then another one with 640x400 the buffers allocated for the first
> playback are too small, but my .queue_setup callback isn't called.
> 
> I think the culprit is the following line in vb2_reqbufs()
> (drivers/media/video/videobuf2-core.c):
> 
> 	/*
> 	 * If the same number of buffers and memory access method is
> requested
> 	 * then return immediately.
> 	 */
> 	if (q->memory == req->memory && req->count == q->num_buffers)
> 		return 0;
> 
> which exits vb2_reqbufs before
> 
> 	ret = call_qop(q, queue_setup, q, &num_buffers, &num_planes,
> 		       plane_sizes, q->alloc_ctx);
> 
> Reading the vb1 code, this shortcut isn't implemented there.
> 
> As I'm quite new to all that v4l2 stuff, I'm not sure what to do.
> Just removing the return 0 (i.e. reverting 31901a07) seems to do the
> right thing for me.
> 
> Thoughts?

You are definitely right. I've missed the case that the format might
have changed in-between the reqbufs() calls. My commit 31901a07 is
completely wrong indeed, I've simplified it too much. I will send
revert soon.

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center

[PATCH] Don't shortcut vb2_reqbufs in case the format changed

[Uwe Kleine-König]

Just checking for number of buffers and memory access method isn't
enough because the format might have changed since the buffers were
allocated and the new format might need bigger ones.

This reverts commit 31901a078af29c33c736dcbf815656920e904632.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

as I need the patch now anyhow, here it is for you, too.

Best regards
Uwe

 drivers/media/video/videobuf2-core.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
index 6ba1461..6489aa2 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
@@ -492,13 +492,6 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 		return -EINVAL;
 	}
 
-	/*
-	 * If the same number of buffers and memory access method is requested
-	 * then return immediately.
-	 */
-	if (q->memory == req->memory && req->count == q->num_buffers)
-		return 0;
-
 	if (req->count == 0 || q->num_buffers != 0 || q->memory != req->memory) {
 		/*
 		 * We already have buffers allocated, so first check if they
-- 
1.7.5.3

[PATCH] Don't shortcut vb2_reqbufs in case the format changed

[Uwe Kleine-König]

Just checking for number of buffers and memory access method isn't
enough because the format might have changed since the buffers were
allocated and the new format might need bigger ones.

This reverts commit 31901a078af29c33c736dcbf815656920e904632.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

I resend, because the original post didn't made it into patchwork. (The
web interface showed a database error around that time.)
And please note that my name is still wrong in patchwork.

Best regards
Uwe

 drivers/media/video/videobuf2-core.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
index 6ba1461..6489aa2 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
@@ -492,13 +492,6 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 		return -EINVAL;
 	}
 
-	/*
-	 * If the same number of buffers and memory access method is requested
-	 * then return immediately.
-	 */
-	if (q->memory == req->memory && req->count == q->num_buffers)
-		return 0;
-
 	if (req->count == 0 || q->num_buffers != 0 || q->memory != req->memory) {
 		/*
 		 * We already have buffers allocated, so first check if they
-- 
1.7.5.3