Backed out 4 changesets (bug 1500713) on request of jcristau for causing Bug 1535603 a=Backout

Backed out changeset 5d0cad2c99a4 (bug 1500713)
Backed out changeset f96a12eedd89 (bug 1500713)
Backed out changeset 2633cea7d119 (bug 1500713)
Backed out changeset 49c4cfbf2bee (bug 1500713)

dom/media/gtest/TestBufferReader.cpp

@@ -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);
-}

dom/media/gtest/TestMP3Demuxer.cpp

@@ -47,11 +47,8 @@ class MockMP3StreamMediaResource
 };
 
 struct MP3Resource {
-  enum HeaderType { NONE, XING, VBRI };
-
   const char* mFilePath;
   bool mIsVBR;
-  HeaderType mHeaderType;
   int64_t mFileSize;
   int32_t mMPEGLayer;
   int32_t mMPEGVersion;
@@ -86,7 +83,6 @@ class MP3DemuxerTest : public ::testing::Test {
       MP3Resource res;
       res.mFilePath = "noise.mp3";
       res.mIsVBR = false;
-      res.mHeaderType = MP3Resource::NONE;
       res.mFileSize = 965257;
       res.mMPEGLayer = 3;
       res.mMPEGVersion = 1;
@@ -131,7 +127,6 @@ class MP3DemuxerTest : public ::testing::Test {
       // 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;
@@ -171,7 +166,6 @@ class MP3DemuxerTest : public ::testing::Test {
       MP3Resource res;
       res.mFilePath = "noise_vbr.mp3";
       res.mIsVBR = true;
-      res.mHeaderType = MP3Resource::XING;
       res.mFileSize = 583679;
       res.mMPEGLayer = 3;
       res.mMPEGVersion = 1;
@@ -210,7 +204,6 @@ class MP3DemuxerTest : public ::testing::Test {
       MP3Resource res;
       res.mFilePath = "small-shot.mp3";
       res.mIsVBR = true;
-      res.mHeaderType = MP3Resource::XING;
       res.mFileSize = 6825;
       res.mMPEGLayer = 3;
       res.mMPEGVersion = 1;
@@ -251,7 +244,6 @@ class MP3DemuxerTest : public ::testing::Test {
       // 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;
@@ -290,7 +282,6 @@ class MP3DemuxerTest : public ::testing::Test {
       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;
@@ -325,45 +316,6 @@ class MP3DemuxerTest : public ::testing::Test {
       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());
@@ -395,15 +347,12 @@ TEST_F(MP3DemuxerTest, VBRHeader) {
 
     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());
     }

dom/media/gtest/moz.build

@@ -23,7 +23,6 @@ UNIFIED_SOURCES += [
     'TestAudioTrackEncoder.cpp',
     'TestBitWriter.cpp',
     'TestBlankVideoDataCreator.cpp',
-    'TestBufferReader.cpp',
     'TestCDMStorage.cpp',
     'TestDataMutex.cpp',
     'TestGMPCrossOrigin.cpp',
@@ -74,7 +73,6 @@ TEST_HARNESS_FILES.gtest += [
     'test_case_1224361.vp8.ivf',
     'test_case_1224363.vp8.ivf',
     'test_case_1224369.vp8.ivf',
-    'test_vbri.mp3',
 ]
 
 TEST_DIRS += [

dom/media/mp3/MP3Demuxer.cpp

@@ -297,17 +297,16 @@ 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);
@@ -323,11 +322,7 @@ TimeIntervals MP3TrackDemuxer::GetBuffered() {
     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(); }
@@ -338,8 +333,8 @@ TimeUnit MP3TrackDemuxer::Duration() const {
   }
 
   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);
@@ -733,13 +728,6 @@ double MP3TrackDemuxer::AverageFrameLength() const {
   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

dom/media/mp3/MP3Demuxer.h

@@ -122,10 +122,6 @@ class MP3TrackDemuxer : public MediaTrackDemuxer,
   // 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;
 

dom/media/mp3/MP3FrameParser.cpp

@@ -13,7 +13,6 @@
 #include "mozilla/EndianUtils.h"
 #include "mozilla/Pair.h"
 #include "mozilla/ResultExtensions.h"
-#include "mozilla/ScopeExit.h"
 #include "VideoUtils.h"
 
 extern mozilla::LazyLogModule gMediaDemuxerLog;
@@ -83,7 +82,8 @@ Result<bool, nsresult> FrameParser::Parse(BufferReader* aReader,
     // 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;
@@ -356,10 +356,7 @@ Result<bool, nsresult> FrameParser::VBRHeader::ParseXing(
   };
 
   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();
@@ -405,6 +402,7 @@ Result<bool, nsresult> FrameParser::VBRHeader::ParseXing(
     mScale = Some(scale);
   }
 
+  aReader->Seek(prevReaderOffset);
   return mType == XING;
 }
 
@@ -423,10 +421,7 @@ Result<bool, nsresult> FrameParser::VBRHeader::ParseVBRI(
   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) {
@@ -438,9 +433,11 @@ Result<bool, nsresult> FrameParser::VBRHeader::ParseVBRI(
       MOZ_TRY_VAR(frames, aReader->ReadU32());
       mNumAudioFrames = Some(frames);
       mType = VBRI;
+      aReader->Seek(prevReaderOffset);
       return true;
     }
   }
+  aReader->Seek(prevReaderOffset);
   return false;
 }