Query: Reqbufs returning without calling queue_setup.

Query: Reqbufs returning without calling queue_setup.

[vkalia]

Hi

There is a check in __reqbufs in videobuf2-core.c in which if the count is
same then the function returns immediately. __reqbufs also calls
queue_setup callback into driver which updates the plane counts and sizes
of vb2 queue.  The count and size can be affected/changed by S_FMT and
G_FMT ioctl calls. This causes an issue with following call flow:

1. reqbufs.count = 8;
   ioctl(fd, VIDIOC_REQBUFS, &reqbufs);
2. ioctl(fd, VIDIOC_S_FMT, &format)  --> update format with differen
height/width etc which effect the sizeofimage
3. reqbufs.count = 8;
   ioctl(fd, VIDIOC_REQBUFS, &reqbufs); to update the vb2_queue num planes
and size of each plane. But this call never goes to the driver since
the count is same.

Shouldn't we query the driver for any change in plane count/size before
deciding to return from reqbufs? Following is the code. This is just a
temporary patch to point the issue.

diff --git a/drivers/media/v4l2-core/videobuf2-core.c
b/drivers/media/v4l2-core/videobuf2-core.c
index e42eb0d..57e18c2 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -590,13 +590,6 @@ static int __reqbufs(struct vb2_queue *q, struct
v4l2_requestbuffers *req)
                return -EBUSY;
        }

-       /*
-        * 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

Thanks
Vinay

Re: Query: Reqbufs returning without calling queue_setup.

[Hans Verkuil]

On 03/13/2014 09:13 PM, vkalia@codeaurora.org wrote:
> Hi
> 
> There is a check in __reqbufs in videobuf2-core.c in which if the count is
> same then the function returns immediately. __reqbufs also calls
> queue_setup callback into driver which updates the plane counts and sizes
> of vb2 queue.  The count and size can be affected/changed by S_FMT and
> G_FMT ioctl calls. This causes an issue with following call flow:
> 
> 1. reqbufs.count = 8;
>    ioctl(fd, VIDIOC_REQBUFS, &reqbufs);
> 2. ioctl(fd, VIDIOC_S_FMT, &format)  --> update format with differen
> height/width etc which effect the sizeofimage
> 3. reqbufs.count = 8;
>    ioctl(fd, VIDIOC_REQBUFS, &reqbufs); to update the vb2_queue num planes
> and size of each plane. But this call never goes to the driver since
> the count is same.
> 
> Shouldn't we query the driver for any change in plane count/size before
> deciding to return from reqbufs? Following is the code. This is just a
> temporary patch to point the issue.
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> b/drivers/media/v4l2-core/videobuf2-core.c
> index e42eb0d..57e18c2 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -590,13 +590,6 @@ static int __reqbufs(struct vb2_queue *q, struct
> v4l2_requestbuffers *req)
>                 return -EBUSY;
>         }
> 
> -       /*
> -        * 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;
> -

What strange kernel version are you using? This obviously wrong code was part of
the first videobuf2 appearance in kernel 2.6.39 and was fixed in 3.0. Furthermore,
in 2.6.39 videobuf2-core.c was in the video directory, not v4l2-core, and the
__reqbufs function didn't exist yet.

It seems someone made a Frankenstein version of videobuf2-core.c for whatever
kernel you are using. A Qualcomm kernel perhaps?

Regards,

	Hans

>         if (req->count == 0 || q->num_buffers != 0 || q->memory !=
> req->memory) {
>                 /*
>                  * We already have buffers allocated, so first check if they
> 
> Thanks
> Vinay
> 
> --
> 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
>