Drop audio packets in the chromoting client less agressively.

Previously chromoting client would drop all queued audio samples when amount 
of data in the buffer exceeds 150ms. Because of that it wouldn't always have 
enough data buffered which results in audible gliches. Fixed AudioPlayer so 
that it drops just enough packets to keep the buffer size under the limit.

BUG=164308


Review URL: https://chromiumcodereview.appspot.com/11697009

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

3 files changed, 26 insertions, 30 deletions

diff --git a/remoting/client/audio_player.cc b/remoting/client/audio_player.cc
index f2ec27b..a26f1bc 100644
--- a/remoting/client/audio_player.cc
+++ b/remoting/client/audio_player.cc
@@ -11,7 +11,7 @@
 
 // The number of channels in the audio stream (only supporting stereo audio
 // for now).
-const int kChannels = 2u;
+const int kChannels = 2;
 const int kSampleSizeBytes = 2;
 
 // If queue grows bigger than 150ms we start dropping packets.
@@ -22,7 +22,7 @@ namespace remoting {
 AudioPlayer::AudioPlayer()
     : sampling_rate_(AudioPacket::SAMPLING_RATE_INVALID),
       start_failed_(false),
-      queued_samples_(0),
+      queued_bytes_(0),
       bytes_consumed_(0) {
 }
 
@@ -63,13 +63,19 @@ void AudioPlayer::ProcessAudioPacket(scoped_ptr<AudioPacket> packet) {
 
   base::AutoLock auto_lock(lock_);
 
-  if (queued_samples_ > kMaxQueueLatencyMs * sampling_rate_ /
-      base::Time::kMillisecondsPerSecond) {
-    ResetQueue();
-  }
-
-  queued_samples_ += packet->data(0).size() / (kChannels * kSampleSizeBytes);
+  queued_bytes_ += packet->data(0).size();
   queued_packets_.push_back(packet.release());
+
+  int max_buffer_size_ =
+      kMaxQueueLatencyMs * sampling_rate_ * kSampleSizeBytes * kChannels /
+      base::Time::kMillisecondsPerSecond;
+  while (queued_bytes_ > max_buffer_size_) {
+    queued_bytes_ -= queued_packets_.front()->data(0).size() - bytes_consumed_;
+    DCHECK_GE(queued_bytes_, 0);
+    delete queued_packets_.front();
+    queued_packets_.pop_front();
+    bytes_consumed_ = 0;
+  }
 }
 
 // static
@@ -83,7 +89,7 @@ void AudioPlayer::AudioPlayerCallback(void* samples,
 void AudioPlayer::ResetQueue() {
   lock_.AssertAcquired();
   STLDeleteElements(&queued_packets_);
-  queued_samples_ = 0;
+  queued_bytes_ = 0;
   bytes_consumed_ = 0;
 }
 
@@ -123,8 +129,8 @@ void AudioPlayer::FillWithSamples(void* samples, uint32 buffer_size) {
     next_sample += bytes_to_copy;
     bytes_consumed_ += bytes_to_copy;
     bytes_extracted += bytes_to_copy;
-    queued_samples_ -= bytes_to_copy / kSampleSizeBytes / kChannels;
-    DCHECK_GE(queued_samples_, 0);
+    queued_bytes_ -= bytes_to_copy;
+    DCHECK_GE(queued_bytes_, 0);
   }
 }
 
diff --git a/remoting/client/audio_player.h b/remoting/client/audio_player.h
index 0f7e3a8..e131737 100644
--- a/remoting/client/audio_player.h
+++ b/remoting/client/audio_player.h
@@ -52,7 +52,7 @@ class AudioPlayer {
   base::Lock lock_;
 
   AudioPacketQueue queued_packets_;
-  int queued_samples_;
+  int queued_bytes_;
 
   // The number of bytes from |queued_packets_| that have been consumed.
   size_t bytes_consumed_;
diff --git a/remoting/client/audio_player_unittest.cc b/remoting/client/audio_player_unittest.cc
index 175c5a1..d68dff1 100644
--- a/remoting/client/audio_player_unittest.cc
+++ b/remoting/client/audio_player_unittest.cc
@@ -73,12 +73,8 @@ class AudioPlayerTest : public ::testing::Test {
     }
   }
 
-  void SetQueuedSamples(int num_samples) {
-    audio_->queued_samples_ = num_samples;
-  }
-
   int GetNumQueuedSamples() {
-    return audio_->queued_samples_;
+    return audio_->queued_bytes_ / kAudioSampleBytes;
   }
 
   int GetNumQueuedPackets() {
@@ -154,20 +150,14 @@ TEST_F(AudioPlayerTest, ChangeSampleRate) {
 }
 
 TEST_F(AudioPlayerTest, ExceedLatency) {
-  scoped_ptr<AudioPacket> packet1(CreatePacket44100Hz(10));
-  audio_->ProcessAudioPacket(packet1.Pass());
-  ASSERT_EQ(10, GetNumQueuedSamples());
-  ASSERT_EQ(1, GetNumQueuedPackets());
-
-  // Fake lots of queued samples.
-  SetQueuedSamples(20000);
+  // Push about 4 seconds worth of samples.
+  for (int i = 0; i < 100; ++i) {
+    scoped_ptr<AudioPacket> packet1(CreatePacket48000Hz(2000));
+    audio_->ProcessAudioPacket(packet1.Pass());
+  }
 
-  // Previous sample should have been deleted because of latency (too many
-  // unprocessed samples).
-  scoped_ptr<AudioPacket> packet2(CreatePacket44100Hz(20));
-  audio_->ProcessAudioPacket(packet2.Pass());
-  ASSERT_EQ(20, GetNumQueuedSamples());
-  ASSERT_EQ(1, GetNumQueuedPackets());
+  // Verify that we don't have more than 0.5s.
+  EXPECT_LT(GetNumQueuedSamples(), 24000);
 }
 
 // Incoming packets: 100