Bug 1500713: P2 - Seek MP3 parser to the original position if it fails parsing. r=kinetik
☠☠ backed out by 89988d424c06 ☠ ☠
authorChun-Min Chang <chun.m.chang@gmail.com>
Sat, 09 Mar 2019 01:09:26 +0000
changeset 521481 670b24af24d4
parent 521480 7e5fa7b1f7bc
child 521482 98265537ef34
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)
reviewerskinetik
bugs1500713
milestone67.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 1500713: P2 - Seek MP3 parser to the original position if it fails parsing. r=kinetik Depends on D19096 Differential Revision: https://phabricator.services.mozilla.com/D19097
dom/media/mp3/MP3FrameParser.cpp
--- a/dom/media/mp3/MP3FrameParser.cpp
+++ b/dom/media/mp3/MP3FrameParser.cpp
@@ -8,16 +8,17 @@
 
 #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__))
@@ -77,18 +78,17 @@ 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();
-    uint32_t tagSize;
-    MOZ_TRY_VAR(tagSize, mID3Parser.Parse(aReader));
+    const uint32_t tagSize = mID3Parser.Parse(aReader).unwrapOr(0);
     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,17 +351,20 @@ 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();
   }
 
@@ -397,17 +400,16 @@ 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;
@@ -416,33 +418,34 @@ 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) {