Revert 115229 - Revert 115214 (caused media_unittests failure in FFmpegGlueTest,

suspiciously immediately after the code modified in this test.)

FFmpegGlueTest failure was a flake.

Fix ChunkDemuxer seeks that occur during a partially parsed cluster.


BUG=104160
TEST=ChunkDemuxerTest.TestSeekWhileParsingCluster, WebMParserTest.TestReset


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

TBR=acolwell@chromium.org
Review URL: http://codereview.chromium.org/8989041

TBR=dmazzoni@chromium.org
Review URL: http://codereview.chromium.org/9019020

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

7 files changed, 158 insertions, 33 deletions

diff --git a/media/base/byte_queue.cc b/media/base/byte_queue.cc
index 8f4fee8..e91bfb7 100644
--- a/media/base/byte_queue.cc
+++ b/media/base/byte_queue.cc
@@ -20,6 +20,11 @@ ByteQueue::ByteQueue()
 
 ByteQueue::~ByteQueue() {}
 
+void ByteQueue::Reset() {
+  offset_ = 0;
+  used_ = 0;
+}
+
 void ByteQueue::Push(const uint8* data, int size) {
   DCHECK(data);
   DCHECK_GT(size, 0);
diff --git a/media/base/byte_queue.h b/media/base/byte_queue.h
index 69f2728..d3ef605 100644
--- a/media/base/byte_queue.h
+++ b/media/base/byte_queue.h
@@ -20,6 +20,9 @@ class ByteQueue {
   ByteQueue();
   ~ByteQueue();
 
+  // Reset the queue to the empty state.
+  void Reset();
+
   // Appends new bytes onto the end of the queue.
   void Push(const uint8* data, int size);
 
diff --git a/media/filters/chunk_demuxer.cc b/media/filters/chunk_demuxer.cc
index cda484b..64fad53 100644
--- a/media/filters/chunk_demuxer.cc
+++ b/media/filters/chunk_demuxer.cc
@@ -410,6 +410,9 @@ void ChunkDemuxer::FlushData() {
   if (video_.get())
     video_->Flush();
 
+  byte_queue_.Reset();
+  cluster_parser_->Reset();
+
   seek_waits_for_data_ = true;
   ChangeState_Locked(INITIALIZED);
 }
diff --git a/media/filters/chunk_demuxer_unittest.cc b/media/filters/chunk_demuxer_unittest.cc
index 1b4719f..4f628d6 100644
--- a/media/filters/chunk_demuxer_unittest.cc
+++ b/media/filters/chunk_demuxer_unittest.cc
@@ -16,7 +16,6 @@ using ::testing::AnyNumber;
 using ::testing::InSequence;
 using ::testing::Return;
 using ::testing::SetArgumentPointee;
-using ::testing::NiceMock;
 using ::testing::_;
 
 namespace media {
@@ -32,6 +31,11 @@ static const int kTracksSizeOffset = 4;
 static const int kVideoTrackNum = 1;
 static const int kAudioTrackNum = 2;
 
+MATCHER_P(HasTimestamp, timestamp_in_ms, "") {
+  return !arg->IsEndOfStream() &&
+      arg->GetTimestamp().InMilliseconds() == timestamp_in_ms;
+}
+
 class MockChunkDemuxerClient : public ChunkDemuxerClient {
  public:
   MockChunkDemuxerClient() {}
@@ -192,6 +196,14 @@ class ChunkDemuxerTest : public testing::Test {
     cb->AddSimpleBlock(track_num, timecode, 0, data.get(), size);
   }
 
+  MOCK_METHOD1(ReadDone, void(const scoped_refptr<Buffer>&));
+
+  void ExpectRead(DemuxerStream* stream, int64 timestamp_in_ms) {
+    EXPECT_CALL(*this, ReadDone(HasTimestamp(timestamp_in_ms)));
+    stream->Read(base::Bind(&ChunkDemuxerTest::ReadDone,
+                            base::Unretained(this)));
+  }
+
   MOCK_METHOD1(Checkpoint, void(int id));
 
   MockDemuxerHost mock_demuxer_host_;
@@ -277,6 +289,57 @@ TEST_F(ChunkDemuxerTest, TestAppendDataAfterSeek) {
   Checkpoint(2);
 }
 
+// Test the case where a Seek() is requested while the parser
+// is in the middle of cluster. This is to verify that the parser
+// resets itself on seek and is in the right state when data from
+// the new seek point arrives.
+TEST_F(ChunkDemuxerTest, TestSeekWhileParsingCluster) {
+  InitDemuxer(true, true);
+
+  scoped_refptr<DemuxerStream> audio =
+      demuxer_->GetStream(DemuxerStream::AUDIO);
+  scoped_refptr<DemuxerStream> video =
+      demuxer_->GetStream(DemuxerStream::VIDEO);
+
+  InSequence s;
+
+  ClusterBuilder cb;
+  cb.SetClusterTimecode(0);
+  AddSimpleBlock(&cb, kAudioTrackNum, 1);
+  AddSimpleBlock(&cb, kVideoTrackNum, 2);
+  AddSimpleBlock(&cb, kAudioTrackNum, 10);
+  AddSimpleBlock(&cb, kVideoTrackNum, 20);
+  scoped_ptr<Cluster> cluster_a(cb.Finish());
+
+  cb.SetClusterTimecode(5000);
+  AddSimpleBlock(&cb, kAudioTrackNum, 5000);
+  AddSimpleBlock(&cb, kVideoTrackNum, 5005);
+  AddSimpleBlock(&cb, kAudioTrackNum, 5007);
+  AddSimpleBlock(&cb, kVideoTrackNum, 5035);
+  scoped_ptr<Cluster> cluster_b(cb.Finish());
+
+  // Append all but the last byte so that everything but
+  // the last block can be parsed.
+  AppendData(cluster_a->data(), cluster_a->size() - 1);
+
+  ExpectRead(audio, 1);
+  ExpectRead(video, 2);
+  ExpectRead(audio, 10);
+
+  demuxer_->FlushData();
+  demuxer_->Seek(base::TimeDelta::FromSeconds(5),
+                 NewExpectedStatusCB(PIPELINE_OK));
+
+
+  // Append the new cluster and verify that only the blocks
+  // in the new cluster are returned.
+  AppendData(cluster_b->data(), cluster_b->size());
+  ExpectRead(audio, 5000);
+  ExpectRead(video, 5005);
+  ExpectRead(audio, 5007);
+  ExpectRead(video, 5035);
+}
+
 // Test the case where AppendData() is called before Init().
 TEST_F(ChunkDemuxerTest, TestAppendDataBeforeInit) {
   scoped_array<uint8> info_tracks;
@@ -334,30 +397,30 @@ TEST_F(ChunkDemuxerTest, TestOutOfOrderClusters) {
   AddSimpleBlock(&cb, kVideoTrackNum, 10);
   AddSimpleBlock(&cb, kAudioTrackNum, 33);
   AddSimpleBlock(&cb, kVideoTrackNum, 43);
-  scoped_ptr<Cluster> clusterA(cb.Finish());
+  scoped_ptr<Cluster> cluster_a(cb.Finish());
 
-  AppendData(clusterA->data(), clusterA->size());
+  AppendData(cluster_a->data(), cluster_a->size());
 
-  // Cluster B starts before clusterA and has data
+  // Cluster B starts before cluster_a and has data
   // that overlaps.
   cb.SetClusterTimecode(5);
   AddSimpleBlock(&cb, kAudioTrackNum, 5);
   AddSimpleBlock(&cb, kVideoTrackNum, 7);
   AddSimpleBlock(&cb, kAudioTrackNum, 28);
   AddSimpleBlock(&cb, kVideoTrackNum, 40);
-  scoped_ptr<Cluster> clusterB(cb.Finish());
+  scoped_ptr<Cluster> cluster_b(cb.Finish());
 
   // Make sure that AppendData() fails because this cluster data
   // is before previous data.
   EXPECT_CALL(mock_demuxer_host_, OnDemuxerError(PIPELINE_ERROR_DECODE));
-  AppendData(clusterB->data(), clusterB->size());
+  AppendData(cluster_b->data(), cluster_b->size());
 
   // Verify that AppendData() doesn't accept more data now.
   cb.SetClusterTimecode(45);
   AddSimpleBlock(&cb, kAudioTrackNum, 45);
   AddSimpleBlock(&cb, kVideoTrackNum, 45);
-  scoped_ptr<Cluster> clusterC(cb.Finish());
-  EXPECT_FALSE(demuxer_->AppendData(clusterC->data(), clusterC->size()));
+  scoped_ptr<Cluster> cluster_c(cb.Finish());
+  EXPECT_FALSE(demuxer_->AppendData(cluster_c->data(), cluster_c->size()));
 }
 
 TEST_F(ChunkDemuxerTest, TestNonMonotonicButAboveClusterTimecode) {
@@ -372,17 +435,17 @@ TEST_F(ChunkDemuxerTest, TestNonMonotonicButAboveClusterTimecode) {
   AddSimpleBlock(&cb, kVideoTrackNum, 10);
   AddSimpleBlock(&cb, kAudioTrackNum, 7);
   AddSimpleBlock(&cb, kVideoTrackNum, 15);
-  scoped_ptr<Cluster> clusterA(cb.Finish());
+  scoped_ptr<Cluster> cluster_a(cb.Finish());
 
   EXPECT_CALL(mock_demuxer_host_, OnDemuxerError(PIPELINE_ERROR_DECODE));
-  AppendData(clusterA->data(), clusterA->size());
+  AppendData(cluster_a->data(), cluster_a->size());
 
   // Verify that AppendData() doesn't accept more data now.
   cb.SetClusterTimecode(20);
   AddSimpleBlock(&cb, kAudioTrackNum, 20);
   AddSimpleBlock(&cb, kVideoTrackNum, 20);
-  scoped_ptr<Cluster> clusterB(cb.Finish());
-  EXPECT_FALSE(demuxer_->AppendData(clusterB->data(), clusterB->size()));
+  scoped_ptr<Cluster> cluster_b(cb.Finish());
+  EXPECT_FALSE(demuxer_->AppendData(cluster_b->data(), cluster_b->size()));
 }
 
 TEST_F(ChunkDemuxerTest, TestBackwardsAndBeforeClusterTimecode) {
@@ -397,17 +460,17 @@ TEST_F(ChunkDemuxerTest, TestBackwardsAndBeforeClusterTimecode) {
   AddSimpleBlock(&cb, kVideoTrackNum, 5);
   AddSimpleBlock(&cb, kAudioTrackNum, 3);
   AddSimpleBlock(&cb, kVideoTrackNum, 3);
-  scoped_ptr<Cluster> clusterA(cb.Finish());
+  scoped_ptr<Cluster> cluster_a(cb.Finish());
 
   EXPECT_CALL(mock_demuxer_host_, OnDemuxerError(PIPELINE_ERROR_DECODE));
-  AppendData(clusterA->data(), clusterA->size());
+  AppendData(cluster_a->data(), cluster_a->size());
 
   // Verify that AppendData() doesn't accept more data now.
   cb.SetClusterTimecode(6);
   AddSimpleBlock(&cb, kAudioTrackNum, 6);
   AddSimpleBlock(&cb, kVideoTrackNum, 6);
-  scoped_ptr<Cluster> clusterB(cb.Finish());
-  EXPECT_FALSE(demuxer_->AppendData(clusterB->data(), clusterB->size()));
+  scoped_ptr<Cluster> cluster_b(cb.Finish());
+  EXPECT_FALSE(demuxer_->AppendData(cluster_b->data(), cluster_b->size()));
 }
 
 
@@ -439,24 +502,24 @@ TEST_F(ChunkDemuxerTest, TestMonotonicallyIncreasingTimestampsAcrossClusters) {
   cb.SetClusterTimecode(5);
   AddSimpleBlock(&cb, kAudioTrackNum, 5);
   AddSimpleBlock(&cb, kVideoTrackNum, 5);
-  scoped_ptr<Cluster> clusterA(cb.Finish());
+  scoped_ptr<Cluster> cluster_a(cb.Finish());
 
-  AppendData(clusterA->data(), clusterA->size());
+  AppendData(cluster_a->data(), cluster_a->size());
 
   cb.SetClusterTimecode(5);
   AddSimpleBlock(&cb, kAudioTrackNum, 5);
   AddSimpleBlock(&cb, kVideoTrackNum, 7);
-  scoped_ptr<Cluster> clusterB(cb.Finish());
+  scoped_ptr<Cluster> cluster_b(cb.Finish());
 
   EXPECT_CALL(mock_demuxer_host_, OnDemuxerError(PIPELINE_ERROR_DECODE));
-  AppendData(clusterB->data(), clusterB->size());
+  AppendData(cluster_b->data(), cluster_b->size());
 
   // Verify that AppendData() doesn't accept more data now.
   cb.SetClusterTimecode(10);
   AddSimpleBlock(&cb, kAudioTrackNum, 10);
   AddSimpleBlock(&cb, kVideoTrackNum, 10);
-  scoped_ptr<Cluster> clusterC(cb.Finish());
-  EXPECT_FALSE(demuxer_->AppendData(clusterC->data(), clusterC->size()));
+  scoped_ptr<Cluster> cluster_c(cb.Finish());
+  EXPECT_FALSE(demuxer_->AppendData(cluster_c->data(), cluster_c->size()));
 }
 
 // Test the case where a cluster is passed to AppendData() before
@@ -670,24 +733,24 @@ TEST_F(ChunkDemuxerTest, TestAppendingInPieces) {
   cb.SetClusterTimecode(0);
   AddSimpleBlock(&cb, kAudioTrackNum, 32, 512);
   AddSimpleBlock(&cb, kVideoTrackNum, 123, 1024);
-  scoped_ptr<Cluster> clusterA(cb.Finish());
+  scoped_ptr<Cluster> cluster_a(cb.Finish());
 
   cb.SetClusterTimecode(125);
   AddSimpleBlock(&cb, kAudioTrackNum, 125, 2048);
   AddSimpleBlock(&cb, kVideoTrackNum, 150, 2048);
-  scoped_ptr<Cluster> clusterB(cb.Finish());
+  scoped_ptr<Cluster> cluster_b(cb.Finish());
 
-  size_t buffer_size = info_tracks_size + clusterA->size() + clusterB->size();
+  size_t buffer_size = info_tracks_size + cluster_a->size() + cluster_b->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, cluster_a->data(), cluster_a->size());
+  dst += cluster_a->size();
 
-  memcpy(dst, clusterB->data(), clusterB->size());
-  dst += clusterB->size();
+  memcpy(dst, cluster_b->data(), cluster_b->size());
+  dst += cluster_b->size();
 
   AppendDataInPieces(buffer.get(), buffer_size);
 
diff --git a/media/webm/webm_cluster_parser.cc b/media/webm/webm_cluster_parser.cc
index fd716b2..fc64176 100644
--- a/media/webm/webm_cluster_parser.cc
+++ b/media/webm/webm_cluster_parser.cc
@@ -33,6 +33,14 @@ WebMClusterParser::WebMClusterParser(int64 timecode_scale,
 
 WebMClusterParser::~WebMClusterParser() {}
 
+void WebMClusterParser::Reset() {
+  audio_buffers_.clear();
+  video_buffers_.clear();
+  last_block_timecode_ = -1;
+  cluster_timecode_ = -1;
+  parser_.Reset();
+}
+
 int WebMClusterParser::Parse(const uint8* buf, int size) {
   audio_buffers_.clear();
   video_buffers_.clear();
diff --git a/media/webm/webm_cluster_parser.h b/media/webm/webm_cluster_parser.h
index 7fb1b94..445d207 100644
--- a/media/webm/webm_cluster_parser.h
+++ b/media/webm/webm_cluster_parser.h
@@ -25,6 +25,9 @@ class WebMClusterParser : public WebMParserClient {
                     base::TimeDelta video_default_duration);
   virtual ~WebMClusterParser();
 
+  // Resets the parser state so it can accept a new cluster.
+  void Reset();
+
   // Parses a WebM cluster element in |buf|.
   //
   // Returns -1 if the parse fails.
diff --git a/media/webm/webm_parser_unittest.cc b/media/webm/webm_parser_unittest.cc
index e377fda..3c8600b 100644
--- a/media/webm/webm_parser_unittest.cc
+++ b/media/webm/webm_parser_unittest.cc
@@ -60,11 +60,13 @@ static Cluster* CreateCluster(int timecode,
 static void CreateClusterExpectations(int timecode,
                                       const SimpleBlockInfo* block_info,
                                       int block_count,
+                                      bool is_complete_cluster,
                                       MockWebMParserClient* client) {
 
   InSequence s;
   EXPECT_CALL(*client, OnListStart(kWebMIdCluster)).WillOnce(Return(true));
-  EXPECT_CALL(*client, OnUInt(kWebMIdTimecode, 0)).WillOnce(Return(true));
+  EXPECT_CALL(*client, OnUInt(kWebMIdTimecode, timecode))
+      .WillOnce(Return(true));
 
   for (int i = 0; i < block_count; i++) {
     EXPECT_CALL(*client, OnSimpleBlock(block_info[i].track_num,
@@ -73,7 +75,8 @@ static void CreateClusterExpectations(int timecode,
         .WillOnce(Return(true));
   }
 
-  EXPECT_CALL(*client, OnListEnd(kWebMIdCluster)).WillOnce(Return(true));
+  if (is_complete_cluster)
+    EXPECT_CALL(*client, OnListEnd(kWebMIdCluster)).WillOnce(Return(true));
 }
 
 TEST_F(WebMParserTest, EmptyCluster) {
@@ -211,7 +214,7 @@ TEST_F(WebMParserTest, ParseListElementWithSingleCall) {
   int block_count = arraysize(kBlockInfo);
 
   scoped_ptr<Cluster> cluster(CreateCluster(0, kBlockInfo, block_count));
-  CreateClusterExpectations(0, kBlockInfo, block_count, &client_);
+  CreateClusterExpectations(0, kBlockInfo, block_count, true, &client_);
 
   WebMListParser parser(kWebMIdCluster);
   int result = parser.Parse(cluster->data(), cluster->size(), &client_);
@@ -230,7 +233,7 @@ TEST_F(WebMParserTest, ParseListElementWithMultipleCalls) {
   int block_count = arraysize(kBlockInfo);
 
   scoped_ptr<Cluster> cluster(CreateCluster(0, kBlockInfo, block_count));
-  CreateClusterExpectations(0, kBlockInfo, block_count, &client_);
+  CreateClusterExpectations(0, kBlockInfo, block_count, true, &client_);
 
   const uint8* data = cluster->data();
   int size = cluster->size();
@@ -261,4 +264,41 @@ TEST_F(WebMParserTest, ParseListElementWithMultipleCalls) {
   EXPECT_TRUE(parser.IsParsingComplete());
 }
 
+TEST_F(WebMParserTest, TestReset) {
+  InSequence s;
+
+  const SimpleBlockInfo kBlockInfo[] = {
+    { 0, 1 },
+    { 1, 2 },
+    { 0, 3 },
+    { 0, 4 },
+    { 1, 4 },
+  };
+  int block_count = arraysize(kBlockInfo);
+
+  scoped_ptr<Cluster> cluster(CreateCluster(0, kBlockInfo, block_count));
+
+  // First expect all but the last block.
+  CreateClusterExpectations(0, kBlockInfo, block_count - 1, false, &client_);
+
+  // Now expect all blocks.
+  CreateClusterExpectations(0, kBlockInfo, block_count, true, &client_);
+
+  WebMListParser parser(kWebMIdCluster);
+
+  // Send slightly less than the full cluster so all but the last block is
+  // parsed.
+  int result = parser.Parse(cluster->data(), cluster->size() - 1, &client_);
+  EXPECT_GT(result, 0);
+  EXPECT_LT(result, cluster->size());
+  EXPECT_FALSE(parser.IsParsingComplete());
+
+  parser.Reset();
+
+  // Now parse a whole cluster to verify that all the blocks will get parsed.
+  result = parser.Parse(cluster->data(), cluster->size(), &client_);
+  EXPECT_EQ(result, cluster->size());
+  EXPECT_TRUE(parser.IsParsingComplete());
+}
+
 }  // namespace media