On 09/01/14 17:36, Hans de Goede wrote:
> 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.

The code looks good and I've tested it with cx88 which worked (well, except
for the zillion other bugs in cx88, but that's another story). I do want to
do a few more tests on Friday, though. This time with a more stable driver.

I think we should consider adding this to the stable tree as well.

Regards,

        Hans

> 
> 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 <hdego...@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;
>>>
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to