Bug 1406503 - P5. Do not ignore small flac packets. r=jwwang
authorJean-Yves Avenard <jyavenard@mozilla.com>
Sat, 28 Oct 2017 11:11:45 +0200
changeset 389041 f96ce9a8172ce7d489d78b6cf237afb1d6a851f7
parent 389040 6a20de1a556f868360ac4c55d38efad5cdb82bf7
child 389042 1b2815b0c7f728b0cba209ccfa58b1dcb60a36d2
push id32777
push userarchaeopteryx@coole-files.de
push dateMon, 30 Oct 2017 22:44:45 +0000
treeherdermozilla-central@dd0f265a1300 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwwang
bugs1406503
milestone58.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 1406503 - P5. Do not ignore small flac packets. r=jwwang A FLAC packet may have a size as small as 11 bytes. We should parse as much data as we can and not make assumptions on when to stop early. MozReview-Commit-ID: 9skvwbt15MY
dom/media/flac/FlacDemuxer.cpp
--- a/dom/media/flac/FlacDemuxer.cpp
+++ b/dom/media/flac/FlacDemuxer.cpp
@@ -39,21 +39,19 @@ public:
 
   // Return the index (in samples) from the beginning of the track.
   int64_t Index() const { return mIndex; }
 
   // Parse the current packet and check that it made a valid flac frame header.
   // From https://xiph.org/flac/format.html#frame_header
   // A valid header is one that can be decoded without error and that has a
   // valid CRC.
-  // aPacket must points to a buffer that is at least FLAC_MAX_FRAME_HEADER_SIZE
-  // bytes.
-  bool Parse(const uint8_t* aPacket)
+  bool Parse(const uint8_t* aPacket, size_t aBytes)
   {
-    mp4_demuxer::BitReader br(aPacket, FLAC_MAX_FRAME_HEADER_SIZE * 8);
+    mp4_demuxer::BitReader br(aPacket, aBytes * 8);
 
     // Frame sync code.
     if ((br.ReadBits(15) & 0x7fff) != 0x7ffc) {
       return false;
     }
 
     // Variable block size stream code.
     mVariableBlockSize = br.ReadBit();
@@ -225,37 +223,42 @@ const uint8_t FrameHeader::CRC8Table[256
 class Frame
 {
 public:
 
   // The FLAC signature is made of 14 bits set to 1; however the 15th bit is
   // mandatorily set to 0, so we need to find either of 0xfffc or 0xfffd 2-bytes
   // signature. We first use a bitmask to see if 0xfc or 0xfd is present. And if
   // so we check for the whole signature.
-  // aData must be pointing to a buffer at least
-  // aLength + FLAC_MAX_FRAME_HEADER_SIZE bytes.
   int64_t FindNext(const uint8_t* aData, const uint32_t aLength)
   {
+    // The non-variable size of a FLAC header is 32 bits followed by variable
+    // size data and a 8 bits CRC.
+    // There's no need to read the last 4 bytes, it can never make a complete
+    // header.
+    if (aLength < 4) {
+      return -1;
+    }
     uint32_t modOffset = aLength % 4;
     uint32_t i, j;
 
     for (i = 0; i < modOffset; i++) {
       if ((BigEndian::readUint16(aData + i) & 0xfffe) == 0xfff8) {
-        if (mHeader.Parse(aData + i)) {
+        if (mHeader.Parse(aData + i, aLength - i)) {
           return i;
         }
       }
     }
 
-    for (; i < aLength; i += 4) {
+    for (; i < aLength - 4; i += 4) {
       uint32_t x = BigEndian::readUint32(aData + i);
       if (((x & ~(x + 0x01010101)) & 0x80808080)) {
         for (j = 0; j < 4; j++) {
           if ((BigEndian::readUint16(aData + i + j) & 0xfffe) == 0xfff8) {
-            if (mHeader.Parse(aData + i + j)) {
+            if (mHeader.Parse(aData + i + j, aLength - i - j)) {
               return i + j;
             }
           }
         }
       }
     }
     return -1;
   }
@@ -277,36 +280,38 @@ public:
       uint32_t read = 0;
       buffer.SetLength(BUFFER_SIZE + innerOffset);
       nsresult rv =
         aResource.Read(buffer.Elements() + innerOffset, BUFFER_SIZE, &read);
       if (NS_FAILED(rv)) {
         return false;
       }
 
-      if (read < FLAC_MAX_FRAME_HEADER_SIZE) {
-        // Assume that we can't have a valid frame in such small content, we
-        // must have reached EOS.
-        // So we're done.
-        mEOS = true;
-        return false;
-      }
-
-      const size_t bufSize = read + innerOffset - FLAC_MAX_FRAME_HEADER_SIZE;
+      const size_t bufSize = read + innerOffset;
       int64_t foundOffset =
         FindNext(reinterpret_cast<uint8_t*>(buffer.Elements()), bufSize);
 
       if (foundOffset >= 0) {
         SetOffset(aResource, foundOffset + offset);
         return true;
       }
 
+      if (read < BUFFER_SIZE) {
+        // Nothing more to try on as we had reached EOS during the previous
+        // read.
+        mEOS = true;
+        return false;
+      }
+
       // Scan the next block;
-      offset += bufSize;
-      buffer.RemoveElementsAt(0, bufSize);
+      // We rewind a bit to re-try what could have been an incomplete packet.
+      // The maximum size of a FLAC header being FLAC_MAX_FRAME_HEADER_SIZE so
+      // we need to retry just after that amount.
+      offset += bufSize - (FLAC_MAX_FRAME_HEADER_SIZE + 1);
+      buffer.RemoveElementsAt(0, bufSize - (FLAC_MAX_FRAME_HEADER_SIZE + 1));
       innerOffset = buffer.Length();
     } while (offset - originalOffset < FLAC_MAX_FRAME_SIZE);
 
     return false;
   }
 
   int64_t Offset() const { return mOffset; }
 
@@ -1041,17 +1046,17 @@ FlacTrackDemuxer::TimeAtEnd()
   mTotalFrameLen = streamLen - mParser->FirstFrame().Offset();
 
   return mParsedFramesDuration;
 }
 
 /* static */ bool
 FlacDemuxer::FlacSniffer(const uint8_t* aData, const uint32_t aLength)
 {
-  if (aLength < FLAC_MAX_FRAME_HEADER_SIZE) {
+  if (aLength < FLAC_MIN_FRAME_SIZE) {
     return false;
   }
 
   flac::Frame frame;
-  return frame.FindNext(aData, aLength - FLAC_MAX_FRAME_HEADER_SIZE) >= 0;
+  return frame.FindNext(aData, aLength) >= 0;
 }
 
 } // namespace mozilla