chop the width/height to even number when incoming frame has odd number dimension.

chop the width/height to even number when incoming frame has odd number dimension.
Some video capture device has odd number for width/height. Need to chop them to even number.

To test handling of dimension, manually changed FakeVideoCaptureDevice to output odd number dimension in ARGB color space. The client of VideoCaptureController would receives odd number dimension and corresponding frames without crash.

BUG=139004
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=151753

[perkj_chrome, 2012-08-14 06:13:49]

Can you please add a simple unittest? 

Also - I think the clients should never receive the frame_info with odd
dimensions.

http://codereview.chromium.org/10837225/diff/1/content/browser/renderer_host/...
File content/browser/renderer_host/media/video_capture_controller.cc (right):

http://codereview.chromium.org/10837225/diff/1/content/browser/renderer_host/...
content/browser/renderer_host/media/video_capture_controller.cc:367: this,
info));
I think it is better you report the new frame_info to the clients. Ie never
report frames with odd numbers. Use frame_info_ instead.

[mflodman_chromium_OOO, 2012-08-14 08:41:54]

http://codereview.chromium.org/10837225/diff/1/content/browser/renderer_host/...
File content/browser/renderer_host/media/video_capture_controller.cc (right):

http://codereview.chromium.org/10837225/diff/1/content/browser/renderer_host/...
content/browser/renderer_host/media/video_capture_controller.cc:350:
frame_info_= info;
Do we know 'info' has odd number of pixels in this case? 'info' will contain the
information we tell the camera to start with, not what is actually delivered by
the device. I think we need to check 'lenght' in OnIncomingCapturedFrame to see
if it is correct or not, perhaps add resolution information in that call from
VideoCaptureDeviec?

[wjia(left Chromium), 2012-08-14 16:07:01]

http://codereview.chromium.org/10837225/diff/1/content/browser/renderer_host/...
File content/browser/renderer_host/media/video_capture_controller.cc (right):

http://codereview.chromium.org/10837225/diff/1/content/browser/renderer_host/...
content/browser/renderer_host/media/video_capture_controller.cc:350:
frame_info_= info;
On 2012/08/14 08:41:54, mflodman wrote:
> Do we know 'info' has odd number of pixels in this case? 'info' will contain
the
> information we tell the camera to start with, not what is actually delivered
by
> the device. I think we need to check 'lenght' in OnIncomingCapturedFrame to
see
> if it is correct or not, perhaps add resolution information in that call from
> VideoCaptureDeviec?

Based on call stack in crbug.com/139004, in some cases, |info| does have odd
number of width/height. VideoCaptureDevice is supposed to deliver real
width/height via OnFrameInfo, instead of requested width/height. This is the
only place where it can convey width/height to VideoCaptureController. In
OnIncomingCapturedFrame(), it has only length as parameter.

http://codereview.chromium.org/10837225/diff/1/content/browser/renderer_host/...
content/browser/renderer_host/media/video_capture_controller.cc:367: this,
info));
On 2012/08/14 06:13:49, perkj wrote:
> I think it is better you report the new frame_info to the clients. Ie never
> report frames with odd numbers. Use frame_info_ instead.

Good catch! I missed this one. We definitely need to send frame_info_ to client,
instead of |info|.

For the unit test, it needs quite some change in FakeVideoCaptureDevice to
support RGB* format and more configuration. That will bloat
FakeVideoCaptureDevice.

[perkj_chrome, 2012-08-15 14:10:53]

lgtm if you test this and explain how to test this in the cl description. 

Can you hardcode a request of 641*481 pixels on MAC (instead of VGA that is
hardcodec today) and run local preview and make sure that is actually what you
get and that his cl crops to 640*480? 


On 2012/08/14 16:07:01, wjia wrote:
>
http://codereview.chromium.org/10837225/diff/1/content/browser/renderer_host/...
> File content/browser/renderer_host/media/video_capture_controller.cc (right):
> 
>
http://codereview.chromium.org/10837225/diff/1/content/browser/renderer_host/...
> content/browser/renderer_host/media/video_capture_controller.cc:350:
> frame_info_= info;
> On 2012/08/14 08:41:54, mflodman wrote:
> > Do we know 'info' has odd number of pixels in this case? 'info' will contain
> the
> > information we tell the camera to start with, not what is actually delivered
> by
> > the device. I think we need to check 'lenght' in OnIncomingCapturedFrame to
> see
> > if it is correct or not, perhaps add resolution information in that call
from
> > VideoCaptureDeviec?
> 
> Based on call stack in crbug.com/139004, in some cases, |info| does have odd
> number of width/height. VideoCaptureDevice is supposed to deliver real
> width/height via OnFrameInfo, instead of requested width/height. This is the
> only place where it can convey width/height to VideoCaptureController. In
> OnIncomingCapturedFrame(), it has only length as parameter.
> 
>
http://codereview.chromium.org/10837225/diff/1/content/browser/renderer_host/...
> content/browser/renderer_host/media/video_capture_controller.cc:367: this,
> info));
> On 2012/08/14 06:13:49, perkj wrote:
> > I think it is better you report the new frame_info to the clients. Ie never
> > report frames with odd numbers. Use frame_info_ instead.
> 
> Good catch! I missed this one. We definitely need to send frame_info_ to
client,
> instead of |info|.
> 
> For the unit test, it needs quite some change in FakeVideoCaptureDevice to
> support RGB* format and more configuration. That will bloat
> FakeVideoCaptureDevice.

[mflodman_chromium_OOO, 2012-08-16 07:28:04]

After the fact LGTM, sorry I missed that yesterday...

[perkj_chrome, 2012-08-16 07:40:06]

On Thu, Aug 16, 2012 at 9:28 AM, <mflodman@chromium.org> wrote:

> After the fact LGTM, sorry I missed that yesterday...
>
>
https://chromiumcodereview.**appspot.com/10837225/<https://chromiumcodereview...
>


So what happened to the test description?

[wjia(left Chromium), 2012-08-16 16:36:34]

On 2012/08/16 07:40:06, perkj wrote:
> On Thu, Aug 16, 2012 at 9:28 AM, <mailto:mflodman@chromium.org> wrote:
> 
> > After the fact LGTM, sorry I missed that yesterday...
> >
> >
>
https://chromiumcodereview.**appspot.com/10837225/%3Chttps://chromiumcoderevi...>
> >
> 
> 
> So what happened to the test description?

Done.