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

media/filters/chunk_demuxer_unittest.cc

 // Copyright (c) 2012 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 <algorithm>
 
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
@@ skipping 1217 matching lines @@
   // Split the cluster into two appends at an arbitrary point near the end.
   int first_append_size = cluster_a->size() - 11;
   int second_append_size = cluster_a->size() - first_append_size;
 
   // Append the first part of the cluster.
   AppendData(cluster_a->data(), first_append_size);
 
   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));
 
   // Append the rest of the cluster.
   AppendData(cluster_a->data() + first_append_size, second_append_size);
 
   // Append the new cluster and verify that only the blocks
   // in the new cluster are returned.
   AppendCluster(GenerateCluster(5000, 6));
   GenerateExpectedReads(5000, 6);
@@ skipping 507 matching lines @@
                        &audio_read_done));
   ReadVideo(base::Bind(&OnReadDone,
                        base::TimeDelta::FromMilliseconds(0),
                        &video_read_done));
 
   // Make sure the reads haven't completed yet.
   EXPECT_FALSE(audio_read_done);
   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_TRUE(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_LT(i, cluster->size());
-
   audio_read_done = false;
   video_read_done = false;
   ReadAudio(base::Bind(&OnReadDone,
                        base::TimeDelta::FromMilliseconds(23),
                        &audio_read_done));
   ReadVideo(base::Bind(&OnReadDone,
                        base::TimeDelta::FromMilliseconds(33),
                        &video_read_done));
 
   // Make sure the reads haven't completed yet.
@@ skipping 1414 matching lines @@
   // Verify that audio & video streams continue to return expected values.
   CheckExpectedBuffers(audio_stream, "160 180");
   CheckExpectedBuffers(video_stream, "180 210");
 }
 
 // TODO(wolenetz): Enable testing of new frame processor based on this flag,
 // once the new processor has landed. See http://crbug.com/249422.
 INSTANTIATE_TEST_CASE_P(LegacyFrameProcessor, ChunkDemuxerTest, Values(true));
 
 }  // namespace media