Bug 1298710 - Remove ByteReader::DiscardRemaining and AutoByteReader …

…- r=jya

DiscardRemaning was needed to prevent debug-time assertion that the buffer was
read completely or explicitly discarded.

However this required extra work in cases where buffer didn't need to be read
to the end.
And also it could cause crashes (in debug versions) if a buffer was not fully
read, be it because the parser was incorrect or because the media file itself
was wrong (though possibly still readable despite that).
Finding parser issues is still possible by manually instrumenting ByteReader
during development.
And reading media file with small recoverable errors is a bonus.

MozReview-Commit-ID: 2RUYzaYAeRW

dom/media/MP3Demuxer.cpp

@@ -493,7 +493,6 @@ MP3TrackDemuxer::FindNextFrame() {
     // If we've found neither an MPEG frame header nor an ID3v2 tag,
     // the reader shouldn't have any bytes remaining.
     MOZ_ASSERT(foundFrame || bytesToSkip || !reader.Remaining());
-    reader.DiscardRemaining();
 
     if (foundFrame && mParser.FirstFrame().Length() &&
         !VerifyFrameConsistency(mParser.FirstFrame(), mParser.CurrentFrame())) {
@@ -584,7 +583,6 @@ MP3TrackDemuxer::GetNextFrame(const MediaByteRange& aRange) {
     // First frame parsed, let's read VBR info if available.
     ByteReader reader(frame->Data(), frame->Size());
     mParser.ParseVBRHeader(&reader);
-    reader.DiscardRemaining();
     mFirstFrameOffset = frame->mOffset;
   }
 

dom/media/flac/FlacFrameParser.cpp

@@ -10,7 +10,7 @@
 #include "OggCodecState.h"
 #include "VideoUtils.h"
 
-using mp4_demuxer::AutoByteReader;
+using mp4_demuxer::ByteReader;
 
 namespace mozilla
 {
@@ -62,7 +62,7 @@ FlacFrameParser::DecodeHeaderBlock(const uint8_t* aPacket, size_t aLength)
     // Not a header block.
     return false;
   }
-  AutoByteReader br(aPacket, aLength);
+  ByteReader br(aPacket, aLength);
 
   mPacketCount++;
 
@@ -210,11 +210,11 @@ FlacFrameParser::IsHeaderBlock(const uint8_t* aPacket, size_t aLength) const
   }
   if (aPacket[0] == 0x7f) {
     // Ogg packet
-    AutoByteReader br(aPacket + 1, aLength - 1);
+    ByteReader br(aPacket + 1, aLength - 1);
     const uint8_t* signature = br.Read(4);
     return signature && !memcmp(signature, "FLAC", 4);
   }
-  AutoByteReader br(aPacket, aLength - 1);
+  ByteReader br(aPacket, aLength - 1);
   const uint8_t* signature = br.Read(4);
   if (signature && !memcmp(signature, "fLaC", 4)) {
     // Flac start header, must have STREAMINFO as first metadata block;

dom/media/mediasource/ContainerParser.cpp

@@ -399,7 +399,6 @@ class MP4ContainerParser : public ContainerParser {
         }
         reader.Read(size - 8);
       }
-      reader.DiscardRemaining();
     }
 
     bool StartWithInitSegment()

dom/media/platforms/agnostic/WAVDecoder.cpp

@@ -118,8 +118,6 @@ WaveDataDecoder::DoDecode(MediaRawData* aSample)
     }
   }
 
-  aReader.DiscardRemaining();
-
   int64_t duration = frames / mInfo.mRate;
 
   mCallback->Output(new AudioData(aOffset,

media/libstagefright/binding/AnnexB.cpp

@@ -99,7 +99,6 @@ AnnexB::ConvertExtraDataToAnnexB(const mozilla::MediaByteBuffer* aExtraData)
 
     // MP4Box adds extra bytes that we ignore. I don't know what they do.
   }
-  reader.DiscardRemaining();
 
   return annexB.forget();
 }
@@ -330,7 +329,6 @@ AnnexB::HasSPS(const mozilla::MediaByteBuffer* aExtraData)
     return false;
   }
   uint8_t numSps = reader.ReadU8() & 0x1f;
-  reader.DiscardRemaining();
 
   return numSps > 0;
 }

media/libstagefright/binding/H264.cpp

@@ -422,14 +422,12 @@ H264::DecodeSPSFromExtraData(const mozilla::MediaByteBuffer* aExtraData, SPSData
 
   if (!(reader.ReadU8() & 0x1f)) {
     // No SPS.
-    reader.DiscardRemaining();
     return false;
   }
   uint16_t length = reader.ReadU16();
 
   if ((reader.PeekU8() & 0x1f) != 7) {
     // Not a SPS NAL type.
-    reader.DiscardRemaining();
     return false;
   }
 
@@ -438,8 +436,6 @@ H264::DecodeSPSFromExtraData(const mozilla::MediaByteBuffer* aExtraData, SPSData
     return false;
   }
 
-  reader.DiscardRemaining();
-
   RefPtr<mozilla::MediaByteBuffer> rawNAL = new mozilla::MediaByteBuffer;
   rawNAL->AppendElements(ptr, length);
 

media/libstagefright/binding/include/mp4_demuxer/Box.h

@@ -73,11 +73,11 @@ class BoxReader
     , mReader(mBuffer.Elements(), mBuffer.Length())
   {
   }
-  AutoByteReader* operator->() { return &mReader; }
+  ByteReader* operator->() { return &mReader; }
 
 private:
   nsTArray<uint8_t> mBuffer;
-  AutoByteReader mReader;
+  ByteReader mReader;
 };
 }
 

media/libstagefright/binding/include/mp4_demuxer/ByteReader.h

@@ -48,20 +48,13 @@ class MOZ_RAII ByteReader
 
   ~ByteReader()
   {
-    NS_ASSERTION(!mRemaining, "Not all bytes have been processed");
   }
 
   size_t Offset()
   {
     return mLength - mRemaining;
   }
 
-  // Make it explicit if we're not using the extra bytes.
-  void DiscardRemaining()
-  {
-    mRemaining = 0;
-  }
-
   size_t Remaining() const { return mRemaining; }
 
   bool CanRead8() { return mRemaining >= 1; }
@@ -351,23 +344,6 @@ class MOZ_RAII ByteReader
   size_t mLength;
 };
 
-// A ByteReader that automatically discards remaining data before destruction.
-class MOZ_RAII AutoByteReader : public ByteReader
-{
-public:
-  AutoByteReader(const uint8_t* aData, size_t aSize)
-    : ByteReader(aData, aSize)
-  {
-  }
-  ~AutoByteReader()
-  {
-    ByteReader::DiscardRemaining();
-  }
-
-  // Prevent unneeded DiscardRemaining() calls.
-  void DiscardRemaining() = delete;
-};
-
 } // namespace mp4_demuxer
 
 #endif