Improved CoreAudioUtil::IsSupported()

media/audio/win/core_audio_util_win.cc

Index: media/audio/win/core_audio_util_win.cc
diff --git a/media/audio/win/core_audio_util_win.cc b/media/audio/win/core_audio_util_win.cc
index 4bae7b93f44caa3cb516935d4d69162d48e3dd4a..a28ce4bbe8a47247a5083320c0556128bf6e37c3 100644
--- a/media/audio/win/core_audio_util_win.cc
+++ b/media/audio/win/core_audio_util_win.cc
@@ -76,6 +76,16 @@ bool LoadAudiosesDll() {
   return (LoadLibraryExW(path, NULL, LOAD_WITH_ALTERED_SEARCH_PATH) != NULL);
 }
 
+bool CanCreateDeviceEnumerator() {
+  ScopedComPtr<IMMDeviceEnumerator> device_enumerator;
+  HRESULT hr = CoCreateInstance(__uuidof(MMDeviceEnumerator),
+                                NULL,
+                                CLSCTX_INPROC_SERVER,
+                                __uuidof(IMMDeviceEnumerator),
+                                device_enumerator.ReceiveVoid());
+  return SUCCEEDED(hr);

[tommi (sloooow) - chröme, 2013/03/05 09:32:10]

add a check here:
CHECK_NE(hr, CO_E_NOTINITIALIZED);

[henrika (OOO until Aug 14), 2013/03/05 09:49:35]

Done.

+}
+
 bool CoreAudioUtil::IsSupported() {
   // Microsoft does not plan to make the Core Audio APIs available for use
   // with earlier versions of Windows, including Microsoft Windows Server 2003,
@@ -90,7 +100,15 @@ bool CoreAudioUtil::IsSupported() {
   // See http://crbug.com/166397 why this extra step is required to guarantee
   // Core Audio support.
   static bool g_audioses_dll_available = LoadAudiosesDll();
-  return g_audioses_dll_available;
+  if (g_audioses_dll_available)

[DaleCurtis, 2013/03/04 17:18:26]

!g_audioses_dll_available ?

[henrika (OOO until Aug 14), 2013/03/05 09:09:11]

My bad. Will fix. Thanks!

+    return false;
+
+  // Being able to load the Audioses.dll does not seem to be sufficient for
+  // all devices to guarantee Core Audio support. To be 100%, we also verify
+  // that it is possible to a create the IMMDeviceEnumerator interface. If this
+  // works as well we should be home free.
+  static bool g_can_create_device_enumerator = CanCreateDeviceEnumerator();

[tommi (sloooow) - chröme, 2013/03/04 17:29:01]

If this function is called on a non-COM initialized thread, then it will fail. 
Also, since we're using the MTA generally, then there's a further requirement
that threads that call this function be initialized into the MTA since the
DLL(s) will potentially make assumptions during initialization about the
apartment model being used.

So, is there a way to guarantee that we are currently in an MTA thread here? 
Otherwise we can erroneously fail and I suspect that that false positive would
cause problems for a lot more users than we're trying to fix the problem for.

[henrika (OOO until Aug 14), 2013/03/05 09:09:11]

Tommi, great comments as usual, please see replies inline:

On 2013/03/04 17:29:01, tommi wrote:
Yes it will but why is that a problem? Then we know that Core Audio is not
supported, right?
Yes, but I don't understand how that is related to this patch (see below).
I added logging of all calls and included thread ID + HRESULT from
CoInitializeEx(COINIT_MULTITHREADED) =>

[2376:3928:0305/095554:VERBOSE1:core_audio_util_win.cc(92)] #
GetCurrentThreadId: 3928
[2376:3928:0305/095554:ERROR:core_audio_util_win.cc(111)] # COINIT_MULTITHREADED
=> hr: 80010106
[2376:6636:0305/095554:VERBOSE1:core_audio_util_win.cc(92)] #
GetCurrentThreadId: 6636
[2376:6636:0305/095554:ERROR:core_audio_util_win.cc(111)] # COINIT_MULTITHREADED
=> hr: 1
[2376:6636:0305/095554:VERBOSE1:core_audio_util_win.cc(92)] #
GetCurrentThreadId: 6636
[2376:6636:0305/095554:ERROR:core_audio_util_win.cc(111)] # COINIT_MULTITHREADED
=> hr: 1
[2376:3928:0305/095554:VERBOSE1:core_audio_util_win.cc(92)] #
GetCurrentThreadId: 3928
[2376:3928:0305/095554:ERROR:core_audio_util_win.cc(111)] # COINIT_MULTITHREADED
=> hr: 80010106
[2376:6636:0305/095554:VERBOSE1:core_audio_util_win.cc(92)] #
GetCurrentThreadId: 6636
[2376:6636:0305/095554:ERROR:core_audio_util_win.cc(111)] # COINIT_MULTITHREADED
=> hr: 1
[2376:6636:0305/095554:VERBOSE1:core_audio_util_win.cc(92)] #
GetCurrentThreadId: 6636
[2376:6636:0305/095554:ERROR:core_audio_util_win.cc(111)] # COINIT_MULTITHREADED
=> hr: 1
[2376:6636:0305/095554:VERBOSE1:core_audio_util_win.cc(92)] #
GetCurrentThreadId: 6636
[2376:6636:0305/095554:ERROR:core_audio_util_win.cc(111)] # COINIT_MULTITHREADED
=> hr: 1
[2376:6636:0305/095554:VERBOSE1:core_audio_util_win.cc(92)] #
GetCurrentThreadId: 6636
[2376:6636:0305/095554:ERROR:core_audio_util_win.cc(111)] # COINIT_MULTITHREADED
=> hr: 1

where RPC_E_CHANGED_MODE <=> 80010106

Thread ID 6636 is the AudioThread and 3928 is the CrBrowserMain thread hence it
seems like the main thread is COM STA. Hence, we do ask for support under two
different models but we never use Core Audio under other than MTA.

Sound OK?

[tommi (sloooow) - chröme, 2013/03/05 09:32:10]

Yes, that sounds OK with me but ask that we add the CHECK_NE() above so that we
will catch false positives.

On 2013/03/05 09:09:11, henrika wrote:

[henrika (OOO until Aug 14), 2013/03/05 09:49:35]

Got it, thanks. Part of my comments above were invalid since I forgot that we
only check once. Never mind. Will make change as proposed.

+  return g_can_create_device_enumerator;
 }
 
 base::TimeDelta CoreAudioUtil::RefererenceTimeToTimeDelta(REFERENCE_TIME time) {
@@ -141,9 +159,8 @@ ScopedComPtr<IMMDeviceEnumerator> CoreAudioUtil::CreateDeviceEnumerator() {
                                 CLSCTX_INPROC_SERVER,
                                 __uuidof(IMMDeviceEnumerator),
                                 device_enumerator.ReceiveVoid());
-  // CO_E_NOTINITIALIZED is the most likely reason for failure and if that
-  // happens we might as well die here.
-  CHECK(SUCCEEDED(hr));
+  LOG_IF(ERROR, FAILED(hr)) << "IMMDeviceEnumerator::CreateDeviceEnumerator: "
+                            << std::hex << hr;
   return device_enumerator;
 }