ffmpeg reset bitstream converters on Seek

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 15e77c6aa03b1795066b28db1e92b8bb352611df..fd4010f5dd7996785e7b5795c0a66384a3cdb1c1 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -434,6 +434,21 @@ void FFmpegDemuxerStream::EnableBitstreamConverter() {
 #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() {
@@ -593,6 +608,14 @@ void FFmpegDemuxer::Seek(base::TimeDelta time, const PipelineStatusCB& cb) {
   // 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.