Fix corrupted audio and video at playback start in ogg containers.

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 92c438ea3a7dfea48b5674c50fccf4014aba73a9..dc7c37d51c54f69776dbaa35d34d0fde96423a62 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -84,10 +84,8 @@ static base::TimeDelta ExtractStartTime(AVStream* stream,
 //
 // FFmpegDemuxerStream
 //
-FFmpegDemuxerStream::FFmpegDemuxerStream(
-    FFmpegDemuxer* demuxer,
-    AVStream* stream,
-    bool discard_negative_timestamps)
+FFmpegDemuxerStream::FFmpegDemuxerStream(FFmpegDemuxer* demuxer,
+                                         AVStream* stream)
     : demuxer_(demuxer),
       task_runner_(base::MessageLoopProxy::current()),
       stream_(stream),
@@ -95,7 +93,7 @@ FFmpegDemuxerStream::FFmpegDemuxerStream(
       end_of_stream_(false),
       last_packet_timestamp_(kNoTimestamp()),
       bitstream_converter_enabled_(false),
-      discard_negative_timestamps_(discard_negative_timestamps) {
+      fixup_negative_ogg_timestamps_(false) {
   DCHECK(demuxer_);
 
   bool is_encrypted = false;
@@ -262,11 +260,21 @@ void FFmpegDemuxerStream::EnqueuePacket(ScopedAVPacket packet) {
       ConvertStreamTimestamp(stream_->time_base, packet->pts);
 
   if (stream_timestamp != kNoTimestamp()) {
-    buffer->set_timestamp(stream_timestamp - demuxer_->start_time());
+    // If this is an OGG file with negative timestamps don't rebase any other
+    // stream types against the negative starting time.
+    base::TimeDelta start_time = demuxer_->start_time();
+    if (fixup_negative_ogg_timestamps_ && type() != AUDIO &&
+        start_time < base::TimeDelta()) {
+      DCHECK(stream_timestamp >= base::TimeDelta());
+      start_time = base::TimeDelta();
+    }
+
+    buffer->set_timestamp(stream_timestamp - start_time);
 
     // If enabled, mark packets with negative timestamps for post-decode
     // discard.
-    if (discard_negative_timestamps_ && stream_timestamp < base::TimeDelta()) {
+    if (fixup_negative_ogg_timestamps_ &&
+        stream_timestamp < base::TimeDelta()) {
       if (stream_timestamp + buffer->duration() < base::TimeDelta()) {
         // Discard the entire packet if it's entirely before zero.
         buffer->set_discard_padding(
@@ -502,12 +510,14 @@ void FFmpegDemuxer::Seek(base::TimeDelta time, const PipelineStatusCB& cb) {
   // FFmpeg requires seeks to be adjusted according to the lowest starting time.
   const base::TimeDelta seek_time = time + start_time_;
 
-  // Choose the preferred stream if |seek_time| occurs after its starting time,
-  // otherwise use the fallback stream.
+  // Choose the seeking stream based on whether it contains the seek time, if no
+  // match can be found prefer the preferred stream.
   DCHECK(preferred_stream_for_seeking_.second != kNoTimestamp());
-  const int stream_index = seek_time >= preferred_stream_for_seeking_.second
-                               ? preferred_stream_for_seeking_.first
-                               : fallback_stream_for_seeking_.first;
+  const int stream_index =
+      seek_time < preferred_stream_for_seeking_.second &&
+              seek_time >= fallback_stream_for_seeking_.second
+          ? fallback_stream_for_seeking_.first
+          : preferred_stream_for_seeking_.first;
   DCHECK_NE(stream_index, -1);
 
   const AVStream* seeking_stream =
@@ -680,6 +690,16 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
   AVFormatContext* format_context = glue_->format_context();
   streams_.resize(format_context->nb_streams);
 
+  // FFmpeg doesn't guarantee packets will be in timestamp order across streams

[DaleCurtis, 2014/06/27 21:10:33]

If you decide we should keep this, I can refactor this whole section into a
static method so it's not polluting OnFindStreamInfoDone().

+  // in an ogg container after a seek.  Internally it only considers the file
+  // position when demuxing.  It's an open question whether FFmpeg should fix
+  // this: http://lists.ffmpeg.org/pipermail/ffmpeg-devel/2014-June/159212.html
+  //
+  // As a workaround to ensure playback starts correctly, use the position of
+  // packets demuxed during avformat_find_stream_info() to adjust the seek info.
+  int first_ogg_stream = -1;
+  int64_t first_ogg_packet_pos = std::numeric_limits<int64_t>::max();
+
   // Estimate the start time for each stream by looking through the packets
   // buffered during avformat_find_stream_info().  These values will be
   // considered later when determining the actual stream start time.
@@ -704,6 +724,11 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
         if (packet_pts < start_time_estimates[stream->index])
           start_time_estimates[stream->index] = packet_pts;
       }
+      if (packet_buffer->pkt.pos != -1 &&
+          packet_buffer->pkt.pos < first_ogg_packet_pos) {
+        first_ogg_stream = stream->index;
+        first_ogg_packet_pos = packet_buffer->pkt.pos;
+      }
       packet_buffer = packet_buffer->next;
     }
   }
@@ -722,7 +747,6 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
     AVStream* stream = format_context->streams[i];
     const AVCodecContext* codec_context = stream->codec;
     const AVMediaType codec_type = codec_context->codec_type;
-    bool discard_negative_timestamps = false;
 
     if (codec_type == AVMEDIA_TYPE_AUDIO) {
       if (audio_stream)
@@ -737,13 +761,6 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
       if (!audio_config.IsValidConfig())
         continue;
       audio_stream = stream;
-
-      // Enable post-decode frame dropping for packets with negative timestamps
-      // as outlined in section A.2 in the Ogg Vorbis spec:
-      // http://xiph.org/vorbis/doc/Vorbis_I_spec.html
-      discard_negative_timestamps =
-          audio_config.codec() == kCodecVorbis &&
-          strcmp(glue_->format_context()->iformat->name, "ogg") == 0;
     } else if (codec_type == AVMEDIA_TYPE_VIDEO) {
       if (video_stream)
         continue;
@@ -766,13 +783,21 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
       continue;
     }
 
-    streams_[i] =
-        new FFmpegDemuxerStream(this, stream, discard_negative_timestamps);
+    streams_[i] = new FFmpegDemuxerStream(this, stream);
     max_duration = std::max(max_duration, streams_[i]->duration());
 
     const base::TimeDelta start_time =
         ExtractStartTime(stream, start_time_estimates[i]);
-    if (start_time == kNoTimestamp())
+    const bool has_start_time = start_time != kNoTimestamp();
+
+    // Always prefer the video stream for seeking.  If none exists, we'll swap
+    // the fallback stream with the preferred stream below.
+    if (codec_type == AVMEDIA_TYPE_VIDEO) {
+      preferred_stream_for_seeking_ =
+          StreamSeekInfo(i, has_start_time ? start_time : base::TimeDelta());
+    }
+
+    if (!has_start_time)
       continue;
 
     if (start_time < start_time_) {
@@ -780,13 +805,8 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
 
       // Choose the stream with the lowest starting time as the fallback stream
       // for seeking.  Video should always be preferred.
-      fallback_stream_for_seeking_ = std::make_pair(i, start_time);
+      fallback_stream_for_seeking_ = StreamSeekInfo(i, start_time);
     }
-
-    // Always prefer the video stream for seeking.  If none exists, we'll swap
-    // the fallback stream with the preferred stream below.
-    if (codec_type == AVMEDIA_TYPE_VIDEO)
-      preferred_stream_for_seeking_ = std::make_pair(i, start_time);
   }
 
   if (!audio_stream && !video_stream) {
@@ -809,12 +829,34 @@ void FFmpegDemuxer::OnFindStreamInfoDone(const PipelineStatusCB& status_cb,
     max_duration = kInfiniteDuration();
   }
 
+  // Ogg has some peculiarities around negative timestamps, so use this flag to
+  // setup the FFmpegDemuxerStreams appropriately.
+  //
+  // Post-decode frame dropping for packets with negative timestamps is outlined
+  // in section A.2 in the Ogg Vorbis spec:
+  // http://xiph.org/vorbis/doc/Vorbis_I_spec.html
+  if (strcmp(format_context->iformat->name, "ogg") == 0 && audio_stream &&
+      audio_stream->codec->codec_id == AV_CODEC_ID_VORBIS) {
+    for (size_t i = 0; i < streams_.size(); ++i) {
+      if (streams_[i])
+        streams_[i]->enable_negative_timestamp_fixups_for_ogg();
+    }
+
+    // Fixup the seeking information to avoid selecting the audio stream unless
+    // it has a lower file position.  See notes on |first_ogg_stream|.
+    if (first_ogg_stream != audio_stream->index &&
+        fallback_stream_for_seeking_.first == audio_stream->index &&
+        fallback_stream_for_seeking_.second < base::TimeDelta()) {
+      fallback_stream_for_seeking_.second = base::TimeDelta();
+    }
+  }
+
   // If no start time could be determined, default to zero and prefer the video
   // stream over the audio stream for seeking.  E.g., The WAV demuxer does not
   // put timestamps on its frames.
   if (start_time_ == kInfiniteDuration()) {
     start_time_ = base::TimeDelta();
-    preferred_stream_for_seeking_ = std::make_pair(
+    preferred_stream_for_seeking_ = StreamSeekInfo(
         video_stream ? video_stream->index : audio_stream->index, start_time_);
   } else if (!video_stream) {
     // If no video stream exists, use the audio or text stream found above.