Move underflow threshold limits out of the video renderer.

media/renderers/renderer_impl.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/renderers/renderer_impl.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/callback_helpers.h"
+#include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
+#include "base/strings/string_number_conversions.h"
 #include "media/base/audio_renderer.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/demuxer_stream_provider.h"
+#include "media/base/media_switches.h"
 #include "media/base/time_source.h"
 #include "media/base/video_renderer.h"
 #include "media/base/wall_clock_time_source.h"
 
 namespace media {
 
 RendererImpl::RendererImpl(
     const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
     scoped_ptr<AudioRenderer> audio_renderer,
     scoped_ptr<VideoRenderer> video_renderer)
     : state_(STATE_UNINITIALIZED),
       task_runner_(task_runner),
       audio_renderer_(audio_renderer.Pass()),
       video_renderer_(video_renderer.Pass()),
       time_source_(NULL),
       time_ticking_(false),
       audio_buffering_state_(BUFFERING_HAVE_NOTHING),
       video_buffering_state_(BUFFERING_HAVE_NOTHING),
       audio_ended_(false),
       video_ended_(false),
       cdm_context_(nullptr),
       underflow_disabled_for_testing_(false),
       clockless_video_playback_enabled_for_testing_(false),
+      video_underflow_threshold_(base::TimeDelta::FromSeconds(3)),

[xhwang, 2015/03/26 21:24:43]

You can still use a file scope const instead of a hard coded value here.

[DaleCurtis, 2015/03/26 21:36:08]

This allows overriding for testing much more easily.

[xhwang, 2015/03/28 00:32:40]

You can declare this in the header file so it's available for the test as
well....

       weak_factory_(this) {
   weak_this_ = weak_factory_.GetWeakPtr();
   DVLOG(1) << __FUNCTION__;
+
+  // TODO(dalecurtis): Remove once experiments for http://crbug.com/470940 are
+  // complete.
+  int threshold_ms = 0;
+  std::string threshold_ms_str(
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          switches::kVideoUnderflowThresholdMs));
+  if (base::StringToInt(threshold_ms_str, &threshold_ms) && threshold_ms > 0) {

[xhwang, 2015/03/26 21:24:43]

s/>/>=?

We should be able to set the threshold to 0 to underflow immediately.

+    video_underflow_threshold_ =
+        base::TimeDelta::FromMilliseconds(threshold_ms);
+  }
 }
 
 RendererImpl::~RendererImpl() {
   DVLOG(1) << __FUNCTION__;
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   // Tear down in opposite order of construction as |video_renderer_| can still
   // need |time_source_| (which can be |audio_renderer_|) to be alive.
   video_renderer_.reset();
   audio_renderer_.reset();
@@ skipping 277 matching lines @@
 
   DCHECK(!init_cb_.is_null());
 
   if (status != PIPELINE_OK) {
     base::ResetAndReturn(&init_cb_).Run(status);
     return;
   }
 
   if (audio_renderer_) {
     time_source_ = audio_renderer_->GetTimeSource();
-  } else {
+  } else if (!time_source_) {

[xhwang, 2015/03/26 21:24:43]

hmm, why we need this?

[DaleCurtis, 2015/03/26 21:36:08]

Because I'm overriding the TimeSource for testing, without this the wall clock
time source would be used instead of the test source.

[xhwang, 2015/03/28 00:32:40]

Thanks for the explanation. Maybe worth a comment.

BTW, can we also override the time source when we have audio? That could make
the logic more general/clear. Not sure whether there's any real value though ;)

     wall_clock_time_source_.reset(new WallClockTimeSource());
     time_source_ = wall_clock_time_source_.get();
   }
 
   state_ = STATE_PLAYING;
   DCHECK(time_source_);
   DCHECK(audio_renderer_ || video_renderer_);
   base::ResetAndReturn(&init_cb_).Run(PIPELINE_OK);
 }
 
@@ skipping 62 matching lines @@
   base::ResetAndReturn(&flush_cb_).Run();
 }
 
 void RendererImpl::OnUpdateStatistics(const PipelineStatistics& stats) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   statistics_cb_.Run(stats);
 }
 
 void RendererImpl::OnBufferingStateChanged(BufferingState* buffering_state,
                                            BufferingState new_buffering_state) {
+  const bool is_audio = buffering_state == &audio_buffering_state_;
   DVLOG(1) << __FUNCTION__ << "(" << *buffering_state << ", "
-           << new_buffering_state << ") "
-           << (buffering_state == &audio_buffering_state_ ? "audio" : "video");
+           << new_buffering_state << ") " << (is_audio ? "audio" : "video");
   DCHECK(task_runner_->BelongsToCurrentThread());
+
   bool was_waiting_for_enough_data = WaitingForEnoughData();
 
+  // When audio is present, defer underflow callbacks for some time to avoid
+  // unnecessary glitches in audio; see http://crbug.com/144683#c53.
+  if (audio_renderer_ && !is_audio && state_ == STATE_PLAYING) {
+    if (video_buffering_state_ == BUFFERING_HAVE_ENOUGH &&
+        new_buffering_state == BUFFERING_HAVE_NOTHING &&
+        deferred_underflow_cb_.IsCancelled()) {
+      deferred_underflow_cb_.Reset(base::Bind(
+          &RendererImpl::OnBufferingStateChanged, weak_factory_.GetWeakPtr(),
+          buffering_state, new_buffering_state));
+      task_runner_->PostDelayedTask(FROM_HERE,
+                                    deferred_underflow_cb_.callback(),
+                                    video_underflow_threshold_);
+      return;
+    }
+
+    deferred_underflow_cb_.Cancel();
+  } else if (!deferred_underflow_cb_.IsCancelled() && is_audio &&
+             new_buffering_state == BUFFERING_HAVE_NOTHING) {
+    // If audio underflows while we have a deferred video underflow in progress
+    // we want to mark video as underflowed immediately and cancel the deferral.
+    deferred_underflow_cb_.Cancel();
+    video_buffering_state_ = BUFFERING_HAVE_NOTHING;
+  }
+
   *buffering_state = new_buffering_state;
 
   // Disable underflow by ignoring updates that renderers have ran out of data.
   if (state_ == STATE_PLAYING && underflow_disabled_for_testing_ &&
       time_ticking_) {
     DVLOG(1) << "Update ignored because underflow is disabled for testing.";
     return;
   }
 
   // Renderer underflowed.
@@ skipping 133 matching lines @@
   }
 
   // After OnError() returns, the pipeline may destroy |this|.
   base::ResetAndReturn(&error_cb_).Run(error);
 
   if (!flush_cb_.is_null())
     base::ResetAndReturn(&flush_cb_).Run();
 }
 
 }  // namespace media