Implement support for 2 source ids (1 for audio and 1 for video).

media/filters/chunk_demuxer.cc

Index: media/filters/chunk_demuxer.cc
diff --git a/media/filters/chunk_demuxer.cc b/media/filters/chunk_demuxer.cc
index 8862ad15fa50cf4f3d07f7716a68f017be04c686..9c0f427c81bfca515f343612e4509aa5fee7364c 100644
--- a/media/filters/chunk_demuxer.cc
+++ b/media/filters/chunk_demuxer.cc
@@ -362,9 +362,7 @@ ChunkDemuxer::ChunkDemuxer(ChunkDemuxerClient* client)
     : state_(WAITING_FOR_INIT),
       host_(NULL),
       client_(client),
-      buffered_bytes_(0),
-      has_audio_(false),
-      has_video_(false) {
+      buffered_bytes_(0) {
   DCHECK(client);
 }
 
@@ -465,6 +463,11 @@ ChunkDemuxer::Status ChunkDemuxer::AddId(const std::string& id,
                                          const std::string& type,
                                          std::vector<std::string>& codecs) {
   DCHECK_GT(codecs.size(), 0u);
+  {
+    base::AutoLock auto_lock(lock_);

[acolwell GONE FROM CHROMIUM, 2012/06/08 00:29:07]

I think we should apply the lock to the whole method. I'm not sure why I left
the lock out originally. That was a mistake.

[annacc, 2012/06/08 22:12:23]

Done.

+    if (state_ != WAITING_FOR_INIT && state_ != INITIALIZING)
+      return kReachedIdLimit;
+  }
 
   bool has_audio = false;
   bool has_video = false;
@@ -472,55 +475,56 @@ ChunkDemuxer::Status ChunkDemuxer::AddId(const std::string& id,
   if (!IsSupported(type, codecs, &factory_function, &has_audio, &has_video))
     return kNotSupported;
 
-  // TODO(acolwell): Support for more than one ID
-  // will be added as part of http://crbug.com/122909
-  if (!source_id_.empty() ||
-      (has_audio && has_audio_) ||
-      (has_video && has_video_))
+  if ((has_audio && !source_id_audio_.empty()) ||
+      (has_video && !source_id_video_.empty()))
     return kReachedIdLimit;
 
-  source_id_ = id;
-
   StreamParser::NewBuffersCB audio_cb;
   StreamParser::NewBuffersCB video_cb;
 
   if (has_audio) {
-    has_audio_ = true;
+    source_id_audio_ = id;
     audio_cb = base::Bind(&ChunkDemuxer::OnAudioBuffers,
                           base::Unretained(this));
   }
 
   if (has_video) {
-    has_video_ = true;
+    source_id_video_ = id;
     video_cb = base::Bind(&ChunkDemuxer::OnVideoBuffers,
                           base::Unretained(this));
   }
 
-  stream_parser_.reset(factory_function());
-  CHECK(stream_parser_.get());
+  scoped_ptr<StreamParser> stream_parser(factory_function());
+  CHECK(stream_parser.get());
 
-  stream_parser_->Init(
+  stream_parser->Init(
       base::Bind(&ChunkDemuxer::OnStreamParserInitDone, this),
-      base::Bind(&ChunkDemuxer::OnNewConfigs, base::Unretained(this)),
+      base::Bind(&ChunkDemuxer::OnNewConfigs, base::Unretained(this),
+                 has_audio, has_video),
       audio_cb,
       video_cb,
       base::Bind(&ChunkDemuxer::OnKeyNeeded, base::Unretained(this)));
 
+  stream_parser_map_[id] = stream_parser.release();
+
   return kOk;
 }
 
 void ChunkDemuxer::RemoveId(const std::string& id) {
-  CHECK(!source_id_.empty());
-  CHECK_EQ(source_id_, id);
-  source_id_ = "";
-  has_audio_ = false;
-  has_video_ = false;
+  CHECK_GT(stream_parser_map_.count(id), 0u);
+  stream_parser_map_.erase(id);

[acolwell GONE FROM CHROMIUM, 2012/06/08 00:29:07]

I think this leaks the parser.

[annacc, 2012/06/08 22:12:23]

Good catch, thanks!

+
+  if (source_id_audio_ == id && audio_)
+    audio_->Shutdown();
+
+  if (source_id_video_ == id && video_)
+    video_->Shutdown();
 }
 
 bool ChunkDemuxer::GetBufferedRanges(const std::string& id,
                                      Ranges* ranges_out) const {
   DCHECK(!id.empty());
-  DCHECK_EQ(source_id_, id);
+  DCHECK_GT(stream_parser_map_.count(id), 0u);
   DCHECK(ranges_out);
 
   base::AutoLock auto_lock(lock_);
@@ -535,15 +539,14 @@ bool ChunkDemuxer::AppendData(const std::string& id,
   DVLOG(1) << "AppendData(" << id << ", " << length << ")";
 
   // TODO(acolwell): Remove when http://webk.it/83788 fix lands.
-  if (source_id_.empty()) {
+  if (stream_parser_map_.count(id) == 0) {

[acolwell GONE FROM CHROMIUM, 2012/06/08 00:29:07]

This whole block & the comment can be remove. The necessary patch has landed.

[annacc, 2012/06/08 22:12:23]

Hmm, seems like it is still needed.  Are you sure this has been fixed?

[acolwell GONE FROM CHROMIUM, 2012/06/11 16:09:33]

Yes. The only test that I saw break when I removed this in my local checkout was
ChunkDemuxerTest.TestAppendDataBeforeInit . This test just needs to be updated
with an AddId() before the CreateInitSegment() call. Are there other breakages
you are seeing?

     std::vector<std::string> codecs(2);
     codecs[0] = "vp8";
     codecs[1] = "vorbis";
     AddId(id, "video/webm", codecs);
   }
 
-  DCHECK(!source_id_.empty());
-  DCHECK_EQ(source_id_, id);
+  DCHECK_GT(stream_parser_map_.count(id), 0u);
   DCHECK(!id.empty());
   DCHECK(data);
   DCHECK_GT(length, 0u);
@@ -559,7 +562,7 @@ bool ChunkDemuxer::AppendData(const std::string& id,
 
     switch (state_) {
       case INITIALIZING:
-        if (!stream_parser_->Parse(data, length)) {
+        if (!stream_parser_map_[id]->Parse(data, length)) {
           DCHECK_EQ(state_, INITIALIZING);
           ReportError_Locked(DEMUXER_ERROR_COULD_NOT_OPEN);
           return true;
@@ -567,7 +570,7 @@ bool ChunkDemuxer::AppendData(const std::string& id,
         break;
 
       case INITIALIZED: {
-        if (!stream_parser_->Parse(data, length)) {
+        if (!stream_parser_map_[id]->Parse(data, length)) {
           ReportError_Locked(PIPELINE_ERROR_DECODE);
           return true;
         }
@@ -606,9 +609,9 @@ bool ChunkDemuxer::AppendData(const std::string& id,
 void ChunkDemuxer::Abort(const std::string& id) {
   DVLOG(1) << "Abort(" << id << ")";
   DCHECK(!id.empty());
-  DCHECK_EQ(source_id_, id);
+  DCHECK_GT(stream_parser_map_.count(id), 0u);
 
-  stream_parser_->Flush();
+  stream_parser_map_[id]->Flush();
 }
 
 bool ChunkDemuxer::EndOfStream(PipelineStatus status) {
@@ -659,7 +662,7 @@ void ChunkDemuxer::Shutdown() {
     if (video_)
       video_->Shutdown();
 
-    stream_parser_.reset();
+    stream_parser_map_.clear();

[acolwell GONE FROM CHROMIUM, 2012/06/08 00:29:07]

This is a memory leak. You need to iterate over the map and delete each element.

[annacc, 2012/06/08 22:12:23]

Whoops.  Done.

 
     ChangeState_Locked(SHUTDOWN);
   }
@@ -743,6 +746,11 @@ void ChunkDemuxer::OnStreamParserInitDone(bool success,
     return;
   }
 
+  // Wait until all streams have initialized.
+  if ((!source_id_audio_.empty() && !audio_) ||
+      (!source_id_video_.empty() && !video_))
+    return;

[annacc, 2012/06/07 22:42:07]

This check is the source of a lot of the AddId() changes in the unittest.

+
   duration_ = duration;

[acolwell GONE FROM CHROMIUM, 2012/06/08 00:29:07]

I wonder if we should move the duration_ assignment before the conditional
above. Perhaps always making duration_ be the max duration we see.

[annacc, 2012/06/08 22:12:23]

Oh, good idea.

   host_->SetDuration(duration_);
 
@@ -752,7 +760,8 @@ void ChunkDemuxer::OnStreamParserInitDone(bool success,
   cb.Run(PIPELINE_OK);
 }
 
-bool ChunkDemuxer::OnNewConfigs(const AudioDecoderConfig& audio_config,
+bool ChunkDemuxer::OnNewConfigs(bool has_audio, bool has_video,
+                                const AudioDecoderConfig& audio_config,
                                 const VideoDecoderConfig& video_config) {
   CHECK(audio_config.IsValidConfig() || video_config.IsValidConfig());
   lock_.AssertAcquired();
@@ -760,11 +769,11 @@ bool ChunkDemuxer::OnNewConfigs(const AudioDecoderConfig& audio_config,
   // Signal an error if we get configuration info for stream types that weren't
   // specified in AddId() or more configs after a stream is initialized.
   // Only allow a single audio config for now.
-  if (audio_config.IsValidConfig() && (!has_audio_ || audio_))
+  if (audio_config.IsValidConfig() && (!has_audio || audio_))
     return false;
 
   // Only allow a single video config for now.
-  if (video_config.IsValidConfig() && (!has_video_ || video_))
+  if (video_config.IsValidConfig() && (!has_video || video_))
     return false;
 
   if (audio_config.IsValidConfig())