Refactoring StreamParser & StreamParserHost interfaces.

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 d3f532cc9fbb13a601ec18ef57ccef79ed50a859..787c75cd66c507ddac10c06fabb512c3101f79bd 100644
--- a/media/filters/chunk_demuxer.cc
+++ b/media/filters/chunk_demuxer.cc
@@ -448,7 +448,6 @@ void ChunkDemuxer::FlushData() {
   if (video_.get())
     video_->Flush();
 
-  byte_queue_.Reset();
   stream_parser_->Flush();
 
   seek_waits_for_data_ = true;
@@ -468,55 +467,34 @@ bool ChunkDemuxer::AppendData(const uint8* data, size_t length) {
   {
     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.
@@ -674,23 +652,29 @@ 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) {
+  DCHECK(audio_config.IsValidConfig() || video_config.IsValidConfig());
   lock_.AssertAcquired();
-  // Only allow a single audio config for now.
-  if (audio_.get())
+
+  if (!audio_config.IsValidConfig() && !video_config.IsValidConfig())

[scherkus (not reviewing), 2012/04/24 01:40:04]

Considering we handle the DCHECK (and that the previous code did not check for
valid configs) -- does this mean we require a unit test for this code path?

If so, then it's tricky because we'll DCHECK so either:
  1) Remove the DCHECK and passing in two invalid configs is valid + write test
  2) Promote the DCHECK to a CHECK and remove this handler

[acolwell GONE FROM CHROMIUM, 2012/04/24 16:25:55]

Done. Picked option 2

     return false;
 
-  audio_ = new ChunkDemuxerStream(config);
-  return true;
-}
+  // Only allow a single audio config for now.

[scherkus (not reviewing), 2012/04/24 01:40:04]

nit: you can either dupe this comment below for video or just include video in
this comment OR if you add LOG(ERROR) (or similar) for the false case the code
should be self documenting

[acolwell GONE FROM CHROMIUM, 2012/04/24 16:25:55]

Done.

+  if (audio_config.IsValidConfig()) {
+    if (audio_.get())
+      return false;
 
-bool ChunkDemuxer::OnNewVideoConfig(const VideoDecoderConfig& config) {
-  lock_.AssertAcquired();
-  // Only allow a single video config for now.
-  if (video_.get())
-    return false;
+    audio_ = new ChunkDemuxerStream(audio_config);
+  }
+
+  if (video_config.IsValidConfig()) {
+    if (video_.get())
+      return false;
+
+    video_ = new ChunkDemuxerStream(video_config);
+  }
 
-  video_ = new ChunkDemuxerStream(config);
   return true;
 }