Improve ChunkDemuxer so AppendData() calls no longer have to be done on element boundries.

BUG=86536
TEST=ChunkDemuxerTest::TestAppendingInPieces


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

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

11 files changed, 415 insertions, 134 deletions

diff --git a/media/base/byte_queue.cc b/media/base/byte_queue.cc
new file mode 100644
index 0000000..8f4fee8
--- /dev/null
+++ b/media/base/byte_queue.cc
@@ -0,0 +1,79 @@
+// Copyright (c) 2011 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/base/byte_queue.h"
+
+#include "base/logging.h"
+
+namespace media {
+
+// Default starting size for the queue.
+enum { kDefaultQueueSize = 1024 };
+
+ByteQueue::ByteQueue()
+    : buffer_(new uint8[kDefaultQueueSize]),
+      size_(kDefaultQueueSize),
+      offset_(0),
+      used_(0) {
+}
+
+ByteQueue::~ByteQueue() {}
+
+void ByteQueue::Push(const uint8* data, int size) {
+  DCHECK(data);
+  DCHECK_GT(size, 0);
+
+  size_t size_needed = used_ + size;
+
+  // Check to see if we need a bigger buffer.
+  if (size_needed > size_) {
+    size_t new_size = 2 * size_;
+    while (size_needed > new_size && new_size > size_)
+      new_size *= 2;
+
+    // Sanity check to make sure we didn't overflow.
+    CHECK_GT(new_size, size_);
+
+    scoped_array<uint8> new_buffer(new uint8[new_size]);
+
+    // Copy the data from the old buffer to the start of the new one.
+    if (used_ > 0)
+      memcpy(new_buffer.get(), front(), used_);
+
+    buffer_.reset(new_buffer.release());
+    size_ = new_size;
+    offset_ = 0;
+  } else if ((offset_ + used_ + size) > size_) {
+    // The buffer is big enough, but we need to move the data in the queue.
+    memmove(buffer_.get(), front(), used_);
+    offset_ = 0;
+  }
+
+  memcpy(front() + used_, data, size);
+  used_ += size;
+}
+
+void ByteQueue::Peek(const uint8** data, int* size) const {
+  DCHECK(data);
+  DCHECK(size);
+  *data = front();
+  *size = used_;
+}
+
+void ByteQueue::Pop(int count) {
+  DCHECK_LE(count, used_);
+
+  offset_ += count;
+  used_ -= count;
+
+  // Move the offset back to 0 if we have reached the end of the buffer.
+  if (offset_ == size_) {
+    DCHECK_EQ(used_, 0);
+    offset_ = 0;
+  }
+}
+
+uint8* ByteQueue::front() const { return buffer_.get() + offset_; }
+
+}  // namespace media
diff --git a/media/base/byte_queue.h b/media/base/byte_queue.h
new file mode 100644
index 0000000..69f2728
--- /dev/null
+++ b/media/base/byte_queue.h
@@ -0,0 +1,54 @@
+// Copyright (c) 2011 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.
+
+#ifndef MEDIA_BASE_BYTE_QUEUE_H_
+#define MEDIA_BASE_BYTE_QUEUE_H_
+
+#include "base/basictypes.h"
+#include "base/memory/scoped_ptr.h"
+
+namespace media {
+
+// Represents a queue of bytes.
+// Data is added to the end of the queue via an Push() call and removed via
+// Pop(). The contents of the queue can be observed via the Peek() method.
+// This class manages the underlying storage of the queue and tries to minimize
+// the number of buffer copies when data is appended and removed.
+class ByteQueue {
+ public:
+  ByteQueue();
+  ~ByteQueue();
+
+  // Appends new bytes onto the end of the queue.
+  void Push(const uint8* data, int size);
+
+  // Get a pointer to the front of the queue and the queue size.
+  // These values are only valid until the next Push() or
+  // Pop() call.
+  void Peek(const uint8** data, int* size) const;
+
+  // Remove |count| bytes from the front of the queue.
+  void Pop(int count);
+
+ private:
+  // Returns a pointer to the front of the queue.
+  uint8* front() const;
+
+  scoped_array<uint8> buffer_;
+
+  // Size of |buffer_|.
+  size_t size_;
+
+  // Offset from the start of |buffer_| that marks the front of the queue.
+  size_t offset_;
+
+  // Number of bytes stored in the queue.
+  int used_;
+
+  DISALLOW_COPY_AND_ASSIGN(ByteQueue);
+};
+
+}  // namespace media
+
+#endif  // MEDIA_BASE_BYTE_QUEUE_H_
diff --git a/media/filters/chunk_demuxer.cc b/media/filters/chunk_demuxer.cc
index ea066d8..d8e0604 100644
--- a/media/filters/chunk_demuxer.cc
+++ b/media/filters/chunk_demuxer.cc
@@ -322,7 +322,7 @@ void ChunkDemuxer::Init(const PipelineStatusCB& cb) {
     base::AutoLock auto_lock(lock_);
     DCHECK_EQ(state_, WAITING_FOR_INIT);
 
-    ChangeState(INITIALIZING);
+    ChangeState_Locked(INITIALIZING);
     init_cb_ = cb;
   }
 
@@ -402,13 +402,14 @@ void ChunkDemuxer::FlushData() {
     video_->Flush();
 
   seek_waits_for_data_ = true;
-  ChangeState(INITIALIZED);
+  ChangeState_Locked(INITIALIZED);
 }
 
-bool ChunkDemuxer::AppendData(const uint8* data, unsigned length) {
+bool ChunkDemuxer::AppendData(const uint8* data, size_t length) {
   VLOG(1) << "AppendData(" << length << ")";
-  DCHECK(data);
-  DCHECK_GT(length, 0u);
+
+  if (!data || length == 0u)
+    return false;
 
   int64 buffered_bytes = 0;
   base::TimeDelta buffered_ts = base::TimeDelta::FromSeconds(-1);
@@ -416,32 +417,62 @@ bool ChunkDemuxer::AppendData(const uint8* data, unsigned length) {
   PipelineStatusCB cb;
   {
     base::AutoLock auto_lock(lock_);
-    switch(state_) {
-      case INITIALIZING:
-        if (!ParseInfoAndTracks_Locked(data, length)) {
-          VLOG(1) << "AppendData(): parsing info & tracks failed";
-          ReportError_Locked(DEMUXER_ERROR_COULD_NOT_OPEN);
-        }
-        return true;
-        break;
-
-      case INITIALIZED:
-        if (!ParseAndAppendData_Locked(data, length)) {
-          VLOG(1) << "AppendData(): parsing data failed";
-          ReportError_Locked(PIPELINE_ERROR_DECODE);
-          return true;
-        }
-        break;
-
-      case WAITING_FOR_INIT:
-      case ENDED:
-      case PARSE_ERROR:
-      case SHUTDOWN:
-        VLOG(1) << "AppendData(): called in unexpected state " << state_;
-        return false;
-    }
 
-    seek_waits_for_data_ = false;
+    byte_queue_.Push(data, length);
+
+    const uint8* cur = NULL;
+    int cur_size = 0;
+    int bytes_parsed = 0;
+    int result = -1;
+    bool parsed_a_cluster = false;
+
+    byte_queue_.Peek(&cur, &cur_size);
+
+    do {
+      switch(state_) {
+        case INITIALIZING:
+          result = ParseInfoAndTracks_Locked(cur, cur_size);
+          if (result < 0) {
+            VLOG(1) << "AppendData(): parsing info & tracks failed";
+            ReportError_Locked(DEMUXER_ERROR_COULD_NOT_OPEN);
+            return true;
+          }
+          break;
+
+        case INITIALIZED:
+          result = ParseCluster_Locked(cur, cur_size);
+          if (result < 0) {
+            VLOG(1) << "AppendData(): parsing data failed";
+            ReportError_Locked(PIPELINE_ERROR_DECODE);
+            return true;
+          }
+
+          parsed_a_cluster = (result > 0);
+          break;
+
+        case WAITING_FOR_INIT:
+        case ENDED:
+        case PARSE_ERROR:
+        case SHUTDOWN:
+          VLOG(1) << "AppendData(): called in unexpected state " << state_;
+          return false;
+      }
+
+      if (result > 0) {
+        cur += result;
+        cur_size -= result;
+        bytes_parsed += result;
+      }
+    } while (result > 0 && cur_size > 0);
+
+    byte_queue_.Pop(bytes_parsed);
+
+    if (parsed_a_cluster && seek_waits_for_data_) {
+      seek_waits_for_data_ = false;
+
+      if (!seek_cb_.is_null())
+        std::swap(cb, seek_cb_);
+    }
 
     base::TimeDelta tmp;
     if (audio_.get() && audio_->GetLastBufferTimestamp(&tmp) &&
@@ -455,9 +486,6 @@ bool ChunkDemuxer::AppendData(const uint8* data, unsigned length) {
     }
 
     buffered_bytes = buffered_bytes_;
-
-    if (!seek_cb_.is_null())
-      std::swap(cb, seek_cb_);
   }
 
   // Notify the host of 'network activity' because we got data.
@@ -491,7 +519,7 @@ void ChunkDemuxer::EndOfStream(PipelineStatus status) {
     return;
   }
 
-  ChangeState(ENDED);
+  ChangeState_Locked(ENDED);
 
   if (status != PIPELINE_OK) {
     ReportError_Locked(status);
@@ -531,7 +559,7 @@ void ChunkDemuxer::Shutdown() {
     if (video_.get())
       video_->Shutdown();
 
-    ChangeState(SHUTDOWN);
+    ChangeState_Locked(SHUTDOWN);
   }
 
   if (!cb.is_null())
@@ -540,12 +568,13 @@ void ChunkDemuxer::Shutdown() {
   client_->DemuxerClosed();
 }
 
-void ChunkDemuxer::ChangeState(State new_state) {
+void ChunkDemuxer::ChangeState_Locked(State new_state) {
   lock_.AssertAcquired();
   state_ = new_state;
 }
 
-bool ChunkDemuxer::ParseInfoAndTracks_Locked(const uint8* data, int size) {
+int ChunkDemuxer::ParseInfoAndTracks_Locked(const uint8* data, int size) {
+  lock_.AssertAcquired();
   DCHECK(data);
   DCHECK_GT(size, 0);
 
@@ -553,20 +582,24 @@ bool ChunkDemuxer::ParseInfoAndTracks_Locked(const uint8* data, int size) {
 
   const uint8* cur = data;
   int cur_size = size;
+  int bytes_parsed = 0;
   WebMInfoParser info_parser;
-  int res = info_parser.Parse(cur, cur_size);
+  int result = info_parser.Parse(cur, cur_size);
 
-  if (res <= 0)
-    return false;
+  if (result <= 0)
+    return result;
 
-  cur += res;
-  cur_size -= res;
+  cur += result;
+  cur_size -= result;
+  bytes_parsed += result;
 
   WebMTracksParser tracks_parser(info_parser.timecode_scale());
-  res = tracks_parser.Parse(cur, cur_size);
+  result = tracks_parser.Parse(cur, cur_size);
 
-  if (res <= 0)
-    return false;
+  if (result <= 0)
+    return result;
+
+  bytes_parsed += result;
 
   double mult = info_parser.timecode_scale() / 1000.0;
   duration_ = base::TimeDelta::FromMicroseconds(info_parser.duration() * mult);
@@ -578,16 +611,14 @@ bool ChunkDemuxer::ParseInfoAndTracks_Locked(const uint8* data, int size) {
       tracks_parser.video_track_num(),
       tracks_parser.video_default_duration()));
 
-  format_context_ = CreateFormatContext(data, size);
-
-  if (!format_context_ || !SetupStreams()) {
-    return false;
-  }
+  format_context_ = CreateFormatContext(data, bytes_parsed);
+  if (!format_context_ || !SetupStreams())
+    return -1;
 
-  ChangeState(INITIALIZED);
+  ChangeState_Locked(INITIALIZED);
   init_cb_.Run(PIPELINE_OK);
   init_cb_.Reset();
-  return true;
+  return bytes_parsed;
 }
 
 AVFormatContext* ChunkDemuxer::CreateFormatContext(const uint8* data,
@@ -656,51 +687,43 @@ bool ChunkDemuxer::SetupStreams() {
   return !no_supported_streams;
 }
 
-bool ChunkDemuxer::ParseAndAppendData_Locked(const uint8* data, int length) {
+int ChunkDemuxer::ParseCluster_Locked(const uint8* data, int size) {
+  lock_.AssertAcquired();
   if (!cluster_parser_.get())
-    return false;
+    return -1;
 
-  const uint8* cur = data;
-  int cur_size = length;
+  int bytes_parsed = cluster_parser_->Parse(data, size);
 
-  while (cur_size > 0) {
-    int res = cluster_parser_->Parse(cur, cur_size);
+  if (bytes_parsed <= 0)
+    return bytes_parsed;
 
-    if (res <= 0) {
-      VLOG(1) << "ParseAndAppendData_Locked() : cluster parsing failed.";
-      return false;
-    }
-
-    // Make sure we can add the buffers to both streams before we acutally
-    // add them. This allows us to accept all of the data or none of it.
-    if ((audio_.get() &&
-         !audio_->CanAddBuffers(cluster_parser_->audio_buffers())) ||
-        (video_.get() &&
-         !video_->CanAddBuffers(cluster_parser_->video_buffers()))) {
-      return false;
-    }
-
-    if (audio_.get())
-      audio_->AddBuffers(cluster_parser_->audio_buffers());
+  // Make sure we can add the buffers to both streams before we actutally
+  // add them. This allows us to accept all of the data or none of it.
+  if ((audio_.get() &&
+       !audio_->CanAddBuffers(cluster_parser_->audio_buffers())) ||
+      (video_.get() &&
+       !video_->CanAddBuffers(cluster_parser_->video_buffers()))) {
+    return -1;
+  }
 
-    if (video_.get())
-      video_->AddBuffers(cluster_parser_->video_buffers());
+  if (audio_.get())
+    audio_->AddBuffers(cluster_parser_->audio_buffers());
 
-    cur += res;
-    cur_size -= res;
-  }
+  if (video_.get())
+    video_->AddBuffers(cluster_parser_->video_buffers());
 
   // TODO(acolwell) : make this more representative of what is actually
   // buffered.
-  buffered_bytes_ += length;
+  buffered_bytes_ += bytes_parsed;
 
-  return true;
+  return bytes_parsed;
 }
 
 void ChunkDemuxer::ReportError_Locked(PipelineStatus error) {
+  lock_.AssertAcquired();
   DCHECK_NE(error, PIPELINE_OK);
 
-  ChangeState(PARSE_ERROR);
+  ChangeState_Locked(PARSE_ERROR);
 
   PipelineStatusCB cb;
 
diff --git a/media/filters/chunk_demuxer.h b/media/filters/chunk_demuxer.h
index 14f6fb5..8f09761 100644
--- a/media/filters/chunk_demuxer.h
+++ b/media/filters/chunk_demuxer.h
@@ -8,6 +8,7 @@
 #include <list>
 
 #include "base/synchronization/lock.h"
+#include "media/base/byte_queue.h"
 #include "media/base/demuxer.h"
 #include "media/webm/webm_cluster_parser.h"
 
@@ -42,7 +43,7 @@ class MEDIA_EXPORT ChunkDemuxer : public Demuxer {
 
   // Appends media data to the stream. Returns false if this method
   // is called in an invalid state.
-  bool AppendData(const uint8* data, unsigned length);
+  bool AppendData(const uint8* data, size_t length);
   void EndOfStream(PipelineStatus status);
   bool HasEnded();
   void Shutdown();
@@ -57,12 +58,15 @@ class MEDIA_EXPORT ChunkDemuxer : public Demuxer {
     SHUTDOWN,
   };
 
-  void ChangeState(State new_state);
+  void ChangeState_Locked(State new_state);
 
-  // Parses a buffer that contains an INFO & TRACKS element. Returns false if
-  // the parse fails. This method handles calling & clearing |init_cb_|
-  // before it returns.
-  bool ParseInfoAndTracks_Locked(const uint8* data, int size);
+  // Parses a buffer that contains an INFO & TRACKS element. This method handles
+  // calling & clearing |init_cb_| before it returns.
+  //
+  // Returns -1 if the parse fails.
+  // Returns 0 if more data is needed.
+  // Returns the number of bytes parsed on success.
+  int ParseInfoAndTracks_Locked(const uint8* data, int size);
 
   // Generates an AVFormatContext for the INFO & TRACKS elements contained
   // in |data|. Returns NULL if parsing |data| fails.
@@ -73,9 +77,13 @@ class MEDIA_EXPORT ChunkDemuxer : public Demuxer {
   // found.
   bool SetupStreams();
 
-  // Parse a buffer that was passed to AppendData(). |data| is expected to
-  // contain one or more WebM Clusters. Returns false if parsing the data fails.
-  bool ParseAndAppendData_Locked(const uint8* data, int length);
+  // Parse a cluster add add the buffers to the appropriate DemxuerStream.
+  // |data| is expected to point to the beginning of a cluster element.
+  //
+  // Returns -1 if the parse fails.
+  // Returns 0 if more data is needed.
+  // Returns the number of bytes parsed on success.
+  int ParseCluster_Locked(const uint8* data, int size);
 
   // Reports an error and puts the demuxer in a state where it won't accept more
   // data.
@@ -112,6 +120,8 @@ class MEDIA_EXPORT ChunkDemuxer : public Demuxer {
   // callback.
   bool seek_waits_for_data_;
 
+  ByteQueue byte_queue_;
+
   DISALLOW_COPY_AND_ASSIGN(ChunkDemuxer);
 };
 
diff --git a/media/filters/chunk_demuxer_unittest.cc b/media/filters/chunk_demuxer_unittest.cc
index 6b0ee3b..dd3dbd98 100644
--- a/media/filters/chunk_demuxer_unittest.cc
+++ b/media/filters/chunk_demuxer_unittest.cc
@@ -113,18 +113,29 @@ class ChunkDemuxerTest : public testing::Test{
     }
   }
 
-  void AppendData(const uint8* data, unsigned length) {
+  void AppendData(const uint8* data, size_t length) {
     EXPECT_CALL(mock_filter_host_, SetBufferedBytes(_)).Times(AnyNumber());
     EXPECT_CALL(mock_filter_host_, SetBufferedTime(_)).Times(AnyNumber());
     EXPECT_CALL(mock_filter_host_, SetNetworkActivity(true)).Times(AnyNumber());
     EXPECT_TRUE(demuxer_->AppendData(data, length));
   }
 
+  void AppendDataInPieces(const uint8* data, size_t length) {
+    const uint8* start = data;
+    const uint8* end = data + length;
+    size_t default_size = 7;
+    while (start < end) {
+      size_t append_size = std::min(default_size,
+                                    static_cast<size_t>(end - start));
+      AppendData(start, append_size);
+      start += append_size;
+    }
+  }
+
   void AppendInfoTracks(bool has_audio, bool has_video) {
     scoped_array<uint8> info_tracks;
     int info_tracks_size = 0;
     CreateInfoTracks(has_audio, has_video, &info_tracks, &info_tracks_size);
-
     AppendData(info_tracks.get(), info_tracks_size);
   }
 
@@ -166,6 +177,12 @@ class ChunkDemuxerTest : public testing::Test{
     cb->AddSimpleBlock(track_num, timecode, 0, data, sizeof(data));
   }
 
+  void AddSimpleBlock(ClusterBuilder* cb, int track_num, int64 timecode,
+                      int size) {
+    scoped_array<uint8> data(new uint8[size]);
+    cb->AddSimpleBlock(track_num, timecode, 0, data.get(), size);
+  }
+
   MOCK_METHOD1(Checkpoint, void(int id));
 
   MockFilterHost mock_filter_host_;
@@ -629,4 +646,78 @@ TEST_F(ChunkDemuxerTest, TestReadsAfterEndOfStream) {
   end_of_stream_helper_3.CheckIfReadDonesWereCalled(true);
 }
 
+// Make sure AppendData() will accept elements that span multiple calls.
+TEST_F(ChunkDemuxerTest, TestAppendingInPieces) {
+
+  EXPECT_CALL(*client_, DemuxerOpened(_));
+  demuxer_->Init(NewExpectedStatusCB(PIPELINE_OK));
+
+  EXPECT_CALL(mock_filter_host_, SetDuration(_));
+  EXPECT_CALL(mock_filter_host_, SetCurrentReadPosition(_));
+  demuxer_->set_host(&mock_filter_host_);
+
+  scoped_array<uint8> info_tracks;
+  int info_tracks_size = 0;
+  CreateInfoTracks(true, true, &info_tracks, &info_tracks_size);
+
+  ClusterBuilder cb;
+  cb.SetClusterTimecode(0);
+  AddSimpleBlock(&cb, kAudioTrackNum, 32, 512);
+  AddSimpleBlock(&cb, kVideoTrackNum, 123, 1024);
+  scoped_ptr<Cluster> clusterA(cb.Finish());
+
+  cb.SetClusterTimecode(125);
+  AddSimpleBlock(&cb, kAudioTrackNum, 125, 2048);
+  AddSimpleBlock(&cb, kVideoTrackNum, 150, 2048);
+  scoped_ptr<Cluster> clusterB(cb.Finish());
+
+  size_t buffer_size = info_tracks_size + clusterA->size() + clusterB->size();
+  scoped_array<uint8> buffer(new uint8[buffer_size]);
+  uint8* dst = buffer.get();
+  memcpy(dst, info_tracks.get(), info_tracks_size);
+  dst += info_tracks_size;
+
+  memcpy(dst, clusterA->data(), clusterA->size());
+  dst += clusterA->size();
+
+  memcpy(dst, clusterB->data(), clusterB->size());
+  dst += clusterB->size();
+
+  AppendDataInPieces(buffer.get(), buffer_size);
+
+  scoped_refptr<DemuxerStream> audio =
+      demuxer_->GetStream(DemuxerStream::AUDIO);
+  scoped_refptr<DemuxerStream> video =
+      demuxer_->GetStream(DemuxerStream::VIDEO);
+
+  ASSERT_TRUE(audio);
+  ASSERT_TRUE(video);
+
+  bool audio_read_done = false;
+  bool video_read_done = false;
+  audio->Read(base::Bind(&OnReadDone,
+                         base::TimeDelta::FromMilliseconds(32),
+                         &audio_read_done));
+
+  video->Read(base::Bind(&OnReadDone,
+                         base::TimeDelta::FromMilliseconds(123),
+                         &video_read_done));
+
+  EXPECT_TRUE(audio_read_done);
+  EXPECT_TRUE(video_read_done);
+
+  audio_read_done = false;
+  video_read_done = false;
+  audio->Read(base::Bind(&OnReadDone,
+                         base::TimeDelta::FromMilliseconds(125),
+                         &audio_read_done));
+
+  video->Read(base::Bind(&OnReadDone,
+                         base::TimeDelta::FromMilliseconds(150),
+                         &video_read_done));
+
+  EXPECT_TRUE(audio_read_done);
+  EXPECT_TRUE(video_read_done);
+}
+
 }  // namespace media
diff --git a/media/media.gyp b/media/media.gyp
index 598378b..3b49e7b 100644
--- a/media/media.gyp
+++ b/media/media.gyp
@@ -89,6 +89,8 @@
         'base/bitstream_buffer.h',
         'base/buffers.cc',
         'base/buffers.h',
+        'base/byte_queue.cc',
+        'base/byte_queue.h',
         'base/channel_layout.cc',
         'base/channel_layout.h',
         'base/clock.cc',
diff --git a/media/webm/webm_cluster_parser.h b/media/webm/webm_cluster_parser.h
index 0266086..ee1caf8 100644
--- a/media/webm/webm_cluster_parser.h
+++ b/media/webm/webm_cluster_parser.h
@@ -27,8 +27,9 @@ class WebMClusterParser : public WebMParserClient {
 
   // Parses a WebM cluster element in |buf|.
   //
-  // Returns the number of bytes parsed on success. Returns -1
-  // if a parse error occurs.
+  // Returns -1 if the parse fails.
+  // Returns 0 if more data is needed.
+  // Returns the number of bytes parsed on success.
   int Parse(const uint8* buf, int size);
 
   const BufferQueue& audio_buffers() const { return audio_buffers_; }
diff --git a/media/webm/webm_info_parser.h b/media/webm/webm_info_parser.h
index 6391f8b..37cf01c 100644
--- a/media/webm/webm_info_parser.h
+++ b/media/webm/webm_info_parser.h
@@ -17,8 +17,9 @@ class WebMInfoParser : public WebMParserClient {
 
   // Parses a WebM Info element in |buf|.
   //
-  // Returns the number of bytes parsed on success. Returns -1
-  // on error.
+  // Returns -1 if the parse fails.
+  // Returns 0 if more data is needed.
+  // Returns the number of bytes parsed on success.
   int Parse(const uint8* buf, int size);
 
   int64 timecode_scale() const { return timecode_scale_; }
diff --git a/media/webm/webm_parser.cc b/media/webm/webm_parser.cc
index 083ebff..43dd509 100644
--- a/media/webm/webm_parser.cc
+++ b/media/webm/webm_parser.cc
@@ -144,9 +144,12 @@ static int ParseWebMElementHeaderField(const uint8* buf, int size,
   DCHECK(buf);
   DCHECK(num);
 
-  if (size <= 0)
+  if (size < 0)
     return -1;
 
+  if (size == 0)
+    return 0;
+
   int mask = 0x80;
   uint8 ch = buf[0];
   int extra_bytes = -1;
@@ -159,9 +162,13 @@ static int ParseWebMElementHeaderField(const uint8* buf, int size,
     mask >>= 1;
   }
 
-  if ((extra_bytes == -1) || ((1 + extra_bytes) > size))
+  if (extra_bytes == -1)
     return -1;
 
+  // Return 0 if we need more data.
+  if ((1 + extra_bytes) > size)
+    return 0;
+
   int bytes_used = 1;
 
   for (int i = 0; i < extra_bytes; ++i)
@@ -281,20 +288,20 @@ static int ParseElementList(const uint8* buf, int size,
   if (!client->OnListStart(id))
     return -1;
 
-  int res = ParseElements(list_info->id_info_,
-                          list_info->id_info_size_,
-                          buf, size,
-                          level + 1,
-                          client);
+  int result = ParseElements(list_info->id_info_,
+                             list_info->id_info_size_,
+                             buf, size,
+                             level + 1,
+                             client);
 
-  if (res < 0)
-    return -1;
+  if (result <= 0)
+    return result;
 
   if (!client->OnListEnd(id))
     return -1;
 
-  DCHECK_EQ(res, size);
-  return res;
+  DCHECK_EQ(result, size);
+  return result;
 }
 
 static int ParseUInt(const uint8* buf, int size, int id,
@@ -370,21 +377,18 @@ static int ParseElements(const ElementIdInfo* id_info,
   while (cur_size > 0) {
     int id = 0;
     int64 element_size = 0;
-    int res = ParseWebMElementHeader(cur, cur_size, &id, &element_size);
-
-    if (res < 0)
-      return res;
+    int result = ParseWebMElementHeader(cur, cur_size, &id, &element_size);
 
-    if (res == 0)
-      break;
+    if (result <= 0)
+      return result;
 
-    cur += res;
-    cur_size -= res;
-    used += res;
+    cur += result;
+    cur_size -= result;
+    used += result;
 
     // Check to see if the element is larger than the remaining data.
     if (element_size > cur_size)
-      return -1;
+      return 0;
 
     const ElementIdInfo* info = FindIdInfo(id, id_info, id_info_size);
 
@@ -452,34 +456,45 @@ static int ParseElements(const ElementIdInfo* id_info,
 // buffer points to an element that does not match |id|.
 int WebMParseListElement(const uint8* buf, int size, int id,
                          int level, WebMParserClient* client) {
-  if (size == 0)
+  if (size < 0)
     return -1;
 
+  if (size == 0)
+    return 0;
+
   const uint8* cur = buf;
   int cur_size = size;
-
+  int bytes_parsed = 0;
   int element_id = 0;
   int64 element_size = 0;
-  int res = ParseWebMElementHeader(cur, cur_size, &element_id, &element_size);
+  int result = ParseWebMElementHeader(cur, cur_size, &element_id,
+                                      &element_size);
 
-  if (res <= 0)
-    return res;
+  if (result <= 0)
+    return result;
 
-  cur += res;
-  cur_size -= res;
+  cur += result;
+  cur_size -= result;
+  bytes_parsed += result;
 
-  if (element_id != id || element_size > cur_size)
+  if (element_id != id)
     return -1;
 
-  res = ParseElementList(cur, element_size, element_id, level, client);
+  if (element_size > cur_size)
+    return 0;
 
-  if (res < 0)
-    return -1;
+  if (element_size > 0) {
+    result = ParseElementList(cur, element_size, element_id, level, client);
 
-  cur += res;
-  cur_size -= res;
+    if (result <= 0)
+      return result;
+
+    cur += result;
+    cur_size -= result;
+    bytes_parsed += result;
+  }
 
-  return size - cur_size;
+  return bytes_parsed;
 }
 
 }  // namespace media
diff --git a/media/webm/webm_parser.h b/media/webm/webm_parser.h
index 0716b80..92dd679 100644
--- a/media/webm/webm_parser.h
+++ b/media/webm/webm_parser.h
@@ -38,6 +38,10 @@ class WebMParserClient {
 
 // Parses a single list element that matches |id|. This method fails if the
 // buffer points to an element that does not match |id|.
+//
+// Returns -1 if the parse fails.
+// Returns 0 if more data is needed.
+// Returns the number of bytes parsed on success.
 int WebMParseListElement(const uint8* buf, int size, int id,
                          int level, WebMParserClient* client);
 
diff --git a/media/webm/webm_tracks_parser.h b/media/webm/webm_tracks_parser.h
index a1d0081..02dc10d 100644
--- a/media/webm/webm_tracks_parser.h
+++ b/media/webm/webm_tracks_parser.h
@@ -19,8 +19,9 @@ class WebMTracksParser : public WebMParserClient {
 
   // Parses a WebM Tracks element in |buf|.
   //
-  // Returns the number of bytes parsed on success. Returns -1
-  // on error.
+  // Returns -1 if the parse fails.
+  // Returns 0 if more data is needed.
+  // Returns the number of bytes parsed on success.
   int Parse(const uint8* buf, int size);
 
   int64 audio_track_num() const { return audio_track_num_; }