Refactor AudioInputController and related interfaces.

media/audio/audio_input_controller.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/audio/audio_input_controller.h"
 
 #include <algorithm>
 #include <limits>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "base/trace_event/trace_event.h"
 #include "media/audio/audio_input_writer.h"
 #include "media/base/user_input_monitor.h"
 
+namespace media {
 namespace {
 
 const int kMaxInputChannels = 3;
 
 #if defined(AUDIO_POWER_MONITORING)
 // Time in seconds between two successive measurements of audio power levels.
 const int kPowerMonitorLogIntervalSeconds = 15;
 
 // A warning will be logged when the microphone audio volume is below this
 // threshold.
 const int kLowLevelMicrophoneLevelPercent = 10;
 
 // Logs if the user has enabled the microphone mute or not. This is normally
 // done by marking a checkbox in an audio-settings UI which is unique for each
 // platform. Elements in this enum should not be added, deleted or rearranged.
 enum MicrophoneMuteResult {
   MICROPHONE_IS_MUTED = 0,
   MICROPHONE_IS_NOT_MUTED = 1,
   MICROPHONE_MUTE_MAX = MICROPHONE_IS_NOT_MUTED
 };
 
 void LogMicrophoneMuteResult(MicrophoneMuteResult result) {
   UMA_HISTOGRAM_ENUMERATION("Media.MicrophoneMuted",
                             result,
                             MICROPHONE_MUTE_MAX + 1);
 }
 
 // Helper method which calculates the average power of an audio bus. Unit is in
 // dBFS, where 0 dBFS corresponds to all channels and samples equal to 1.0.
-float AveragePower(const media::AudioBus& buffer) {
+float AveragePower(const AudioBus& buffer) {
   const int frames = buffer.frames();
   const int channels = buffer.channels();
   if (frames <= 0 || channels <= 0)
     return 0.0f;
 
   // Scan all channels and accumulate the sum of squares for all samples.
   float sum_power = 0.0f;
   for (int ch = 0; ch < channels; ++ch) {
     const float* channel_data = buffer.channel(ch);
     for (int i = 0; i < frames; i++) {
@@ skipping 11 matching lines @@
   const float kInsignificantPower = 1.0e-10f;  // -100 dBFS
   const float power_dbfs = average_power < kInsignificantPower ?
       -std::numeric_limits<float>::infinity() : 10.0f * log10f(average_power);
 
   return power_dbfs;
 }
 #endif  // AUDIO_POWER_MONITORING
 
 }  // namespace
 
-namespace media {
+class AudioInputController::AudioCallback
+    : public AudioInputStream::AudioInputCallback {
+ public:
+  explicit AudioCallback(AudioInputController* controller)
+      : controller_(controller) {}
+  ~AudioCallback() override {}
+
+  bool received_callback() const { return received_callback_; }
+  bool error_during_callback() const { return error_during_callback_; }

[o1ka, 2017/01/12 12:07:38]

Similar data race (see AudioInputController comment) for these two members: they
are updated on callback thread; but we read them on audio thread in
AudioInputController::DoClose() right after we called Stop() on the stream (i.e.
while there may be in progress/pending callbacks).

[tommi (sloooow) - chröme, 2017/01/12 14:21:00]

Yes, that pattern is safe. It's just that the documentation for Stop() was
incorrect.

[o1ka, 2017/01/12 17:27:15]

Acknowledged.

+
+ private:
+  void OnData(AudioInputStream* stream,
+              const AudioBus* source,
+              uint32_t hardware_delay_bytes,
+              double volume) override {
+    TRACE_EVENT0("audio", "AC::OnData");
+
+    received_callback_ = true;
+
+    DeliverDataToSyncWriter(source, hardware_delay_bytes, volume);
+    PerformOptionalDebugRecording(source);
+  }
+
+  void OnError(AudioInputStream* stream) override {
+    error_during_callback_ = true;
+    controller_->task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&AudioInputController::DoReportError, controller_));
+  }
+
+  void PerformOptionalDebugRecording(const AudioBus* source) {
+    // Called on the audio callback thread while recording is enabled.

[o1ka, 2017/01/12 12:07:38]

In other places it's called "hw callback thread" (I like "audio" version more).

[tommi (sloooow) - chröme, 2017/01/12 14:21:00]

hmm... changed to "audio/hw callback thread" :-|

+    if (!controller_->debug_writer_ || !controller_->debug_writer_->WillWrite())
+      return;
+
+    // TODO(tommi): This is costly. AudioBus heap allocs and we create a new
+    // one for every callback. We could instead have a pool of bus objects
+    // that get returned to us somehow.
+    // We should also avoid calling PostTask here since the implementation
+    // of the debug writer will basically do a PostTask straight away anyway.
+    // Might require some modifications to AudioInputDebugWriter though.

[o1ka, 2017/01/12 12:07:38]

"Requires some refactoring since both
AudioInputController::WriteInputDataForDebugging() and
AudioInputDebugWriter::Write() are not thread-safe" - it's not immediately
obvious, and having a comment may prevent a quick dangerous change in the
future.

[tommi (sloooow) - chröme, 2017/01/12 14:21:00]

Comment updated. WriteInputDataForDebugging itself is safe as is, but with this
refactoring, it would be going away anyway.

+    // Btw, AudioInputDebugWriter is the only implementation of
+    // AudioInputWriter...  remove the interface?

[o1ka, 2017/01/12 12:07:38]

The interface is in media together with controller, and the impl is in content -
this seems to be the reason for having it.

[tommi (sloooow) - chröme, 2017/01/12 14:21:00]

ah, makes sense.  removed this part of the comment.

+    std::unique_ptr<AudioBus> source_copy =
+        AudioBus::Create(source->channels(), source->frames());
+    source->CopyTo(source_copy.get());
+    controller_->task_runner_->PostTask(
+        FROM_HERE, base::Bind(&AudioInputController::WriteInputDataForDebugging,
+                              controller_, base::Passed(&source_copy)));
+  }
+
+  void DeliverDataToSyncWriter(const AudioBus* source,
+                               uint32_t hardware_delay_bytes,
+                               double volume) {
+    bool key_pressed = controller_->CheckForKeyboardInput();
+    controller_->sync_writer_->Write(source, volume, key_pressed,
+                                     hardware_delay_bytes);
+    controller_->CheckAudioPower(source, volume);
+  }
+
+  AudioInputController* const controller_;
+  bool received_callback_ = false;
+  bool error_during_callback_ = false;
+};
 
 // static
 AudioInputController::Factory* AudioInputController::factory_ = nullptr;
 
 AudioInputController::AudioInputController(
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner,
     EventHandler* handler,
     SyncWriter* sync_writer,
     std::unique_ptr<AudioInputWriter> debug_writer,
     UserInputMonitor* user_input_monitor,
     const bool agc_is_enabled)
     : creator_task_runner_(base::ThreadTaskRunnerHandle::Get()),
+      task_runner_(std::move(task_runner)),
       handler_(handler),
       stream_(nullptr),
-      should_report_stats(0),
-      state_(CLOSED),
       sync_writer_(sync_writer),
       max_volume_(0.0),
       user_input_monitor_(user_input_monitor),
       agc_is_enabled_(agc_is_enabled),
 #if defined(AUDIO_POWER_MONITORING)
       power_measurement_is_enabled_(false),
       log_silence_state_(false),
       silence_state_(SILENCE_STATE_NO_MEASUREMENT),
 #endif
       prev_key_down_count_(0),
       debug_writer_(std::move(debug_writer)) {
   DCHECK(creator_task_runner_.get());
+  DCHECK(handler_);
+  DCHECK(sync_writer_);
 }
 
 AudioInputController::~AudioInputController() {
-  DCHECK_EQ(state_, CLOSED);
+  DCHECK(!audio_callback_);
+  DCHECK(!stream_);
 }
 
 // static
 scoped_refptr<AudioInputController> AudioInputController::Create(
     AudioManager* audio_manager,
     EventHandler* event_handler,
+    SyncWriter* sync_writer,
     const AudioParameters& params,
     const std::string& device_id,
     UserInputMonitor* user_input_monitor) {
   DCHECK(audio_manager);
+  DCHECK(event_handler);
+  DCHECK(sync_writer);
 
   if (!params.IsValid() || (params.channels() > kMaxInputChannels))
     return nullptr;
 
   if (factory_) {
-    return factory_->Create(audio_manager->GetTaskRunner(),
-                            /*sync_writer*/ nullptr, audio_manager,
-                            event_handler, params, user_input_monitor);
+    return factory_->Create(audio_manager->GetTaskRunner(), sync_writer,
+                            audio_manager, event_handler, params,
+                            user_input_monitor);
   }
 
   scoped_refptr<AudioInputController> controller(new AudioInputController(
-      event_handler, nullptr, nullptr, user_input_monitor, false));
-
-  controller->task_runner_ = audio_manager->GetTaskRunner();
+      audio_manager->GetTaskRunner(), event_handler, sync_writer, nullptr,
+      user_input_monitor, false));
 
   // Create and open a new audio input stream from the existing
   // audio-device thread.
   if (!controller->task_runner_->PostTask(
           FROM_HERE,
           base::Bind(&AudioInputController::DoCreate,
                      controller,
                      base::Unretained(audio_manager),
                      params,
                      device_id))) {
     controller = nullptr;
   }
 
   return controller;
 }
 
 // static
 scoped_refptr<AudioInputController> AudioInputController::CreateLowLatency(
     AudioManager* audio_manager,
     EventHandler* event_handler,
     const AudioParameters& params,
     const std::string& device_id,
     SyncWriter* sync_writer,
     std::unique_ptr<AudioInputWriter> debug_writer,
     UserInputMonitor* user_input_monitor,
     const bool agc_is_enabled) {
   DCHECK(audio_manager);
   DCHECK(sync_writer);
+  DCHECK(event_handler);
 
   if (!params.IsValid() || (params.channels() > kMaxInputChannels))
     return nullptr;
 
   if (factory_) {
     return factory_->Create(audio_manager->GetTaskRunner(), sync_writer,
                             audio_manager, event_handler, params,
                             user_input_monitor);
   }
 
   // Create the AudioInputController object and ensure that it runs on
   // the audio-manager thread.
   scoped_refptr<AudioInputController> controller(new AudioInputController(
-      event_handler, sync_writer, std::move(debug_writer), user_input_monitor,
-      agc_is_enabled));
-  controller->task_runner_ = audio_manager->GetTaskRunner();
+      audio_manager->GetTaskRunner(), event_handler, sync_writer,
+      std::move(debug_writer), user_input_monitor, agc_is_enabled));
 
   // Create and open a new audio input stream from the existing
   // audio-device thread. Use the provided audio-input device.
   if (!controller->task_runner_->PostTask(
           FROM_HERE,
           base::Bind(&AudioInputController::DoCreateForLowLatency,
                      controller,
                      base::Unretained(audio_manager),
                      params,
                      device_id))) {
     controller = nullptr;
   }
 
   return controller;
 }
 
 // static
 scoped_refptr<AudioInputController> AudioInputController::CreateForStream(
     const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
     EventHandler* event_handler,
     AudioInputStream* stream,
     SyncWriter* sync_writer,
     std::unique_ptr<AudioInputWriter> debug_writer,
     UserInputMonitor* user_input_monitor) {
   DCHECK(sync_writer);
   DCHECK(stream);
+  DCHECK(event_handler);
 
   if (factory_) {
     return factory_->Create(
         task_runner, sync_writer, AudioManager::Get(), event_handler,
-        media::AudioParameters::UnavailableDeviceParams(), user_input_monitor);
+        AudioParameters::UnavailableDeviceParams(), user_input_monitor);
   }
 
   // Create the AudioInputController object and ensure that it runs on
   // the audio-manager thread.
   scoped_refptr<AudioInputController> controller(new AudioInputController(
-      event_handler, sync_writer, std::move(debug_writer), user_input_monitor,
-      false));
-  controller->task_runner_ = task_runner;
+      task_runner, event_handler, sync_writer, std::move(debug_writer),
+      user_input_monitor, false));
 
   if (!controller->task_runner_->PostTask(
           FROM_HERE,
           base::Bind(&AudioInputController::DoCreateForStream,
                      controller,
                      stream))) {
     controller = nullptr;
   }
 
   return controller;
 }
 
 void AudioInputController::Record() {
+  DCHECK(creator_task_runner_->BelongsToCurrentThread());
   task_runner_->PostTask(FROM_HERE, base::Bind(
       &AudioInputController::DoRecord, this));
 }
 
 void AudioInputController::Close(const base::Closure& closed_task) {
   DCHECK(!closed_task.is_null());
   DCHECK(creator_task_runner_->BelongsToCurrentThread());
 
   task_runner_->PostTaskAndReply(
       FROM_HERE, base::Bind(&AudioInputController::DoClose, this), closed_task);
 }
 
 void AudioInputController::SetVolume(double volume) {
+  DCHECK(creator_task_runner_->BelongsToCurrentThread());
   task_runner_->PostTask(FROM_HERE, base::Bind(
       &AudioInputController::DoSetVolume, this, volume));
 }
 
 void AudioInputController::DoCreate(AudioManager* audio_manager,
                                     const AudioParameters& params,
                                     const std::string& device_id) {
   DCHECK(task_runner_->BelongsToCurrentThread());
+  DCHECK(!stream_);
   SCOPED_UMA_HISTOGRAM_TIMER("Media.AudioInputController.CreateTime");
-  if (handler_)
-    handler_->OnLog(this, "AIC::DoCreate");
+  handler_->OnLog(this, "AIC::DoCreate");
 
 #if defined(AUDIO_POWER_MONITORING)
   // Disable power monitoring for streams that run without AGC enabled to
   // avoid adding logs and UMA for non-WebRTC clients.
   power_measurement_is_enabled_ = agc_is_enabled_;
   last_audio_level_log_time_ = base::TimeTicks::Now();
   silence_state_ = SILENCE_STATE_NO_MEASUREMENT;
 #endif
 
+  // MakeAudioInputStream might fail and return nullptr. If so,
+  // DoCreateForStream will handle and report it.
   DoCreateForStream(audio_manager->MakeAudioInputStream(
       params, device_id, base::Bind(&AudioInputController::LogMessage, this)));
 }
 
 void AudioInputController::DoCreateForLowLatency(AudioManager* audio_manager,
                                                  const AudioParameters& params,
                                                  const std::string& device_id) {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
 #if defined(AUDIO_POWER_MONITORING)
   // We only log silence state UMA stats for low latency mode and if we use a
   // real device.
   if (params.format() != AudioParameters::AUDIO_FAKE)
     log_silence_state_ = true;
 #endif
 
   low_latency_create_time_ = base::TimeTicks::Now();
   DoCreate(audio_manager, params, device_id);
 }
 
 void AudioInputController::DoCreateForStream(
     AudioInputStream* stream_to_control) {
   DCHECK(task_runner_->BelongsToCurrentThread());
+  DCHECK(!stream_);
 
-  DCHECK(!stream_);
-  stream_ = stream_to_control;
-  should_report_stats = 1;
-
-  if (!stream_) {
-    if (handler_)
-      handler_->OnError(this, STREAM_CREATE_ERROR);
+  if (!stream_to_control) {
     LogCaptureStartupResult(CAPTURE_STARTUP_CREATE_STREAM_FAILED);
+    handler_->OnError(this, STREAM_CREATE_ERROR);
     return;
   }
 
-  if (stream_ && !stream_->Open()) {
-    stream_->Close();
-    stream_ = nullptr;
-    if (handler_)
-      handler_->OnError(this, STREAM_OPEN_ERROR);
+  if (!stream_to_control->Open()) {
+    stream_to_control->Close();
     LogCaptureStartupResult(CAPTURE_STARTUP_OPEN_STREAM_FAILED);
+    handler_->OnError(this, STREAM_OPEN_ERROR);
     return;
   }
 
   // Set AGC state using mode in |agc_is_enabled_| which can only be enabled in
   // CreateLowLatency().
 #if defined(AUDIO_POWER_MONITORING)
-  bool agc_is_supported = false;
-  agc_is_supported = stream_->SetAutomaticGainControl(agc_is_enabled_);
+  bool agc_is_supported =
+      stream_to_control->SetAutomaticGainControl(agc_is_enabled_);
   // Disable power measurements on platforms that does not support AGC at a
   // lower level. AGC can fail on platforms where we don't support the
   // functionality to modify the input volume slider. One such example is
   // Windows XP.
   power_measurement_is_enabled_ &= agc_is_supported;
 #else
-  stream_->SetAutomaticGainControl(agc_is_enabled_);
+  stream_to_control->SetAutomaticGainControl(agc_is_enabled_);
 #endif
 
-
-  state_ = CREATED;
-  if (handler_)
-    handler_->OnCreated(this);
-
   if (user_input_monitor_) {
     user_input_monitor_->EnableKeyPressMonitoring();
     prev_key_down_count_ = user_input_monitor_->GetKeyPressCount();
   }
+
+  // Finally, keep the stream pointer around, update the state and notify.
+  stream_ = stream_to_control;
+  handler_->OnCreated(this);
 }
 
 void AudioInputController::DoRecord() {
   DCHECK(task_runner_->BelongsToCurrentThread());
   SCOPED_UMA_HISTOGRAM_TIMER("Media.AudioInputController.RecordTime");
 
-  if (state_ != CREATED)
+  if (!stream_)
     return;
 
-  {
-    base::AutoLock auto_lock(lock_);
-    state_ = RECORDING;
-  }
+  DCHECK(!audio_callback_);

[Max Morin, 2017/01/12 09:31:10]

DoRecord might be called multiple times, so it should probably just return if
audio_callback_.

[tommi (sloooow) - chröme, 2017/01/12 14:21:00]

Thanks. Done. Do you know in what situation that happens?

[Max Morin, 2017/01/12 15:17:12]

It would only happen with a buggy/misbehaving renderer.

+  audio_callback_.reset(new AudioCallback(this));
 
-  if (handler_)
-    handler_->OnLog(this, "AIC::DoRecord");
+  handler_->OnLog(this, "AIC::DoRecord");
 
-  stream_->Start(this);
+  stream_->Start(audio_callback_.get());
 }
 
 void AudioInputController::DoClose() {
   DCHECK(task_runner_->BelongsToCurrentThread());
   SCOPED_UMA_HISTOGRAM_TIMER("Media.AudioInputController.CloseTime");
-  // If we have already logged something, this does nothing.
-  // Otherwise, we haven't recieved data.
-  LogCaptureStartupResult(CAPTURE_STARTUP_NEVER_GOT_DATA);
 
-  if (state_ == CLOSED)
+  if (!stream_)
     return;
 
-  // If this is a low-latency stream, log the total duration (since DoCreate)
-  // and add it to a UMA histogram.
-  if (!low_latency_create_time_.is_null()) {
-    base::TimeDelta duration =
-        base::TimeTicks::Now() - low_latency_create_time_;
-    UMA_HISTOGRAM_LONG_TIMES("Media.InputStreamDuration", duration);
-    if (handler_) {
-      std::string log_string =
-          base::StringPrintf("AIC::DoClose: stream duration=");
-      log_string += base::Int64ToString(duration.InSeconds());
-      log_string += " seconds";
-      handler_->OnLog(this, log_string);
+  // Allow calling unconditionally and bail if we don't have a stream to close.
+  if (audio_callback_) {
+    stream_->Stop();
+
+    if (!low_latency_create_time_.is_null()) {
+      LogCaptureStartupResult(audio_callback_->received_callback()
+                                  ? CAPTURE_STARTUP_OK
+                                  : CAPTURE_STARTUP_NEVER_GOT_DATA);
+      UMA_HISTOGRAM_BOOLEAN("Media.Audio.Capture.CallbackError",
+                            audio_callback_->error_during_callback());
+      if (audio_callback_->received_callback()) {
+        // Log the total duration (since DoCreate) and update the histogram.
+        base::TimeDelta duration =

[o1ka, 2017/01/12 12:07:38]

const?

[tommi (sloooow) - chröme, 2017/01/12 18:58:19]

Done. (forgot to ack in the patch set that I made it const)

+            base::TimeTicks::Now() - low_latency_create_time_;
+        UMA_HISTOGRAM_LONG_TIMES("Media.InputStreamDuration", duration);
+        std::string log_string =
+            base::StringPrintf("AIC::DoClose: stream duration=");

[Max Morin, 2017/01/12 09:31:10]

Unnecessary StringPrintf (or crate the entire log_string here).

[o1ka, 2017/01/12 12:07:38]

- then can be const as well.

[tommi (sloooow) - chröme, 2017/01/12 14:21:00]

ah, sorry I missed this somehow (this is moved code).  Made const and reduced
the string copying+conversion inefficiencies.

+        log_string += base::Int64ToString(duration.InSeconds()) + " seconds";
+        handler_->OnLog(this, log_string);
+      }
     }
+
+    audio_callback_.reset();
   }
 
-  DoStopCloseAndClearStream();
+  stream_->Close();
+  stream_ = nullptr;
 
-  if (SharedMemoryAndSyncSocketMode())
-    sync_writer_->Close();
+  sync_writer_->Close();
 
   if (user_input_monitor_)
     user_input_monitor_->DisableKeyPressMonitoring();
 
 #if defined(AUDIO_POWER_MONITORING)
   // Send UMA stats if enabled.
   if (log_silence_state_)
     LogSilenceState(silence_state_);
   log_silence_state_ = false;
 #endif
 
   if (debug_writer_)
     debug_writer_->Stop();
 
-  state_ = CLOSED;
+  max_volume_ = 0.0;
+  low_latency_create_time_ = base::TimeTicks();  // Reset to null.
 }
 
 void AudioInputController::DoReportError() {
   DCHECK(task_runner_->BelongsToCurrentThread());
-  if (handler_)
-    handler_->OnError(this, STREAM_ERROR);
+  handler_->OnError(this, STREAM_ERROR);
 }
 
 void AudioInputController::DoSetVolume(double volume) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK_GE(volume, 0);
   DCHECK_LE(volume, 1.0);
 
-  if (state_ != CREATED && state_ != RECORDING)
+  if (!stream_)
     return;
 
   // Only ask for the maximum volume at first call and use cached value
   // for remaining function calls.
   if (!max_volume_) {
     max_volume_ = stream_->GetMaxVolume();
   }
 
   if (max_volume_ == 0.0) {
     DLOG(WARNING) << "Failed to access input volume control";
     return;
   }
 
   // Set the stream volume and scale to a range matched to the platform.
   stream_->SetVolume(max_volume_ * volume);
 }
 
-void AudioInputController::OnData(AudioInputStream* stream,
-                                  const AudioBus* source,
-                                  uint32_t hardware_delay_bytes,
-                                  double volume) {
-  TRACE_EVENT0("audio", "AudioInputController::OnData");
-  if (debug_writer_ && debug_writer_->WillWrite()) {
-    std::unique_ptr<AudioBus> source_copy =
-        AudioBus::Create(source->channels(), source->frames());
-    source->CopyTo(source_copy.get());
-    task_runner_->PostTask(
-        FROM_HERE,
-        base::Bind(
-            &AudioInputController::WriteInputDataForDebugging,
-            this,
-            base::Passed(&source_copy)));
-  }
-  // Now we have data, so we know for sure that startup was ok.
-  LogCaptureStartupResult(CAPTURE_STARTUP_OK);
-
-  {
-    base::AutoLock auto_lock(lock_);
-    if (state_ != RECORDING)
-      return;
-  }
-
-  bool key_pressed = false;
-  if (user_input_monitor_) {
-    size_t current_count = user_input_monitor_->GetKeyPressCount();
-    key_pressed = current_count != prev_key_down_count_;
-    prev_key_down_count_ = current_count;
-    DVLOG_IF(6, key_pressed) << "Detected keypress.";
-  }
-
-  // Use SharedMemory and SyncSocket if the client has created a SyncWriter.
-  // Used by all low-latency clients except WebSpeech.
-  if (SharedMemoryAndSyncSocketMode()) {
-    sync_writer_->Write(source, volume, key_pressed, hardware_delay_bytes);
-
-#if defined(AUDIO_POWER_MONITORING)
-    // Only do power-level measurements if DoCreate() has been called. It will
-    // ensure that logging will mainly be done for WebRTC and WebSpeech
-    // clients.
-    if (!power_measurement_is_enabled_)
-      return;
-
-    // Perform periodic audio (power) level measurements.
-    if ((base::TimeTicks::Now() - last_audio_level_log_time_).InSeconds() >
-        kPowerMonitorLogIntervalSeconds) {
-      // Calculate the average power of the signal, or the energy per sample.
-      const float average_power_dbfs = AveragePower(*source);
-
-      // Add current microphone volume to log and UMA histogram.
-      const int mic_volume_percent = static_cast<int>(100.0 * volume);
-
-      // Use event handler on the audio thread to relay a message to the ARIH
-      // in content which does the actual logging on the IO thread.
-      task_runner_->PostTask(FROM_HERE,
-                             base::Bind(&AudioInputController::DoLogAudioLevels,
-                                        this,
-                                        average_power_dbfs,
-                                        mic_volume_percent));
-
-      last_audio_level_log_time_ = base::TimeTicks::Now();
-    }
-#endif
-    return;
-  }
-
-  // TODO(henrika): Investigate if we can avoid the extra copy here.
-  // (see http://crbug.com/249316 for details). AFAIK, this scope is only
-  // active for WebSpeech clients.
-  std::unique_ptr<AudioBus> audio_data =
-      AudioBus::Create(source->channels(), source->frames());
-  source->CopyTo(audio_data.get());
-
-  // Ownership of the audio buffer will be with the callback until it is run,
-  // when ownership is passed to the callback function.
-  task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(
-          &AudioInputController::DoOnData, this, base::Passed(&audio_data)));
-}
-
-void AudioInputController::DoOnData(std::unique_ptr<AudioBus> data) {
-  DCHECK(task_runner_->BelongsToCurrentThread());
-  if (handler_)
-    handler_->OnData(this, data.get());
-}
-
 void AudioInputController::DoLogAudioLevels(float level_dbfs,
                                             int microphone_volume_percent) {
 #if defined(AUDIO_POWER_MONITORING)
   DCHECK(task_runner_->BelongsToCurrentThread());
-  if (!handler_)
+  if (!stream_)
     return;
 
   // Detect if the user has enabled hardware mute by pressing the mute
   // button in audio settings for the selected microphone.
   const bool microphone_is_muted = stream_->IsMuted();
   if (microphone_is_muted) {
     LogMicrophoneMuteResult(MICROPHONE_IS_MUTED);
     handler_->OnLog(this, "AIC::OnData: microphone is muted!");
     // Return early if microphone is muted. No need to adding logs and UMA stats
     // of audio levels if we know that the micropone is muted.
@@ skipping 13 matching lines @@
 
   UMA_HISTOGRAM_PERCENTAGE("Media.MicrophoneVolume", microphone_volume_percent);
   log_string = base::StringPrintf(
       "AIC::OnData: microphone volume=%d%%", microphone_volume_percent);
   if (microphone_volume_percent < kLowLevelMicrophoneLevelPercent)
     log_string += " <=> low microphone level!";
   handler_->OnLog(this, log_string);
 #endif
 }
 
-void AudioInputController::OnError(AudioInputStream* stream) {
-  // Handle error on the audio-manager thread.
-  task_runner_->PostTask(FROM_HERE, base::Bind(
-      &AudioInputController::DoReportError, this));
-}
-
 void AudioInputController::EnableDebugRecording(
     const base::FilePath& file_name) {
+  DCHECK(creator_task_runner_->BelongsToCurrentThread());
   task_runner_->PostTask(
       FROM_HERE, base::Bind(&AudioInputController::DoEnableDebugRecording, this,
                             file_name));
 }
 
 void AudioInputController::DisableDebugRecording() {
   DCHECK(creator_task_runner_->BelongsToCurrentThread());
   task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&AudioInputController::DoDisableDebugRecording, this));
 }
 
-void AudioInputController::DoStopCloseAndClearStream() {
-  DCHECK(task_runner_->BelongsToCurrentThread());
-
-  // Allow calling unconditionally and bail if we don't have a stream to close.
-  if (stream_ != nullptr) {
-    stream_->Stop();
-    stream_->Close();
-    stream_ = nullptr;
-  }
-
-  // The event handler should not be touched after the stream has been closed.
-  handler_ = nullptr;
-}
-
 #if defined(AUDIO_POWER_MONITORING)
 void AudioInputController::UpdateSilenceState(bool silence) {
   if (silence) {
     if (silence_state_ == SILENCE_STATE_NO_MEASUREMENT) {
       silence_state_ = SILENCE_STATE_ONLY_SILENCE;
     } else if (silence_state_ == SILENCE_STATE_ONLY_AUDIO) {
       silence_state_ = SILENCE_STATE_AUDIO_AND_SILENCE;
     } else {
       DCHECK(silence_state_ == SILENCE_STATE_ONLY_SILENCE ||
              silence_state_ == SILENCE_STATE_AUDIO_AND_SILENCE);
@@ skipping 12 matching lines @@
 
 void AudioInputController::LogSilenceState(SilenceState value) {
   UMA_HISTOGRAM_ENUMERATION("Media.AudioInputControllerSessionSilenceReport",
                             value,
                             SILENCE_STATE_MAX + 1);
 }
 #endif
 
 void AudioInputController::LogCaptureStartupResult(
     CaptureStartupResult result) {
-  // Decrement shall_report_stats and check if it was 1 before decrement,
-  // which would imply that this is the first time this method is called
-  // after initialization. To avoid underflow, we
-  // also check if should_report_stats is one before decrementing.
-  if (base::AtomicRefCountIsOne(&should_report_stats) &&
-      !base::AtomicRefCountDec(&should_report_stats)) {
-    UMA_HISTOGRAM_ENUMERATION("Media.AudioInputControllerCaptureStartupSuccess",
-                              result, CAPTURE_STARTUP_RESULT_MAX + 1);
-  }
+  UMA_HISTOGRAM_ENUMERATION("Media.AudioInputControllerCaptureStartupSuccess",
+                            result, CAPTURE_STARTUP_RESULT_MAX + 1);
 }
 
 void AudioInputController::DoEnableDebugRecording(
     const base::FilePath& file_name) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   if (debug_writer_)
     debug_writer_->Start(file_name);
 }
 
 void AudioInputController::DoDisableDebugRecording() {
   DCHECK(task_runner_->BelongsToCurrentThread());
   if (debug_writer_)
     debug_writer_->Stop();
 }
 
 void AudioInputController::WriteInputDataForDebugging(
     std::unique_ptr<AudioBus> data) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   if (debug_writer_)
     debug_writer_->Write(std::move(data));
 }
 
 void AudioInputController::LogMessage(const std::string& message) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   handler_->OnLog(this, message);
 }
 
+bool AudioInputController::CheckForKeyboardInput() {
+  if (!user_input_monitor_)
+    return false;
+
+  size_t current_count = user_input_monitor_->GetKeyPressCount();
+  bool key_pressed = current_count != prev_key_down_count_;

[o1ka, 2017/01/12 12:07:38]

both const?

[tommi (sloooow) - chröme, 2017/01/12 14:21:00]

Done.

+  prev_key_down_count_ = current_count;
+  DVLOG_IF(6, key_pressed) << "Detected keypress.";
+
+  return key_pressed;
+}
+
+void AudioInputController::CheckAudioPower(const AudioBus* source,
+                                           double volume) {
+#if defined(AUDIO_POWER_MONITORING)
+  // Only do power-level measurements if DoCreate() has been called. It will
+  // ensure that logging will mainly be done for WebRTC and WebSpeech
+  // clients.
+  if (!power_measurement_is_enabled_)
+    return;
+
+  // Perform periodic audio (power) level measurements.
+  if ((base::TimeTicks::Now() - last_audio_level_log_time_).InSeconds() >
+      kPowerMonitorLogIntervalSeconds) {

[o1ka, 2017/01/12 12:07:38]

Can be early return instead.

[tommi (sloooow) - chröme, 2017/01/12 14:21:00]

Done.

+    // Calculate the average power of the signal, or the energy per sample.
+    const float average_power_dbfs = AveragePower(*source);
+
+    // Add current microphone volume to log and UMA histogram.
+    const int mic_volume_percent = static_cast<int>(100.0 * volume);

[o1ka, 2017/01/12 12:07:38]

Is "floor" operation intentional? We won't distinguish between 0.9% and 1% - not
sure if it matters though.

[tommi (sloooow) - chröme, 2017/01/12 14:21:00]

I think it's better that henrika@ explains the thinking. I'll add him to the
review.

[henrika (OOO until Aug 14), 2017/01/12 15:16:14]

Good comment. But it does not matter. The point of the stat was to be able to
look for "very low values" to see if the volume control level could be causing a
claimed low volume. 

Guess we could to static_cast<int>(100 * volume + 0.5) to be "more correct". 

[o1ka, 2017/01/12 17:27:15]

Acknowledged.

+
+    // Use event handler on the audio thread to relay a message to the ARIH
+    // in content which does the actual logging on the IO thread.
+    task_runner_->PostTask(
+        FROM_HERE, base::Bind(&AudioInputController::DoLogAudioLevels, this,
+                              average_power_dbfs, mic_volume_percent));
+
+    last_audio_level_log_time_ = base::TimeTicks::Now();

[o1ka, 2017/01/12 12:07:39]

This is a rather costly call, so I would make it once before l.637.

[tommi (sloooow) - chröme, 2017/01/12 14:21:01]

Agreed and likely more accurate too.

+  }
+#endif
+}
+
 }  // namespace media