Implement mapping blink track id to demuxer streams

media/filters/chunk_demuxer.cc

 // Copyright (c) 2012 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/chunk_demuxer.h"
 
 #include <algorithm>
 #include <limits>
 #include <list>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/location.h"
 #include "base/macros.h"
 #include "base/metrics/histogram_macros.h"
 #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"
 #include "media/filters/frame_processor.h"
 #include "media/filters/stream_parser_factory.h"
 
 using base::TimeDelta;
 
 namespace media {
 
@@ skipping 967 matching lines @@
 
   if (duration_ == kNoTimestamp())
     duration_ = kInfiniteDuration();
 
   // The demuxer is now initialized after the |start_timestamp_| was set.
   ChangeState_Locked(INITIALIZED);
   base::ResetAndReturn(&init_cb_).Run(PIPELINE_OK);
 }
 
 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;
 }
 
 void ChunkDemuxer::OnNewTextTrack(ChunkDemuxerStream* text_stream,
                                   const TextTrackConfig& config) {
   lock_.AssertAcquired();
   DCHECK_NE(state_, SHUTDOWN);
   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;
 }
 
 void ChunkDemuxer::UpdateDuration(TimeDelta new_duration) {
   DCHECK(duration_ != new_duration);
   user_specified_duration_ = -1;
   duration_ = new_duration;
   host_->SetDuration(new_duration);
@@ skipping 86 matching lines @@
 }
 
 void ChunkDemuxer::ShutdownAllStreams() {
   for (MediaSourceStateMap::iterator itr = source_state_map_.begin();
        itr != source_state_map_.end(); ++itr) {
     itr->second->Shutdown();
   }
 }
 
 }  // namespace media