Bug 1423241 - Fix MediaStreamTrackListener::NotifyEnded. r=padenot
authorAndreas Pehrson <apehrson@mozilla.com>
Fri, 23 Nov 2018 15:00:26 +0000
changeset 507038 befba547fb5850fd62d4e31784aa4b5198404500
parent 507037 12f6bb0888a2c9359758be3f62e06377c0ad1333
child 507039 de66629a4a2057724d381b2538b5ee1e885e90c2
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspadenot
bugs1423241
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1423241 - Fix MediaStreamTrackListener::NotifyEnded. r=padenot Without this, NotifyEnded() happens before the track has been played out, at the time it's marked ended by its producer. This change will actually make us wait until the last chunk has been played out and then notify listeners. Differential Revision: https://phabricator.services.mozilla.com/D12269
dom/media/MediaStreamGraph.cpp
dom/media/StreamTracks.h
dom/media/TrackUnionStream.cpp
dom/media/webaudio/AudioNodeStream.cpp
--- a/dom/media/MediaStreamGraph.cpp
+++ b/dom/media/MediaStreamGraph.cpp
@@ -183,16 +183,33 @@ void MediaStreamGraphImpl::UpdateCurrent
     if (isAnyUnblocked) {
       NS_ASSERTION(
           !stream->mNotifiedFinished,
           "Shouldn't have already notified of finish *and* have output!");
       for (uint32_t j = 0; j < stream->mListeners.Length(); ++j) {
         MediaStreamListener* l = stream->mListeners[j];
         l->NotifyOutput(this, mProcessedTime);
       }
+
+      for (StreamTracks::TrackIter track(stream->mTracks); !track.IsEnded();
+           track.Next()) {
+        if (track->IsEnded() &&
+            track->GetEnd() <=
+                stream->GraphTimeToStreamTime(mStateComputedTime) &&
+            !track->NotifiedEnded()) {
+          // Playout of this track ended and listeners have not been notified.
+          for (const TrackBound<MediaStreamTrackListener>& listener :
+               stream->mTrackListeners) {
+            if (listener.mTrackID == track->GetID()) {
+              track->NotifyEnded();
+              listener.mListener->NotifyEnded();
+            }
+          }
+        }
+      }
     }
 
     // The stream is fully finished when all of its track data has been played
     // out.
     if (stream->mFinished && !stream->mNotifiedFinished &&
         mProcessedTime >= stream->StreamTimeToGraphTime(
                               stream->GetStreamTracks().GetAllTracksEnd())) {
       stream->mNotifiedFinished = true;
@@ -2705,19 +2722,16 @@ void SourceMediaStream::ExtractPendingIn
       }
     }
 
     for (TrackBound<MediaStreamTrackListener>& b : mTrackListeners) {
       if (b.mTrackID != data->mID) {
         continue;
       }
       b.mListener->NotifyQueuedChanges(GraphImpl(), offset, *data->mData);
-      if (data->mCommands & SourceMediaStream::TRACK_END) {
-        b.mListener->NotifyEnded();
-      }
     }
     if (data->mCommands & SourceMediaStream::TRACK_CREATE) {
       MediaSegment* segment = data->mData.forget();
       LOG(LogLevel::Debug,
           ("%p: SourceMediaStream %p creating track %d, start %" PRId64
            ", initial end %" PRId64,
            GraphImpl(), this, data->mID, int64_t(data->mStart),
            int64_t(segment->GetDuration())));
--- a/dom/media/StreamTracks.h
+++ b/dom/media/StreamTracks.h
@@ -49,17 +49,21 @@ class StreamTracks {
    * Tracks have a unique ID assigned at creation. This allows us to identify
    * the same track across StreamTrackss. A StreamTracks should never have
    * two tracks with the same ID (even if they don't overlap in time).
    * TODO Tracks can also be enabled and disabled over time.
    * Takes ownership of aSegment.
    */
   class Track final {
     Track(TrackID aID, StreamTime aStart, MediaSegment* aSegment)
-        : mStart(aStart), mSegment(aSegment), mID(aID), mEnded(false) {
+        : mStart(aStart),
+          mSegment(aSegment),
+          mID(aID),
+          mEnded(false),
+          mNotifiedEnded(false) {
       MOZ_COUNT_CTOR(Track);
 
       NS_ASSERTION(aID > TRACK_NONE, "Bad track ID");
       NS_ASSERTION(0 <= aStart && aStart <= aSegment->GetDuration(),
                    "Bad start position");
     }
 
    public:
@@ -74,18 +78,23 @@ class StreamTracks {
     }
 
     MediaSegment* GetSegment() const { return mSegment; }
     TrackID GetID() const { return mID; }
     bool IsEnded() const { return mEnded; }
     StreamTime GetStart() const { return mStart; }
     StreamTime GetEnd() const { return mSegment->GetDuration(); }
     MediaSegment::Type GetType() const { return mSegment->GetType(); }
+    bool NotifiedEnded() const { return mNotifiedEnded; }
 
     void SetEnded() { mEnded = true; }
+    void NotifyEnded() {
+      MOZ_ASSERT(mEnded);
+      mNotifiedEnded = true;
+    }
     void AppendFrom(Track* aTrack) {
       NS_ASSERTION(!mEnded, "Can't append to ended track");
       NS_ASSERTION(aTrack->mID == mID, "IDs must match");
       NS_ASSERTION(aTrack->mStart == 0, "Source track must start at zero");
       NS_ASSERTION(aTrack->mSegment->GetType() == GetType(),
                    "Track types must match");
 
       mSegment->AppendFrom(aTrack->mSegment);
@@ -114,16 +123,18 @@ class StreamTracks {
     StreamTime mStart;
     // The segment data starts at the start of the owning StreamTracks, i.e.,
     // there's mStart silence/no video at the beginning.
     nsAutoPtr<MediaSegment> mSegment;
     // Unique ID
     TrackID mID;
     // True when the track ends with the data in mSegment
     bool mEnded;
+    // True after NotifiedEnded() has been called.
+    bool mNotifiedEnded;
   };
 
   class MOZ_STACK_CLASS CompareTracksByID final {
    public:
     bool Equals(Track* aA, Track* aB) const {
       return aA->GetID() == aB->GetID();
     }
     bool LessThan(Track* aA, Track* aB) const {
--- a/dom/media/TrackUnionStream.cpp
+++ b/dom/media/TrackUnionStream.cpp
@@ -269,21 +269,16 @@ void TrackUnionStream::EndTrack(uint32_t
     StreamTime offset = outputTrack->GetSegment()->GetDuration();
     nsAutoPtr<MediaSegment> segment;
     segment = outputTrack->GetSegment()->CreateEmptyClone();
     l->NotifyQueuedTrackChanges(Graph(), outputTrack->GetID(), offset,
                                 TrackEventCommand::TRACK_EVENT_ENDED, *segment,
                                 mTrackMap[aIndex].mInputPort->GetSource(),
                                 mTrackMap[aIndex].mInputTrackID);
   }
-  for (TrackBound<MediaStreamTrackListener>& b : mTrackListeners) {
-    if (b.mTrackID == outputTrack->GetID()) {
-      b.mListener->NotifyEnded();
-    }
-  }
   outputTrack->SetEnded();
 }
 
 void TrackUnionStream::CopyTrackData(StreamTracks::Track* aInputTrack,
                                      uint32_t aMapIndex, GraphTime aFrom,
                                      GraphTime aTo,
                                      bool* aOutputTrackFinished) {
   TrackMapEntry* map = &mTrackMap[aMapIndex];
--- a/dom/media/webaudio/AudioNodeStream.cpp
+++ b/dom/media/webaudio/AudioNodeStream.cpp
@@ -596,26 +596,19 @@ void AudioNodeStream::AdvanceOutputSegme
 
 void AudioNodeStream::FinishOutput() {
   StreamTracks::Track* track = EnsureTrack(AUDIO_TRACK);
   track->SetEnded();
 
   for (uint32_t j = 0; j < mListeners.Length(); ++j) {
     MediaStreamListener* l = mListeners[j];
     AudioSegment emptySegment;
-    l->NotifyQueuedTrackChanges(
-        Graph(), AUDIO_TRACK, track->GetSegment()->GetDuration(),
-        TrackEventCommand::TRACK_EVENT_ENDED, emptySegment);
-  }
-  for (TrackBound<MediaStreamTrackListener>& b : mTrackListeners) {
-    // Notify MediaStreamTrackListeners.
-    if (b.mTrackID != AUDIO_TRACK) {
-      continue;
-    }
-    b.mListener->NotifyEnded();
+    l->NotifyQueuedTrackChanges(Graph(), AUDIO_TRACK, track->GetEnd(),
+                                TrackEventCommand::TRACK_EVENT_ENDED,
+                                emptySegment);
   }
 }
 
 void AudioNodeStream::AddInput(MediaInputPort* aPort) {
   ProcessedMediaStream::AddInput(aPort);
   AudioNodeStream* ns = aPort->GetSource()->AsAudioNodeStream();
   // Streams that are not AudioNodeStreams are considered active.
   if (!ns || (ns->mIsActive && !ns->IsAudioParamStream())) {