Add GetDeviceSupportedFormats to Win DirectShow.

Add GetDeviceSupportedFormats to Win DirectShow.

Extracted pixel format translation logic to 
TranslateMediaSubtypeToPixelFormat().

BUG=309554, 314161
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=251703

[mcasas, 2014-02-17 09:58:13]

tommi@ PTAL.

[tommi (sloooow) - chröme, 2014-02-17 12:58:29]

https://codereview.chromium.org/156583004/diff/230001/media/video/capture/win...
File media/video/capture/win/video_capture_device_win.cc (right):

https://codereview.chromium.org/156583004/diff/230001/media/video/capture/win...
media/video/capture/win/video_capture_device_win.cc:153: VideoPixelFormat
TranslateMediaSubtypeToPixelFormat(GUID subtype) {
sub_type?  Also, pass by const&?

https://codereview.chromium.org/156583004/diff/230001/media/video/capture/win...
media/video/capture/win/video_capture_device_win.cc:155: if (subtype ==
kMediaSubTypeI420) {
I would like to see this data driven instead of multiple conditionals:

struct {
  const GUID& sub_type;
  VideoPixelFormat format;
} pixel_formats[] = {
  { kMediaSubTypeI420, PIXEL_FORMAT_I420 },
  { MEDIASUBTYPE_IYUV, PIXEL_FORMAT_I420 },
  ...
};

for (size_t i = 0; i < ARRAYSIZE_UNSAFE(pixel_formats); ++i) {
  if (sub_type == pixel_format[i].sub_type)
    return pixel_format[i].format;
}

#ifndef NDEBUG
  WCHAR guid_str[128];
  StringFromGUID2(subtype, guid_str, arraysize(guid_str));
  DVLOG(2) << "Device (also) supports an unknown media type " << guid_str;
#endif

return PIXEL_FORMAT_UNKNOWN;

https://codereview.chromium.org/156583004/diff/230001/media/video/capture/win...
media/video/capture/win/video_capture_device_win.cc:172:
StringFromGUID2(subtype, guid_str, arraysize(guid_str));
(I know this is just moved code)
This should be inside an #ifndef NDEBUG block (see above) since otherwise it's
just code bloat.

https://codereview.chromium.org/156583004/diff/230001/media/video/capture/win...
media/video/capture/win/video_capture_device_win.cc:326: // Walk the capture
devices. No need to check for "google camera adapter".
maybe elaborate on why not?

https://codereview.chromium.org/156583004/diff/230001/media/video/capture/win...
media/video/capture/win/video_capture_device_win.cc:401: ?
static_cast<int>(kSecondsToReferenceTime / h->AvgTimePerFrame)
nit: I think the convention is to have the operator last in line and not first.
<cond> ?
Foo() :
Bar();

[mcasas, 2014-02-17 14:56:19]

tommi@ PTAL.

https://codereview.chromium.org/156583004/diff/230001/media/video/capture/win...
File media/video/capture/win/video_capture_device_win.cc (right):

https://codereview.chromium.org/156583004/diff/230001/media/video/capture/win...
media/video/capture/win/video_capture_device_win.cc:153: VideoPixelFormat
TranslateMediaSubtypeToPixelFormat(GUID subtype) {
On 2014/02/17 12:58:29, tommi wrote:
> sub_type?  Also, pass by const&?

Done.

https://codereview.chromium.org/156583004/diff/230001/media/video/capture/win...
media/video/capture/win/video_capture_device_win.cc:155: if (subtype ==
kMediaSubTypeI420) {
On 2014/02/17 12:58:29, tommi wrote:
> I would like to see this data driven instead of multiple conditionals:
> 
> struct {
>   const GUID& sub_type;
>   VideoPixelFormat format;
> } pixel_formats[] = {
>   { kMediaSubTypeI420, PIXEL_FORMAT_I420 },
>   { MEDIASUBTYPE_IYUV, PIXEL_FORMAT_I420 },
>   ...
> };
> 
> for (size_t i = 0; i < ARRAYSIZE_UNSAFE(pixel_formats); ++i) {
>   if (sub_type == pixel_format[i].sub_type)
>     return pixel_format[i].format;
> }
> 
> #ifndef NDEBUG
>   WCHAR guid_str[128];
>   StringFromGUID2(subtype, guid_str, arraysize(guid_str));
>   DVLOG(2) << "Device (also) supports an unknown media type " << guid_str;
> #endif
> 
> return PIXEL_FORMAT_UNKNOWN;

Done.

https://codereview.chromium.org/156583004/diff/230001/media/video/capture/win...
media/video/capture/win/video_capture_device_win.cc:172:
StringFromGUID2(subtype, guid_str, arraysize(guid_str));
On 2014/02/17 12:58:29, tommi wrote:
> (I know this is just moved code)
> This should be inside an #ifndef NDEBUG block (see above) since otherwise it's
> just code bloat.

Done.

https://codereview.chromium.org/156583004/diff/230001/media/video/capture/win...
media/video/capture/win/video_capture_device_win.cc:326: // Walk the capture
devices. No need to check for "google camera adapter".
On 2014/02/17 12:58:29, tommi wrote:
> maybe elaborate on why not?

Done.

https://codereview.chromium.org/156583004/diff/230001/media/video/capture/win...
media/video/capture/win/video_capture_device_win.cc:401: ?
static_cast<int>(kSecondsToReferenceTime / h->AvgTimePerFrame)
On 2014/02/17 12:58:29, tommi wrote:
> nit: I think the convention is to have the operator last in line and not
first.
> <cond> ?
> Foo() :
> Bar();

Done.

[tommi (sloooow) - chröme, 2014-02-17 15:26:43]

lgtm

[mcasas, 2014-02-17 16:19:23]

The CQ bit was checked by mcasas@chromium.org

[commit-bot: I haz the power, 2014-02-17 16:19:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mcasas@chromium.org/156583004/290001

[commit-bot: I haz the power, 2014-02-17 19:17:55]

Change committed as 251703