Windows video capture: Remove duplicated code from GetDeviceSupportedFormats*

Windows video capture: Remove duplicated code from GetDeviceSupportedFormats*

The code for GetDeviceSupportedFormatsMediaFoundation and GetDeviceSupportedFormatsDirectShow are already implemented in video_capture_device_mf_win.cc and video_capture_device_win.cc respectively. This CL refactors the implementation so that video_capture_device_factory_win.cc can make use of that code.

Committed: https://crrev.com/b6dae80f8f10b8af1e3026de9f5af4f3128e812a
Cr-Commit-Position: refs/heads/master@{#301891}

[magjed_chromium, 2014-09-09 16:10:42]

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

[magjed_chromium, 2014-09-09 18:46:53]

Please take a look.

[magjed_chromium, 2014-09-10 16:39:00]

Patchset #1 (id:1) has been deleted

[magjed_chromium, 2014-09-10 16:39:08]

Patchset #1 (id:20001) has been deleted

[perkj_chrome, 2014-09-19 10:04:55]

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

https://codereview.chromium.org/558503003/diff/40001/media/video/capture/win/...
media/video/capture/win/video_capture_device_mf_win.cc:69: kFirstVideoStream,
stream_index, type.Receive()))) {
indentation.
while (SUCCEEDED(hr = source->GetNativeMediaType(
    kFirstVideoStream, stream_index,type.Receive()))) {   


I think.....

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

https://codereview.chromium.org/558503003/diff/40001/media/video/capture/win/...
media/video/capture/win/video_capture_device_win.cc:437: bool
VideoCaptureDeviceWin::GetDeviceSupportedFormats(
It looks like this is doing much more that gettign the supported formats. Can it
be split up in multiple functions that only do what the function name suggests?

[magjed_chromium, 2014-10-16 10:44:12]

Patchset #3 (id:80001) has been deleted

[magjed_chromium, 2014-10-16 10:44:20]

Patchset #2 (id:60001) has been deleted

[magjed_chromium, 2014-10-16 10:44:42]

mcasas: Please take a look.

[magjed_chromium, 2014-10-22 16:00:45]

mcasas: Friendly ping.

[tommi (sloooow) - chröme, 2014-10-22 16:23:13]

https://codereview.chromium.org/558503003/diff/40001/media/video/capture/win/...
File media/video/capture/win/capability_list_win.h (right):

https://codereview.chromium.org/558503003/diff/40001/media/video/capture/win/...
media/video/capture/win/capability_list_win.h:22: int stream_index;
can this be const?

https://codereview.chromium.org/558503003/diff/40001/media/video/capture/win/...
File media/video/capture/win/video_capture_device_factory_win.cc (right):

https://codereview.chromium.org/558503003/diff/40001/media/video/capture/win/...
media/video/capture/win/video_capture_device_factory_win.cc:325:
formats->clear();
if formats needs to be empty on entry, I prefer to add a
DCHECK(formats->empty()) - but only if that's really a requirement of the
function.  Otherwise, I'd just like to remove this line and leave it up to the
caller to make sure it's empty (which is the most common case - or perhaps the
only case?)

https://codereview.chromium.org/558503003/diff/40001/media/video/capture/win/...
media/video/capture/win/video_capture_device_factory_win.cc:330: for
(VideoCaptureFormats::iterator it = formats->begin();
nit: empty line before this one

https://codereview.chromium.org/558503003/diff/40001/media/video/capture/win/...
File media/video/capture/win/video_capture_device_win.h (right):

https://codereview.chromium.org/558503003/diff/40001/media/video/capture/win/...
media/video/capture/win/video_capture_device_win.h:67:
base::win::ScopedComPtr<IBaseFilter>* capture_filter,
this is a bit strange for COM code.  If this is meant to be an out pointer, the
convention is like GetDeviceFilter is.  Same for the next param.

[mcasas, 2014-10-23 08:25:09]

lgtm with a nit.

Please 2x check that there's no extra operations when creating
VideoCaptureDeviceWin + Init()ing it + GetDeviceSupportedFormats
as opposed to what's done now... With all those devices out there,
apparently innocent changes cause big troubles! :)

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

https://codereview.chromium.org/558503003/diff/100001/media/video/capture/win...
media/video/capture/win/video_capture_device_factory_win.cc:345: for (const
VideoCaptureFormat& format : *formats)
indent

[magjed_chromium, 2014-10-29 15:35:46]

Patchset #5 (id:160001) has been deleted

[magjed_chromium, 2014-10-29 15:35:52]

Patchset #4 (id:140001) has been deleted

[magjed_chromium, 2014-10-29 15:35:59]

Patchset #3 (id:120001) has been deleted

[magjed_chromium, 2014-10-29 17:06:37]

Patchset #3 (id:180001) has been deleted

[magjed_chromium, 2014-10-29 17:56:35]

The CQ bit was checked by magjed@chromium.org

[commit-bot: I haz the power, 2014-10-29 17:57:50]

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

[commit-bot: I haz the power, 2014-10-29 18:06:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-29 18:06:43]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[tommi (sloooow) - chröme, 2014-10-29 18:08:15]

The CQ bit was checked by tommi@chromium.org

[tommi (sloooow) - chröme, 2014-10-29 18:08:15]

lgtm

[commit-bot: I haz the power, 2014-10-29 18:08:43]

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

[commit-bot: I haz the power, 2014-10-29 18:47:29]

Committed patchset #3 (id:200001)

[commit-bot: I haz the power, 2014-10-29 18:48:08]

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

[dmichael (off chromium), 2014-10-30 20:48:07]

A revert of this CL (patchset #3 id:200001) has been created in
https://codereview.chromium.org/691143002/ by dmichael@chromium.org.

The reason for reverting is: See
https://code.google.com/p/chromium/issues/detail?id=428958.