Refactor StreamParser creation & enforce configs matching expected stream types.

BUG=122909
TEST=ChunkDemuxer.TestAVHeadersWithAudioOnlyType, ChunkDemuxer.TestAVHeadersWithVideoOnlyType


Review URL: https://chromiumcodereview.appspot.com/10389125

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

4 files changed, 138 insertions, 22 deletions

diff --git a/media/filters/chunk_demuxer.cc b/media/filters/chunk_demuxer.cc
index fdd1a97..92489ce 100644
--- a/media/filters/chunk_demuxer.cc
+++ b/media/filters/chunk_demuxer.cc
@@ -15,24 +15,59 @@
 
 namespace media {
 
+struct CodecInfo {
+  const char* name;
+  DemuxerStream::Type type;
+};
+
+typedef StreamParser* (*ParserFactoryFunction)();
+
 struct SupportedTypeInfo {
   const char* type;
-  const char** codecs;
+  const ParserFactoryFunction factory_function;
+  const CodecInfo** codecs;
 };
 
-static const char* kVideoWebMCodecs[] = { "vp8", "vorbis", NULL };
-static const char* kAudioWebMCodecs[] = { "vorbis", NULL };
+static const CodecInfo kVP8CodecInfo = { "vp8", DemuxerStream::VIDEO };
+static const CodecInfo kVorbisCodecInfo = { "vorbis", DemuxerStream::AUDIO };
+
+static const CodecInfo* kVideoWebMCodecs[] = {
+  &kVP8CodecInfo,
+  &kVorbisCodecInfo,
+  NULL
+};
+
+static const CodecInfo* kAudioWebMCodecs[] = {
+  &kVorbisCodecInfo,
+  NULL
+};
+
+static StreamParser* BuildWebMParser() {
+  return new WebMStreamParser();
+}
 
 static const SupportedTypeInfo kSupportedTypeInfo[] = {
-  { "video/webm", kVideoWebMCodecs },
-  { "audio/webm", kAudioWebMCodecs },
+  { "video/webm", &BuildWebMParser, kVideoWebMCodecs },
+  { "audio/webm", &BuildWebMParser, kAudioWebMCodecs },
 };
 
 // Checks to see if the specified |type| and |codecs| list are supported.
 // Returns true if |type| and all codecs listed in |codecs| are supported.
-// Returns false otherwise.
+//         |factory_function| contains a function that can build a StreamParser
+//                            for this type.
+//         |has_audio| is true if an audio codec was specified.
+//         |has_video| is true if a video codec was specified.
+// Returns false otherwise. The values of |factory_function|, |has_audio|,
+//         and |has_video| are undefined.
 static bool IsSupported(const std::string& type,
-                        std::vector<std::string>& codecs) {
+                        std::vector<std::string>& codecs,
+                        ParserFactoryFunction* factory_function,
+                        bool* has_audio,
+                        bool* has_video) {
+  *factory_function = NULL;
+  *has_audio = false;
+  *has_video = false;
+
   // Search for the SupportedTypeInfo for |type|
   for (size_t i = 0; i < arraysize(kSupportedTypeInfo); ++i) {
     const SupportedTypeInfo& type_info = kSupportedTypeInfo[i];
@@ -42,13 +77,35 @@ static bool IsSupported(const std::string& type,
       for (size_t j = 0; j < codecs.size(); ++j) {
         // Search the type info for a match.
         bool found_codec = false;
-        for (int k = 0; type_info.codecs[k] && !found_codec; ++k)
-          found_codec = (codecs[j] == type_info.codecs[k]);
+        DemuxerStream::Type codec_type = DemuxerStream::UNKNOWN;
+
+        for (int k = 0; type_info.codecs[k]; ++k) {
+          if (codecs[j] == type_info.codecs[k]->name) {
+            found_codec = true;
+            codec_type = type_info.codecs[k]->type;
+            break;
+          }
+        }
 
         if (!found_codec)
           return false;
+
+        switch (codec_type) {
+          case DemuxerStream::AUDIO:
+            *has_audio = true;
+            break;
+          case DemuxerStream::VIDEO:
+            *has_video = true;
+            break;
+          default:
+            DVLOG(1) << "Unsupported codec type '"<< codec_type << "' for "
+                     << codecs[j];
+            return false;
+        }
       }
 
+      *factory_function = type_info.factory_function;
+
       // All codecs were supported by this |type|.
       return true;
     }
@@ -417,15 +474,6 @@ void ChunkDemuxer::Initialize(DemuxerHost* host,
 
     ChangeState_Locked(INITIALIZING);
     init_cb_ = cb;
-
-    source_buffer_.reset(new SourceBuffer());
-
-    source_buffer_->Init(
-        base::Bind(&ChunkDemuxer::OnSourceBufferInitDone, this),
-        base::Bind(&ChunkDemuxer::OnNewConfigs, base::Unretained(this)),
-        base::Bind(&ChunkDemuxer::OnAudioBuffers, base::Unretained(this)),
-        base::Bind(&ChunkDemuxer::OnVideoBuffers, base::Unretained(this)),
-        base::Bind(&ChunkDemuxer::OnKeyNeeded, base::Unretained(this)));
   }
 
   client_->DemuxerOpened(this);
@@ -517,7 +565,10 @@ ChunkDemuxer::Status ChunkDemuxer::AddId(const std::string& id,
                                          std::vector<std::string>& codecs) {
   DCHECK_GT(codecs.size(), 0u);
 
-  if (!IsSupported(type, codecs))
+  bool has_audio = false;
+  bool has_video = false;
+  ParserFactoryFunction factory_function = NULL;
+  if (!IsSupported(type, codecs, &factory_function, &has_audio, &has_video))
     return kNotSupported;
 
   // TODO(acolwell): Support for more than one ID
@@ -526,6 +577,33 @@ ChunkDemuxer::Status ChunkDemuxer::AddId(const std::string& id,
     return kReachedIdLimit;
 
   source_id_ = id;
+
+  StreamParser::NewBuffersCB audio_cb;
+  StreamParser::NewBuffersCB video_cb;
+
+  if (has_audio) {
+    audio_cb = base::Bind(&ChunkDemuxer::OnAudioBuffers,
+                          base::Unretained(this));
+  }
+
+  if (has_video) {
+    video_cb = base::Bind(&ChunkDemuxer::OnVideoBuffers,
+                          base::Unretained(this));
+  }
+
+  scoped_ptr<StreamParser> stream_parser(factory_function());
+
+  CHECK(stream_parser.get());
+
+  source_buffer_.reset(new SourceBuffer());
+  source_buffer_->Init(
+      stream_parser.Pass(),
+      base::Bind(&ChunkDemuxer::OnSourceBufferInitDone, this),
+      base::Bind(&ChunkDemuxer::OnNewConfigs, base::Unretained(this)),
+      audio_cb,
+      video_cb,
+      base::Bind(&ChunkDemuxer::OnKeyNeeded, base::Unretained(this)));
+
   return kOk;
 }
 
diff --git a/media/filters/chunk_demuxer_unittest.cc b/media/filters/chunk_demuxer_unittest.cc
index eac0d7c..43ec4ad 100644
--- a/media/filters/chunk_demuxer_unittest.cc
+++ b/media/filters/chunk_demuxer_unittest.cc
@@ -1076,4 +1076,32 @@ TEST_F(ChunkDemuxerTest, TestParseErrorDuringInit) {
   ASSERT_TRUE(demuxer_->AppendData(kSourceId, &tmp, 1));
 }
 
+TEST_F(ChunkDemuxerTest, TestAVHeadersWithAudioOnlyType) {
+  EXPECT_CALL(*client_, DemuxerOpened(_));
+  demuxer_->Initialize(
+      &host_, CreateInitDoneCB(kDefaultDuration(),
+                               DEMUXER_ERROR_COULD_NOT_OPEN));
+
+  std::vector<std::string> codecs(1);
+  codecs[0] = "vorbis";
+  ASSERT_EQ(demuxer_->AddId(kSourceId, "audio/webm", codecs),
+            ChunkDemuxer::kOk);
+
+  ASSERT_TRUE(AppendInfoTracks(true, true, false));
+}
+
+TEST_F(ChunkDemuxerTest, TestAVHeadersWithVideoOnlyType) {
+  EXPECT_CALL(*client_, DemuxerOpened(_));
+  demuxer_->Initialize(
+      &host_, CreateInitDoneCB(kDefaultDuration(),
+                               DEMUXER_ERROR_COULD_NOT_OPEN));
+
+  std::vector<std::string> codecs(1);
+  codecs[0] = "vp8";
+  ASSERT_EQ(demuxer_->AddId(kSourceId, "video/webm", codecs),
+            ChunkDemuxer::kOk);
+
+  ASSERT_TRUE(AppendInfoTracks(true, true, false));
+}
+
 }  // namespace media
diff --git a/media/filters/source_buffer.cc b/media/filters/source_buffer.cc
index 4d2fd4f..90f0434 100644
--- a/media/filters/source_buffer.cc
+++ b/media/filters/source_buffer.cc
@@ -14,12 +14,14 @@ SourceBuffer::SourceBuffer() {}
 
 SourceBuffer::~SourceBuffer() {}
 
-void SourceBuffer::Init(const InitCB& init_cb,
+void SourceBuffer::Init(scoped_ptr<StreamParser> parser,
+                        const InitCB& init_cb,
                         const NewConfigCB& config_cb,
                         const NewBuffersCB& audio_cb,
                         const NewBuffersCB& video_cb,
                         const KeyNeededCB& key_needed_cb) {
   DCHECK(init_cb_.is_null());
+  DCHECK(parser.get());
   DCHECK(!init_cb.is_null());
   DCHECK(!config_cb.is_null());
   DCHECK(!audio_cb.is_null() || !video_cb.is_null());
@@ -31,7 +33,7 @@ void SourceBuffer::Init(const InitCB& init_cb,
   video_cb_ = video_cb;
   key_needed_cb_ = key_needed_cb;
 
-  stream_parser_.reset(new WebMStreamParser());
+  stream_parser_.reset(parser.release());
 
   stream_parser_->Init(
       base::Bind(&SourceBuffer::OnStreamParserInitDone, base::Unretained(this)),
@@ -59,6 +61,13 @@ bool SourceBuffer::OnNewConfigs(const AudioDecoderConfig& audio_config,
                     const VideoDecoderConfig& video_config) {
   CHECK(audio_config.IsValidConfig() || video_config.IsValidConfig());
 
+  // Signal an error if we get configuration info for stream types
+  // we don't have a callback to handle.
+  if ((audio_config.IsValidConfig() && audio_cb_.is_null()) ||
+      (video_config.IsValidConfig() && video_cb_.is_null())) {
+    return false;
+  }
+
   return config_cb_.Run(audio_config, video_config);
 }
 
diff --git a/media/filters/source_buffer.h b/media/filters/source_buffer.h
index 35d058c..a03c75e 100644
--- a/media/filters/source_buffer.h
+++ b/media/filters/source_buffer.h
@@ -25,7 +25,8 @@ class MEDIA_EXPORT SourceBuffer {
   typedef base::Callback<bool(const StreamParser::BufferQueue&)> NewBuffersCB;
   typedef base::Callback<bool(scoped_array<uint8>, int)> KeyNeededCB;
 
-  void Init(const InitCB& init_cb,
+  void Init(scoped_ptr<StreamParser> parser,
+            const InitCB& init_cb,
             const NewConfigCB& config_cb,
             const NewBuffersCB& audio_cb,
             const NewBuffersCB& video_cb,