Add AppendMuxedCluster() method to make test intentions clearer.

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 2326de2de6600ef7c2c204cccb38b2f1a1d5aae7..62f66fd8528ffbee65a9f0e4be3f83ad90ab2812 100644
--- a/media/filters/chunk_demuxer_unittest.cc
+++ b/media/filters/chunk_demuxer_unittest.cc
@@ -436,6 +436,37 @@ class ChunkDemuxerTest : public ::testing::Test {
     AppendCluster(source_id, cb.Finish());
   }
 
+  struct MuxedStreamInfo {
+    int track_number;
+    const char* description;

[wolenetz, 2014/07/07 21:31:45]

nit: s/description/cluster_description/. Also maybe relocate or duplicate
|cluster_description| parameter comment from AppendSingleStreamCluster()?

[acolwell GONE FROM CHROMIUM, 2014/07/07 23:56:36]

Done.
For now I just added a comment that references AppendSingleStreamCluster(). The
follow-up CL factors out the parsing logic so I'll update the to point to the
parsing function in the followup.

+  };
+
+  void AppendMuxedCluster(const MuxedStreamInfo& msi_1,
+                          const MuxedStreamInfo& msi_2) {
+    std::vector<MuxedStreamInfo> msi(2);
+    msi[0] = msi_1;
+    msi[1] = msi_2;
+    AppendMuxedCluster(msi);
+  }
+
+  void AppendMuxedCluster(const MuxedStreamInfo& msi_1,
+                          const MuxedStreamInfo& msi_2,
+                          const MuxedStreamInfo& msi_3) {
+    std::vector<MuxedStreamInfo> msi(3);
+    msi[0] = msi_1;
+    msi[1] = msi_2;
+    msi[2] = msi_3;
+    AppendMuxedCluster(msi);
+  }
+
+  void AppendMuxedCluster(const std::vector<MuxedStreamInfo> msi) {
+    for (size_t i = 0; i < msi.size(); ++i) {
+      AppendSingleStreamCluster(kSourceId,

[wolenetz, 2014/07/07 21:31:44]

nit: Include a TODO for the follow-up CL to actually append just 1 muxed cluster
here? At your discretion based on your expectation of sending follow-up CL.

[acolwell GONE FROM CHROMIUM, 2014/07/07 23:56:36]

I have the follow-up CL written already. I was just waiting to send it once this
lands.

[wolenetz, 2014/07/08 00:47:33]

sgtm

+                                msi[i].track_number,
+                                msi[i].description);
+    }
+  }
+
   void AppendData(const std::string& source_id,
                   const uint8* data, size_t length) {
     EXPECT_CALL(host_, AddBufferedTimeRange(_, _)).Times(AnyNumber());
@@ -1198,9 +1229,10 @@ TEST_F(ChunkDemuxerTest, SingleTextTrackIdChange) {
   ASSERT_TRUE(video_stream);
   ASSERT_TRUE(text_stream);
 
-  AppendSingleStreamCluster(kSourceId, kAudioTrackNum, "0K 23K");
-  AppendSingleStreamCluster(kSourceId, kVideoTrackNum, "0K 30");
-  AppendSingleStreamCluster(kSourceId, kTextTrackNum, "10K");
+  AppendMuxedCluster(
+      { kAudioTrackNum, "0K 23K" },
+      { kVideoTrackNum, "0K 30" },
+      { kTextTrackNum, "10K" });
   CheckExpectedRanges(kSourceId, "{ [0,46) }");
 
   scoped_ptr<uint8[]> info_tracks;
@@ -1213,9 +1245,10 @@ TEST_F(ChunkDemuxerTest, SingleTextTrackIdChange) {
                        append_window_end_for_next_append_,
                        &timestamp_offset_map_[kSourceId]);
 
-  AppendSingleStreamCluster(kSourceId, kAudioTrackNum, "46K 69K");
-  AppendSingleStreamCluster(kSourceId, kVideoTrackNum, "60K");
-  AppendSingleStreamCluster(kSourceId, kAlternateTextTrackNum, "45K");
+  AppendMuxedCluster(
+      { kAudioTrackNum, "46K 69K" },
+      { kVideoTrackNum, "60K" },
+      { kAlternateTextTrackNum, "45K" });
 
   CheckExpectedRanges(kSourceId, "{ [0,92) }");
   CheckExpectedBuffers(audio_stream, "0 23 46 69");
@@ -1242,15 +1275,17 @@ TEST_F(ChunkDemuxerTest, InitSegmentSetsNeedRandomAccessPointFlag) {
   DemuxerStream* video_stream = demuxer_->GetStream(DemuxerStream::VIDEO);
   ASSERT_TRUE(audio_stream && video_stream && text_stream);
 
-  AppendSingleStreamCluster(kSourceId, kAudioTrackNum, "0 23K");
-  AppendSingleStreamCluster(kSourceId, kVideoTrackNum, "0 30K");
-  AppendSingleStreamCluster(kSourceId, kTextTrackNum, "0 40K");
+  AppendMuxedCluster(
+      { kAudioTrackNum, "0 23K" },
+      { kVideoTrackNum, "0 30K" },
+      { kTextTrackNum, "0 40K" });
   CheckExpectedRanges(kSourceId, "{ [30,46) }");
 
   AppendInitSegment(HAS_TEXT | HAS_AUDIO | HAS_VIDEO);
-  AppendSingleStreamCluster(kSourceId, kAudioTrackNum, "46 69K");
-  AppendSingleStreamCluster(kSourceId, kVideoTrackNum, "60 90K");
-  AppendSingleStreamCluster(kSourceId, kTextTrackNum, "80 90K");
+  AppendMuxedCluster(
+      { kAudioTrackNum, "46 69K" },
+      { kVideoTrackNum, "60 90K" },
+      { kTextTrackNum, "80 90K" });
   CheckExpectedRanges(kSourceId, "{ [30,92) }");
 
   CheckExpectedBuffers(audio_stream, "23 69");
@@ -1730,8 +1765,9 @@ TEST_F(ChunkDemuxerTest, EndOfStreamRangeChanges) {
       .WillOnce(SaveArg<0>(&text_stream));
   ASSERT_TRUE(InitDemuxer(HAS_AUDIO | HAS_VIDEO | HAS_TEXT));
 
-  AppendSingleStreamCluster(kSourceId, kVideoTrackNum, "0K 33");
-  AppendSingleStreamCluster(kSourceId, kAudioTrackNum, "0K 23K");
+  AppendMuxedCluster(
+      { kVideoTrackNum, "0K 33" },
+      { kAudioTrackNum, "0K 23K" });
 
   // Check expected ranges and verify that an empty text track does not
   // affect the expected ranges.
@@ -1752,7 +1788,10 @@ TEST_F(ChunkDemuxerTest, EndOfStreamRangeChanges) {
   // Add text track data and verify that the buffered ranges don't change
   // since the intersection of all the tracks doesn't change.
   EXPECT_CALL(host_, SetDuration(base::TimeDelta::FromMilliseconds(200)));
-  AppendSingleStreamCluster(kSourceId, kTextTrackNum, "0K 100K");
+  AppendMuxedCluster(
+      { kVideoTrackNum, "0K 33" },
+      { kAudioTrackNum, "0K 23K" },
+      { kTextTrackNum, "0K 100K" });
   CheckExpectedRanges(kSourceId, "{ [0,46) }");
 
   // Mark end of stream and verify that text track data is reflected in
@@ -2369,22 +2408,26 @@ TEST_F(ChunkDemuxerTest, GetBufferedRanges_AudioVideoText) {
   ASSERT_TRUE(InitDemuxer(HAS_AUDIO | HAS_VIDEO | HAS_TEXT));
 
   // Append audio & video data
-  AppendSingleStreamCluster(kSourceId, kAudioTrackNum, "0K 23");
-  AppendSingleStreamCluster(kSourceId, kVideoTrackNum, "0K 33");
+  AppendMuxedCluster(
+      { kAudioTrackNum, "0K 23" },
+      { kVideoTrackNum, "0K 33" });
 
   // Verify that a text track with no cues does not result in an empty buffered
   // range.
   CheckExpectedRanges("{ [0,46) }");
 
   // Add some text cues.
-  AppendSingleStreamCluster(kSourceId, kTextTrackNum, "0K 100K");
+  AppendMuxedCluster(
+      { kAudioTrackNum, "100K 123" },
+      { kVideoTrackNum, "100K 133" },
+      { kTextTrackNum, "100K 200K"});

[wolenetz, 2014/07/07 21:31:44]

nit: s/"}/" }/

[acolwell GONE FROM CHROMIUM, 2014/07/07 23:56:36]

Done.

 
-  // Verify that the new cues did not affect the buffered ranges.
-  CheckExpectedRanges("{ [0,46) }");
+  // Verify that the text cues are not reflected in the buffered ranges.
+  CheckExpectedRanges("{ [0,46) [100,146) }");
 
-  // Remove the buffered range.
+  // Remove the buffered ranges.

[wolenetz, 2014/07/07 21:31:44]

nit: Fix related to my comment below, since the text track may still contain
non-empty track buffer ranges.

[acolwell GONE FROM CHROMIUM, 2014/07/07 23:56:36]

I changed the remove range so that everything gets removed.

   demuxer_->Remove(kSourceId, base::TimeDelta(),
-                   base::TimeDelta::FromMilliseconds(46));
+                   base::TimeDelta::FromMilliseconds(146));
   CheckExpectedRanges("{ }");

[wolenetz, 2014/07/07 21:31:44]

nit (maybe best left to a separate CL): If MarkEndOfStream(PIPELINE_OK) were
done after this line, would a subsequent CheckExpectedRanges() need to expect
just "{ }", even though there is still a not-removed text track buffer [200,
300) and (possibly if supported) a partially remaining text track buffer [146,
200)? (SourceBuffer.buffered() step 4.2 cannot change end time on last range in
a completely empty track's track ranges, so resulting intersection ranges even
in "ended" readyState for this scenario should be empty, right?
EndOfStreamRangeChanges and GetBufferedRanges_EndOfStream don't test this
specific scenario where only text track buffer(s) remain after remove() +
endOfStream(). Further, re-appending some audio+video like just 0K for each +
endOfStream() should then expose the still-not-removed text buffer(s) in
CheckExpectedRanges(). ISTM like a good scenario to test. Spec reference:
https://dvcs.w3.org/hg/html-media/raw-file/default/media-source/media-source....

[acolwell GONE FROM CHROMIUM, 2014/07/07 23:56:36]

This seems like a completely separate issue that should be addressed in a
different CL. Please file a bug to track the scenario you are worried about.
Since text tracks aren't on by default this should probably be a P3.

[wolenetz, 2014/07/08 00:47:33]

sgtm. I've filed https://crbug.com/392006

 }
 
@@ -2394,8 +2437,9 @@ TEST_F(ChunkDemuxerTest, GetBufferedRanges_AudioVideoText) {
 TEST_F(ChunkDemuxerTest, GetBufferedRanges_EndOfStream) {
   ASSERT_TRUE(InitDemuxer(HAS_AUDIO | HAS_VIDEO));
 
-  AppendSingleStreamCluster(kSourceId, kAudioTrackNum, "0K 23K");
-  AppendSingleStreamCluster(kSourceId, kVideoTrackNum, "0K 33");
+  AppendMuxedCluster(
+      { kAudioTrackNum, "0K 23K" },
+      { kVideoTrackNum, "0K 33"});

[wolenetz, 2014/07/07 21:31:44]

nit: s/"}/" }/

[acolwell GONE FROM CHROMIUM, 2014/07/07 23:56:36]

Done.

 
   CheckExpectedRanges("{ [0,46) }");
 
@@ -2414,9 +2458,9 @@ TEST_F(ChunkDemuxerTest, GetBufferedRanges_EndOfStream) {
 
   EXPECT_CALL(host_, SetDuration(base::TimeDelta::FromMilliseconds(246)));
   EXPECT_CALL(host_, SetDuration(base::TimeDelta::FromMilliseconds(398)));
-  AppendSingleStreamCluster(kSourceId, kAudioTrackNum, "200K 223K");
-  AppendSingleStreamCluster(kSourceId, kVideoTrackNum,
-                            "200K 233 266 299 332K 365");
+  AppendMuxedCluster(
+      { kAudioTrackNum, "200K 223K" },
+      { kVideoTrackNum, "200K 233 266 299 332K 365" });
 
   // At this point, the per-stream ranges are as follows:
   // Audio: [0,46) [200,246)
@@ -2431,8 +2475,9 @@ TEST_F(ChunkDemuxerTest, GetBufferedRanges_EndOfStream) {
   // Video: [0,66) [332,398)
   CheckExpectedRanges("{ [0,46) }");
 
-  AppendSingleStreamCluster(kSourceId, kAudioTrackNum, "200K 223K");
-  AppendSingleStreamCluster(kSourceId, kVideoTrackNum, "200K 233");
+  AppendMuxedCluster(
+      { kAudioTrackNum, "200K 223K" },
+      { kVideoTrackNum, "200K 233" });
 
   // At this point, the per-stream ranges are as follows:
   // Audio: [0,46) [200,246)
@@ -3288,9 +3333,9 @@ TEST_F(ChunkDemuxerTest, AppendWindow_Text) {
 
   // Append a cluster that starts before and ends after the append
   // window.
-  AppendSingleStreamCluster(kSourceId, kVideoTrackNum,
-                            "0K 30 60 90 120K 150 180 210 240K 270 300 330K");
-  AppendSingleStreamCluster(kSourceId, kTextTrackNum, "0K 100K 200K 300K");
+  AppendMuxedCluster(
+      { kVideoTrackNum, "0K 30 60 90 120K 150 180 210 240K 270 300 330K" },
+      { kTextTrackNum, "0K 100K 200K 300K" });
 
   // Verify that text cues that start outside the window are not included
   // in the buffer. Also verify that cues that extend beyond the
@@ -3303,9 +3348,9 @@ TEST_F(ChunkDemuxerTest, AppendWindow_Text) {
   append_window_end_for_next_append_ = base::TimeDelta::FromMilliseconds(650);
 
   // Append more data and verify that a new range is created.
-  AppendSingleStreamCluster(kSourceId, kVideoTrackNum,
-                            "360 390 420K 450 480 510 540K 570 600 630K");
-  AppendSingleStreamCluster(kSourceId, kTextTrackNum, "400K 500K 600K 700K");
+  AppendMuxedCluster(
+      { kVideoTrackNum, "360 390 420K 450 480 510 540K 570 600 630K" },
+      { kTextTrackNum, "400K 500K 600K 700K" });
   CheckExpectedRanges(kSourceId, "{ [120,270) [420,630) }");
 
   // Seek to the new range and verify that the expected buffers are returned.
@@ -3331,11 +3376,10 @@ TEST_F(ChunkDemuxerTest, Remove_AudioVideoText) {
   DemuxerStream* audio_stream = demuxer_->GetStream(DemuxerStream::AUDIO);
   DemuxerStream* video_stream = demuxer_->GetStream(DemuxerStream::VIDEO);
 
-  AppendSingleStreamCluster(kSourceId, kAudioTrackNum,
-                            "0K 20K 40K 60K 80K 100K 120K 140K");
-  AppendSingleStreamCluster(kSourceId, kVideoTrackNum,
-                            "0K 30 60 90 120K 150 180");
-  AppendSingleStreamCluster(kSourceId, kTextTrackNum, "0K 100K 200K");
+  AppendMuxedCluster(
+      { kAudioTrackNum, "0K 20K 40K 60K 80K 100K 120K 140K" },
+      { kVideoTrackNum, "0K 30 60 90 120K 150 180" },
+      { kTextTrackNum, "0K 100K 200K" });
 
   CheckExpectedBuffers(audio_stream, "0 20 40 60 80 100 120 140");
   CheckExpectedBuffers(video_stream, "0 30 60 90 120 150 180");
@@ -3350,11 +3394,10 @@ TEST_F(ChunkDemuxerTest, Remove_AudioVideoText) {
 
   // Append new buffers that are clearly different than the original
   // ones and verify that only the new buffers are returned.
-  AppendSingleStreamCluster(kSourceId, kAudioTrackNum,
-                            "1K 21K 41K 61K 81K 101K 121K 141K");
-  AppendSingleStreamCluster(kSourceId, kVideoTrackNum,
-                            "1K 31 61 91 121K 151 181");
-  AppendSingleStreamCluster(kSourceId, kTextTrackNum, "1K 101K 201K");
+  AppendMuxedCluster(
+      { kAudioTrackNum, "1K 21K 41K 61K 81K 101K 121K 141K" },
+      { kVideoTrackNum, "1K 31 61 91 121K 151 181" },
+      { kTextTrackNum, "1K 101K 201K" });
 
   Seek(base::TimeDelta());
   CheckExpectedBuffers(audio_stream, "1 21 41 61 81 101 121 141");
@@ -3416,10 +3459,9 @@ TEST_F(ChunkDemuxerTest, SeekCompletesWithoutTextCues) {
                                &text_read_done));
 
   // Append audio & video data so the seek completes.
-  AppendSingleStreamCluster(kSourceId, kAudioTrackNum,
-                            "0K 20K 40K 60K 80K 100K 120K 140K 160K 180K");
-  AppendSingleStreamCluster(kSourceId, kVideoTrackNum,
-                            "0K 30 60 90 120K 150 180 210");
+  AppendMuxedCluster(
+      { kAudioTrackNum, "0K 20K 40K 60K 80K 100K 120K 140K 160K 180K" },
+      { kVideoTrackNum, "0K 30 60 90 120K 150 180 210" });
 
   message_loop_.RunUntilIdle();
   EXPECT_TRUE(seek_cb_was_called);
@@ -3433,7 +3475,10 @@ TEST_F(ChunkDemuxerTest, SeekCompletesWithoutTextCues) {
 
   // Append text cues that start after the seek point and verify that
   // they are returned by Read() calls.
-  AppendSingleStreamCluster(kSourceId, kTextTrackNum, "125K 175K 225K");
+  AppendMuxedCluster(
+      { kAudioTrackNum, "0K 20K 40K 60K 80K 100K 120K 140K 160K 180K" },
+      { kVideoTrackNum, "0K 30 60 90 120K 150 180 210" },
+      { kTextTrackNum, "125K 175K 225K" });
 
   message_loop_.RunUntilIdle();
   EXPECT_TRUE(text_read_done);