Fix incorrect opus seek preroll and flaky pre-skip removal.

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 "base/base64.h"
 #include "base/bind.h"
@@ skipping 76 matching lines @@
                                          AVStream* stream)
     : demuxer_(demuxer),
       task_runner_(base::ThreadTaskRunnerHandle::Get()),
       stream_(stream),
       type_(UNKNOWN),
       liveness_(LIVENESS_UNKNOWN),
       end_of_stream_(false),
       last_packet_timestamp_(kNoTimestamp()),
       last_packet_duration_(kNoTimestamp()),
       video_rotation_(VIDEO_ROTATION_0),
-      fixup_negative_ogg_timestamps_(false) {
+      fixup_negative_timestamps_(false) {
   DCHECK(demuxer_);
 
   bool is_encrypted = false;
   int rotation = 0;
   AVDictionaryEntry* rotation_entry = NULL;
 
   // Determine our media format.
   switch (stream->codec->codec_type) {
     case AVMEDIA_TYPE_AUDIO:
       type_ = AUDIO;
@@ skipping 179 matching lines @@
     buffer->set_duration(kNoTimestamp());
   }
 
   // Note: If pts is AV_NOPTS_VALUE, stream_timestamp will be kNoTimestamp().
   const base::TimeDelta stream_timestamp =
       ConvertStreamTimestamp(stream_->time_base, packet->pts);
 
   if (stream_timestamp != kNoTimestamp()) {
     const bool is_audio = type() == AUDIO;
 
-    // If this is an OGG file with negative timestamps don't rebase any other
-    // stream types against the negative starting time.
+    // If this file has 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_ && !is_audio &&
+    if (fixup_negative_timestamps_ && !is_audio &&
         start_time < base::TimeDelta()) {
       start_time = base::TimeDelta();
     }
 
     // Don't rebase timestamps for positive start times, the HTML Media Spec
     // details this in section "4.8.10.6 Offsets into the media resource." We
     // will still need to rebase timestamps before seeking with FFmpeg though.
     if (start_time > base::TimeDelta())
       start_time = base::TimeDelta();
 
     buffer->set_timestamp(stream_timestamp - start_time);
 
-    // If enabled, mark audio packets with negative timestamps for post-decode
-    // discard.
-    if (fixup_negative_ogg_timestamps_ && is_audio &&
+    // If enabled, and no codec delay is present, mark audio packets with
+    // negative timestamps for post-decode discard.
+    if (fixup_negative_timestamps_ && is_audio &&
         stream_timestamp < base::TimeDelta() &&
         buffer->duration() != kNoTimestamp()) {
-      if (stream_timestamp + buffer->duration() < base::TimeDelta()) {
-        // Discard the entire packet if it's entirely before zero.
-        buffer->set_discard_padding(
-            std::make_pair(kInfiniteDuration(), base::TimeDelta()));
+      if (!stream_->codec->delay) {
+        DCHECK_EQ(buffer->discard_padding().first, base::TimeDelta());
+
+        if (stream_timestamp + buffer->duration() < base::TimeDelta()) {
+          DCHECK_EQ(buffer->discard_padding().second, base::TimeDelta());
+
+          // Discard the entire packet if it's entirely before zero.
+          buffer->set_discard_padding(
+              std::make_pair(kInfiniteDuration(), base::TimeDelta()));
+        } else {
+          // Only discard part of the frame if it overlaps zero.
+          buffer->set_discard_padding(std::make_pair(
+              -stream_timestamp, buffer->discard_padding().second));
+        }
       } else {
-        // Only discard part of the frame if it overlaps zero.
-        buffer->set_discard_padding(
-            std::make_pair(-stream_timestamp, base::TimeDelta()));
+        // Verify that codec delay would cover discard and that we don't need to
+        // mark the packet for post decode discard.  Since timestamps may be in
+        // milliseconds and codec delay in nanosecond precision, round up to the
+        // nearest millisecond.  See enable_negative_timestamp_fixups().
+        DCHECK_LE(-std::ceil(FramesToTimeDelta(
+                                 audio_decoder_config().codec_delay(),
+                                 audio_decoder_config().samples_per_second())
+                                 .InMillisecondsF()),
+                  stream_timestamp.InMillisecondsF());
       }
     }
   } else {
     // If this happens on the first packet, decoders will throw an error.
     buffer->set_timestamp(kNoTimestamp());
   }
 
   if (last_packet_timestamp_ != kNoTimestamp()) {
     // FFmpeg doesn't support chained ogg correctly.  Instead of guaranteeing
     // continuity across links in the chain it uses the timestamp information
     // from each link directly.  Doing so can lead to timestamps which appear to
     // go backwards in time.
     //
     // If the new link starts with a negative timestamp or a timestamp less than
     // the original (positive) |start_time|, we will get a negative timestamp
     // here.  It's also possible FFmpeg returns kNoTimestamp() here if it's not
     // able to work out a timestamp using the previous link and the next.
     //
     // Fixing chained ogg is non-trivial, so for now just reuse the last good
     // timestamp.  The decoder will rewrite the timestamps to be sample accurate
     // later.  See http://crbug.com/396864.
-    if (fixup_negative_ogg_timestamps_ &&
+    if (fixup_negative_timestamps_ &&
         (buffer->timestamp() == kNoTimestamp() ||
          buffer->timestamp() < last_packet_timestamp_)) {
       buffer->set_timestamp(last_packet_timestamp_ +
                             (last_packet_duration_ != kNoTimestamp()
                                  ? last_packet_duration_
                                  : base::TimeDelta::FromMicroseconds(1)));
     }
 
     // The demuxer should always output positive timestamps.
     DCHECK(buffer->timestamp() >= base::TimeDelta());
@@ skipping 263 matching lines @@
   // otherwise we can end up waiting for a pre-seek read to complete even though
   // we know we're going to drop it on the floor.
 
   // FFmpeg requires seeks to be adjusted according to the lowest starting time.
   // Since EnqueuePacket() rebased negative timestamps by the start time, we
   // must correct the shift here.
   //
   // Additionally, to workaround limitations in how we expose seekable ranges to
   // Blink (http://crbug.com/137275), we also want to clamp seeks before the
   // start time to the start time.
-  const base::TimeDelta seek_time =
-      start_time_ < base::TimeDelta() ? time + start_time_
-                                      : time < start_time_ ? start_time_ : time;
+  base::TimeDelta seek_time = start_time_ < base::TimeDelta()
+                                  ? time + start_time_
+                                  : time < start_time_ ? start_time_ : time;
+
+  // When seeking in an opus stream we need to ensure we deliver enough data to
+  // satisfy the seek preroll; otherwise the audio at the actual seek time will
+  // not be entirely accurate.
+  FFmpegDemuxerStream* audio_stream = GetFFmpegStream(DemuxerStream::AUDIO);
+  if (audio_stream) {
+    const AudioDecoderConfig& config = audio_stream->audio_decoder_config();
+    if (config.codec() == kCodecOpus)
+      seek_time = std::max(start_time_, seek_time - config.seek_preroll());
+  }
 
   // Choose the seeking stream based on whether it contains the seek time, if no
   // match can be found prefer the preferred stream.
   //
   // TODO(dalecurtis): Currently FFmpeg does not ensure that all streams in a
   // given container will demux all packets after the seek point.  Instead it
   // only guarantees that all packets after the file position of the seek will
   // be demuxed.  It's an open question whether FFmpeg should fix this:
   // http://lists.ffmpeg.org/pipermail/ffmpeg-devel/2014-June/159212.html
   // Tracked by http://crbug.com/387996.
@@ skipping 309 matching lines @@
     // maximum between it and the duration from A/V streams.
     const AVRational av_time_base = {1, AV_TIME_BASE};
     max_duration =
         std::max(max_duration,
                  ConvertFromTimeBase(av_time_base, format_context->duration));
   } else {
     // The duration is unknown, in which case this is likely a live stream.
     max_duration = kInfiniteDuration();
   }
 
-  // Ogg has some peculiarities around negative timestamps, so use this flag to
-  // setup the FFmpegDemuxerStreams appropriately.
+  // FFmpeg represents audio data marked as before the beginning of stream as
+  // having negative timestamps.  This data must be discarded after it has been
+  // decoded, not before since it is used to warmup the decoder.  There are
+  // currently two known cases for this: vorbis in ogg and opus in ogg and webm.
+  //
+  // For API clarity, it was decided that the rest of the media pipeline should
+  // not be exposed to negative timestamps.  Which means we need to rebase these
+  // negative timestamps and mark them for discard post decoding.
   //
   // 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) {
+  //
+  // FFmpeg's use of negative timestamps for opus pre-skip is nonstandard, but
+  // for more information on pre-skip see section 4.2 of the Ogg Opus spec:
+  // https://tools.ietf.org/html/draft-ietf-codec-oggopus-08#section-4.2
+  if (audio_stream && (audio_stream->codec->codec_id == AV_CODEC_ID_OPUS ||
+                       (strcmp(format_context->iformat->name, "ogg") == 0 &&
+                        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();
+        streams_[i]->enable_negative_timestamp_fixups();
     }
 
     // Fixup the seeking information to avoid selecting the audio stream simply
     // because it has a lower starting time.
     if (fallback_stream_for_seeking_.first == audio_stream->index &&
         fallback_stream_for_seeking_.second < base::TimeDelta()) {
       fallback_stream_for_seeking_.second = base::TimeDelta();
     }
   }
 
@@ skipping 213 matching lines @@
     // 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);
     }
 
-    // Special case for opus in ogg.  FFmpeg is pre-trimming the codec delay
-    // from the packet timestamp.  Chrome expects to handle this itself inside
-    // the decoder, so shift timestamps by the delay in this case.
-    // TODO(dalecurtis): Try to get fixed upstream.  See http://crbug.com/328207
-    if (strcmp(glue_->format_context()->iformat->name, "ogg") == 0) {
-      const AVCodecContext* codec_context =
-          glue_->format_context()->streams[packet->stream_index]->codec;
-      if (codec_context->codec_id == AV_CODEC_ID_OPUS &&
-          codec_context->delay > 0) {
-        packet->pts += codec_context->delay;
-      }
-    }
-
     FFmpegDemuxerStream* demuxer_stream = streams_[packet->stream_index];
     demuxer_stream->EnqueuePacket(packet.Pass());
   }
 
   // Keep reading until we've reached capacity.
   ReadFrameIfNeeded();
 }
 
 bool FFmpegDemuxer::StreamsHaveAvailableCapacity() {
   DCHECK(task_runner_->BelongsToCurrentThread());
@@ skipping 73 matching lines @@
 
 void FFmpegDemuxer::SetLiveness(DemuxerStream::Liveness liveness) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   for (const auto& stream : streams_) {  // |stream| is a ref to a pointer.
     if (stream)
       stream->SetLiveness(liveness);
   }
 }
 
 }  // namespace media