Add GetDeviceSupportedFormats to VideoCaptureDeviceMFWin

Add GetDeviceSupportedFormats to VideoCaptureDeviceMFWin

Added GetDeviceSupportedFormats() to VideoCaptureWin that
simply multiplexes the MF/DS implementation. DS remains
NOTIMPLEMENTED(), MF version is written.

Added a --force-mediafoundation flag to force use of MF
in Win 7 and Win 8 non-metro.

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

[mcasas, 2014-01-29 18:19:28]

Hi tommi@ could you have a first and rough look at this?

I tentatively added a flag --force-mediafoundation, 
currently used to force MF in Win7 and 8-non-metro.
If it doesn't sound ok I'll remove it, if it does 
I should replicate its behaviour to other parts of
video_capture_device_win.cc.

Thanks!

[tommi (sloooow) - chröme, 2014-01-30 13:25:17]

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
File media/video/capture/win/video_capture_device_mf_win.cc (right):

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:289: //static
// static

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:299: if (!SUCCEEDED(hr =
MFCreateSourceReaderFromMediaSource(source, NULL,
instead of "if (!SUCCEEDED" use "if (FAILED"

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:301: DLOG(WARNING) <<
"MFCreateSourceReaderFromMediaSource: " << std::hex << hr;
you must bail out at this point.  If MFCreateSourceReaderFromMediaSource failed,
then |reader| will be NULL.  A couple of lines below, you actually use |reader|
and that's where we'll crash if this happens.

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:306: GUID type_guid;
For these variable definitions (type_guid, width, height, etc), please define
them for the scope they are used.

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:309:
MF_SOURCE_READER_FIRST_VIDEO_STREAM, stream_index, type.Receive()))) {
strange indent

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:310: 
remove empty line.

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:311: VideoCaptureFormat
capture_format;
define this variable where you need it (smallest scope)

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:314: type,
MF_MT_FRAME_SIZE, &width, &height))) {
I would prefer to format this as:

    if (FAILED(hr = MFGetAttributeSize(
            type, MF_MT_FRAME_SIZE, &width, &height))) {

same for below

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:315: DLOG(WARNING) <<
"MFGetAttributeSize: " << std::hex << hr;
DLOG(ERROR)

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:321: DLOG(WARNING) <<
"MFGetAttributeSize: " << std::hex << hr;
DLOG(ERROR)

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:327: DLOG(WARNING) <<
"GetGUID: " << std::hex << hr;
DLOG(ERROR)

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:333: stream_index++;
nit: ++stream_index;

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:340: return;
remove

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

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_win.cc:279: //static
// static

[mcasas, 2014-01-30 16:29:23]

tommi@ PTAL.

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
File media/video/capture/win/video_capture_device_mf_win.cc (right):

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:289: //static
On 2014/01/30 13:25:17, tommi wrote:
> // static

Done.

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:299: if (!SUCCEEDED(hr =
MFCreateSourceReaderFromMediaSource(source, NULL,
On 2014/01/30 13:25:17, tommi wrote:
> instead of "if (!SUCCEEDED" use "if (FAILED"

Done.

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:301: DLOG(WARNING) <<
"MFCreateSourceReaderFromMediaSource: " << std::hex << hr;
On 2014/01/30 13:25:17, tommi wrote:
> you must bail out at this point.  If MFCreateSourceReaderFromMediaSource
failed,
> then |reader| will be NULL.  A couple of lines below, you actually use
|reader|
> and that's where we'll crash if this happens.

Done.

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:306: GUID type_guid;
On 2014/01/30 13:25:17, tommi wrote:
> For these variable definitions (type_guid, width, height, etc), please define
> them for the scope they are used.

Moved closest to their use.

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:309:
MF_SOURCE_READER_FIRST_VIDEO_STREAM, stream_index, type.Receive()))) {
On 2014/01/30 13:25:17, tommi wrote:
> strange indent

Done.

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:310: 
On 2014/01/30 13:25:17, tommi wrote:
> remove empty line.

Done.

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:314: type,
MF_MT_FRAME_SIZE, &width, &height))) {
On 2014/01/30 13:25:17, tommi wrote:
> I would prefer to format this as:
> 
>     if (FAILED(hr = MFGetAttributeSize(
>             type, MF_MT_FRAME_SIZE, &width, &height))) {
> 
> same for below

Done.

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:315: DLOG(WARNING) <<
"MFGetAttributeSize: " << std::hex << hr;
On 2014/01/30 13:25:17, tommi wrote:
> DLOG(ERROR)

Done.

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:321: DLOG(WARNING) <<
"MFGetAttributeSize: " << std::hex << hr;
On 2014/01/30 13:25:17, tommi wrote:
> DLOG(ERROR)

Done.

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:327: DLOG(WARNING) <<
"GetGUID: " << std::hex << hr;
On 2014/01/30 13:25:17, tommi wrote:
> DLOG(ERROR)

Done.

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:333: stream_index++;
On 2014/01/30 13:25:17, tommi wrote:
> nit: ++stream_index;

Done.

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:340: return;
On 2014/01/30 13:25:17, tommi wrote:
> remove

Done.

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

https://codereview.chromium.org/149443003/diff/80001/media/video/capture/win/...
media/video/capture/win/video_capture_device_win.cc:279: //static
On 2014/01/30 13:25:17, tommi wrote:
> // static

Done.

[tommi (sloooow) - chröme, 2014-01-30 17:39:08]

lgtm

https://codereview.chromium.org/149443003/diff/100001/media/video/capture/win...
File media/video/capture/win/video_capture_device_mf_win.cc (right):

https://codereview.chromium.org/149443003/diff/100001/media/video/capture/win...
media/video/capture/win/video_capture_device_mf_win.cc:315: VideoCaptureFormat
capture_format;
nit: you don't really need this variable until after the next if(), right?

https://codereview.chromium.org/149443003/diff/100001/media/video/capture/win...
media/video/capture/win/video_capture_device_mf_win.cc:324:
capture_format.frame_rate = denominator ? numerator/denominator : 0;
nit: spaces around /

[mcasas, 2014-01-30 17:52:15]

tommi@ PTAL (nits and extended --force-avfoundation 
to GetDeviceNames() ).

scherkus@: media/base/media_switches.{cc,h}

https://codereview.chromium.org/149443003/diff/100001/media/video/capture/win...
File media/video/capture/win/video_capture_device_mf_win.cc (right):

https://codereview.chromium.org/149443003/diff/100001/media/video/capture/win...
media/video/capture/win/video_capture_device_mf_win.cc:315: VideoCaptureFormat
capture_format;
On 2014/01/30 17:39:08, tommi wrote:
> nit: you don't really need this variable until after the next if(), right?

Is used in l.316.

https://codereview.chromium.org/149443003/diff/100001/media/video/capture/win...
media/video/capture/win/video_capture_device_mf_win.cc:324:
capture_format.frame_rate = denominator ? numerator/denominator : 0;
On 2014/01/30 17:39:08, tommi wrote:
> nit: spaces around /

Done.

[tommi (sloooow) - chröme, 2014-01-30 17:54:05]

lgtm

[scherkus (not reviewing), 2014-01-31 00:20:08]

lgtm w/ nit

https://codereview.chromium.org/149443003/diff/120001/media/video/capture/win...
File media/video/capture/win/video_capture_device_mf_win.cc (right):

https://codereview.chromium.org/149443003/diff/120001/media/video/capture/win...
media/video/capture/win/video_capture_device_mf_win.cc:292: 
remove extra blank line

[commit-bot: I haz the power, 2014-01-31 08:14:33]

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

[commit-bot: I haz the power, 2014-01-31 13:52:10]

Change committed as 248178

[commit-bot: I haz the power, 2014-01-31 13:52:12]

CQ bit was unchecked on CL. Ignoring.