Fix handling of MP3s with appended data

Issue: #4954

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=218357113

RELEASENOTES.md

@@ -24,6 +24,8 @@
 * Audio:
   * Support seeking based on MLLT metadata
     ([#3241](https://github.com/google/ExoPlayer/issues/3241)).
+  * Fix handling of MP3s with appended data
+    ([#4954](https://github.com/google/ExoPlayer/issues/4954)).
 * Fix issue where buffered position is not updated correctly when transitioning
   between periods
   ([#4899](https://github.com/google/ExoPlayer/issues/4899)).

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

@@ -39,4 +39,9 @@ import com.google.android.exoplayer2.extractor.MpegAudioHeader;
   public long getTimeUs(long position) {
     return getTimeUsAtPosition(position);
   }
+
+  @Override
+  public long getDataEndPosition() {
+    return C.POSITION_UNSET;
+  }
 }

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

@@ -118,4 +118,8 @@ import com.google.android.exoplayer2.util.Util;
     }
   }
 
+  @Override
+  public long getDataEndPosition() {
+    return C.POSITION_UNSET;
+  }
 }

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

@@ -223,7 +223,7 @@ public final class Mp3Extractor implements Extractor {
   private int readSample(ExtractorInput extractorInput) throws IOException, InterruptedException {
     if (sampleBytesRemaining == 0) {
       extractorInput.resetPeekPosition();
-      if (!extractorInput.peekFully(scratch.data, 0, 4, true)) {
+      if (peekEndOfStreamOrHeader(extractorInput)) {
         return RESULT_END_OF_INPUT;
       }
       scratch.setPosition(0);
@@ -285,10 +285,13 @@ public final class Mp3Extractor implements Extractor {
       }
     }
     while (true) {
-      if (!input.peekFully(scratch.data, 0, 4, validFrameCount > 0)) {
+      if (peekEndOfStreamOrHeader(input)) {
+        if (validFrameCount > 0) {
           // We reached the end of the stream but found at least one valid frame.
           break;
         }
+        throw new EOFException();
+      }
       scratch.setPosition(0);
       int headerData = scratch.readInt();
       int frameSize;
@@ -332,6 +335,17 @@ public final class Mp3Extractor implements Extractor {
     return true;
   }
 
+  /**
+   * Returns whether the extractor input is peeking the end of the stream. If {@code false},
+   * populates the scratch buffer with the next four bytes.
+   */
+  private boolean peekEndOfStreamOrHeader(ExtractorInput extractorInput)
+      throws IOException, InterruptedException {
+    return (seeker != null && extractorInput.getPeekPosition() == seeker.getDataEndPosition())
+        || !extractorInput.peekFully(
+            scratch.data, /* offset= */ 0, /* length= */ 4, /* allowEndOfInput= */ true);
+  }
+
   /**
    * Consumes the next frame from the {@code input} if it contains VBRI or Xing seeking metadata,
    * returning a {@link Seeker} if the metadata was present and valid, or {@code null} otherwise.
@@ -433,8 +447,9 @@ public final class Mp3Extractor implements Extractor {
   }
 
   /**
-   * {@link SeekMap} that also allows mapping from position (byte offset) back to time, which can be
+   * {@link SeekMap} that provides the end position of audio data and also allows mapping from
-   * used to work out the new sample basis timestamp after seeking and resynchronization.
+   * position (byte offset) back to time, which can be used to work out the new sample basis
+   * timestamp after seeking and resynchronization.
    */
   /* package */ interface Seeker extends SeekMap {
 
@@ -446,6 +461,11 @@ public final class Mp3Extractor implements Extractor {
      */
     long getTimeUs(long position);
 
+    /**
+     * Returns the position (byte offset) in the stream that is immediately after audio data, or
+     * {@link C#POSITION_UNSET} if not known.
+     */
+    long getDataEndPosition();
   }
 
 }

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

@@ -89,17 +89,19 @@ import com.google.android.exoplayer2.util.Util;
     if (inputLength != C.LENGTH_UNSET && inputLength != position) {
       Log.w(TAG, "VBRI data size mismatch: " + inputLength + ", " + position);
     }
-    return new VbriSeeker(timesUs, positions, durationUs);
+    return new VbriSeeker(timesUs, positions, durationUs, /* dataEndPosition= */ position);
   }
 
   private final long[] timesUs;
   private final long[] positions;
   private final long durationUs;
+  private final long dataEndPosition;
 
-  private VbriSeeker(long[] timesUs, long[] positions, long durationUs) {
+  private VbriSeeker(long[] timesUs, long[] positions, long durationUs, long dataEndPosition) {
     this.timesUs = timesUs;
     this.positions = positions;
     this.durationUs = durationUs;
+    this.dataEndPosition = dataEndPosition;
   }
 
   @Override
@@ -129,4 +131,8 @@ import com.google.android.exoplayer2.util.Util;
     return durationUs;
   }
 
+  @Override
+  public long getDataEndPosition() {
+    return dataEndPosition;
+  }
 }

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

@@ -75,17 +75,17 @@ import com.google.android.exoplayer2.util.Util;
     if (inputLength != C.LENGTH_UNSET && inputLength != position + dataSize) {
       Log.w(TAG, "XING data size mismatch: " + inputLength + ", " + (position + dataSize));
     }
-    return new XingSeeker(position, mpegAudioHeader.frameSize, durationUs, dataSize,
+    return new XingSeeker(
-        tableOfContents);
+        position, mpegAudioHeader.frameSize, durationUs, dataSize, tableOfContents);
   }
 
   private final long dataStartPosition;
   private final int xingFrameSize;
   private final long durationUs;
-  /**
+  /** Data size, including the XING frame. */
-   * Data size, including the XING frame.
-   */
   private final long dataSize;
+
+  private final long dataEndPosition;
   /**
    * Entries are in the range [0, 255], but are stored as long integers for convenience. Null if the
    * table of contents was missing from the header, in which case seeking is not be supported.
@@ -93,7 +93,12 @@ import com.google.android.exoplayer2.util.Util;
   private final @Nullable long[] tableOfContents;
 
   private XingSeeker(long dataStartPosition, int xingFrameSize, long durationUs) {
-    this(dataStartPosition, xingFrameSize, durationUs, C.LENGTH_UNSET, null);
+    this(
+        dataStartPosition,
+        xingFrameSize,
+        durationUs,
+        /* dataSize= */ C.LENGTH_UNSET,
+        /* tableOfContents= */ null);
   }
 
   private XingSeeker(
@@ -105,8 +110,9 @@ import com.google.android.exoplayer2.util.Util;
     this.dataStartPosition = dataStartPosition;
     this.xingFrameSize = xingFrameSize;
     this.durationUs = durationUs;
-    this.dataSize = dataSize;
     this.tableOfContents = tableOfContents;
+    this.dataSize = dataSize;
+    dataEndPosition = dataSize == C.LENGTH_UNSET ? C.POSITION_UNSET : dataStartPosition + dataSize;
   }
 
   @Override
@@ -166,6 +172,11 @@ import com.google.android.exoplayer2.util.Util;
     return durationUs;
   }
 
+  @Override
+  public long getDataEndPosition() {
+    return dataEndPosition;
+  }
+
   /**
    * Returns the time in microseconds for a given table index.
    *