Update media duration if data is appended after the previous duration

media/base/pipeline.cc

Index: media/base/pipeline.cc
diff --git a/media/base/pipeline.cc b/media/base/pipeline.cc
index 24868cfac2c7941c714459e51b99360297d1df99..ca00baf5e756c8b037f97ac0ceb12be016aa1d7e 100644
--- a/media/base/pipeline.cc
+++ b/media/base/pipeline.cc
@@ -92,14 +92,15 @@ Pipeline::~Pipeline() {
 void Pipeline::Start(scoped_ptr<FilterCollection> collection,
                      const PipelineStatusCB& ended_cb,
                      const PipelineStatusCB& error_cb,
-                     const PipelineStatusCB& start_cb) {
+                     const PipelineStatusCB& start_cb,
+                     const base::Closure& new_duration_cb) {
   base::AutoLock auto_lock(lock_);
   CHECK(!running_) << "Media pipeline is already running";
 
   running_ = true;
   message_loop_->PostTask(FROM_HERE, base::Bind(
       &Pipeline::StartTask, this, base::Passed(&collection),
-      ended_cb, error_cb, start_cb));
+      ended_cb, error_cb, start_cb, new_duration_cb));
 }
 
 void Pipeline::Stop(const base::Closure& stop_cb) {
@@ -419,8 +420,15 @@ void Pipeline::SetDuration(TimeDelta duration) {
           MediaLogEvent::DURATION_SET, "duration", duration));
   UMA_HISTOGRAM_LONG_TIMES("Media.Duration", duration);
 
-  base::AutoLock auto_lock(lock_);
-  clock_->SetDuration(duration);
+  base::TimeDelta old_duration;
+  {
+    base::AutoLock auto_lock(lock_);
+    old_duration = clock_->Duration();

[acolwell GONE FROM CHROMIUM, 2012/07/27 00:03:23]

nit: How about just having a duration_changed bool to control when the
new_duration_cb_ is called and only calling SetDuration() when the duration has
changed? Then SetDuration() & the callback are called under the same
circumstances which seems more consistent to me.

[vrk (LEFT CHROMIUM), 2012/07/28 01:42:05]

No longer necessary after getting rid of new_duration_cb_.

+    clock_->SetDuration(duration);
+  }
+
+  if (old_duration != duration && !new_duration_cb_.is_null())
+    new_duration_cb_.Run();
 }
 
 void Pipeline::SetTotalBytes(int64 total_bytes) {
@@ -580,13 +588,15 @@ void Pipeline::OnUpdateStatistics(const PipelineStatistics& stats) {
 void Pipeline::StartTask(scoped_ptr<FilterCollection> filter_collection,
                          const PipelineStatusCB& ended_cb,
                          const PipelineStatusCB& error_cb,
-                         const PipelineStatusCB& start_cb) {
+                         const PipelineStatusCB& start_cb,
+                         const base::Closure& new_duration_cb) {

[acolwell GONE FROM CHROMIUM, 2012/07/27 00:03:23]

WDYT about just having WebMediaPlayerImpl or even HTMLMediaElement call
duration() before and after a sourceAppend() or sourceEndOfStream() to check for
duration changes? Could that work? I'm just worried about JavaScript visible
inconsistancy between duration, HTMLMediaElement.buffered, and
SourceBuffer.buffered since this callback requires a thread hop. If no
inconsistency is visible then I'm fine with the callback.

[vrk (LEFT CHROMIUM), 2012/07/28 01:42:05]

Done (in WebMediaPlayerImpl) and I agree, this is superior! I didn't feel good
about adding this callback so I'm glad I can remove it!

   DCHECK(message_loop_->BelongsToCurrentThread());
   DCHECK_EQ(kCreated, state_);
   filter_collection_ = filter_collection.Pass();
   ended_cb_ = ended_cb;
   error_cb_ = error_cb;
   seek_cb_ = start_cb;
+  new_duration_cb_ = new_duration_cb;
 
   // Kick off initialization.
   pipeline_init_state_.reset(new PipelineInitState());