Implement mapping blink track id to demuxer streams

media/filters/chunk_demuxer.cc

Index: media/filters/chunk_demuxer.cc
diff --git a/media/filters/chunk_demuxer.cc b/media/filters/chunk_demuxer.cc
index 9b1bbd7fdaec9dae76d16d10455f03a10be56719..5e9f6795fa2ac81b00dce13d2b66c3d8cfc0408e 100644
--- a/media/filters/chunk_demuxer.cc
+++ b/media/filters/chunk_demuxer.cc
@@ -17,6 +17,7 @@
 #include "base/stl_util.h"
 #include "media/base/audio_decoder_config.h"
 #include "media/base/bind_to_current_loop.h"
+#include "media/base/media_tracks.h"
 #include "media/base/stream_parser_buffer.h"
 #include "media/base/timestamp_constants.h"
 #include "media/base/video_decoder_config.h"
@@ -1004,31 +1005,30 @@ void ChunkDemuxer::OnSourceInitDone(
 }
 
 ChunkDemuxerStream* ChunkDemuxer::CreateDemuxerStream(
-    DemuxerStream::Type type) {
-  switch (type) {
-    case DemuxerStream::AUDIO:
+    const MediaTrack& media_track) {
+  // Demuxer streams can only be created when ChunkDemuxer::AppendData is in
+  // progress.
+  lock_.AssertAcquired();
+  switch (media_track.type()) {
+    case MediaTrack::Audio:
       if (audio_)
         return NULL;
       audio_.reset(
           new ChunkDemuxerStream(DemuxerStream::AUDIO, splice_frames_enabled_));
       return audio_.get();
       break;
-    case DemuxerStream::VIDEO:
+    case MediaTrack::Video:
       if (video_)
         return NULL;
       video_.reset(
           new ChunkDemuxerStream(DemuxerStream::VIDEO, splice_frames_enabled_));
       return video_.get();
       break;
-    case DemuxerStream::TEXT: {
+    case MediaTrack::Text: {
       return new ChunkDemuxerStream(DemuxerStream::TEXT,
                                     splice_frames_enabled_);
       break;
     }
-    case DemuxerStream::UNKNOWN:
-    case DemuxerStream::NUM_TYPES:
-      NOTREACHED();
-      return NULL;
   }
   NOTREACHED();
   return NULL;
@@ -1041,6 +1041,31 @@ void ChunkDemuxer::OnNewTextTrack(ChunkDemuxerStream* text_stream,
   host_->AddTextStream(text_stream, config);
 }
 
+void ChunkDemuxer::OnTrackIdsAssigned(const MediaTracks& tracks,
+                                      const std::vector<unsigned>& track_ids) {
+  // New tracks and therefore track id assignements can happen only during
+  // ChunkDemuxer::AppendData processing, which should be holding the lock.
+  lock_.AssertAcquired();
+
+  const auto& new_track_id_map = tracks.OnTrackIdsAssigned(track_ids);

[chcunningham, 2016/06/02 21:24:45]

This is a little strange to me.

For one, calling local.On* for a local you don't own is not typical. I think
this is mostly just a case of name choice though. MediaTracks.OnTrackIdsAssigned
is a const method, purely for building this id->stream map. Could you instead
call it something like MapTrackIdsToDemuxerStreams(...)?

I also think its weird that both here and in ffmpeg demuxer, the track and
track_id args are passed in, only to have them be combined to produce the map
and never used after that. I think we could instead have the callers do the
combining and pass in the map.

I debated other designs where we let the map of DemuxerStreams to MediaTracks
instead live here in ChunkDemuxer, or perhaps MediaSourceState... it seems more
complicated that way though and I like how these maps disappear along with
MediaTracks once the final track-id -> DemuxerStream mapping is established. 

[servolk, 2016/06/02 23:53:11]

Yeah, in fact I tried maintaining both maps in demuxers, but that results in
more complicated code, so we came up with this after a few iterations of the
previous CL that did this (see https://codereview.chromium.org/1812543003). The
current approach here allows us to avoid duplicating the code for building the
maps between FFmpeg/ChunkDemuxer. And it gets rid of the media track -> demuxer
stream map as soon as it's no longer needed, which simplifies things a bit. Take
a look at the history of patchsets in the older CL
https://codereview.chromium.org/1812543003 and my discussion with Matt there.

[chcunningham, 2016/06/03 20:42:49]

I follow, but I think this only addresses my last paragraph. What about my first
two concerns?

[chcunningham, 2016/06/03 20:44:25]

My comment here may take precedence over this
https://codereview.chromium.org/1922333002/diff/220001/media/blink/webmediapl...

+
+  // ChunkDemuxer might have multiple media track sets (since it can have
+  // multiple SourceBuffers), so we need to merge the map for the current set of
+  // tracks with the global |track_id_to_demux_stream_| map shared across all
+  // SourceBuffers.
+  for (const auto& it : new_track_id_map) {
+    track_id_to_demux_stream_[it.first] = it.second;
+  }
+}
+
+const DemuxerStream* ChunkDemuxer::GetDemuxerStreamByTrackId(
+    unsigned track_id) const {
+  base::AutoLock auto_lock(lock_);
+  const auto& it = track_id_to_demux_stream_.find(track_id);
+  CHECK(it != track_id_to_demux_stream_.end());
+  return it->second;
+}
+
 bool ChunkDemuxer::IsValidId(const std::string& source_id) const {
   lock_.AssertAcquired();
   return source_state_map_.count(source_id) > 0u;