Added volume adjust sound behind the flag. Partially based on...

Added volume adjust sound behind the flag. Partially based on https://codereview.chromium.org/24618003/.

BUG=225886
TEST=media_unittests:AudioStreamHandler*

Review URL: https://codereview.chromium.org/115693004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@245342 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/ui/ash/DEPS

+include_rules = [
+  "+media",
+]

chrome/browser/ui/ash/volume_controller_chromeos.cc

@@ -5,9 +5,16 @@
 #include "chrome/browser/ui/ash/volume_controller_chromeos.h"
 
 #include "ash/ash_switches.h"
+#include "ash/audio/sounds.h"
+#include "base/command_line.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/api/system_private/system_private_api.h"
+#include "chromeos/audio/chromeos_sounds.h"
+#include "chromeos/chromeos_switches.h"
 #include "content/public/browser/user_metrics.h"
+#include "grit/browser_resources.h"
+#include "media/audio/sounds/sounds_manager.h"
+#include "ui/base/resource/resource_bundle.h"
 
 using chromeos::CrasAudioHandler;
 
@@ -16,10 +23,22 @@ namespace {
 // Percent by which the volume should be changed when a volume key is pressed.
 const double kStepPercentage = 4.0;
 
+void PlayVolumeAdjustSound() {
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          chromeos::switches::kEnableVolumeAdjustSound)) {
+    ash::PlaySystemSound(chromeos::SOUND_VOLUME_ADJUST,
+                         true /* honor_spoken_feedback */);
+  }
+}
+
 }  // namespace
 
 VolumeController::VolumeController() {
   CrasAudioHandler::Get()->AddAudioObserver(this);
+  ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
+  media::SoundsManager::Get()->Initialize(
+      chromeos::SOUND_VOLUME_ADJUST,
+      bundle.GetRawDataResource(IDR_SOUND_VOLUME_ADJUST_WAV));
 }
 
 VolumeController::~VolumeController() {
@@ -46,6 +65,8 @@ bool VolumeController::HandleVolumeDown(const ui::Accelerator& accelerator) {
     audio_handler->AdjustOutputVolumeByPercent(-kStepPercentage);
     if (audio_handler->IsOutputVolumeBelowDefaultMuteLvel())
       audio_handler->SetOutputMute(true);
+    else
+      PlayVolumeAdjustSound();
   }
   return true;
 }
@@ -55,13 +76,18 @@ bool VolumeController::HandleVolumeUp(const ui::Accelerator& accelerator) {
     content::RecordAction(base::UserMetricsAction("Accel_VolumeUp_F10"));
 
   CrasAudioHandler* audio_handler = CrasAudioHandler::Get();
+  bool play_sound = false;
   if (audio_handler->IsOutputMuted()) {
-      audio_handler->SetOutputMute(false);
-      audio_handler->AdjustOutputVolumeToAudibleLevel();
+    audio_handler->SetOutputMute(false);
+    audio_handler->AdjustOutputVolumeToAudibleLevel();
+    play_sound = true;
   } else {
+    play_sound = audio_handler->GetOutputVolumePercent() != 100;
     audio_handler->AdjustOutputVolumeByPercent(kStepPercentage);
   }
 
+  if (play_sound)
+    PlayVolumeAdjustSound();
   return true;
 }
 

chromeos/audio/chromeos_sounds.h

@@ -15,7 +15,8 @@ enum {
   SOUND_UNLOCK,
   SOUND_SHUTDOWN,
   SOUND_SPOKEN_FEEDBACK_ENABLED,
-  SOUND_SPOKEN_FEEDBACK_DISABLED
+  SOUND_SPOKEN_FEEDBACK_DISABLED,
+  SOUND_VOLUME_ADJUST
 };
 
 }  // namespace chromeos

chromeos/chromeos_switches.cc

@@ -90,6 +90,9 @@ const char kEnableStubPortalledWifi[] = "enable-stub-portalled-wifi";
 const char kEnableTouchpadThreeFingerClick[]
     = "enable-touchpad-three-finger-click";
 
+// Enables volume adjust sound.
+const char kEnableVolumeAdjustSound[] = "enable-volume-adjust-sound";
+
 // Specifies stub network types to be enabled.  If this switch is not specified,
 // ethernet, wifi and vpn are enabled by default.
 //

chromeos/chromeos_switches.h

@@ -46,6 +46,7 @@ CHROMEOS_EXPORT extern const char kEnableRequestTabletSite[];
 CHROMEOS_EXPORT extern const char kEnableStubInteractive[];
 CHROMEOS_EXPORT extern const char kEnableStubPortalledWifi[];
 CHROMEOS_EXPORT extern const char kEnableTouchpadThreeFingerClick[];
+CHROMEOS_EXPORT extern const char kEnableVolumeAdjustSound[];
 CHROMEOS_EXPORT extern const char kEnabledStubNetworkTypes[];
 CHROMEOS_EXPORT extern const char kEnterpriseEnrollmentInitialModulus[];
 CHROMEOS_EXPORT extern const char kEnterpriseEnrollmentModulusLimit[];

media/audio/sounds/audio_stream_handler.cc

@@ -6,8 +6,11 @@
 
 #include <string>
 
+#include "base/cancelable_callback.h"
 #include "base/logging.h"
 #include "base/single_thread_task_runner.h"
+#include "base/synchronization/lock.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"
@@ -22,6 +25,9 @@ const double kOutputVolumePercent = 0.8;
 // The number of frames each OnMoreData() call will request.
 const int kDefaultFrameCount = 1024;
 
+// Keep alive timeout for audio stream.
+const int kKeepAliveMs = 1500;
+
 AudioStreamHandler::TestObserver* g_observer_for_testing = NULL;
 AudioOutputStream::AudioSourceCallback* g_audio_source_for_testing = NULL;
 
@@ -30,12 +36,12 @@ 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),
+        started_(false),
+        delayed_stop_posted_(false) {
   }
 
   virtual ~AudioStreamContainer() {
@@ -46,20 +52,38 @@ class AudioStreamHandler::AudioStreamContainer
     DCHECK(AudioManager::Get()->GetTaskRunner()->BelongsToCurrentThread());
 
     if (!stream_) {
-      stream_ = AudioManager::Get()->MakeAudioOutputStreamProxy(params_,
-                                                                std::string(),
-                                                                std::string());
+      const AudioParameters& p = wav_audio_.params();
+      const AudioParameters params(AudioParameters::AUDIO_PCM_LOW_LATENCY,
+                                   p.channel_layout(),
+                                   p.sample_rate(),
+                                   p.bits_per_sample(),
+                                   kDefaultFrameCount);
+      stream_ = AudioManager::Get()->MakeAudioOutputStreamProxy(
+          params, std::string(), std::string());
       if (!stream_ || !stream_->Open()) {
         LOG(ERROR) << "Failed to open an output stream.";
         return;
       }
       stream_->SetVolume(kOutputVolumePercent);
-    } else {
-      // TODO (ygorshenin@): implement smart stream rewind.
-      stream_->Stop();
     }
 
-    cursor_ = 0;
+    {
+      base::AutoLock al(state_lock_);
+
+      delayed_stop_posted_ = false;
+      stop_closure_.Reset(base::Bind(
+          &AudioStreamContainer::StopStream, base::Unretained(this)));
+
+      if (started_) {
+        if (wav_audio_.AtEnd(cursor_))
+          cursor_ = 0;
+        return;
+      }
+
+      cursor_ = 0;
+      started_ = true;
+    }
+
     if (g_audio_source_for_testing)
       stream_->Start(g_audio_source_for_testing);
     else
@@ -71,14 +95,10 @@ class AudioStreamHandler::AudioStreamContainer
 
   void Stop() {
     DCHECK(AudioManager::Get()->GetTaskRunner()->BelongsToCurrentThread());
-    if (!stream_)
-      return;
-    stream_->Stop();
-    stream_->Close();
+    StopStream();
+    if (stream_)
+      stream_->Close();
     stream_ = NULL;
-
-    if (g_observer_for_testing)
-      g_observer_for_testing->OnStop(cursor_);
   }
 
  private:
@@ -86,16 +106,21 @@ class AudioStreamHandler::AudioStreamContainer
   // Following methods could be called from *ANY* thread.
   virtual int OnMoreData(AudioBus* dest,
                          AudioBuffersState /* state */) OVERRIDE {
+    base::AutoLock al(state_lock_);
     size_t bytes_written = 0;
+
     if (wav_audio_.AtEnd(cursor_) ||
         !wav_audio_.CopyTo(dest, cursor_, &bytes_written)) {
-      AudioManager::Get()->GetTaskRunner()->PostTask(
+      if (delayed_stop_posted_)
+        return 0;
+      delayed_stop_posted_ = true;
+      AudioManager::Get()->GetTaskRunner()->PostDelayedTask(
           FROM_HERE,
-          base::Bind(&AudioStreamContainer::Stop, base::Unretained(this)));
+          stop_closure_.callback(),
+          base::TimeDelta::FromMilliseconds(kKeepAliveMs));
       return 0;
     }
     cursor_ += bytes_written;
-
     return dest->frames();
   }
 
@@ -109,12 +134,28 @@ class AudioStreamHandler::AudioStreamContainer
     LOG(ERROR) << "Error during system sound reproduction.";
   }
 
+  void StopStream() {
+    DCHECK(AudioManager::Get()->GetTaskRunner()->BelongsToCurrentThread());
+
+    base::AutoLock al(state_lock_);
+
+    if (stream_ && started_) {
+      stream_->Stop();
+      if (g_observer_for_testing)
+        g_observer_for_testing->OnStop(cursor_);
+    }
+    started_ = false;
+  }
+
   AudioOutputStream* stream_;
 
   const WavAudioHandler wav_audio_;
-  const AudioParameters params_;
 
+  base::Lock state_lock_;
   size_t cursor_;
+  bool started_;
+  bool delayed_stop_posted_;
+  base::CancelableClosure stop_closure_;
 
   DISALLOW_COPY_AND_ASSIGN(AudioStreamContainer);
 };
@@ -127,16 +168,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;
 }
 
@@ -146,7 +182,7 @@ AudioStreamHandler::~AudioStreamHandler() {
       FROM_HERE,
       base::Bind(&AudioStreamContainer::Stop, base::Unretained(stream_.get())));
   AudioManager::Get()->GetTaskRunner()->DeleteSoon(FROM_HERE,
-                                                    stream_.release());
+                                                   stream_.release());
 }
 
 bool AudioStreamHandler::IsInitialized() const {

media/audio/sounds/audio_stream_handler.h

@@ -42,10 +42,12 @@ class MEDIA_EXPORT AudioStreamHandler : public base::NonThreadSafe {
   // Returns true iff AudioStreamHandler is correctly initialized;
   bool IsInitialized() const;
 
-  // Stops any previous playback if it's still not completed and
-  // starts new playback. Volume level will be set according to
-  // current settings and won't be changed during playback. Returns
-  // true iff new playback was successfully started.
+  // Plays sound.  Volume level will be set according to current settings
+  // and won't be changed during playback. Returns true iff new playback
+  // was successfully started.
+  //
+  // NOTE: if current playback isn't at end of stream, playback request
+  // is dropped, but true is returned.
   bool Play();
 
   // Stops current playback.

media/audio/sounds/audio_stream_handler_unittest.cc

@@ -74,7 +74,7 @@ TEST_F(AudioStreamHandlerTest, Play) {
   ASSERT_EQ(4, observer.cursor());
 }
 
-TEST_F(AudioStreamHandlerTest, Rewind) {
+TEST_F(AudioStreamHandlerTest, ConsecutivePlayRequests) {
   base::RunLoop run_loop;
   TestObserver observer(run_loop.QuitClosure());
   SineWaveAudioSource source(CHANNEL_LAYOUT_STEREO, 200.0, 8000);
@@ -89,19 +89,19 @@ TEST_F(AudioStreamHandlerTest, Rewind) {
       FROM_HERE,
       base::Bind(base::IgnoreResult(&AudioStreamHandler::Play),
                  base::Unretained(audio_stream_handler())),
-      base::TimeDelta::FromSeconds(3));
+      base::TimeDelta::FromSeconds(1));
   base::MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
       base::Bind(&AudioStreamHandler::Stop,
                  base::Unretained(audio_stream_handler())),
-      base::TimeDelta::FromSeconds(6));
+      base::TimeDelta::FromSeconds(2));
 
   run_loop.Run();
 
   SetObserverForTesting(NULL);
   SetAudioSourceForTesting(NULL);
 
-  ASSERT_EQ(2, observer.num_play_requests());
+  ASSERT_EQ(1, observer.num_play_requests());
   ASSERT_EQ(1, observer.num_stop_requests());
 }
 

media/audio/sounds/sounds_manager.cc

@@ -75,9 +75,7 @@ base::TimeDelta SoundsManagerImpl::GetDuration(SoundKey key) {
     return base::TimeDelta();
   }
   const WavAudioHandler& wav_audio = handlers_[key]->wav_audio_handler();
-  const int64 size = wav_audio.size();
-  const int64 rate = wav_audio.byte_rate();
-  return base::TimeDelta::FromMicroseconds(size * 1000000 / rate);
+  return wav_audio.params().GetBufferDuration();
 }
 
 // SoundsManagerStub ---------------------------------------------------

media/audio/sounds/sounds_manager_unittest.cc

@@ -54,7 +54,7 @@ TEST_F(SoundsManagerTest, Play) {
   ASSERT_TRUE(SoundsManager::Get()->Initialize(
       kTestAudioKey,
       base::StringPiece(kTestAudioData, arraysize(kTestAudioData))));
-  ASSERT_EQ(41,
+  ASSERT_EQ(20,
             SoundsManager::Get()->GetDuration(kTestAudioKey).InMicroseconds());
   ASSERT_TRUE(SoundsManager::Get()->Play(kTestAudioKey));
   run_loop.Run();

media/audio/sounds/wav_audio_handler.cc

@@ -33,14 +33,14 @@ const size_t kFmtChunkMinimumSize = 16;
 const size_t kAudioFormatOffset = 0;
 const size_t kChannelOffset = 2;
 const size_t kSampleRateOffset = 4;
-const size_t kByteRateOffset = 8;
 const size_t kBitsPerSampleOffset = 14;
 
 // Some constants for audio format.
 const int kAudioFormatPCM = 1;
 
 // Reads an integer from |data| with |offset|.
-template<typename T> T ReadInt(const base::StringPiece& data, size_t offset) {
+template <typename T>
+T ReadInt(const base::StringPiece& data, size_t offset) {
   CHECK_LE(offset + sizeof(T), data.size());
   T result;
   memcpy(&result, data.data() + offset, sizeof(T));
@@ -57,7 +57,6 @@ namespace media {
 WavAudioHandler::WavAudioHandler(const base::StringPiece& wav_data)
     : num_channels_(0),
       sample_rate_(0),
-      byte_rate_(0),
       bits_per_sample_(0) {
   CHECK_LE(kWavFileHeaderSize, wav_data.size()) << "wav data is too small";
   CHECK(wav_data.starts_with(kChunkId) &&
@@ -72,11 +71,17 @@ WavAudioHandler::WavAudioHandler(const base::StringPiece& wav_data)
     CHECK_LE(0, length) << "can't parse wav sub-chunk";
     offset += length;
   }
-}
 
-WavAudioHandler::~WavAudioHandler() {
+  const int frame_count = data_.size() * 8 / num_channels_ / bits_per_sample_;
+  params_ = AudioParameters(AudioParameters::AUDIO_PCM_LOW_LATENCY,
+                            GuessChannelLayout(num_channels_),
+                            sample_rate_,
+                            bits_per_sample_,
+                            frame_count);
 }
 
+WavAudioHandler::~WavAudioHandler() {}
+
 bool WavAudioHandler::AtEnd(size_t cursor) const {
   return data_.size() <= cursor;
 }
@@ -86,18 +91,20 @@ bool WavAudioHandler::CopyTo(AudioBus* bus,
                              size_t* bytes_written) const {
   if (!bus)
     return false;
-  if (bus->channels() != num_channels_) {
-    DLOG(ERROR) << "Number of channel mismatch.";
+  if (bus->channels() != params_.channels()) {
+    DVLOG(1) << "Number of channel mismatch.";
     return false;
   }
   if (AtEnd(cursor)) {
     bus->Zero();
     return true;
   }
-  const int remaining_frames = (data_.size() - cursor) / bytes_per_frame_;
+  const int remaining_frames =
+      (data_.size() - cursor) / params_.GetBytesPerFrame();
   const int frames = std::min(bus->frames(), remaining_frames);
-  bus->FromInterleaved(data_.data() + cursor, frames, bytes_per_sample_);
-  *bytes_written = frames * bytes_per_frame_;
+  bus->FromInterleaved(data_.data() + cursor, frames,
+                       params_.bits_per_sample() / 8);
+  *bytes_written = frames * params_.GetBytesPerFrame();
   bus->ZeroFramesPartial(frames, bus->frames() - frames);
   return true;
 }
@@ -126,10 +133,7 @@ bool WavAudioHandler::ParseFmtChunk(const base::StringPiece& data) {
   DCHECK_EQ(ReadInt<uint16>(data, kAudioFormatOffset), kAudioFormatPCM);
   num_channels_ = ReadInt<uint16>(data, kChannelOffset);
   sample_rate_ = ReadInt<uint32>(data, kSampleRateOffset);
-  byte_rate_ = ReadInt<uint32>(data, kByteRateOffset);
   bits_per_sample_ = ReadInt<uint16>(data, kBitsPerSampleOffset);
-  bytes_per_sample_ = bits_per_sample_ >> 3;
-  bytes_per_frame_ = num_channels_ * bytes_per_sample_;
   return true;
 }
 

media/audio/sounds/wav_audio_handler.h

@@ -6,6 +6,8 @@
 #define MEDIA_AUDIO_SOUNDS_WAV_AUDIO_HANDLER_H_
 
 #include "base/strings/string_piece.h"
+#include "base/time/time.h"
+#include "media/audio/audio_parameters.h"
 #include "media/base/media_export.h"
 
 namespace media {
@@ -27,11 +29,8 @@ class MEDIA_EXPORT WavAudioHandler {
   // |bytes_written|. |bytes_written| should not be NULL.
   bool CopyTo(AudioBus* bus, size_t cursor, size_t* bytes_written) const;
 
-  int size() const { return data_.size(); }
-  uint16 num_channels() const { return num_channels_; }
-  uint32 sample_rate() const { return sample_rate_; }
-  uint32 byte_rate() const { return byte_rate_; }
-  uint16 bits_per_sample() const { return bits_per_sample_; }
+  const AudioParameters& params() const { return params_; }
+  const base::StringPiece& data() const { return data_; }
 
  private:
   // Parses a chunk of wav format data. Returns the length of the chunk.
@@ -46,12 +45,11 @@ class MEDIA_EXPORT WavAudioHandler {
   // Data part of the |wav_data_|.
   base::StringPiece data_;
 
+  AudioParameters params_;
+
   uint16 num_channels_;
   uint32 sample_rate_;
-  uint32 byte_rate_;
   uint16 bits_per_sample_;
-  int bytes_per_sample_;
-  int bytes_per_frame_;
 };
 
 }  // namespace media

media/audio/sounds/wav_audio_handler_unittest.cc

@@ -16,18 +16,22 @@ namespace media {
 TEST(WavAudioHandlerTest, SampleDataTest) {
   WavAudioHandler handler(base::StringPiece(kTestAudioData,
                                             arraysize(kTestAudioData)));
-  ASSERT_EQ(static_cast<uint16>(2), handler.num_channels());
-  ASSERT_EQ(static_cast<uint16>(16), handler.bits_per_sample());
-  ASSERT_EQ(static_cast<uint32>(48000), handler.sample_rate());
-  ASSERT_EQ(static_cast<uint32>(96000), handler.byte_rate());
+  const AudioParameters& params = handler.params();
+  ASSERT_EQ(2, params.channels());
+  ASSERT_EQ(16, params.bits_per_sample());
+  ASSERT_EQ(48000, params.sample_rate());
+  ASSERT_EQ(192000, params.GetBytesPerSecond());
+
+  ASSERT_EQ(4U, handler.data().size());
+  const char kData[] = "\x01\x00\x01\x00";
+  ASSERT_EQ(base::StringPiece(kData, arraysize(kData) - 1), handler.data());
 
-  ASSERT_EQ(4, handler.size());
   scoped_ptr<AudioBus> bus = AudioBus::Create(
-      handler.num_channels(),
-      handler.size() / handler.num_channels());
+      params.channels(), handler.data().size() / params.channels());
+
   size_t bytes_written;
   ASSERT_TRUE(handler.CopyTo(bus.get(), 0, &bytes_written));
-  ASSERT_EQ(static_cast<size_t>(handler.size()), bytes_written);
+  ASSERT_EQ(static_cast<size_t>(handler.data().size()), bytes_written);
 }
 
 }  // namespace media