Misc improvements for loopack audio support on Windows.

Given recent work in https://bugs.chromium.org/p/chromium/issues/detail?id=938938
it was found that the code related to loopback support on Windows could contain
parts where it was not clear if the device role was taken into account properly.

This change is an attempt to clean up parts code where usage of a loopback device
might be broken. Basically, the change is an attempt to make the code more readable.

BUG=956526

Change-Id: Ia4f4ea177f3cd50805677790dbf0bdbd743561b7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1583708
Commit-Queue: Henrik Andreasson <henrika@chromium.org>
Reviewed-by: Henrik Grunell <grunell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#656375}

media/audio/win/audio_low_latency_input_win.cc

@@ -618,38 +618,40 @@ HRESULT WASAPIAudioInputStream::SetCaptureDevice() {
   // Retrieve the IMMDevice by using the specified role or the specified
   // unique endpoint device-identification string.
 
-  if (device_id_ == AudioDeviceDescription::kDefaultDeviceId) {
-    // Retrieve the default capture audio endpoint for the specified role.
-    // Note that, in Windows Vista, the MMDevice API supports device roles
-    // but the system-supplied user interface programs do not.
-    hr = enumerator->GetDefaultAudioEndpoint(eCapture, eConsole,
-                                             endpoint_device_.GetAddressOf());
-  } else if (device_id_ == AudioDeviceDescription::kCommunicationsDeviceId) {
-    hr = enumerator->GetDefaultAudioEndpoint(eCapture, eCommunications,
-                                             endpoint_device_.GetAddressOf());
-  } else if (device_id_ == AudioDeviceDescription::kLoopbackWithMuteDeviceId) {
-    // Capture the default playback stream.
-    hr = enumerator->GetDefaultAudioEndpoint(eRender, eConsole,
-                                             endpoint_device_.GetAddressOf());
-
-    if (SUCCEEDED(hr)) {
-      endpoint_device_->Activate(__uuidof(IAudioEndpointVolume), CLSCTX_ALL,
-                                 NULL, &system_audio_volume_);
-    }
-  } else if (device_id_ == AudioDeviceDescription::kLoopbackInputDeviceId) {
-    // Capture the default playback stream.
-    hr = enumerator->GetDefaultAudioEndpoint(eRender, eConsole,
-                                             endpoint_device_.GetAddressOf());
+  // To open a stream in loopback mode, the client must obtain an IMMDevice
+  // interface for the rendering endpoint device. Make that happen if needed;
+  // otherwise use default capture data-flow direction.
+  const EDataFlow data_flow =
+      AudioDeviceDescription::IsLoopbackDevice(device_id_) ? eRender : eCapture;
+  // Determine selected role to be used if the device is a default device.
+  const ERole role = AudioDeviceDescription::IsCommunicationsDevice(device_id_)
+                         ? eCommunications
+                         : eConsole;
+  if (AudioDeviceDescription::IsDefaultDevice(device_id_) ||
+      AudioDeviceDescription::IsCommunicationsDevice(device_id_) ||
+      AudioDeviceDescription::IsLoopbackDevice(device_id_)) {
+    hr =
+        enumerator->GetDefaultAudioEndpoint(data_flow, role, &endpoint_device_);
   } else {
     hr = enumerator->GetDevice(base::UTF8ToUTF16(device_id_).c_str(),
                                endpoint_device_.GetAddressOf());
   }
-
   if (FAILED(hr)) {
     open_result_ = OPEN_RESULT_NO_ENDPOINT;
     return hr;
   }
 
+  // If loopback device with muted system audio is requested, get the volume
+  // interface for the endpoint.
+  if (device_id_ == AudioDeviceDescription::kLoopbackWithMuteDeviceId) {
+    hr = endpoint_device_->Activate(__uuidof(IAudioEndpointVolume), CLSCTX_ALL,
+                                    nullptr, &system_audio_volume_);
+    if (FAILED(hr)) {
+      open_result_ = OPEN_RESULT_ACTIVATION_FAILED;
+      return hr;
+    }
+  }
+
   // Verify that the audio endpoint device is active, i.e., the audio
   // adapter that connects to the endpoint device is present and enabled.
   DWORD state = DEVICE_STATE_DISABLED;
@@ -807,7 +809,7 @@ HRESULT WASAPIAudioInputStream::InitializeAudioEngine() {
       100 * 1000 * 10,  // Buffer duration, 100 ms expressed in 100-ns units.
       0,                // Device period, n/a for shared mode.
       reinterpret_cast<const WAVEFORMATEX*>(&input_format_),
-      device_id_ == AudioDeviceDescription::kCommunicationsDeviceId
+      AudioDeviceDescription::IsCommunicationsDevice(device_id_)
           ? &kCommunicationsSessionId
           : nullptr);
 
@@ -887,7 +889,10 @@ HRESULT WASAPIAudioInputStream::InitializeAudioEngine() {
     hr = audio_render_client_for_loopback_->Initialize(
         AUDCLNT_SHAREMODE_SHARED,
         AUDCLNT_STREAMFLAGS_EVENTCALLBACK | AUDCLNT_STREAMFLAGS_NOPERSIST, 0, 0,
-        reinterpret_cast<const WAVEFORMATEX*>(&input_format_), NULL);
+        reinterpret_cast<const WAVEFORMATEX*>(&input_format_),
+        AudioDeviceDescription::IsCommunicationsDevice(device_id_)
+            ? &kCommunicationsSessionId
+            : nullptr);
     if (FAILED(hr)) {
       open_result_ = OPEN_RESULT_LOOPBACK_INIT_FAILED;
       return hr;

media/audio/win/core_audio_util_win.cc

@@ -443,6 +443,27 @@ ComPtr<IMMDevice> CreateDeviceInternal(const std::string& device_id,
                                        ERole role,
                                        const UMALogCallback& uma_log_cb) {
   ComPtr<IMMDevice> endpoint_device;
+  // In loopback mode, a client of WASAPI can capture the audio stream that
+  // is being played by a rendering endpoint device.
+  // See https://crbug.com/956526 for why we use both a DCHECK and then deal
+  // with the error here and below.
+  DCHECK(!(AudioDeviceDescription::IsLoopbackDevice(device_id) &&
+           data_flow != eCapture));
+  if (AudioDeviceDescription::IsLoopbackDevice(device_id) &&
+      data_flow != eCapture) {
+    LOG(WARNING) << "Loopback device must be an input device";
+    return endpoint_device;
+  }
+
+  // Usage of AudioDeviceDescription::kCommunicationsDeviceId as |device_id|
+  // is not allowed. Instead, set |device_id| to kDefaultDeviceId and select
+  // between default device and default communication device by using different
+  // |role| values (eConsole or eCommunications).
+  DCHECK(!AudioDeviceDescription::IsCommunicationsDevice(device_id));
+  if (AudioDeviceDescription::IsCommunicationsDevice(device_id)) {
+    LOG(WARNING) << "Invalid device identifier";
+    return endpoint_device;
+  }
 
   // Create the IMMDeviceEnumerator interface.
   ComPtr<IMMDeviceEnumerator> device_enum(
@@ -455,9 +476,8 @@ ComPtr<IMMDevice> CreateDeviceInternal(const std::string& device_id,
     hr =
         device_enum->GetDefaultAudioEndpoint(data_flow, role, &endpoint_device);
   } else if (AudioDeviceDescription::IsLoopbackDevice(device_id)) {
-    // Obtain an IMMDevice interface for the rendering endpoint device since
-    // loopback mode is selected.
-    // TODO(http://crbug/956526): clean up code related to loopback mode.
+    // To open a stream in loopback mode, the client must obtain an IMMDevice
+    // interface for the *rendering* endpoint device.
     hr = device_enum->GetDefaultAudioEndpoint(eRender, role, &endpoint_device);
   } else {
     hr = device_enum->GetDevice(base::UTF8ToUTF16(device_id).c_str(),
@@ -1070,6 +1090,15 @@ HRESULT CoreAudioUtil::GetPreferredAudioParameters(const std::string& device_id,
   UMALogCallback uma_log_cb(
       is_output_device ? base::BindRepeating(&LogUMAPreferredOutputParams)
                        : base::BindRepeating(&LogUMAEmptyCb));
+
+  // Loopback audio streams must be input streams.
+  DCHECK(!(AudioDeviceDescription::IsLoopbackDevice(device_id) &&
+           is_output_device));
+  if (AudioDeviceDescription::IsLoopbackDevice(device_id) && is_output_device) {
+    LOG(WARNING) << "Loopback device must be an input device";
+    return E_FAIL;
+  }
+
   ComPtr<IMMDevice> device(
       CreateDeviceByID(device_id, is_output_device, uma_log_cb));
   if (!device.Get())
@@ -1103,7 +1132,10 @@ HRESULT CoreAudioUtil::GetPreferredAudioParameters(const std::string& device_id,
 
 ChannelConfig CoreAudioUtil::GetChannelConfig(const std::string& device_id,
                                               EDataFlow data_flow) {
-  ComPtr<IAudioClient> client(CreateClient(device_id, data_flow, eConsole));
+  const ERole role = AudioDeviceDescription::IsCommunicationsDevice(device_id)
+                         ? eCommunications
+                         : eConsole;
+  ComPtr<IAudioClient> client(CreateClient(device_id, data_flow, role));
 
   WAVEFORMATEXTENSIBLE mix_format;
   if (!client.Get() ||

media/audio/win/core_audio_util_win.h

@@ -205,8 +205,6 @@ class MEDIA_EXPORT CoreAudioUtil {
   // speaker, and so on, continuing in the order defined in KsMedia.h.
   // See http://msdn.microsoft.com/en-us/library/windows/hardware/ff537083(v=vs.85).aspx
   // for more details.
-  // To get the channel config of the default device, pass an empty string
-  // for |device_id|.
   static ChannelConfig GetChannelConfig(const std::string& device_id,
                                         EDataFlow data_flow);
 

media/audio/win/core_audio_util_win_unittest.cc

@@ -131,6 +131,18 @@ TEST_F(CoreAudioUtilWinTest, CreateDeviceEnumerator) {
   EXPECT_TRUE(enumerator.Get());
 }
 
+TEST_F(CoreAudioUtilWinTest, GetDefaultDeviceIDs) {
+  ABORT_AUDIO_TEST_IF_NOT(DevicesAvailable());
+  std::string default_device_id = CoreAudioUtil::GetDefaultInputDeviceID();
+  EXPECT_FALSE(default_device_id.empty());
+  default_device_id = CoreAudioUtil::GetDefaultOutputDeviceID();
+  EXPECT_FALSE(default_device_id.empty());
+  default_device_id = CoreAudioUtil::GetCommunicationsInputDeviceID();
+  EXPECT_FALSE(default_device_id.empty());
+  default_device_id = CoreAudioUtil::GetCommunicationsOutputDeviceID();
+  EXPECT_FALSE(default_device_id.empty());
+}
+
 TEST_F(CoreAudioUtilWinTest, CreateDefaultDevice) {
   ABORT_AUDIO_TEST_IF_NOT(DevicesAvailable());
 
@@ -171,7 +183,7 @@ TEST_F(CoreAudioUtilWinTest, CreateDevice) {
   EXPECT_TRUE(SUCCEEDED(CoreAudioUtil::GetDeviceName(
       default_render_device.Get(), &default_render_name)));
 
-  // Use the uniqe ID as input to CreateDevice() and create a corresponding
+  // Use the unique ID as input to CreateDevice() and create a corresponding
   // IMMDevice.
   ComPtr<IMMDevice> audio_device = CoreAudioUtil::CreateDevice(
       default_render_name.unique_id, EDataFlow(), ERole());
@@ -383,10 +395,17 @@ TEST_F(CoreAudioUtilWinTest, GetPreferredAudioParameters) {
   // and capture devices.
   for (size_t i = 0; i < base::size(data); ++i) {
     AudioParameters params;
+    const bool is_output_device = (data[i] == eRender);
     EXPECT_TRUE(SUCCEEDED(CoreAudioUtil::GetPreferredAudioParameters(
-        AudioDeviceDescription::kDefaultDeviceId, data[i] == eRender,
-        &params)));
+        AudioDeviceDescription::kDefaultDeviceId, is_output_device, &params)));
     EXPECT_TRUE(params.IsValid());
+    if (!is_output_device) {
+      // Loopack devices are supported for input streams.
+      EXPECT_TRUE(SUCCEEDED(CoreAudioUtil::GetPreferredAudioParameters(
+          AudioDeviceDescription::kLoopbackInputDeviceId, is_output_device,
+          &params)));
+      EXPECT_TRUE(params.IsValid());
+    }
   }
 }
 
@@ -396,10 +415,22 @@ TEST_F(CoreAudioUtilWinTest, GetChannelConfig) {
   EDataFlow data_flows[] = {eRender, eCapture};
 
   for (auto data_flow : data_flows) {
-    ChannelConfig config =
+    ChannelConfig config1 =
         CoreAudioUtil::GetChannelConfig(std::string(), data_flow);
-    EXPECT_NE(config, CHANNEL_LAYOUT_NONE);
-    EXPECT_NE(config, CHANNEL_LAYOUT_UNSUPPORTED);
+    EXPECT_NE(config1, CHANNEL_LAYOUT_NONE);
+    EXPECT_NE(config1, CHANNEL_LAYOUT_UNSUPPORTED);
+    ChannelConfig config2 = CoreAudioUtil::GetChannelConfig(
+        AudioDeviceDescription::kDefaultDeviceId, data_flow);
+    EXPECT_EQ(config1, config2);
+    // For loopback input devices, verify that the channel configuration is
+    // same as for the default output device.
+    if (data_flow == eCapture) {
+      config1 = CoreAudioUtil::GetChannelConfig(
+          AudioDeviceDescription::kLoopbackInputDeviceId, data_flow);
+      config2 = CoreAudioUtil::GetChannelConfig(
+          AudioDeviceDescription::kDefaultDeviceId, eRender);
+      EXPECT_EQ(config1, config2);
+    }
   }
 }
 
@@ -583,13 +614,6 @@ TEST_F(CoreAudioUtilWinTest, GetMatchingOutputDeviceID) {
   EXPECT_TRUE(found_a_pair);
 }
 
-TEST_F(CoreAudioUtilWinTest, GetDefaultOutputDeviceID) {
-  ABORT_AUDIO_TEST_IF_NOT(DevicesAvailable());
-
-  std::string default_device_id(CoreAudioUtil::GetDefaultOutputDeviceID());
-  EXPECT_FALSE(default_device_id.empty());
-}
-
 TEST_F(CoreAudioUtilWinTest, CheckGetPreferredAudioParametersUMAStats) {
   base::HistogramTester tester;
   ABORT_AUDIO_TEST_IF_NOT(DevicesAvailable());