Further enhance ID3 decoder + support

library/src/main/java/com/google/android/exoplayer2/extractor/GaplessInfoHolder.java

@@ -16,6 +16,8 @@
 package com.google.android.exoplayer2.extractor;
 
 import com.google.android.exoplayer2.Format;
+import com.google.android.exoplayer2.metadata.Metadata;
+import com.google.android.exoplayer2.metadata.id3.CommentFrame;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -66,6 +68,25 @@ public final class GaplessInfoHolder {
   }
 
   /**
+   * Populates the holder with data parsed from ID3 {@link Metadata}.
+   *
+   * @param metadata The metadata from which to parse the gapless information.
+   * @return Whether the holder was populated.
+   */
+  public boolean setFromMetadata(Metadata metadata) {
+    for (int i = 0; i < metadata.length(); i++) {
+      Metadata.Entry entry = metadata.get(i);
+      if (entry instanceof CommentFrame) {
+        CommentFrame commentFrame = (CommentFrame) entry;
+        if (setFromComment(commentFrame.description, commentFrame.text)) {
+          return true;
+        }
+      }
+    }
+    return false;
+  }
+
+  /**
    * Populates the holder with data parsed from a gapless playback comment (stored in an ID3 header
    * or MPEG 4 user data), if valid and non-zero.
    *

library/src/main/java/com/google/android/exoplayer2/extractor/mp3/Id3Util.java

-/*
- * Copyright (C) 2016 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package com.google.android.exoplayer2.extractor.mp3;
-
-import com.google.android.exoplayer2.extractor.ExtractorInput;
-import com.google.android.exoplayer2.metadata.Metadata;
-import com.google.android.exoplayer2.metadata.MetadataDecoderException;
-import com.google.android.exoplayer2.metadata.id3.Id3Decoder;
-import com.google.android.exoplayer2.util.ParsableByteArray;
-import com.google.android.exoplayer2.util.Util;
-import java.io.IOException;
-
-/**
- * Utility for parsing ID3 version 2 metadata in MP3 files.
- */
-/* package */ final class Id3Util {
-
-  /**
-   * The maximum valid length for metadata in bytes.
-   */
-  private static final int MAXIMUM_METADATA_SIZE = 3 * 1024 * 1024;
-
-  private static final int ID3_TAG = Util.getIntegerCodeForString("ID3");
-
-  /**
-   * Peeks data from the input and parses ID3 metadata, including gapless playback information.
-   *
-   * @param input The {@link ExtractorInput} from which data should be peeked.
-   * @return The metadata, if present, {@code null} otherwise.
-   * @throws IOException If an error occurred peeking from the input.
-   * @throws InterruptedException If the thread was interrupted.
-   */
-  public static Metadata parseId3(ExtractorInput input)
-      throws IOException, InterruptedException {
-    Metadata result = null;
-    ParsableByteArray scratch = new ParsableByteArray(10);
-    int peekedId3Bytes = 0;
-    while (true) {
-      input.peekFully(scratch.data, 0, 10);
-      scratch.setPosition(0);
-      if (scratch.readUnsignedInt24() != ID3_TAG) {
-        break;
-      }
-
-      int majorVersion = scratch.readUnsignedByte();
-      int minorVersion = scratch.readUnsignedByte();
-      int flags = scratch.readUnsignedByte();
-      int length = scratch.readSynchSafeInt();
-      int frameLength = length + 10;
-
-      try {
-        if (canParseMetadata(majorVersion, minorVersion, flags, length)) {
-          input.resetPeekPosition();
-          byte[] frame = new byte[frameLength];
-          input.peekFully(frame, 0, frameLength);
-          return new Id3Decoder().decode(frame, frameLength);
-        } else {
-          input.advancePeekPosition(length);
-        }
-      } catch (MetadataDecoderException e) {
-        e.printStackTrace();
-      }
-
-      peekedId3Bytes += frameLength;
-    }
-    input.resetPeekPosition();
-    input.advancePeekPosition(peekedId3Bytes);
-    return result;
-  }
-
-  private static boolean canParseMetadata(int majorVersion, int minorVersion, int flags,
-      int length) {
-    return minorVersion != 0xFF && majorVersion >= 2 && majorVersion <= 4
-        && length <= MAXIMUM_METADATA_SIZE
-        && !(majorVersion == 2 && ((flags & 0x3F) != 0 || (flags & 0x40) != 0))
-        && !(majorVersion == 3 && (flags & 0x1F) != 0)
-        && !(majorVersion == 4 && (flags & 0x0F) != 0);
-  }
-
-  private Id3Util() {}
-
-}

library/src/main/java/com/google/android/exoplayer2/extractor/mp3/Mp3Extractor.java

@@ -15,6 +15,7 @@
  */
 package com.google.android.exoplayer2.extractor.mp3;
 
+import android.util.Log;
 import com.google.android.exoplayer2.C;
 import com.google.android.exoplayer2.Format;
 import com.google.android.exoplayer2.ParserException;
@@ -28,7 +29,8 @@ import com.google.android.exoplayer2.extractor.PositionHolder;
 import com.google.android.exoplayer2.extractor.SeekMap;
 import com.google.android.exoplayer2.extractor.TrackOutput;
 import com.google.android.exoplayer2.metadata.Metadata;
-import com.google.android.exoplayer2.metadata.id3.CommentFrame;
+import com.google.android.exoplayer2.metadata.MetadataDecoderException;
+import com.google.android.exoplayer2.metadata.id3.Id3Decoder;
 import com.google.android.exoplayer2.util.ParsableByteArray;
 import com.google.android.exoplayer2.util.Util;
 import java.io.EOFException;
@@ -51,6 +53,8 @@ public final class Mp3Extractor implements Extractor {
 
   };
 
+  private static final String TAG = "Mp3Extractor";
+
   /**
    * The maximum number of bytes to search when synchronizing, before giving up.
    */
@@ -59,6 +63,18 @@ public final class Mp3Extractor implements Extractor {
    * The maximum number of bytes to peek when sniffing, excluding the ID3 header, before giving up.
    */
   private static final int MAX_SNIFF_BYTES = MpegAudioHeader.MAX_FRAME_SIZE_BYTES;
+  /**
+   * First three bytes of a well formed ID3 tag header.
+   */
+  private static final int ID3_TAG = Util.getIntegerCodeForString("ID3");
+  /**
+   * Length of an ID3 tag header.
+   */
+  private static final int ID3_HEADER_LENGTH = 10;
+  /**
+   * Maximum length of data read into {@link #scratch}.
+   */
+  private static final int SCRATCH_LENGTH = 10;
 
   /**
    * Mask that includes the audio header values that must match between frames.
@@ -100,7 +116,7 @@ public final class Mp3Extractor implements Extractor {
    */
   public Mp3Extractor(long forcedFirstSampleTimestampUs) {
     this.forcedFirstSampleTimestampUs = forcedFirstSampleTimestampUs;
-    scratch = new ParsableByteArray(4);
+    scratch = new ParsableByteArray(SCRATCH_LENGTH);
     synchronizedHeader = new MpegAudioHeader();
     gaplessInfoHolder = new GaplessInfoHolder();
     basisTimeUs = C.TIME_UNSET;
@@ -147,7 +163,7 @@ public final class Mp3Extractor implements Extractor {
       trackOutput.format(Format.createAudioSampleFormat(null, synchronizedHeader.mimeType, null,
           Format.NO_VALUE, MpegAudioHeader.MAX_FRAME_SIZE_BYTES, synchronizedHeader.channels,
           synchronizedHeader.sampleRate, Format.NO_VALUE, gaplessInfoHolder.encoderDelay,
-          gaplessInfoHolder.encoderPadding, null, null, 0, null, metadata));
+          gaplessInfoHolder.encoderPadding, null, null, 0, null, null));
     }
     return readSample(input);
   }
@@ -202,18 +218,7 @@ public final class Mp3Extractor implements Extractor {
     int searchLimitBytes = sniffing ? MAX_SNIFF_BYTES : MAX_SYNC_BYTES;
     input.resetPeekPosition();
     if (input.getPosition() == 0) {
-      metadata = Id3Util.parseId3(input);
-      if (!gaplessInfoHolder.hasGaplessInfo()) {
-        for (int i = 0; i < metadata.length(); i++) {
-          Metadata.Entry entry = metadata.get(i);
-          if (entry instanceof CommentFrame) {
-            CommentFrame commentFrame = (CommentFrame) entry;
-            if (gaplessInfoHolder.setFromComment(commentFrame.description, commentFrame.text)) {
-              break;
-            }
-          }
-        }
-      }
+      peekId3Data(input);
       peekedId3Bytes = (int) input.getPeekPosition();
       if (!sniffing) {
         input.skipFully(peekedId3Bytes);
@@ -268,6 +273,49 @@ public final class Mp3Extractor implements Extractor {
   }
 
   /**
+   * Peeks ID3 data from the input, including gapless playback information.
+   *
+   * @param input The {@link ExtractorInput} from which data should be peeked.
+   * @throws IOException If an error occurred peeking from the input.
+   * @throws InterruptedException If the thread was interrupted.
+   */
+  private void peekId3Data(ExtractorInput input) throws IOException, InterruptedException {
+    int peekedId3Bytes = 0;
+    while (true) {
+      input.peekFully(scratch.data, 0, ID3_HEADER_LENGTH);
+      scratch.setPosition(0);
+      if (scratch.readUnsignedInt24() != ID3_TAG) {
+        // Not an ID3 tag.
+        break;
+      }
+      scratch.skipBytes(3); // Skip major version, minor version and flags.
+      int framesLength = scratch.readSynchSafeInt();
+      int tagLength = ID3_HEADER_LENGTH + framesLength;
+
+      try {
+        if (metadata == null) {
+          byte[] id3Data = new byte[tagLength];
+          System.arraycopy(scratch.data, 0, id3Data, 0, ID3_HEADER_LENGTH);
+          input.peekFully(id3Data, ID3_HEADER_LENGTH, framesLength);
+          metadata = new Id3Decoder().decode(id3Data, tagLength);
+          if (metadata != null) {
+            gaplessInfoHolder.setFromMetadata(metadata);
+          }
+        } else {
+          input.advancePeekPosition(framesLength);
+        }
+      } catch (MetadataDecoderException e) {
+        Log.e(TAG, "Failed to decode ID3 tag", e);
+      }
+
+      peekedId3Bytes += tagLength;
+    }
+
+    input.resetPeekPosition();
+    input.advancePeekPosition(peekedId3Bytes);
+  }
+
+  /**
    * Returns a {@link Seeker} to seek using metadata read from {@code input}, which should provide
    * data from the start of the first frame in the stream. On returning, the input's position will
    * be set to the start of the first frame of audio.