fix: MP4 parser is more robust (#55)

* add a file cursor so we can track the position of the buffer in the file

* add a cursor to track the position in the file

* change the doc of the ID3v2

* use buffer and fix the data boxes

* a bit of doc to explain the change

* remove print

Author: ClementBeal

lib/src/parsers/id3v2.dart

@@ -290,20 +290,18 @@ class ID3v2Parser extends TagParser {
     return metadata;
   }
 
-  /**
-   * Search and return the first MP3 frame header.
-   * Returns null if none has been found.
-   * 
-   * The MP3 frame has a magic word : 0xFFF or 0xFFE
-   * 
-   * Sometimes the MP3 files contains blocks of 0x00 or 0xFF and relying on the magic word 
-   * is not reliable anymore. 
-   * 
-   * To prevent false positives, we need to verify that the bytes after the potential
-   * valid word are correct. The MP3 specs specify several flags that must be set or not.
-   * 
-   * Credit to [exiftool](https://github.com/exiftool/exiftool/blob/master/lib/Image/ExifTool/MPEG.pm#L464)
-   */
+  /// Search and return the first MP3 frame header.
+  /// Returns null if none has been found.
+  ///
+  /// The MP3 frame has a magic word : 0xFFF or 0xFFE
+  ///
+  /// Sometimes the MP3 files contains blocks of 0x00 or 0xFF and relying on the magic word
+  /// is not reliable anymore.
+  ///
+  /// To prevent false positives, we need to verify that the bytes after the potential
+  /// valid word are correct. The MP3 specs specify several flags that must be set or not.
+  ///
+  /// Credit to [exiftool](https://github.com/exiftool/exiftool/blob/master/lib/Image/ExifTool/MPEG.pm#L464)
   Uint8List? _findFirstMp3Frame(Buffer buffer) {
     Uint8List frameHeader = buffer.readAtMost(4);
 

lib/src/parsers/mp4.dart

@@ -4,6 +4,7 @@ import 'dart:typed_data';
 
 import 'package:audio_metadata_reader/src/metadata/mp4_metadata.dart';
 import 'package:audio_metadata_reader/src/parsers/tag_parser.dart';
+import 'package:audio_metadata_reader/src/utils/buffer.dart';
 import 'package:mime/mime.dart';
 
 import '../../audio_metadata_reader.dart';
@@ -68,24 +69,26 @@ final supportedBox = [
 ///
 class MP4Parser extends TagParser {
   Mp4Metadata tags = Mp4Metadata();
+  late final Buffer buffer;
 
   MP4Parser({fetchImage = false}) : super(fetchImage: fetchImage);
 
   @override
   ParserTag parse(RandomAccessFile reader) {
     reader.setPositionSync(0);
+    buffer = Buffer(randomAccessFile: reader);
 
     final lengthFile = reader.lengthSync();
 
-    while (reader.positionSync() < lengthFile) {
-      final box = _readBox(reader);
+    while (buffer.fileCursor < lengthFile) {
+      final box = _readBox(buffer);
 
       if (supportedBox.contains(box.type)) {
-        processBox(reader, box);
+        processBox(buffer, box);
       } else {
         // We substract 8 to the box size because we already read the data for
         // the box header
-        reader.setPositionSync(reader.positionSync() + box.size - 8);
+        buffer.skip(box.size - 8);
       }
     }
 
@@ -100,8 +103,8 @@ class MP4Parser extends TagParser {
   /// [0...3] -> box size (header + body)
   /// [4...7] -> box name (ASCII)
   ///
-  BoxHeader _readBox(RandomAccessFile reader) {
-    final headerBytes = reader.readSync(8);
+  BoxHeader _readBox(Buffer buffer) {
+    final headerBytes = buffer.read(8);
     final parser = ByteData.sublistView(headerBytes);
 
     final boxSize = parser.getUint32(0);
@@ -119,43 +122,48 @@ class MP4Parser extends TagParser {
     return BoxHeader(boxSize, String.fromCharCodes(boxNameBytes));
   }
 
-  ///
   /// Parse a box
   ///
   /// The metadata are inside special boxes. We only read data when we need it
   /// otherwise we skip them
-  ///
-  void processBox(RandomAccessFile reader, BoxHeader box) {
+  void processBox(Buffer buffer, BoxHeader box) {
     if (box.type == "moov") {
-      parseRecurvise(reader, box);
+      parseRecurvise(buffer, box);
     } else if (box.type == "mvhd") {
-      final bytes = reader.readSync(100);
+      final bytes = buffer.read(100);
 
       final timeScale = getUint32(bytes.sublist(12, 16));
       final timeUnit = getUint32(bytes.sublist(16, 20));
 
       double microseconds = (timeUnit / timeScale) * 1000000;
       tags.duration = Duration(microseconds: microseconds.toInt());
     } else if (box.type == "udta") {
-      parseRecurvise(reader, box);
+      parseRecurvise(buffer, box);
     } else if (box.type == "ilst") {
-      parseRecurvise(reader, box);
+      parseRecurvise(buffer, box);
     } else if (["trak", "mdia", "minf", "stbl", "stsd"].contains(box.type)) {
-      parseRecurvise(reader, box);
+      parseRecurvise(buffer, box);
     } else if (box.type == "meta") {
-      reader.readSync(4);
+      buffer.read(4);
 
-      parseRecurvise(reader, box);
+      parseRecurvise(buffer, box);
     } else if (box.type[0] == "©" ||
         ["gnre", "trkn", "disk", "tmpo", "cpil", "too", "covr", "pgap", "gen"]
             .contains(box.type)) {
-      final bytes = reader.readSync(box.size - 8);
+      final metadataValue = buffer.read(box.size - 8);
+
+      // sometimes the data is stored inside another box called `data`
+      // we try to find out if the data contains the box type "data" (0:4 is the box size)
+      // otherwise we just skip the Apple's tag of 4 chars
+      final data = (String.fromCharCodes(metadataValue.sublist(4, 8)) == "data")
+          ? metadataValue.sublist(16)
+          : metadataValue.sublist(4);
 
       String value;
       try {
-        value = const Utf8Decoder().convert(bytes.sublist(16));
+        value = utf8.decode(data);
       } catch (e) {
-        value = String.fromCharCodes(bytes.sublist(16));
+        value = latin1.decode(data);
       }
 
       final boxName = (box.type[0] == "©") ? box.type.substring(1) : box.type;
@@ -190,37 +198,37 @@ class MP4Parser extends TagParser {
         case "too":
           break;
         case "disk":
-          tags.discNumber = getUint16(bytes.sublist(18, 20));
-          tags.totalDiscs = getUint16(bytes.sublist(20, 22));
+          tags.discNumber = getUint16(data.sublist(2, 4));
+          tags.totalDiscs = getUint16(data.sublist(4, 6));
           break;
 
         case "covr":
           if (fetchImage) {
-            final imageData = bytes.sublist(16);
+            final imageData = data;
             tags.picture = Picture(
                 imageData,
                 lookupMimeType("no path", headerBytes: imageData) ?? "",
                 PictureType.coverFront);
           }
         case "trkn":
-          final a = getUint16(bytes.sublist(18, 20));
-          final totalTracks = getUint16(bytes.sublist(20, 22));
+          final a = getUint16(data.sublist(2, 4));
+          final totalTracks = getUint16(data.sublist(4, 6));
           tags.totalTracks = totalTracks;
           if (a > 0) {
             tags.trackNumber = a;
           }
           break;
       }
     } else if (box.type == "----") {
-      final mean = _readBox(reader);
-      String.fromCharCodes(reader.readSync(mean.size - 8)); // mean value
+      final mean = _readBox(buffer);
+      String.fromCharCodes(buffer.read(mean.size - 8)); // mean value
 
-      final name = _readBox(reader);
+      final name = _readBox(buffer);
 
       final nameValue =
-          String.fromCharCodes(reader.readSync(name.size - 8).sublist(4));
-      final dataBox = _readBox(reader);
-      final data = reader.readSync(dataBox.size - 8);
+          String.fromCharCodes(buffer.read(name.size - 8).sublist(4));
+      final dataBox = _readBox(buffer);
+      final data = buffer.read(dataBox.size - 8);
       final finalValue = String.fromCharCodes(data.sublist(8));
 
       switch (nameValue) {
@@ -233,19 +241,17 @@ class MP4Parser extends TagParser {
         default:
       }
     } else if (box.type == "mp4a") {
-      final bytes = reader.readSync(box.size - 8);
+      final bytes = buffer.read(box.size - 8);
 
       // tags.bitrate = timeScale;
       tags.sampleRate = getUint32(bytes.sublist(22, 26));
     } else {
-      reader.setPositionSync(reader.positionSync() + box.size - 8);
+      buffer.setPositionSync(buffer.fileCursor + box.size - 8);
     }
   }
 
-  ///
   /// Parse a box with multiple sub boxes.
-  ///
-  void parseRecurvise(RandomAccessFile reader, BoxHeader box) {
+  void parseRecurvise(Buffer buffer, BoxHeader box) {
     final limit = box.size - 8;
     int offset = 0;
 
@@ -254,26 +260,24 @@ class MP4Parser extends TagParser {
       offset += 4;
     } else if (box.type == "stsd") {
       offset += 8;
-      reader.readSync(8);
+      buffer.read(8);
     }
 
     while (offset < limit) {
-      final newBox = _readBox(reader);
+      final newBox = _readBox(buffer);
 
       if (supportedBox.contains(newBox.type)) {
-        processBox(reader, newBox);
+        processBox(buffer, newBox);
       } else {
-        reader.setPositionSync(reader.positionSync() + newBox.size - 8);
+        buffer.skip(newBox.size - 8);
       }
 
       offset += newBox.size;
     }
   }
 
-  ///
   /// To detect if this parser can be used to parse this file, we need to detect
   /// the first box. It should be a `ftyp` box
-  ///
   static bool canUserParser(RandomAccessFile reader) {
     reader.setPositionSync(4);
 

lib/src/utils/buffer.dart

@@ -7,8 +7,18 @@ import '../../audio_metadata_reader.dart';
 class Buffer {
   final RandomAccessFile randomAccessFile;
   final Uint8List _buffer;
+
+  /// It's like positionSync() but adapted with the buffer
+  int fileCursor = 0;
+
+  /// Position of the cursor in the buffer of size [_bufferSize]
   int _cursor = 0;
-  int _bufferedBytes = 0; // Track how many bytes are actually in the buffer
+
+  /// Track how many bytes are actually in the buffer
+  int _bufferedBytes = 0;
+
+  /// The buffer size is always a power of 2.
+  /// To reach good performance, we need at least 4096
   static final int _bufferSize = 16384;
 
   /// The number of bytes remaining to be read from the file.
@@ -39,6 +49,8 @@ class Buffer {
   }
 
   Uint8List read(int size) {
+    fileCursor += size;
+
     // if we read something big (~100kb), we can read it directly from file
     // it makes the read faster
     // no need to use the buffer
@@ -101,6 +113,7 @@ class Buffer {
   }
 
   void setPositionSync(int position) {
+    fileCursor = position;
     randomAccessFile.setPositionSync(position);
     _fill();
   }
@@ -110,11 +123,14 @@ class Buffer {
     final remainingInBuffer = _bufferedBytes - _cursor;
 
     if (length <= remainingInBuffer) {
+      fileCursor += length;
+
       // If we can skip within the current buffer, just move the cursor
       _cursor += length;
     } else {
       // Calculate the actual file position we need to skip to
       int currentPosition = randomAccessFile.positionSync() - remainingInBuffer;
+      fileCursor += currentPosition;
       // Skip to the new position
       randomAccessFile.setPositionSync(currentPosition + length);
       // Refill the buffer at the new position

test/mp4/mp4_test.dart

@@ -44,8 +44,17 @@ void main() {
     expect(result.title, "Sexy Ladies (Remix) [feat. 50 Cent]");
     expect(result.album, "FutureSex/LoveSounds (Deluxe Edition)");
     expect(result.artist, "Justin Timberlake");
-    expect(result.year, (DateTime.utc(2006, 9, 12)));
+    expect(result.year, DateTime.utc(2006, 9, 12));
     expect(result.trackNumber, 15);
     expect(result.trackTotal, 15);
   });
+
+  test("Should work with .mov file", () {
+    final track = File('./test/mp4/track.mov');
+    final result = readMetadata(track, getImage: false);
+
+    expect(result.title, "Blue Test Pattern");
+    expect(result.artist, "FFmpeg Generator");
+    expect(result.year, (DateTime(2023, 10, 27)));
+  });
 }