Bug 1234230: Don't pass null images for video encoding, and don't reencode the same image r=roc
authorRandell Jesup <rjesup@jesup.org>
Mon, 29 Feb 2016 17:25:35 -0500
changeset 286172 0ac27aa6a4d8f6bed8e987480dc3e3ba2bba512a
parent 286171 8fed15997251f9b923c7485b1422df93296dd212
child 286173 8fd524b617206c44e0b0974da0dec0bb603cce83
push id30039
push usercbook@mozilla.com
push dateTue, 01 Mar 2016 11:02:11 +0000
treeherdermozilla-central@5cafa6f3019b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs1234230
milestone47.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 1234230: Don't pass null images for video encoding, and don't reencode the same image r=roc Largely clones logic from OMXEncoder. MozReview-Commit-ID: JfSERuCCPzT
dom/media/encoder/TrackEncoder.cpp
dom/media/encoder/VorbisTrackEncoder.cpp
--- a/dom/media/encoder/TrackEncoder.cpp
+++ b/dom/media/encoder/TrackEncoder.cpp
@@ -244,21 +244,35 @@ VideoTrackEncoder::AppendVideoSegment(co
 {
   ReentrantMonitorAutoEnter mon(mReentrantMonitor);
 
   // Append all video segments from MediaStreamGraph, including null an
   // non-null frames.
   VideoSegment::ChunkIterator iter(const_cast<VideoSegment&>(aSegment));
   while (!iter.IsEnded()) {
     VideoChunk chunk = *iter;
-    RefPtr<layers::Image> image = chunk.mFrame.GetImage();
-    mRawSegment.AppendFrame(image.forget(),
-                            chunk.GetDuration(),
-                            chunk.mFrame.GetIntrinsicSize(),
-                            chunk.mFrame.GetForceBlack());
+    mTotalFrameDuration += chunk.GetDuration();
+    // Send only the unique video frames for encoding
+    if (mLastFrame != chunk.mFrame) {
+      RefPtr<layers::Image> image = chunk.mFrame.GetImage();
+      // Because we may get chunks with a null image (due to input blocking),
+      // accumulate duration and give it to the next frame that arrives.
+      // Canonically incorrect - the duration should go to the previous frame
+      // - but that would require delaying until the next frame arrives.
+      // Best would be to do like OMXEncoder and pass an effective timestamp
+      // in with each frame (don't zero mTotalFrameDuration)
+      if (image) {
+        mRawSegment.AppendFrame(image.forget(),
+                                mTotalFrameDuration,
+                                chunk.mFrame.GetIntrinsicSize(),
+                                chunk.mFrame.GetForceBlack());
+        mTotalFrameDuration = 0;
+      }
+    }
+    mLastFrame.TakeFrom(&chunk.mFrame);
     iter.Next();
   }
 
   if (mRawSegment.GetDuration() > 0) {
     mReentrantMonitor.NotifyAll();
   }
 
   return NS_OK;
--- a/dom/media/encoder/VorbisTrackEncoder.cpp
+++ b/dom/media/encoder/VorbisTrackEncoder.cpp
@@ -51,17 +51,17 @@ VorbisTrackEncoder::Init(int aChannels, 
   mChannels = aChannels;
   mSamplingRate = aSamplingRate;
 
   int ret = 0;
   vorbis_info_init(&mVorbisInfo);
   double quality = mAudioBitrate ? (double)mAudioBitrate/aSamplingRate :
                    BASE_QUALITY;
 
-  printf("quality %f \n", quality);
+  VORBISLOG("quality %f", quality);
   ret = vorbis_encode_init_vbr(&mVorbisInfo, mChannels, mSamplingRate,
                                quality);
 
   mInitialized = (ret == 0);
 
   if (mInitialized) {
     // Set up the analysis state and auxiliary encoding storage
     vorbis_analysis_init(&mVorbisDsp, &mVorbisInfo);