Bug 1536766 - End a track only after the graph has reported reaching its end time in DecodedStream. r=jya,padenot
☠☠ backed out by 41f1dcbe9caa ☠ ☠
authorAndreas Pehrson <apehrson@mozilla.com>
Thu, 18 Apr 2019 15:25:54 +0000
changeset 470105 72d37a08f281c24833d1c49e947fb6db4df2fb87
parent 470104 63fc8588506005ac9eb0f1ec45ed34e430c6cf4a
child 470106 9f70dad286ffd871197861dbf435e5e4314bbd53
push id35888
push useraiakab@mozilla.com
push dateFri, 19 Apr 2019 09:47:45 +0000
treeherdermozilla-central@0160424142d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjya, padenot
bugs1536766
milestone68.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 1536766 - End a track only after the graph has reported reaching its end time in DecodedStream. r=jya,padenot This gives us a guarantee that the first frame of a media file can be rendered with a second media element and mozCaptureStream(), even if the file is very very short. With longer video-only files there were also cases where nothing ended up being rendered. Probably because the MediaStreamGraph ended up switching from an AudioCallbackDriver to a SystemClockDriver and this took enough time to put the SourceMediaStream::EndTrack and the SourceMediaStream::AddTrackListener calls for this video track to be processed in the same iteration. The listener would then always lose to the ending track and update main thread state too late, leading to its media element not rendering any frames and nasty intermittent failures. Differential Revision: https://phabricator.services.mozilla.com/D27270
dom/media/mediasink/DecodedStream.cpp
--- a/dom/media/mediasink/DecodedStream.cpp
+++ b/dom/media/mediasink/DecodedStream.cpp
@@ -87,16 +87,27 @@ class DecodedStreamGraphListener {
       mStream->AddTrackListener(mVideoTrackListener, mVideoTrackID);
     } else {
       mVideoEnded = true;
       mVideoEndedHolder.ResolveIfExists(true, __func__);
     }
   }
 
   void NotifyOutput(TrackID aTrackID, StreamTime aCurrentTrackTime) {
+    if (aTrackID == mAudioTrackID) {
+      if (aCurrentTrackTime >= mAudioEnd) {
+        mStream->EndTrack(mAudioTrackID);
+      }
+    } else if (aTrackID == mVideoTrackID) {
+      if (aCurrentTrackTime >= mVideoEnd) {
+        mStream->EndTrack(mVideoTrackID);
+      }
+    } else {
+      MOZ_CRASH("Unexpected TrackID");
+    }
     if (aTrackID != mAudioTrackID && mAudioTrackID != TRACK_NONE &&
         !mAudioEnded) {
       // Only audio playout drives the clock forward, if present and live.
       return;
     }
     MOZ_ASSERT_IF(aTrackID == mAudioTrackID, !mAudioEnded);
     MOZ_ASSERT_IF(aTrackID == mVideoTrackID, !mVideoEnded);
     mOnOutput.Notify(mStream->StreamTimeToMicroseconds(aCurrentTrackTime));
@@ -115,16 +126,46 @@ class DecodedStreamGraphListener {
             "DecodedStreamGraphListener::DoNotifyTrackEnded", this,
             &DecodedStreamGraphListener::DoNotifyTrackEnded, aTrackID));
   }
 
   TrackID AudioTrackID() const { return mAudioTrackID; }
 
   TrackID VideoTrackID() const { return mVideoTrackID; }
 
+  /**
+   * Tell the graph listener to end the given track after it has seen at least
+   * aEnd worth of output reported as processed by the graph.
+   *
+   * A StreamTime of STREAM_TIME_MAX indicates that the track has no end and is
+   * the default.
+   *
+   * This method of ending tracks is needed because the MediaStreamGraph
+   * processes ended tracks (through SourceMediaStream::EndTrack) at the
+   * beginning of an iteration, but waits until the end of the iteration to
+   * process any ControlMessages. When such a ControlMessage is a listener that
+   * is to be added to a track that has ended in its very first iteration, the
+   * track ends before the listener tracking this ending is added. This can lead
+   * to a MediaStreamTrack ending on main thread (it uses another listener)
+   * before the listeners to render the track get added, potentially meaning a
+   * media element doesn't progress before reaching the end although data was
+   * available.
+   *
+   * Callable from any thread.
+   */
+  void EndTrackAt(TrackID aTrackID, StreamTime aEnd) {
+    if (aTrackID == mAudioTrackID) {
+      mAudioEnd = aEnd;
+    } else if (aTrackID == mVideoTrackID) {
+      mVideoEnd = aEnd;
+    } else {
+      MOZ_CRASH("Unexpected TrackID");
+    }
+  }
+
   void DoNotifyTrackEnded(TrackID aTrackID) {
     MOZ_ASSERT(NS_IsMainThread());
     if (aTrackID == mAudioTrackID) {
       mAudioEndedHolder.ResolveIfExists(true, __func__);
     } else if (aTrackID == mVideoTrackID) {
       mVideoEndedHolder.ResolveIfExists(true, __func__);
     } else {
       MOZ_CRASH("Unexpected track id");
@@ -167,17 +208,19 @@ class DecodedStreamGraphListener {
 
   // Graph thread only.
   bool mAudioEnded = false;
   bool mVideoEnded = false;
 
   // Any thread.
   const RefPtr<SourceMediaStream> mStream;
   const TrackID mAudioTrackID;
+  Atomic<StreamTime> mAudioEnd{STREAM_TIME_MAX};
   const TrackID mVideoTrackID;
+  Atomic<StreamTime> mVideoEnd{STREAM_TIME_MAX};
   const RefPtr<AbstractThread> mAbstractMainThread;
 };
 
 DecodedStreamTrackListener::DecodedStreamTrackListener(
     DecodedStreamGraphListener* aGraphListener, SourceMediaStream* aStream,
     TrackID aTrackID)
     : mGraphListener(aGraphListener), mStream(aStream), mTrackID(aTrackID) {}
 
@@ -612,17 +655,17 @@ void DecodedStream::SendAudio(double aVo
   // |mNextAudioTime| is updated as we process each audio sample in
   // SendStreamAudio().
   if (output.GetDuration() > 0) {
     mData->mStreamAudioWritten +=
         sourceStream->AppendToTrack(audioTrackId, &output);
   }
 
   if (mAudioQueue.IsFinished() && !mData->mHaveSentFinishAudio) {
-    sourceStream->EndTrack(audioTrackId);
+    mData->mListener->EndTrackAt(audioTrackId, mData->mStreamAudioWritten);
     mData->mHaveSentFinishAudio = true;
   }
 }
 
 void DecodedStreamData::WriteVideoToSegment(
     layers::Image* aImage, const TimeUnit& aStart, const TimeUnit& aEnd,
     const gfx::IntSize& aIntrinsicSize, const TimeStamp& aTimeStamp,
     VideoSegment* aOutput, const PrincipalHandle& aPrincipalHandle) {
@@ -805,17 +848,17 @@ void DecodedStream::SendVideo(bool aIsSa
           &endSegment, aPrincipalHandle);
       MOZ_ASSERT(endSegment.GetDuration() > 0);
       if (!aIsSameOrigin) {
         endSegment.ReplaceWithDisabled();
       }
       mData->mStreamVideoWritten +=
           sourceStream->AppendToTrack(videoTrackId, &endSegment);
     }
-    sourceStream->EndTrack(videoTrackId);
+    mData->mListener->EndTrackAt(videoTrackId, mData->mStreamVideoWritten);
     mData->mHaveSentFinishVideo = true;
   }
 }
 
 StreamTime DecodedStream::SentDuration() {
   AssertOwnerThread();
 
   if (!mData) {