Added volume adjust sound behind the flag.

media/audio/sounds/audio_stream_handler.cc

Index: media/audio/sounds/audio_stream_handler.cc
diff --git a/media/audio/sounds/audio_stream_handler.cc b/media/audio/sounds/audio_stream_handler.cc
index 08608ac4187aa0863b777e5e42701f2b7a327f69..08dbdcd89162e4c76add16b95e14b1c951ac2878 100644
--- a/media/audio/sounds/audio_stream_handler.cc
+++ b/media/audio/sounds/audio_stream_handler.cc
@@ -6,8 +6,10 @@
 
 #include <string>
 
+#include "base/cancelable_callback.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop_proxy.h"
+#include "base/time/time.h"
 #include "media/audio/audio_manager.h"
 #include "media/audio/audio_manager_base.h"
 #include "media/base/channel_layout.h"
@@ -19,8 +21,8 @@ namespace {
 // Volume percent.
 const double kOutputVolumePercent = 0.8;
 
-// The number of frames each OnMoreData() call will request.
-const int kDefaultFrameCount = 1024;
+// Delay between end of sound and call to AudioOutputStream::Stop().
+const int kStopStreamDelaySec = 3;
 
 AudioStreamHandler::TestObserver* g_observer_for_testing = NULL;
 AudioOutputStream::AudioSourceCallback* g_audio_source_for_testing = NULL;
@@ -30,36 +32,52 @@ AudioOutputStream::AudioSourceCallback* g_audio_source_for_testing = NULL;
 class AudioStreamHandler::AudioStreamContainer
     : public AudioOutputStream::AudioSourceCallback {
  public:
-  AudioStreamContainer(const WavAudioHandler& wav_audio,
-                       const AudioParameters& params)
+  AudioStreamContainer(const WavAudioHandler& wav_audio)
       : stream_(NULL),
         wav_audio_(wav_audio),
-        params_(params),
-        cursor_(0) {
-  }
+        cursor_(0),
+        replay_scheduled_(false),

[DaleCurtis, 2013/12/18 21:20:21]

I think instead of all this replay_scheduled_ and schedule_replay_for_testing
stuff, you should just use a base::OneShotTimer to stop the stream after a
couple seconds. A lot of this code gets much simpler then:
- Add a lock around cursor_.
- Rename is_idle_ to is_stream_stopped_.
- Add timer which calls StopStream() (which sets is_stream_stopped_).
- During Play(), run Timer::Stop() if !is_stream_stopped_. No need to check
replay bits.
- During OnMoreData(), if AtEnd(), reset the timer which will start countdown
until Stopping.

Timer's are self canceling, so you don't need any CancellableClosure, or WeakPtr
to clean it up upon destruction either.

[ygorshenin1, 2013/12/19 15:42:29]

* added a lock around internal state;
* is_stream_idle_ is replaced by is_stream_stopped_;

Not sure that I understood your idea about OneShotTimer. We need a replay bit,
because otherwise if frequency of notifications from keyboard (about 40ms on
Lumpy for pressed volume up key) is comparable with duration of a sound (48ms
for volume adjust sound), there will be undesired sound effects on each Play()
request, like overlapping of sounds or gaps between sounds.

[DaleCurtis, 2013/12/19 19:44:08]

I'm only suggesting the timer is used to Stop() the stream.  Are you concerned
that setting cursor_ = 0 during each Play() request will chop off the current
playing sound if the user is pressing keys too quickly?

Where are you getting the 40ms number from?  Someone would have to hit the
volume key 25 times per second to reach your 40ms number (1000/25).  I'm a
pretty fast typist and was only able to manage 5 presses in a second :)

[ygorshenin1, 2013/12/20 09:05:05]

You don't need to press a key 5 times per second, you just need to keep the key
pressed for a while. In the last case on my Lumpy notifications come each 40ms.
So, without such replay logic if user'll just keep the volume up key pressed for
a while, volume adjust sound will be pretty harsh.

On 2013/12/19 19:44:08, DaleCurtis (OOO Dec 19-Jan2) wrote:

+        is_stream_idle_(true),
+        schedule_replay_request_for_testing_(0) {}
 
   virtual ~AudioStreamContainer() {
     DCHECK(AudioManager::Get()->GetMessageLoop()->BelongsToCurrentThread());
+    stop_stream_closure_.Cancel();
+    StopAndCloseStream();
   }
 
   void Play() {
     DCHECK(AudioManager::Get()->GetMessageLoop()->BelongsToCurrentThread());
 
     if (!stream_) {
-      stream_ = AudioManager::Get()->MakeAudioOutputStreamProxy(params_,
-                                                                std::string(),
-                                                                std::string());
+      stream_ = AudioManager::Get()->MakeAudioOutputStreamProxy(
+          wav_audio_.params(), std::string(), std::string());
       if (!stream_ || !stream_->Open()) {
         LOG(ERROR) << "Failed to open an output stream.";
         return;
       }
+      cursor_ = 0;
       stream_->SetVolume(kOutputVolumePercent);
+    } else if (is_stream_idle_) {
+      // If stream exists and idle, just stop the stream and start again.
+      cursor_ = 0;
+      StopStream();
     } else {
-      // TODO (ygorshenin@): implement smart stream rewind.
-      stream_->Stop();
+      // Stream exists and not idle, so let the current sound to
+      // finish playing.  If less than |wav_audio_.duration()| / 2
+      // until the end (including all replay requests), then replay is
+      // sheduled.  The main purpose of this strategy is to smooth
+      // sound reproduction when multiple requests are made, for
+      // instance, when user presses volume(up|down) button for a long
+      // time.
+      if (ReplayRequestCouldBeScheduled())
+        replay_scheduled_ = true;
+      return;
     }
 
-    cursor_ = 0;
+    last_play_start_time_ = base::TimeTicks::Now();
+    replay_scheduled_ = false;
+    is_stream_idle_ = false;
     if (g_audio_source_for_testing)
       stream_->Start(g_audio_source_for_testing);
     else
@@ -71,27 +89,41 @@ class AudioStreamHandler::AudioStreamContainer
 
   void Stop() {
     DCHECK(AudioManager::Get()->GetMessageLoop()->BelongsToCurrentThread());
-    if (!stream_)
-      return;
-    stream_->Stop();
-    stream_->Close();
-    stream_ = NULL;
-
-    if (g_observer_for_testing)
-      g_observer_for_testing->OnStop(cursor_);
+    StopStream();
   }
 
  private:
+  friend class AudioStreamHandler;
+
   // AudioOutputStream::AudioSourceCallback overrides:
   // Following methods could be called from *ANY* thread.
   virtual int OnMoreData(AudioBus* dest,
                          AudioBuffersState /* state */) OVERRIDE {
+    if (is_stream_idle_)
+      return 0;
+
     size_t bytes_written = 0;
     if (wav_audio_.AtEnd(cursor_) ||
         !wav_audio_.CopyTo(dest, cursor_, &bytes_written)) {
-      AudioManager::Get()->GetMessageLoop()->PostTask(
-          FROM_HERE,
-          base::Bind(&AudioStreamContainer::Stop, base::Unretained(this)));
+      // If there are replay requests, |cursor_| is reset and will
+      // continue to play from the start. Otherwise, stream will be
+      // stopped (not closed!) in |kStopStreamDelaySec| seconds,
+      // unless new Play() requests arrive during this time period.
+      if (replay_scheduled_) {
+        cursor_ = 0;
+        last_play_start_time_ = base::TimeTicks::Now();
+        replay_scheduled_ = false;
+        if (g_observer_for_testing)
+          g_observer_for_testing->OnReplay();
+      } else {
+        is_stream_idle_ = true;
+        stop_stream_closure_.Reset(base::Bind(&AudioStreamContainer::StopStream,
+                                              base::Unretained(this)));
+        AudioManager::Get()->GetMessageLoop()->PostDelayedTask(
+            FROM_HERE,
+            stop_stream_closure_.callback(),
+            base::TimeDelta::FromSeconds(kStopStreamDelaySec));
+      }
       return 0;
     }
     cursor_ += bytes_written;
@@ -109,13 +141,54 @@ class AudioStreamHandler::AudioStreamContainer
     LOG(ERROR) << "Error during system sound reproduction.";
   }
 
+  void StopStream() {
+    stop_stream_closure_.Cancel();
+    replay_scheduled_ = false;
+    if (stream_)
+      stream_->Stop();
+    if (g_observer_for_testing)
+      g_observer_for_testing->OnStop(cursor_);
+  }
+
+  void StopAndCloseStream() {
+    StopStream();
+    if (stream_) {
+      stream_->Close();
+      stream_ = NULL;
+    }
+  }
+
+  bool ReplayRequestCouldBeScheduled() {
+    if (schedule_replay_request_for_testing_) {
+      --schedule_replay_request_for_testing_;
+      return true;
+    }
+    const base::TimeDelta delta =
+        base::TimeTicks::Now() - last_play_start_time_;
+    return 2 * delta > wav_audio_.duration() && !replay_scheduled_;
+  }
+
+  void AllowReplayOnceForTesting() {
+    ++schedule_replay_request_for_testing_;
+  }
+
   AudioOutputStream* stream_;
 
-  const WavAudioHandler wav_audio_;
-  const AudioParameters params_;
+  WavAudioHandler wav_audio_;
 
   size_t cursor_;
 
+  // True if replay of the current sound is scheduled.
+  bool replay_scheduled_;
+
+  base::CancelableClosure stop_stream_closure_;
+  bool is_stream_idle_;
+
+  // Last time sound started to play from the start.
+  base::TimeTicks last_play_start_time_;
+
+  int schedule_replay_request_for_testing_;
+
   DISALLOW_COPY_AND_ASSIGN(AudioStreamContainer);
 };
 
@@ -127,16 +200,11 @@ AudioStreamHandler::AudioStreamHandler(const base::StringPiece& wav_data)
     LOG(ERROR) << "Can't get access to audio manager.";
     return;
   }
-  AudioParameters params(AudioParameters::AUDIO_PCM_LOW_LATENCY,
-                         GuessChannelLayout(wav_audio_.num_channels()),
-                         wav_audio_.sample_rate(),
-                         wav_audio_.bits_per_sample(),
-                         kDefaultFrameCount);
-  if (!params.IsValid()) {
+  if (!wav_audio_.params().IsValid()) {
     LOG(ERROR) << "Audio params are invalid.";
     return;
   }
-  stream_.reset(new AudioStreamContainer(wav_audio_, params));
+  stream_.reset(new AudioStreamContainer(wav_audio_));
   initialized_ = true;
 }
 
@@ -185,4 +253,11 @@ void AudioStreamHandler::SetAudioSourceForTesting(
   g_audio_source_for_testing = source;
 }
 
+void AudioStreamHandler::AllowReplayOnceForTesting() {
+  AudioManager::Get()->GetMessageLoop()->PostTask(
+      FROM_HERE,
+      base::Bind(&AudioStreamContainer::AllowReplayOnceForTesting,
+                 base::Unretained(stream_.get())));
+}
+
 }  // namespace media