Stop audio FFmpegDemuxerStreams if we get notified that audio rendering is disabled.

This prevents reading the entire file until end of stream when attempting to read audio data while disabled.

Part of this bug was due to keeping two separate arrays of FFmpegDemuxerStreams and inspecting the wrong one. To further reinforce this change, the two arrays have been merged.

BUG=106735
TEST=disable audio service on Windows 7 then navigate to various HTML5 audio/video resource with a clean profile: it should take less time to load and the buffer bar should be only partially full instead of 100% full

Review URL: http://codereview.chromium.org/8890071

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@114699 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 120 insertions, 76 deletions

diff --git a/media/filters/ffmpeg_demuxer.cc b/media/filters/ffmpeg_demuxer.cc
index c1a056c..aa24e7a 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -362,9 +362,13 @@ void FFmpegDemuxer::Initialize(DataSource* data_source,
 
 scoped_refptr<DemuxerStream> FFmpegDemuxer::GetStream(
     DemuxerStream::Type type) {
-  DCHECK_GE(type, 0);
-  DCHECK_LT(type, DemuxerStream::NUM_TYPES);
-  return streams_[type];
+  StreamVector::iterator iter;
+  for (iter = streams_.begin(); iter != streams_.end(); ++iter) {
+    if (*iter && (*iter)->type() == type) {
+      return *iter;
+    }
+  }
+  return NULL;
 }
 
 base::TimeDelta FFmpegDemuxer::GetStartTime() const {
@@ -479,38 +483,48 @@ void FFmpegDemuxer::InitializeTask(DataSource* data_source,
     return;
   }
 
-  // Create demuxer streams for all supported streams.
-  streams_.resize(DemuxerStream::NUM_TYPES);
+  // Create demuxer stream entries for each possible AVStream.
+  streams_.resize(format_context_->nb_streams);
+  bool found_audio_stream = false;
+  bool found_video_stream = false;
+
   base::TimeDelta max_duration;
-  bool no_supported_streams = true;
   for (size_t i = 0; i < format_context_->nb_streams; ++i) {
     AVCodecContext* codec_context = format_context_->streams[i]->codec;
     AVMediaType codec_type = codec_context->codec_type;
-    if (codec_type == AVMEDIA_TYPE_AUDIO || codec_type == AVMEDIA_TYPE_VIDEO) {
-      AVStream* stream = format_context_->streams[i];
-      scoped_refptr<FFmpegDemuxerStream> demuxer_stream(
-          new FFmpegDemuxerStream(this, stream));
-      if (!streams_[demuxer_stream->type()]) {
-        no_supported_streams = false;
-        streams_[demuxer_stream->type()] = demuxer_stream;
-        max_duration = std::max(max_duration, demuxer_stream->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;
-        }
-      }
-      packet_streams_.push_back(demuxer_stream);
+
+    if (codec_type == AVMEDIA_TYPE_AUDIO) {
+      if (found_audio_stream)
+        continue;
+      found_audio_stream = true;
+    } else if (codec_type == AVMEDIA_TYPE_VIDEO) {
+      if (found_video_stream)
+        continue;
+      found_video_stream = true;
     } else {
-      packet_streams_.push_back(NULL);
+      continue;
+    }
+
+    AVStream* stream = format_context_->streams[i];
+    scoped_refptr<FFmpegDemuxerStream> demuxer_stream(
+        new FFmpegDemuxerStream(this, stream));
+
+    streams_[i] = demuxer_stream;
+    max_duration = std::max(max_duration, demuxer_stream->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 (no_supported_streams) {
+
+  if (!found_audio_stream && !found_video_stream) {
     callback.Run(DEMUXER_ERROR_NO_SUPPORTED_STREAMS);
     return;
   }
+
   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.
@@ -642,26 +656,24 @@ void FFmpegDemuxer::DemuxTask() {
   // worried about downstream filters (i.e., decoders) executing on this
   // thread.
   DCHECK_GE(packet->stream_index, 0);
-  DCHECK_LT(packet->stream_index, static_cast<int>(packet_streams_.size()));
-  FFmpegDemuxerStream* demuxer_stream = NULL;
-  size_t i = packet->stream_index;
+  DCHECK_LT(packet->stream_index, static_cast<int>(streams_.size()));
+
   // Defend against ffmpeg giving us a bad stream index.
-  if (i < packet_streams_.size()) {
-    demuxer_stream = packet_streams_[i];
-  }
-  if (demuxer_stream) {
-    // Queue the packet with the appropriate stream.  The stream takes
-    // ownership of the AVPacket.
-    if (packet.get()) {
-      // If a packet is returned by FFmpeg's av_parser_parse2()
-      // the packet will reference an inner memory of FFmpeg.
-      // In this case, the packet's "destruct" member is NULL,
-      // and it MUST be duplicated. This fixes issue with MP3 and possibly
-      // other codecs.  It is safe to call this function even if the packet does
-      // not refer to inner memory from FFmpeg.
-      av_dup_packet(packet.get());
-      demuxer_stream->EnqueuePacket(packet.release());
-    }
+  if (packet->stream_index >= 0 &&
+      packet->stream_index < static_cast<int>(streams_.size()) &&
+      streams_[packet->stream_index]) {
+    FFmpegDemuxerStream* demuxer_stream = streams_[packet->stream_index];
+
+    // If a packet is returned by FFmpeg's av_parser_parse2()
+    // the packet will reference an inner memory of FFmpeg.
+    // In this case, the packet's "destruct" member is NULL,
+    // and it MUST be duplicated. This fixes issue with MP3 and possibly
+    // other codecs.  It is safe to call this function even if the packet does
+    // not refer to inner memory from FFmpeg.
+    av_dup_packet(packet.get());
+
+    // The stream takes ownership of the AVPacket.
+    demuxer_stream->EnqueuePacket(packet.release());
   }
 
   // Create a loop by posting another task.  This allows seek and message loop
@@ -687,17 +699,10 @@ void FFmpegDemuxer::StopTask(const base::Closure& callback) {
 
 void FFmpegDemuxer::DisableAudioStreamTask() {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
-
   StreamVector::iterator iter;
-  for (size_t i = 0; i < packet_streams_.size(); ++i) {
-    if (!packet_streams_[i])
-      continue;
-
-    // If the codec type is audio, remove the reference. DemuxTask() will
-    // look for such reference, and this will result in deleting the
-    // audio packets after they are demuxed.
-    if (packet_streams_[i]->type() == DemuxerStream::AUDIO) {
-      packet_streams_[i] = NULL;
+  for (iter = streams_.begin(); iter != streams_.end(); ++iter) {
+    if (*iter && (*iter)->type() == DemuxerStream::AUDIO) {
+      (*iter)->Stop();
     }
   }
 }
diff --git a/media/filters/ffmpeg_demuxer.h b/media/filters/ffmpeg_demuxer.h
index 3630481..3f2e596 100644
--- a/media/filters/ffmpeg_demuxer.h
+++ b/media/filters/ffmpeg_demuxer.h
@@ -84,6 +84,7 @@ class FFmpegDemuxerStream : public DemuxerStream {
   virtual const VideoDecoderConfig& video_decoder_config() OVERRIDE;
 
  private:
+  friend class FFmpegDemuxerTest;
   virtual ~FFmpegDemuxerStream();
 
   // Carries out enqueuing a pending read on the demuxer thread.
@@ -217,20 +218,17 @@ class MEDIA_EXPORT FFmpegDemuxer : public Demuxer, public FFmpegURLProtocol {
   // FFmpeg context handle.
   AVFormatContext* format_context_;
 
-  // Two vector of streams:
-  //   - |streams_| is indexed by type for the Demuxer interface GetStream(),
-  //     and contains NULLs for types which aren't present.
-  //   - |packet_streams_| is indexed to mirror AVFormatContext when dealing
-  //     with AVPackets returned from av_read_frame() and contain NULL entries
-  //     representing unsupported streams where we throw away the data.
+  // |streams_| mirrors the AVStream array in |format_context_|. It contains
+  // FFmpegDemuxerStreams encapsluating AVStream objects at the same index.
   //
-  // Ownership is handled via reference counting.
+  // Since we only support a single audio and video stream, |streams_| will
+  // contain NULL entries for additional audio/video streams as well as for
+  // stream types that we do not currently support.
   //
   // Once initialized, operations on FFmpegDemuxerStreams should be carried out
   // on the demuxer thread.
-  typedef std::vector< scoped_refptr<FFmpegDemuxerStream> > StreamVector;
+  typedef std::vector<scoped_refptr<FFmpegDemuxerStream> > StreamVector;
   StreamVector streams_;
-  StreamVector packet_streams_;
 
   // Reference to the data source. Asynchronous read requests are submitted to
   // this object.
diff --git a/media/filters/ffmpeg_demuxer_unittest.cc b/media/filters/ffmpeg_demuxer_unittest.cc
index 2c46894..79c3b44 100644
--- a/media/filters/ffmpeg_demuxer_unittest.cc
+++ b/media/filters/ffmpeg_demuxer_unittest.cc
@@ -126,6 +126,12 @@ class FFmpegDemuxerTest : public testing::Test {
     return demuxer_->GetBitrate() > 0;
   }
 
+  bool IsStreamStopped(DemuxerStream::Type type) {
+    DemuxerStream* stream = demuxer_->GetStream(type);
+    CHECK(stream);
+    return static_cast<FFmpegDemuxerStream*>(stream)->stopped_;
+  }
+
   // Fixture members.
   scoped_refptr<FFmpegDemuxer> demuxer_;
   StrictMock<MockFilterHost> host_;
@@ -157,7 +163,7 @@ TEST_F(FFmpegDemuxerTest, Initialize_OpenFails) {
 //}
 
 TEST_F(FFmpegDemuxerTest, Initialize_NoStreams) {
-  // Open a file with no parseable streams.
+  // Open a file with no streams whatsoever.
   EXPECT_CALL(host_, SetCurrentReadPosition(_));
   demuxer_->Initialize(
       CreateDataSource("no_streams.webm"),
@@ -165,14 +171,13 @@ TEST_F(FFmpegDemuxerTest, Initialize_NoStreams) {
   message_loop_.RunAllPending();
 }
 
-// TODO(acolwell): Find a subtitle only file so we can uncomment this test.
-//
-//TEST_F(FFmpegDemuxerTest, Initialize_DataStreamOnly) {
-//  demuxer_->Initialize(
-//      CreateDataSource("subtitles_only.mp4"),,
-//      NewExpectedStatusCB(DEMUXER_ERROR_NO_SUPPORTED_STREAMS));
-//  message_loop_.RunAllPending();
-//}
+TEST_F(FFmpegDemuxerTest, Initialize_NoAudioVideo) {
+  // Open a file containing streams but none of which are audio/video streams.
+  demuxer_->Initialize(
+      CreateDataSource("no_audio_video.webm"),
+      NewExpectedStatusCB(DEMUXER_ERROR_NO_SUPPORTED_STREAMS));
+  message_loop_.RunAllPending();
+}
 
 TEST_F(FFmpegDemuxerTest, Initialize_Successful) {
   InitializeDemuxer(CreateDataSource("bear-320x240.webm"));
@@ -211,6 +216,37 @@ TEST_F(FFmpegDemuxerTest, Initialize_Successful) {
   EXPECT_EQ(44100, audio_config.samples_per_second());
   EXPECT_TRUE(audio_config.extra_data());
   EXPECT_GT(audio_config.extra_data_size(), 0u);
+
+  // Unknown stream should never be present.
+  EXPECT_FALSE(demuxer_->GetStream(DemuxerStream::UNKNOWN));
+}
+
+TEST_F(FFmpegDemuxerTest, Initialize_Multitrack) {
+  // Open a file containing the following streams:
+  //   Stream #0: Video (VP8)
+  //   Stream #1: Audio (Vorbis)
+  //   Stream #2: Subtitles (SRT)
+  //   Stream #3: Video (Theora)
+  //   Stream #4: Audio (16-bit signed little endian PCM)
+  //
+  // We should only pick the first audio/video streams we come across.
+  InitializeDemuxer(CreateDataSource("bear-320x240-multitrack.webm"));
+
+  // Video stream should be VP8.
+  scoped_refptr<DemuxerStream> stream =
+      demuxer_->GetStream(DemuxerStream::VIDEO);
+  ASSERT_TRUE(stream);
+  EXPECT_EQ(DemuxerStream::VIDEO, stream->type());
+  EXPECT_EQ(kCodecVP8, stream->video_decoder_config().codec());
+
+  // Audio stream should be Vorbis.
+  stream = demuxer_->GetStream(DemuxerStream::AUDIO);
+  ASSERT_TRUE(stream);
+  EXPECT_EQ(DemuxerStream::AUDIO, stream->type());
+  EXPECT_EQ(kCodecVorbis, stream->audio_decoder_config().codec());
+
+  // Unknown stream should never be present.
+  EXPECT_FALSE(demuxer_->GetStream(DemuxerStream::UNKNOWN));
 }
 
 TEST_F(FFmpegDemuxerTest, Read_Audio) {
@@ -491,17 +527,22 @@ TEST_F(FFmpegDemuxerTest, DisableAudioStream) {
   ASSERT_TRUE(video);
   ASSERT_TRUE(audio);
 
-  // Attempt a read from the video stream and run the message loop until done.
+  // The audio stream should have been prematurely stopped.
+  EXPECT_FALSE(IsStreamStopped(DemuxerStream::VIDEO));
+  EXPECT_TRUE(IsStreamStopped(DemuxerStream::AUDIO));
+
+  // Attempt a read from the video stream: it should return valid data.
   scoped_refptr<DemuxerStreamReader> reader(new DemuxerStreamReader());
   reader->Read(video);
   message_loop_.RunAllPending();
-  EXPECT_TRUE(reader->called());
+  ASSERT_TRUE(reader->called());
   ValidateBuffer(FROM_HERE, reader->buffer(), 22084, 0);
 
+  // Attempt a read from the audio stream: it should immediately return end of
+  // stream without requiring the message loop to read data.
   reader->Reset();
   reader->Read(audio);
-  message_loop_.RunAllPending();
-  EXPECT_TRUE(reader->called());
+  ASSERT_TRUE(reader->called());
   EXPECT_TRUE(reader->buffer()->IsEndOfStream());
 }