diff --git a/dom/media/gtest/TestBufferReader.cpp b/dom/media/gtest/TestBufferReader.cpp deleted file mode 100644 index b2d5f7ae5a9c..000000000000 --- 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(buffer); - const uint8_t* const TAIL = HEAD + BUFFER_SIZE; - - BufferReader reader(HEAD, BUFFER_SIZE); - ASSERT_EQ(reader.Offset(), static_cast(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(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 diff --git a/dom/media/gtest/TestMP3Demuxer.cpp b/dom/media/gtest/TestMP3Demuxer.cpp index c76790cc70bd..e480e7efedf0 100644 --- a/dom/media/gtest/TestMP3Demuxer.cpp +++ b/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()); } diff --git a/dom/media/gtest/moz.build b/dom/media/gtest/moz.build index 4e052cbf5505..0c597cbfc847 100644 --- a/dom/media/gtest/moz.build +++ b/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 += [ diff --git a/dom/media/gtest/test_vbri.mp3 b/dom/media/gtest/test_vbri.mp3 deleted file mode 100644 index efd74503385c..000000000000 Binary files a/dom/media/gtest/test_vbri.mp3 and /dev/null differ diff --git a/dom/media/mp3/MP3Demuxer.cpp b/dom/media/mp3/MP3Demuxer.cpp index 43d1d0f5b9ee..194a7eb132e4 100644 --- a/dom/media/mp3/MP3Demuxer.cpp +++ b/dom/media/mp3/MP3Demuxer.cpp @@ -297,17 +297,16 @@ int64_t MP3TrackDemuxer::GetResourceOffset() const { return mOffset; } TimeIntervals MP3TrackDemuxer::GetBuffered() { AutoPinned 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 MP3TrackDemuxer::ValidNumAudioFrames() const { - return mParser.VBRInfo().IsValid() && - mParser.VBRInfo().NumAudioFrames().valueOr(0) + 1 > 1 - ? mParser.VBRInfo().NumAudioFrames() - : Maybe(); -} - } // namespace mozilla #undef MP3LOG diff --git a/dom/media/mp3/MP3Demuxer.h b/dom/media/mp3/MP3Demuxer.h index 8aac86f97bc3..2fdbe9370280 100644 --- a/dom/media/mp3/MP3Demuxer.h +++ b/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 ValidNumAudioFrames() const; - // The (hopefully) MPEG resource. MediaResourceIndex mSource; diff --git a/dom/media/mp3/MP3FrameParser.cpp b/dom/media/mp3/MP3FrameParser.cpp index 8310d8f4dba1..a77b960a17db 100644 --- a/dom/media/mp3/MP3FrameParser.cpp +++ b/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 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 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 FrameParser::VBRHeader::ParseXing( mScale = Some(scale); } + aReader->Seek(prevReaderOffset); return mType == XING; } @@ -423,10 +421,7 @@ Result 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 FrameParser::VBRHeader::ParseVBRI( MOZ_TRY_VAR(frames, aReader->ReadU32()); mNumAudioFrames = Some(frames); mType = VBRI; + aReader->Seek(prevReaderOffset); return true; } } + aReader->Seek(prevReaderOffset); return false; }