Backed out 4 changesets (bug 1500713) for build bustage
authorDorel Luca <dluca@mozilla.com>
Tue, 12 Mar 2019 07:16:59 +0200
changeset 521485 89988d424c06
parent 521484 d8b3da69c3a0
child 521486 bd7db47ad078
push id10866
push usernerli@mozilla.com
push dateTue, 12 Mar 2019 18:59:09 +0000
treeherdermozilla-beta@445c24a51727 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1500713
milestone67.0a1
backs outd25ff3b04eeb
98265537ef34
670b24af24d4
7e5fa7b1f7bc
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
Backed out 4 changesets (bug 1500713) for build bustage Backed out changeset d25ff3b04eeb (bug 1500713) Backed out changeset 98265537ef34 (bug 1500713) Backed out changeset 670b24af24d4 (bug 1500713) Backed out changeset 7e5fa7b1f7bc (bug 1500713)
dom/media/gtest/TestBufferReader.cpp
dom/media/gtest/TestMP3Demuxer.cpp
dom/media/gtest/moz.build
dom/media/gtest/test_vbri.mp3
dom/media/mp3/MP3Demuxer.cpp
dom/media/mp3/MP3Demuxer.h
dom/media/mp3/MP3FrameParser.cpp
deleted file mode 100644
--- a/dom/media/gtest/TestBufferReader.cpp
+++ /dev/null
@@ -1,35 +0,0 @@
-/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-#include "gtest/gtest.h"
-#include "BufferReader.h"
-
-TEST(BufferReader, ReaderCursor) {
-  // Allocate a buffer and create a BufferReader.
-  const size_t BUFFER_SIZE = 10;
-  uint8_t buffer[BUFFER_SIZE] = { 0 };
-
-  const uint8_t* const HEAD = reinterpret_cast<uint8_t*>(buffer);
-  const uint8_t* const TAIL = HEAD + BUFFER_SIZE;
-
-  BufferReader reader(HEAD, BUFFER_SIZE);
-  ASSERT_EQ(reader.Offset(), static_cast<size_t>(0));
-  ASSERT_EQ(reader.Peek(BUFFER_SIZE), HEAD);
-
-  // Keep reading to the end, and make sure the final read failed.
-  const size_t READ_SIZE = 4;
-  ASSERT_NE(BUFFER_SIZE % READ_SIZE, static_cast<size_t>(0));
-  for (const uint8_t* ptr = reader.Peek(0); ptr != nullptr;
-       ptr = reader.Read(READ_SIZE))
-    ;
-
-  // Check the reading cursor of the BufferReader is correct
-  // after reading and seeking.
-  const uint8_t* tail = reader.Peek(0);
-  const uint8_t* head = reader.Seek(0);
-
-  EXPECT_EQ(head, HEAD);
-  EXPECT_EQ(tail, TAIL);
-}
\ No newline at end of file
--- a/dom/media/gtest/TestMP3Demuxer.cpp
+++ b/dom/media/gtest/TestMP3Demuxer.cpp
@@ -42,21 +42,18 @@ class MockMP3StreamMediaResource
 
   int64_t GetLength() override { return -1; }
 
  protected:
   virtual ~MockMP3StreamMediaResource() {}
 };
 
 struct MP3Resource {
-  enum HeaderType { NONE, XING, VBRI };
-
   const char* mFilePath;
   bool mIsVBR;
-  HeaderType mHeaderType;
   int64_t mFileSize;
   int32_t mMPEGLayer;
   int32_t mMPEGVersion;
   uint8_t mID3MajorVersion;
   uint8_t mID3MinorVersion;
   uint8_t mID3Flags;
   uint32_t mID3Size;
 
@@ -81,17 +78,16 @@ struct MP3Resource {
 
 class MP3DemuxerTest : public ::testing::Test {
  protected:
   void SetUp() override {
     {
       MP3Resource res;
       res.mFilePath = "noise.mp3";
       res.mIsVBR = false;
-      res.mHeaderType = MP3Resource::NONE;
       res.mFileSize = 965257;
       res.mMPEGLayer = 3;
       res.mMPEGVersion = 1;
       res.mID3MajorVersion = 3;
       res.mID3MinorVersion = 0;
       res.mID3Flags = 0;
       res.mID3Size = 2141;
       res.mDuration = 30067000;
@@ -126,17 +122,16 @@ class MP3DemuxerTest : public ::testing:
       MP3Resource res;
       // This file trips up the MP3 demuxer if ID3v2 tags aren't properly
       // skipped. If skipping is not properly implemented, depending on the
       // strictness of the MPEG frame parser a false sync will be detected
       // somewhere within the metadata at or after 112087, or failing that, at
       // the artificially added extraneous header at 114532.
       res.mFilePath = "id3v2header.mp3";
       res.mIsVBR = false;
-      res.mHeaderType = MP3Resource::NONE;
       res.mFileSize = 191302;
       res.mMPEGLayer = 3;
       res.mMPEGVersion = 1;
       res.mID3MajorVersion = 3;
       res.mID3MinorVersion = 0;
       res.mID3Flags = 0;
       res.mID3Size = 115304;
       res.mDuration = 3166167;
@@ -166,17 +161,16 @@ class MP3DemuxerTest : public ::testing:
       streamRes.mDemuxer = new MP3TrackDemuxer(streamRes.mResource);
       mTargets.push_back(streamRes);
     }
 
     {
       MP3Resource res;
       res.mFilePath = "noise_vbr.mp3";
       res.mIsVBR = true;
-      res.mHeaderType = MP3Resource::XING;
       res.mFileSize = 583679;
       res.mMPEGLayer = 3;
       res.mMPEGVersion = 1;
       res.mID3MajorVersion = 3;
       res.mID3MinorVersion = 0;
       res.mID3Flags = 0;
       res.mID3Size = 2221;
       res.mDuration = 30081000;
@@ -205,17 +199,16 @@ class MP3DemuxerTest : public ::testing:
       streamRes.mDemuxer = new MP3TrackDemuxer(streamRes.mResource);
       mTargets.push_back(streamRes);
     }
 
     {
       MP3Resource res;
       res.mFilePath = "small-shot.mp3";
       res.mIsVBR = true;
-      res.mHeaderType = MP3Resource::XING;
       res.mFileSize = 6825;
       res.mMPEGLayer = 3;
       res.mMPEGVersion = 1;
       res.mID3MajorVersion = 4;
       res.mID3MinorVersion = 0;
       res.mID3Flags = 0;
       res.mID3Size = 24;
       res.mDuration = 336686;
@@ -246,17 +239,16 @@ class MP3DemuxerTest : public ::testing:
     }
 
     {
       MP3Resource res;
       // This file contains a false frame sync at 34, just after the ID3 tag,
       // which should be identified as a false positive and skipped.
       res.mFilePath = "small-shot-false-positive.mp3";
       res.mIsVBR = true;
-      res.mHeaderType = MP3Resource::XING;
       res.mFileSize = 6845;
       res.mMPEGLayer = 3;
       res.mMPEGVersion = 1;
       res.mID3MajorVersion = 4;
       res.mID3MinorVersion = 0;
       res.mID3Flags = 0;
       res.mID3Size = 24;
       res.mDuration = 336686;
@@ -285,17 +277,16 @@ class MP3DemuxerTest : public ::testing:
       streamRes.mDemuxer = new MP3TrackDemuxer(streamRes.mResource);
       mTargets.push_back(streamRes);
     }
 
     {
       MP3Resource res;
       res.mFilePath = "small-shot-partial-xing.mp3";
       res.mIsVBR = true;
-      res.mHeaderType = MP3Resource::XING;
       res.mFileSize = 6825;
       res.mMPEGLayer = 3;
       res.mMPEGVersion = 1;
       res.mID3MajorVersion = 4;
       res.mID3MinorVersion = 0;
       res.mID3Flags = 0;
       res.mID3Size = 24;
       res.mDuration = 336686;
@@ -320,55 +311,16 @@ class MP3DemuxerTest : public ::testing:
       res.mDemuxer = new MP3TrackDemuxer(res.mResource);
       mTargets.push_back(res);
 
       streamRes.mResource = new MockMP3StreamMediaResource(streamRes.mFilePath);
       streamRes.mDemuxer = new MP3TrackDemuxer(streamRes.mResource);
       mTargets.push_back(streamRes);
     }
 
-    {
-      MP3Resource res;
-      res.mFilePath = "test_vbri.mp3";
-      res.mIsVBR = true;
-      res.mHeaderType = MP3Resource::VBRI;
-      res.mFileSize = 16519;
-      res.mMPEGLayer = 3;
-      res.mMPEGVersion = 1;
-      res.mID3MajorVersion = 3;
-      res.mID3MinorVersion = 0;
-      res.mID3Flags = 0;
-      res.mID3Size = 4202;
-      res.mDuration = 783660;
-      res.mDurationError = 0.01f;
-      res.mSeekError = 0.02f;
-      res.mSampleRate = 44100;
-      res.mSamplesPerFrame = 1152;
-      res.mNumSamples = 29;
-      res.mNumTrailingFrames = 0;
-      res.mBitrate = 0;
-      res.mSlotSize = 1;
-      res.mPrivate = 0;
-      const int syncs[] = {4212, 4734, 5047, 5464, 5986, 6403};
-      res.mSyncOffsets.insert(res.mSyncOffsets.begin(), syncs, syncs + 6);
-
-      // VBR stream resources contain header info on total frames numbers, which
-      // is used to estimate the total duration.
-      MP3Resource streamRes = res;
-      streamRes.mFileSize = -1;
-
-      res.mResource = new MockMP3MediaResource(res.mFilePath);
-      res.mDemuxer = new MP3TrackDemuxer(res.mResource);
-      mTargets.push_back(res);
-
-      streamRes.mResource = new MockMP3StreamMediaResource(streamRes.mFilePath);
-      streamRes.mDemuxer = new MP3TrackDemuxer(streamRes.mResource);
-      mTargets.push_back(streamRes);
-    }
-
     for (auto& target : mTargets) {
       ASSERT_EQ(NS_OK, target.mResource->Open());
       ASSERT_TRUE(target.mDemuxer->Init());
     }
   }
 
   std::vector<MP3Resource> mTargets;
 };
@@ -390,25 +342,22 @@ TEST_F(MP3DemuxerTest, ID3Tags) {
 
 TEST_F(MP3DemuxerTest, VBRHeader) {
   for (const auto& target : mTargets) {
     RefPtr<MediaRawData> frame(target.mDemuxer->DemuxSample());
     ASSERT_TRUE(frame);
 
     const auto& vbr = target.mDemuxer->VBRInfo();
 
-    if (target.mHeaderType == MP3Resource::XING) {
+    if (target.mIsVBR) {
       EXPECT_EQ(FrameParser::VBRHeader::XING, vbr.Type());
       // TODO: find reference number which accounts for trailing headers.
       // EXPECT_EQ(target.mNumSamples / target.mSamplesPerFrame,
       // vbr.NumAudioFrames().value());
-    } else if (target.mHeaderType == MP3Resource::VBRI) {
-      EXPECT_TRUE(target.mIsVBR);
-      EXPECT_EQ(FrameParser::VBRHeader::VBRI, vbr.Type());
-    } else {  // MP3Resource::NONE
+    } else {
       EXPECT_EQ(FrameParser::VBRHeader::NONE, vbr.Type());
       EXPECT_FALSE(vbr.NumAudioFrames());
     }
   }
 }
 
 TEST_F(MP3DemuxerTest, FrameParsing) {
   for (const auto& target : mTargets) {
--- a/dom/media/gtest/moz.build
+++ b/dom/media/gtest/moz.build
@@ -18,17 +18,16 @@ UNIFIED_SOURCES += [
     'TestAudioBuffers.cpp',
     'TestAudioCompactor.cpp',
     'TestAudioMixer.cpp',
     'TestAudioPacketizer.cpp',
     'TestAudioSegment.cpp',
     'TestAudioTrackEncoder.cpp',
     'TestBitWriter.cpp',
     'TestBlankVideoDataCreator.cpp',
-    'TestBufferReader.cpp',
     'TestCDMStorage.cpp',
     'TestDataMutex.cpp',
     'TestGMPCrossOrigin.cpp',
     'TestGMPRemoveAndDelete.cpp',
     'TestGMPUtils.cpp',
     'TestGroupId.cpp',
     'TestIntervalSet.cpp',
     'TestMediaDataDecoder.cpp',
@@ -69,17 +68,16 @@ TEST_HARNESS_FILES.gtest += [
     'short-zero-inband.mov',
     'small-shot-false-positive.mp3',
     'small-shot-partial-xing.mp3',
     'small-shot.mp3',
     'test.webm',
     'test_case_1224361.vp8.ivf',
     'test_case_1224363.vp8.ivf',
     'test_case_1224369.vp8.ivf',
-    'test_vbri.mp3',
 ]
 
 TEST_DIRS += [
     'mp4_demuxer',
 ]
 
 include('/ipc/chromium/chromium-config.mozbuild')
 
deleted file mode 100644
index efd74503385c9d8c6afaf59a8923cf67ebd71805..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
GIT binary patch
literal 0
Hc$@<O00001
--- a/dom/media/mp3/MP3Demuxer.cpp
+++ b/dom/media/mp3/MP3Demuxer.cpp
@@ -292,59 +292,54 @@ MP3TrackDemuxer::SkipToNextRandomAccessP
   return SkipAccessPointPromise::CreateAndReject(
       SkipFailureHolder(NS_ERROR_DOM_MEDIA_DEMUXER_ERR, 0), __func__);
 }
 
 int64_t MP3TrackDemuxer::GetResourceOffset() const { return mOffset; }
 
 TimeIntervals MP3TrackDemuxer::GetBuffered() {
   AutoPinned<MediaResource> stream(mSource.GetResource());
-  TimeIntervals duration;
-  duration += TimeInterval(TimeUnit(), Duration());
+  TimeIntervals buffered;
 
   if (Duration() > TimeUnit() && stream->IsDataCachedToEndOfResource(0)) {
     // Special case completely cached files. This also handles local files.
+    buffered += TimeInterval(TimeUnit(), Duration());
     MP3LOGV("buffered = [[%" PRId64 ", %" PRId64 "]]",
             TimeUnit().ToMicroseconds(), Duration().ToMicroseconds());
-    return duration;
+    return buffered;
   }
 
-  TimeIntervals buffered;
   MediaByteRangeSet ranges;
   nsresult rv = stream->GetCachedRanges(ranges);
   NS_ENSURE_SUCCESS(rv, buffered);
 
   for (const auto& range : ranges) {
     if (range.IsEmpty()) {
       continue;
     }
     TimeUnit start = Duration(FrameIndexFromOffset(range.mStart));
     TimeUnit end = Duration(FrameIndexFromOffset(range.mEnd));
     MP3LOGV("buffered += [%" PRId64 ", %" PRId64 "]", start.ToMicroseconds(),
             end.ToMicroseconds());
     buffered += TimeInterval(start, end);
   }
 
-  // If the number of frames presented in header is valid, the duration
-  // calculated from it should be the maximal duration.
-  return ValidNumAudioFrames().isSome() && buffered.GetEnd() > duration.GetEnd()
-             ? duration
-             : buffered;
+  return buffered;
 }
 
 int64_t MP3TrackDemuxer::StreamLength() const { return mSource.GetLength(); }
 
 TimeUnit MP3TrackDemuxer::Duration() const {
   if (!mNumParsedFrames) {
     return TimeUnit::FromMicroseconds(-1);
   }
 
   int64_t numFrames = 0;
-  const auto numAudioFrames = ValidNumAudioFrames();
-  if (numAudioFrames.isSome()) {
+  const auto numAudioFrames = mParser.VBRInfo().NumAudioFrames();
+  if (mParser.VBRInfo().IsValid() && numAudioFrames.valueOr(0) + 1 > 1) {
     // VBR headers don't include the VBR header frame.
     numFrames = numAudioFrames.value() + 1;
     return Duration(numFrames);
   }
 
   const int64_t streamLen = StreamLength();
   if (streamLen < 0) {  // Live streams.
     // Unknown length, we can't estimate duration.
@@ -728,19 +723,12 @@ double MP3TrackDemuxer::AverageFrameLeng
   const auto& vbr = mParser.VBRInfo();
   if (vbr.IsComplete() && vbr.NumAudioFrames().value() + 1) {
     return static_cast<double>(vbr.NumBytes().value()) /
            (vbr.NumAudioFrames().value() + 1);
   }
   return 0.0;
 }
 
-Maybe<uint32_t> MP3TrackDemuxer::ValidNumAudioFrames() const {
-  return mParser.VBRInfo().IsValid() &&
-                 mParser.VBRInfo().NumAudioFrames().valueOr(0) + 1 > 1
-             ? mParser.VBRInfo().NumAudioFrames()
-             : Maybe<uint32_t>();
-}
-
 }  // namespace mozilla
 
 #undef MP3LOG
 #undef MP3LOGV
--- a/dom/media/mp3/MP3Demuxer.h
+++ b/dom/media/mp3/MP3Demuxer.h
@@ -117,20 +117,16 @@ class MP3TrackDemuxer : public MediaTrac
 
   // Reads aSize bytes into aBuffer from the source starting at aOffset.
   // Returns the actual size read.
   int32_t Read(uint8_t* aBuffer, int64_t aOffset, int32_t aSize);
 
   // Returns the average frame length derived from the previously parsed frames.
   double AverageFrameLength() const;
 
-  // Returns the number of frames reported by the header if it's valid. Nothing
-  // otherwise.
-  Maybe<uint32_t> ValidNumAudioFrames() const;
-
   // The (hopefully) MPEG resource.
   MediaResourceIndex mSource;
 
   // MPEG frame parser used to detect frames and extract side info.
   FrameParser mParser;
 
   // Whether we've locked onto a valid sequence of frames or not.
   bool mFrameLock;
--- a/dom/media/mp3/MP3FrameParser.cpp
+++ b/dom/media/mp3/MP3FrameParser.cpp
@@ -8,17 +8,16 @@
 
 #include <algorithm>
 #include <inttypes.h>
 
 #include "mozilla/Assertions.h"
 #include "mozilla/EndianUtils.h"
 #include "mozilla/Pair.h"
 #include "mozilla/ResultExtensions.h"
-#include "mozilla/ScopeExit.h"
 #include "VideoUtils.h"
 
 extern mozilla::LazyLogModule gMediaDemuxerLog;
 #define MP3LOG(msg, ...) \
   MOZ_LOG(gMediaDemuxerLog, LogLevel::Debug, ("MP3Demuxer " msg, ##__VA_ARGS__))
 #define MP3LOGV(msg, ...)                      \
   MOZ_LOG(gMediaDemuxerLog, LogLevel::Verbose, \
           ("MP3Demuxer " msg, ##__VA_ARGS__))
@@ -78,17 +77,18 @@ Result<bool, nsresult> FrameParser::Pars
   MOZ_ASSERT(aReader && aBytesToSkip);
   *aBytesToSkip = 0;
 
   if (!mID3Parser.Header().Size() && !mFirstFrame.Length()) {
     // No MP3 frames have been parsed yet, look for ID3v2 headers at file begin.
     // ID3v1 tags may only be at file end.
     // TODO: should we try to read ID3 tags at end of file/mid-stream, too?
     const size_t prevReaderOffset = aReader->Offset();
-    const uint32_t tagSize = mID3Parser.Parse(aReader).unwrapOr(0);
+    uint32_t tagSize;
+    MOZ_TRY_VAR(tagSize, mID3Parser.Parse(aReader));
     if (!!tagSize) {
       // ID3 tag found, skip past it.
       const uint32_t skipSize = tagSize - ID3Parser::ID3Header::SIZE;
 
       if (skipSize > aReader->Remaining()) {
         // Skipping across the ID3v2 tag would take us past the end of the
         // buffer, therefore we return immediately and let the calling function
         // handle skipping the rest of the tag.
@@ -351,20 +351,17 @@ Result<bool, nsresult> FrameParser::VBRH
   enum Flags {
     NUM_FRAMES = 0x01,
     NUM_BYTES = 0x02,
     TOC = 0x04,
     VBR_SCALE = 0x08
   };
 
   MOZ_ASSERT(aReader);
-
-  // Seek backward to the original position before leaving this scope.
   const size_t prevReaderOffset = aReader->Offset();
-  auto scopeExit = MakeScopeExit([&] { aReader->Seek(prevReaderOffset); });
 
   // We have to search for the Xing header as its position can change.
   for (auto res = aReader->PeekU32();
        res.isOk() && res.unwrap() != XING_TAG && res.unwrap() != INFO_TAG;) {
     aReader->Read(1);
     res = aReader->PeekU32();
   }
 
@@ -400,16 +397,17 @@ Result<bool, nsresult> FrameParser::VBRH
     }
   }
   if (flags & VBR_SCALE) {
     uint32_t scale;
     MOZ_TRY_VAR(scale, aReader->ReadU32());
     mScale = Some(scale);
   }
 
+  aReader->Seek(prevReaderOffset);
   return mType == XING;
 }
 
 Result<bool, nsresult> FrameParser::VBRHeader::ParseVBRI(
     BufferReader* aReader) {
   static const uint32_t TAG = BigEndian::readUint32("VBRI");
   static const uint32_t OFFSET = 32 + FrameParser::FrameHeader::SIZE;
   static const uint32_t FRAME_COUNT_OFFSET = OFFSET + 14;
@@ -418,34 +416,33 @@ Result<bool, nsresult> FrameParser::VBRH
   MOZ_ASSERT(aReader);
   // ParseVBRI assumes that the ByteReader offset points to the beginning of a
   // frame, therefore as a simple check, we look for the presence of a frame
   // sync at that position.
   auto sync = aReader->PeekU16();
   if (sync.isOk()) {  // To avoid compiler complains 'set but unused'.
     MOZ_ASSERT((sync.unwrap() & 0xFFE0) == 0xFFE0);
   }
-
-  // Seek backward to the original position before leaving this scope.
   const size_t prevReaderOffset = aReader->Offset();
-  auto scopeExit = MakeScopeExit([&] { aReader->Seek(prevReaderOffset); });
 
   // VBRI have a fixed relative position, so let's check for it there.
   if (aReader->Remaining() > MIN_FRAME_SIZE) {
     aReader->Seek(prevReaderOffset + OFFSET);
     uint32_t tag, frames;
     MOZ_TRY_VAR(tag, aReader->ReadU32());
     if (tag == TAG) {
       aReader->Seek(prevReaderOffset + FRAME_COUNT_OFFSET);
       MOZ_TRY_VAR(frames, aReader->ReadU32());
       mNumAudioFrames = Some(frames);
       mType = VBRI;
+      aReader->Seek(prevReaderOffset);
       return true;
     }
   }
+  aReader->Seek(prevReaderOffset);
   return false;
 }
 
 bool FrameParser::VBRHeader::Parse(BufferReader* aReader) {
   auto res = MakePair(ParseVBRI(aReader), ParseXing(aReader));
   const bool rv = (res.first().isOk() && res.first().unwrap()) ||
                   (res.second().isOk() && res.second().unwrap());
   if (rv) {