Update duration when demuxed packets exceed known duration.

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 7d1fcea9835ee2ab4eb576fc146486f02fe7afe8..c46effad8e6793653222212df64795f899b7161a 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -566,6 +566,10 @@ void FFmpegDemuxerStream::EnqueuePacket(ScopedAVPacket packet) {
   last_packet_timestamp_ = buffer->timestamp();
   last_packet_duration_ = buffer->duration();
 
+  const base::TimeDelta new_duration = last_packet_timestamp_;

[chcunningham, 2017/01/23 19:14:10]

Should this be last_packet_timestamp_ + last_packet_duration_?

[DaleCurtis, 2017/01/24 02:05:19]

Adding duration explodes a lot of stuff in our tests and on all the real world
content in my vault; it appears to be frequently inaccurate for the last packet
in the stream. E.g., bear-320x240.webm will increase in duration, due to the
audio stream, from 2744ms to 2767ms, but the actual audio stream is truly 2744ms
(verified by inspection of wave data). As such I think we should leave it as is.

+  if (new_duration > duration_ || duration_ == kNoTimestamp)
+    duration_ = new_duration;
+
   buffer_queue_.Push(buffer);
   SatisfyPendingRead();
 }
@@ -1445,6 +1449,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;
@@ -1706,25 +1711,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, 2017/01/23 19:14:10]

It seems like now that stream->duration() is automatically updating, we should
be able to get rid of stream->GetElapsedTime(). IIUC, GetElapsedTime is only
being used when reading the first frame. Could we just make duration() do the
behavior of GetElapsedTime() whenever FFmpegDemxuerStream::duration_ is
kNoTimestamp?

[DaleCurtis, 2017/01/24 02:05:19]

This fails a couple tests, so I'll take a closer look. Seems reasonable though.

+      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_) {
+      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();
@@ -1753,6 +1757,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.