ffmpeg reset bitstream converters on Seek

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 416 matching lines @@
   DCHECK(task_runner_->BelongsToCurrentThread());
 
 #if defined(USE_PROPRIETARY_CODECS)
   CHECK(bitstream_converter_.get());
   bitstream_converter_enabled_ = true;
 #else
   NOTREACHED() << "Proprietary codecs not enabled.";
 #endif
 }
 
+void FFmpegDemuxerStream::ResetBitstreamConverter() {
+  // If a bitstream converter exists, reset it.

[DaleCurtis, 2014/10/27 20:36:02]

DCHECK unnecessary since you access it directly below.

[kjoswiak, 2014/10/27 20:47:32]

Done.

+  DCHECK(stream_);
+
+  if (!bitstream_converter_enabled_)
+    return;
+
+#if defined(USE_PROPRIETARY_CODECS)
+  if (bitstream_converter_ != NULL) {

[DaleCurtis, 2014/10/27 20:36:02]

if (!bitstream_converter_)
  return;

bitstream_converter_.reset(new ...).

[kjoswiak, 2014/10/27 20:47:32]

Done.

[xhwang, 2014/10/27 20:59:21]

How about do the same as l.152?

 if (stream_->codec->codec_id == AV_CODEC_ID_H264) {
    bitstream_converter_.reset(
        new FFmpegH264ToAnnexBBitstreamConverter(stream_->codec));

The code about bitstream_converter_enabled_ and bitstream_converter_ is a bit
convoluted. We always create the converted if codec is H264, but we only use it
when EnableBitstreamConverter() is called. I think we should do some cleanup
here...

[kjoswiak, 2014/10/27 21:34:17]

Ya, it does seem a little weird. Should I just go ahead and try to move creation
into EnableBitstreamConverter?

[kjoswiak, 2014/10/27 21:56:41]

https://chromiumcodereview.appspot.com/10879057 is why, going to look into to
see if still necessary

[kjoswiak, 2014/10/27 23:49:59]

Primary issue is that we weren't guaranteed that stream_->codec still exists,
think its safer to just leave as is

+    bitstream_converter_.reset(
+        new FFmpegH264ToAnnexBBitstreamConverter(stream_->codec));
+  }
+#endif  // defined(USE_PROPRIETARY_CODECS)
+}
+
 bool FFmpegDemuxerStream::SupportsConfigChanges() { return false; }
 
 AudioDecoderConfig FFmpegDemuxerStream::audio_decoder_config() {
   DCHECK(task_runner_->BelongsToCurrentThread());
   CHECK_EQ(type_, AUDIO);
   return audio_config_;
 }
 
 VideoDecoderConfig FFmpegDemuxerStream::video_decoder_config() {
   DCHECK(task_runner_->BelongsToCurrentThread());
@@ skipping 139 matching lines @@
 }
 
 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.
 
+  FFmpegDemuxerStream* video = GetFFmpegStream(DemuxerStream::VIDEO);
+  // H264 and VP8 require that we resend the header after flush.

[xhwang, 2014/10/27 20:59:21]

The bit stream converter is really only for H264.

[kjoswiak, 2014/10/27 23:49:59]

Done.

+  // Reset the bitstream converter to do so.
+  // This is related to chromium issue 140371 (http://crbug.com/140371).
+  if (video) {

[DaleCurtis, 2014/10/27 20:36:02]

No {} necessary.

[kjoswiak, 2014/10/27 20:47:32]

Done.

+    video->ResetBitstreamConverter();
+  }
+
   // 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;
@@ skipping 642 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