Replace Pipeline::kEnded state and HasEnded() methods with renderer-specific bools.

media/base/pipeline.cc

Index: media/base/pipeline.cc
diff --git a/media/base/pipeline.cc b/media/base/pipeline.cc
index 470f1868e734b483ea57375a7cd4fad05735712b..89d8a90a94eb59c0ededb953efa79686c3c46a8f 100644
--- a/media/base/pipeline.cc
+++ b/media/base/pipeline.cc
@@ -88,6 +88,8 @@ Pipeline::Pipeline(MessageLoop* message_loop, MediaLog* media_log)
       has_video_(false),
       state_(kCreated),
       seek_timestamp_(kNoTimestamp()),
+      audio_ended_(false),
+      video_ended_(false),
       audio_disabled_(false),
       creation_time_(base::Time::Now()) {
   media_log_->AddEvent(media_log_->CreatePipelineStateChangedEvent(kCreated));
@@ -151,7 +153,6 @@ bool Pipeline::IsInitialized() const {
     case kSeeking:
     case kStarting:
     case kStarted:
-    case kEnded:
       return true;
     default:
       return false;
@@ -527,10 +528,17 @@ void Pipeline::OnNaturalVideoSizeChanged(const gfx::Size& size) {
   natural_size_ = size;
 }
 
-void Pipeline::OnRendererEnded() {
-  DCHECK(IsRunning());
+void Pipeline::OnAudioRendererEnded() {
+  // Force post to process ended messages after current execution frame.
   message_loop_->PostTask(FROM_HERE, base::Bind(
-      &Pipeline::OnRendererEndedTask, this));
+      &Pipeline::DoAudioRendererEnded, this));
+  media_log_->AddEvent(media_log_->CreateEvent(MediaLogEvent::ENDED));

[Ami GONE FROM CHROMIUM, 2012/08/08 21:48:26]

Why is it useful to media_log this twice?
Alternatively, should each event be tagged w/ an audio/video  bit?

[scherkus (not reviewing), 2012/08/08 22:28:34]

Done by dup'ing the the events (I find adding a param to an event doesn't
represent itself as nicely inside chrome://media-internals)

+}
+
+void Pipeline::OnVideoRendererEnded() {
+  // Force post to process ended messages after current execution frame.
+  message_loop_->PostTask(FROM_HERE, base::Bind(
+      &Pipeline::DoVideoRendererEnded, this));
   media_log_->AddEvent(media_log_->CreateEvent(MediaLogEvent::ENDED));
 }
 
@@ -796,7 +804,7 @@ void Pipeline::SeekTask(TimeDelta time, const PipelineStatusCB& seek_cb) {
   DCHECK(!IsPipelineStopPending());
 
   // Suppress seeking if we're not fully started.
-  if (state_ != kStarted && state_ != kEnded) {
+  if (state_ != kStarted) {
     // TODO(scherkus): should we run the callback?  I'm tempted to say the API
     // will only execute the first Seek() request.
     DVLOG(1) << "Media pipeline has not started, ignoring seek to "
@@ -809,12 +817,14 @@ void Pipeline::SeekTask(TimeDelta time, const PipelineStatusCB& seek_cb) {
 
   // We'll need to pause every filter before seeking.  The state transition
   // is as follows:
-  //   kStarted/kEnded
+  //   kStarted
   //   kPausing (for each filter)
   //   kSeeking (for each filter)
   //   kStarting (for each filter)
   //   kStarted
   SetState(kPausing);
+  audio_ended_ = false;

[scherkus (not reviewing), 2012/08/08 20:59:58]

FYI I think this is the only spot where we need to reset these but worth sanity
checking

+  video_ended_ = false;
   seek_timestamp_ = std::max(time, demuxer_->GetStartTime());
   seek_cb_ = seek_cb;
 
@@ -827,35 +837,46 @@ void Pipeline::SeekTask(TimeDelta time, const PipelineStatusCB& seek_cb) {
   DoPause(base::Bind(&Pipeline::OnFilterStateTransition, this));
 }
 
-void Pipeline::OnRendererEndedTask() {
+void Pipeline::DoAudioRendererEnded() {
   DCHECK(message_loop_->BelongsToCurrentThread());
 
-  // We can only end if we were actually playing.
-  if (state_ != kStarted) {
+  if (state_ != kStarted)
     return;
-  }
 
-  DCHECK(audio_renderer_ || video_renderer_);
+  DCHECK(!audio_ended_);
+  audio_ended_ = true;
 
-  // Make sure every extant renderer has ended.
-  if (audio_renderer_ && !audio_disabled_) {
-    if (!audio_renderer_->HasEnded()) {
-      return;
-    }
-
-    // Start clock since there is no more audio to
-    // trigger clock updates.
+  // Start clock since there is no more audio to trigger clock updates.
+  if (!audio_disabled_) {
     base::AutoLock auto_lock(lock_);
     clock_->SetMaxTime(clock_->Duration());
     StartClockIfWaitingForTimeUpdate_Locked();
   }
 
-  if (video_renderer_ && !video_renderer_->HasEnded()) {
+  RunEndedCallbackIfNeeded();
+}
+
+void Pipeline::DoVideoRendererEnded() {
+  DCHECK(message_loop_->BelongsToCurrentThread());
+
+  if (state_ != kStarted)
+    return;
+
+  DCHECK(!video_ended_);
+  video_ended_ = true;
+
+  RunEndedCallbackIfNeeded();
+}
+
+void Pipeline::RunEndedCallbackIfNeeded() {
+  DCHECK(message_loop_->BelongsToCurrentThread());
+
+  if (audio_renderer_ && !audio_ended_ && !audio_disabled_)
+    return;
+
+  if (video_renderer_ && !video_ended_)
     return;
-  }
 
-  // Transition to ended, executing the callback if present.
-  SetState(kEnded);
   {
     base::AutoLock auto_lock(lock_);
     clock_->EndOfStream();
@@ -990,7 +1011,6 @@ void Pipeline::TeardownStateTransitionTask() {
     case kStarting:
     case kStopped:
     case kStarted:
-    case kEnded:
       NOTREACHED() << "Unexpected state for teardown: " << state_;
       break;
     // default: intentionally left out to force new states to cause compiler
@@ -1132,7 +1152,7 @@ bool Pipeline::InitializeAudioRenderer(
       base::Bind(&Pipeline::OnFilterInitialize, this),
       base::Bind(&Pipeline::OnAudioUnderflow, this),
       base::Bind(&Pipeline::OnAudioTimeUpdate, this),
-      base::Bind(&Pipeline::OnRendererEnded, this),
+      base::Bind(&Pipeline::OnAudioRendererEnded, this),
       base::Bind(&Pipeline::OnAudioDisabled, this),
       base::Bind(&Pipeline::SetError, this));
   return true;
@@ -1158,7 +1178,7 @@ bool Pipeline::InitializeVideoRenderer(
       base::Bind(&Pipeline::OnUpdateStatistics, this),
       base::Bind(&Pipeline::OnVideoTimeUpdate, this),
       base::Bind(&Pipeline::OnNaturalVideoSizeChanged, this),
-      base::Bind(&Pipeline::OnRendererEnded, this),
+      base::Bind(&Pipeline::OnVideoRendererEnded, this),
       base::Bind(&Pipeline::SetError, this),
       base::Bind(&Pipeline::GetMediaTime, this),
       base::Bind(&Pipeline::GetMediaDuration, this));
@@ -1219,7 +1239,6 @@ void Pipeline::TearDownPipeline() {
       break;
 
     case kStarted:
-    case kEnded:
       SetState(kPausing);
       DoPause(base::Bind(&Pipeline::OnTeardownStateTransition, this));
       break;