VideoCaptureDevice Win: Enumerate devices' supported formats when they don't have a DevicePath prop…

VideoCaptureDevice Win: Enumerate devices' supported formats when they don't have a DevicePath property.

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

[mcasas, 2014-06-09 18:33:40]

niklase@ let's see if this patch does the trick.

[Niklas Enbom, 2014-06-09 20:34:40]

Getting there... but it seems like the frame rate is truncated to int:

[6548:18888:0609/133318:VERBOSE1:video_capture_device_factory_win.cc(303)]
Decklink Video Capture resolution: 720x486, fps: 29, pixel format: 3
[6548:18888:0609/133318:VERBOSE1:video_capture_device_factory_win.cc(303)]
Decklink Video Capture resolution: 720x576, fps: 25, pixel format: 3
[6548:18888:0609/133318:VERBOSE1:video_capture_device_factory_win.cc(303)]
Decklink Video Capture resolution: 1920x1080, fps: 25, pixel format: 3
[6548:18888:0609/133318:VERBOSE1:video_capture_device_factory_win.cc(303)]
Decklink Video Capture resolution: 1920x1080, fps: 29, pixel format: 3
[6548:18888:0609/133318:VERBOSE1:video_capture_device_factory_win.cc(303)]
Decklink Video Capture resolution: 1920x1080, fps: 30, pixel format: 3
[6548:18888:0609/133318:VERBOSE1:video_capture_device_factory_win.cc(303)]
Decklink Video Capture resolution: 1920x1080, fps: 50, pixel format: 3
[6548:18888:0609/133318:VERBOSE1:video_capture_device_factory_win.cc(303)]
Decklink Video Capture resolution: 1920x1080, fps: 59, pixel format: 3
[6548:18888:0609/133318:VERBOSE1:video_capture_device_factory_win.cc(303)]
Decklink Video Capture resolution: 1920x1080, fps: 60, pixel format: 3
[6548:18888:0609/133318:VERBOSE1:video_capture_device_factory_win.cc(303)]
Decklink Video Capture resolution: 1280x720, fps: 50, pixel format: 3
[6548:18888:0609/133318:VERBOSE1:video_capture_device_factory_win.cc(303)]
Decklink Video Capture resolution: 1280x720, fps: 59, pixel format: 3



On 2014/06/09 18:33:40, mcasas wrote:
> niklase@ let's see if this patch does the trick.

[mcasas, 2014-06-09 20:46:48]

Float frame rates now.

[Niklas Enbom, 2014-06-09 22:03:23]

This works! Assuming you set constraints forcing the right frame rate.
On 2014/06/09 20:46:48, mcasas wrote:
> Float frame rates now.

[mcasas, 2014-06-09 22:07:34]

tommi@ PTAL.

[tommi (sloooow) - chröme, 2014-06-10 07:36:29]

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

https://codereview.chromium.org/328463002/diff/20001/media/video/capture/win/...
media/video/capture/win/video_capture_device_factory_win.cc:226: // If there is
no clear DevicePath, try with Description and FriendlyName.
Is fetching the Description before FriendlyName the right thing to do?  I would
have guessed the opposite order but I don't have any data to back that up.  Can
you document in what cases this can happen? (no devicepath, description is more
correct than FriendlyName, etc.

https://codereview.chromium.org/328463002/diff/20001/media/video/capture/win/...
media/video/capture/win/video_capture_device_factory_win.cc:300:
(kSecondsToReferenceTime / static_cast<float>(h->AvgTimePerFrame)) :
nit: extra parenthesis aren't needed

[mcasas, 2014-06-10 15:01:15]

tommi@ PTAL.

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

https://codereview.chromium.org/328463002/diff/20001/media/video/capture/win/...
media/video/capture/win/video_capture_device_factory_win.cc:226: // If there is
no clear DevicePath, try with Description and FriendlyName.
On 2014/06/10 07:36:29, tommi wrote:
> Is fetching the Description before FriendlyName the right thing to do?  I
would
> have guessed the opposite order but I don't have any data to back that up. 
Can
> you document in what cases this can happen? (no devicepath, description is
more
> correct than FriendlyName, etc.

The idea/sinppet comes from l.128-l.130 of this file, GetDeviceNames().

Added some documentation.

https://codereview.chromium.org/328463002/diff/20001/media/video/capture/win/...
media/video/capture/win/video_capture_device_factory_win.cc:300:
(kSecondsToReferenceTime / static_cast<float>(h->AvgTimePerFrame)) :
On 2014/06/10 07:36:29, tommi wrote:
> nit: extra parenthesis aren't needed

Done.

[tommi (sloooow) - chröme, 2014-06-11 12:46:45]

lgtm.  To me it still feels like FriendlyName should come first.  It would be
good to figure that out.  Can you add a TODO for me where this is done so that
I'll look into it at some point?

[mcasas, 2014-06-11 16:11:17]

The CQ bit was checked by mcasas@chromium.org

[commit-bot: I haz the power, 2014-06-11 16:14:25]

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

[mcasas, 2014-06-12 00:35:25]

The CQ bit was checked by mcasas@chromium.org

[commit-bot: I haz the power, 2014-06-12 00:36:13]

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

[mcasas, 2014-06-12 00:36:25]

tommi@: Landing this CL with the TODO added. A Note: part of it happened to
spill (my fault) into another CL (https://codereview.chromium.org/317073010/),
hence the smart rebase. Luckily they were both in flight very closely...

[commit-bot: I haz the power, 2014-06-12 02:21:07]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/18954)

[commit-bot: I haz the power, 2014-06-12 02:23:28]

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

[commit-bot: I haz the power, 2014-06-12 02:23:32]

Try jobs failed on following builders:
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/19004)

[mcasas, 2014-06-12 03:07:04]

The CQ bit was checked by mcasas@chromium.org

[commit-bot: I haz the power, 2014-06-12 03:10:13]

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

[commit-bot: I haz the power, 2014-06-12 06:56:14]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-06-12 08:55:41]

Change committed as 276590