Re: [PATCH] videobuf: Allow reqbufs(0) to free current buffers

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 09/01/2014 03:29 PM, Hans Verkuil wrote:
> Hi Hans,
> 
> At first glance this looks fine. But making changes in videobuf is always scary :-)
> so I hope Marek can look at this as well.
> 
> How well was this tested?

I ran some tests on bttv which all ran well.

Note that the code already allowed for going from say 4 buffers to 1,
and the old code path for reqbufs was already calling __videobuf_free()
before re-allocating the buffers again. So in essence this just changes
things to allow the 4 buffers to 1 case to also be 4 buffers to 0.

Regards,

Hans


> 
> I'll try do test this as well.
> 
> Regards,
> 
> 	Hans
> 
> On 08/31/2014 12:19 PM, Hans de Goede wrote:
>> All the infrastructure for this is already there, and despite our desires for
>> the old videobuf code to go away, it is currently still in use in 18 drivers.
>>
>> Allowing reqbufs(0) makes these drivers behave consistent with modern drivers,
>> making live easier for userspace, see e.g. :
>> https://bugzilla.gnome.org/show_bug.cgi?id=735660
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/media/v4l2-core/videobuf-core.c | 11 ++++++-----
>>  1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/videobuf-core.c b/drivers/media/v4l2-core/videobuf-core.c
>> index fb5ee5d..b91a266 100644
>> --- a/drivers/media/v4l2-core/videobuf-core.c
>> +++ b/drivers/media/v4l2-core/videobuf-core.c
>> @@ -441,11 +441,6 @@ int videobuf_reqbufs(struct videobuf_queue *q,
>>  	unsigned int size, count;
>>  	int retval;
>>  
>> -	if (req->count < 1) {
>> -		dprintk(1, "reqbufs: count invalid (%d)\n", req->count);
>> -		return -EINVAL;
>> -	}
>> -
>>  	if (req->memory != V4L2_MEMORY_MMAP     &&
>>  	    req->memory != V4L2_MEMORY_USERPTR  &&
>>  	    req->memory != V4L2_MEMORY_OVERLAY) {
>> @@ -471,6 +466,12 @@ int videobuf_reqbufs(struct videobuf_queue *q,
>>  		goto done;
>>  	}
>>  
>> +	if (req->count == 0) {
>> +		dprintk(1, "reqbufs: count invalid (%d)\n", req->count);
>> +		retval = __videobuf_free(q);
>> +		goto done;
>> +	}
>> +
>>  	count = req->count;
>>  	if (count > VIDEO_MAX_FRAME)
>>  		count = VIDEO_MAX_FRAME;
>>
>