Fix seeking when the start time is non-zero.

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 <string>
 
 #include "base/base64.h"
@@ skipping 37 matching lines @@
   }
 
   return base::Time();
 }
 
 static base::TimeDelta FramesToTimeDelta(int frames, double sample_rate) {
   return base::TimeDelta::FromMicroseconds(
       frames * base::Time::kMicrosecondsPerSecond / sample_rate);
 }
 
+static base::TimeDelta ExtractStartTime(AVStream* stream) {
+  if (stream->start_time == static_cast<int64_t>(AV_NOPTS_VALUE))
+    return kNoTimestamp();
+
+  // First try to use the |start_time| value directly.
+  const base::TimeDelta start_time =
+      ConvertFromTimeBase(stream->time_base, stream->start_time);
+
+  // Then compare against the first timestamp to see if adjustment is required.
+  if (stream->first_dts == static_cast<int64_t>(AV_NOPTS_VALUE))
+    return start_time;
+
+  const base::TimeDelta first_dts =
+      ConvertFromTimeBase(stream->time_base, stream->first_dts);
+
+  return first_dts < start_time ? first_dts : start_time;
+}
+
 //
 // FFmpegDemuxerStream
 //
 FFmpegDemuxerStream::FFmpegDemuxerStream(
     FFmpegDemuxer* demuxer,
     AVStream* stream)
     : demuxer_(demuxer),
       task_runner_(base::MessageLoopProxy::current()),
       stream_(stream),
       type_(UNKNOWN),
       end_of_stream_(false),
       last_packet_timestamp_(kNoTimestamp()),
-      bitstream_converter_enabled_(false) {
+      bitstream_converter_enabled_(false),
+      discard_negative_timestamps_(false) {
   DCHECK(demuxer_);
 
   bool is_encrypted = false;
 
   // Determine our media format.
   switch (stream->codec->codec_type) {
     case AVMEDIA_TYPE_AUDIO:
       type_ = AUDIO;
       AVStreamToAudioDecoderConfig(stream, &audio_config_, true);
       is_encrypted = audio_config_.is_encrypted();
@@ skipping 138 matching lines @@
           audio_decoder_config().samples_per_second();
       buffer->set_discard_padding(std::make_pair(
           FramesToTimeDelta(discard_front_samples, samples_per_second),
           FramesToTimeDelta(discard_end_samples, samples_per_second)));
     }
 
     if (decrypt_config)
       buffer->set_decrypt_config(decrypt_config.Pass());
   }
 
-  buffer->set_timestamp(ConvertStreamTimestamp(
-      stream_->time_base, packet->pts));
-  buffer->set_duration(ConvertStreamTimestamp(
-      stream_->time_base, packet->duration));
+  const base::TimeDelta stream_timestamp =
+      ConvertStreamTimestamp(stream_->time_base, packet->pts);
+
+  buffer->set_timestamp(stream_timestamp - demuxer_->start_time());

[acolwell GONE FROM CHROMIUM, 2014/06/10 23:20:38]

Does this hold up with chained Ogg? Does FFmpeg guarantee that timestamps will
always increase? I'm just wondering if it protects us from having to worry about
timestamps later in the stream that end up being < start_time()

[DaleCurtis, 2014/06/11 16:59:07]

Depends on how short the first link in the chain is I guess. I've never seen the
chained ogg go backwards by more than few seconds.  I suspect it's possible
today and my patch doesn't do anything to fix it.  It's possible if we add the
DCHECK(timestamp > 0) to the decoder we'll now fail hard instead of soft there
though.

[acolwell GONE FROM CHROMIUM, 2014/06/11 17:40:24]

I guess my concern is that if FFmpeg allows timestamps to go backwards, how can
we reliably seek? If you use something like oggz-chop to split an ogg file into
2 pieces(A & B) and then concatenate them so they are in B,A order, what does
FFmpeg do? 

I realize that this may be broken in the existing code as well. I'm just trying
to understand what will happen.

[DaleCurtis, 2014/06/12 00:21:56]

I chopped the file as you described it seems timestamps are not rewritten, you
get the zero timestamp for the first part at the end of the second part which
generates a non-monotonic timestamp warning with a huge number.  FFmpeg then
returns a packet without a timestamp which errors out playback.  The behavior is
unchanged by my patch.

+  buffer->set_duration(
+      ConvertStreamTimestamp(stream_->time_base, packet->duration));
+
+  // If enabled, mark packets with negative timestamps for post-decode discard.
+  if (discard_negative_timestamps_ && stream_timestamp < base::TimeDelta()) {
+    buffer->set_discard_padding(
+        std::make_pair(kInfiniteDuration(), base::TimeDelta()));
+  }
+
   if (buffer->timestamp() != kNoTimestamp() &&
       last_packet_timestamp_ != kNoTimestamp() &&
       last_packet_timestamp_ < buffer->timestamp()) {
     buffered_ranges_.Add(last_packet_timestamp_, buffer->timestamp());
     demuxer_->NotifyBufferingChanged();
   }
   last_packet_timestamp_ = buffer->timestamp();
 
   buffer_queue_.Push(buffer);
   SatisfyPendingRead();
@@ skipping 18 matching lines @@
   buffer_queue_.Clear();
   if (!read_cb_.is_null()) {
     base::ResetAndReturn(&read_cb_).Run(
         DemuxerStream::kOk, DecoderBuffer::CreateEOSBuffer());
   }
   demuxer_ = NULL;
   stream_ = NULL;
   end_of_stream_ = true;
 }
 
-base::TimeDelta FFmpegDemuxerStream::duration() {
-  return duration_;
-}
-
 DemuxerStream::Type FFmpegDemuxerStream::type() {
   DCHECK(task_runner_->BelongsToCurrentThread());
   return type_;
 }
 
 void FFmpegDemuxerStream::Read(const ReadCB& read_cb) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   CHECK(read_cb_.is_null()) << "Overlapping reads are not supported";
   read_cb_ = BindToCurrentLoop(read_cb);
 
@@ skipping 123 matching lines @@
     const NeedKeyCB& need_key_cb,
     const scoped_refptr<MediaLog>& media_log)
     : host_(NULL),
       task_runner_(task_runner),
       blocking_thread_("FFmpegDemuxer"),
       pending_read_(false),
       pending_seek_(false),
       data_source_(data_source),
       media_log_(media_log),
       bitrate_(0),
-      start_time_(kNoTimestamp()),
+      start_time_(kInfiniteDuration()),

[acolwell GONE FROM CHROMIUM, 2014/06/10 23:20:38]

This seems a little out of place since we typically use kNoTimestamp() to signal
"not set". It seems like your use of kInfiniteDurat() should be constrained to
OnFindStreamInfoDone() so it is closer to the code that actually uses it for
special purposes.

[DaleCurtis, 2014/06/12 00:21:55]

Done.

       liveness_(LIVENESS_UNKNOWN),
       text_enabled_(false),
       duration_known_(false),
       need_key_cb_(need_key_cb),
+      stream_index_for_seeking_(0),
       weak_factory_(this) {
   DCHECK(task_runner_.get());
   DCHECK(data_source_);
 }
 
 FFmpegDemuxer::~FFmpegDemuxer() {}
 
 void FFmpegDemuxer::Stop(const base::Closure& callback) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   url_protocol_->Abort();
   data_source_->Stop(
       BindToCurrentLoop(base::Bind(&FFmpegDemuxer::OnDataSourceStopped,
                                    weak_factory_.GetWeakPtr(),
                                    BindToCurrentLoop(callback))));
   data_source_ = NULL;
 }
 
 void FFmpegDemuxer::Seek(base::TimeDelta time, const PipelineStatusCB& cb) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   CHECK(!pending_seek_);
 
   // TODO(scherkus): Inspect |pending_read_| and cancel IO via |blocking_url_|,
   // 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.
 
+  const AVStream* seeking_stream =
+      glue_->format_context()->streams[stream_index_for_seeking_];
+
   // Always seek to a timestamp less than or equal to the desired timestamp.
-  int flags = AVSEEK_FLAG_BACKWARD;
+  const int flags = AVSEEK_FLAG_BACKWARD;

[acolwell GONE FROM CHROMIUM, 2014/06/10 23:20:38]

Why not simply inline this below?

[DaleCurtis, 2014/06/12 00:21:55]

Done.

 
-  // Passing -1 as our stream index lets FFmpeg pick a default stream.  FFmpeg
-  // will attempt to use the lowest-index video stream, if present, followed by
-  // the lowest-index audio stream.
   pending_seek_ = true;
   base::PostTaskAndReplyWithResult(
       blocking_thread_.message_loop_proxy().get(),
       FROM_HERE,
-      base::Bind(&av_seek_frame,
-                 glue_->format_context(),
-                 -1,
-                 time.InMicroseconds(),
-                 flags),
+      base::Bind(
+          &av_seek_frame,
+          glue_->format_context(),
+          stream_index_for_seeking_,
+          ConvertToTimeBase(seeking_stream->time_base, time + start_time()),
+          flags),
       base::Bind(
           &FFmpegDemuxer::OnSeekFrameDone, weak_factory_.GetWeakPtr(), cb));
 }
 
 void FFmpegDemuxer::Initialize(DemuxerHost* host,
                                const PipelineStatusCB& status_cb,
                                bool enable_text_tracks) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   host_ = host;
   text_enabled_ = enable_text_tracks;
@@ skipping 28 matching lines @@
     DemuxerStream::Type type) const {
   StreamVector::const_iterator iter;
   for (iter = streams_.begin(); iter != streams_.end(); ++iter) {
     if (*iter && (*iter)->type() == type) {
       return *iter;
     }
   }
   return NULL;
 }
 
-base::TimeDelta FFmpegDemuxer::GetStartTime() const {
-  DCHECK(task_runner_->BelongsToCurrentThread());
-  return start_time_;
-}
-
 base::Time FFmpegDemuxer::GetTimelineOffset() const {
   return timeline_offset_;
 }
 
 Demuxer::Liveness FFmpegDemuxer::GetLiveness() const {
   DCHECK(task_runner_->BelongsToCurrentThread());
   return liveness_;
 }
 
 void FFmpegDemuxer::AddTextStreams() {
@@ skipping 100 matching lines @@
   AudioDecoderConfig audio_config;
 
   AVStream* video_stream = NULL;
   VideoDecoderConfig video_config;
 
   base::TimeDelta max_duration;
   for (size_t i = 0; i < format_context->nb_streams; ++i) {
     AVStream* stream = format_context->streams[i];
     AVCodecContext* codec_context = stream->codec;
     AVMediaType codec_type = codec_context->codec_type;
+    bool discard_negative_timestamps = false;
 
     if (codec_type == AVMEDIA_TYPE_AUDIO) {
       if (audio_stream)
         continue;
 
       // Log the codec detected, whether it is supported or not.
       UMA_HISTOGRAM_SPARSE_SLOWLY("Media.DetectedAudioCodec",
                                   codec_context->codec_id);
       // Ensure the codec is supported. IsValidConfig() also checks that the
       // channel layout and sample format are valid.
       AVStreamToAudioDecoderConfig(stream, &audio_config, false);
       if (!audio_config.IsValidConfig())
         continue;
       audio_stream = stream;
+
+      const base::TimeDelta audio_start_time = ExtractStartTime(stream);
+      if (audio_start_time != kNoTimestamp() &&
+          audio_start_time < start_time_) {
+        stream_index_for_seeking_ = i;
+        start_time_ = audio_start_time;
+      }
+
+      // 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;
 
       // Log the codec detected, whether it is supported or not.
       UMA_HISTOGRAM_SPARSE_SLOWLY("Media.DetectedVideoCodec",
                                   codec_context->codec_id);
       // Ensure the codec is supported. IsValidConfig() also checks that the
       // frame size and visible size are valid.
       AVStreamToVideoDecoderConfig(stream, &video_config, false);
 
       if (!video_config.IsValidConfig())
         continue;
       video_stream = stream;
+
+      // Find the video start time.  Note the <= comparison causes the video
+      // stream to be preferred for seeking when start times match.
+      const base::TimeDelta video_start_time = ExtractStartTime(stream);
+      if (video_start_time != kNoTimestamp() &&
+          video_start_time <= start_time_) {
+        stream_index_for_seeking_ = i;
+        start_time_ = video_start_time;
+      }
     } else if (codec_type == AVMEDIA_TYPE_SUBTITLE) {
       if (codec_context->codec_id != AV_CODEC_ID_WEBVTT || !text_enabled_) {
         continue;
       }
     } else {
       continue;
     }
 
     streams_[i] = new FFmpegDemuxerStream(this, stream);
     max_duration = std::max(max_duration, streams_[i]->duration());
-
-    if (stream->first_dts != static_cast<int64_t>(AV_NOPTS_VALUE)) {
-      const base::TimeDelta first_dts = ConvertFromTimeBase(
-          stream->time_base, stream->first_dts);
-      if (start_time_ == kNoTimestamp() || first_dts < start_time_)
-        start_time_ = first_dts;
-    }
+    if (discard_negative_timestamps)
+      streams_[i]->enable_negative_timestamp_discard();

[acolwell GONE FROM CHROMIUM, 2014/06/10 23:20:38]

nit: Just pass this via the constructor 2 lines up since this value isn't
expected to change during the life of the stream.

[DaleCurtis, 2014/06/12 00:21:55]

Done.

   }
 
   if (!audio_stream && !video_stream) {
     status_cb.Run(DEMUXER_ERROR_NO_SUPPORTED_STREAMS);
     return;
   }
 
   if (text_enabled_)
     AddTextStreams();
 
   if (format_context->duration != static_cast<int64_t>(AV_NOPTS_VALUE)) {
     // If there is a duration value in the container use that to find the
     // 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();
   }
 
-  // Some demuxers, like WAV, do not put timestamps on their frames. We
-  // assume the the start time is 0.
-  if (start_time_ == kNoTimestamp())
+  // 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();

[acolwell GONE FROM CHROMIUM, 2014/06/10 23:20:38]

If we don't know at this point, shouldn't we just try to defer this decision
until we are able to get the first packet from all of the streams? If we don't
it seems like we could accidentally trigger bad behavior if the first packets
happen to be negative or really large.

[DaleCurtis, 2014/06/11 16:59:07]

Maybe.  Couldn't a seek happen before the first frame is read?  I don't think
it's worth adding this functionality.  What I've done here will fall back to
what we do today.

[acolwell GONE FROM CHROMIUM, 2014/06/11 17:40:24]

A seek can't happen until we transition to HAVE_METADATA and that can't happen
until the demuxer calls its init callback and all the renderers are initialized.
It should be pretty straightforward to delay calling the init callback until we
have received the first buffer for each stream. Either that or we should block
anything with a timestamp < start_time_ from being sent downstream so that other
components don't have to deal with it.

[DaleCurtis, 2014/06/12 00:21:55]

This is not easy to do since the preroll decision is handled by pipeline the 
demuxer and renderers have completed initialization.  We could trigger Preroll
until we've received all stream start times, but that may delay the rest of
pipeline initialization since those would be network loads.

I've instead just added a DCHECK() for this to the demuxer.

+    stream_index_for_seeking_ =
+        video_stream ? video_stream->index : audio_stream->index;
+  }
 
   // MPEG-4 B-frames cause grief for a simple container like AVI. Enable PTS
   // generation so we always get timestamps, see http://crbug.com/169570
   if (strcmp(format_context->iformat->name, "avi") == 0)
     format_context->flags |= AVFMT_FLAG_GENPTS;
 
   timeline_offset_ = ExtractTimelineOffset(format_context);
 
+  // Since we're shifting the externally visible start time to zero, we need to
+  // adjust the timeline offset to compensate.
+  if (!timeline_offset_.is_null())
+    timeline_offset_ += start_time_;
+
   if (max_duration == kInfiniteDuration() && !timeline_offset_.is_null()) {
     liveness_ = LIVENESS_LIVE;
   } else if (max_duration != kInfiniteDuration()) {
     liveness_ = LIVENESS_RECORDED;
   } else {
     liveness_ = LIVENESS_UNKNOWN;
   }
 
   // Good to go: set the duration and bitrate and notify we're done
   // initializing.
@@ skipping 51 matching lines @@
                            video_codec->time_base.num,
                            video_codec->time_base.den));
     media_log_->SetStringProperty(
         "video_format", VideoFrame::FormatToString(video_config.format()));
     media_log_->SetBooleanProperty("video_is_encrypted",
                                    video_config.is_encrypted());
   } else {
     media_log_->SetBooleanProperty("found_video_stream", false);
   }
 
-
   media_log_->SetTimeProperty("max_duration", max_duration);
   media_log_->SetTimeProperty("start_time", start_time_);
   media_log_->SetIntegerProperty("bitrate", bitrate_);
 
   status_cb.Run(PIPELINE_OK);
 }
 
 void FFmpegDemuxer::OnSeekFrameDone(const PipelineStatusCB& cb, int result) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   CHECK(pending_seek_);
@@ skipping 215 matching lines @@
   }
   for (size_t i = 0; i < buffered.size(); ++i)
     host_->AddBufferedTimeRange(buffered.start(i), buffered.end(i));
 }
 
 void FFmpegDemuxer::OnDataSourceError() {
   host_->OnDemuxerError(PIPELINE_ERROR_READ);
 }
 
 }  // namespace media