Limit audio buffer size in the audio player used by the chromoting client.

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

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

3 files changed, 44 insertions, 31 deletions

diff --git a/remoting/client/plugin/pepper_audio_player.cc b/remoting/client/plugin/pepper_audio_player.cc
index d9c53a5..b52a40c 100644
--- a/remoting/client/plugin/pepper_audio_player.cc
+++ b/remoting/client/plugin/pepper_audio_player.cc
@@ -9,14 +9,18 @@
 #include "base/stl_util.h"
 
 namespace {
-// Constants used to create an audio configuration resource.
-// The sample count we will request from the browser.
-const uint32_t kSampleFrameCount = 4096u;
+
+// The frame size we will request from the browser.
+const int kFrameSizeMs = 40;
+
 // The number of channels in the audio stream (only supporting stereo audio
 // for now).
-const uint32_t kChannels = 2u;
+const int kChannels = 2u;
 const int kSampleSizeBytes = 2;
 
+// If queue grows bigger than 150ms we start dropping packets.
+const int kMaxQueueLatencyMs = 150;
+
 PP_AudioSampleRate ConvertToPepperSampleRate(
     remoting::AudioPacket::SamplingRate sampling_rate) {
   switch (sampling_rate) {
@@ -37,9 +41,10 @@ namespace remoting {
 PepperAudioPlayer::PepperAudioPlayer(pp::Instance* instance)
     : instance_(instance),
       sampling_rate_(AudioPacket::SAMPLING_RATE_INVALID),
-      samples_per_frame_(kSampleFrameCount),
-      bytes_consumed_(0),
-      start_failed_(false) {
+      samples_per_frame_(0),
+      start_failed_(false),
+      queued_samples_(0),
+      bytes_consumed_(0) {
 }
 
 PepperAudioPlayer::~PepperAudioPlayer() {}
@@ -50,16 +55,14 @@ bool PepperAudioPlayer::ResetAudioPlayer(
   PP_AudioSampleRate sample_rate =
       ConvertToPepperSampleRate(sampling_rate);
 
-  // Ask the browser/device for an appropriate sample frame count size.
-  samples_per_frame_ =
-      pp::AudioConfig::RecommendSampleFrameCount(instance_,
-                                                 sample_rate,
-                                                 kSampleFrameCount);
+  // Ask the browser/device for an appropriate frame size.
+  samples_per_frame_ = pp::AudioConfig::RecommendSampleFrameCount(
+      instance_, sample_rate,
+      kFrameSizeMs * sampling_rate / base::Time::kMillisecondsPerSecond);
 
   // Create an audio configuration resource.
-  pp::AudioConfig audio_config = pp::AudioConfig(instance_,
-                                                 sample_rate,
-                                                 samples_per_frame_);
+  pp::AudioConfig audio_config = pp::AudioConfig(
+      instance_, sample_rate, samples_per_frame_);
 
   // Create an audio resource.
   audio_ = pp::Audio(instance_, audio_config, PepperAudioPlayerCallback, this);
@@ -72,20 +75,12 @@ bool PepperAudioPlayer::ResetAudioPlayer(
 }
 
 void PepperAudioPlayer::ProcessAudioPacket(scoped_ptr<AudioPacket> packet) {
-  // TODO(kxing): Limit the size of the queue so that latency doesn't grow
-  // too large.
-
   CHECK_EQ(1, packet->data_size());
   DCHECK_EQ(AudioPacket::ENCODING_RAW, packet->encoding());
   DCHECK_NE(AudioPacket::SAMPLING_RATE_INVALID, packet->sampling_rate());
   DCHECK_EQ(kSampleSizeBytes, packet->bytes_per_sample());
   DCHECK_EQ(static_cast<int>(kChannels), packet->channels());
-
-  if (packet->data(0).size() % (kChannels * kSampleSizeBytes) != 0) {
-    LOG(WARNING) << "Received corrupted packet.";
-    return;
-  }
-  base::AutoLock auto_lock(lock_);
+  DCHECK_EQ(packet->data(0).size() % (kChannels * kSampleSizeBytes), 0u);
 
   // No-op if the Pepper player won't start.
   if (start_failed_) {
@@ -96,7 +91,11 @@ void PepperAudioPlayer::ProcessAudioPacket(scoped_ptr<AudioPacket> packet) {
   if (sampling_rate_ != packet->sampling_rate()) {
     // Drop all packets currently in the queue, since they are sampled at the
     // wrong rate.
-    STLDeleteElements(&queued_packets_);
+    {
+      base::AutoLock auto_lock(lock_);
+      STLDeleteElements(&queued_packets_);
+      queued_samples_ = 0;
+    }
 
     bool success = ResetAudioPlayer(packet->sampling_rate());
     if (!success) {
@@ -105,6 +104,15 @@ void PepperAudioPlayer::ProcessAudioPacket(scoped_ptr<AudioPacket> packet) {
     }
   }
 
+  base::AutoLock auto_lock(lock_);
+
+  if (queued_samples_ > kMaxQueueLatencyMs * sampling_rate_ /
+      base::Time::kMillisecondsPerSecond) {
+    STLDeleteElements(&queued_packets_);
+    queued_samples_ = 0;
+  }
+
+  queued_samples_ += packet->data(0).size() / (kChannels * kSampleSizeBytes);
   queued_packets_.push_back(packet.release());
 }
 
@@ -152,6 +160,8 @@ void PepperAudioPlayer::FillWithSamples(void* samples, uint32_t 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);
   }
 }
 
diff --git a/remoting/client/plugin/pepper_audio_player.h b/remoting/client/plugin/pepper_audio_player.h
index ad7c53d..2a0cf757 100644
--- a/remoting/client/plugin/pepper_audio_player.h
+++ b/remoting/client/plugin/pepper_audio_player.h
@@ -48,18 +48,19 @@ class PepperAudioPlayer : public AudioPlayer {
   // The count of sample frames per channel in an audio buffer.
   uint32_t samples_per_frame_;
 
-  // Protects |queued_packets_| and |bytes_consumed_|.
-  // This is necessary to prevent races,
-  // because Pepper will callback on a separate thread.
+  bool start_failed_;
+
+  // Protects |queued_packets_|, |queued_samples_ and |bytes_consumed_|. This is
+  // necessary to prevent races, because Pepper will call the  callback on a
+  // separate thread.
   base::Lock lock_;
 
   AudioPacketQueue queued_packets_;
+  int queued_samples_;
 
   // The number of bytes from |queued_packets_| that have been consumed.
   size_t bytes_consumed_;
 
-  bool start_failed_;
-
   DISALLOW_COPY_AND_ASSIGN(PepperAudioPlayer);
 };
 
diff --git a/remoting/codec/audio_decoder_verbatim.cc b/remoting/codec/audio_decoder_verbatim.cc
index e8143fa..6c9b639 100644
--- a/remoting/codec/audio_decoder_verbatim.cc
+++ b/remoting/codec/audio_decoder_verbatim.cc
@@ -22,7 +22,9 @@ scoped_ptr<AudioPacket> AudioDecoderVerbatim::Decode(
       (packet->data_size() != 1) ||
       (packet->sampling_rate() == AudioPacket::SAMPLING_RATE_INVALID) ||
       (packet->bytes_per_sample() != AudioPacket::BYTES_PER_SAMPLE_2) ||
-      (packet->channels() != AudioPacket::CHANNELS_STEREO)) {
+      (packet->channels() != AudioPacket::CHANNELS_STEREO) ||
+      (packet->data(0).size() %
+       (AudioPacket::CHANNELS_STEREO * AudioPacket::BYTES_PER_SAMPLE_2) != 0)) {
     LOG(WARNING) << "Verbatim decoder received an invalid packet.";
     return scoped_ptr<AudioPacket>();
   }