Disable preroll duration increase on underflow for MediaSource playback.

BUG=244334
TEST=None. Underflow covered by existing tests.

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

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

9 files changed, 32 insertions, 18 deletions

diff --git a/media/base/audio_renderer.h b/media/base/audio_renderer.h
index 43cc226..14bf7c5 100644
--- a/media/base/audio_renderer.h
+++ b/media/base/audio_renderer.h
@@ -81,10 +81,7 @@ class MEDIA_EXPORT AudioRenderer {
   virtual void SetVolume(float volume) = 0;
 
   // Resumes playback after underflow occurs.
-  //
-  // |buffer_more_audio| is set to true if you want to increase the size of the
-  // decoded audio buffer.
-  virtual void ResumeAfterUnderflow(bool buffer_more_audio) = 0;
+  virtual void ResumeAfterUnderflow() = 0;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(AudioRenderer);
diff --git a/media/base/mock_filters.h b/media/base/mock_filters.h
index 5b9062a..b1be2ad 100644
--- a/media/base/mock_filters.h
+++ b/media/base/mock_filters.h
@@ -149,7 +149,7 @@ class MockAudioRenderer : public AudioRenderer {
   MOCK_METHOD1(SetPlaybackRate, void(float playback_rate));
   MOCK_METHOD2(Preroll, void(base::TimeDelta time, const PipelineStatusCB& cb));
   MOCK_METHOD1(SetVolume, void(float volume));
-  MOCK_METHOD1(ResumeAfterUnderflow, void(bool buffer_more_audio));
+  MOCK_METHOD0(ResumeAfterUnderflow, void());
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockAudioRenderer);
diff --git a/media/base/pipeline.cc b/media/base/pipeline.cc
index 7123930..67cc0f3 100644
--- a/media/base/pipeline.cc
+++ b/media/base/pipeline.cc
@@ -928,7 +928,7 @@ void Pipeline::OnAudioUnderflow() {
     return;
 
   if (audio_renderer_)
-    audio_renderer_->ResumeAfterUnderflow(true);
+    audio_renderer_->ResumeAfterUnderflow();
 }
 
 void Pipeline::StartClockIfWaitingForTimeUpdate_Locked() {
diff --git a/media/filters/audio_renderer_impl.cc b/media/filters/audio_renderer_impl.cc
index 66a6160..61b190b 100644
--- a/media/filters/audio_renderer_impl.cc
+++ b/media/filters/audio_renderer_impl.cc
@@ -42,7 +42,8 @@ AudioRendererImpl::AudioRendererImpl(
     const scoped_refptr<base::MessageLoopProxy>& message_loop,
     media::AudioRendererSink* sink,
     ScopedVector<AudioDecoder> decoders,
-    const SetDecryptorReadyCB& set_decryptor_ready_cb)
+    const SetDecryptorReadyCB& set_decryptor_ready_cb,
+    bool increase_preroll_on_underflow)
     : message_loop_(message_loop),
       weak_factory_(this),
       sink_(sink),
@@ -57,6 +58,7 @@ AudioRendererImpl::AudioRendererImpl(
       audio_time_buffered_(kNoTimestamp()),
       current_time_(kNoTimestamp()),
       underflow_disabled_(false),
+      increase_preroll_on_underflow_(increase_preroll_on_underflow),
       preroll_aborted_(false),
       actual_frames_per_buffer_(0) {
 }
@@ -284,7 +286,7 @@ void AudioRendererImpl::OnDecoderSelected(
   base::ResetAndReturn(&init_cb_).Run(PIPELINE_OK);
 }
 
-void AudioRendererImpl::ResumeAfterUnderflow(bool buffer_more_audio) {
+void AudioRendererImpl::ResumeAfterUnderflow() {
   DCHECK(message_loop_->BelongsToCurrentThread());
   base::AutoLock auto_lock(lock_);
   if (state_ == kUnderflow) {
@@ -294,7 +296,7 @@ void AudioRendererImpl::ResumeAfterUnderflow(bool buffer_more_audio) {
     // number of bytes that need to be buffered for preroll to complete)
     // does not increase due to an aborted preroll.
     // TODO(vrk): Fix this bug correctly! (crbug.com/151352)
-    if (buffer_more_audio && !preroll_aborted_)
+    if (increase_preroll_on_underflow_ && !preroll_aborted_)
       algorithm_->IncreaseQueueCapacity();
 
     state_ = kRebuffering;
diff --git a/media/filters/audio_renderer_impl.h b/media/filters/audio_renderer_impl.h
index 8202751..d96bfdd 100644
--- a/media/filters/audio_renderer_impl.h
+++ b/media/filters/audio_renderer_impl.h
@@ -52,10 +52,14 @@ class MEDIA_EXPORT AudioRendererImpl
   //
   // |set_decryptor_ready_cb| is fired when the audio decryptor is available
   // (only applicable if the stream is encrypted and we have a decryptor).
+  //
+  // |increase_preroll_on_underflow| Set to true if the preroll duration
+  // should be increased when ResumeAfterUnderflow() is called.
   AudioRendererImpl(const scoped_refptr<base::MessageLoopProxy>& message_loop,
                     AudioRendererSink* sink,
                     ScopedVector<AudioDecoder> decoders,
-                    const SetDecryptorReadyCB& set_decryptor_ready_cb);
+                    const SetDecryptorReadyCB& set_decryptor_ready_cb,
+                    bool increase_preroll_on_underflow);
   virtual ~AudioRendererImpl();
 
   // AudioRenderer implementation.
@@ -74,7 +78,7 @@ class MEDIA_EXPORT AudioRendererImpl
   virtual void SetPlaybackRate(float rate) OVERRIDE;
   virtual void Preroll(base::TimeDelta time,
                        const PipelineStatusCB& cb) OVERRIDE;
-  virtual void ResumeAfterUnderflow(bool buffer_more_audio) OVERRIDE;
+  virtual void ResumeAfterUnderflow() OVERRIDE;
   virtual void SetVolume(float volume) OVERRIDE;
 
   // Disables underflow support.  When used, |state_| will never transition to
@@ -256,6 +260,7 @@ class MEDIA_EXPORT AudioRendererImpl
   size_t total_frames_filled_;
 
   bool underflow_disabled_;
+  bool increase_preroll_on_underflow_;
 
   // True if the renderer receives a buffer with kAborted status during preroll,
   // false otherwise. This flag is cleared on the next Preroll() call.
diff --git a/media/filters/audio_renderer_impl_unittest.cc b/media/filters/audio_renderer_impl_unittest.cc
index 4c8004c..cec3441 100644
--- a/media/filters/audio_renderer_impl_unittest.cc
+++ b/media/filters/audio_renderer_impl_unittest.cc
@@ -63,7 +63,8 @@ class AudioRendererImplTest : public ::testing::Test {
         message_loop_.message_loop_proxy(),
         new NiceMock<MockAudioRendererSink>(),
         decoders.Pass(),
-        SetDecryptorReadyCB()));
+        SetDecryptorReadyCB(),
+        false));
 
     // Stub out time.
     renderer_->set_now_cb_for_testing(base::Bind(
@@ -279,7 +280,7 @@ class AudioRendererImplTest : public ::testing::Test {
   }
 
   void CallResumeAfterUnderflow() {
-    renderer_->ResumeAfterUnderflow(false);
+    renderer_->ResumeAfterUnderflow();
   }
 
   TimeDelta CalculatePlayTime(int bytes_filled) {
@@ -436,7 +437,7 @@ TEST_F(AudioRendererImplTest, Underflow) {
   EXPECT_CALL(*this, OnUnderflow());
   EXPECT_FALSE(ConsumeBufferedData(kDataSize, NULL));
 
-  renderer_->ResumeAfterUnderflow(false);
+  renderer_->ResumeAfterUnderflow();
 
   // Verify after resuming that we're still not getting data.
   bool muted = false;
diff --git a/media/filters/pipeline_integration_test_base.cc b/media/filters/pipeline_integration_test_base.cc
index b2163a0..7e1e59b 100644
--- a/media/filters/pipeline_integration_test_base.cc
+++ b/media/filters/pipeline_integration_test_base.cc
@@ -261,7 +261,8 @@ PipelineIntegrationTestBase::CreateFilterCollection(
       audio_decoders.Pass(),
       base::Bind(&PipelineIntegrationTestBase::SetDecryptor,
                  base::Unretained(this),
-                 decryptor));
+                 decryptor),
+      true);
   // Disable underflow if hashing is enabled.
   if (hashing_enabled_) {
     audio_sink_->StartAudioHashForTesting();
diff --git a/media/tools/player_x11/player_x11.cc b/media/tools/player_x11/player_x11.cc
index 80357a2..efa7573 100644
--- a/media/tools/player_x11/player_x11.cc
+++ b/media/tools/player_x11/player_x11.cc
@@ -136,7 +136,8 @@ void InitPipeline(media::Pipeline* pipeline,
       message_loop,
       new media::NullAudioSink(message_loop),
       audio_decoders.Pass(),
-      media::SetDecryptorReadyCB()));
+      media::SetDecryptorReadyCB(),
+      true));
   collection->SetAudioRenderer(audio_renderer.Pass());
 
   base::WaitableEvent event(true, false);
diff --git a/webkit/renderer/media/webmediaplayer_impl.cc b/webkit/renderer/media/webmediaplayer_impl.cc
index fbfedfe..c70ed27 100644
--- a/webkit/renderer/media/webmediaplayer_impl.cc
+++ b/webkit/renderer/media/webmediaplayer_impl.cc
@@ -1050,7 +1050,7 @@ void WebMediaPlayerImpl::NotifyDownloading(bool is_downloading) {
 
 void WebMediaPlayerImpl::StartPipeline(WebKit::WebMediaSource* media_source) {
   const CommandLine* cmd_line = CommandLine::ForCurrentProcess();
-
+  bool increase_preroll_on_underflow = true;
 
   // Figure out which demuxer to use.
   if (!media_source) {
@@ -1076,6 +1076,12 @@ void WebMediaPlayerImpl::StartPipeline(WebKit::WebMediaSource* media_source) {
     // Disable GpuVideoDecoder creation until it supports codec config changes.
     // TODO(acolwell): Remove this once http://crbug.com/151045 is fixed.
     gpu_factories_ = NULL;
+
+    // Disable preroll increases on underflow since the web application has no
+    // way to detect that this is happening and runs the risk of triggering
+    // unwanted garbage collection if it is to aggressive about appending data.
+    // TODO(acolwell): Remove this once http://crbug.com/144683 is fixed.
+    increase_preroll_on_underflow = false;
   }
 
   scoped_ptr<media::FilterCollection> filter_collection(
@@ -1102,7 +1108,8 @@ void WebMediaPlayerImpl::StartPipeline(WebKit::WebMediaSource* media_source) {
       new media::AudioRendererImpl(media_thread_.message_loop_proxy(),
                                    audio_source_provider_.get(),
                                    audio_decoders.Pass(),
-                                   set_decryptor_ready_cb));
+                                   set_decryptor_ready_cb,
+                                   increase_preroll_on_underflow));
   filter_collection->SetAudioRenderer(audio_renderer.Pass());
 
   // Create our video decoders and renderer.