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

--HG--
extra : rebase_source : 26c41758b1b2c87542bf4e41d08e361198ca5b13

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 @@ private:
         }
         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 @@ public:
     , 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,7 +48,6 @@ public:
 
   ~ByteReader()
   {
-    NS_ASSERTION(!mRemaining, "Not all bytes have been processed");
   }
 
   size_t Offset()
@@ -56,12 +55,6 @@ public:
     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 @@ private:
   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