Implement mapping blink track id to demuxer streams

media/blink/webmediaplayer_impl.cc

Index: media/blink/webmediaplayer_impl.cc
diff --git a/media/blink/webmediaplayer_impl.cc b/media/blink/webmediaplayer_impl.cc
index 0b6354a9f296e6f22c4cd2ec20ff7793d48f53c0..3b373485a2ae011fd57fcf8bdd8e060aa1357bb7 100644
--- a/media/blink/webmediaplayer_impl.cc
+++ b/media/blink/webmediaplayer_impl.cc
@@ -577,6 +577,22 @@ bool WebMediaPlayerImpl::hasAudio() const {
   return pipeline_metadata_.has_audio;
 }
 
+void WebMediaPlayerImpl::enabledAudioTracksChanged(
+    const blink::WebVector<blink::WebMediaPlayer::TrackId>& enabledTrackIds) {
+  DCHECK(main_task_runner_->BelongsToCurrentThread());
+  CHECK(demuxer_.get());
+
+  // TODO(servolk): Implement disabling/enabling audio streams.
+}
+
+void WebMediaPlayerImpl::selectedVideoTrackChanged(
+    blink::WebMediaPlayer::TrackId* selectedTrackId) {
+  DCHECK(main_task_runner_->BelongsToCurrentThread());
+  CHECK(demuxer_.get());
+
+  // TODO(servolk): Implement disabling/enabling video stream.
+}
+
 blink::WebSize WebMediaPlayerImpl::naturalSize() const {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
 
@@ -841,6 +857,7 @@ void WebMediaPlayerImpl::OnFFmpegMediaTracksUpdated(
   DCHECK(!chunk_demuxer_);
 
   // Report the media track information to blink.
+  std::vector<blink::WebMediaPlayer::TrackId> blinkTrackIds;
   for (const auto& track : tracks->tracks()) {
     if (track->type() == MediaTrack::Audio) {
       auto track_id = client_->addAudioTrack(
@@ -849,7 +866,7 @@ void WebMediaPlayerImpl::OnFFmpegMediaTracksUpdated(
           blink::WebString::fromUTF8(track->label()),
           blink::WebString::fromUTF8(track->language()),
           /*enabled*/ true);
-      (void)track_id;
+      blinkTrackIds.push_back(track_id);
     } else if (track->type() == MediaTrack::Video) {
       auto track_id = client_->addVideoTrack(
           blink::WebString::fromUTF8(track->id()),
@@ -857,12 +874,14 @@ void WebMediaPlayerImpl::OnFFmpegMediaTracksUpdated(
           blink::WebString::fromUTF8(track->label()),
           blink::WebString::fromUTF8(track->language()),
           /*selected*/ true);
-      (void)track_id;
+      blinkTrackIds.push_back(track_id);
     } else {
       // Text tracks are not supported through this code path yet.
       NOTREACHED();
     }
   }
+
+  demuxer_->OnTrackIdsAssigned(*tracks.get(), blinkTrackIds);

[xhwang, 2016/06/03 18:52:45]

It looks really odd that we create the track in chromium, then call into blink
to get the id, then call chromium again to assign that id...

Since the track ID is just a number, I guess as long as it's unique we are fine.
Can we actually just create that ID in chromium? Then we don't need
OnTrackIdsAssigned().

Also, having comments in a lot of places that the |tracks| and |track_ids| must
have the same size and must be in the same order sounds a bad pattern and is
error prone.

[servolk, 2016/06/03 20:06:45]

Thanks, good point. Blink has had separate bytestream track ids and
WebMediaPlayer::TrackId for a long time (long before I touched any of that
code), and I assumed that both were necessary for some reason. But now after
these comments I went and looked closer and I've started wondering if we can get
rid of WebMediaPlayer::TrackId completely and live with identifying media tracks
by their bytestream ids. Sure, there will be some minor overhead for using
strings instead of unsigned integers as ids. But I think it's reasonable to
expect that the number of media tracks is typically very low (vast majority of
cases is just 1 audio + 1 video track, in 99% of other cases I'm sure the total
number of track will be fewer than ~5-10), so the overhead shouldn't be
significant.
Matt, now that philipj@ is no longer active in this area, I guess it's your
call. WDYT about getting rid of WebMediaPlayer::TrackId and using bytestream
track ids everywhere instead?

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

I'm also looking for away to let Chromium decide the track Ids, or at least a
way to get rid of the mapping being carried around in MediaTracks.

You mentioned in the chunk demuxer tests that separate bytestream tracks (from
separate init segments) may actually map to a single blink track - how would
this work if we just use bytestream tracks everywhere? 

If we can't use bytstream tracks everywhere, another proposal is to pass a
callback from blink into chromium to allow for reserving of a track id. Chromium
could reserve the id at the time its creating the demuxer stream and associate
them immediately. Then set that track id on the MediaTracks object when you
bubble things up to blink. 

[servolk, 2016/06/03 20:57:10]

Yes, multiple bytestream track ids might map to the same blink track id, but
only in cases when we have a single track of the given type (see the comment at
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/medias...).
So if we have only a single audio and a single video track, we don't need to pay
attention to track ids at all, we'll know what track ids gets enabled/disabled
simply because we know the track type. And for cases of multiple audio and/or
video tracks bytestream ids will remain unchanged between different init
segments.
That's why I think we could get rid of WebMediaPlayer::TrackId. IIUC  track type
+ bytestream track id should be sufficient to uniquely identify a media track.

[servolk, 2016/06/03 23:35:36]

Well, completely removing WebMediaPlayer::TrackId is difficult, since it's used
in many places. But I really wanted to give it a try, so I tried changing
WebMediaPlayer::TrackId type from unsigned to WebString and then removed blink
internal track ids completely and replaced them with byte stream track ids. Here
is what I tried:
https://codereview.chromium.org/2034273003/
And this seems to be working. Well, at least blink media and mediasource layout
tests pass. So I guess this is a viable idea.

[servolk, 2016/06/06 17:42:56]

Update: I though about it some more over the weekend, and I realized that
switching to using bytestream track ids might be problematic in case of multiple
SourceBuffers. In that case bytestream track ids are guaranteed to be unique
with a bytestream (i.e. a single SourceBuffer), but are not guaranteed to be
unique across different SourceBuffers. So we'll still need to have the concept
of WebMediaPlayer::TrackId that will be guaranteed to be unique with the
WebMediaPlayer, even if it has multiple SourceBuffers. But we could still try to
move the WMP::TrackId generation from WMP into demuxers. That would simplify
mapping between TrackIds and demuxer streams. I'll give it a try today.

 }
 
 void WebMediaPlayerImpl::SetCdm(const CdmAttachedCB& cdm_attached_cb,