Improve video capture columns in chrome://media-internals

Video Capture Capabilities on chrome://media-internals page are updated with:

- Layout adjustments
- Parsed capture formats (resolution, fps, [pixel format])
- String representation of capture API types
- Unit test checking all capture API types and video format strings are up to date

Preview screenshot (linux): http://goo.gl/QTPUhj

BUG=425029, 344882
Committed: https://crrev.com/4bf4ed181bd608ddba456ae1b0e95729b8e0f31e
Cr-Commit-Position: refs/heads/master@{#300561}

[burnik, 2014-10-20 17:36:26]

Adjusted layout and capture format parsing

[burnik, 2014-10-20 17:46:03]

Clean up

[burnik, 2014-10-20 18:05:00]

Simplify capture api type stringification

[burnik, 2014-10-20 19:54:12]

Bugfix for capture api type string + unit test

[burnik, 2014-10-20 20:29:00]

Implement Capture API type string

[burnik, 2014-10-20 20:31:26]

Patchset #1 (id:1) has been deleted

[burnik, 2014-10-20 20:31:35]

Patchset #1 (id:20001) has been deleted

[burnik, 2014-10-20 20:31:43]

Patchset #1 (id:40001) has been deleted

[burnik, 2014-10-20 20:31:49]

Patchset #1 (id:60001) has been deleted

[burnik, 2014-10-20 21:37:23]

Add test for video capture format

[burnik, 2014-10-20 21:40:00]

burnik@chromium.org changed reviewers:
+ mcasas@chromium.org, perkj@chromium.org, tommi@chromium.org,
xhwang@chromium.org

[burnik, 2014-10-20 21:42:59]

burnik@chromium.org changed reviewers:
+ jansson@chromium.org

[burnik, 2014-10-20 21:47:53]

Patchset #3 (id:110001) has been deleted

[burnik, 2014-10-20 21:48:21]

Add test for video capture format

[burnik, 2014-10-20 21:56:02]

Patchset #3 (id:130001) has been deleted

[burnik, 2014-10-20 21:56:22]

Add test for video capture format

[burnik, 2014-10-20 21:58:41]

I believe this CL should show proper improvement according to the bug
description.

[tommi (sloooow) - chröme, 2014-10-20 22:09:43]

Wow, this is several flies with one swat :) lgtm % one request

https://codereview.chromium.org/637953008/diff/150001/media/video/capture/vid...
File media/video/capture/video_capture_device.h (right):

https://codereview.chromium.org/637953008/diff/150001/media/video/capture/vid...
media/video/capture/video_capture_device.h:111: const std::string
capture_api_type_string() const {
nit: these methods might as well just return const char* since it's always a
static string that's being returned and there's no need to create a new
std::string from it.

Also, would like for the implementation to live in the cc file.

[jansson, 2014-10-21 07:36:33]

On 2014/10/20 21:58:41, burnik wrote:
> I believe this CL should show proper improvement according to the bug
> description.

Layout looks good (preview screenshot)!

[perkj_chrome, 2014-10-21 07:39:53]

On 2014/10/21 07:36:33, jansson wrote:
> On 2014/10/20 21:58:41, burnik wrote:
> > I believe this CL should show proper improvement according to the bug
> > description.
> 
> Layout looks good (preview screenshot)!

Nice screenshot- can we remove the color format since it will always show I420?
The camera does not actually output I420 so this information is misleading.

[mcasas, 2014-10-21 08:16:23]

LGTM modulo all comments standing out.

https://codereview.chromium.org/637953008/diff/150001/content/browser/media/m...
File content/browser/media/media_internals_unittest.cc (right):

https://codereview.chromium.org/637953008/diff/150001/content/browser/media/m...
content/browser/media/media_internals_unittest.cc:140: // in an expected format
and there are no public methods for accessing the
remove "in"

[burnik, 2014-10-21 11:19:18]

On 2014/10/21 07:39:53, perkj wrote:
> On 2014/10/21 07:36:33, jansson wrote:
> > On 2014/10/20 21:58:41, burnik wrote:
> > > I believe this CL should show proper improvement according to the bug
> > > description.
> > 
> > Layout looks good (preview screenshot)!
> 
> Nice screenshot- can we remove the color format since it will always show
I420?
> The camera does not actually output I420 so this information is misleading.

Done

[burnik, 2014-10-21 11:26:21]

Move to impl, update comments, remove pixel format

[burnik, 2014-10-21 11:27:04]

Updated.

https://codereview.chromium.org/637953008/diff/150001/content/browser/media/m...
File content/browser/media/media_internals_unittest.cc (right):

https://codereview.chromium.org/637953008/diff/150001/content/browser/media/m...
content/browser/media/media_internals_unittest.cc:140: // in an expected format
and there are no public methods for accessing the
On 2014/10/21 08:16:23, mcasas wrote:
> remove "in"

Done.

https://codereview.chromium.org/637953008/diff/150001/media/video/capture/vid...
File media/video/capture/video_capture_device.h (right):

https://codereview.chromium.org/637953008/diff/150001/media/video/capture/vid...
media/video/capture/video_capture_device.h:111: const std::string
capture_api_type_string() const {
On 2014/10/20 22:09:42, tommi wrote:
> nit: these methods might as well just return const char* since it's always a
> static string that's being returned and there's no need to create a new
> std::string from it.
> 
> Also, would like for the implementation to live in the cc file.

Done.

[burnik, 2014-10-21 12:15:39]

burnik@chromium.org changed reviewers:
- jansson@chromium.org

[burnik, 2014-10-21 12:15:52]

burnik@chromium.org changed reviewers:
- perkj@chromium.org

[xhwang, 2014-10-21 19:07:49]

lgtm % nit

https://codereview.chromium.org/637953008/diff/30007/content/browser/media/me...
File content/browser/media/media_internals_unittest.cc (right):

https://codereview.chromium.org/637953008/diff/30007/content/browser/media/me...
content/browser/media/media_internals_unittest.cc:123:
"DIRECT_SHOW_WDM_CROSSBAR";
Since these are strings, does it make sense to make them more reader friendly,
e.g. s/DIRECT_SHOW/Direct Show/ ?

https://codereview.chromium.org/637953008/diff/30007/media/video/capture/vide...
File media/video/capture/video_capture_device.cc (right):

https://codereview.chromium.org/637953008/diff/30007/media/video/capture/vide...
media/video/capture/video_capture_device.cc:71: return "API_TYPE_UNKNOWN";
ditto about readable names.

[burnik, 2014-10-21 19:25:16]

Human readable capture API type

[burnik, 2014-10-21 19:37:11]

Done. :)

https://codereview.chromium.org/637953008/diff/30007/content/browser/media/me...
File content/browser/media/media_internals_unittest.cc (right):

https://codereview.chromium.org/637953008/diff/30007/content/browser/media/me...
content/browser/media/media_internals_unittest.cc:123:
"DIRECT_SHOW_WDM_CROSSBAR";
On 2014/10/21 19:07:49, xhwang wrote:
> Since these are strings, does it make sense to make them more reader friendly,
> e.g. s/DIRECT_SHOW/Direct Show/ ?

Done.

https://codereview.chromium.org/637953008/diff/30007/media/video/capture/vide...
File media/video/capture/video_capture_device.cc (right):

https://codereview.chromium.org/637953008/diff/30007/media/video/capture/vide...
media/video/capture/video_capture_device.cc:71: return "API_TYPE_UNKNOWN";
On 2014/10/21 19:07:49, xhwang wrote:
> ditto about readable names.

Done.

[burnik, 2014-10-21 20:31:48]

The CQ bit was checked by burnik@chromium.org

[commit-bot: I haz the power, 2014-10-21 20:33:20]

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

[commit-bot: I haz the power, 2014-10-21 22:35:15]

Committed patchset #5 (id:180001)

[commit-bot: I haz the power, 2014-10-21 22:36:07]

Patchset 5 (id:??) landed as
https://crrev.com/4bf4ed181bd608ddba456ae1b0e95729b8e0f31e
Cr-Commit-Position: refs/heads/master@{#300561}