Add config change handling to SourceBufferStream & ChunkDemuxer

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 a05d2342e6945d832f9f3c29d8823571c92ae1ac..0ee48085ea729d694615e221eb12991b92106e0d 100644
--- a/media/filters/chunk_demuxer_unittest.cc
+++ b/media/filters/chunk_demuxer_unittest.cc
@@ -356,6 +356,48 @@ class ChunkDemuxerTest : public testing::Test {
     return AppendInitSegment(true, true, false);
   }
 
+  bool InitDemuxerWithConfigChangeData() {

[vrk (LEFT CHROMIUM), 2012/07/25 21:18:22]

I think you should add a comment before this method to explain what it does and
to highlight the hardcoded values that one would need to know in order to write
the TestConfigChange tests that you wrote below. I think you should mention
there are 2 configs: config 1 describes 320x240 video, config 2 describes
640x360. Then mention that this method fills the buffered region from [0, 2737)
and that (0, 467) is associated with config 1, (501 to 767) config 2, and (801,
end) config 1.

+    scoped_refptr<DecoderBuffer> bear1 = ReadTestDataFile("bear-320x240.webm");
+    scoped_refptr<DecoderBuffer> bear2 = ReadTestDataFile("bear-640x360.webm");
+
+    EXPECT_CALL(*client_, DemuxerOpened(_));
+    demuxer_->Initialize(
+        &host_, CreateInitDoneCB(base::TimeDelta::FromMilliseconds(2744),
+                                 PIPELINE_OK));
+
+    if (AddId(kSourceId, true, true) != ChunkDemuxer::kOk)
+      return false;
+
+    // Append the whole bear1 file.
+    if (!AppendData(bear1->GetData(), bear1->GetDataSize()))
+      return false;
+    CheckExpectedRanges(kSourceId, "{ [0,2737) }");
+
+    // Append initialization segment for bear2

[vrk (LEFT CHROMIUM), 2012/07/25 21:18:22]

nit: period at the end

[acolwell GONE FROM CHROMIUM, 2012/07/25 23:39:23]

Done.

+    // Note: Offsets here and below are derived from
+    // media/test/data/bear-640x360-manifest.js and
+    // media/test/data/bear-320x240-manifest.js which were
+    // generated from media/test/data/bear-640x360.webm and
+    // media/test/data/bear-320x240.webm respectively.
+    if (!AppendData(bear2->GetData(), 4459))
+      return false;
+
+    // Append a media segment that goes from [0.477000, 0.988000)

[vrk (LEFT CHROMIUM), 2012/07/25 21:18:22]

nit: period at end

[acolwell GONE FROM CHROMIUM, 2012/07/25 23:39:23]

Done.

+    if (!AppendData(bear2->GetData() + 55173, 19021))
+      return false;
+    CheckExpectedRanges(kSourceId, "{ [0,1002) [1201,2737) }");
+
+    // Append initialization segment for bear1 & fill gap with [770-1197)
+    // segment.
+    if (!AppendData(bear1->GetData(), 4370) ||
+        !AppendData(bear1->GetData() + 72737, 28183)) {
+      return false;
+    }
+    CheckExpectedRanges(kSourceId, "{ [0,2737) }");
+
+    return demuxer_->EndOfStream(PIPELINE_OK);
+  }
+
   void ShutdownDemuxer() {
     if (demuxer_) {
       EXPECT_CALL(*client_, DemuxerClosed());
@@ -531,6 +573,12 @@ class ChunkDemuxerTest : public testing::Test {
                             base::Unretained(this)));
   }
 
+  void ExpectConfigChanged(DemuxerStream* stream) {
+    EXPECT_CALL(*this, ReadDone(DemuxerStream::kConfigChanged, _));
+    stream->Read(base::Bind(&ChunkDemuxerTest::ReadDone,
+                            base::Unretained(this)));
+  }
+
   MOCK_METHOD1(Checkpoint, void(int id));
 
   struct BufferTimestamps {
@@ -1801,4 +1849,131 @@ TEST_F(ChunkDemuxerTest, TestEndOfStreamDuringSeek) {
   end_of_stream_helper.CheckIfReadDonesWereCalled(true);
 }
 
+static void ConfigChangeReadDone(DemuxerStream::Status* status_out,
+                                 scoped_refptr<DecoderBuffer>* buffer_out,
+                                 DemuxerStream::Status status,
+                                 const scoped_refptr<DecoderBuffer>& buffer) {
+  *status_out = status;
+  *buffer_out = buffer;
+}
+
+static void ReadUntilNotOkOrEndOfStream(
+    const scoped_refptr<DemuxerStream>& stream,
+    DemuxerStream::Status* status,
+    base::TimeDelta* last_timestamp) {
+  scoped_refptr<DecoderBuffer> buffer;
+
+  *last_timestamp = kNoTimestamp();
+  do {
+    stream->Read(base::Bind(&ConfigChangeReadDone, status, &buffer));
+    if (*status == DemuxerStream::kOk && !buffer->IsEndOfStream())
+      *last_timestamp = buffer->GetTimestamp();
+
+  } while (*status == DemuxerStream::kOk && !buffer->IsEndOfStream());
+}
+
+TEST_F(ChunkDemuxerTest, TestConfigChange) {
+  InSequence s;
+
+  ASSERT_TRUE(InitDemuxerWithConfigChangeData());
+
+  scoped_refptr<DemuxerStream> stream =
+      demuxer_->GetStream(DemuxerStream::VIDEO);
+  DemuxerStream::Status status;
+  base::TimeDelta last_timestamp;
+
+  // Fetch initial video config and verify it matches what we expect.
+  const VideoDecoderConfig& video_config = stream->video_decoder_config();
+  ASSERT_TRUE(video_config.IsValidConfig());
+  EXPECT_EQ(video_config.natural_size().width(), 320);
+  EXPECT_EQ(video_config.natural_size().height(), 240);
+
+  ReadUntilNotOkOrEndOfStream(stream, &status, &last_timestamp);
+
+  ASSERT_EQ(status, DemuxerStream::kConfigChanged);
+  EXPECT_EQ(last_timestamp.InMilliseconds(), 467);
+
+  // Verify that another read will result in kConfigChanged being returned
+  // again.
+  ExpectConfigChanged(stream);
+
+  // Fetch the new decoder config.
+  const VideoDecoderConfig& video_config2 = stream->video_decoder_config();
+  ASSERT_TRUE(video_config2.IsValidConfig());
+  EXPECT_EQ(video_config2.natural_size().width(), 640);
+  EXPECT_EQ(video_config2.natural_size().height(), 360);
+
+  ExpectRead(stream, 501);
+
+  // Read until the next config change.
+  ReadUntilNotOkOrEndOfStream(stream, &status, &last_timestamp);
+  ASSERT_EQ(status, DemuxerStream::kConfigChanged);
+  EXPECT_EQ(last_timestamp.InMilliseconds(), 767);
+
+  // Verify we get another ConfigChanged status.
+  ExpectConfigChanged(stream);
+
+  // Get the new config and verify that it matches the first one.
+  ASSERT_TRUE(video_config.Matches(stream->video_decoder_config()));

[vrk (LEFT CHROMIUM), 2012/07/25 21:18:22]

nit: In this test and below, maybe s/video_config/&1 to make it extra clear that
it's the first video config.

[acolwell GONE FROM CHROMIUM, 2012/07/25 23:39:23]

Done.

+
+  ExpectRead(stream, 801);
+
+  // Read until the end of the stream just to make sure there aren't any other
+  // config changes.
+  ReadUntilNotOkOrEndOfStream(stream, &status, &last_timestamp);
+  ASSERT_EQ(status, DemuxerStream::kOk);
+}
+
+TEST_F(ChunkDemuxerTest, TestConfigChange_Seek) {
+  InSequence s;
+
+  ASSERT_TRUE(InitDemuxerWithConfigChangeData());
+
+  scoped_refptr<DemuxerStream> stream =
+      demuxer_->GetStream(DemuxerStream::VIDEO);
+
+  // Fetch initial video config and verify it matches what we expect.
+  const VideoDecoderConfig& video_config = stream->video_decoder_config();
+  ASSERT_TRUE(video_config.IsValidConfig());
+  EXPECT_EQ(video_config.natural_size().width(), 320);
+  EXPECT_EQ(video_config.natural_size().height(), 240);
+
+  ExpectRead(stream, 0);
+
+  // Seek to a location with a different config.
+  demuxer_->Seek(base::TimeDelta::FromMilliseconds(501),
+                 NewExpectedStatusCB(PIPELINE_OK));
+
+  // Verify that the config change is signalled.
+  ExpectConfigChanged(stream);
+
+  // Fetch the new decoder config and verify it is what we expect.
+  const VideoDecoderConfig& video_config2 = stream->video_decoder_config();
+  ASSERT_TRUE(video_config2.IsValidConfig());
+  EXPECT_EQ(video_config2.natural_size().width(), 640);
+  EXPECT_EQ(video_config2.natural_size().height(), 360);
+
+  // Verify that Read() will return a buffer now.
+  ExpectRead(stream, 501);
+
+  // Seek back to the beginning and verify we get another config change.
+  demuxer_->Seek(base::TimeDelta::FromMilliseconds(0),
+                 NewExpectedStatusCB(PIPELINE_OK));
+  ExpectConfigChanged(stream);
+  ASSERT_TRUE(video_config.Matches(stream->video_decoder_config()));
+  ExpectRead(stream, 0);
+
+  // Seek to a location that requires a config change and then
+  // seek to a new location that has the same configuration as
+  // the start of the file without a Read() in the middle.
+  demuxer_->Seek(base::TimeDelta::FromMilliseconds(501),
+                 NewExpectedStatusCB(PIPELINE_OK));
+  demuxer_->Seek(base::TimeDelta::FromMilliseconds(801),
+                 NewExpectedStatusCB(PIPELINE_OK));
+
+  // Verify that no config change is signalled.
+  ExpectRead(stream, 801);
+  ASSERT_TRUE(video_config.Matches(stream->video_decoder_config()));
+}
+
 }  // namespace media