Improves media::CoreAudioUtil::GetPreferredAudioParameters.

This CL is an attempt to reduce risk of crashes in GetPreferredAudioParameters
which lately has added support for IAudioClient3.

Adds version check and an improved usage of ComPtr::As.

Bug: 898789
Change-Id: I3e8e58543cfd5d4d5029aa90668119acb411a867
Reviewed-on: https://chromium-review.googlesource.com/c/1312949Reviewed-by: Oskar Sundbom <ossu@chromium.org>
Commit-Queue: Henrik Andreasson <henrika@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605318}

media/audio/win/core_audio_util_win.cc

@@ -21,6 +21,7 @@
 #include "base/win/scoped_handle.h"
 #include "base/win/scoped_propvariant.h"
 #include "base/win/scoped_variant.h"
+#include "base/win/windows_version.h"
 #include "media/audio/audio_device_description.h"
 #include "media/base/media_switches.h"
 
@@ -183,6 +184,10 @@ ChannelConfig ChannelLayoutToChannelConfig(ChannelLayout layout) {
   }
 }
 
+bool IAudioClient3IsSupported() {
+  return CoreAudioUtil::GetIAudioClientVersion() >= 3;
+}
+
 std::string GetDeviceID(IMMDevice* device) {
   ScopedCoMem<WCHAR> device_id_com;
   std::string device_id;
@@ -367,6 +372,21 @@ ComPtr<IAudioClient> CreateClientInternal(IMMDevice* audio_device,
   return audio_client;
 }
 
+// Creates and activates an IAudioClient3 COM object given the selected
+// endpoint device.
+ComPtr<IAudioClient3> CreateClientInternal3(IMMDevice* audio_device,
+                                            const UMALogCallback& uma_log_cb) {
+  if (!audio_device)
+    return ComPtr<IAudioClient3>();
+
+  ComPtr<IAudioClient3> audio_client;
+  HRESULT hr = audio_device->Activate(
+      __uuidof(IAudioClient3), CLSCTX_INPROC_SERVER, NULL, &audio_client);
+  DVLOG_IF(1, FAILED(hr)) << "IMMDevice::Activate: " << std::hex << hr;
+  uma_log_cb.Run(UmaLogStep::CREATE_CLIENT, hr);
+  return audio_client;
+}
+
 HRESULT GetPreferredAudioParametersInternal(IAudioClient* client,
                                             bool is_output_device,
                                             AudioParameters* params,
@@ -378,19 +398,25 @@ HRESULT GetPreferredAudioParametersInternal(IAudioClient* client,
     return hr;
 
   // Preferred sample rate.
-  int sample_rate = mix_format.Format.nSamplesPerSec;
+  const int sample_rate = mix_format.Format.nSamplesPerSec;
 
   int min_frames_per_buffer = 0;
   int max_frames_per_buffer = 0;
-  int frames_per_buffer;
+  int frames_per_buffer = 0;
+
+  const bool supports_iac3 = IAudioClient3IsSupported();
 
+  if (supports_iac3) {
+    // Try to obtain an IAudioClient3 interface from the IAudioClient object.
+    // Use ComPtr::As for doing QueryInterface calls on COM objects.
+    ComPtr<IAudioClient> audio_client(client);
     ComPtr<IAudioClient3> audio_client_3;
-  hr = client->QueryInterface(audio_client_3.GetAddressOf());
+    hr = audio_client.As(&audio_client_3);
     if (SUCCEEDED(hr)) {
-    UINT32 default_period_frames;
-    UINT32 fundamental_period_frames;
-    UINT32 min_period_frames;
-    UINT32 max_period_frames;
+      UINT32 default_period_frames = 0;
+      UINT32 fundamental_period_frames = 0;
+      UINT32 min_period_frames = 0;
+      UINT32 max_period_frames = 0;
       hr = audio_client_3->GetSharedModeEnginePeriod(
           &(mix_format.Format), &default_period_frames,
           &fundamental_period_frames, &min_period_frames, &max_period_frames);
@@ -401,11 +427,14 @@ HRESULT GetPreferredAudioParametersInternal(IAudioClient* client,
         max_frames_per_buffer = max_period_frames;
         frames_per_buffer = default_period_frames;
       }
+      DVLOG(1) << "IAudioClient3 => min_period_frames: " << min_period_frames;
+      DVLOG(1) << "IAudioClient3 => frames_per_buffer: " << frames_per_buffer;
+    }
   }
 
   // If we don't have access to IAudioClient3 or if the call to
   // GetSharedModeEnginePeriod() fails we fall back to GetDevicePeriod().
-  if (FAILED(hr)) {
+  if (!supports_iac3 || FAILED(hr)) {
     REFERENCE_TIME default_period = 0;
     hr = CoreAudioUtil::GetDevicePeriod(client, AUDCLNT_SHAREMODE_SHARED,
                                         &default_period);
@@ -416,11 +445,11 @@ HRESULT GetPreferredAudioParametersInternal(IAudioClient* client,
     // We are using the native device period to derive the smallest possible
     // buffer size in shared mode. Note that the actual endpoint buffer will be
     // larger than this size but it will be possible to fill it up in two calls.
-    // TODO(henrika): ensure that this scheme works for capturing as well.
     frames_per_buffer = static_cast<int>(
         sample_rate * CoreAudioUtil::ReferenceTimeToTimeDelta(default_period)
                           .InSecondsF() +
         0.5);
+    DVLOG(1) << "IAudioClient => frames_per_buffer: " << frames_per_buffer;
   }
 
   ChannelLayout channel_layout = GetChannelLayout(mix_format);
@@ -484,6 +513,19 @@ base::TimeDelta CoreAudioUtil::ReferenceTimeToTimeDelta(REFERENCE_TIME time) {
   return base::TimeDelta::FromMicroseconds(0.1 * time + 0.5);
 }
 
+uint32_t CoreAudioUtil::GetIAudioClientVersion() {
+  if (base::win::GetVersion() >= base::win::VERSION_WIN10) {
+    // Minimum supported client: Windows 10.
+    // Minimum supported server: Windows Server 2016
+    return 3;
+  } else if (base::win::GetVersion() >= base::win::VERSION_WIN8) {
+    // Minimum supported client: Windows 8.
+    // Minimum supported server: Windows Server 2012.
+    return 2;
+  }
+  return 1;
+}
+
 AUDCLNT_SHAREMODE CoreAudioUtil::GetShareMode() {
   const base::CommandLine* cmd_line = base::CommandLine::ForCurrentProcess();
   if (cmd_line->HasSwitch(switches::kEnableExclusiveAudio))
@@ -702,14 +744,20 @@ ComPtr<IAudioClient> CoreAudioUtil::CreateClient(const std::string& device_id,
                                                  EDataFlow data_flow,
                                                  ERole role) {
   ComPtr<IMMDevice> device(CreateDevice(device_id, data_flow, role));
-
   return CreateClientInternal(device.Get(),
                               base::BindRepeating(&LogUMAEmptyCb));
 }
 
+ComPtr<IAudioClient3> CoreAudioUtil::CreateClient3(const std::string& device_id,
+                                                   EDataFlow data_flow,
+                                                   ERole role) {
+  ComPtr<IMMDevice> device(CreateDevice(device_id, data_flow, role));
+  return CreateClientInternal3(device.Get(),
+                               base::BindRepeating(&LogUMAEmptyCb));
+}
+
 HRESULT CoreAudioUtil::GetSharedModeMixFormat(
     IAudioClient* client, WAVEFORMATPCMEX* format) {
-  VLOG(1) << __FUNCTION__;
   ScopedCoMem<WAVEFORMATPCMEX> format_pcmex;
   HRESULT hr = client->GetMixFormat(
       reinterpret_cast<WAVEFORMATEX**>(&format_pcmex));
@@ -814,7 +862,6 @@ HRESULT CoreAudioUtil::GetDevicePeriod(IAudioClient* client,
 HRESULT CoreAudioUtil::GetPreferredAudioParameters(const std::string& device_id,
                                                    bool is_output_device,
                                                    AudioParameters* params) {
-  DVLOG(1) << __FUNCTION__;
   UMALogCallback uma_log_cb(
       is_output_device ? base::BindRepeating(&LogUMAPreferredOutputParams)
                        : base::BindRepeating(&LogUMAEmptyCb));
@@ -883,16 +930,21 @@ HRESULT CoreAudioUtil::SharedModeInitialize(IAudioClient* client,
     stream_flags |= AUDCLNT_STREAMFLAGS_EVENTCALLBACK;
   DVLOG(2) << "stream_flags: 0x" << std::hex << stream_flags;
 
+  const bool supports_iac3 = IAudioClient3IsSupported();
+
   HRESULT hr;
-  if (requested_buffer_size > 0) {
+  if (supports_iac3 && requested_buffer_size > 0) {
+    // Try to obtain an IAudioClient3 interface from the IAudioClient object.
+    // Use ComPtr::As for doing QueryInterface calls on COM objects.
+    ComPtr<IAudioClient> audio_client(client);
     ComPtr<IAudioClient3> audio_client_3;
-    hr = client->QueryInterface(audio_client_3.GetAddressOf());
+    hr = audio_client.As(&audio_client_3);
     if (FAILED(hr)) {
-      DVLOG(1) << "Failed to QueryInterface on IAudioClient3 with explicit "
-                  "buffer size: "
-               << std::hex << hr;
+      DVLOG(1) << "Failed to obtain IAudioClient3 interface: " << std::hex
+               << hr;
       return hr;
     }
+    // Initialize a low-latency client using IAudioClient3.
     hr = audio_client_3->InitializeSharedAudioStream(
         stream_flags, requested_buffer_size, &(format->Format), session_guid);
     if (FAILED(hr)) {

media/audio/win/core_audio_util_win.h

@@ -47,6 +47,10 @@ class MEDIA_EXPORT CoreAudioUtil {
   // Example: double s = RefererenceTimeToTimeDelta(t).InMillisecondsF();
   static base::TimeDelta ReferenceTimeToTimeDelta(REFERENCE_TIME time);
 
+  // Returns 1, 2, or 3 corresponding to the highest version of IAudioClient
+  // the platform supports.
+  static uint32_t GetIAudioClientVersion();
+
   // Returns AUDCLNT_SHAREMODE_EXCLUSIVE if --enable-exclusive-mode is used
   // as command-line flag and AUDCLNT_SHAREMODE_SHARED otherwise (default).
   static AUDCLNT_SHAREMODE GetShareMode();
@@ -113,10 +117,12 @@ class MEDIA_EXPORT CoreAudioUtil {
   // manage the flow of audio data between the application and an audio endpoint
   // device.
 
-  // Create an IAudioClient instance for a specific device _or_ the default
+  // Create an IAudioClient instance for a specific device or the default
   // device if AudioDeviceDescription::IsDefaultDevice(device_id).
   static Microsoft::WRL::ComPtr<IAudioClient>
   CreateClient(const std::string& device_id, EDataFlow data_flow, ERole role);
+  static Microsoft::WRL::ComPtr<IAudioClient3>
+  CreateClient3(const std::string& device_id, EDataFlow data_flow, ERole role);
 
   // Get the mix format that the audio engine uses internally for processing
   // of shared-mode streams. This format is not necessarily a format that the
@@ -190,8 +196,6 @@ class MEDIA_EXPORT CoreAudioUtil {
                                       uint32_t* endpoint_buffer_size,
                                       const GUID* session_guid);
 
-  // TODO(henrika): add ExclusiveModeInitialize(...)
-
   // Create an IAudioRenderClient client for an existing IAudioClient given by
   // |client|. The IAudioRenderClient interface enables a client to write
   // output data to a rendering endpoint buffer.

media/audio/win/core_audio_util_win_unittest.cc

@@ -8,6 +8,7 @@
 #include <stdint.h>
 
 #include "base/macros.h"
+#include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/test/metrics/histogram_tester.h"
@@ -43,6 +44,12 @@ class CoreAudioUtilWinTest : public ::testing::Test {
   ScopedCOMInitializer com_init_;
 };
 
+TEST_F(CoreAudioUtilWinTest, GetIAudioClientVersion) {
+  uint32_t client_version = CoreAudioUtil::GetIAudioClientVersion();
+  EXPECT_GE(client_version, 1u);
+  EXPECT_LE(client_version, 3u);
+}
+
 TEST_F(CoreAudioUtilWinTest, NumberOfActiveDevices) {
   ABORT_AUDIO_TEST_IF_NOT(DevicesAvailable());
 
@@ -79,7 +86,7 @@ TEST_F(CoreAudioUtilWinTest, CreateDefaultDevice) {
 
   // Create default devices for all flow/role combinations above.
   ComPtr<IMMDevice> audio_device;
-  for (size_t i = 0; i < arraysize(data); ++i) {
+  for (size_t i = 0; i < base::size(data); ++i) {
     audio_device = CoreAudioUtil::CreateDevice(
         AudioDeviceDescription::kDefaultDeviceId, data[i].flow, data[i].role);
     EXPECT_TRUE(audio_device.Get());
@@ -132,7 +139,7 @@ TEST_F(CoreAudioUtilWinTest, GetDefaultDeviceName) {
   // Get name and ID of default devices for all flow/role combinations above.
   ComPtr<IMMDevice> audio_device;
   AudioDeviceName device_name;
-  for (size_t i = 0; i < arraysize(data); ++i) {
+  for (size_t i = 0; i < base::size(data); ++i) {
     audio_device = CoreAudioUtil::CreateDevice(
         AudioDeviceDescription::kDefaultDeviceId, data[i].flow, data[i].role);
     EXPECT_TRUE(SUCCEEDED(
@@ -152,7 +159,7 @@ TEST_F(CoreAudioUtilWinTest, GetAudioControllerID) {
   // Enumerate all active input and output devices and fetch the ID of
   // the associated device.
   EDataFlow flows[] = { eRender , eCapture };
-  for (size_t i = 0; i < arraysize(flows); ++i) {
+  for (size_t i = 0; i < base::size(flows); ++i) {
     ComPtr<IMMDeviceCollection> collection;
     ASSERT_TRUE(SUCCEEDED(enumerator->EnumAudioEndpoints(
         flows[i], DEVICE_STATE_ACTIVE, collection.GetAddressOf())));
@@ -199,10 +206,35 @@ TEST_F(CoreAudioUtilWinTest, CreateClient) {
 
   EDataFlow data[] = {eRender, eCapture};
 
-  for (size_t i = 0; i < arraysize(data); ++i) {
+  for (size_t i = 0; i < base::size(data); ++i) {
+    ComPtr<IAudioClient> client = CoreAudioUtil::CreateClient(
+        AudioDeviceDescription::kDefaultDeviceId, data[i], eConsole);
+    EXPECT_TRUE(client.Get());
+  }
+}
+
+TEST_F(CoreAudioUtilWinTest, CreateClient3) {
+  ABORT_AUDIO_TEST_IF_NOT(DevicesAvailable() &&
+                          CoreAudioUtil::GetIAudioClientVersion() >= 3);
+
+  EDataFlow data[] = {eRender, eCapture};
+
+  for (size_t i = 0; i < base::size(data); ++i) {
+    ComPtr<IAudioClient3> client3 = CoreAudioUtil::CreateClient3(
+        AudioDeviceDescription::kDefaultDeviceId, data[i], eConsole);
+    EXPECT_TRUE(client3.Get());
+  }
+
+  // Use ComPtr notation to achieve the same thing as above. ComPtr::As wraps
+  // QueryInterface calls on existing COM objects. In this case we use an
+  // existing IAudioClient to obtain the IAudioClient3 interface.
+  for (size_t i = 0; i < base::size(data); ++i) {
     ComPtr<IAudioClient> client = CoreAudioUtil::CreateClient(
         AudioDeviceDescription::kDefaultDeviceId, data[i], eConsole);
     EXPECT_TRUE(client.Get());
+    ComPtr<IAudioClient3> client3;
+    EXPECT_TRUE(SUCCEEDED(client.As(&client3)));
+    EXPECT_TRUE(client3.Get());
   }
 }
 
@@ -257,7 +289,7 @@ TEST_F(CoreAudioUtilWinTest, GetDevicePeriod) {
 
   // Verify that the device periods are valid for the default render and
   // capture devices.
-  for (size_t i = 0; i < arraysize(data); ++i) {
+  for (size_t i = 0; i < base::size(data); ++i) {
     ComPtr<IAudioClient> client;
     REFERENCE_TIME shared_time_period = 0;
     REFERENCE_TIME exclusive_time_period = 0;
@@ -281,7 +313,7 @@ TEST_F(CoreAudioUtilWinTest, GetPreferredAudioParameters) {
 
   // Verify that the preferred audio parameters are OK for the default render
   // and capture devices.
-  for (size_t i = 0; i < arraysize(data); ++i) {
+  for (size_t i = 0; i < base::size(data); ++i) {
     AudioParameters params;
     EXPECT_TRUE(SUCCEEDED(CoreAudioUtil::GetPreferredAudioParameters(
         AudioDeviceDescription::kDefaultDeviceId, data[i] == eRender,
@@ -365,7 +397,7 @@ TEST_F(CoreAudioUtilWinTest, CreateRenderAndCaptureClients) {
   WAVEFORMATPCMEX format;
   uint32_t endpoint_buffer_size = 0;
 
-  for (size_t i = 0; i < arraysize(data); ++i) {
+  for (size_t i = 0; i < base::size(data); ++i) {
     ComPtr<IAudioClient> client;
     ComPtr<IAudioRenderClient> render_client;
     ComPtr<IAudioCaptureClient> capture_client;