Re-land Add WebRTC logging to Windows audio input implementation.

Log failures in Open() and Start().

Previous (reverted) CL: https://chromium-review.googlesource.com/c/chromium/src/+/795727

Bug: 757737, 790557
Change-Id: Idecad5ba1a692da1386823d30ce65462c9a05ff3
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/795727
Commit-Queue: Henrik Grunell <grunell@chromium.org>
Reviewed-by: Max Morin <maxmorin@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#520562}
Reviewed-on: https://chromium-review.googlesource.com/803219
Cr-Commit-Position: refs/heads/master@{#520930}

media/audio/audio_manager.h

@@ -91,7 +91,7 @@ class MEDIA_EXPORT AudioManager {
 
   // Log callback used for sending log messages from a stream to the object
   // that manages the stream.
-  using LogCallback = base::Callback<void(const std::string&)>;
+  using LogCallback = base::RepeatingCallback<void(const std::string&)>;
 
   // Factory for all the supported stream formats. |params| defines parameters
   // of the audio stream to be created.

media/audio/win/audio_low_latency_input_win.cc

@@ -12,6 +12,7 @@
 
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/trace_event/trace_event.h"
 #include "media/audio/audio_device_description.h"
@@ -27,7 +28,9 @@
 using base::win::ScopedCOMInitializer;
 
 namespace media {
+
 namespace {
+
 bool IsSupportedFormatForConversion(const WAVEFORMATEX& format) {
   if (format.nSamplesPerSec < limits::kMinSampleRate ||
       format.nSamplesPerSec > limits::kMaxSampleRate) {
@@ -50,14 +53,18 @@ bool IsSupportedFormatForConversion(const WAVEFORMATEX& format) {
 
   return true;
 }
+
 }  // namespace
 
-WASAPIAudioInputStream::WASAPIAudioInputStream(AudioManagerWin* manager,
-                                               const AudioParameters& params,
-                                               const std::string& device_id)
-    : manager_(manager), device_id_(device_id) {
+WASAPIAudioInputStream::WASAPIAudioInputStream(
+    AudioManagerWin* manager,
+    const AudioParameters& params,
+    const std::string& device_id,
+    const AudioManager::LogCallback& log_callback)
+    : manager_(manager), device_id_(device_id), log_callback_(log_callback) {
   DCHECK(manager_);
   DCHECK(!device_id_.empty());
+  DCHECK(!log_callback_.is_null());
 
   // Load the Avrt DLL if not already loaded. Required to support MMCSS.
   bool avrt_init = avrt::Initialize();
@@ -102,15 +109,17 @@ bool WASAPIAudioInputStream::Open() {
   DCHECK_EQ(OPEN_RESULT_OK, open_result_);
 
   // Verify that we are not already opened.
-  if (opened_)
+  if (opened_) {
+    log_callback_.Run("WASAPIAIS::Open: already open");
     return false;
+  }
 
   // Obtain a reference to the IMMDevice interface of the capturing
   // device with the specified unique identifier or role which was
   // set at construction.
   HRESULT hr = SetCaptureDevice();
   if (FAILED(hr)) {
-    ReportOpenResult();
+    ReportOpenResult(hr);
     return false;
   }
 
@@ -120,7 +129,7 @@ bool WASAPIAudioInputStream::Open() {
                                   NULL, &audio_client_);
   if (FAILED(hr)) {
     open_result_ = OPEN_RESULT_ACTIVATION_FAILED;
-    ReportOpenResult();
+    ReportOpenResult(hr);
     return false;
   }
 
@@ -133,9 +142,10 @@ bool WASAPIAudioInputStream::Open() {
 
   // Verify that the selected audio endpoint supports the specified format
   // set during construction.
-  if (!DesiredFormatIsSupported()) {
+  hr = S_OK;
+  if (!DesiredFormatIsSupported(&hr)) {
     open_result_ = OPEN_RESULT_FORMAT_NOT_SUPPORTED;
-    ReportOpenResult();
+    ReportOpenResult(hr);
     return false;
   }
 
@@ -144,7 +154,7 @@ bool WASAPIAudioInputStream::Open() {
   hr = InitializeAudioEngine();
   if (SUCCEEDED(hr) && converter_)
     open_result_ = OPEN_RESULT_OK_WITH_RESAMPLING;
-  ReportOpenResult();  // Report before we assign a value to |opened_|.
+  ReportOpenResult(hr);  // Report before we assign a value to |opened_|.
   opened_ = SUCCEEDED(hr);
 
   return opened_;
@@ -191,10 +201,20 @@ void WASAPIAudioInputStream::Start(AudioInputCallback* callback) {
 
   // Start streaming data between the endpoint buffer and the audio engine.
   HRESULT hr = audio_client_->Start();
-  DLOG_IF(ERROR, FAILED(hr)) << "Failed to start input streaming.";
+  if (FAILED(hr)) {
+    DLOG(ERROR) << "Failed to start input streaming.";
+    log_callback_.Run(base::StringPrintf(
+        "WASAPIAIS::Start: Failed to start audio client, hresult = %#lx", hr));
+  }
 
-  if (SUCCEEDED(hr) && audio_render_client_for_loopback_.Get())
+  if (SUCCEEDED(hr) && audio_render_client_for_loopback_.Get()) {
     hr = audio_render_client_for_loopback_->Start();
+    if (FAILED(hr))
+      log_callback_.Run(base::StringPrintf(
+          "WASAPIAIS::Start: Failed to start render client for loopback, "
+          "hresult = %#lx",
+          hr));
+  }
 
   started_ = SUCCEEDED(hr);
 }
@@ -610,23 +630,24 @@ HRESULT WASAPIAudioInputStream::GetAudioEngineStreamFormat() {
   return hr;
 }
 
-bool WASAPIAudioInputStream::DesiredFormatIsSupported() {
+bool WASAPIAudioInputStream::DesiredFormatIsSupported(HRESULT* hr) {
   // An application that uses WASAPI to manage shared-mode streams can rely
   // on the audio engine to perform only limited format conversions. The audio
   // engine can convert between a standard PCM sample size used by the
   // application and the floating-point samples that the engine uses for its
   // internal processing. However, the format for an application stream
   // typically must have the same number of channels and the same sample
-  // rate as the stream format used byfCHANNEL_LAYOUT_UNSUPPORTED the device.
+  // rate as the stream format used by the device.
   // Many audio devices support both PCM and non-PCM stream formats. However,
   // the audio engine can mix only PCM streams.
   base::win::ScopedCoMem<WAVEFORMATEX> closest_match;
-  HRESULT hr = audio_client_->IsFormatSupported(AUDCLNT_SHAREMODE_SHARED,
-                                                &format_, &closest_match);
-  DLOG_IF(ERROR, hr == S_FALSE)
+  HRESULT hresult = audio_client_->IsFormatSupported(AUDCLNT_SHAREMODE_SHARED,
+                                                     &format_, &closest_match);
+  DLOG_IF(ERROR, hresult == S_FALSE)
       << "Format is not supported but a closest match exists.";
 
-  if (hr == S_FALSE && IsSupportedFormatForConversion(*closest_match.get())) {
+  if (hresult == S_FALSE &&
+      IsSupportedFormatForConversion(*closest_match.get())) {
     DVLOG(1) << "Audio capture data conversion needed.";
     // Ideally, we want a 1:1 ratio between the buffers we get and the buffers
     // we give to OnData so that each buffer we receive from the OS can be
@@ -681,10 +702,15 @@ bool WASAPIAudioInputStream::DesiredFormatIsSupported() {
         << "Audio capture data conversion: Need to inject fifo";
 
     // Indicate that we're good to go with a close match.
-    hr = S_OK;
+    hresult = S_OK;
   }
 
-  return (hr == S_OK);
+  // At this point, |hresult| == S_OK if the desired format is supported. If
+  // |hresult| == S_FALSE, the OS supports a closest match but we don't support
+  // conversion to it. Thus, SUCCEEDED() or FAILED() can't be used to determine
+  // if the desired format is supported.
+  *hr = hresult;
+  return (hresult == S_OK);
 }
 
 HRESULT WASAPIAudioInputStream::InitializeAudioEngine() {
@@ -820,10 +846,16 @@ HRESULT WASAPIAudioInputStream::InitializeAudioEngine() {
   return hr;
 }
 
-void WASAPIAudioInputStream::ReportOpenResult() const {
+void WASAPIAudioInputStream::ReportOpenResult(HRESULT hr) const {
   DCHECK(!opened_);  // This method must be called before we set this flag.
   UMA_HISTOGRAM_ENUMERATION("Media.Audio.Capture.Win.Open", open_result_,
                             OPEN_RESULT_MAX + 1);
+  if (open_result_ != OPEN_RESULT_OK &&
+      open_result_ != OPEN_RESULT_OK_WITH_RESAMPLING) {
+    log_callback_.Run(base::StringPrintf(
+        "WASAPIAIS::Open: failed, result = %d, hresult = %#lx", open_result_,
+        hr));
+  }
 }
 
 double WASAPIAudioInputStream::ProvideInput(AudioBus* audio_bus,

media/audio/win/audio_low_latency_input_win.h

@@ -75,6 +75,7 @@
 #include "base/win/scoped_com_initializer.h"
 #include "base/win/scoped_handle.h"
 #include "media/audio/agc_audio_stream.h"
+#include "media/audio/audio_manager.h"
 #include "media/base/audio_converter.h"
 #include "media/base/audio_parameters.h"
 #include "media/base/media_export.h"
@@ -95,7 +96,8 @@ class MEDIA_EXPORT WASAPIAudioInputStream
   // the audio manager who is creating this object.
   WASAPIAudioInputStream(AudioManagerWin* manager,
                          const AudioParameters& params,
-                         const std::string& device_id);
+                         const std::string& device_id,
+                         const AudioManager::LogCallback& log_callback);
 
   // The dtor is typically called by the AudioManager only and it is usually
   // triggered by calling AudioInputStream::Close().
@@ -123,9 +125,13 @@ class MEDIA_EXPORT WASAPIAudioInputStream
   // The Open() method is divided into these sub methods.
   HRESULT SetCaptureDevice();
   HRESULT GetAudioEngineStreamFormat();
-  bool DesiredFormatIsSupported();
+  // Returns whether the desired format is supported or not and writes the
+  // result of a failing system call to |*hr|, or S_OK if successful. If this
+  // function returns false with |*hr| == S_FALSE, the OS supports a closest
+  // match but we don't support conversion to it.
+  bool DesiredFormatIsSupported(HRESULT* hr);
   HRESULT InitializeAudioEngine();
-  void ReportOpenResult() const;
+  void ReportOpenResult(HRESULT hr) const;
 
   // AudioConverter::InputCallback implementation.
   double ProvideInput(AudioBus* audio_bus, uint32_t frames_delayed) override;
@@ -246,6 +252,9 @@ class MEDIA_EXPORT WASAPIAudioInputStream
   std::unique_ptr<AudioBus> convert_bus_;
   bool imperfect_buffer_size_conversion_ = false;
 
+  // Callback to send log messages.
+  AudioManager::LogCallback log_callback_;
+
   SEQUENCE_CHECKER(sequence_checker_);
 
   DISALLOW_COPY_AND_ASSIGN(WASAPIAudioInputStream);

media/audio/win/audio_low_latency_input_win_unittest.cc

@@ -11,6 +11,7 @@
 
 #include <memory>
 
+#include "base/bind.h"
 #include "base/environment.h"
 #include "base/files/file_util.h"
 #include "base/macros.h"
@@ -41,6 +42,12 @@ using ::testing::NotNull;
 
 namespace media {
 
+namespace {
+
+void LogCallbackDummy(const std::string& /* message */) {}
+
+}  // namespace
+
 ACTION_P4(CheckCountAndPostQuitTask, count, limit, task_runner, quit_closure) {
   if (++*count >= limit)
     task_runner->PostTask(FROM_HERE, quit_closure);
@@ -207,7 +214,7 @@ class AudioInputStreamWrapper {
     params.set_frames_per_buffer(frames_per_buffer_);
     AudioInputStream* ais = audio_man_->MakeAudioInputStream(
         params, AudioDeviceDescription::kDefaultDeviceId,
-        AudioManager::LogCallback());
+        base::BindRepeating(&LogCallbackDummy));
     EXPECT_TRUE(ais);
     return ais;
   }
@@ -453,7 +460,7 @@ TEST_F(WinAudioInputTest, WASAPIAudioInputStreamLoopback) {
 
   ScopedAudioInputStream stream(audio_manager_->MakeAudioInputStream(
       params, AudioDeviceDescription::kLoopbackInputDeviceId,
-      AudioManager::LogCallback()));
+      base::BindRepeating(&LogCallbackDummy)));
   ASSERT_TRUE(stream->Open());
   FakeAudioInputCallback sink;
   stream->Start(&sink);

media/audio/win/audio_manager_win.cc

@@ -249,7 +249,7 @@ AudioInputStream* AudioManagerWin::MakeLowLatencyInputStream(
     const LogCallback& log_callback) {
   // Used for both AUDIO_PCM_LOW_LATENCY and AUDIO_PCM_LINEAR.
   DVLOG(1) << "MakeLowLatencyInputStream: " << device_id;
-  return new WASAPIAudioInputStream(this, params, device_id);
+  return new WASAPIAudioInputStream(this, params, device_id, log_callback);
 }
 
 std::string AudioManagerWin::GetDefaultOutputDeviceID() {