Fix unit test failures with estimated durations and splice frames.

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 147 matching lines @@
       : append_window_end_for_next_append_(kInfiniteDuration()) {
     CreateNewDemuxer();
   }
 
   void CreateNewDemuxer() {
     base::Closure open_cb =
         base::Bind(&ChunkDemuxerTest::DemuxerOpened, base::Unretained(this));
     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() {
     ShutdownDemuxer();
   }
 
   void CreateInitSegment(int stream_flags,
                          bool is_audio_encrypted, bool is_video_encrypted,
                          scoped_ptr<uint8[]>* buffer,
                          int* size) {
@@ skipping 383 matching lines @@
                                  PIPELINE_OK), true);
 
     if (AddId(kSourceId, HAS_AUDIO | HAS_VIDEO) != ChunkDemuxer::kOk)
       return false;
 
     // Append the whole bear1 file.
     // TODO(wolenetz/acolwell): Remove this extra SetDuration expectation once
     // 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).
     // Last video frame has timestamp 2703 and duration 33 (from TrackEntry
     // DefaultDuration for video track).
     CheckExpectedRanges(kSourceId, "{ [0,2736) }");
 
     // Append initialization segment for bear2.
     // Note: Offsets here and below are derived from
     // media/test/data/bear-640x360-manifest.js and
@@ skipping 1072 matching lines @@
     {33, 3},
     {67, 6},
     {100, 9},
     {133, 12},
     {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)));
+  EXPECT_CALL(host_, SetDuration(base::TimeDelta::FromMilliseconds(2746)));
 
   ASSERT_TRUE(ParseWebMFile("bear-320x240.webm", buffer_timestamps,
                             base::TimeDelta::FromMilliseconds(2744)));
 }
 
 TEST_F(ChunkDemuxerTest, WebMFile_LiveAudioAndVideo) {
   struct BufferTimestamps buffer_timestamps[] = {
     {0, 0},
     {33, 3},
     {67, 6},
@@ skipping 12 matching lines @@
     {kSkip, 3},
     {kSkip, 6},
     {kSkip, 9},
     {kSkip, 12},
     {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)));
+  EXPECT_CALL(host_, SetDuration(base::TimeDelta::FromMilliseconds(2746)));
 
   ASSERT_TRUE(ParseWebMFile("bear-320x240-audio-only.webm", buffer_timestamps,
                             base::TimeDelta::FromMilliseconds(2744),
                             HAS_AUDIO));
 }
 
 TEST_F(ChunkDemuxerTest, WebMFile_VideoOnly) {
   struct BufferTimestamps buffer_timestamps[] = {
     {0, kSkip},
     {33, kSkip},
@@ skipping 16 matching lines @@
 TEST_F(ChunkDemuxerTest, WebMFile_AltRefFrames) {
   struct BufferTimestamps buffer_timestamps[] = {
     {0, 0},
     {33, 3},
     {33, 6},
     {67, 9},
     {100, 12},
     {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)));
 }
 
 // Verify that we output buffers before the entire cluster has been parsed.
 TEST_F(ChunkDemuxerTest, IncrementalClusterParsing) {
   ASSERT_TRUE(InitDemuxer(HAS_AUDIO | HAS_VIDEO));
   AppendEmptyCluster(0);
 
   scoped_ptr<Cluster> cluster(GenerateCluster(0, 6));
@@ skipping 526 matching lines @@
   CheckExpectedRanges("{ [0,66) }");
 
   // Verify that the range reverts to the intersection when end of stream
   // has been cancelled.
   demuxer_->UnmarkEndOfStream();
   CheckExpectedRanges("{ [0,46) }");
 
   // 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");
 
   // At this point, the per-stream ranges are as follows:
   // Audio: [0,46) [200,246)
   // Video: [0,66) [200,366)

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

s/366/334

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

Done.

   CheckExpectedRanges("{ [0,46) [200,246) }");
 
   demuxer_->Remove(kSourceId, base::TimeDelta::FromMilliseconds(200),
                    base::TimeDelta::FromMilliseconds(300));
 
   // At this point, the per-stream ranges are as follows:
   // Audio: [0,46)
   // Video: [0,66) [300,366)

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

s/366/334

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

Done.

   CheckExpectedRanges("{ [0,46) }");
 
   AppendSingleStreamCluster(kSourceId, kAudioTrackNum, "200K 223K");
   AppendSingleStreamCluster(kSourceId, kVideoTrackNum, "200K 233");
 
   // At this point, the per-stream ranges are as follows:
   // 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);
 
   // NOTE: The last range on each stream gets extended to the highest
   // 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) {
   ASSERT_TRUE(InitDemuxer(HAS_AUDIO | HAS_VIDEO));
 
   // Create a cluster where the video timecode begins 25ms after the audio.
   AppendCluster(GenerateCluster(0, 25, 8));
 
   Seek(base::TimeDelta::FromSeconds(0));
   GenerateExpectedReads(0, 25, 8);
@@ skipping 232 matching lines @@
   DemuxerStream* audio = demuxer_->GetStream(DemuxerStream::AUDIO);
 
   // Fetch initial audio config and verify it matches what we expect.
   const AudioDecoderConfig& audio_config_1 = audio->audio_decoder_config();
   ASSERT_TRUE(audio_config_1.IsValidConfig());
   EXPECT_EQ(audio_config_1.samples_per_second(), 44100);
   EXPECT_EQ(audio_config_1.extra_data_size(), 3863u);
 
   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();
   ASSERT_TRUE(audio_config_2.IsValidConfig());
   EXPECT_EQ(audio_config_2.samples_per_second(), 44100);
   EXPECT_EQ(audio_config_2.extra_data_size(), 3935u);
 
-  ExpectRead(DemuxerStream::AUDIO, 527);
-
-  // Read until the next config change.
+  // 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(), 759);
+  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);
+
+  ReadUntilNotOkOrEndOfStream(DemuxerStream::AUDIO, &status, &last_timestamp);
+
+  ASSERT_EQ(status, DemuxerStream::kConfigChanged);
+  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) {
   InSequence s;
 
   ASSERT_TRUE(InitDemuxerWithConfigChangeData());
 
   DemuxerStream* video = demuxer_->GetStream(DemuxerStream::VIDEO);
 
   // Fetch initial video config and verify it matches what we expect.
@@ skipping 580 matching lines @@
   // NOTE: we start at 175 here because the buffer at 125 was returned
   // to the pending read initiated above.
   CheckExpectedBuffers(text_stream, "175 225");
 
   // Verify that audio & video streams continue to return expected values.
   CheckExpectedBuffers(audio_stream, "160 180");
   CheckExpectedBuffers(video_stream, "180 210");
 }
 
 }  // namespace media