Increase VIDIOC_REQBUFS count from 2 to 4

Increase VIDIOC_REQBUFS count from 2 to 4

The original config of 2 buffers is too few. The kernel may be throttled with no
buffer to queue when Chrome doesn't return the buffer in time. Reserve two more
buffers gives Chrome extra 60ms to process incoming frame on a 30fps camera.

BUG=chrome-os-partner:32570
TEST=Check 'uvcvideo: Dropping payload' messages in kernel logs

Committed: https://crrev.com/133987bb651125718ea249ddb3215402e1aaa9e1
Cr-Commit-Position: refs/heads/master@{#321333}

[Justin Chuang, 2015-03-18 07:38:38]

jchuang@chromium.org changed reviewers:
+ posciak@chromium.org

[Justin Chuang, 2015-03-18 07:46:49]

On 2015/03/18 07:38:38, Justin Chuang wrote:
> mailto:jchuang@chromium.org changed reviewers:
> + mailto:posciak@chromium.org

Pawel, this number is a bit random. On Nyan_big, need to increase to 4 to avoid
the 'uvcvideo: Dropping payload (out of sync)' warnings. 

But it really depends on the performance of different device.  I don't know
which number is ideal.

Agree that at least the current number (2 buffers) is too few, better to
increase than keeping old value.

[Pawel Osciak, 2015-03-18 11:17:02]

https://codereview.chromium.org/1008343006/diff/1/media/video/capture/linux/v...
File media/video/capture/linux/video_capture_device_linux.cc (right):

https://codereview.chromium.org/1008343006/diff/1/media/video/capture/linux/v...
media/video/capture/linux/video_capture_device_linux.cc:32: const uint32
kMaxVideoBuffers = 4;
s/kMaxVideoBuffers/kNumVideoBuffers/

https://codereview.chromium.org/1008343006/diff/1/media/video/capture/linux/v...
media/video/capture/linux/video_capture_device_linux.cc:472: r_buffer.count =
std::min(r_buffer.count, kMaxVideoBuffers);
This isn't right. The device is free to allocate more and we should use more if
possible. I don't see a good reason not to do that and this line was added very
recently. I think we should remove this.

[Justin Chuang, 2015-03-18 11:18:52]

https://codereview.chromium.org/1008343006/diff/1/media/video/capture/linux/v...
File media/video/capture/linux/video_capture_device_linux.cc (right):

https://codereview.chromium.org/1008343006/diff/1/media/video/capture/linux/v...
media/video/capture/linux/video_capture_device_linux.cc:472: r_buffer.count =
std::min(r_buffer.count, kMaxVideoBuffers);
On 2015/03/18 11:17:02, Pawel Osciak wrote:
> This isn't right. The device is free to allocate more and we should use more
if
> possible. I don't see a good reason not to do that and this line was added
very
> recently. I think we should remove this.

Would you like to do it in this CL or refactor in another one?
I'm okay for both case.

[Pawel Osciak, 2015-03-18 11:24:21]

posciak@chromium.org changed reviewers:
+ mcasas@chromium.org

[Pawel Osciak, 2015-03-18 11:24:34]

On 2015/03/18 11:18:52, Justin Chuang wrote:
>
https://codereview.chromium.org/1008343006/diff/1/media/video/capture/linux/v...
> File media/video/capture/linux/video_capture_device_linux.cc (right):
> 
>
https://codereview.chromium.org/1008343006/diff/1/media/video/capture/linux/v...
> media/video/capture/linux/video_capture_device_linux.cc:472: r_buffer.count =
> std::min(r_buffer.count, kMaxVideoBuffers);
> On 2015/03/18 11:17:02, Pawel Osciak wrote:
> > This isn't right. The device is free to allocate more and we should use more
> if
> > possible. I don't see a good reason not to do that and this line was added
> very
> > recently. I think we should remove this.
> 
> Would you like to do it in this CL or refactor in another one?
> I'm okay for both case.

I think we should remove it now. +mcasas FYI.

[Justin Chuang, 2015-03-18 16:05:51]

Patchset #3 (id:40001) has been deleted

[Justin Chuang, 2015-03-18 16:13:20]

On 2015/03/18 11:24:34, Pawel Osciak wrote:
> 
> I think we should remove it now. +mcasas FYI.

Done. 

In K3.14 tree, VIDIOC_REQBUFS in UVC is directly handled by vb2_reqbufs. 
UVC doesn't set vb2_queue.min_buffers_needed, so the number of returned buffer
will be the same as the requested if the 1 <= count <= 32.

[Pawel Osciak, 2015-03-19 07:12:15]

On 2015/03/18 16:13:20, Justin Chuang wrote:
> On 2015/03/18 11:24:34, Pawel Osciak wrote:
> > 
> > I think we should remove it now. +mcasas FYI.
> 
> Done. 
> 
> In K3.14 tree, VIDIOC_REQBUFS in UVC is directly handled by vb2_reqbufs. 
> UVC doesn't set vb2_queue.min_buffers_needed, so the number of returned buffer
> will be the same as the requested if the 1 <= count <= 32.

there is also the queue_setup callback into uvc driver, which is what would be
adjusting. But it does not.

[Pawel Osciak, 2015-03-19 07:12:16]

On 2015/03/18 16:13:20, Justin Chuang wrote:
> On 2015/03/18 11:24:34, Pawel Osciak wrote:
> > 
> > I think we should remove it now. +mcasas FYI.
> 
> Done. 
> 
> In K3.14 tree, VIDIOC_REQBUFS in UVC is directly handled by vb2_reqbufs. 
> UVC doesn't set vb2_queue.min_buffers_needed, so the number of returned buffer
> will be the same as the requested if the 1 <= count <= 32.

there is also the queue_setup callback into uvc driver, which is what would be
adjusting. But it does not.

[Pawel Osciak, 2015-03-19 07:12:57]

lgtm

[Justin Chuang, 2015-03-19 09:54:57]

The CQ bit was checked by jchuang@chromium.org

[commit-bot: I haz the power, 2015-03-19 09:55:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1008343006/60001

[commit-bot: I haz the power, 2015-03-19 11:09:56]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-03-19 11:10:40]

Patchset 3 (id:??) landed as
https://crrev.com/133987bb651125718ea249ddb3215402e1aaa9e1
Cr-Commit-Position: refs/heads/master@{#321333}