MSE: Use frame duration, if available, in LegacyFrameProcessor

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 475ac39d0a3c2a5748c005324d20b3ce89810c8f..7bc7385bc3828303a7e6cce7dacd5c060dd27aed 100644
--- a/media/filters/chunk_demuxer_unittest.cc
+++ b/media/filters/chunk_demuxer_unittest.cc
@@ -381,12 +381,23 @@ class ChunkDemuxerTest : public testing::Test {
       if (i == 0)
         cb.SetClusterTimecode(timestamp_in_ms);
 
-      if (track_number == kTextTrackNum) {
-        cb.AddBlockGroup(track_number, timestamp_in_ms, kTextBlockDuration,
-                         block_flags, &data[0], data.size());
-      } else {
-        cb.AddSimpleBlock(track_number, timestamp_in_ms, block_flags,
-                          &data[0], data.size());
+      switch (track_number) {
+        case kVideoTrackNum:
+          AddVideoBlockGroup(&cb, kVideoTrackNum, timestamp_in_ms,
+                             kVideoBlockDuration, block_flags);
+          break;
+        case kAudioTrackNum:
+          cb.AddBlockGroup(kAudioTrackNum, timestamp_in_ms,
+                           kAudioBlockDuration, block_flags, &data[0],
+                           data.size());
+          break;
+        case kTextTrackNum:
+          cb.AddBlockGroup(kTextTrackNum, timestamp_in_ms,
+                           kTextBlockDuration, block_flags, &data[0],
+                           data.size());
+          break;
+        default:
+          DCHECK(false) << "Unknown track number: " << track_number;
       }
     }
     AppendCluster(source_id, cb.Finish());
@@ -556,8 +567,14 @@ class ChunkDemuxerTest : public testing::Test {
     scoped_refptr<DecoderBuffer> bear2 = ReadTestDataFile("bear-640x360.webm");
 
     EXPECT_CALL(*this, DemuxerOpened());
+
+    // TODO(wolenetz/acolwell): Remove this SetDuration expectation once the
+    // files are fixed to have the correct duration in their init segments.
+    // See http://crbug.com/354284.
+    EXPECT_CALL(host_, SetDuration(base::TimeDelta::FromMilliseconds(2744)));

[acolwell GONE FROM CHROMIUM, 2014/03/21 15:50:15]

What triggers this? Does this need to be here or can it be moved closer to the
code that triggers it?

[wolenetz, 2014/03/21 18:14:01]

The file has 2744 encoded in its init segment. Parsing the init segment causes
SetDuration(2744) (which needs to be expected). However, the full file append,
along with the usage of parser's frame duration or estimate, causes the duration
to be not just the highest presentation timestamp, but increased to the
HPET+frame duration (==2768 with this change's usage of
SBS::GetBufferedDuration() to estimate the full duration). Bug 354284 tracks
getting the file fixed. Note, fixing that bug and the expectations here will
likely be impacted by change(s) we're working on to get WebM stream parser to
emit at least an estimated frame duration (part of fixing bug 351166), so that
the coded frame processor can use it to directly calculate the duration increase
instead of SBS::GetBufferedDuration().

Done, here and in similar places elsewhere in this file that reference
http://crbug.com/354284.

+
     demuxer_->Initialize(
-        &host_, CreateInitDoneCB(base::TimeDelta::FromMilliseconds(2744),
+        &host_, CreateInitDoneCB(base::TimeDelta::FromMilliseconds(2768),
                                  PIPELINE_OK), true);
 
     if (AddId(kSourceId, HAS_AUDIO | HAS_VIDEO) != ChunkDemuxer::kOk)
@@ -635,35 +652,23 @@ class ChunkDemuxerTest : public testing::Test {
     int audio_timecode = first_audio_timecode;
     int video_timecode = first_video_timecode;
 
-    // Create simple blocks for everything except the last 2 blocks.
-    // The first video frame must be a keyframe.
+    // Create block groups for everything to ensure parser can emit durations
+    // for every buffer. The first video frame must be a keyframe.
+    // TODO(acolwell): Also use simple blocks once WebM stream parser can emit
+    // valid frame durations for them. See http://crbug.com/351166.
     uint8 video_flag = kWebMFlagKeyframe;
-    for (int i = 0; i < block_count - 2; i++) {
+    for (int i = 0; i < block_count; i++) {
       if (audio_timecode <= video_timecode) {
-        cb.AddSimpleBlock(kAudioTrackNum, audio_timecode, kWebMFlagKeyframe,
-                          data.get(), size);
+        cb.AddBlockGroup(kAudioTrackNum, audio_timecode, kAudioBlockDuration,
+                         kWebMFlagKeyframe, data.get(), size);
         audio_timecode += kAudioBlockDuration;
         continue;
       }
 
-      cb.AddSimpleBlock(kVideoTrackNum, video_timecode, video_flag, data.get(),
-                        size);
-      video_timecode += kVideoBlockDuration;
-      video_flag = 0;
-    }
-
-    // Make the last 2 blocks BlockGroups so that they don't get delayed by the
-    // block duration calculation logic.
-    if (audio_timecode <= video_timecode) {
-      cb.AddBlockGroup(kAudioTrackNum, audio_timecode, kAudioBlockDuration,
-                       kWebMFlagKeyframe, data.get(), size);
       AddVideoBlockGroup(&cb, kVideoTrackNum, video_timecode,
                          kVideoBlockDuration, video_flag);
-    } else {
-      AddVideoBlockGroup(&cb, kVideoTrackNum, video_timecode,
-                         kVideoBlockDuration, video_flag);
-      cb.AddBlockGroup(kAudioTrackNum, audio_timecode, kAudioBlockDuration,
-                       kWebMFlagKeyframe, data.get(), size);
+      video_timecode += kVideoBlockDuration;
+      video_flag = 0;
     }
 
     return cb.Finish();
@@ -680,22 +685,24 @@ class ChunkDemuxerTest : public testing::Test {
     ClusterBuilder cb;
     cb.SetClusterTimecode(timecode);
 
+    // Create block groups for everything to ensure parser can emit durations
+    // for every buffer. Sets every frame to be a key frame.
+    // TODO(acolwell): Also use simple blocks once WebM stream parser can emit
+    // valid frame durations for them. See http://crbug.com/351166.
+
     // Create simple blocks for everything except the last block.
-    for (int i = 0; timecode < (end_timecode - block_duration); i++) {
-      cb.AddSimpleBlock(track_number, timecode, kWebMFlagKeyframe,
-                        &data[0], data.size());
+    for (int i = 0; timecode < end_timecode; i++) {
+      if (track_number == kVideoTrackNum) {
+        AddVideoBlockGroup(&cb, track_number, timecode, block_duration,
+                           kWebMFlagKeyframe);
+      } else {
+        cb.AddBlockGroup(track_number, timecode, block_duration,
+                         kWebMFlagKeyframe, &data[0], data.size());
+      }
+
       timecode += block_duration;
     }
 
-    // Make the last block a BlockGroup so that it doesn't get delayed by the
-    // block duration calculation logic.
-    if (track_number == kVideoTrackNum) {

[acolwell GONE FROM CHROMIUM, 2014/03/21 15:50:15]

heh.. this is a left over from my previous attempt at making the parser generate
durations.

-      AddVideoBlockGroup(&cb, track_number, timecode, block_duration,
-                         kWebMFlagKeyframe);
-    } else {
-      cb.AddBlockGroup(track_number, timecode, block_duration,
-                       kWebMFlagKeyframe, &data[0], data.size());
-    }
     return cb.Finish();
   }
 
@@ -1576,19 +1583,19 @@ TEST_F(ChunkDemuxerTest, EndOfStreamRangeChanges) {
       .WillOnce(SaveArg<0>(&text_stream));
   ASSERT_TRUE(InitDemuxer(HAS_AUDIO | HAS_VIDEO | HAS_TEXT));
 
-  AppendSingleStreamCluster(kSourceId, kVideoTrackNum, "0K 30");
+  AppendSingleStreamCluster(kSourceId, kVideoTrackNum, "0K 33");
   AppendSingleStreamCluster(kSourceId, kAudioTrackNum, "0K 23K");
 
   // Check expected ranges and verify that an empty text track does not
   // affect the expected ranges.
   CheckExpectedRanges(kSourceId, "{ [0,46) }");
 
-  EXPECT_CALL(host_, SetDuration(base::TimeDelta::FromMilliseconds(60)));
+  EXPECT_CALL(host_, SetDuration(base::TimeDelta::FromMilliseconds(66)));
   MarkEndOfStream(PIPELINE_OK);
 
   // Check expected ranges and verify that an empty text track does not
   // affect the expected ranges.
-  CheckExpectedRanges(kSourceId, "{ [0,60) }");
+  CheckExpectedRanges(kSourceId, "{ [0,66) }");
 
   // Unmark end of stream state and verify that the ranges return to
   // their pre-"end of stream" values.
@@ -1650,8 +1657,13 @@ TEST_F(ChunkDemuxerTest, WebMFile_AudioAndVideo) {
     {kSkip, kSkip},
   };
 
+  // TODO(wolenetz/acolwell): Remove this SetDuration expectation once the
+  // file is fixed to have the correct duration in the init segment.
+  // See http://crbug.com/354284.
+  EXPECT_CALL(host_, SetDuration(base::TimeDelta::FromMilliseconds(2744)));
+
   ASSERT_TRUE(ParseWebMFile("bear-320x240.webm", buffer_timestamps,
-                            base::TimeDelta::FromMilliseconds(2744)));
+                            base::TimeDelta::FromMilliseconds(2768)));
 }
 
 TEST_F(ChunkDemuxerTest, WebMFile_LiveAudioAndVideo) {
@@ -1678,8 +1690,13 @@ TEST_F(ChunkDemuxerTest, WebMFile_AudioOnly) {
     {kSkip, kSkip},
   };
 
+  // TODO(wolenetz/acolwell): Remove this SetDuration expectation once the
+  // file is fixed to have the correct duration in the init segment.
+  // See http://crbug.com/354284.
+  EXPECT_CALL(host_, SetDuration(base::TimeDelta::FromMilliseconds(2744)));
+
   ASSERT_TRUE(ParseWebMFile("bear-320x240-audio-only.webm", buffer_timestamps,
-                            base::TimeDelta::FromMilliseconds(2744),
+                            base::TimeDelta::FromMilliseconds(2768),
                             HAS_AUDIO));
 }
 
@@ -1693,8 +1710,13 @@ TEST_F(ChunkDemuxerTest, WebMFile_VideoOnly) {
     {kSkip, kSkip},
   };
 
+  // TODO(wolenetz/acolwell): Remove this SetDuration expectation once the
+  // file is fixed to have the correct duration in the init segment.
+  // See http://crbug.com/354284.
+  EXPECT_CALL(host_, SetDuration(base::TimeDelta::FromMilliseconds(2703)));
+
   ASSERT_TRUE(ParseWebMFile("bear-320x240-video-only.webm", buffer_timestamps,
-                            base::TimeDelta::FromMilliseconds(2703),
+                            base::TimeDelta::FromMilliseconds(2737),
                             HAS_VIDEO));
 }
 
@@ -1708,8 +1730,13 @@ TEST_F(ChunkDemuxerTest, WebMFile_AltRefFrames) {
     {kSkip, kSkip},
   };
 
+  // TODO(wolenetz/acolwell): Remove this SetDuration expectation once the
+  // file is fixed to have the correct duration in the init segment.
+  // See http://crbug.com/354284.
+  EXPECT_CALL(host_, SetDuration(base::TimeDelta::FromMilliseconds(2767)));
+
   ASSERT_TRUE(ParseWebMFile("bear-320x240-altref.webm", buffer_timestamps,
-                            base::TimeDelta::FromMilliseconds(2767)));
+                            base::TimeDelta::FromMilliseconds(2768)));
 }
 
 // Verify that we output buffers before the entire cluster has been parsed.
@@ -2723,22 +2750,34 @@ TEST_F(ChunkDemuxerTest, DurationChange) {
 
   CheckExpectedRanges(kSourceId, "{ [201191,201224) }");
 
-  // Add data at the currently set duration. The duration should not increase.
+  // Add data beginning at the currently set duration and expect a new duration
+  // to be signaled. Note that the last video block will have a higher end
+  // timestamp than the last audio block.
+  // TODO(wolenetz): Compliant coded frame processor will emit a max of one
+  // duration change per each ProcessFrames(). Remove the first expectation here
+  // once compliant coded frame processor is used. See http://crbug.com/249422.
+  const int kNewStreamDurationAudio = kStreamDuration + kAudioBlockDuration;
+  EXPECT_CALL(host_, SetDuration(
+      base::TimeDelta::FromMilliseconds(kNewStreamDurationAudio)));
+  const int kNewStreamDurationVideo = kStreamDuration + kVideoBlockDuration;
+  EXPECT_CALL(host_, SetDuration(
+      base::TimeDelta::FromMilliseconds(kNewStreamDurationVideo)));
   AppendCluster(GenerateCluster(kDefaultDuration().InMilliseconds(), 2));
 
-  // Range should not be affected.
-  CheckExpectedRanges(kSourceId, "{ [201191,201224) }");
+  CheckExpectedRanges(kSourceId, "{ [201191,201247) }");
 
-  // Now add data past the duration and expect a new duration to be signalled.
-  const int kNewStreamDuration = kStreamDuration + kAudioBlockDuration * 2;
+  // Add more data to the end of each media type. Note that the last audio block
+  // will have a higher end timestamp than the last video block.
+  const int kFinalStreamDuration = kStreamDuration + kAudioBlockDuration * 3;
   EXPECT_CALL(host_, SetDuration(
-      base::TimeDelta::FromMilliseconds(kNewStreamDuration)));
+      base::TimeDelta::FromMilliseconds(kFinalStreamDuration)));
   AppendCluster(GenerateCluster(kStreamDuration + kAudioBlockDuration,
                                 kStreamDuration + kVideoBlockDuration,
-                                2));
+                                3));
 
-  // See that the range has increased appropriately.
-  CheckExpectedRanges(kSourceId, "{ [201191,201270) }");
+  // See that the range has increased appropriately (but not to the full
+  // duration of 201293, since there is not enough video appended for that).
+  CheckExpectedRanges(kSourceId, "{ [201191,201290) }");
 }
 
 TEST_F(ChunkDemuxerTest, DurationChangeTimestampOffset) {
@@ -2746,9 +2785,15 @@ TEST_F(ChunkDemuxerTest, DurationChangeTimestampOffset) {
 
   ASSERT_TRUE(SetTimestampOffset(kSourceId, kDefaultDuration()));
 
+  // TODO(wolenetz): Compliant coded frame processor will emit a max of one
+  // duration change per each ProcessFrames(). Remove the first expectation here
+  // once compliant coded frame processor is used. See http://crbug.com/249422.
   EXPECT_CALL(host_, SetDuration(
       kDefaultDuration() + base::TimeDelta::FromMilliseconds(
           kAudioBlockDuration * 2)));
+  EXPECT_CALL(host_, SetDuration(
+      kDefaultDuration() + base::TimeDelta::FromMilliseconds(
+          kVideoBlockDuration * 2)));
   AppendCluster(GenerateCluster(0, 4));
 }
 
@@ -2813,7 +2858,7 @@ TEST_F(ChunkDemuxerTest, ReadAfterAudioDisabled) {
   EXPECT_TRUE(audio_read_done);
 }
 
-// Verifies that signalling end of stream while stalled at a gap
+// Verifies that signaling end of stream while stalled at a gap
 // boundary does not trigger end of stream buffers to be returned.
 TEST_F(ChunkDemuxerTest, EndOfStreamWhileWaitingForGapToBeFilled) {
   ASSERT_TRUE(InitDemuxer(HAS_AUDIO | HAS_VIDEO));
@@ -2822,7 +2867,6 @@ TEST_F(ChunkDemuxerTest, EndOfStreamWhileWaitingForGapToBeFilled) {
   AppendCluster(300, 10);
   CheckExpectedRanges(kSourceId, "{ [0,132) [300,432) }");
 
-
   GenerateExpectedReads(0, 10);
 
   bool audio_read_done = false;
@@ -2847,18 +2891,18 @@ TEST_F(ChunkDemuxerTest, EndOfStreamWhileWaitingForGapToBeFilled) {
 
   demuxer_->UnmarkEndOfStream();
 
-  AppendCluster(138, 24);
+  AppendCluster(138, 22);
 
   message_loop_.RunUntilIdle();
 
-  CheckExpectedRanges(kSourceId, "{ [0,438) }");
+  CheckExpectedRanges(kSourceId, "{ [0,435) }");
 
   // Verify that the reads have completed.
   EXPECT_TRUE(audio_read_done);
   EXPECT_TRUE(video_read_done);
 
   // Read the rest of the buffers.
-  GenerateExpectedReads(161, 171, 22);
+  GenerateExpectedReads(161, 171, 20);
 
   // Verify that reads block because the append cleared the end of stream state.
   audio_read_done = false;
@@ -2872,6 +2916,7 @@ TEST_F(ChunkDemuxerTest, EndOfStreamWhileWaitingForGapToBeFilled) {
   EXPECT_FALSE(audio_read_done);
   EXPECT_FALSE(video_read_done);
 
+  EXPECT_CALL(host_, SetDuration(base::TimeDelta::FromMilliseconds(437)));
   MarkEndOfStream(PIPELINE_OK);
 
   EXPECT_TRUE(audio_read_done);
@@ -2960,9 +3005,9 @@ TEST_F(ChunkDemuxerTest, AppendWindow_Video) {
 
   // Verify that GOPs that start outside the window are not included
   // in the buffer. Also verify that buffers that start inside the
-  // window and extend beyond the end of the window are included.
-  CheckExpectedRanges(kSourceId, "{ [120,300) }");
-  CheckExpectedBuffers(stream, "120 150 180 210 240 270");
+  // window and extend beyond the end of the window are not included.
+  CheckExpectedRanges(kSourceId, "{ [120,273) }");
+  CheckExpectedBuffers(stream, "120 150 180 210 240");
 
   // Extend the append window to [20,650).
   append_window_end_for_next_append_ = base::TimeDelta::FromMilliseconds(650);
@@ -2971,7 +3016,7 @@ TEST_F(ChunkDemuxerTest, AppendWindow_Video) {
   // keyframe.
   AppendSingleStreamCluster(kSourceId, kVideoTrackNum,
                             "360 390 420K 450 480 510 540K 570 600 630K");
-  CheckExpectedRanges(kSourceId, "{ [120,300) [420,660) }");
+  CheckExpectedRanges(kSourceId, "{ [120,273) [420,633) }");
 }
 
 TEST_F(ChunkDemuxerTest, AppendWindow_Audio) {
@@ -2989,9 +3034,9 @@ TEST_F(ChunkDemuxerTest, AppendWindow_Audio) {
 
   // Verify that frames that start outside the window are not included
   // in the buffer. Also verify that buffers that start inside the
-  // window and extend beyond the end of the window are included.
-  CheckExpectedRanges(kSourceId, "{ [30,300) }");
-  CheckExpectedBuffers(stream, "30 60 90 120 150 180 210 240 270");
+  // window and extend beyond the end of the window are not included.

[acolwell GONE FROM CHROMIUM, 2014/03/21 15:50:15]

Why did this change? Is this an artifact of supporting step 1.10 of the coded
frame processing algorithm?

[wolenetz, 2014/03/21 18:14:01]

Yes. I included that step's support in this change. Previously, only the frame
PTS was compared to append window end. Now, if there is a frame duration, frame
PTS+duration is compared to append window end. And the tests here now always
provide duration.

Window is [20,280]. Audio duration is 23ms. Audio frames appended at 270,300,330
are dropped. Similar occurs for video and text in AppendWindow_Text, below.

+  CheckExpectedRanges(kSourceId, "{ [30,263) }");
+  CheckExpectedBuffers(stream, "30 60 90 120 150 180 210 240");
 
   // Extend the append window to [20,650).
   append_window_end_for_next_append_ = base::TimeDelta::FromMilliseconds(650);
@@ -3000,7 +3045,7 @@ TEST_F(ChunkDemuxerTest, AppendWindow_Audio) {
   AppendSingleStreamCluster(
       kSourceId, kAudioTrackNum,
       "360K 390K 420K 450K 480K 510K 540K 570K 600K 630K");
-  CheckExpectedRanges(kSourceId, "{ [30,300) [360,660) }");
+  CheckExpectedRanges(kSourceId, "{ [30,263) [360,623) }");
 }
 
 TEST_F(ChunkDemuxerTest, AppendWindow_Text) {
@@ -3022,10 +3067,10 @@ 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 included.
-  CheckExpectedRanges(kSourceId, "{ [120,300) }");
-  CheckExpectedBuffers(video_stream, "120 150 180 210 240 270");
-  CheckExpectedBuffers(text_stream, "100 200");
+  // window are not included.
+  CheckExpectedRanges(kSourceId, "{ [120,273) }");
+  CheckExpectedBuffers(video_stream, "120 150 180 210 240");
+  CheckExpectedBuffers(text_stream, "100");
 
   // Extend the append window to [20,650).
   append_window_end_for_next_append_ = base::TimeDelta::FromMilliseconds(650);
@@ -3034,12 +3079,12 @@ TEST_F(ChunkDemuxerTest, AppendWindow_Text) {
   AppendSingleStreamCluster(kSourceId, kVideoTrackNum,
                             "360 390 420K 450 480 510 540K 570 600 630K");
   AppendSingleStreamCluster(kSourceId, kTextTrackNum, "400K 500K 600K 700K");
-  CheckExpectedRanges(kSourceId, "{ [120,300) [420,660) }");
+  CheckExpectedRanges(kSourceId, "{ [120,273) [420,633) }");
 
   // Seek to the new range and verify that the expected buffers are returned.
   Seek(base::TimeDelta::FromMilliseconds(420));
-  CheckExpectedBuffers(video_stream, "420 450 480 510 540 570 600 630");
-  CheckExpectedBuffers(text_stream, "400 500 600");
+  CheckExpectedBuffers(video_stream, "420 450 480 510 540 570 600");
+  CheckExpectedBuffers(text_stream, "400 500");
 }
 
 TEST_F(ChunkDemuxerTest, StartWaitingForSeekAfterParseError) {
@@ -3120,7 +3165,7 @@ TEST_F(ChunkDemuxerTest, SeekCompletesWithoutTextCues) {
   AppendSingleStreamCluster(kSourceId, kAudioTrackNum,
                             "0K 20K 40K 60K 80K 100K 120K 140K 160K 180K");
   AppendSingleStreamCluster(kSourceId, kVideoTrackNum,
-                            "0K 30 60 90 120K 150 180 210");
+                            "0K 30 60 90K 120K 150 180 210");
 
   message_loop_.RunUntilIdle();
   EXPECT_TRUE(seek_cb_was_called);
@@ -3143,7 +3188,7 @@ TEST_F(ChunkDemuxerTest, SeekCompletesWithoutTextCues) {
   // to the pending read initiated above.
   CheckExpectedBuffers(text_stream, "175 225");
 
-  // Verify that audio & video streams contiue to return expected values.
+  // Verify that audio & video streams continue to return expected values.
   CheckExpectedBuffers(audio_stream, "160 180");
   CheckExpectedBuffers(video_stream, "180 210");
 }