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

media/filters/chunk_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/chunk_demuxer.h"
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "media/base/audio_decoder_config.h"
@@ skipping 344 matching lines @@
       return;
     closures->push_back(base::Bind(read_cbs_.front(), buffer));
     read_cbs_.pop_front();
   }
 }
 
 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);
 }
 
 void ChunkDemuxer::Initialize(DemuxerHost* host,
                               const PipelineStatusCB& cb) {
   DVLOG(1) << "Init()";
   {
     base::AutoLock auto_lock(lock_);
     DCHECK_EQ(state_, WAITING_FOR_INIT);
     host_ = host;
@@ skipping 80 matching lines @@
   if (video_)
     video_->StartWaitingForSeek();
 
   ChangeState_Locked(INITIALIZED);
 }
 
 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;
   ParserFactoryFunction factory_function = NULL;
   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_);
 
   // TODO(annacc): Calculate buffered ranges (http://crbug.com/129852 ).
   return false;
 }
 
 bool ChunkDemuxer::AppendData(const std::string& id,
                               const uint8* data,
                               size_t length) {
   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);
 
   int64 buffered_bytes = 0;
 
   PipelineStatusCB cb;
   {
     base::AutoLock auto_lock(lock_);
 
     // Capture if the SourceBuffer has a pending seek before we start parsing.
     bool old_seek_pending = IsSeekPending_Locked();
 
     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;
         }
         break;
 
       case INITIALIZED: {
-        if (!stream_parser_->Parse(data, length)) {
+        if (!stream_parser_map_[id]->Parse(data, length)) {
           ReportError_Locked(PIPELINE_ERROR_DECODE);
           return true;
         }
       } break;
 
       case WAITING_FOR_INIT:
       case ENDED:
       case PARSE_ERROR:
       case SHUTDOWN:
         DVLOG(1) << "AppendData(): called in unexpected state " << state_;
@@ skipping 18 matching lines @@
 
   if (!cb.is_null())
     cb.Run(PIPELINE_OK);
 
   return true;
 }
 
 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) {
   DVLOG(1) << "EndOfStream(" << status << ")";
   base::AutoLock auto_lock(lock_);
   DCHECK_NE(state_, WAITING_FOR_INIT);
   DCHECK_NE(state_, ENDED);
 
   if (state_ == SHUTDOWN || state_ == PARSE_ERROR)
     return true;
@@ skipping 30 matching lines @@
       return;
 
     std::swap(cb, seek_cb_);
 
     if (audio_)
       audio_->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);
   }
 
   if (!cb.is_null())
     cb.Run(PIPELINE_ERROR_ABORT);
 
   client_->DemuxerClosed();
 }
 
 void ChunkDemuxer::ChangeState_Locked(State new_state) {
   lock_.AssertAcquired();
   DVLOG(1) << "ChunkDemuxer::ChangeState_Locked() : "
            << state_ << " -> " << new_state;
   state_ = new_state;
 }
 
 ChunkDemuxer::~ChunkDemuxer() {
   DCHECK_NE(state_, INITIALIZED);

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

You need a loop here that deletes all the parsers in the stream_parser_map_.

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

Done.

 }
 
 void ChunkDemuxer::ReportError_Locked(PipelineStatus error) {
   DVLOG(1) << "ReportError_Locked(" << error << ")";
   lock_.AssertAcquired();
   DCHECK_NE(error, PIPELINE_OK);
 
   ChangeState_Locked(PARSE_ERROR);
 
   PipelineStatusCB cb;
@@ skipping 44 matching lines @@
                                           base::TimeDelta duration) {
   DVLOG(1) << "OnSourceBufferInitDone(" << success << ", "
            << duration.InSecondsF() << ")";
   lock_.AssertAcquired();
   DCHECK_EQ(state_, INITIALIZING);
   if (!success || (!audio_ && !video_)) {
     ReportError_Locked(DEMUXER_ERROR_COULD_NOT_OPEN);
     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_);
 
   ChangeState_Locked(INITIALIZED);
   PipelineStatusCB cb;
   std::swap(cb, init_cb_);
   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();
 
   // 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())
     audio_ = new ChunkDemuxerStream(audio_config);
 
   if (video_config.IsValidConfig())
     video_ = new ChunkDemuxerStream(video_config);
 
   return true;
 }
@@ skipping 22 matching lines @@
   return true;
 }
 
 bool ChunkDemuxer::OnKeyNeeded(scoped_array<uint8> init_data,
                                int init_data_size) {
   client_->KeyNeeded(init_data.Pass(), init_data_size);
   return true;
 }
 
 }  // namespace media