Bug 1333341 - Fix accumulating rounding error in VideoTrackEncoder. r?jesup draft
authorAndreas Pehrson <pehrsons@gmail.com>
Tue, 24 Jan 2017 16:18:48 +0100
changeset 465629 0ec23ed2255d6aca3ed035ca56353f59178db34e
parent 465600 8604e8de61e61c69ec02295ccd8a92fe770e2783
child 465630 09d03e7a10a3d1d4879d036993d8ba8804cf4515
push id42656
push userbmo:pehrson@telenordigital.com
push dateTue, 24 Jan 2017 15:23:31 +0000
reviewersjesup
bugs1333341
milestone53.0a1
Bug 1333341 - Fix accumulating rounding error in VideoTrackEncoder. r?jesup MozReview-Commit-ID: GoVNRqvZPSF
dom/media/encoder/TrackEncoder.cpp
dom/media/encoder/TrackEncoder.h
--- a/dom/media/encoder/TrackEncoder.cpp
+++ b/dom/media/encoder/TrackEncoder.cpp
@@ -312,21 +312,23 @@ VideoTrackEncoder::AppendVideoSegment(co
       chunk.mDuration = 0;
 
       TRACK_LOG(LogLevel::Verbose,
                 ("[VideoTrackEncoder]: Got first video chunk after %lld ticks.",
                  nullDuration));
       // Adapt to the time before the first frame. This extends the first frame
       // from [start, end] to [0, end], but it'll do for now.
       CheckedInt64 diff = FramesToUsecs(nullDuration, mTrackRate);
-      MOZ_ASSERT(diff.isValid());
-      if (diff.isValid()) {
-        mLastChunk.mTimeStamp -= TimeDuration::FromMicroseconds(diff.value());
-        mLastChunk.mDuration += nullDuration;
+      if (!diff.isValid()) {
+        NS_ERROR("null duration overflow");
+        return NS_ERROR_DOM_MEDIA_OVERFLOW_ERR;
       }
+
+      mLastChunk.mTimeStamp -= TimeDuration::FromMicroseconds(diff.value());
+      mLastChunk.mDuration += nullDuration;
     }
 
     MOZ_ASSERT(!mLastChunk.IsNull());
     if (mLastChunk.CanCombineWithFollowing(chunk) || chunk.IsNull()) {
       TRACK_LOG(LogLevel::Verbose,
                 ("[VideoTrackEncoder]: Got dupe or null chunk."));
       // This is the same frame as before (or null). We extend the last chunk
       // with its duration.
@@ -352,35 +354,51 @@ VideoTrackEncoder::AppendVideoSegment(co
 
       if (chunk.IsNull()) {
         // Ensure that we don't pass null to the encoder by making mLastChunk
         // null later on.
         chunk.mFrame = mLastChunk.mFrame;
       }
     }
 
-    TimeDuration diff = chunk.mTimeStamp - mLastChunk.mTimeStamp;
-    if (diff <= TimeDuration::FromSeconds(0)) {
-      // The timestamp from mLastChunk is newer than from chunk.
-      // This means the durations reported from MediaStreamGraph for mLastChunk
-      // were larger than the timestamp diff - and durations were used to
-      // trigger the 1-second frame above. This could happen due to drift or
-      // underruns in the graph.
-      TRACK_LOG(LogLevel::Warning,
-                ("[VideoTrackEncoder]: Underrun detected. Diff=%.5fs",
-                 diff.ToSeconds()));
-      chunk.mTimeStamp = mLastChunk.mTimeStamp;
-    } else {
-      RefPtr<layers::Image> lastImage = mLastChunk.mFrame.GetImage();
-      TRACK_LOG(LogLevel::Verbose,
-                ("[VideoTrackEncoder]: Appending video frame %p, duration=%.5f",
-                 lastImage.get(), diff.ToSeconds()));
-      CheckedInt64 duration = UsecsToFrames(diff.ToMicroseconds(), mTrackRate);
-      MOZ_ASSERT(duration.isValid());
-      if (duration.isValid()) {
+    if (mStartOffset.IsNull()) {
+      mStartOffset = mLastChunk.mTimeStamp;
+    }
+
+    TimeDuration relativeTime = chunk.mTimeStamp - mStartOffset;
+    RefPtr<layers::Image> lastImage = mLastChunk.mFrame.GetImage();
+    TRACK_LOG(LogLevel::Verbose,
+              ("[VideoTrackEncoder]: Appending video frame %p, at pos %.5fs",
+               lastImage.get(), relativeTime.ToSeconds()));
+    CheckedInt64 totalDuration =
+      UsecsToFrames(relativeTime.ToMicroseconds(), mTrackRate);
+    if (!totalDuration.isValid()) {
+      NS_ERROR("Duration overflow");
+      return NS_ERROR_DOM_MEDIA_OVERFLOW_ERR;
+    }
+
+    CheckedInt64 duration = totalDuration - mEncodedTicks;
+    if (!duration.isValid()) {
+      NS_ERROR("Duration overflow");
+      return NS_ERROR_DOM_MEDIA_OVERFLOW_ERR;
+    }
+
+    if (duration.isValid()) {
+      if (duration.value() <= 0) {
+        // The timestamp for mLastChunk is newer than for chunk.
+        // This means the durations reported from MediaStreamGraph for
+        // mLastChunk were larger than the timestamp diff - and durations were
+        // used to trigger the 1-second frame above. This could happen due to
+        // drift or underruns in the graph.
+        TRACK_LOG(LogLevel::Warning,
+                  ("[VideoTrackEncoder]: Underrun detected. Diff=%lld",
+                   duration.value()));
+        chunk.mTimeStamp = mLastChunk.mTimeStamp;
+      } else {
+        mEncodedTicks += duration.value();
         mRawSegment.AppendFrame(lastImage.forget(),
                                 duration.value(),
                                 mLastChunk.mFrame.GetIntrinsicSize(),
                                 PRINCIPAL_HANDLE_NONE,
                                 mLastChunk.mFrame.GetForceBlack(),
                                 mLastChunk.mTimeStamp);
       }
     }
--- a/dom/media/encoder/TrackEncoder.h
+++ b/dom/media/encoder/TrackEncoder.h
@@ -71,17 +71,17 @@ public:
   /**
    * Notifies from MediaEncoder to cancel the encoding, and wakes up
    * mReentrantMonitor if encoder is waiting on it.
    */
   void NotifyCancel()
   {
     ReentrantMonitorAutoEnter mon(mReentrantMonitor);
     mCanceled = true;
-    mReentrantMonitor.NotifyAll();
+    NotifyEndOfStream();
   }
 
   virtual void SetBitrate(const uint32_t aBitrate) {}
 
 protected:
   /**
    * Notifies track encoder that we have reached the end of source stream, and
    * wakes up mReentrantMonitor if encoder is waiting for any source data.
@@ -250,16 +250,17 @@ class VideoTrackEncoder : public TrackEn
 public:
   explicit VideoTrackEncoder(TrackRate aTrackRate)
     : TrackEncoder()
     , mFrameWidth(0)
     , mFrameHeight(0)
     , mDisplayWidth(0)
     , mDisplayHeight(0)
     , mTrackRate(aTrackRate)
+    , mEncodedTicks(0)
     , mVideoBitrate(0)
   {
     mLastChunk.mDuration = 0;
   }
 
   /**
    * Notified by the same callback of MediaEncoder when it has received a track
    * change from MediaStreamGraph. Called on the MediaStreamGraph thread.
@@ -344,14 +345,26 @@ protected:
    */
   VideoChunk mLastChunk;
 
   /**
    * A segment queue of audio track data, protected by mReentrantMonitor.
    */
   VideoSegment mRawSegment;
 
+  /**
+   * The number of mTrackRate ticks we have passed to the encoder.
+   * Only accessed in AppendVideoSegment().
+   */
+  StreamTime mEncodedTicks;
+
+  /**
+   * The time of the first real video frame passed to the encoder.
+   * Only accessed in AppendVideoSegment().
+   */
+  TimeStamp mStartOffset;
+
   uint32_t mVideoBitrate;
 };
 
 } // namespace mozilla
 
 #endif