Replace AudioSilenceDetector with an AudioPowerMonitor.

media/audio/audio_output_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_output_controller.h"
 
 #include "base/bind.h"
 #include "base/debug/trace_event.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/threading/platform_thread.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
-#include "media/audio/audio_silence_detector.h"
+#include "media/audio/audio_power_monitor.h"
 #include "media/audio/audio_util.h"
 #include "media/audio/shared_memory_util.h"
+#include "media/base/bind_to_loop.h"
 #include "media/base/scoped_histogram_timer.h"
 
 using base::Time;
 using base::TimeDelta;
 
 namespace media {
 
-// Amount of contiguous time where all audio is silent before considering the
-// stream to have transitioned and EventHandler::OnAudible() should be called.
-static const int kQuestionableSilencePeriodMillis = 50;
+// Time constant for AudioPowerMonitor.  See AudioPowerMonitor ctor comments for
+// semantics.  This value was arbitrarily chosen, but seems to work well.
+static const int kPowerMeasurementTimeConstantMillis = 10;
 
-// Sample value range below which audio is considered indistinguishably silent.
-//
-// TODO(miu): This value should be specified in dbFS units rather than full
-// scale.  See TODO in audio_silence_detector.h.
-static const float kIndistinguishableSilenceThreshold =
-    1.0f / 4096.0f;  // Note: This is approximately -72 dbFS.
+// Desired frequency of calls to EventHandler::OnPowerMeasured() for reporting
+// power levels in the audio signal.
+static const int kPowerMeasurementsPerSecond = 30;
 
 // Polling-related constants.
 const int AudioOutputController::kPollNumAttempts = 3;
 const int AudioOutputController::kPollPauseInMilliseconds = 3;
 
 AudioOutputController::AudioOutputController(AudioManager* audio_manager,
                                              EventHandler* handler,
                                              const AudioParameters& params,
                                              const std::string& input_device_id,
                                              SyncReader* sync_reader)
@@ skipping 44 matching lines @@
       &AudioOutputController::DoPlay, this));
 }
 
 void AudioOutputController::Pause() {
   message_loop_->PostTask(FROM_HERE, base::Bind(
       &AudioOutputController::DoPause, this));
 }
 
 void AudioOutputController::Close(const base::Closure& closed_task) {
   DCHECK(!closed_task.is_null());
-  message_loop_->PostTaskAndReply(FROM_HERE, base::Bind(
-      &AudioOutputController::DoClose, this), closed_task);
+  // See comment in DoClose(), which explains why this can't be a simple
+  // PostTaskAndReply() operation.
+  message_loop_->PostTask(FROM_HERE, base::Bind(
+      &AudioOutputController::DoClose, this, BindToCurrentLoop(closed_task)));
 }
 
 void AudioOutputController::SetVolume(double volume) {
   message_loop_->PostTask(FROM_HERE, base::Bind(
       &AudioOutputController::DoSetVolume, this, volume));
 }
 
 void AudioOutputController::DoCreate(bool is_for_device_change) {
   DCHECK(message_loop_->BelongsToCurrentThread());
   SCOPED_UMA_HISTOGRAM_TIMER("Media.AudioOutputController.CreateTime");
@@ skipping 41 matching lines @@
   SCOPED_UMA_HISTOGRAM_TIMER("Media.AudioOutputController.PlayTime");
 
   // We can start from created or paused state.
   if (state_ != kCreated && state_ != kPaused)
     return;
 
   // Ask for first packet.
   sync_reader_->UpdatePendingBytes(0);
 
   state_ = kPlaying;
-  silence_detector_.reset(new AudioSilenceDetector(
+
+  // Start monitoring power levels and send an initial notification that we're
+  // starting in silence.
+  handler_->OnPowerMeasured(AudioPowerMonitor::zero_power(), false);
+  power_monitor_.reset(new AudioPowerMonitor(
       params_.sample_rate(),
-      TimeDelta::FromMilliseconds(kQuestionableSilencePeriodMillis),
-      kIndistinguishableSilenceThreshold));
+      TimeDelta::FromMilliseconds(kPowerMeasurementTimeConstantMillis),
+      TimeDelta::FromSeconds(1) / kPowerMeasurementsPerSecond,
+      base::MessageLoop::current(),
+      base::Bind(&EventHandler::OnPowerMeasured, base::Unretained(handler_))));
 
   // We start the AudioOutputStream lazily.
   AllowEntryToOnMoreIOData();
   stream_->Start(this);
 
-  // Tell the event handler that we are now playing, and also start the silence
-  // detection notifications.
   handler_->OnPlaying();
-  silence_detector_->Start(
-      base::Bind(&EventHandler::OnAudible, base::Unretained(handler_)));
 }
 
 void AudioOutputController::StopStream() {
   DCHECK(message_loop_->BelongsToCurrentThread());
 
   if (state_ == kPlaying) {
     stream_->Stop();
     DisallowEntryToOnMoreIOData();
-    silence_detector_->Stop(true);
-    silence_detector_.reset();
+    power_monitor_.reset();
+    // Send a final notification that we're ending in silence.
+    handler_->OnPowerMeasured(AudioPowerMonitor::zero_power(), false);
     state_ = kPaused;
   }
 }
 
 void AudioOutputController::DoPause() {
   DCHECK(message_loop_->BelongsToCurrentThread());
   SCOPED_UMA_HISTOGRAM_TIMER("Media.AudioOutputController.PauseTime");
 
   StopStream();
 
   if (state_ != kPaused)
     return;
 
   // Send a special pause mark to the low-latency audio thread.
   sync_reader_->UpdatePendingBytes(kPauseMark);
 
   handler_->OnPaused();
 }
 
-void AudioOutputController::DoClose() {
+void AudioOutputController::DoClose(const base::Closure& done_callback) {
   DCHECK(message_loop_->BelongsToCurrentThread());
   SCOPED_UMA_HISTOGRAM_TIMER("Media.AudioOutputController.CloseTime");
 
   if (state_ != kClosed) {
     DoStopCloseAndClearStream();
     sync_reader_->Close();
     state_ = kClosed;
   }
+
+  // The AudioOutputController is now closed.  Until a playing stream was fully

[DaleCurtis, 2013/07/22 17:27:21]

Is it really necessary to send a zero during close?  Won't
AudioRenderHost::DeleteEntry() be called immediately after this?  Kind of stinks
to add an extra hop to every Close() call.

[miu, 2013/07/22 21:10:48]

You're right, this is unnecessary since the final call to MediaObserver will
notify the stream has stopped playing in AudioRendererHost::DeleteEntry(). 
However, it is still needed for DoPause(), to notify that things are silent. 
I've moved it there.

Eliminating the extra hop: Although this is only a "trampoline" within the
current thread (i.e., this is not a thread hop), it just occurred to me I can
make this all simpler by using a base::CancelableCallback.  WDYT (patch set 8)?

+  // stopped, however, there may have been additional tasks posted to
+  // |message_loop_| (e.g., by the AudioPowerMonitor).  Such tasks may attempt
+  // to invoke EventHandler methods.  Therefore, ensure that |done_callback| is
+  // run *after* those tasks to guarantee EventHandler remains alive for them.
+  message_loop_->PostTask(FROM_HERE, done_callback);
 }
 
 void AudioOutputController::DoSetVolume(double volume) {
   DCHECK(message_loop_->BelongsToCurrentThread());
 
   // Saves the volume to a member first. We may not be able to set the volume
   // right away but when the stream is created we'll set the volume.
   volume_ = volume;
 
   switch (state_) {
@@ skipping 40 matching lines @@
     const bool kShouldBlock = true;
 #else
     const bool kShouldBlock = diverting_to_stream_ != NULL;
 #endif
 
   const int frames = sync_reader_->Read(kShouldBlock, source, dest);
   DCHECK_LE(0, frames);
   sync_reader_->UpdatePendingBytes(
       buffers_state.total_bytes() + frames * params_.GetBytesPerFrame());
 
-  silence_detector_->Scan(dest, frames);
+  power_monitor_->Scan(*dest, frames);
 
   AllowEntryToOnMoreIOData();
   return frames;
 }
 
 void AudioOutputController::OnError(AudioOutputStream* stream) {
   // Handle error on the audio controller thread.
   message_loop_->PostTask(FROM_HERE, base::Bind(
       &AudioOutputController::DoReportError, this));
 }
@@ skipping 93 matching lines @@
   DCHECK(base::AtomicRefCountIsZero(&num_allowed_io_));
   base::AtomicRefCountInc(&num_allowed_io_);
 }
 
 void AudioOutputController::DisallowEntryToOnMoreIOData() {
   const bool is_zero = !base::AtomicRefCountDec(&num_allowed_io_);
   DCHECK(is_zero);
 }
 
 }  // namespace media