Update duration when demuxed packets exceed known duration.

media/filters/ffmpeg_demuxer.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/filters/ffmpeg_demuxer.h"
 
 #include <algorithm>
 #include <memory>
 #include <set>
 #include <utility>
@@ skipping 548 matching lines @@
       demuxer_->NotifyBufferingChanged();
     }
   }
 
   if (packet.get()->flags & AV_PKT_FLAG_KEY)
     buffer->set_is_key_frame(true);
 
   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();
 }
 
 void FFmpegDemuxerStream::SetEndOfStream() {
   DCHECK(task_runner_->BelongsToCurrentThread());
   end_of_stream_ = true;
   SatisfyPendingRead();
 }
 
@@ skipping 859 matching lines @@
     SetLiveness(DemuxerStream::LIVENESS_LIVE);
   } else if (max_duration != kInfiniteDuration) {
     SetLiveness(DemuxerStream::LIVENESS_RECORDED);
   } else {
     SetLiveness(DemuxerStream::LIVENESS_UNKNOWN);
   }
 
   // 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;
   url_protocol_->GetSize(&filesize_in_bytes);
   bitrate_ = CalculateBitrate(format_context, max_duration, filesize_in_bytes);
   if (bitrate_ > 0)
     data_source_->SetBitrate(bitrate_);
 
   LogMetadata(format_context, max_duration);
   media_tracks_updated_cb_.Run(std::move(media_tracks));
@@ skipping 241 matching lines @@
 
   if (!blocking_thread_.IsRunning() || !pending_seek_cb_.is_null())
     return;
 
   // Consider the stream as ended if:
   // - either underlying ffmpeg returned an error
   // - or FFMpegDemuxer reached the maximum allowed memory usage.
   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;
+    // Update the duration based on the highest elapsed time across all streams.
+    base::TimeDelta max_duration;
+    for (const auto& stream : streams_) {
+      if (!stream)
+        continue;
 
-      for (const auto& stream : streams_) {
-        if (!stream)
-          continue;
+      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;
+    }
 
-        base::TimeDelta duration = stream->GetElapsedTime();
-        if (duration != kNoTimestamp && duration > max_duration)
-          max_duration = duration;
-      }
+    if (duration_ == kInfiniteDuration || max_duration > duration_) {
+      host_->SetDuration(max_duration);
+      duration_known_ = true;
+      duration_ = max_duration;
+    }
 
-      if (max_duration > base::TimeDelta()) {
-        host_->SetDuration(max_duration);
-        duration_known_ = true;
-      }
-    }
     // If we have reached the end of stream, tell the downstream filters about
     // the event.
     StreamHasEnded();
     return;
   }
 
   // Queue the packet with the appropriate stream.
   DCHECK_GE(packet->stream_index, 0);
   DCHECK_LT(packet->stream_index, static_cast<int>(streams_.size()));
 
   // Defend against ffmpeg giving us a bad stream index.
   if (packet->stream_index >= 0 &&
       packet->stream_index < static_cast<int>(streams_.size()) &&
       streams_[packet->stream_index]) {
     // TODO(scherkus): Fix demuxing upstream to never return packets w/o data
     // when av_read_frame() returns success code. See bug comment for ideas:
     //
     // https://code.google.com/p/chromium/issues/detail?id=169133#c10
     if (!packet->data) {
       ScopedAVPacket new_packet(new AVPacket());
       av_new_packet(new_packet.get(), 0);
       av_packet_copy_props(new_packet.get(), packet.get());
       packet.swap(new_packet);
     }
 
     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.
   ReadFrameIfNeeded();
 }
 
 bool FFmpegDemuxer::StreamsHaveAvailableCapacity() {
   DCHECK(task_runner_->BelongsToCurrentThread());
   for (const auto& stream : streams_) {
     if (stream && stream->HasAvailableCapacity())
@@ skipping 42 matching lines @@
 
 void FFmpegDemuxer::SetLiveness(DemuxerStream::Liveness liveness) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   for (const auto& stream : streams_) {
     if (stream)
       stream->SetLiveness(liveness);
   }
 }
 
 }  // namespace media