Refactoring StreamParser & StreamParserHost interfaces.

- Moving ByteQueue from ChunkDemuxer into WebMStreamParser to simplify handling
  multiple StreamParsers.
- Replacing OnAudioConfig & OnVideoConfig with a single OnNewConfigs method to
  make it easier to verify that initialization segments all contain the same
  number and type of streams.


BUG=122909
TEST=Existing ChunkDemuxer tests.


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

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

5 files changed, 104 insertions, 92 deletions

diff --git a/media/base/stream_parser.h b/media/base/stream_parser.h
index 8397fad1..daeee4b 100644
--- a/media/base/stream_parser.h
+++ b/media/base/stream_parser.h
@@ -27,13 +27,14 @@ class MEDIA_EXPORT StreamParserHost {
   StreamParserHost();
   virtual ~StreamParserHost();
 
-  // A new audio decoder configuration was encountered. All audio buffers
-  // after this call will be associated with this configuration.
-  virtual bool OnNewAudioConfig(const AudioDecoderConfig& config) = 0;
-
-  // A new video decoder configuration was encountered. All video buffers
-  // after this call will be associated with this configuration.
-  virtual bool OnNewVideoConfig(const VideoDecoderConfig& config) = 0;
+  // New audio and/or video decoder configurations were encountered. All audio
+  // and video buffers after this call will be associated with these
+  // configurations.
+  // Returns true if the new configurations are accepted.
+  // Returns false if the new configurations are not supported and indicates
+  // that a parsing error should be signalled.
+  virtual bool OnNewConfigs(const AudioDecoderConfig& audio_config,
+                            const VideoDecoderConfig& video_config) = 0;
 
   // New audio buffers have been received.
   virtual bool OnAudioBuffers(const BufferQueue& buffers) = 0;
@@ -72,10 +73,8 @@ class MEDIA_EXPORT StreamParser {
 
   // Called when there is new data to parse.
   //
-  // Returns < 0 if the parse fails.
-  // Returns 0 if more data is needed.
-  // Returning > 0 indicates success & the number of bytes parsed.
-  virtual int Parse(const uint8* buf, int size) = 0;
+  // Returns true if the parse succeeds.
+  virtual bool Parse(const uint8* buf, int size) = 0;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(StreamParser);
diff --git a/media/filters/chunk_demuxer.cc b/media/filters/chunk_demuxer.cc
index 25bc0bc..8b82449 100644
--- a/media/filters/chunk_demuxer.cc
+++ b/media/filters/chunk_demuxer.cc
@@ -239,7 +239,7 @@ void ChunkDemuxerStream::Read(const ReadCB& read_cb) {
   {
     base::AutoLock auto_lock(lock_);
 
-    switch(state_) {
+    switch (state_) {
       case RETURNING_DATA_FOR_READS:
         // If we don't have any buffers ready or already have
         // pending reads, then defer this read.
@@ -451,7 +451,6 @@ void ChunkDemuxer::FlushData() {
   if (video_.get())
     video_->Flush();
 
-  byte_queue_.Reset();
   stream_parser_->Flush();
 
   seek_waits_for_data_ = true;
@@ -500,55 +499,34 @@ bool ChunkDemuxer::AppendData(const std::string& id,
   {
     base::AutoLock auto_lock(lock_);
 
-    byte_queue_.Push(data, length);
-
-    const uint8* cur = NULL;
-    int cur_size = 0;
-    int bytes_parsed = 0;
-    int result = -1;
-
     // Capture |seek_waits_for_data_| state before we start parsing.
     // Its state can be changed by OnAudioBuffers() or OnVideoBuffers()
     // calls during the parse.
     bool old_seek_waits_for_data = seek_waits_for_data_;
 
-    byte_queue_.Peek(&cur, &cur_size);
-
-    do {
-      switch(state_) {
-        case INITIALIZING:
-          result = stream_parser_->Parse(cur, cur_size);
-          if (result < 0) {
-            DCHECK_EQ(state_, INITIALIZING);
-            ReportError_Locked(DEMUXER_ERROR_COULD_NOT_OPEN);
-            return true;
-          }
-          break;
-
-        case INITIALIZED: {
-          result = stream_parser_->Parse(cur, cur_size);
-          if (result < 0) {
-            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_;
-          return false;
-      }
+    switch (state_) {
+      case INITIALIZING:
+        if (!stream_parser_->Parse(data, length)) {
+          DCHECK_EQ(state_, INITIALIZING);
+          ReportError_Locked(DEMUXER_ERROR_COULD_NOT_OPEN);
+          return true;
+        }
+        break;
 
-      if (result > 0) {
-        cur += result;
-        cur_size -= result;
-        bytes_parsed += result;
-      }
-    } while (result > 0 && cur_size > 0);
+      case INITIALIZED: {
+        if (!stream_parser_->Parse(data, length)) {
+          ReportError_Locked(PIPELINE_ERROR_DECODE);
+          return true;
+        }
+      } break;
 
-    byte_queue_.Pop(bytes_parsed);
+      case WAITING_FOR_INIT:
+      case ENDED:
+      case PARSE_ERROR:
+      case SHUTDOWN:
+        DVLOG(1) << "AppendData(): called in unexpected state " << state_;
+        return false;
+    }
 
     // Check to see if parsing triggered seek_waits_for_data_ to go from true to
     // false. This indicates we have parsed enough data to complete the seek.
@@ -706,23 +684,27 @@ void ChunkDemuxer::OnStreamParserInitDone(bool success,
   cb.Run(PIPELINE_OK);
 }
 
-bool ChunkDemuxer::OnNewAudioConfig(const AudioDecoderConfig& config) {
+bool ChunkDemuxer::OnNewConfigs(const AudioDecoderConfig& audio_config,
+                                const VideoDecoderConfig& video_config) {
+  CHECK(audio_config.IsValidConfig() || video_config.IsValidConfig());
   lock_.AssertAcquired();
+
   // Only allow a single audio config for now.
-  if (audio_.get())
-    return false;
+  if (audio_config.IsValidConfig()) {
+    if (audio_.get())
+      return false;
 
-  audio_ = new ChunkDemuxerStream(config);
-  return true;
-}
+    audio_ = new ChunkDemuxerStream(audio_config);
+  }
 
-bool ChunkDemuxer::OnNewVideoConfig(const VideoDecoderConfig& config) {
-  lock_.AssertAcquired();
   // Only allow a single video config for now.
-  if (video_.get())
-    return false;
+  if (video_config.IsValidConfig()) {
+    if (video_.get())
+      return false;
+
+    video_ = new ChunkDemuxerStream(video_config);
+  }
 
-  video_ = new ChunkDemuxerStream(config);
   return true;
 }
 
diff --git a/media/filters/chunk_demuxer.h b/media/filters/chunk_demuxer.h
index 3942b23..7c4ae36 100644
--- a/media/filters/chunk_demuxer.h
+++ b/media/filters/chunk_demuxer.h
@@ -87,8 +87,8 @@ class MEDIA_EXPORT ChunkDemuxer : public Demuxer, public StreamParserHost {
   void OnStreamParserInitDone(bool success, base::TimeDelta duration);
 
   // StreamParserHost implementation.
-  virtual bool OnNewAudioConfig(const AudioDecoderConfig& config) OVERRIDE;
-  virtual bool OnNewVideoConfig(const VideoDecoderConfig& config) OVERRIDE;
+  virtual bool OnNewConfigs(const AudioDecoderConfig& audio_config,
+                            const VideoDecoderConfig& video_config) OVERRIDE;
   virtual bool OnAudioBuffers(const BufferQueue& buffer) OVERRIDE;
   virtual bool OnVideoBuffers(const BufferQueue& buffer) OVERRIDE;
 
@@ -113,8 +113,6 @@ class MEDIA_EXPORT ChunkDemuxer : public Demuxer, public StreamParserHost {
   // callback.
   bool seek_waits_for_data_;
 
-  ByteQueue byte_queue_;
-
   // TODO(acolwell): Remove this when fixing http://crbug.com/122909
   std::string source_id_;
 
diff --git a/media/webm/webm_stream_parser.cc b/media/webm/webm_stream_parser.cc
index d986deb..c0b01fc 100644
--- a/media/webm/webm_stream_parser.cc
+++ b/media/webm/webm_stream_parser.cc
@@ -181,43 +181,76 @@ bool FFmpegConfigHelper::SetupStreamConfigs() {
 }
 
 WebMStreamParser::WebMStreamParser()
-    : state_(WAITING_FOR_INIT),
+    : state_(kWaitingForInit),
       host_(NULL) {
 }
 
 WebMStreamParser::~WebMStreamParser() {}
 
 void WebMStreamParser::Init(const InitCB& init_cb, StreamParserHost* host) {
-  DCHECK_EQ(state_, WAITING_FOR_INIT);
+  DCHECK_EQ(state_, kWaitingForInit);
   DCHECK(init_cb_.is_null());
   DCHECK(!host_);
   DCHECK(!init_cb.is_null());
   DCHECK(host);
 
-  ChangeState(PARSING_HEADERS);
+  ChangeState(kParsingHeaders);
   init_cb_ = init_cb;
   host_ = host;
 }
 
 void WebMStreamParser::Flush() {
-  DCHECK_NE(state_, WAITING_FOR_INIT);
+  DCHECK_NE(state_, kWaitingForInit);
 
-  if (state_ != PARSING_CLUSTERS)
+  byte_queue_.Reset();
+
+  if (state_ != kParsingClusters)
     return;
 
   cluster_parser_->Reset();
 }
 
-int WebMStreamParser::Parse(const uint8* buf, int size) {
-  DCHECK_NE(state_, WAITING_FOR_INIT);
+bool WebMStreamParser::Parse(const uint8* buf, int size) {
+  DCHECK_NE(state_, kWaitingForInit);
+
+  if (state_ == kError)
+    return false;
 
-  if (state_ == PARSING_HEADERS)
-    return ParseInfoAndTracks(buf, size);
+  byte_queue_.Push(buf, size);
 
-  if (state_ == PARSING_CLUSTERS)
-    return ParseCluster(buf, size);
+  int result = 0;
+  int bytes_parsed = 0;
+  const uint8* cur = NULL;
+  int cur_size = 0;
+
+  byte_queue_.Peek(&cur, &cur_size);
+  do {
+    switch (state_) {
+      case kParsingHeaders:
+        result = ParseInfoAndTracks(cur, cur_size);
+        break;
+
+      case kParsingClusters:
+        result = ParseCluster(cur, cur_size);
+        break;
+
+      case kWaitingForInit:
+      case kError:
+        return false;
+    }
 
-  return -1;
+    if (result < 0) {
+      ChangeState(kError);
+      return false;
+    }
+
+    cur += result;
+    cur_size -= result;
+    bytes_parsed += result;
+  } while (result > 0 && cur_size > 0);
+
+  byte_queue_.Pop(bytes_parsed);
+  return true;
 }
 
 void WebMStreamParser::ChangeState(State new_state) {
@@ -295,11 +328,8 @@ int WebMStreamParser::ParseInfoAndTracks(const uint8* data, int size) {
   if (!config_helper.Parse(data, bytes_parsed))
     return -1;
 
-  if (config_helper.audio_config().IsValidConfig())
-    host_->OnNewAudioConfig(config_helper.audio_config());
-
-  if (config_helper.video_config().IsValidConfig())
-    host_->OnNewVideoConfig(config_helper.video_config());
+  host_->OnNewConfigs(config_helper.audio_config(),
+                      config_helper.video_config());
 
   cluster_parser_.reset(new WebMClusterParser(
       info_parser.timecode_scale(),
@@ -310,7 +340,7 @@ int WebMStreamParser::ParseInfoAndTracks(const uint8* data, int size) {
       tracks_parser.video_encryption_key_id(),
       tracks_parser.video_encryption_key_id_size()));
 
-  ChangeState(PARSING_CLUSTERS);
+  ChangeState(kParsingClusters);
   init_cb_.Run(true, duration);
   init_cb_.Reset();
 
diff --git a/media/webm/webm_stream_parser.h b/media/webm/webm_stream_parser.h
index 9b45c44..699b469 100644
--- a/media/webm/webm_stream_parser.h
+++ b/media/webm/webm_stream_parser.h
@@ -9,6 +9,7 @@
 #include "base/memory/ref_counted.h"
 #include "media/base/audio_decoder_config.h"
 #include "media/base/buffers.h"
+#include "media/base/byte_queue.h"
 #include "media/base/stream_parser.h"
 #include "media/base/video_decoder_config.h"
 #include "media/webm/webm_cluster_parser.h"
@@ -23,13 +24,14 @@ class WebMStreamParser : public StreamParser {
   // StreamParser implementation.
   virtual void Init(const InitCB& init_cb, StreamParserHost* host) OVERRIDE;
   virtual void Flush() OVERRIDE;
-  virtual int Parse(const uint8* buf, int size) OVERRIDE;
+  virtual bool Parse(const uint8* buf, int size) OVERRIDE;
 
  private:
   enum State {
-    WAITING_FOR_INIT,
-    PARSING_HEADERS,
-    PARSING_CLUSTERS
+    kWaitingForInit,
+    kParsingHeaders,
+    kParsingClusters,
+    kError
   };
 
   void ChangeState(State new_state);
@@ -58,6 +60,7 @@ class WebMStreamParser : public StreamParser {
   StreamParserHost* host_;
 
   scoped_ptr<WebMClusterParser> cluster_parser_;
+  ByteQueue byte_queue_;
 
   DISALLOW_COPY_AND_ASSIGN(WebMStreamParser);
 };