Implement support for 2 source ids (1 for audio and 1 for video).

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 6eed52c43c58107e4b293c5ccbce2595a1e785f5..629d2549e972e9c92f67f4820ce2296f4c9dd107 100644
--- a/media/filters/chunk_demuxer_unittest.cc
+++ b/media/filters/chunk_demuxer_unittest.cc
@@ -76,6 +76,12 @@ static void OnReadDone(const base::TimeDelta& expected_time,
   *called = true;
 }
 
+static void OnReadDone_EOSExpected(bool* called,
+                                   const scoped_refptr<DecoderBuffer>& buffer) {
+  EXPECT_TRUE(buffer->IsEndOfStream());

[annacc, 2012/06/07 22:42:07]

I thought about modifying EndOfStreamHelper to check for just one stream, but
found this to be simpler.

+  *called = true;
+}
+
 class MockChunkDemuxerClient : public ChunkDemuxerClient {
  public:
   MockChunkDemuxerClient() {}
@@ -175,19 +181,43 @@ class ChunkDemuxerTest : public testing::Test {
   }
 
   ChunkDemuxer::Status AddId() {
-    std::vector<std::string> codecs(2);
-    codecs[0] = "vp8";
-    codecs[1] = "vorbis";
-    return demuxer_->AddId(kSourceId, "video/webm", codecs);
+    return AddId(kSourceId, true, true);
+  }
+
+  ChunkDemuxer::Status AddId(const std::string& source_id,
+                             bool has_audio, bool has_video) {
+    std::vector<std::string> codecs;
+    std::string type;
+
+    if (has_audio) {
+      codecs.push_back("vorbis");
+      type = "audio/webm";
+    }
+
+    if (has_video) {
+      codecs.push_back("vp8");
+      type = "video/webm";
+    }
+
+    if (!has_audio && !has_video) {
+      return AddId(kSourceId, true, true);
+    }

[annacc, 2012/06/07 22:42:07]

This is required because TestInit appends info tracks with no audio and no
video.  I don't think we want to allow that in real life (in fact the codecs
specified in AddId() must be > 0).  I'm not sure if this is the right
solution...

[acolwell GONE FROM CHROMIUM, 2012/06/08 00:29:07]

This looks right to me.

+
+    return demuxer_->AddId(source_id, type, codecs);
   }
 
   bool AppendData(const uint8* data, size_t length) {
+    return AppendData(kSourceId, data, length);
+  }
+
+  bool AppendData(const std::string& source_id,
+                  const uint8* data, size_t length) {
     CHECK(length);
     EXPECT_CALL(host_, AddBufferedByteRange(_, _)).Times(AnyNumber())
         .WillRepeatedly(SaveArg<1>(&buffered_bytes_));
     EXPECT_CALL(host_, SetNetworkActivity(true))
         .Times(AnyNumber());
-    return demuxer_->AppendData(kSourceId, data, length);
+    return demuxer_->AppendData(source_id, data, length);
   }
 
   bool AppendDataInPieces(const uint8* data, size_t length) {
@@ -212,11 +242,18 @@ class ChunkDemuxerTest : public testing::Test {
 
   bool AppendInfoTracks(bool has_audio, bool has_video,
                         bool video_content_encoded) {
+    return AppendInfoTracks(kSourceId, has_audio, has_video,
+                            video_content_encoded);
+  }
+
+  bool AppendInfoTracks(const std::string& source_id,
+                        bool has_audio, bool has_video,
+                        bool video_content_encoded) {
     scoped_array<uint8> info_tracks;
     int info_tracks_size = 0;
     CreateInfoTracks(has_audio, has_video, video_content_encoded,
                      &info_tracks, &info_tracks_size);
-    return AppendData(info_tracks.get(), info_tracks_size);
+    return AppendData(source_id, info_tracks.get(), info_tracks_size);
   }
 
   void InitDoneCalled(PipelineStatus expected_status,
@@ -243,12 +280,29 @@ class ChunkDemuxerTest : public testing::Test {
     demuxer_->Initialize(
         &host_, CreateInitDoneCB(kDefaultDuration(), expected_status));
 
-    if (AddId() != ChunkDemuxer::kOk)
+    if (AddId(kSourceId, has_audio, has_video) != ChunkDemuxer::kOk)
       return false;
 
     return AppendInfoTracks(has_audio, has_video, video_content_encoded);
   }
 
+  bool InitDemuxerAudioAndVideoSources(const std::string& audio_id,
+                                       const std::string& video_id) {
+    EXPECT_CALL(*client_, DemuxerOpened(_));
+    demuxer_->Initialize(
+        &host_, CreateInitDoneCB(kDefaultDuration(), PIPELINE_OK));
+
+    if (AddId(audio_id, true, false) != ChunkDemuxer::kOk)
+      return false;
+    if (AddId(video_id, false, true) != ChunkDemuxer::kOk)
+      return false;
+
+    bool success;
+    success = AppendInfoTracks(audio_id, true, false, false);

[acolwell GONE FROM CHROMIUM, 2012/06/08 00:29:07]

Move this assignement to the line above.

[annacc, 2012/06/08 22:12:23]

Done.

+    success = success && AppendInfoTracks(video_id, false, true, false);

[acolwell GONE FROM CHROMIUM, 2012/06/08 00:29:07]

You could use &= here

[annacc, 2012/06/08 22:12:23]

Done.

+    return success;
+  }
+
   void ShutdownDemuxer() {
     if (demuxer_) {
       EXPECT_CALL(*client_, DemuxerClosed());
@@ -315,6 +369,32 @@ class ChunkDemuxerTest : public testing::Test {
     return cb.Finish();
   }
 
+  scoped_ptr<Cluster> GenerateSingleStreamCluster(int audio_timecode,

[acolwell GONE FROM CHROMIUM, 2012/06/08 00:29:07]

how about just timecode since I don't see anything audio specific here?

[annacc, 2012/06/08 22:12:23]

Whoops, leftover from a previous idea.  Done.

+                                                  int block_count,
+                                                  int track_number,
+                                                  int block_duration) {
+    CHECK_GT(block_count, 0);
+
+    int size = 10;
+    scoped_array<uint8> data(new uint8[size]);
+
+    ClusterBuilder cb;
+    cb.SetClusterTimecode(audio_timecode);
+
+    // Create simple blocks for everything except the last block.
+    for (int i = 0; i < block_count - 1; i++) {
+      cb.AddSimpleBlock(track_number, audio_timecode, kWebMFlagKeyframe,
+                        data.get(), size);
+      audio_timecode += block_duration;
+    }
+
+    // Make the last block a BlockGroups so that it doesn't get delayed by the

[acolwell GONE FROM CHROMIUM, 2012/06/08 00:29:07]

s/BlockGroups/BlockGroup

[annacc, 2012/06/08 22:12:23]

Done.

+    // block duration calculation logic.
+    cb.AddBlockGroup(track_number, audio_timecode, block_duration,
+                      kWebMFlagKeyframe, data.get(), size);
+    return cb.Finish();
+  }
+
   void GenerateExpectedReads(int timecode, int block_count,
                              DemuxerStream* audio,
                              DemuxerStream* video) {
@@ -364,11 +444,18 @@ class ChunkDemuxerTest : public testing::Test {
   bool ParseWebMFile(const std::string& filename,
                      const BufferTimestamps* timestamps,
                      const base::TimeDelta& duration) {
+    return ParseWebMFile(filename, timestamps, duration, true, true);
+  }
+
+  bool ParseWebMFile(const std::string& filename,
+                     const BufferTimestamps* timestamps,
+                     const base::TimeDelta& duration,
+                     bool has_audio, bool has_video) {
     EXPECT_CALL(*client_, DemuxerOpened(_));
     demuxer_->Initialize(
         &host_, CreateInitDoneCB(duration, PIPELINE_OK));
 
-    if (AddId() != ChunkDemuxer::kOk)
+    if (AddId(kSourceId, has_audio, has_video) != ChunkDemuxer::kOk)
       return false;
 
     // Read a WebM file into memory and send the data to the demuxer.
@@ -956,7 +1043,8 @@ TEST_F(ChunkDemuxerTest, TestWebMFile_AudioOnly) {
   };
 
   ASSERT_TRUE(ParseWebMFile("bear-320x240-audio-only.webm", buffer_timestamps,
-                            base::TimeDelta::FromMilliseconds(2744)));
+                            base::TimeDelta::FromMilliseconds(2744),
+                            true, false));
 }
 
 TEST_F(ChunkDemuxerTest, TestWebMFile_VideoOnly) {
@@ -970,7 +1058,8 @@ TEST_F(ChunkDemuxerTest, TestWebMFile_VideoOnly) {
   };
 
   ASSERT_TRUE(ParseWebMFile("bear-320x240-video-only.webm", buffer_timestamps,
-                            base::TimeDelta::FromMilliseconds(2703)));
+                            base::TimeDelta::FromMilliseconds(2703),
+                            false, true));
 }
 
 TEST_F(ChunkDemuxerTest, TestWebMFile_AltRefFrames) {
@@ -1096,4 +1185,162 @@ TEST_F(ChunkDemuxerTest, TestAVHeadersWithVideoOnlyType) {
   ASSERT_TRUE(AppendInfoTracks(true, true, false));
 }
 
+TEST_F(ChunkDemuxerTest, TestAddSeparateSourcesForAudioAndVideo) {
+  std::string audio_id = "audio1";
+  std::string video_id = "video1";
+  ASSERT_TRUE(InitDemuxerAudioAndVideoSources(audio_id, video_id));
+
+  scoped_refptr<DemuxerStream> audio =
+      demuxer_->GetStream(DemuxerStream::AUDIO);
+  scoped_refptr<DemuxerStream> video =
+      demuxer_->GetStream(DemuxerStream::VIDEO);
+
+  bool audio_read_done = false;
+  bool video_read_done = false;
+  audio->Read(base::Bind(&OnReadDone,
+                         base::TimeDelta::FromMilliseconds(0),
+                         &audio_read_done));
+
+  video->Read(base::Bind(&OnReadDone,
+                         base::TimeDelta::FromMilliseconds(0),
+                         &video_read_done));
+
+  scoped_ptr<Cluster> cluster_a(
+      GenerateSingleStreamCluster(0, 4, kAudioTrackNum, kAudioBlockDuration));
+
+  scoped_ptr<Cluster> cluster_v(
+      GenerateSingleStreamCluster(0, 4, kVideoTrackNum, kVideoBlockDuration));
+
+  // Append audio and video data into separate source ids.
+  ASSERT_TRUE(AppendData(audio_id, cluster_a->data(), cluster_a->size()));
+  ASSERT_TRUE(AppendData(video_id, cluster_v->data(), cluster_v->size()));
+
+  EXPECT_TRUE(audio_read_done);
+  EXPECT_TRUE(video_read_done);

[acolwell GONE FROM CHROMIUM, 2012/06/08 00:29:07]

Consider creating the single stream equivalent of GenerateExpectedReads() and
verify that we get the expected buffers.

[annacc, 2012/06/08 22:12:23]

Done.

+}
+
+TEST_F(ChunkDemuxerTest, TestAddIdFailures) {
+  EXPECT_CALL(*client_, DemuxerOpened(_));
+  demuxer_->Initialize(
+      &host_, CreateInitDoneCB(kDefaultDuration(), PIPELINE_OK));
+
+  std::string audio_id = "audio1";
+  std::string video_id = "video1";
+
+  ASSERT_EQ(AddId(audio_id, true, false), ChunkDemuxer::kOk);
+
+  // Adding an id with audio/video should fail because we already added audio.
+  ASSERT_EQ(AddId(), ChunkDemuxer::kReachedIdLimit);
+
+  ASSERT_TRUE(AppendInfoTracks(audio_id, true, false, false));
+
+  // Adding an id after append should fail.
+  ASSERT_EQ(AddId(video_id, false, true), ChunkDemuxer::kReachedIdLimit);
+}
+
+// Test that Read() calls after a RemoveId() return "end of stream" buffers.
+TEST_F(ChunkDemuxerTest, TestRemoveId) {
+  std::string audio_id = "audio1";
+  std::string video_id = "video1";
+  ASSERT_TRUE(InitDemuxerAudioAndVideoSources(audio_id, video_id));
+
+  scoped_ptr<Cluster> cluster_a(
+      GenerateSingleStreamCluster(0, 4, kAudioTrackNum, kAudioBlockDuration));
+
+  scoped_ptr<Cluster> cluster_v(
+      GenerateSingleStreamCluster(0, 4, kVideoTrackNum, kVideoBlockDuration));
+
+  // Append audio and video data into separate source ids.
+  ASSERT_TRUE(AppendData(audio_id, cluster_a->data(), cluster_a->size()));
+  ASSERT_TRUE(AppendData(video_id, cluster_v->data(), cluster_v->size()));
+
+  // Read() from audio should return normal buffers.
+  bool audio_read_done = false;
+  scoped_refptr<DemuxerStream> audio =
+      demuxer_->GetStream(DemuxerStream::AUDIO);
+  audio->Read(base::Bind(&OnReadDone,
+                         base::TimeDelta::FromMilliseconds(0),
+                         &audio_read_done));
+  EXPECT_TRUE(audio_read_done);
+
+  // Remove the audio id.
+  demuxer_->RemoveId(audio_id);
+
+  // Read() from audio should return "end of stream" buffers.
+  audio_read_done = false;
+  audio->Read(base::Bind(&OnReadDone_EOSExpected,
+                         &audio_read_done));
+  EXPECT_TRUE(audio_read_done);
+
+  // Read() from video should still return normal buffers.
+  bool video_read_done = false;
+  scoped_refptr<DemuxerStream> video =
+      demuxer_->GetStream(DemuxerStream::VIDEO);
+  video->Read(base::Bind(&OnReadDone,
+                         base::TimeDelta::FromMilliseconds(0),
+                         &video_read_done));
+  EXPECT_TRUE(video_read_done);
+}
+
+// Test that Seek() successfully seeks to all source IDs.
+TEST_F(ChunkDemuxerTest, TestSeekAudioAndVideoSources) {
+  std::string audio_id = "audio1";
+  std::string video_id = "video1";
+  ASSERT_TRUE(InitDemuxerAudioAndVideoSources(audio_id, video_id));
+
+  scoped_ptr<Cluster> cluster_a1(
+      GenerateSingleStreamCluster(0, 4, kAudioTrackNum, kAudioBlockDuration));
+
+  scoped_ptr<Cluster> cluster_v1(
+      GenerateSingleStreamCluster(0, 4, kVideoTrackNum, kVideoBlockDuration));
+
+  ASSERT_TRUE(AppendData(audio_id, cluster_a1->data(), cluster_a1->size()));
+  ASSERT_TRUE(AppendData(video_id, cluster_v1->data(), cluster_v1->size()));
+
+  // Read() should return buffers at 0.
+  bool audio_read_done = false;
+  bool video_read_done = false;
+  scoped_refptr<DemuxerStream> audio =
+      demuxer_->GetStream(DemuxerStream::AUDIO);
+  scoped_refptr<DemuxerStream> video =
+      demuxer_->GetStream(DemuxerStream::VIDEO);
+
+  audio->Read(base::Bind(&OnReadDone,
+                         base::TimeDelta::FromMilliseconds(0),
+                         &audio_read_done));
+  video->Read(base::Bind(&OnReadDone,
+                         base::TimeDelta::FromMilliseconds(0),
+                         &video_read_done));
+  EXPECT_TRUE(audio_read_done);
+  EXPECT_TRUE(video_read_done);
+
+  // Seek to 3.

[acolwell GONE FROM CHROMIUM, 2012/06/08 00:29:07]

Might want to also indicate that 3 is used because its an unbuffered region.

[annacc, 2012/06/08 22:12:23]

Done.

+  demuxer_->StartWaitingForSeek();
+  demuxer_->Seek(base::TimeDelta::FromSeconds(3),
+                 NewExpectedStatusCB(PIPELINE_OK));
+
+  scoped_ptr<Cluster> cluster_a2(
+      GenerateSingleStreamCluster(3000, 4, kAudioTrackNum,
+                                  kAudioBlockDuration));
+
+  scoped_ptr<Cluster> cluster_v2(
+      GenerateSingleStreamCluster(3000, 4, kVideoTrackNum,
+                                  kVideoBlockDuration));
+
+  ASSERT_TRUE(AppendData(audio_id, cluster_a2->data(), cluster_a2->size()));
+  ASSERT_TRUE(AppendData(video_id, cluster_v2->data(), cluster_v2->size()));
+
+  // Read() should return buffers at 3 after the seek.
+  audio_read_done = false;
+  video_read_done = false;
+  audio->Read(base::Bind(&OnReadDone,

[acolwell GONE FROM CHROMIUM, 2012/06/08 00:29:07]

WDYT about putting these reads right above the cluster_a2 declaration and adding
EXPECT_FALSE(audio_read_done) &EXPECT_FALSE(video_read_done) just to make sure
they don't accidentally return early for some reason?

+                         base::TimeDelta::FromSeconds(3),
+                         &audio_read_done));
+  video->Read(base::Bind(&OnReadDone,
+                         base::TimeDelta::FromSeconds(3),
+                         &video_read_done));
+  EXPECT_TRUE(audio_read_done);
+  EXPECT_TRUE(video_read_done);
+}
+
 }  // namespace media