Support multiple media tracks in MSE / 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 0d7c9a1c0d04b668ef4789ac40576f72dcad1290..3155d5e3fdb77442205979632baeb5978452b1c1 100644
--- a/media/filters/chunk_demuxer_unittest.cc
+++ b/media/filters/chunk_demuxer_unittest.cc
@@ -125,13 +125,10 @@ MATCHER_P(HasTimestamp, timestamp_in_ms, "") {
 
 MATCHER(IsEndOfStream, "") { return arg.get() && arg->end_of_stream(); }
 
-MATCHER_P(SegmentMissingFrames, frame_types_string, "") {
-  return CONTAINS_STRING(arg, "Media segment did not contain any " +
-                                  std::string(frame_types_string) +
-                                  " coded frames, mismatching initialization "
-                                  "segment. Therefore, MSE coded frame "
-                                  "processing may not interoperably detect "
-                                  "discontinuities in appended media.");
+MATCHER_P(SegmentMissingFrames, track_id, "") {
+  return CONTAINS_STRING(
+      arg, "Media segment did not contain any coded frames for track " +
+               std::string(track_id));
 }
 
 MATCHER(StreamParsingFailed, "") {
@@ -150,19 +147,15 @@ MATCHER_P2(CodecName, stream_type_string, codec_string, "") {
          CONTAINS_STRING(arg, std::string(codec_string));
 }
 
-MATCHER_P2(InitSegmentMismatchesMimeType,
-           track_type_string_with_article,
-           mime_missing_track_type_bool,
-           "") {
-  return CONTAINS_STRING(
-      arg, "Initialization segment " +
-               std::string(mime_missing_track_type_bool ? "has "
-                                                        : "does not have ") +
-               std::string(track_type_string_with_article) +
-               " track, but the mimetype " +
-               std::string(mime_missing_track_type_bool ? "does not specify "
-                                                        : "specifies ") +
-               std::string(track_type_string_with_article) + " codec.");
+MATCHER_P2(InitSegmentMismatchesMimeType, stream_type, codec_name, "") {
+  return CONTAINS_STRING(arg, std::string(stream_type) + " stream codec " +
+                                  std::string(codec_name) +
+                                  " doesn't match SourceBuffer codecs.");
+}
+
+MATCHER_P(InitSegmentMissesExpectedTrack, missing_codec, "") {
+  return CONTAINS_STRING(arg, "Initialization segment misses expected " +
+                                  std::string(missing_codec) + " track.");
 }
 
 MATCHER_P2(GeneratedSplice, duration_microseconds, time_microseconds, "") {
@@ -388,22 +381,6 @@ class ChunkDemuxerTest : public ::testing::Test {
 
     // TODO(xhwang): Simplify this! Probably have test data files that contain
     // ContentEncodings directly instead of trying to create one at run-time.
-    if (has_audio) {
-      memcpy(buf, audio_track_entry->data(),
-             audio_track_entry->data_size());
-      if (is_audio_encrypted) {
-        memcpy(buf + audio_track_entry->data_size(),
-               audio_content_encodings->data(),
-               audio_content_encodings->data_size());
-        WriteInt64(buf + kAudioTrackSizeOffset,
-                   audio_track_entry->data_size() +
-                   audio_content_encodings->data_size() -
-                   kAudioTrackEntryHeaderSize);
-        buf += audio_content_encodings->data_size();
-      }
-      buf += audio_track_entry->data_size();
-    }
-
     if (has_video) {
       memcpy(buf, video_track_entry->data(),
              video_track_entry->data_size());
@@ -420,6 +397,21 @@ class ChunkDemuxerTest : public ::testing::Test {
       buf += video_track_entry->data_size();
     }
 
+    if (has_audio) {
+      memcpy(buf, audio_track_entry->data(), audio_track_entry->data_size());

[wolenetz, 2016/09/13 21:03:13]

aside: Why reorder this block here vs before "if (has_video)" previously?

[servolk, 2016/09/14 18:15:24]

Some tests in this file verify that media log events come in the expected order
by using testing::InSequence. Previously we would always report audio before
video (since in the old OnNewConfigs we had the code for handling audio above
the code for handling video). But with the new OnNewConfigs implementation the
order of a/v reporting will match the actual stream order in the init
segment/bitstream. And it just so happens that most of our test files have video
stream #0 and audio stream #1. So I've changed the order here as well to match
that.

[wolenetz, 2016/09/14 23:31:21]

Acknowledged.

+      if (is_audio_encrypted) {
+        memcpy(buf + audio_track_entry->data_size(),
+               audio_content_encodings->data(),
+               audio_content_encodings->data_size());
+        WriteInt64(buf + kAudioTrackSizeOffset,
+                   audio_track_entry->data_size() +
+                       audio_content_encodings->data_size() -
+                       kAudioTrackEntryHeaderSize);
+        buf += audio_content_encodings->data_size();
+      }
+      buf += audio_track_entry->data_size();
+    }
+
     if (has_text) {
       memcpy(buf, text_track_entry->data(),
              text_track_entry->data_size());
@@ -434,16 +426,19 @@ class ChunkDemuxerTest : public ::testing::Test {
   ChunkDemuxer::Status AddId(const std::string& source_id, int stream_flags) {
     bool has_audio = (stream_flags & HAS_AUDIO) != 0;
     bool has_video = (stream_flags & HAS_VIDEO) != 0;
-    std::vector<std::string> codecs;
+    std::string codecs;
     std::string type;
 
     if (has_audio) {
-      codecs.push_back("vorbis");
+      codecs += "vorbis";
       type = "audio/webm";
     }
 
     if (has_video) {
-      codecs.push_back("vp8");
+      if (codecs == "")
+        codecs = "vp8";
+      else
+        codecs += ",vp8";
       type = "video/webm";
     }
 
@@ -451,28 +446,19 @@ class ChunkDemuxerTest : public ::testing::Test {
       return AddId(kSourceId, HAS_AUDIO | HAS_VIDEO);
     }
 
-    ChunkDemuxer::Status status = demuxer_->AddId(source_id, type, codecs);
-    if (status == ChunkDemuxer::kOk)
-      demuxer_->SetTracksWatcher(
-          source_id, base::Bind(&ChunkDemuxerTest::InitSegmentReceived,
-                                base::Unretained(this)));
-    return status;
+    return AddId(source_id, type, codecs);
   }
 
-#if BUILDFLAG(ENABLE_MSE_MPEG2TS_STREAM_PARSER)
-  ChunkDemuxer::Status AddIdForMp2tSource(const std::string& source_id) {
-    std::vector<std::string> codecs;
-    std::string type = "video/mp2t";
-    codecs.push_back("mp4a.40.2");
-    codecs.push_back("avc1.640028");
-    ChunkDemuxer::Status status = demuxer_->AddId(source_id, type, codecs);
+  ChunkDemuxer::Status AddId(const std::string& source_id,
+                             const std::string& mime_type,
+                             const std::string& codecs) {
+    ChunkDemuxer::Status status = demuxer_->AddId(source_id, mime_type, codecs);
     if (status == ChunkDemuxer::kOk)
       demuxer_->SetTracksWatcher(
           source_id, base::Bind(&ChunkDemuxerTest::InitSegmentReceived,
                                 base::Unretained(this)));
     return status;
   }
-#endif
 
   bool AppendData(const uint8_t* data, size_t length) {
     return AppendData(kSourceId, data, length);
@@ -818,15 +804,14 @@ class ChunkDemuxerTest : public ::testing::Test {
   }
 
   void ExpectInitMediaLogs(int stream_flags) {
-    if (stream_flags & HAS_AUDIO) {
-      EXPECT_MEDIA_LOG(FoundStream("audio"));
-      EXPECT_MEDIA_LOG(CodecName("audio", "vorbis"));
-    }
-
     if (stream_flags & HAS_VIDEO) {
       EXPECT_MEDIA_LOG(FoundStream("video"));
       EXPECT_MEDIA_LOG(CodecName("video", "vp8"));
     }
+    if (stream_flags & HAS_AUDIO) {
+      EXPECT_MEDIA_LOG(FoundStream("audio"));
+      EXPECT_MEDIA_LOG(CodecName("audio", "vorbis"));
+    }
   }
 
   bool InitDemuxerWithEncryptionInfo(
@@ -864,6 +849,8 @@ class ChunkDemuxerTest : public ::testing::Test {
       EXPECT_CALL(*this, InitSegmentReceivedMock(_));
     } else {
       // OnNewConfigs() requires at least one audio, video, or text track.
+      EXPECT_MEDIA_LOG(InitSegmentMissesExpectedTrack("vorbis"));
+      EXPECT_MEDIA_LOG(InitSegmentMissesExpectedTrack("vp8"));
       EXPECT_MEDIA_LOG(StreamParsingFailed());
     }
 
@@ -2410,16 +2397,14 @@ TEST_F(ChunkDemuxerTest, AVHeadersWithAudioOnlyType) {
       &host_, CreateInitDoneCB(kNoTimestamp, CHUNK_DEMUXER_ERROR_APPEND_FAILED),
       true);
 
-  std::vector<std::string> codecs(1);
-  codecs[0] = "vorbis";
-  ASSERT_EQ(demuxer_->AddId(kSourceId, "audio/webm", codecs),
+  ASSERT_EQ(demuxer_->AddId(kSourceId, "audio/webm", "vorbis"),
             ChunkDemuxer::kOk);
   demuxer_->SetTracksWatcher(kSourceId,
                              base::Bind(&ChunkDemuxerTest::InitSegmentReceived,
                                         base::Unretained(this)));
 
   // Video track is unexpected per mimetype.
-  EXPECT_MEDIA_LOG(InitSegmentMismatchesMimeType("a video", true));
+  EXPECT_MEDIA_LOG(InitSegmentMismatchesMimeType("Video", "vp8"));
   EXPECT_MEDIA_LOG(StreamParsingFailed());
   ASSERT_FALSE(AppendInitSegment(HAS_AUDIO | HAS_VIDEO));
 }
@@ -2430,16 +2415,15 @@ TEST_F(ChunkDemuxerTest, AVHeadersWithVideoOnlyType) {
       &host_, CreateInitDoneCB(kNoTimestamp, CHUNK_DEMUXER_ERROR_APPEND_FAILED),
       true);
 
-  std::vector<std::string> codecs(1);
-  codecs[0] = "vp8";
-  ASSERT_EQ(demuxer_->AddId(kSourceId, "video/webm", codecs),
-            ChunkDemuxer::kOk);
+  ASSERT_EQ(demuxer_->AddId(kSourceId, "video/webm", "vp8"), ChunkDemuxer::kOk);
   demuxer_->SetTracksWatcher(kSourceId,
                              base::Bind(&ChunkDemuxerTest::InitSegmentReceived,
                                         base::Unretained(this)));
 
   // Audio track is unexpected per mimetype.
-  EXPECT_MEDIA_LOG(InitSegmentMismatchesMimeType("an audio", true));
+  EXPECT_MEDIA_LOG(FoundStream("video"));
+  EXPECT_MEDIA_LOG(CodecName("video", "vp8"));
+  EXPECT_MEDIA_LOG(InitSegmentMismatchesMimeType("Audio", "vorbis"));
   EXPECT_MEDIA_LOG(StreamParsingFailed());
   ASSERT_FALSE(AppendInitSegment(HAS_AUDIO | HAS_VIDEO));
 }
@@ -2450,17 +2434,16 @@ TEST_F(ChunkDemuxerTest, AudioOnlyHeaderWithAVType) {
       &host_, CreateInitDoneCB(kNoTimestamp, CHUNK_DEMUXER_ERROR_APPEND_FAILED),
       true);
 
-  std::vector<std::string> codecs(2);
-  codecs[0] = "vorbis";
-  codecs[1] = "vp8";
-  ASSERT_EQ(demuxer_->AddId(kSourceId, "video/webm", codecs),
+  ASSERT_EQ(demuxer_->AddId(kSourceId, "video/webm", "vorbis,vp8"),
             ChunkDemuxer::kOk);
   demuxer_->SetTracksWatcher(kSourceId,
                              base::Bind(&ChunkDemuxerTest::InitSegmentReceived,
                                         base::Unretained(this)));
 
   // Video track is also expected per mimetype.
-  EXPECT_MEDIA_LOG(InitSegmentMismatchesMimeType("a video", false));
+  EXPECT_MEDIA_LOG(FoundStream("audio"));
+  EXPECT_MEDIA_LOG(CodecName("audio", "vorbis"));
+  EXPECT_MEDIA_LOG(InitSegmentMissesExpectedTrack("vp8"));
   EXPECT_MEDIA_LOG(StreamParsingFailed());
   ASSERT_FALSE(AppendInitSegment(HAS_AUDIO));
 }
@@ -2471,17 +2454,16 @@ TEST_F(ChunkDemuxerTest, VideoOnlyHeaderWithAVType) {
       &host_, CreateInitDoneCB(kNoTimestamp, CHUNK_DEMUXER_ERROR_APPEND_FAILED),
       true);
 
-  std::vector<std::string> codecs(2);
-  codecs[0] = "vorbis";
-  codecs[1] = "vp8";
-  ASSERT_EQ(demuxer_->AddId(kSourceId, "video/webm", codecs),
+  ASSERT_EQ(demuxer_->AddId(kSourceId, "video/webm", "vorbis,vp8"),
             ChunkDemuxer::kOk);
   demuxer_->SetTracksWatcher(kSourceId,
                              base::Bind(&ChunkDemuxerTest::InitSegmentReceived,
                                         base::Unretained(this)));
 
   // Audio track is also expected per mimetype.
-  EXPECT_MEDIA_LOG(InitSegmentMismatchesMimeType("an audio", false));
+  EXPECT_MEDIA_LOG(FoundStream("video"));
+  EXPECT_MEDIA_LOG(CodecName("video", "vp8"));
+  EXPECT_MEDIA_LOG(InitSegmentMissesExpectedTrack("vorbis"));
   EXPECT_MEDIA_LOG(StreamParsingFailed());
   ASSERT_FALSE(AppendInitSegment(HAS_VIDEO));
 }
@@ -2548,9 +2530,6 @@ TEST_F(ChunkDemuxerTest, AddIdFailures) {
 
   ASSERT_EQ(AddId(audio_id, HAS_AUDIO), ChunkDemuxer::kOk);
 
-  // Adding an id with audio/video should fail because we already added audio.
-  ASSERT_EQ(AddId(), ChunkDemuxer::kReachedIdLimit);
-
   ExpectInitMediaLogs(HAS_AUDIO);
   EXPECT_CALL(*this, InitSegmentReceivedMock(_));
   ASSERT_TRUE(AppendInitSegmentWithSourceId(audio_id, HAS_AUDIO));
@@ -2576,12 +2555,16 @@ TEST_F(ChunkDemuxerTest, RemoveId) {
   // Read() from audio should return normal buffers.
   GenerateAudioStreamExpectedReads(0, 4);
 
+  // Audio stream will become inaccessible after |audio_id| is removed, so save
+  // it here to read from it after RemoveId.
+  DemuxerStream* audio_stream = demuxer_->GetStream(DemuxerStream::AUDIO);
+
   // Remove the audio id.
   demuxer_->RemoveId(audio_id);
 
   // Read() from audio should return "end of stream" buffers.
   bool audio_read_done = false;
-  ReadAudio(base::Bind(&OnReadDone_EOSExpected, &audio_read_done));
+  audio_stream->Read(base::Bind(&OnReadDone_EOSExpected, &audio_read_done));
   base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(audio_read_done);
 
@@ -3147,10 +3130,7 @@ TEST_F(ChunkDemuxerTest, CodecPrefixMatching) {
 #endif
 #endif
 
-  std::vector<std::string> codecs;
-  codecs.push_back("avc1.4D4041");
-
-  EXPECT_EQ(demuxer_->AddId("source_id", "video/mp4", codecs), expected);
+  EXPECT_EQ(demuxer_->AddId("source_id", "video/mp4", "avc1.4D4041"), expected);
 }
 
 // Test codec ID's that are not compliant with RFC6381, but have been
@@ -3168,11 +3148,8 @@ TEST_F(ChunkDemuxerTest, CodecIDsThatAreNotRFC6381Compliant) {
   };
 
   for (size_t i = 0; i < arraysize(codec_ids); ++i) {
-    std::vector<std::string> codecs;
-    codecs.push_back(codec_ids[i]);
-
     ChunkDemuxer::Status result =
-        demuxer_->AddId("source_id", "audio/mp4", codecs);
+        demuxer_->AddId("source_id", "audio/mp4", codec_ids[i]);
 
     EXPECT_EQ(result, expected)
         << "Fail to add codec_id '" << codec_ids[i] << "'";
@@ -3474,6 +3451,11 @@ TEST_F(ChunkDemuxerTest, IsParsingMediaSegmentMidMediaSegment) {
 
 #if defined(USE_PROPRIETARY_CODECS)
 #if BUILDFLAG(ENABLE_MSE_MPEG2TS_STREAM_PARSER)
+namespace {
+const char* kMp2tMimeType = "video/mp2t";
+const char* kMp2tCodecs = "mp4a.40.2,avc1.640028";
+}
+
 TEST_F(ChunkDemuxerTest, EmitBuffersDuringAbort) {
   EXPECT_CALL(*this, DemuxerOpened());
   EXPECT_MEDIA_LOG(FoundStream("audio"));
@@ -3482,7 +3464,7 @@ TEST_F(ChunkDemuxerTest, EmitBuffersDuringAbort) {
   EXPECT_MEDIA_LOG(CodecName("video", "h264"));
   demuxer_->Initialize(&host_, CreateInitDoneCB(kInfiniteDuration, PIPELINE_OK),
                        true);
-  EXPECT_EQ(ChunkDemuxer::kOk, AddIdForMp2tSource(kSourceId));
+  EXPECT_EQ(ChunkDemuxer::kOk, AddId(kSourceId, kMp2tMimeType, kMp2tCodecs));

[wolenetz, 2016/09/13 21:03:13]

aside: nice elimination of that helper, above.

[servolk, 2016/09/14 18:15:24]

Acknowledged.

 
   // For info:
   // DTS/PTS derived using dvbsnoop -s ts -if bear-1280x720.ts -tssubdecode
@@ -3530,7 +3512,7 @@ TEST_F(ChunkDemuxerTest, SeekCompleteDuringAbort) {
   EXPECT_MEDIA_LOG(CodecName("video", "h264"));
   demuxer_->Initialize(&host_, CreateInitDoneCB(kInfiniteDuration, PIPELINE_OK),
                        true);
-  EXPECT_EQ(ChunkDemuxer::kOk, AddIdForMp2tSource(kSourceId));
+  EXPECT_EQ(ChunkDemuxer::kOk, AddId(kSourceId, kMp2tMimeType, kMp2tCodecs));
 
   // For info:
   // DTS/PTS derived using dvbsnoop -s ts -if bear-1280x720.ts -tssubdecode
@@ -4454,48 +4436,36 @@ TEST_F(ChunkDemuxerTest, EvictCodedFramesTest) {
                                          80));
 
   Seek(base::TimeDelta::FromMilliseconds(0));
-  // Audio stream had 8 buffers, video stream had 15. We told EvictCodedFrames

[wolenetz, 2016/09/13 21:03:13]

Why is this portion of this test dropped?

[servolk, 2016/09/14 18:15:24]

This logic got broken when I temporarily switched to hard-coded estimates of the
eviction sizes for each stream. But now I've restored the logic in
EvictCodedFrames to use current stream buffered size to estimate the amount of
data that belongs to the current stream in the muxed data chunk, and now this
logic is valid again, so I've restored the test in the latest patchset.

-  // that the new data size is 8 blocks muxed, i.e. 80 bytes. Given the current
-  // ratio of video to the total data size (15 : (8+15) ~= 0.65) the estimated
-  // sizes of video and audio data in the new 80 byte chunk are 52 bytes for
-  // video (80*0.65 = 52) and 28 bytes for audio (80 - 52).
-  // Given these numbers MSE GC will remove just one audio block (since current
-  // audio size is 80 bytes, new data is 28 bytes, we need to remove just one 10
-  // byte block to stay under 100 bytes memory limit after append
-  // 80 - 10 + 28 = 98).
-  // For video stream 150 + 52 = 202. Video limit is 150 bytes. We need to
-  // remove at least 6 blocks to stay under limit.
-  CheckExpectedBuffers(audio_stream, "40K 80K 120K 160K 200K 240K 280K");
-  CheckExpectedBuffers(video_stream, "60K 70 80K 90 100K 110 120K 130 140K");
 }
 
 TEST_F(ChunkDemuxerTest, SegmentMissingAudioFrame_AudioOnly) {
   ASSERT_TRUE(InitDemuxer(HAS_AUDIO));
-  EXPECT_MEDIA_LOG(SegmentMissingFrames("audio"));
+  EXPECT_MEDIA_LOG(SegmentMissingFrames("2"));
   ASSERT_TRUE(AppendCluster(GenerateEmptyCluster(0)));
 }
 
 TEST_F(ChunkDemuxerTest, SegmentMissingVideoFrame_VideoOnly) {
   ASSERT_TRUE(InitDemuxer(HAS_VIDEO));
-  EXPECT_MEDIA_LOG(SegmentMissingFrames("video"));
+  EXPECT_MEDIA_LOG(SegmentMissingFrames("1"));
   ASSERT_TRUE(AppendCluster(GenerateEmptyCluster(0)));
 }
 
 TEST_F(ChunkDemuxerTest, SegmentMissingAudioFrame_AudioVideo) {
   ASSERT_TRUE(InitDemuxer(HAS_AUDIO | HAS_VIDEO));
-  EXPECT_MEDIA_LOG(SegmentMissingFrames("audio"));
+  EXPECT_MEDIA_LOG(SegmentMissingFrames("2"));
   AppendSingleStreamCluster(kSourceId, kVideoTrackNum, 0, 10);
 }
 
 TEST_F(ChunkDemuxerTest, SegmentMissingVideoFrame_AudioVideo) {
   ASSERT_TRUE(InitDemuxer(HAS_AUDIO | HAS_VIDEO));
-  EXPECT_MEDIA_LOG(SegmentMissingFrames("video"));
+  EXPECT_MEDIA_LOG(SegmentMissingFrames("1"));
   AppendSingleStreamCluster(kSourceId, kAudioTrackNum, 0, 10);
 }
 
 TEST_F(ChunkDemuxerTest, SegmentMissingAudioVideoFrames) {
   ASSERT_TRUE(InitDemuxer(HAS_AUDIO | HAS_VIDEO));
-  EXPECT_MEDIA_LOG(SegmentMissingFrames("audio or video"));
+  EXPECT_MEDIA_LOG(SegmentMissingFrames("1"));
+  EXPECT_MEDIA_LOG(SegmentMissingFrames("2"));
   ASSERT_TRUE(AppendCluster(GenerateEmptyCluster(0)));
 }
 
@@ -4629,7 +4599,7 @@ TEST_F(ChunkDemuxerTest,
   DemuxerStream* audio_stream = demuxer_->GetStream(DemuxerStream::AUDIO);
   DemuxerStream* video_stream = demuxer_->GetStream(DemuxerStream::VIDEO);
 
-  EXPECT_MEDIA_LOG(SegmentMissingFrames("video"));
+  EXPECT_MEDIA_LOG(SegmentMissingFrames("1"));
   AppendMuxedCluster(MuxedStreamInfo(kAudioTrackNum, "0K 10K 20D10K"),
                      MuxedStreamInfo(kVideoTrackNum, ""));
   demuxer_->Remove(kSourceId, base::TimeDelta(),
@@ -4744,4 +4714,80 @@ TEST_F(ChunkDemuxerTest, StreamStatusNotifications) {
   CheckStreamStatusNotifications(video_stream);
 }
 
+TEST_F(ChunkDemuxerTest, MultipleIds) {
+  CreateNewDemuxer();
+  EXPECT_CALL(*this, DemuxerOpened());
+  EXPECT_CALL(host_, SetDuration(_)).Times(2);
+  demuxer_->Initialize(&host_, CreateInitDoneCB(kNoTimestamp, PIPELINE_OK),
+                       true);
+
+  const char* kId1 = "id1";
+  const char* kId2 = "id2";
+  EXPECT_EQ(AddId(kId1, "video/webm", "opus,vp9"), ChunkDemuxer::kOk);
+  EXPECT_EQ(AddId(kId2, "video/webm", "opus,vp9"), ChunkDemuxer::kOk);
+  scoped_refptr<DecoderBuffer> data1 = ReadTestDataFile("green-a300hz.webm");
+  scoped_refptr<DecoderBuffer> data2 = ReadTestDataFile("red-a500hz.webm");
+
+  EXPECT_MEDIA_LOG(FoundStream("audio")).Times(2);
+  EXPECT_MEDIA_LOG(FoundStream("video")).Times(2);
+  EXPECT_MEDIA_LOG(CodecName("audio", "opus")).Times(2);
+  EXPECT_MEDIA_LOG(CodecName("video", "vp9")).Times(2);
+  EXPECT_CALL(*this, InitSegmentReceivedMock(_)).Times(2);
+  EXPECT_MEDIA_LOG(SegmentMissingFrames("1")).Times(AnyNumber());

[wolenetz, 2016/09/13 21:03:13]

Is this because the test media indeed has muxed segments that sometimes have
missing track media frames?  I just want to make sure that's understood.

[servolk, 2016/09/14 18:15:25]

Yes, if I remove this then the test fails due to unmatched media log message.
I'm not sure why some media segments are missing in the files generated by
ffmpeg. Perhaps we should use different set of codecs for this test?

[wolenetz, 2016/09/14 23:31:21]

The binary files used by this test don't apply when I try to apply this patch
locally, so I can't tell directly if the webm files are muxed with only audio or
only video in some segments. Can you share these files with me directly please?

[servolk, 2016/09/15 00:18:32]

Done.

[wolenetz, 2016/09/15 01:00:08]

Thank you. The files look ok. Specifically, "red-a500hz.webm" has a trailing
audio-only media segment that contains audio coded frames included in the last
video frame's presentation interval in the previous cluster. I don't know why
ffmpeg chose to partition the clusters this way, but that media log is
warranted. green-a300hz.webm has no such trailing single-track cluster, nor any
media log warning.

red-a500hz.webm shows the following in chrome://media-internals:

Media segment did not contain any video coded frames, mismatching initialization
segment. Therefore, MSE coded frame processing may not interoperably detect
discontinuities in appended media.

So, s/AnyNumber()/1/ and we can resolve this comment :)

[servolk, 2016/09/15 02:13:07]

Done.

+
+  EXPECT_TRUE(AppendData(kId1, data1->data(), data1->data_size()));
+  EXPECT_TRUE(AppendData(kId2, data2->data(), data2->data_size()));
+  CheckExpectedRanges(kId1, "{ [0,10007) }");
+  CheckExpectedRanges(kId2, "{ [0,10007) }");

[wolenetz, 2016/09/13 21:03:13]

nit: might be a slightly better test if the two sourcebuffers had differing
buffered range(s).

[servolk, 2016/09/14 18:15:24]

Done.

+}
+
+TEST_F(ChunkDemuxerTest, CompleteInitAfterIdRemoved) {
+  CreateNewDemuxer();
+  EXPECT_CALL(*this, DemuxerOpened());
+  demuxer_->Initialize(&host_,
+                       CreateInitDoneCB(kDefaultDuration(), PIPELINE_OK), true);
+
+  // Add two ids, then remove one of the ids and verify that adding data only

[wolenetz, 2016/09/13 21:03:13]

nit: s/data/init segment/

[servolk, 2016/09/14 18:15:24]

Done.

+  // for the remaining id still triggers the InitDoneCB.
+  const char* kId1 = "id1";
+  const char* kId2 = "id2";
+  EXPECT_EQ(AddId(kId1, "video/webm", "vp8"), ChunkDemuxer::kOk);
+  EXPECT_EQ(AddId(kId2, "video/webm", "vp9"), ChunkDemuxer::kOk);
+  demuxer_->RemoveId(kId2);
+
+  EXPECT_CALL(*this, InitSegmentReceivedMock(_));
+  EXPECT_MEDIA_LOG(WebMSimpleBlockDurationEstimated(30));

[wolenetz, 2016/09/13 21:03:13]

nit ditto of below.

[servolk, 2016/09/14 18:15:25]

Acknowledged.

+  EXPECT_MEDIA_LOG(FoundStream("video"));
+  EXPECT_MEDIA_LOG(CodecName("video", "vp8"));
+
+  ASSERT_TRUE(AppendInitSegmentWithSourceId(kId1, HAS_VIDEO));
+  AppendSingleStreamCluster(kId1, kVideoTrackNum, "0K 30 60 90");

[wolenetz, 2016/09/13 21:03:13]

nit: this line should not be necessary to trigger the initdone.

[servolk, 2016/09/14 18:15:25]

I agree this is not strictly necessary, so I can remove it, if you insist. But I
think it's useful to get some extra test coverage by appending some data as
well, since this will ensure that we exercise also the paths that handle data
stream parsing after RemoveId. For example, this will ensure that there is still
a track buffer for the remaining video id/stream, and that data parsing succeeds
(or at least doesn't cause any crashes due to missing some internal structures
that might have been accidentally removed by RemoveId).

[wolenetz, 2016/09/14 23:31:21]

I'm fine with keeping it.

[servolk, 2016/09/15 00:18:32]

Acknowledged.

+}
+
+TEST_F(ChunkDemuxerTest, RemovingIdMustRemoveStreams) {
+  CreateNewDemuxer();
+  EXPECT_CALL(*this, DemuxerOpened());
+  demuxer_->Initialize(&host_,
+                       CreateInitDoneCB(kDefaultDuration(), PIPELINE_OK), true);
+
+  const char* kId1 = "id1";
+  EXPECT_EQ(AddId(kId1, "video/webm", "vorbis,vp8"), ChunkDemuxer::kOk);
+
+  EXPECT_CALL(*this, InitSegmentReceivedMock(_));
+  EXPECT_MEDIA_LOG(FoundStream("video"));
+  EXPECT_MEDIA_LOG(CodecName("video", "vp8"));
+  EXPECT_MEDIA_LOG(FoundStream("audio"));
+  EXPECT_MEDIA_LOG(CodecName("audio", "vorbis"));
+
+  // Append init segment to ensure demuxer streams get created.
+  ASSERT_TRUE(AppendInitSegmentWithSourceId(kId1, HAS_AUDIO | HAS_VIDEO));
+  EXPECT_NE(nullptr, demuxer_->GetStream(DemuxerStream::AUDIO));
+  EXPECT_NE(nullptr, demuxer_->GetStream(DemuxerStream::VIDEO));
+
+  // Removing the id should remove also the DemuxerStreams.
+  demuxer_->RemoveId(kId1);
+  EXPECT_EQ(nullptr, demuxer_->GetStream(DemuxerStream::AUDIO));
+  EXPECT_EQ(nullptr, demuxer_->GetStream(DemuxerStream::VIDEO));
+}
+
 }  // namespace media