Support multiple media tracks in MSE / ChunkDemuxer

media/filters/media_source_state.cc

 // Copyright 2016 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 "media/filters/media_source_state.h"
 
+#include <set>
+
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "media/base/media_switches.h"
 #include "media/base/media_track.h"
 #include "media/base/media_tracks.h"
+#include "media/base/mime_util.h"
 #include "media/filters/chunk_demuxer.h"
 #include "media/filters/frame_processor.h"
 #include "media/filters/source_buffer_stream.h"
 
 namespace media {
 
 enum {
   // Limits the number of MEDIA_LOG() calls warning the user that a muxed stream
   // media segment is missing a block from at least one of the audio or video
   // tracks.
   kMaxMissingTrackInSegmentLogs = 10,
 };
 
-static TimeDelta EndTimestamp(const StreamParser::BufferQueue& queue) {
+namespace {
+
+TimeDelta EndTimestamp(const StreamParser::BufferQueue& queue) {
   return queue.back()->timestamp() + queue.back()->duration();
 }
 
+const char* ToStr(MediaTrack::Type type) {

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

nit: useful elsewhere? If so, move to some method in media_track.cc please.

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

Done.

+  switch (type) {
+    case MediaTrack::Audio:
+      return "audio";
+    case MediaTrack::Text:
+      return "text";
+    case MediaTrack::Video:
+      return "video";
+  }
+  NOTREACHED();
+  return "INVALID";
+}
+
+}  // namespace
+
 // List of time ranges for each SourceBuffer.
 // static
 Ranges<TimeDelta> MediaSourceState::ComputeRangesIntersection(
     const RangesList& active_ranges,
     bool ended) {
   // TODO(servolk): Perhaps this can be removed in favor of blink implementation
   // (MediaSource::buffered)? Currently this is only used on Android and for
   // updating DemuxerHost's buffered ranges during AppendData() as well as
   // SourceBuffer.buffered property implemetation.
   // Implementation of HTMLMediaElement.buffered algorithm in MSE spec.
@@ skipping 46 matching lines @@
 }
 
 MediaSourceState::MediaSourceState(
     std::unique_ptr<StreamParser> stream_parser,
     std::unique_ptr<FrameProcessor> frame_processor,
     const CreateDemuxerStreamCB& create_demuxer_stream_cb,
     const scoped_refptr<MediaLog>& media_log)
     : create_demuxer_stream_cb_(create_demuxer_stream_cb),
       timestamp_offset_during_append_(NULL),
       parsing_media_segment_(false),
-      media_segment_contained_audio_frame_(false),
-      media_segment_contained_video_frame_(false),
       stream_parser_(stream_parser.release()),
-      audio_(NULL),
-      video_(NULL),
       frame_processor_(frame_processor.release()),
       media_log_(media_log),
       state_(UNINITIALIZED),
       auto_update_timestamp_offset_(false) {
   DCHECK(!create_demuxer_stream_cb_.is_null());
   DCHECK(frame_processor_);
 }
 
 MediaSourceState::~MediaSourceState() {
   Shutdown();
 
   base::STLDeleteValues(&text_stream_map_);
 }
 
 void MediaSourceState::Init(
     const StreamParser::InitCB& init_cb,
-    bool allow_audio,
-    bool allow_video,
+    const std::string& expected_codecs,
     const StreamParser::EncryptedMediaInitDataCB& encrypted_media_init_data_cb,
     const NewTextTrackCB& new_text_track_cb) {
   DCHECK_EQ(state_, UNINITIALIZED);
   new_text_track_cb_ = new_text_track_cb;
   init_cb_ = init_cb;
 
   state_ = PENDING_PARSER_CONFIG;
   stream_parser_->Init(
       base::Bind(&MediaSourceState::OnSourceInitDone, base::Unretained(this)),
       base::Bind(&MediaSourceState::OnNewConfigs, base::Unretained(this),
-                 allow_audio, allow_video),
+                 expected_codecs),
       base::Bind(&MediaSourceState::OnNewBuffers, base::Unretained(this)),
       new_text_track_cb_.is_null(), encrypted_media_init_data_cb,
       base::Bind(&MediaSourceState::OnNewMediaSegment, base::Unretained(this)),
       base::Bind(&MediaSourceState::OnEndOfMediaSegment,
                  base::Unretained(this)),
       media_log_);
 }
 
 void MediaSourceState::SetSequenceMode(bool sequence_mode) {
   DCHECK(!parsing_media_segment_);
@@ skipping 48 matching lines @@
   DCHECK(!timestamp_offset_during_append_);
   timestamp_offset_during_append_ = timestamp_offset;
   append_window_start_during_append_ = append_window_start;
   append_window_end_during_append_ = append_window_end;
 
   stream_parser_->Flush();
   timestamp_offset_during_append_ = NULL;
 
   frame_processor_->Reset();
   parsing_media_segment_ = false;
-  media_segment_contained_audio_frame_ = false;
-  media_segment_contained_video_frame_ = false;
+  media_segment_has_data_for_track_.clear();
 }
 
 void MediaSourceState::Remove(TimeDelta start,
                               TimeDelta end,
                               TimeDelta duration) {
-  if (audio_)
-    audio_->Remove(start, end, duration);
+  for (const auto& it : audio_streams_) {
+    it.second->Remove(start, end, duration);
+  }
 
-  if (video_)
-    video_->Remove(start, end, duration);
+  for (const auto& it : video_streams_) {
+    it.second->Remove(start, end, duration);
+  }
 
   for (TextStreamMap::iterator itr = text_stream_map_.begin();
        itr != text_stream_map_.end(); ++itr) {
     itr->second->Remove(start, end, duration);
   }
 }
 
-size_t MediaSourceState::EstimateVideoDataSize(
-    size_t muxed_data_chunk_size) const {
-  DCHECK(audio_);
-  DCHECK(video_);
-
-  size_t videoBufferedSize = video_->GetBufferedSize();
-  size_t audioBufferedSize = audio_->GetBufferedSize();
-  if (videoBufferedSize == 0 || audioBufferedSize == 0) {
-    // At this point either audio or video buffer is empty, which means buffer
-    // levels are probably low anyway and we should have enough space in the
-    // buffers for appending new data, so just take a very rough guess.
-    return muxed_data_chunk_size * 7 / 8;
-  }
-
-  // We need to estimate how much audio and video data is going to be in the
-  // newly appended data chunk to make space for the new data. And we need to do
-  // that without parsing the data (which will happen later, in the Append
-  // phase). So for now we can only rely on some heuristic here. Let's assume
-  // that the proportion of the audio/video in the new data chunk is the same as
-  // the current ratio of buffered audio/video.
-  // Longer term this should go away once we further change the MSE GC algorithm
-  // to work across all streams of a SourceBuffer (see crbug.com/520704).
-  double videoBufferedSizeF = static_cast<double>(videoBufferedSize);
-  double audioBufferedSizeF = static_cast<double>(audioBufferedSize);
-
-  double totalBufferedSizeF = videoBufferedSizeF + audioBufferedSizeF;
-  CHECK_GT(totalBufferedSizeF, 0.0);
-
-  double videoRatio = videoBufferedSizeF / totalBufferedSizeF;
-  CHECK_GE(videoRatio, 0.0);
-  CHECK_LE(videoRatio, 1.0);
-  double estimatedVideoSize = muxed_data_chunk_size * videoRatio;
-  return static_cast<size_t>(estimatedVideoSize);
-}
-
 bool MediaSourceState::EvictCodedFrames(DecodeTimestamp media_time,
                                         size_t newDataSize) {
   bool success = true;
 
   DVLOG(3) << __func__ << " media_time=" << media_time.InSecondsF()
-           << " newDataSize=" << newDataSize
-           << " videoBufferedSize=" << (video_ ? video_->GetBufferedSize() : 0)
-           << " audioBufferedSize=" << (audio_ ? audio_->GetBufferedSize() : 0);
+           << " newDataSize=" << newDataSize;
 
-  size_t newAudioSize = 0;
-  size_t newVideoSize = 0;
-  if (audio_ && video_) {
-    newVideoSize = EstimateVideoDataSize(newDataSize);
-    newAudioSize = newDataSize - newVideoSize;
-  } else if (video_) {
-    newVideoSize = newDataSize;
-  } else if (audio_) {
-    newAudioSize = newDataSize;
+  DVLOG(4) << "Before EvictCodedFrames:";
+  for (const auto& it : audio_streams_) {
+    DVLOG(4) << "Audio track_id=" << it.second->media_track_id()
+             << " buffered_size=" << it.second->GetBufferedSize();
+  }
+  for (const auto& it : video_streams_) {
+    DVLOG(4) << "Video track_id=" << it.second->media_track_id()
+             << " buffered_size=" << it.second->GetBufferedSize();
   }
 
-  DVLOG(3) << __func__
-           << " estimated audio/video sizes: newVideoSize=" << newVideoSize
-           << " newAudioSize=" << newAudioSize;
+  size_t estimatedAudioSize = newDataSize;
+  size_t estimatedVideoSize = newDataSize;
+  if (!audio_streams_.empty() && !video_streams_.empty()) {
+    estimatedAudioSize = newDataSize / 16;

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

nit: Assumption is overall, across all A+V tracks in this SB, 1/16 of the size
of new data is audio (across all audio tracks). If one video track + 10 audio
tracks, though, this breaks down rapidly. Can the assumption be more like: each
audio track gets factor 1, each video segment gets factor 15. Sum the factors
for denominator and numerator is per-stream factor to get per-stream multiplier
of newDataSize for estimate. s/15/7/ or any other reasonable 1 track video : 1
track audio proportion.
Also, this change now ignores any already-buffered proportion of audio/video,
though. Probably fine. Hence just nit.
Regardless, would be good to document the reasons for particular estimation
choices here.

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

Yeah, after pondering this a bit more, I think we can simplify this a bit. I
have just realized that we can always use the proportion of the
stream->GetBufferedSize to the current total buffered size for estimating how
much data actually belongs to a given stream in the new data chunk. We don't
need to handle cases when total buffered size is 0 or the current stream
buffered size is 0, since in those cases there's no point in even trying to
evict codec frames - there's nothing to evict. With this change I've also
restored the previously removed part of the EvictCodedFrames test.

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

Acknowledged.

+    estimatedVideoSize = newDataSize - estimatedAudioSize;
+  }
+  if (audio_streams_.size() > 0)
+    estimatedAudioSize /= audio_streams_.size();
+  if (video_streams_.size() > 0)
+    estimatedVideoSize /= video_streams_.size();
 
-  if (audio_)
-    success = audio_->EvictCodedFrames(media_time, newAudioSize) && success;
-
-  if (video_)
-    success = video_->EvictCodedFrames(media_time, newVideoSize) && success;
+  for (const auto& it : audio_streams_) {
+    success &= it.second->EvictCodedFrames(media_time, estimatedAudioSize);
+  }
+  for (const auto& it : video_streams_) {
+    success &= it.second->EvictCodedFrames(media_time, estimatedVideoSize);
+  }
 
   for (TextStreamMap::iterator itr = text_stream_map_.begin();
        itr != text_stream_map_.end(); ++itr) {
-    success = itr->second->EvictCodedFrames(media_time, 0) && success;
+    success &= itr->second->EvictCodedFrames(media_time, 0);
   }
 
-  DVLOG(3) << __func__ << " result=" << success
-           << " videoBufferedSize=" << (video_ ? video_->GetBufferedSize() : 0)
-           << " audioBufferedSize=" << (audio_ ? audio_->GetBufferedSize() : 0);
+  DVLOG(4) << "After EvictCodedFrames (success=" << success << "):";
+  for (const auto& it : audio_streams_) {
+    DVLOG(4) << "Audio track_id=" << it.second->media_track_id()
+             << " buffered_size=" << it.second->GetBufferedSize();
+  }
+  for (const auto& it : video_streams_) {
+    DVLOG(4) << "Video track_id=" << it.second->media_track_id()
+             << " buffered_size=" << it.second->GetBufferedSize();
+  }
 
   return success;
 }
 
 Ranges<TimeDelta> MediaSourceState::GetBufferedRanges(TimeDelta duration,

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

hmm. I think that TODO was wrong. SourceBuffer.buffered spec:
For each track buffer managed by this SourceBuffer, run the following steps:... 

Note that it isn't scoped to just the currently enabled/selected.

Note that the extended-for-MSE HTMLME.buffered *is* scoped to just the SB in
activeSourceBuffers.

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

Ok, in that case we can just drop the check for enabled() and remove the TODO.

                                                       bool ended) const {
-  // TODO(acolwell): When we start allowing disabled tracks we'll need to update
-  // this code to only add ranges from active tracks.
   RangesList ranges_list;
-  if (audio_)
-    ranges_list.push_back(audio_->GetBufferedRanges(duration));
+  for (const auto& it : audio_streams_) {
+    if (it.second->enabled())
+      ranges_list.push_back(it.second->GetBufferedRanges(duration));
+  }
 
-  if (video_)
-    ranges_list.push_back(video_->GetBufferedRanges(duration));
+  for (const auto& it : video_streams_) {
+    if (it.second->enabled())
+      ranges_list.push_back(it.second->GetBufferedRanges(duration));
+  }
 
   for (TextStreamMap::const_iterator itr = text_stream_map_.begin();
        itr != text_stream_map_.end(); ++itr) {
     ranges_list.push_back(itr->second->GetBufferedRanges(duration));
   }
 
   return ComputeRangesIntersection(ranges_list, ended);
 }
 
 TimeDelta MediaSourceState::GetHighestPresentationTimestamp() const {
   TimeDelta max_pts;
 
-  if (audio_)
-    max_pts = std::max(max_pts, audio_->GetHighestPresentationTimestamp());
+  for (const auto& it : audio_streams_) {
+    max_pts = std::max(max_pts, it.second->GetHighestPresentationTimestamp());
+  }
 
-  if (video_)
-    max_pts = std::max(max_pts, video_->GetHighestPresentationTimestamp());
+  for (const auto& it : video_streams_) {
+    max_pts = std::max(max_pts, it.second->GetHighestPresentationTimestamp());
+  }
 
   for (TextStreamMap::const_iterator itr = text_stream_map_.begin();
        itr != text_stream_map_.end(); ++itr) {
     max_pts = std::max(max_pts, itr->second->GetHighestPresentationTimestamp());
   }
 
   return max_pts;
 }
 
 TimeDelta MediaSourceState::GetMaxBufferedDuration() const {
   TimeDelta max_duration;
 
-  if (audio_)
-    max_duration = std::max(max_duration, audio_->GetBufferedDuration());
+  for (const auto& it : audio_streams_) {
+    max_duration = std::max(max_duration, it.second->GetBufferedDuration());
+  }
 
-  if (video_)
-    max_duration = std::max(max_duration, video_->GetBufferedDuration());
+  for (const auto& it : video_streams_) {
+    max_duration = std::max(max_duration, it.second->GetBufferedDuration());
+  }
 
   for (TextStreamMap::const_iterator itr = text_stream_map_.begin();
        itr != text_stream_map_.end(); ++itr) {
     max_duration = std::max(max_duration, itr->second->GetBufferedDuration());
   }
 
   return max_duration;
 }
 
 void MediaSourceState::StartReturningData() {
-  if (audio_)
-    audio_->StartReturningData();
+  for (const auto& it : audio_streams_) {
+    it.second->StartReturningData();
+  }
 
-  if (video_)
-    video_->StartReturningData();
+  for (const auto& it : video_streams_) {
+    it.second->StartReturningData();
+  }
 
   for (TextStreamMap::iterator itr = text_stream_map_.begin();
        itr != text_stream_map_.end(); ++itr) {
     itr->second->StartReturningData();
   }
 }
 
 void MediaSourceState::AbortReads() {
-  if (audio_)
-    audio_->AbortReads();
+  for (const auto& it : audio_streams_) {
+    it.second->AbortReads();
+  }
 
-  if (video_)
-    video_->AbortReads();
+  for (const auto& it : video_streams_) {
+    it.second->AbortReads();
+  }
 
   for (TextStreamMap::iterator itr = text_stream_map_.begin();
        itr != text_stream_map_.end(); ++itr) {
     itr->second->AbortReads();
   }
 }
 
 void MediaSourceState::Seek(TimeDelta seek_time) {
-  if (audio_)
-    audio_->Seek(seek_time);
+  for (const auto& it : audio_streams_) {
+    it.second->Seek(seek_time);
+  }
 
-  if (video_)
-    video_->Seek(seek_time);
+  for (const auto& it : video_streams_) {
+    it.second->Seek(seek_time);
+  }
 
   for (TextStreamMap::iterator itr = text_stream_map_.begin();
        itr != text_stream_map_.end(); ++itr) {
     itr->second->Seek(seek_time);
   }
 }
 
 void MediaSourceState::CompletePendingReadIfPossible() {
-  if (audio_)
-    audio_->CompletePendingReadIfPossible();
+  for (const auto& it : audio_streams_) {
+    it.second->CompletePendingReadIfPossible();
+  }
 
-  if (video_)
-    video_->CompletePendingReadIfPossible();
+  for (const auto& it : video_streams_) {
+    it.second->CompletePendingReadIfPossible();
+  }
 
   for (TextStreamMap::iterator itr = text_stream_map_.begin();
        itr != text_stream_map_.end(); ++itr) {
     itr->second->CompletePendingReadIfPossible();
   }
 }
 
 void MediaSourceState::OnSetDuration(TimeDelta duration) {
-  if (audio_)
-    audio_->OnSetDuration(duration);
+  for (const auto& it : audio_streams_) {
+    it.second->OnSetDuration(duration);
+  }
 
-  if (video_)
-    video_->OnSetDuration(duration);
+  for (const auto& it : video_streams_) {
+    it.second->OnSetDuration(duration);
+  }
 
   for (TextStreamMap::iterator itr = text_stream_map_.begin();
        itr != text_stream_map_.end(); ++itr) {
     itr->second->OnSetDuration(duration);
   }
 }
 
 void MediaSourceState::MarkEndOfStream() {
-  if (audio_)
-    audio_->MarkEndOfStream();
+  for (const auto& it : audio_streams_) {
+    it.second->MarkEndOfStream();
+  }
 
-  if (video_)
-    video_->MarkEndOfStream();
+  for (const auto& it : video_streams_) {
+    it.second->MarkEndOfStream();
+  }
 
   for (TextStreamMap::iterator itr = text_stream_map_.begin();
        itr != text_stream_map_.end(); ++itr) {
     itr->second->MarkEndOfStream();
   }
 }
 
 void MediaSourceState::UnmarkEndOfStream() {
-  if (audio_)
-    audio_->UnmarkEndOfStream();
+  for (const auto& it : audio_streams_) {
+    it.second->UnmarkEndOfStream();
+  }
 
-  if (video_)
-    video_->UnmarkEndOfStream();
+  for (const auto& it : video_streams_) {
+    it.second->UnmarkEndOfStream();
+  }
 
   for (TextStreamMap::iterator itr = text_stream_map_.begin();
        itr != text_stream_map_.end(); ++itr) {
     itr->second->UnmarkEndOfStream();
   }
 }
 
 void MediaSourceState::Shutdown() {
-  if (audio_)
-    audio_->Shutdown();
+  for (const auto& it : audio_streams_) {
+    it.second->Shutdown();
+  }
 
-  if (video_)
-    video_->Shutdown();
+  for (const auto& it : video_streams_) {
+    it.second->Shutdown();
+  }
 
   for (TextStreamMap::iterator itr = text_stream_map_.begin();
        itr != text_stream_map_.end(); ++itr) {
     itr->second->Shutdown();
   }
 }
 
 void MediaSourceState::SetMemoryLimits(DemuxerStream::Type type,

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

nit:ForTest here, in ChunkDemuxer, and in Pipeline? Or are there uses of this
method outside of tests still (even with the cmdline flags now available)?

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

Well, because of the cmdline flags I think we shouldn't call these ForTest.
Since  flags can be added anywhere, not necessarily in tests.

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

Unless I'm missing something new in this CL, or cs.chromium.org isn't showing
all call hierarchies, the latter shows only unit tests exercise this method. Is
that different perhaps only in downstream repos?

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

Ah, ok, you are right, I've got confused, thought we were talking about
MSS::SetLimits. Renamed.

                                        size_t memory_limit) {
   switch (type) {
     case DemuxerStream::AUDIO:
-      if (audio_)
-        audio_->SetStreamMemoryLimit(memory_limit);
+      for (const auto& it : audio_streams_) {
+        it.second->SetStreamMemoryLimit(memory_limit);
+      }
       break;
     case DemuxerStream::VIDEO:
-      if (video_)
-        video_->SetStreamMemoryLimit(memory_limit);
+      for (const auto& it : video_streams_) {
+        it.second->SetStreamMemoryLimit(memory_limit);
+      }
       break;
     case DemuxerStream::TEXT:
       for (TextStreamMap::iterator itr = text_stream_map_.begin();
            itr != text_stream_map_.end(); ++itr) {
         itr->second->SetStreamMemoryLimit(memory_limit);
       }
       break;
     case DemuxerStream::UNKNOWN:
     case DemuxerStream::NUM_TYPES:
       NOTREACHED();
       break;
   }
 }
 
 bool MediaSourceState::IsSeekWaitingForData() const {
-  if (audio_ && audio_->IsSeekWaitingForData())
-    return true;
+  for (const auto& it : audio_streams_) {
+    if (it.second->IsSeekWaitingForData())
+      return true;
+  }
 
-  if (video_ && video_->IsSeekWaitingForData())
-    return true;
+  for (const auto& it : video_streams_) {
+    if (it.second->IsSeekWaitingForData())
+      return true;
+  }
 
   // NOTE: We are intentionally not checking the text tracks
   // because text tracks are discontinuous and may not have data
   // for the seek position. This is ok and playback should not be
   // stalled because we don't have cues. If cues, with timestamps after
   // the seek time, eventually arrive they will be delivered properly
   // in response to ChunkDemuxerStream::Read() calls.
 
   return false;
 }
 
+bool CheckBytestreamTrackIds(

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

move to anon namespace at top of file

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

Done.

+    const MediaTracks& tracks,
+    const StreamParser::TextTrackConfigMap& text_configs) {
+  std::set<StreamParser::TrackId> bytestream_ids;
+  for (const auto& track : tracks.tracks()) {
+    const StreamParser::TrackId& track_id = track->bytestream_track_id();
+    if (bytestream_ids.find(track_id) != bytestream_ids.end()) {
+      return false;
+    }
+    bytestream_ids.insert(track_id);
+  }
+  for (const auto& text_track : text_configs) {
+    const StreamParser::TrackId& track_id = text_track.first;
+    if (bytestream_ids.find(track_id) != bytestream_ids.end()) {
+      return false;
+    }
+    bytestream_ids.insert(track_id);
+  }
+  return true;
+}
+
 bool MediaSourceState::OnNewConfigs(
-    bool allow_audio,
-    bool allow_video,
+    std::string expected_codecs,
     std::unique_ptr<MediaTracks> tracks,
     const StreamParser::TextTrackConfigMap& text_configs) {
+  DCHECK(tracks.get());
+  DVLOG(1) << __func__ << " expected_codecs=" << expected_codecs
+           << " tracks=" << tracks->tracks().size();
   DCHECK_GE(state_, PENDING_PARSER_CONFIG);
-  DCHECK(tracks.get());
-
-  MediaTrack* audio_track = nullptr;
-  MediaTrack* video_track = nullptr;
-  AudioDecoderConfig audio_config;
-  VideoDecoderConfig video_config;
+
+  // Check that there is no clashing bytestream track ids.
+  if (!CheckBytestreamTrackIds(*tracks, text_configs)) {
+    MEDIA_LOG(ERROR, media_log_)
+        << "Error: duplicate bytestream track ids detected";

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

nit: s/Error: duplicate/Duplicate/  (ERROR already populates as error, and if it
doesn't that's a distinct bug not to fix by putting it in this message here.)

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

Done.

+    for (const auto& track : tracks->tracks()) {
+      const StreamParser::TrackId& track_id = track->bytestream_track_id();
+      MEDIA_LOG(ERROR, media_log_) << ToStr(track->type()) << " track "

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

nit:s/ERROR/DEBUG/ so we can expose the last cached ERROR message eventually to
devtools with at least some meaning still. Keep the ERROR type of log on l.545.

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

Done.

+                                   << " bytestream track id=" << track_id;
+    }
+    return false;
+  }
+
+  // MSE spec allows new configs to be emitted only during Append, but not
+  // during Flush or parser reset operations.
+  CHECK(append_in_progress_);
+
+  bool success = true;
+
+  std::vector<std::string> expected_codecs_parsed;
+  ParseCodecString(expected_codecs, &expected_codecs_parsed, false);

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

nit: Every init segment, we parse these? Can we instead parse once (including
splitting out the codecs expectations by type, and validating none are
kUnknown{Audio,Video}Codec when creating the StreamParser and curry the
expectation there? Perhaps the cost difference is negligible, since we'd need to
deepcopy the expectations (or maintain a map of "codecs found in this init
segment") below, so doing the parse here is potentially more maintainable and
not really perf-regressing. WDYT?

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

Yeah, I don't think it's going to make a big difference, but it's easy enough to
do, so why not?

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

Acknowledged.

+
+  std::vector<AudioCodec> expected_acodecs;
+  std::vector<VideoCodec> expected_vcodecs;
+  for (const auto& codec_id : expected_codecs_parsed) {
+    AudioCodec acodec = StringToAudioCodec(codec_id);
+    if (acodec != kUnknownAudioCodec) {
+      expected_acodecs.push_back(acodec);
+      continue;
+    }
+    VideoCodec vcodec = StringToVideoCodec(codec_id);
+    if (vcodec != kUnknownVideoCodec) {
+      expected_vcodecs.push_back(vcodec);
+      continue;
+    }
+    MEDIA_LOG(INFO, media_log_) << "Unrecognized media codec: " << codec_id;
+  }
+
   for (const auto& track : tracks->tracks()) {
     const auto& track_id = track->bytestream_track_id();
 
     if (track->type() == MediaTrack::Audio) {
-      if (audio_track) {
-        MEDIA_LOG(ERROR, media_log_)
-            << "Error: more than one audio track is currently not supported.";
+      AudioDecoderConfig audio_config = tracks->getAudioConfig(track_id);
+      DVLOG(1) << "Audio track_id=" << track_id
+               << " config: " << audio_config.AsHumanReadableString();
+      DCHECK(audio_config.IsValidConfig());
+
+      const auto& it = std::find(expected_acodecs.begin(),
+                                 expected_acodecs.end(), audio_config.codec());
+      if (it == expected_acodecs.end()) {
+        MEDIA_LOG(ERROR, media_log_) << "Audio stream codec "
+                                     << GetCodecName(audio_config.codec())
+                                     << " doesn't match SourceBuffer codecs.";
         return false;
       }
-      audio_track = track.get();
-      audio_config = tracks->getAudioConfig(track_id);
-      DCHECK(audio_config.IsValidConfig());
+      expected_acodecs.erase(it);

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

This is a problem for the following test I suggest adding:

addId(x, "audio/webm" "vorbis")
appendInitSegment(x, two vorbis tracks)

That's a valid init segment, but the second audio track would cause parse/decode
error in l.591-596, above, with this current patch set.

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

Wait, shouldn't the mime type be 'audio/webm; codecs=vorbis,vorbis' if there are
actually two vorbis tracks? I tried creating DASH manifests from multi-track mp4
files using GPAC MP4Box and I got manifests that specified the same codec
multiple times i.e. it looked like codecs="avc1.4D401F,avc1.4D401F". So I
assumed the codecs parameter must have one value for each stream/track, and in
fact I was wondering if we need to enforce also the order of those
tracks/streams to match the codecs parameter (since that could simplify the
logic here).
I don't see this covered explicitly in the MSE spec. It simply says that the
parameter of the addSourceBuffer is a mime type string, and does't go into
details about how the codecs parameter should be interpreted. Can you please
clarify this in the MSE spec?

Also, unfortunately we can't create a test like you described yet, because our
WebM parser currently doesn't support multiple audio/video tracks, it still has
that 1 a + 1 v track limitation. And MP4 parser, which supports multiple tracks,
cannot be used in tests due to MP4 using proprietary codecs. I'll leave a TODO
to add such a test in the future, when WebM parser support multiple tracks.

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

I've filed spec bug (MSE vNext) https://github.com/w3c/media-source/issues/161
to get clarity. Unfortunately other UAs don't always require codec strings and
may instead sniff them out during initialization segment received processing;
isTypeSupported('video/mp4') on Edge, for instance, is true and
addSourceBUffer() of same might work (I'm unsure there). Following EME's route
of strict matching, Chrome MSE is more strict, and MSE vNext bug
(https://github.com/w3c/media-source/issues/137) tracks making the spec more
strict too. However, such strictness (#137) does not necessarily imply that
multiple tracks of same codec type each have distinct entries in the codec
string. I propose we start strict here (like you have currently), and only relax
as necessary if web authors discover issue here only in Chrome while #161
remains open in the spec.

Please add a "TODO(wolenetz): Update codec string strictness, if necessary, once
spec issue https://github.com/w3c/media-source/issues/161 is resolved."

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

Done.

+
+      ChunkDemuxerStream* stream = nullptr;
+      if (!first_init_segment_received_) {
+        DCHECK(audio_streams_.find(track_id) == audio_streams_.end());
+        stream = create_demuxer_stream_cb_.Run(DemuxerStream::AUDIO);
+        if (!stream || !frame_processor_->AddTrack(track_id, stream)) {
+          MEDIA_LOG(ERROR, media_log_) << "Failed to create audio stream.";
+          return false;
+        }
+        audio_streams_[track_id] = stream;
+        media_log_->SetBooleanProperty("found_audio_stream", true);
+        media_log_->SetStringProperty("audio_codec_name",
+                                      GetCodecName(audio_config.codec()));
+      } else {
+        if (audio_streams_.size() > 1) {
+          stream = audio_streams_[track_id];
+        } else {
+          // If there is only one video track then bytestream id might change in

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

nit:s/video/audio/

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

Done.

+          // a new init segment. So update our state and nofity frame processor.
+          const auto& it = audio_streams_.begin();
+          if (it != audio_streams_.end()) {
+            stream = it->second;
+            if (it->first != track_id) {
+              frame_processor_->UpdateTrack(it->first, track_id);
+              audio_streams_[track_id] = stream;
+              audio_streams_.erase(it->first);
+            }
+          }
+        }
+        if (!stream) {
+          MEDIA_LOG(ERROR, media_log_) << "Got unexpected audio track"
+                                       << " track_id=" << track_id;
+          return false;
+        }
+      }
+
+      track->set_id(stream->media_track_id());
+      frame_processor_->OnPossibleAudioConfigUpdate(audio_config);
+      success &= stream->UpdateAudioConfig(audio_config, media_log_);
     } else if (track->type() == MediaTrack::Video) {
-      if (video_track) {
-        MEDIA_LOG(ERROR, media_log_)
-            << "Error: more than one video track is currently not supported.";
+      VideoDecoderConfig video_config = tracks->getVideoConfig(track_id);
+      DVLOG(1) << "Video track_id=" << track_id
+               << " config: " << video_config.AsHumanReadableString();
+      DCHECK(video_config.IsValidConfig());
+
+      const auto& it = std::find(expected_vcodecs.begin(),
+                                 expected_vcodecs.end(), video_config.codec());
+      if (it == expected_vcodecs.end()) {
+        MEDIA_LOG(ERROR, media_log_) << "Video stream codec "
+                                     << GetCodecName(video_config.codec())
+                                     << " doesn't match SourceBuffer codecs.";
         return false;
       }
-      video_track = track.get();
-      video_config = tracks->getVideoConfig(track_id);
-      DCHECK(video_config.IsValidConfig());
+      expected_vcodecs.erase(it);

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

ditto missing test and problem for multiple video tracks of same codec in same
SourceBuffer.

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

Acknowledged.

+
+      ChunkDemuxerStream* stream = nullptr;
+      if (!first_init_segment_received_) {
+        DCHECK(video_streams_.find(track_id) == video_streams_.end());
+        stream = create_demuxer_stream_cb_.Run(DemuxerStream::VIDEO);
+        if (!stream || !frame_processor_->AddTrack(track_id, stream)) {
+          MEDIA_LOG(ERROR, media_log_) << "Failed to create video stream.";
+          return false;
+        }
+        video_streams_[track_id] = stream;
+        media_log_->SetBooleanProperty("found_video_stream", true);
+        media_log_->SetStringProperty("video_codec_name",
+                                      GetCodecName(video_config.codec()));
+      } else {
+        if (video_streams_.size() > 1) {
+          stream = video_streams_[track_id];
+        } else {
+          // If there is only one video track then bytestream id might change in
+          // a new init segment. So update our state and nofity frame processor.
+          const auto& it = video_streams_.begin();
+          if (it != video_streams_.end()) {
+            stream = it->second;
+            if (it->first != track_id) {
+              frame_processor_->UpdateTrack(it->first, track_id);
+              video_streams_[track_id] = stream;
+              video_streams_.erase(it->first);
+            }
+          }
+        }
+        if (!stream) {
+          MEDIA_LOG(ERROR, media_log_) << "Got unexpected video track"
+                                       << " track_id=" << track_id;
+          return false;
+        }
+      }
+
+      track->set_id(stream->media_track_id());
+      success &= stream->UpdateVideoConfig(video_config, media_log_);
     } else {
       MEDIA_LOG(ERROR, media_log_) << "Error: unsupported media track type "
                                    << track->type();
       return false;
     }
   }
 
-  DVLOG(1) << "OnNewConfigs(" << allow_audio << ", " << allow_video << ", "
-           << audio_config.IsValidConfig() << ", "
-           << video_config.IsValidConfig() << ")";
-  // MSE spec allows new configs to be emitted only during Append, but not
-  // during Flush or parser reset operations.
-  CHECK(append_in_progress_);
-
-  if (!audio_config.IsValidConfig() && !video_config.IsValidConfig()) {
-    DVLOG(1) << "OnNewConfigs() : Audio & video config are not valid!";
+  if (!expected_acodecs.empty() || !expected_vcodecs.empty()) {
+    for (const auto& acodec : expected_acodecs) {
+      MEDIA_LOG(ERROR, media_log_) << "Initialization segment misses expected "
+                                   << GetCodecName(acodec) << " track.";
+    }
+    for (const auto& vcodec : expected_vcodecs) {
+      MEDIA_LOG(ERROR, media_log_) << "Initialization segment misses expected "
+                                   << GetCodecName(vcodec) << " track.";
+    }
     return false;
   }
 
-  // Signal an error if we get configuration info for stream types that weren't
-  // specified in AddId() or more configs after a stream is initialized.
-  if (allow_audio != audio_config.IsValidConfig()) {
-    MEDIA_LOG(ERROR, media_log_)
-        << "Initialization segment"
-        << (audio_config.IsValidConfig() ? " has" : " does not have")
-        << " an audio track, but the mimetype"
-        << (allow_audio ? " specifies" : " does not specify")
-        << " an audio codec.";
-    return false;
-  }
-
-  if (allow_video != video_config.IsValidConfig()) {
-    MEDIA_LOG(ERROR, media_log_)
-        << "Initialization segment"
-        << (video_config.IsValidConfig() ? " has" : " does not have")
-        << " a video track, but the mimetype"
-        << (allow_video ? " specifies" : " does not specify")
-        << " a video codec.";
-    return false;
-  }
-
-  bool success = true;
-  if (audio_config.IsValidConfig()) {
-    if (!audio_) {
-      media_log_->SetBooleanProperty("found_audio_stream", true);
-    }
-    if (!audio_ ||
-        audio_->audio_decoder_config().codec() != audio_config.codec()) {
-      media_log_->SetStringProperty("audio_codec_name",
-                                    GetCodecName(audio_config.codec()));
-    }
-
-    bool audio_stream_just_created = false;
-    if (!audio_) {
-      audio_ = create_demuxer_stream_cb_.Run(DemuxerStream::AUDIO);
-
-      if (!audio_) {
-        DVLOG(1) << "Failed to create an audio stream.";
-        return false;
-      }
-      audio_stream_just_created = true;
-
-      if (!frame_processor_->AddTrack(FrameProcessor::kAudioTrackId, audio_)) {
-        DVLOG(1) << "Failed to add audio track to frame processor.";
-        return false;
-      }
-    }
-
-    frame_processor_->OnPossibleAudioConfigUpdate(audio_config);
-    success &= audio_->UpdateAudioConfig(audio_config, media_log_);
-
-    if (audio_stream_just_created) {
-      std::string audio_buf_limit_switch =
-          base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-              switches::kMSEAudioBufferSizeLimit);
-      unsigned audio_buf_size_limit = 0;
-      if (base::StringToUint(audio_buf_limit_switch, &audio_buf_size_limit) &&
-          audio_buf_size_limit > 0) {
-        MEDIA_LOG(INFO, media_log_) << "Custom audio SourceBuffer size limit="
-                                    << audio_buf_size_limit;
-        audio_->SetStreamMemoryLimit(audio_buf_size_limit);
-      }
-    }
-  }
-
-  if (video_config.IsValidConfig()) {
-    if (!video_) {
-      media_log_->SetBooleanProperty("found_video_stream", true);
-    }
-    if (!video_ ||
-        video_->video_decoder_config().codec() != video_config.codec()) {
-      media_log_->SetStringProperty("video_codec_name",
-                                    GetCodecName(video_config.codec()));
-    }
-
-    bool video_stream_just_created = false;
-    if (!video_) {
-      video_ = create_demuxer_stream_cb_.Run(DemuxerStream::VIDEO);
-
-      if (!video_) {
-        DVLOG(1) << "Failed to create a video stream.";
-        return false;
-      }
-      video_stream_just_created = true;
-
-      if (!frame_processor_->AddTrack(FrameProcessor::kVideoTrackId, video_)) {
-        DVLOG(1) << "Failed to add video track to frame processor.";
-        return false;
-      }
-    }
-
-    success &= video_->UpdateVideoConfig(video_config, media_log_);
-
-    if (video_stream_just_created) {
-      std::string video_buf_limit_switch =
-          base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-              switches::kMSEVideoBufferSizeLimit);
-      unsigned video_buf_size_limit = 0;
-      if (base::StringToUint(video_buf_limit_switch, &video_buf_size_limit) &&
-          video_buf_size_limit > 0) {
-        MEDIA_LOG(INFO, media_log_) << "Custom video SourceBuffer size limit="
-                                    << video_buf_size_limit;
-        video_->SetStreamMemoryLimit(video_buf_size_limit);
-      }
-    }
-  }
-
   typedef StreamParser::TextTrackConfigMap::const_iterator TextConfigItr;
   if (text_stream_map_.empty()) {
     for (TextConfigItr itr = text_configs.begin(); itr != text_configs.end();
          ++itr) {
       ChunkDemuxerStream* const text_stream =
           create_demuxer_stream_cb_.Run(DemuxerStream::TEXT);
       if (!frame_processor_->AddTrack(itr->first, text_stream)) {
         success &= false;
         MEDIA_LOG(ERROR, media_log_) << "Failed to add text track ID "
                                      << itr->first << " to frame processor.";
@@ skipping 55 matching lines @@
           success &= false;
           MEDIA_LOG(ERROR, media_log_) << "New text track config for track ID "
                                        << config_itr->first
                                        << " does not match old one.";
           break;
         }
       }
     }
   }
 
+  if (audio_streams_.empty() && video_streams_.empty()) {
+    DVLOG(1) << __func__ << ": couldn't find a valid audio or video stream";
+    return false;
+  }
+
   frame_processor_->SetAllTrackBuffersNeedRandomAccessPoint();
 
-  if (audio_track) {
-    DCHECK(audio_);
-    audio_track->set_id(audio_->media_track_id());
-  }
-  if (video_track) {
-    DCHECK(video_);
-    video_track->set_id(video_->media_track_id());
+  if (!first_init_segment_received_) {
+    first_init_segment_received_ = true;
+    SetStreamMemoryLimits();
   }
 
   DVLOG(1) << "OnNewConfigs() : " << (success ? "success" : "failed");
   if (success) {
     if (state_ == PENDING_PARSER_CONFIG)
       state_ = PENDING_PARSER_INIT;
     DCHECK(!init_segment_received_cb_.is_null());
     init_segment_received_cb_.Run(std::move(tracks));
   }
 
   return success;
 }
 
+void MediaSourceState::SetStreamMemoryLimits() {
+  auto cmd_line = base::CommandLine::ForCurrentProcess();
+
+  std::string audio_buf_limit_switch =
+      cmd_line->GetSwitchValueASCII(switches::kMSEAudioBufferSizeLimit);
+  unsigned audio_buf_size_limit = 0;
+  if (base::StringToUint(audio_buf_limit_switch, &audio_buf_size_limit) &&
+      audio_buf_size_limit > 0) {
+    MEDIA_LOG(INFO, media_log_) << "Custom audio SourceBuffer size limit="

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

nit: audio *per-track* SourceBuffer size limit=....

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

Done.

+                                << audio_buf_size_limit;
+    for (const auto& it : audio_streams_) {
+      it.second->SetStreamMemoryLimit(audio_buf_size_limit);
+    }
+  }
+
+  std::string video_buf_limit_switch =
+      cmd_line->GetSwitchValueASCII(switches::kMSEVideoBufferSizeLimit);
+  unsigned video_buf_size_limit = 0;
+  if (base::StringToUint(video_buf_limit_switch, &video_buf_size_limit) &&
+      video_buf_size_limit > 0) {
+    MEDIA_LOG(INFO, media_log_) << "Custom video SourceBuffer size limit="

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

nit ditto

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

Done.

+                                << video_buf_size_limit;
+    for (const auto& it : video_streams_) {
+      it.second->SetStreamMemoryLimit(video_buf_size_limit);
+    }
+  }
+}
+
 void MediaSourceState::OnNewMediaSegment() {
   DVLOG(2) << "OnNewMediaSegment()";
   DCHECK_EQ(state_, PARSER_INITIALIZED);
   parsing_media_segment_ = true;
-  media_segment_contained_audio_frame_ = false;
-  media_segment_contained_video_frame_ = false;
+  media_segment_has_data_for_track_.clear();
 }
 
 void MediaSourceState::OnEndOfMediaSegment() {
   DVLOG(2) << "OnEndOfMediaSegment()";
   DCHECK_EQ(state_, PARSER_INITIALIZED);
   parsing_media_segment_ = false;
 
-  const bool missing_audio = audio_ && !media_segment_contained_audio_frame_;
-  const bool missing_video = video_ && !media_segment_contained_video_frame_;
-  if (!missing_audio && !missing_video)
-    return;
-
-  LIMITED_MEDIA_LOG(DEBUG, media_log_, num_missing_track_logs_,
-                    kMaxMissingTrackInSegmentLogs)
-      << "Media segment did not contain any "
-      << (missing_audio && missing_video ? "audio or video"
-                                         : missing_audio ? "audio" : "video")
-      << " coded frames, mismatching initialization segment. Therefore, MSE "
-         "coded frame processing may not interoperably detect discontinuities "
-         "in appended media.";
+  for (const auto& it : audio_streams_) {
+    if (!media_segment_has_data_for_track_[it.first]) {
+      LIMITED_MEDIA_LOG(DEBUG, media_log_, num_missing_track_logs_,
+                        kMaxMissingTrackInSegmentLogs)
+          << "Media segment did not contain any coded frames for track "
+          << it.first << ", mismatching initialization segment. Therefore, MSE"
+                         " coded frame processing may not interoperably detect"
+                         " discontinuities in appended media.";
+    }
+  }
+  for (const auto& it : video_streams_) {
+    if (!media_segment_has_data_for_track_[it.first]) {
+      LIMITED_MEDIA_LOG(DEBUG, media_log_, num_missing_track_logs_,
+                        kMaxMissingTrackInSegmentLogs)
+          << "Media segment did not contain any coded frames for track "
+          << it.first << ", mismatching initialization segment. Therefore, MSE"
+                         " coded frame processing may not interoperably detect"
+                         " discontinuities in appended media.";
+    }
+  }
 }
 
 bool MediaSourceState::OnNewBuffers(
     const StreamParser::BufferQueueMap& buffer_queue_map) {
-  DVLOG(2) << "OnNewBuffers()";
+  DVLOG(2) << __func__ << " buffer_queues=" << buffer_queue_map.size();
   DCHECK_EQ(state_, PARSER_INITIALIZED);
   DCHECK(timestamp_offset_during_append_);
   DCHECK(parsing_media_segment_);
 
   for (const auto& it : buffer_queue_map) {
     const StreamParser::BufferQueue& bufq = it.second;
     DCHECK(!bufq.empty());
-    if (bufq[0]->type() == DemuxerStream::AUDIO) {
-      media_segment_contained_audio_frame_ = true;
-    } else if (bufq[0]->type() == DemuxerStream::VIDEO) {
-      media_segment_contained_video_frame_ = true;
-    }
+    media_segment_has_data_for_track_[it.first] = true;
   }
 
   const TimeDelta timestamp_offset_before_processing =
       *timestamp_offset_during_append_;
 
   // Calculate the new timestamp offset for audio/video tracks if the stream
   // parser has requested automatic updates.
   TimeDelta new_timestamp_offset = timestamp_offset_before_processing;
   if (auto_update_timestamp_offset_) {
     TimeDelta min_end_timestamp = kNoTimestamp;
@@ skipping 17 matching lines @@
   }
 
   // Only update the timestamp offset if the frame processor hasn't already.
   if (auto_update_timestamp_offset_ &&
       timestamp_offset_before_processing == *timestamp_offset_during_append_) {
     *timestamp_offset_during_append_ = new_timestamp_offset;
   }
 
   return true;
 }
-
 void MediaSourceState::OnSourceInitDone(
     const StreamParser::InitParameters& params) {
   DCHECK_EQ(state_, PENDING_PARSER_INIT);
   state_ = PARSER_INITIALIZED;
   auto_update_timestamp_offset_ = params.auto_update_timestamp_offset;
   base::ResetAndReturn(&init_cb_).Run(params);
 }
 
 }  // namespace media