Fix unit test failures with estimated durations and splice frames.

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 a8fdc4b4a8ef78e82fd2da499903c13e32d68d58..fa24a99f5080c5d877163b3e2a9db9323749b82d 100644
--- a/media/filters/chunk_demuxer_unittest.cc
+++ b/media/filters/chunk_demuxer_unittest.cc
@@ -165,7 +165,7 @@ class ChunkDemuxerTest : public testing::Test {
     Demuxer::NeedKeyCB need_key_cb =
         base::Bind(&ChunkDemuxerTest::DemuxerNeedKey, base::Unretained(this));
     demuxer_.reset(
-        new ChunkDemuxer(open_cb, need_key_cb, base::Bind(&LogFunc), false));
+        new ChunkDemuxer(open_cb, need_key_cb, base::Bind(&LogFunc), true));
   }
 
   virtual ~ChunkDemuxerTest() {
@@ -569,7 +569,7 @@ class ChunkDemuxerTest : public testing::Test {
     // the files are fixed to have the correct duration in their init segments,
     // and the CreateInitDoneCB() call, above, is fixed to used that duration.
     // See http://crbug.com/354284.
-    EXPECT_CALL(host_, SetDuration(base::TimeDelta::FromMilliseconds(2768)));
+    EXPECT_CALL(host_, SetDuration(base::TimeDelta::FromMilliseconds(2746)));
     AppendData(bear1->data(), bear1->data_size());
     // Last audio frame has timestamp 2721 and duration 24 (estimated from max
     // seen so far for audio track).
@@ -1662,7 +1662,7 @@ TEST_F(ChunkDemuxerTest, WebMFile_AudioAndVideo) {
   // TODO(wolenetz/acolwell): Remove this SetDuration expectation and update the
   // ParseWebMFile() call's expected duration, below, 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(2768)));
+  EXPECT_CALL(host_, SetDuration(base::TimeDelta::FromMilliseconds(2746)));
 
   ASSERT_TRUE(ParseWebMFile("bear-320x240.webm", buffer_timestamps,
                             base::TimeDelta::FromMilliseconds(2744)));
@@ -1695,7 +1695,7 @@ TEST_F(ChunkDemuxerTest, WebMFile_AudioOnly) {
   // TODO(wolenetz/acolwell): Remove this SetDuration expectation and update the
   // ParseWebMFile() call's expected duration, below, 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(2768)));
+  EXPECT_CALL(host_, SetDuration(base::TimeDelta::FromMilliseconds(2746)));
 
   ASSERT_TRUE(ParseWebMFile("bear-320x240-audio-only.webm", buffer_timestamps,
                             base::TimeDelta::FromMilliseconds(2744),
@@ -1732,11 +1732,6 @@ TEST_F(ChunkDemuxerTest, WebMFile_AltRefFrames) {
     {kSkip, kSkip},
   };
 
-  // TODO(wolenetz/acolwell): Remove this SetDuration expectation and update the
-  // ParseWebMFile() call's expected duration, below, 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(2768)));
-
   ASSERT_TRUE(ParseWebMFile("bear-320x240-altref.webm", buffer_timestamps,
                             base::TimeDelta::FromMilliseconds(2767)));
 }
@@ -2283,7 +2278,7 @@ 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)));
-  EXPECT_CALL(host_, SetDuration(base::TimeDelta::FromMilliseconds(366)));
+  EXPECT_CALL(host_, SetDuration(base::TimeDelta::FromMilliseconds(334)));
   AppendSingleStreamCluster(kSourceId, kAudioTrackNum, "200K 223K");
   AppendSingleStreamCluster(kSourceId, kVideoTrackNum,
                             "200K 233 266 299 300K 333");
@@ -2308,7 +2303,7 @@ TEST_F(ChunkDemuxerTest, GetBufferedRanges_EndOfStream) {
   // Audio: [0,46) [200,246)
   // Video: [0,66) [200,266) [300,366)

[wolenetz, 2014/04/02 00:47:07]

nit: this comment is now incorrect, since 1ms was the minimum video frame
duration since last webm segment. s/266/234/ and s/366/334/

[DaleCurtis, 2014/04/02 21:53:43]

Done.

[wolenetz, 2014/04/03 20:01:40]

(For posterity/clarification, I meant "since last webm init segment". This
doesn't change my meaning nor impact your "done" comment.)

   // NOTE: The last range on each stream do not overlap in time.
-  CheckExpectedRanges("{ [0,46) [200,246) }");
+  CheckExpectedRanges("{ [0,46) [200,234) }");
 
   MarkEndOfStream(PIPELINE_OK);
 
@@ -2316,7 +2311,7 @@ TEST_F(ChunkDemuxerTest, GetBufferedRanges_EndOfStream) {
   // end timestamp according to the spec. The last audio range gets extended
   // from [200,246) to [200,366) which is why the intersection results in the

[wolenetz, 2014/04/02 00:47:07]

s/366/334, but then the rest of the comment is incorrect. Maybe drop the 299
video frame from ever being appended, above, to not have 1ms be minimum video
frame duration estimation?

[DaleCurtis, 2014/04/02 21:53:43]

Comment removed. Nah, that avoids the multiple generated ranges, which I assume
was intentional.

[wolenetz, 2014/04/03 20:01:40]

That's fine, though removing the 299 append would still result in multiple
generated video ranges due to the subsequent demuxer_->Remove(200-300). Leaving
299 in (and 1ms minimum video frame dur) is more interesting - see acolwell's
comment
https://codereview.chromium.org/220103002/diff/60001/media/filters/chunk_demu...

   // middle range getting larger AND the new range appearing.
-  CheckExpectedRanges("{ [0,46) [200,266) [300,366) }");
+  CheckExpectedRanges("{ [0,46) [200,234) [300,334) }");
 }
 
 TEST_F(ChunkDemuxerTest, DifferentStreamTimecodes) {
@@ -2569,10 +2564,20 @@ TEST_F(ChunkDemuxerTest, ConfigChange_Audio) {
 
   ExpectRead(DemuxerStream::AUDIO, 0);
 
+  // The first config change seen is from a splice frame representing an overlap
+  // of buffer from config 1 by buffers from config 2.
   ReadUntilNotOkOrEndOfStream(DemuxerStream::AUDIO, &status, &last_timestamp);
-
   ASSERT_EQ(status, DemuxerStream::kConfigChanged);
   EXPECT_EQ(last_timestamp.InMilliseconds(), 524);
+  const AudioDecoderConfig& audio_config_1_too = audio->audio_decoder_config();

[wolenetz, 2014/04/02 00:47:07]

I'm not convinced that we need to emit new, but unchanged, decoder configs. If
the config has not changed since the last read, why not suppress such redundant
notifications?

[DaleCurtis, 2014/04/02 21:53:43]

Switched to matches.

[wolenetz, 2014/04/03 20:01:40]

I believe my original question is not yet answered.

[DaleCurtis, 2014/04/08 21:29:08]

Every time a config change occurs it must be completed with a call to retrieve
the decoder config.

+  ASSERT_TRUE(audio_config_1_too.IsValidConfig());
+  EXPECT_EQ(audio_config_1_too.samples_per_second(), 44100);

[wolenetz, 2014/04/02 00:47:07]

nit: directly compare configs using ::Matches(), here and below?

[DaleCurtis, 2014/04/02 21:53:43]

Done.

+  EXPECT_EQ(audio_config_1_too.extra_data_size(), 3863u);
+
+  // The next is due to a typical config difference.
+  ReadUntilNotOkOrEndOfStream(DemuxerStream::AUDIO, &status, &last_timestamp);
+  ASSERT_EQ(status, DemuxerStream::kConfigChanged);
+  EXPECT_EQ(last_timestamp.InMilliseconds(), 527);
 
   // Fetch the new decoder config.
   const AudioDecoderConfig& audio_config_2 = audio->audio_decoder_config();
@@ -2580,22 +2585,29 @@ TEST_F(ChunkDemuxerTest, ConfigChange_Audio) {
   EXPECT_EQ(audio_config_2.samples_per_second(), 44100);
   EXPECT_EQ(audio_config_2.extra_data_size(), 3935u);
 
-  ExpectRead(DemuxerStream::AUDIO, 527);
+  // The next config change is from a splice frame representing an overlap of
+  // buffers from config 2 by buffers from config 1.
+  ReadUntilNotOkOrEndOfStream(DemuxerStream::AUDIO, &status, &last_timestamp);
+  ASSERT_EQ(status, DemuxerStream::kConfigChanged);
+  EXPECT_EQ(last_timestamp.InMilliseconds(), 782);
+  const AudioDecoderConfig& audio_config_2_too = audio->audio_decoder_config();
+  ASSERT_TRUE(audio_config_2_too.IsValidConfig());
+  EXPECT_EQ(audio_config_2_too.samples_per_second(), 44100);
+  EXPECT_EQ(audio_config_2_too.extra_data_size(), 3935u);
 
-  // Read until the next config change.
   ReadUntilNotOkOrEndOfStream(DemuxerStream::AUDIO, &status, &last_timestamp);
+
   ASSERT_EQ(status, DemuxerStream::kConfigChanged);
-  EXPECT_EQ(last_timestamp.InMilliseconds(), 759);
+  EXPECT_EQ(last_timestamp.InMilliseconds(), 779);
 
   // Get the new config and verify that it matches the first one.
   ASSERT_TRUE(audio_config_1.Matches(audio->audio_decoder_config()));
 
-  ExpectRead(DemuxerStream::AUDIO, 779);
-
   // Read until the end of the stream just to make sure there aren't any other
   // config changes.
   ReadUntilNotOkOrEndOfStream(DemuxerStream::AUDIO, &status, &last_timestamp);
   ASSERT_EQ(status, DemuxerStream::kOk);
+  EXPECT_EQ(last_timestamp.InMilliseconds(), 2744);
 }
 
 TEST_F(ChunkDemuxerTest, ConfigChange_Seek) {