Update media duration if data is appended after the previous duration

media/filters/chunk_demuxer.cc

Index: media/filters/chunk_demuxer.cc
diff --git a/media/filters/chunk_demuxer.cc b/media/filters/chunk_demuxer.cc
index d90052876629057ed7479e71744afd14efe3ed82..1e3dba35ca5bb73129bd8730420ce753a9593b47 100644
--- a/media/filters/chunk_demuxer.cc
+++ b/media/filters/chunk_demuxer.cc
@@ -761,6 +761,14 @@ bool ChunkDemuxer::AppendData(const std::string& id,
     }
   }
 
+  if (ranges.size() > 0) {
+    base::TimeDelta last_timestamp_buffered = ranges.end(ranges.size() - 1);
+    if (last_timestamp_buffered > duration_) {
+      duration_ = last_timestamp_buffered;
+      host_->SetDuration(last_timestamp_buffered);
+    }
+  }
+
   for (size_t i = 0; i < ranges.size(); ++i)
     host_->AddBufferedTimeRange(ranges.start(i), ranges.end(i));
 
@@ -940,8 +948,6 @@ void ChunkDemuxer::OnStreamParserInitDone(bool success, TimeDelta duration) {
       (!source_id_video_.empty() && !video_))
     return;
 
-  host_->SetDuration(duration_);

[acolwell GONE FROM CHROMIUM, 2012/07/20 21:18:36]

This isn't needed anymore because of the code in OnNewMediaSegment()?

[vrk (LEFT CHROMIUM), 2012/07/26 22:27:26]

Yes. My thought was, I was moving this line to happen right before transitioning
to the INITIALIZED state, which is where it happened prior to my change that
added WAITING_FOR_START_TIME. However, there's no harm in keeping this here, and
that means I don't have to change unit tests :) so I reinstated these lines!

-
   ChangeState_Locked(WAITING_FOR_START_TIME);
 }
 
@@ -1059,6 +1065,9 @@ void ChunkDemuxer::OnNewMediaSegment(const std::string& source_id,
     video_->Seek(start_time_);
   }
 
+  duration_ = std::max(duration_, start_time_);

[acolwell GONE FROM CHROMIUM, 2012/07/20 21:18:36]

A couple questions here:
- Shouldn't the SetDuration() call be conditional on whether duration_ changed
or not? 
- Don't we want to use (start_time - start_time_) here for a duration? If the
first append starts at 10 seconds I don't think we want the duration to be 10
seconds.
- WDYT about creating a UpdateDuration() helper function so duration_ updates
and host_->SetDuration() calls are all in one place?

[vrk (LEFT CHROMIUM), 2012/07/26 22:27:26]

As I said in my other comment, my thinking was I was moving the SetDuration()
call from OnStreamParserInitDone() to here.

But adding a conditional makes sense for 2 reasons: a) unittests require fewer
changes, b) Append()s may occur before we call the init_cb_, and these Append()s
may increase the duration.
I think we run into weirdness when the duration is less than the current time
(i.e. a lot of the code assumes duration == end of file). See bug:
crbug.com/137275

It is unideal/unintuitive logic - clearly the duration is going to be >
start_time_, since start_time_ is where it starts! But I need to set a more
accurate duration before calling the |init_cb_|, and if start_time_ > duration_,
then I believe start_time_ is the most accurate duration we have.
SG! Done.

+  host_->SetDuration(duration_);
+
   // The demuxer is now initialized after the |start_timestamp_| was set.
   ChangeState_Locked(INITIALIZED);
   base::ResetAndReturn(&init_cb_).Run(PIPELINE_OK);