MSE: Make WebMClusterParser hold back buffers at or beyond buffer missing duration

media/filters/chunk_demuxer_unittest.cc

Index: media/filters/chunk_demuxer_unittest.cc
diff --git a/media/filters/chunk_demuxer_unittest.cc b/media/filters/chunk_demuxer_unittest.cc
index e477eee1d982fb954978dfaedbd5e8127befb66e..06506c33e2ceff9954ba35510ad3403e077f2c45 100644
--- a/media/filters/chunk_demuxer_unittest.cc
+++ b/media/filters/chunk_demuxer_unittest.cc
@@ -1235,10 +1235,14 @@ TEST_P(ChunkDemuxerTest, SeekWhileParsingCluster) {
   ExpectRead(DemuxerStream::AUDIO, 0);
   ExpectRead(DemuxerStream::VIDEO, 0);
   ExpectRead(DemuxerStream::AUDIO, kAudioBlockDuration);
-  // Note: We skip trying to read a video buffer here because computing
-  // the duration for this block relies on successfully parsing the last block
-  // in the cluster the cluster.
-  ExpectRead(DemuxerStream::AUDIO, 2 * kAudioBlockDuration);
+
+  // Note: No further reads are expected to be able to complete at this point.
+  // Video read cannot complete because computing the duration for the next
+  // video buffer relies on parsing the next video block or reaching cluster
+  // end. Audio read cannot complete because the next audio buffer, though
+  // parsed and having duration, is held back by the parser because it has
+  // timestamp (0.046s), which is higher than the currently held-back video
+  // buffer's timestamp (0.033s).

[acolwell GONE FROM CHROMIUM, 2014/04/23 22:36:39]

nit: What is the purpose of this comment? It doesn't appear to be relevant to
what is being tested. I'd say drop it since none of the code below appears to be
taking action based on this information.

[wolenetz, 2014/04/25 20:04:21]

Done.

 
   Seek(base::TimeDelta::FromSeconds(5));
 
@@ -1766,24 +1770,20 @@ TEST_P(ChunkDemuxerTest, IncrementalClusterParsing) {
   EXPECT_FALSE(video_read_done);
 
   // Append data one byte at a time until the audio read completes.
+  // Initial (timestamp 0) audio and video reads should complete in the same
+  // iteration since neither buffer originally had an embedded duration, so were
+  // mutually holding back each other until enough data was available for the
+  // parser to determine durations for each.

[acolwell GONE FROM CHROMIUM, 2014/04/23 22:36:39]

nit: I don't think it is worth having this here either since the audio
completing before the video isn't really the primary point of this test. The
primary goal is to make sure that buffers are incrementally handed out even if
the whole cluster isn't available.

[wolenetz, 2014/04/25 20:04:21]

Done.

   int i = 0;
   for (; i < cluster->size() && !audio_read_done; ++i) {
+    EXPECT_FALSE(video_read_done);

[acolwell GONE FROM CHROMIUM, 2014/04/23 22:36:39]

nit: Remove since this test really shouldn't concern itself about the specific
order?

[wolenetz, 2014/04/25 20:04:21]

Done.

     AppendData(cluster->data() + i, 1);
     message_loop_.RunUntilIdle();
   }
 
   EXPECT_TRUE(audio_read_done);
-  EXPECT_FALSE(video_read_done);
-  EXPECT_GT(i, 0);
-  EXPECT_LT(i, cluster->size());
-
-  // Append data one byte at a time until the video read completes.
-  for (; i < cluster->size() && !video_read_done; ++i) {
-    AppendData(cluster->data() + i, 1);
-    message_loop_.RunUntilIdle();
-  }
-
   EXPECT_TRUE(video_read_done);
+  EXPECT_GT(i, 0);
   EXPECT_LT(i, cluster->size());
 
   audio_read_done = false;