Move underflow threshold limits out of the video renderer.

media/renderers/renderer_impl.cc

Index: media/renderers/renderer_impl.cc
diff --git a/media/renderers/renderer_impl.cc b/media/renderers/renderer_impl.cc
index e81c97aec4996f8904ca330c128dfa984851982e..c9a851a76a1e658eb480345739be748d2bacb6c4 100644
--- a/media/renderers/renderer_impl.cc
+++ b/media/renderers/renderer_impl.cc
@@ -7,12 +7,15 @@
 #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"
@@ -36,9 +39,21 @@ RendererImpl::RendererImpl(
       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() {
@@ -336,7 +351,7 @@ void RendererImpl::OnVideoRendererInitializeDone(PipelineStatus status) {
 
   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();
   }
@@ -419,12 +434,37 @@ void RendererImpl::OnUpdateStatistics(const PipelineStatistics& 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.