Fire canplaythrough event at the proper time for audio/video

media/base/pipeline_impl.cc

Index: media/base/pipeline_impl.cc
diff --git a/media/base/pipeline_impl.cc b/media/base/pipeline_impl.cc
index e0d82703ac047342ae5147f822070d5daaa99535..d88bdb1a9554e0639a71f6c294cd15387664c7a0 100644
--- a/media/base/pipeline_impl.cc
+++ b/media/base/pipeline_impl.cc
@@ -186,9 +186,8 @@ float PipelineImpl::GetPlaybackRate() const {
 }
 
 void PipelineImpl::SetPlaybackRate(float playback_rate) {
-  if (playback_rate < 0.0f) {
+  if (playback_rate < 0.0f)
     return;
-  }
 
   base::AutoLock auto_lock(lock_);
   playback_rate_ = playback_rate;
@@ -204,9 +203,8 @@ float PipelineImpl::GetVolume() const {
 }
 
 void PipelineImpl::SetVolume(float volume) {
-  if (volume < 0.0f || volume > 1.0f) {
+  if (volume < 0.0f || volume > 1.0f)
     return;
-  }
 
   base::AutoLock auto_lock(lock_);
   volume_ = volume;
@@ -239,6 +237,7 @@ base::TimeDelta PipelineImpl::GetCurrentTime() const {
 }
 
 base::TimeDelta PipelineImpl::GetCurrentTime_Locked() const {
+  lock_.AssertAcquired();
   base::TimeDelta elapsed = clock_->Elapsed();
   if (state_ == kEnded || elapsed > duration_) {
     return duration_;
@@ -258,9 +257,8 @@ base::TimeDelta PipelineImpl::GetBufferedTime() {
   base::TimeDelta current_time = GetCurrentTime_Locked();
 
   // If buffered time was set, we report that value directly.
-  if (buffered_time_.ToInternalValue() > 0) {
+  if (buffered_time_.ToInternalValue() > 0)
     return std::max(buffered_time_, current_time);
-  }
 
   if (total_bytes_ == 0)
     return base::TimeDelta();
@@ -371,6 +369,7 @@ void PipelineImpl::ResetState() {
   waiting_for_clock_update_ = false;
   audio_disabled_   = false;
   clock_->SetTime(kZero);
+  is_downloading_data_ = false;

[acolwell GONE FROM CHROMIUM, 2011/11/09 00:50:22]

Does download_rate_monitor_ need to be reset here?

[vrk (LEFT CHROMIUM), 2011/11/11 02:51:06]

Yes, done.

 }
 
 void PipelineImpl::SetState(State next_state) {
@@ -422,6 +421,13 @@ void PipelineImpl::FinishInitialization() {
     seek_callback_.Run(status_);
     seek_callback_.Reset();
   }
+
+  base::AutoLock auto_lock(lock_);

[acolwell GONE FROM CHROMIUM, 2011/11/09 00:50:22]

This is neaded because the SetXXXBytes() calls are on the Demuxer thread? You
should add a comment indicating why the lock is needed eventhough we are on the
pipeline thread here.

[vrk (LEFT CHROMIUM), 2011/11/11 02:51:06]

Actually the SetXXXBytes method calls (and SetLoaded, and SetNetworkActivity)
originate from BufferedDataSource and so are called on the renderer thread. But
yes, those methods are the reason for the lock.

Though your question leads me to ask: Looking at the code, it appears that the
demuxer runs on the pipeline's thread. Is that not true?
Done.

+  int bitrate = 0;
+  if (demuxer_.get())
+    bitrate = demuxer_->GetBitrate();
+  download_rate_monitor_.Start(
+      base::Bind(&PipelineImpl::OnCanPlayThrough, this), bitrate);
 }
 
 // static
@@ -524,16 +530,17 @@ void PipelineImpl::SetTotalBytes(int64 total_bytes) {
 
   base::AutoLock auto_lock(lock_);
   total_bytes_ = total_bytes;
+  download_rate_monitor_.SetTotalBytes(total_bytes_);
 }
 
 void PipelineImpl::SetBufferedBytes(int64 buffered_bytes) {
   DCHECK(IsRunning());
   base::AutoLock auto_lock(lock_);
-
   // See comments in SetCurrentReadPosition() about capping.
   if (buffered_bytes < current_bytes_)
     current_bytes_ = buffered_bytes;
   buffered_bytes_ = buffered_bytes;
+  download_rate_monitor_.SetBufferedBytes(buffered_bytes, base::Time::Now());
 }
 
 void PipelineImpl::SetNaturalVideoSize(const gfx::Size& size) {
@@ -570,13 +577,25 @@ void PipelineImpl::SetLoaded(bool loaded) {
 
   base::AutoLock auto_lock(lock_);
   loaded_ = loaded;
+  download_rate_monitor_.SetLoaded(loaded_);
 }
 
 void PipelineImpl::SetNetworkActivity(bool is_downloading_data) {
   DCHECK(IsRunning());
+
+  NetworkEvent type = DOWNLOAD_PAUSED;
+  if (is_downloading_data)
+    type = DOWNLOAD_CONTINUED;
+
+  {
+    base::AutoLock auto_lock(lock_);
+    download_rate_monitor_.SetNetworkActivity(is_downloading_data);
+  }
+
   message_loop_->PostTask(FROM_HERE,
       base::Bind(
-          &PipelineImpl::NotifyNetworkEventTask, this, is_downloading_data));
+          &PipelineImpl::NotifyNetworkEventTask, this, type,
+          is_downloading_data));

[acolwell GONE FROM CHROMIUM, 2011/11/09 00:50:22]

Is is_downloading_data still needed given that it's value is encoded in type? We
have the lock above so we could update is_downloading_data_ here.

[vrk (LEFT CHROMIUM), 2011/11/11 02:51:06]

AH oops, you're right! It served a purpose when the canplaythrough logic was
still in this class. Deleted. 
Actually the lock is for download_rate_monitor_.

   media_log_->AddEvent(
       media_log_->CreateBooleanEvent(
           MediaLogEvent::NETWORK_ACTIVITY_SET,
@@ -682,7 +701,6 @@ void PipelineImpl::InitializeTask() {
          state_ == kInitVideoDecoder ||
          state_ == kInitVideoRenderer);
 
-
   // Demuxer created, create audio decoder.
   if (state_ == kInitDemuxer) {
     SetState(kInitAudioDecoder);
@@ -940,10 +958,12 @@ void PipelineImpl::NotifyEndedTask() {
   }
 }
 
-void PipelineImpl::NotifyNetworkEventTask(bool is_downloading_data) {
+void PipelineImpl::NotifyNetworkEventTask(NetworkEvent type,
+                                          bool is_downloading_data) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
+  is_downloading_data_ = is_downloading_data;
   if (!network_callback_.is_null())
-    network_callback_.Run(is_downloading_data);
+    network_callback_.Run(type);
 }
 
 void PipelineImpl::DisableAudioRendererTask() {
@@ -1405,4 +1425,15 @@ void PipelineImpl::OnAudioUnderflow() {
     audio_renderer_->ResumeAfterUnderflow(true);
 }
 
+void PipelineImpl::OnCanPlayThrough() {
+  message_loop_->PostTask(FROM_HERE,
+      base::Bind(
+          &PipelineImpl::NotifyCanPlayThrough, this));

[scherkus (not reviewing), 2011/11/09 02:55:15]

nit: can put this on previous line

[vrk (LEFT CHROMIUM), 2011/11/11 02:51:06]

Done.

+}
+
+void PipelineImpl::NotifyCanPlayThrough() {
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
+  NotifyNetworkEventTask(CAN_PLAY_THROUGH, is_downloading_data_);
+}
+
 }  // namespace media