Switch to using an explicit ended signal instead of time comparison.

media/filters/ffmpeg_demuxer.cc

Index: media/filters/ffmpeg_demuxer.cc
diff --git a/media/filters/ffmpeg_demuxer.cc b/media/filters/ffmpeg_demuxer.cc
index 7bfbf23bf2a64466c57433806fa69eb31393e081..6ab1715804ae6896b830addf2749c68daba832f5 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -573,6 +573,11 @@ void FFmpegDemuxerStream::EnqueuePacket(ScopedAVPacket packet) {
   last_packet_timestamp_ = buffer->timestamp();
   last_packet_duration_ = buffer->duration();
 
+  const base::TimeDelta new_duration =
+      last_packet_timestamp_ + last_packet_duration_;
+  if (new_duration > duration_ || duration_ == kNoTimestamp)
+    duration_ = new_duration;
+
   buffer_queue_.Push(buffer);
   SatisfyPendingRead();
 }
@@ -1437,6 +1442,7 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
   // Good to go: set the duration and bitrate and notify we're done
   // initializing.
   host_->SetDuration(max_duration);
+  duration_ = max_duration;
   duration_known_ = (max_duration != kInfiniteDuration);
 
   int64_t filesize_in_bytes = 0;
@@ -1672,25 +1678,24 @@ void FFmpegDemuxer::OnReadFrameDone(ScopedAVPacket packet, int result) {
   if (result < 0 || IsMaxMemoryUsageReached()) {
     DVLOG(1) << __func__ << " result=" << result
              << " IsMaxMemoryUsageReached=" << IsMaxMemoryUsageReached();
-    // Update the duration based on the highest elapsed time across all streams
-    // if it was previously unknown.
-    if (!duration_known_) {
-      base::TimeDelta max_duration;
-
-      for (const auto& stream : streams_) {
-        if (!stream)
-          continue;
+    // Update the duration based on the highest elapsed time across all streams.
+    base::TimeDelta max_duration;
+    for (const auto& stream : streams_) {
+      if (!stream)
+        continue;
 
-        base::TimeDelta duration = stream->GetElapsedTime();
-        if (duration != kNoTimestamp && duration > max_duration)
-          max_duration = duration;
-      }
+      base::TimeDelta duration =
+          duration_known_ ? stream->duration() : stream->GetElapsedTime();

[chcunningham, 2016/10/24 20:18:53]

why not always pick GetElapsedTime here? Or why not always use
stream->duration()? Should we instead just take the max of these?

+      if (duration != kNoTimestamp && duration > max_duration)
+        max_duration = duration;
+    }
 
-      if (max_duration > base::TimeDelta()) {
-        host_->SetDuration(max_duration);
-        duration_known_ = true;
-      }
+    if (duration_ == kInfiniteDuration || max_duration > duration_) {

[chcunningham, 2016/10/24 20:18:54]

I'm confused about when/why we want to override Inifinity. This logic would
allow duration to go from Infinity -> some finite number. But, when  updating
the duration below (:1729), we guard against this, only updating duration if it
was already "known" (not infinite). 

+      host_->SetDuration(max_duration);
+      duration_known_ = true;
+      duration_ = max_duration;
     }
+
     // If we have reached the end of stream, tell the downstream filters about
     // the event.
     StreamHasEnded();
@@ -1719,6 +1724,15 @@ void FFmpegDemuxer::OnReadFrameDone(ScopedAVPacket packet, int result) {
     FFmpegDemuxerStream* demuxer_stream = streams_[packet->stream_index].get();
     if (demuxer_stream->enabled())
       demuxer_stream->EnqueuePacket(std::move(packet));
+
+    // If duration estimate was incorrect, update it and tell higher layers.
+    if (duration_known_) {
+      const base::TimeDelta duration = demuxer_stream->duration();
+      if (duration != kNoTimestamp && duration > duration_) {
+        duration_ = duration;
+        host_->SetDuration(duration_);
+      }
+    }
   }
 
   // Keep reading until we've reached capacity.