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

media/base/pipeline.cc

 // Copyright (c) 2012 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/base/pipeline.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 70 matching lines @@
       volume_(1.0f),
       playback_rate_(0.0f),
       pending_playback_rate_(0.0f),
       clock_(new Clock(&base::Time::Now)),
       waiting_for_clock_update_(false),
       status_(PIPELINE_OK),
       has_audio_(false),
       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));
   media_log_->AddEvent(
       media_log_->CreateEvent(MediaLogEvent::PIPELINE_CREATED));
 }
 
 Pipeline::~Pipeline() {
   base::AutoLock auto_lock(lock_);
   DCHECK(!running_) << "Stop() must complete before destroying object";
@@ skipping 43 matching lines @@
   // TODO(scherkus): perhaps replace this with a bool that is set/get under the
   // lock, because this is breaching the contract that |state_| is only accessed
   // on |message_loop_|.
   base::AutoLock auto_lock(lock_);
   switch (state_) {
     case kPausing:
     case kFlushing:
     case kSeeking:
     case kStarting:
     case kStarted:
-    case kEnded:
       return true;
     default:
       return false;
   }
 }
 
 bool Pipeline::HasAudio() const {
   base::AutoLock auto_lock(lock_);
   return has_audio_;
 }
@@ skipping 355 matching lines @@
 
 void Pipeline::OnNaturalVideoSizeChanged(const gfx::Size& size) {
   DCHECK(IsRunning());
   media_log_->AddEvent(media_log_->CreateVideoSizeSetEvent(
       size.width(), size.height()));
 
   base::AutoLock auto_lock(lock_);
   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));
+}
+
 // Called from any thread.
 void Pipeline::OnFilterInitialize(PipelineStatus status) {
   // Continue the initialize task by proceeding to the next stage.
   message_loop_->PostTask(FROM_HERE, base::Bind(
       &Pipeline::InitializeTask, this, status));
 }
 
 // Called from any thread.
 // This method makes the PipelineStatusCB behave like a Closure. It
 // makes it look like a host()->SetError() call followed by a call to
@@ skipping 242 matching lines @@
 
   if (audio_renderer_)
     audio_renderer_->SetVolume(volume);
 }
 
 void Pipeline::SeekTask(TimeDelta time, const PipelineStatusCB& seek_cb) {
   DCHECK(message_loop_->BelongsToCurrentThread());
   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 "
              << time.InMicroseconds() << " (current state: " << state_ << ")";
     return;
   }
 
   DCHECK(!seek_pending_);
   seek_pending_ = true;
 
   // 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;
 
   // Kick off seeking!
   {
     base::AutoLock auto_lock(lock_);
     if (clock_->IsPlaying())
       clock_->Pause();
   }
   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;
-  }
 
-  // Transition to ended, executing the callback if present.
-  SetState(kEnded);
+  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;
+
   {
     base::AutoLock auto_lock(lock_);
     clock_->EndOfStream();
   }
 
   ReportStatus(ended_cb_, status_);
 }
 
 void Pipeline::AudioDisabledTask() {
   DCHECK(message_loop_->BelongsToCurrentThread());
@@ skipping 114 matching lines @@
     case kError:
     case kInitDemuxer:
     case kInitAudioDecoder:
     case kInitAudioRenderer:
     case kInitVideoDecoder:
     case kInitVideoRenderer:
     case kSeeking:
     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
     // errors.
   };
 }
 
 void Pipeline::FinishDestroyingFiltersTask() {
   DCHECK(message_loop_->BelongsToCurrentThread());
   DCHECK(IsPipelineStopped());
@@ skipping 121 matching lines @@
   if (!audio_renderer_) {
     SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING);
     return false;
   }
 
   audio_renderer_->Initialize(
       decoder,
       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;
 }
 
 bool Pipeline::InitializeVideoRenderer(
     const scoped_refptr<VideoDecoder>& decoder) {
   DCHECK(message_loop_->BelongsToCurrentThread());
   DCHECK(IsPipelineOk());
 
   if (!decoder)
     return false;
 
   filter_collection_->SelectVideoRenderer(&video_renderer_);
   if (!video_renderer_) {
     SetError(PIPELINE_ERROR_REQUIRED_FILTER_MISSING);
     return false;
   }
 
   video_renderer_->Initialize(
       decoder,
       base::Bind(&Pipeline::OnFilterInitialize, this),
       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));
   return true;
 }
 
 void Pipeline::TearDownPipeline() {
   DCHECK(message_loop_->BelongsToCurrentThread());
   DCHECK_NE(kStopped, state_);
 
@@ skipping 40 matching lines @@
       DoStop(base::Bind(&Pipeline::OnTeardownStateTransition, this));
 
       if (seek_pending_) {
         seek_pending_ = false;
         FinishInitialization();
       }
 
       break;
 
     case kStarted:
-    case kEnded:
       SetState(kPausing);
       DoPause(base::Bind(&Pipeline::OnTeardownStateTransition, this));
       break;
 
     case kStopping:
     case kStopped:
       NOTREACHED() << "Unexpected state for teardown: " << state_;
       break;
     // default: intentionally left out to force new states to cause compiler
     // errors.
@@ skipping 42 matching lines @@
 void Pipeline::StartClockIfWaitingForTimeUpdate_Locked() {
   lock_.AssertAcquired();
   if (!waiting_for_clock_update_)
     return;
 
   waiting_for_clock_update_ = false;
   clock_->Play();
 }
 
 }  // namespace media