Pass MSE media track info from ChunkDemuxer to blink::SourceBuffer

third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp

Index: third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp
diff --git a/third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp b/third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp
index d8b8e8a59271ae800fca7006260dc6a950d2dfc7..cb424ba56c12eb5ba816beb9aa60fc4f7a55d5cf 100644
--- a/third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp
+++ b/third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp
@@ -42,6 +42,10 @@
 #include "core/html/HTMLMediaElement.h"
 #include "core/html/MediaError.h"
 #include "core/html/TimeRanges.h"
+#include "core/html/track/AudioTrack.h"
+#include "core/html/track/AudioTrackList.h"
+#include "core/html/track/VideoTrack.h"
+#include "core/html/track/VideoTrackList.h"
 #include "core/streams/Stream.h"
 #include "modules/mediasource/MediaSource.h"
 #include "platform/Logging.h"
@@ -122,6 +126,9 @@ SourceBuffer::SourceBuffer(PassOwnPtr<WebSourceBuffer> webSourceBuffer, MediaSou
 {
     ASSERT(m_webSourceBuffer);
     ASSERT(m_source);
+    ASSERT(m_source->mediaElement());
+    m_audioTracks = AudioTrackList::create(*m_source->mediaElement());

[philipj_slow, 2016/03/29 11:59:21]

Looks like you could put this in the initializer list as source->mediaElement(),
unless doing the ASSERTS first is important?

[servolk, 2016/03/29 17:30:34]

Yeah, I wanted to check (via ASSERT) that we have a valid
m_source->mediaElement(), so I'd prefer to keep it here, instead of the
initializer list.

+    m_videoTracks = VideoTrackList::create(*m_source->mediaElement());
     m_webSourceBuffer->setClient(this);
 }
 
@@ -228,6 +235,18 @@ void SourceBuffer::setTimestampOffset(double offset, ExceptionState& exceptionSt
     m_timestampOffset = offset;
 }
 
+AudioTrackList& SourceBuffer::audioTracks()
+{
+    ASSERT(RuntimeEnabledFeatures::audioVideoTracksEnabled());
+    return *m_audioTracks;
+}
+
+VideoTrackList& SourceBuffer::videoTracks()
+{
+    ASSERT(RuntimeEnabledFeatures::audioVideoTracksEnabled());
+    return *m_videoTracks;
+}
+
 double SourceBuffer::appendWindowStart() const
 {
     return m_appendWindowStart;
@@ -471,17 +490,70 @@ void SourceBuffer::removedFromMediaSource()
     m_asyncEventQueue = nullptr;
 }
 
-void SourceBuffer::initializationSegmentReceived()
+template<class T>
+T* findTrackById(const TrackListBase<T>& trackList, const WebString& id)
 {
-    WTF_LOG(Media, "SourceBuffer::initializationSegmentReceived %p", this);
+    // According to MSE specification (https://www.w3.org/TR/media-source/#sourcebuffer-init-segment-received) step 3.1:
+    // > If more than one track for a single type are present (ie 2 audio tracks), then the Track IDs match the ones in the first initialization segment.
+    // I.e. we only need to search by TrackID if there is more than one track, otherwise we can assume that the only

[philipj_slow, 2016/03/29 11:59:21]

Is this an optimization, or is it expected to ever be observable? Just using
trackList.getTrackById() if that works seems simpler.

Comments if findTrackById needs to stay:
 - getTrackById takes a const String&, so this could too to move the conversion
closer to where the WebString came from.
 - Link to
https://w3c.github.io/media-source/#sourcebuffer-init-segment-received (see
other links in this file)
 - Naming could be more clear about how it's different from getTrackById.

[servolk, 2016/03/29 17:30:34]

wolenetz@ mentioned that MSE spec allows bytestream track id to change if there
is only a single track of a given kind. E.g. if you have only a single audio
track, and then you receive a new init segment with a single audio track, then
you must assume it's the same audio track as before, even if bytestream id is
different. And after reading the MSE spec, especially the quoted sentence, I'm
inclined to agree with it. This function just implements that logic.

+    // track of the given type is the same one that we had in previous init segments.
+    if (trackList.length() == 1)
+        return trackList.anonymousIndexedGetter(0);
+    return trackList.getTrackById(id);
+}
+
+std::vector<WebMediaPlayer::TrackId> SourceBuffer::initializationSegmentReceived(const std::vector<MediaTrackInfo>& newTracks)
+{
+    WTF_LOG(Media, "SourceBuffer::initializationSegmentReceived2 %p tracks=%zu", this, newTracks.size());
     ASSERT(m_source);
+    ASSERT(m_source->mediaElement());
     ASSERT(m_updating);
 
-    // https://dvcs.w3.org/hg/html-media/raw-file/tip/media-source/media-source.html#sourcebuffer-init-segment-received
-    // FIXME: Make steps 1-7 synchronous with this call.
-    // FIXME: Augment the interface to this method to implement compliant steps 4-7 here.
-    // Step 3 (if the first initialization segment received flag is true) is
-    // implemented by caller.
+    // TODO(servolk): Implement proper 'initialization segment received' algorithm according to MSE spec:
+    // https://www.w3.org/TR/media-source/#sourcebuffer-init-segment-received

[philipj_slow, 2016/03/29 11:59:21]

https://w3c.github.io/media-source/#sourcebuffer-init-segment-received here too

[servolk, 2016/03/29 17:30:34]

Done.

+    std::vector<WebMediaPlayer::TrackId> result;
+    for (const auto& trackInfo : newTracks) {
+        const auto& trackType = std::get<0>(trackInfo);
+        const auto& id = std::get<1>(trackInfo);
+        const auto& kind = std::get<2>(trackInfo);
+        const auto& label = std::get<3>(trackInfo);
+        const auto& language = std::get<4>(trackInfo);
+
+        const TrackBase* trackBase = nullptr;
+        if (trackType == WebMediaPlayer::AudioTrack) {
+            AudioTrack* audioTrack = nullptr;
+            if (!m_firstInitializationSegmentReceived) {
+                audioTrack = AudioTrack::create(id, kind, label, language, false);
+                audioTracks().add(audioTrack);
+                m_source->mediaElement()->audioTracks().add(audioTrack);
+            } else {
+                audioTrack = findTrackById(audioTracks(), id);

[philipj_slow, 2016/03/29 11:59:21]

How about making it part of the API that
if(m_firstInitializationSegmentReceived), an empty vector is returned instead? I
assume the Chromium side of this won't have any use for the IDs the second time
around anyway?

[servolk, 2016/03/29 17:30:34]

No, see the comments above for findExistingTrackById. We can't return the empty
vector here. It's important to return the track ids to Chromium for each init
segment, so that Chromium could update mappings between blink track ids and
Chromium media tracks/demuxer streams in case there was a media track with the
new bytestream id in the new init segment that got matched and assigned a track
id from the previous/existing track that was in the previous init segment.

+                ASSERT(audioTrack);
+            }
+            trackBase = audioTrack;
+            result.push_back(audioTrack->trackId());
+        } else if (trackType == WebMediaPlayer::VideoTrack) {
+            VideoTrack* videoTrack = nullptr;
+            if (!m_firstInitializationSegmentReceived) {
+                videoTrack = VideoTrack::create(id, kind, label, language, false);
+                videoTracks().add(videoTrack);
+                m_source->mediaElement()->videoTracks().add(videoTrack);
+            } else {
+                videoTrack = findTrackById(videoTracks(), id);
+                ASSERT(videoTrack);
+            }
+            trackBase = videoTrack;
+            result.push_back(videoTrack->trackId());
+        } else {
+            NOTREACHED();
+        }
+        const char* logActionStr = m_firstInitializationSegmentReceived ? "using existing" : "added";

[philipj_slow, 2016/03/29 11:59:21]

The trackBase variable is only for logging it seems, can you move the logging to
before the loop, use #if !LOG_DISABLED to avoid (void) suppression and just use
the inputs instead of trackBase->?

[servolk, 2016/03/29 17:30:34]

Well, actually one important thing that we want to see in the log is the
trackBase->trackId(), I'd say it's more important than all the other track
properties, since it's necessary to understand which demuxer stream/media track
on the Chromium side is represented by this blink track object. So I'd prefer to
keep this logging here, where we've already created/found the track and thus
know it's trackId.
Though I agree, we can make it look a little nicer with the #if !LOG_DISABLED.

+        const char* logTrackTypeStr = (trackType == WebMediaPlayer::AudioTrack) ? "audio" : "video";
+        (void)logActionStr;
+        (void)logTrackTypeStr;
+        (void)trackBase;
+        WTF_LOG(Media, "Tracks (sb=%p): %s %sTrack %p trackId=%d id=%s label=%s lang=%s", this, logActionStr, logTrackTypeStr, trackBase, trackBase->trackId(), trackBase->id().utf8().data(), trackBase->label().utf8().data(), trackBase->language().utf8().data());
+    }
 
     if (!m_firstInitializationSegmentReceived) {
         // 5. If active track flag equals true, then run the following steps:
@@ -493,6 +565,8 @@ void SourceBuffer::initializationSegmentReceived()
         // 6. Set first initialization segment received flag to true.
         m_firstInitializationSegmentReceived = true;
     }
+
+    return result;
 }
 
 bool SourceBuffer::hasPendingActivity() const
@@ -873,6 +947,8 @@ DEFINE_TRACE(SourceBuffer)
     visitor->trace(m_removeAsyncPartRunner);
     visitor->trace(m_appendStreamAsyncPartRunner);
     visitor->trace(m_stream);
+    visitor->trace(m_audioTracks);
+    visitor->trace(m_videoTracks);
     RefCountedGarbageCollectedEventTargetWithInlineData<SourceBuffer>::trace(visitor);
     ActiveDOMObject::trace(visitor);
 }