Backed out 2 changesets (bug 1669503) for crashing in mozilla::MP3TrackDemuxer::GetNextFrame. a=backout DONTBUILD

Backed out changeset 863ecb9b15d5 (bug 1669503)
Backed out changeset d5cbc458d318 (bug 1669503)

dom/media/MediaData.cpp

@@ -13,7 +13,6 @@
 #include "mozilla/layers/ImageBridgeChild.h"
 #include "mozilla/layers/KnowsCompositor.h"
 #include "mozilla/layers/SharedRGBImage.h"
-#include "nsMathUtils.h"
 
 #include <stdint.h>
 
@@ -74,17 +73,15 @@ bool AudioData::SetTrimWindow(const media::TimeInterval& aTrim) {
     return false;
   }
 
-  auto roundToNearestFrame = [&](const TimeUnit& aTime) -> uint64_t {
-    MOZ_ASSERT((TimeUnitToFrames(aTime, mRate) + 1).isValid());
-    return NS_round(aTime.ToSeconds() * mRate);
-  };
-
-  uint64_t trimBefore = roundToNearestFrame(aTrim.mStart - mOriginalTime);
+  auto trimBefore = TimeUnitToFrames(aTrim.mStart - mOriginalTime, mRate);
   auto trimAfter = aTrim.mEnd == GetEndTime()
                        ? originalFrames
-                       : roundToNearestFrame(aTrim.mEnd - mOriginalTime);
-
-  MOZ_DIAGNOSTIC_ASSERT(trimAfter >= trimBefore,
+                       : TimeUnitToFrames(aTrim.mEnd - mOriginalTime, mRate);
+  if (!trimBefore.isValid() || !trimAfter.isValid()) {
+    // Overflow.
+    return false;
+  }
+  MOZ_DIAGNOSTIC_ASSERT(trimAfter.value() >= trimBefore.value(),
                         "Something went wrong with trimming value");
   if (!mTrimWindow && trimBefore == 0 && trimAfter == originalFrames) {
     // Nothing to change, abort early to prevent rounding errors.
@@ -92,13 +89,13 @@ bool AudioData::SetTrimWindow(const media::TimeInterval& aTrim) {
   }
 
   mTrimWindow = Some(aTrim);
-  mDataOffset = trimBefore * mChannels;
+  mDataOffset = trimBefore.value() * mChannels;
   MOZ_DIAGNOSTIC_ASSERT(mDataOffset <= mAudioData.Length(),
                         "Data offset outside original buffer");
-  mFrames = trimAfter - trimBefore;
+  mFrames = (trimAfter - trimBefore).value();
   MOZ_DIAGNOSTIC_ASSERT(mFrames <= originalFrames,
                         "More frames than found in container");
-  mTime = mOriginalTime + FramesToTimeUnit(trimBefore, mRate);
+  mTime = mOriginalTime + FramesToTimeUnit(trimBefore.value(), mRate);
   mDuration = FramesToTimeUnit(mFrames, mRate);
 
   return true;

dom/media/mp3/MP3Demuxer.cpp

@@ -6,13 +6,11 @@
 
 #include "MP3Demuxer.h"
 
-#include <inttypes.h>
-
 #include <algorithm>
+#include <inttypes.h>
 #include <limits>
 
 #include "ByteWriter.h"
-#include "Intervals.h"
 #include "TimeUnits.h"
 #include "VideoUtils.h"
 #include "mozilla/Assertions.h"
@@ -24,7 +22,6 @@ extern mozilla::LazyLogModule gMediaDemuxerLog;
   DDMOZ_LOG(gMediaDemuxerLog, LogLevel::Verbose, msg, ##__VA_ARGS__)
 
 using mozilla::BufferReader;
-using mozilla::media::Interval;
 using mozilla::media::TimeInterval;
 using mozilla::media::TimeIntervals;
 using mozilla::media::TimeUnit;
@@ -634,6 +631,15 @@ already_AddRefed<MediaRawData> MP3TrackDemuxer::GetNextFrame(
 
   UpdateState(aRange);
 
+  frame->mTime = Duration(mFrameIndex - 1);
+  frame->mDuration = Duration(1);
+  frame->mTimecode = frame->mTime;
+  frame->mKeyframe = true;
+  frame->mEOS = mEOS;
+
+  MOZ_ASSERT(!frame->mTime.IsNegative());
+  MOZ_ASSERT(frame->mDuration.IsPositive());
+
   if (mNumParsedFrames == 1) {
     // First frame parsed, let's read VBR info if available.
     BufferReader reader(frame->Data(), frame->Size());
@@ -650,31 +656,6 @@ already_AddRefed<MediaRawData> MP3TrackDemuxer::GetNextFrame(
     }
   }
 
-  frame->mTime = Duration(mFrameIndex - 1);
-  frame->mDuration = Duration(1);
-  frame->mTimecode = frame->mTime;
-  frame->mKeyframe = true;
-  frame->mEOS = mEOS;
-
-  auto duration = Duration();
-  if (duration) {
-    auto actualFramesInterval = TimeInterval(
-        FramesToTimeUnit(mEncoderDelay, mSamplesPerSecond),
-        *duration - FramesToTimeUnit(mEncoderPadding, mSamplesPerSecond));
-
-    auto frameInterval = TimeInterval(frame->mTime, frame->GetEndTime());
-    auto realFrameInterval = actualFramesInterval.Intersection(frameInterval);
-    if (realFrameInterval != frameInterval) {
-      frame->mOriginalPresentationWindow = Some(frameInterval);
-      frame->mDuration = realFrameInterval.Length();
-      frame->mTime = realFrameInterval.mStart;
-    }
-  }
-
-  MOZ_ASSERT(!frame->mTime.IsNegative());
-  MOZ_ASSERT(frame->mDuration.IsPositive() ||
-             frame->mDuration.ToSeconds() == 0.);
-
   MP3LOGV("GetNext() End mOffset=%" PRIu64 " mNumParsedFrames=%" PRIu64
           " mFrameIndex=%" PRId64 " mTotalFrameLen=%" PRIu64
           " mSamplesPerFrame=%d mSamplesPerSecond=%d mChannels=%d, mEOS=%s",

dom/media/platforms/ffmpeg/FFmpegAudioDecoder.cpp

@@ -22,7 +22,14 @@ FFmpegAudioDecoder<LIBAV_VER>::FFmpegAudioDecoder(FFmpegLibWrapper* aLib,
   if (aConfig.mCodecSpecificConfig && aConfig.mCodecSpecificConfig->Length()) {
     mExtraData = new MediaByteBuffer;
     mExtraData->AppendElements(*aConfig.mCodecSpecificConfig);
-    BufferReader reader(mExtraData->Elements(), mExtraData->Length());
+    if (mCodecID == AV_CODEC_ID_MP3) {
+      BufferReader reader(mExtraData->Elements(), mExtraData->Length());
+      mEncoderDelay = reader.ReadU32().unwrapOr(0);
+      mEncoderPadding = reader.ReadU32().unwrapOr(0);
+      FFMPEG_LOG("FFmpegAudioDecoder, found encoder delay (%" PRIu32
+                 ") and padding values (%" PRIu32 ") in extra data",
+                 mEncoderDelay, mEncoderPadding);
+    }
   }
 }
 
@@ -229,6 +236,25 @@ MediaResult FFmpegAudioDecoder<LIBAV_VER>::DoDecode(MediaRawData* aSample,
       if (!audio) {
         return MediaResult(NS_ERROR_OUT_OF_MEMORY, __func__);
       }
+
+      bool trimmed = false;
+      if (mEncoderDelay) {
+        trimmed = true;
+        uint32_t toPop = std::min((uint32_t)mFrame->nb_samples, mEncoderDelay);
+        audio.PopFront(toPop * numChannels);
+        mFrame->nb_samples -= toPop;
+        mEncoderDelay -= toPop;
+      }
+
+      if (aSample->mEOS && mEncoderPadding) {
+        trimmed = true;
+        uint32_t toTrim =
+            std::min((uint32_t)mFrame->nb_samples, mEncoderPadding);
+        mEncoderPadding -= toTrim;
+        audio.PopBack(toTrim * numChannels);
+        mFrame->nb_samples = audio.Length() / numChannels;
+      }
+
       media::TimeUnit duration =
           FramesToTimeUnit(mFrame->nb_samples, samplingRate);
       if (!duration.IsValid()) {
@@ -246,7 +272,8 @@ MediaResult FFmpegAudioDecoder<LIBAV_VER>::DoDecode(MediaRawData* aSample,
       RefPtr<AudioData> data =
           new AudioData(samplePosition, pts, std::move(audio), numChannels,
                         samplingRate, mCodecContext->channel_layout);
-      MOZ_DIAGNOSTIC_ASSERT(duration == data->mDuration, "must be equal");
+      MOZ_DIAGNOSTIC_ASSERT(duration == data->mDuration || trimmed,
+                            "must be equal");
       aResults.AppendElement(std::move(data));
 
       pts = newpts;

dom/media/platforms/ffmpeg/FFmpegAudioDecoder.h

@@ -38,6 +38,8 @@ class FFmpegAudioDecoder<LIBAV_VER>
  private:
   MediaResult DoDecode(MediaRawData* aSample, uint8_t* aData, int aSize,
                        bool* aGotFrame, DecodedData& aResults) override;
+  uint32_t mEncoderDelay = 0;
+  uint32_t mEncoderPadding = 0;
 };
 
 }  // namespace mozilla

dom/media/webaudio/test/mochitest.ini

@@ -33,8 +33,6 @@ support-files =
   sine-440-10s.opus
   half-a-second-8000.mp3
   half-a-second-48000.mp3
-  cropped_8000hz_8kbs_mono_lame3.100.abr.mp3
-  v3.mp3
   webaudio.js
   ../../webrtc/tests/mochitests/mediaStreamPlayback.js
   ../../webrtc/tests/mochitests/head.js

dom/media/webaudio/test/test_decoderDelay.html

@@ -12,33 +12,18 @@
 SimpleTest.waitForExplicitFinish();
 
 var tests = [
-  {
-    name: "half-a-second-8000.mp3",
-    duration: 0.5
-  },
-  {
-    name: "half-a-second-48000.mp3",
-    duration: 0.5
-  },
-  {
-    name: "v3.mp3",
-    duration: 3
-  },
-  {
-    // Weird file that has the end padding larger than a packet size.
-    name: "cropped_8000hz_8kbs_mono_lame3.100.abr.mp3",
-    duration: 28.65675
-  }
+  "half-a-second-8000.mp3",
+  "half-a-second-48000.mp3",
 ];
 
 async function doit(t) {
   var count = 0;
   var context = new OfflineAudioContext(1, 128, 48000);
-  tests.forEach(async testcase => {
-    var response = await fetch(testcase.name);
+  tests.forEach(async testfile => {
+    var response = await fetch(testfile);
     var buffer = await response.arrayBuffer();
     var decoded = await context.decodeAudioData(buffer);
-    is(decoded.duration, testcase.duration, "The file has the correct duration.");
+    is(decoded.duration, 0.5, "The file is half a second.");
     if (++count == tests.length) {
       SimpleTest.finish();
     }