Update ChunkDemuxer unit tests to use real muxed clusters.

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 770f28423be05d6bf4cccd8e9d1c18aa38ff5971..200d0d908a970f62035582c49be5db1d6dbb58c0 100644
--- a/media/filters/chunk_demuxer_unittest.cc
+++ b/media/filters/chunk_demuxer_unittest.cc
@@ -397,60 +397,107 @@ class ChunkDemuxerTest : public ::testing::Test {
                       timecode, end_timecode, track_number, block_duration));
   }
 
-  // |cluster_description| - A space delimited string of buffer info that
-  //  is used to construct a cluster. Each buffer info is a timestamp in
-  //  milliseconds and optionally followed by a 'K' to indicate that a buffer
-  //  should be marked as a keyframe. For example "0K 30 60" should constuct
-  //  a cluster with 3 blocks: a keyframe with timestamp 0 and 2 non-keyframes
-  //  at 30ms and 60ms.
-  void AppendSingleStreamCluster(const std::string& source_id, int track_number,
-                                 const std::string& cluster_description) {
+  struct BlockInfo {
+    int track_number;
+    int timestamp_in_ms;
+    int flags;
+    int duration;
+
+    bool operator< (const BlockInfo& rhs) const {
+      // We want pop() to return blocks in timestamp increasing order

[wolenetz, 2014/07/08 19:33:53]

nit: This could become confusing later if BlockInfo used for other purposes.
Could we just use < and populate the blocks vector in AppendMuxedCluster() in
reverse order as we top/pop from the pri queue?

[acolwell GONE FROM CHROMIUM, 2014/07/08 22:00:38]

Done.

+      // so timestamps with lower values have higher priority.
+      return timestamp_in_ms > rhs.timestamp_in_ms;
+    }
+  };
+
+  // |track_number| - The track number to place in
+  // |blocks_description| - A space delimited string of block info that
+  //  is used to populate |blocks|. Each block info has a timestamp in
+  //  milliseconds and optionally followed by a 'K' to indicate that a block
+  //  should be marked as a keyframe. For example "0K 30 60" should populate
+  //  |blocks| with 3 BlockInfo objects: a keyframe with timestamp 0 |and 2

[wolenetz, 2014/07/08 19:33:53]

nit: s/|and/and

[acolwell GONE FROM CHROMIUM, 2014/07/08 22:00:38]

Done.

+  //  non-keyframes at 30ms and 60ms.
+  void ParseBlockDescription(int track_number,
+                             const std::string blocks_description,
+                             std::vector<BlockInfo>* blocks) {
     std::vector<std::string> timestamps;
-    base::SplitString(cluster_description, ' ', &timestamps);
+    base::SplitString(blocks_description, ' ', &timestamps);
 
-    ClusterBuilder cb;
-    std::vector<uint8> data(10);
     for (size_t i = 0; i < timestamps.size(); ++i) {
       std::string timestamp_str = timestamps[i];
-      int block_flags = 0;
+      BlockInfo block_info;
+
+      block_info.track_number = track_number;
+      block_info.flags = 0;
+      block_info.duration = 0;
+
       if (EndsWith(timestamp_str, "K", true)) {
-        block_flags = kWebMFlagKeyframe;
+        block_info.flags = kWebMFlagKeyframe;
         // Remove the "K" off of the token.
         timestamp_str = timestamp_str.substr(0, timestamps[i].length() - 1);
       }
-      int timestamp_in_ms;
-      CHECK(base::StringToInt(timestamp_str, &timestamp_in_ms));
-
-      if (i == 0)
-        cb.SetClusterTimecode(timestamp_in_ms);
+      CHECK(base::StringToInt(timestamp_str, &block_info.timestamp_in_ms));
 
       if (track_number == kTextTrackNum ||
           track_number == kAlternateTextTrackNum) {
-        cb.AddBlockGroup(track_number, timestamp_in_ms, kTextBlockDuration,
-                         block_flags, &data[0], data.size());
+        block_info.duration = kTextBlockDuration;
+        ASSERT_EQ(block_info.flags, kWebMFlagKeyframe)

[wolenetz, 2014/07/08 19:33:53]

nit: ASSERT_EQ(expected, actual) per
https://code.google.com/p/chromium/codesearch#chromium/src/testing/gtest/incl...
Though this is not really adhered to much in this file.

[acolwell GONE FROM CHROMIUM, 2014/07/08 22:00:37]

Done.
/me shakes fist at junit.

+            << "Text block with timestamp " << block_info.timestamp_in_ms
+            << " was not marked as a keyframe."
+            << " All text blocks must be keyframes";
+      }
+
+      blocks->push_back(block_info);
+    }
+  }
+
+  scoped_ptr<Cluster> GenerateCluster(const std::vector<BlockInfo>& blocks) {
+    ClusterBuilder cb;

[wolenetz, 2014/07/08 19:33:53]

nit: Add ASSERT_LT(0, blocks.size()); since we won't have a cluster timecode if
blocks is empty; caller could instead explicitly use GenerateEmptyCluster()?

[acolwell GONE FROM CHROMIUM, 2014/07/08 22:00:38]

Done.

+
+    std::vector<uint8> data(10);
+    for (size_t i = 0; i < blocks.size(); ++i) {
+      if (i == 0)
+        cb.SetClusterTimecode(blocks[i].timestamp_in_ms);
+
+      if (blocks[i].duration) {
+        cb.AddBlockGroup(blocks[i].track_number, blocks[i].timestamp_in_ms,
+                         blocks[i].duration, blocks[i].flags,
+                         &data[0], data.size());
       } else {
-        cb.AddSimpleBlock(track_number, timestamp_in_ms, block_flags,
+        cb.AddSimpleBlock(blocks[i].track_number, blocks[i].timestamp_in_ms,
+                          blocks[i].flags,
                           &data[0], data.size());
       }
     }
-    AppendCluster(source_id, cb.Finish());
+
+    return cb.Finish();
+  }
+
+  // |block_description| - The block description used to construct the cluster.
+  // See the documentation for ParseBlockDescription() for details on the string
+  // format.
+  void AppendSingleStreamCluster(const std::string& source_id, int track_number,
+                                 const std::string& block_description) {
+    std::vector<BlockInfo> blocks;
+    ParseBlockDescription(track_number, block_description, &blocks);
+    AppendCluster(source_id, GenerateCluster(blocks));
   }
 
   struct MuxedStreamInfo {
     MuxedStreamInfo()
         : track_number(0),
-          cluster_description("")
+          block_description("")
     {}
 
-    MuxedStreamInfo(int track_num, const char* cluster_desc)
+    MuxedStreamInfo(int track_num, const char* block_desc)
         : track_number(track_num),
-          cluster_description(cluster_desc) {
+          block_description(block_desc) {
     }
 
     int track_number;
-    // The cluster description passed to AppendSingleStreamCluster().
+    // The block description passed to ParseBlockDescription().
     // See the documentation for that method for details on the string format.
-    const char* cluster_description;
+    const char* block_description;

[wolenetz, 2014/07/08 19:33:53]

nit: s/block_description/block_descriptions or blocks_description  or
block_description. Name similarly the ParseBlockDescription parameter to
whichever you choose here.

[acolwell GONE FROM CHROMIUM, 2014/07/08 22:00:38]

Done.

   };
 
   void AppendMuxedCluster(const MuxedStreamInfo& msi_1,
@@ -472,11 +519,23 @@ class ChunkDemuxerTest : public ::testing::Test {
   }
 
   void AppendMuxedCluster(const std::vector<MuxedStreamInfo> msi) {
+    std::priority_queue<BlockInfo> block_queue;
     for (size_t i = 0; i < msi.size(); ++i) {
-      AppendSingleStreamCluster(kSourceId,
-                                msi[i].track_number,
-                                msi[i].cluster_description);
+      std::vector<BlockInfo> track_blocks;
+      ParseBlockDescription(msi[i].track_number, msi[i].block_description,
+                            &track_blocks);
+
+      for (size_t j = 0; j < track_blocks.size(); ++j)
+        block_queue.push(track_blocks[j]);
+    }
+
+    std::vector<BlockInfo> blocks(block_queue.size());
+    for (size_t i = 0; !block_queue.empty(); ++i) {
+      blocks[i] = block_queue.top();
+      block_queue.pop();
     }
+
+    AppendCluster(kSourceId, GenerateCluster(blocks));
   }
 
   void AppendData(const std::string& source_id,
@@ -1290,21 +1349,19 @@ TEST_F(ChunkDemuxerTest, InitSegmentSetsNeedRandomAccessPointFlag) {
   AppendMuxedCluster(
       MuxedStreamInfo(kAudioTrackNum, "0 23K"),
       MuxedStreamInfo(kVideoTrackNum, "0 30K"),
-      MuxedStreamInfo(kTextTrackNum, "0 40K"));
-  CheckExpectedRanges(kSourceId, "{ [30,46) }");
+      MuxedStreamInfo(kTextTrackNum, "25K 40K"));

[acolwell GONE FROM CHROMIUM, 2014/07/08 16:32:00]

This was moved to avoid the text keyframe from causing the range to start at 0
which could mask what is going on in the other streams. The original code had a
text keyframe even though the string doesn't show it. I added an assert in
ParseBlockDescription() to make sure that all text blocks are actually marked as
keyframes in the string.

+  CheckExpectedRanges(kSourceId, "{ [23,46) }");

[wolenetz, 2014/07/08 19:33:53]

IIUC, this change from 30 to 23 is unrelated to the text buffers, right?
(Rather, it's related to previous code appended multiple clusters, each
triggering a new media segment notification, and this CL appends one cluster
with minimum surviving timestamp of 23.)

[acolwell GONE FROM CHROMIUM, 2014/07/08 22:00:37]

That is correct.

 
   AppendInitSegment(HAS_TEXT | HAS_AUDIO | HAS_VIDEO);
   AppendMuxedCluster(
       MuxedStreamInfo(kAudioTrackNum, "46 69K"),
       MuxedStreamInfo(kVideoTrackNum, "60 90K"),
-      MuxedStreamInfo(kTextTrackNum, "80 90K"));
-  CheckExpectedRanges(kSourceId, "{ [30,92) }");
+      MuxedStreamInfo(kTextTrackNum, "80K 90K"));
+  CheckExpectedRanges(kSourceId, "{ [23,92) }");
 
   CheckExpectedBuffers(audio_stream, "23 69");
   CheckExpectedBuffers(video_stream, "30 90");
-
-  // WebM parser marks all text buffers as keyframes.
-  CheckExpectedBuffers(text_stream, "0 40 80 90");
+  CheckExpectedBuffers(text_stream, "25 40 80 90");
 }
 
 // Make sure that the demuxer reports an error if Shutdown()
@@ -2468,7 +2525,6 @@ TEST_F(ChunkDemuxerTest, GetBufferedRanges_EndOfStream) {
 
   // Append and remove data so that the 2 streams' end ranges do not overlap.
 
-  EXPECT_CALL(host_, SetDuration(base::TimeDelta::FromMilliseconds(246)));

[acolwell GONE FROM CHROMIUM, 2014/07/08 16:32:00]

This extra call was being triggered because there were 2 appends. Now there is
only one append so only 1 duration change occurs.

   EXPECT_CALL(host_, SetDuration(base::TimeDelta::FromMilliseconds(398)));
   AppendMuxedCluster(
       MuxedStreamInfo(kAudioTrackNum, "200K 223K"),
@@ -3353,7 +3409,7 @@ TEST_F(ChunkDemuxerTest, AppendWindow_Text) {
   // Verify that text cues that start outside the window are not included
   // in the buffer. Also verify that cues that extend beyond the
   // window are not included.
-  CheckExpectedRanges(kSourceId, "{ [120,270) }");
+  CheckExpectedRanges(kSourceId, "{ [100,270) }");

[acolwell GONE FROM CHROMIUM, 2014/07/08 16:32:00]

The text buffer is the first frame within the append window so it becomes the
media segment start time.

   CheckExpectedBuffers(video_stream, "120 150 180 210 240");
   CheckExpectedBuffers(text_stream, "100");
 
@@ -3365,7 +3421,7 @@ TEST_F(ChunkDemuxerTest, AppendWindow_Text) {
       MuxedStreamInfo(kVideoTrackNum,
                       "360 390 420K 450 480 510 540K 570 600 630K"),
       MuxedStreamInfo(kTextTrackNum, "400K 500K 600K 700K" ));
-  CheckExpectedRanges(kSourceId, "{ [120,270) [420,630) }");
+  CheckExpectedRanges(kSourceId, "{ [100,270) [400,630) }");
 
   // Seek to the new range and verify that the expected buffers are returned.
   Seek(base::TimeDelta::FromMilliseconds(420));