Bug 1506093 - Fix DecodedStream A/V sync. r=padenot
authorAndreas Pehrson <apehrson@mozilla.com>
Fri, 22 Mar 2019 11:41:48 +0000
changeset 465631 663521b3a70fe6ab2265d0eccbd8ade7506b5f9e
parent 465630 7b980f1c50a127050f831b0a7d580eddc2939d27
child 465632 35977d5dcf5c994b9e763d844ffd4c0c48276793
push id35744
push userapavel@mozilla.com
push dateFri, 22 Mar 2019 16:44:08 +0000
treeherdermozilla-central@e66a2b59914d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspadenot
bugs1506093
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 1506093 - Fix DecodedStream A/V sync. r=padenot DecodedStream sends video to its video tracks by initially buffering a set of images, then appending future ones by adding them one by one. A long time ago we refactored how MediaStreamGraph sends images to the screen, i.e., to an ImageContainer. It used to send all future frames to ImageContainer::SetCurrentFrames each time it sent something. After the refactor we just forward any new frame from a direct listener to ImageContainer::SetCurrentFrames. So in case DecodedStream has already sent 10 future frames to its track, and sends another, we end up calling ImageContainer::SetCurrentFrames(frame11). However, this is not how ImageContainer works. The refactor was wrong. Even though the timestamp for frame11 is after a previously buffered frame, it will be ignored. SetCurrentFrames wipes any previously set frames. Hence the word "Current" in its name. This patch largely restores the old behaviour by adding a thin buffering layer between the MSG (in a direct listener) and the ImageContainer. This does not give 100% identical frame sync to VideoSink (how we normally render video), because VideoSink can update the timestamps of already-pushed images by pushing them again. We can't do that here because the SourceMediaStream API only allows appending. It does however get in sync for frames appended after the first frame has been rendered. Differential Revision: https://phabricator.services.mozilla.com/D22897
dom/media/VideoSegment.h
dom/media/VideoStreamTrack.cpp
--- a/dom/media/VideoSegment.h
+++ b/dom/media/VideoSegment.h
@@ -118,16 +118,48 @@ class VideoSegment : public MediaSegment
     if (!c) {
       return nullptr;
     }
     if (aStart) {
       *aStart = mDuration - c->mDuration;
     }
     return &c->mFrame;
   }
+  VideoChunk* FindChunkContainingTime(const TimeStamp& aTime) {
+    VideoChunk* previousChunk = nullptr;
+    for (VideoChunk& c : mChunks) {
+      if (c.mTimeStamp.IsNull()) {
+        continue;
+      }
+      if (c.mTimeStamp > aTime) {
+        return previousChunk;
+      }
+      previousChunk = &c;
+    }
+    return previousChunk;
+  }
+  void ForgetUpToTime(const TimeStamp& aTime) {
+    VideoChunk* chunk = FindChunkContainingTime(aTime);
+    if (!chunk) {
+      return;
+    }
+    StreamTime duration = 0;
+    size_t chunksToRemove = 0;
+    for (const VideoChunk& c : mChunks) {
+      if (c.mTimeStamp >= chunk->mTimeStamp) {
+        break;
+      }
+      duration += c.GetDuration();
+      ++chunksToRemove;
+    }
+    mChunks.RemoveElementsAt(0, chunksToRemove);
+    mDuration -= duration;
+    MOZ_ASSERT(mChunks.Capacity() >= DEFAULT_SEGMENT_CAPACITY,
+               "Capacity must be retained after removing chunks");
+  }
   // Override default impl
   void ReplaceWithDisabled() override {
     for (ChunkIterator i(*this); !i.IsEnded(); i.Next()) {
       VideoChunk& chunk = *i;
       chunk.SetForceBlack(true);
     }
   }
 
--- a/dom/media/VideoStreamTrack.cpp
+++ b/dom/media/VideoStreamTrack.cpp
@@ -19,22 +19,40 @@ class VideoOutput : public DirectMediaSt
  public:
   explicit VideoOutput(VideoFrameContainer* aContainer)
       : mVideoFrameContainer(aContainer) {}
   void NotifyRealtimeTrackData(MediaStreamGraph* aGraph,
                                StreamTime aTrackOffset,
                                const MediaSegment& aMedia) override {
     MOZ_ASSERT(aMedia.GetType() == MediaSegment::VIDEO);
     const VideoSegment& video = static_cast<const VideoSegment&>(aMedia);
-    mVideoFrameContainer->SetCurrentFrames(video);
+    mSegment.ForgetUpToTime(TimeStamp::Now());
+    for (VideoSegment::ConstChunkIterator i(video); !i.IsEnded(); i.Next()) {
+      if (!mLastFrameTime.IsNull() && i->mTimeStamp < mLastFrameTime) {
+        // Time can go backwards if the source is a captured MediaDecoder and
+        // it seeks, as the previously buffered frames would stretch into the
+        // future. If this happens, we clear the buffered frames and start over.
+        mSegment.Clear();
+      }
+      const VideoFrame& f = i->mFrame;
+      mSegment.AppendFrame(do_AddRef(f.GetImage()), 0, f.GetIntrinsicSize(),
+                           f.GetPrincipalHandle(), f.GetForceBlack(),
+                           i->mTimeStamp);
+      mLastFrameTime = i->mTimeStamp;
+    }
+    mVideoFrameContainer->SetCurrentFrames(mSegment);
   }
-  void NotifyDirectListenerUninstalled() override {
+  void NotifyRemoved() override {
+    mSegment.Clear();
     mVideoFrameContainer->ClearFrames();
   }
+  void NotifyEnded() override { mSegment.Clear(); }
 
+  TimeStamp mLastFrameTime;
+  VideoSegment mSegment;
   const RefPtr<VideoFrameContainer> mVideoFrameContainer;
 };
 
 namespace dom {
 
 VideoStreamTrack::VideoStreamTrack(DOMMediaStream* aStream, TrackID aTrackID,
                                    TrackID aInputTrackID,
                                    MediaStreamTrackSource* aSource,
@@ -51,24 +69,26 @@ void VideoStreamTrack::AddVideoOutput(Vi
   for (const auto& output : mVideoOutputs) {
     if (output->mVideoFrameContainer == aSink) {
       MOZ_ASSERT_UNREACHABLE("A VideoFrameContainer was already added");
       return;
     }
   }
   RefPtr<VideoOutput>& output =
       *mVideoOutputs.AppendElement(MakeRefPtr<VideoOutput>(aSink));
-  GetOwnedStream()->AddDirectTrackListener(output, mTrackID);
+  AddDirectListener(output);
+  AddListener(output);
 }
 
 void VideoStreamTrack::RemoveVideoOutput(VideoFrameContainer* aSink) {
   for (const auto& output : nsTArray<RefPtr<VideoOutput>>(mVideoOutputs)) {
     if (output->mVideoFrameContainer == aSink) {
       mVideoOutputs.RemoveElement(output);
-      GetOwnedStream()->RemoveDirectTrackListener(output, mTrackID);
+      RemoveDirectListener(output);
+      RemoveListener(output);
     }
   }
 }
 
 void VideoStreamTrack::GetLabel(nsAString& aLabel, CallerType aCallerType) {
   if (nsContentUtils::ResistFingerprinting(aCallerType)) {
     aLabel.AssignLiteral("Internal Camera");
     return;