From 706ce49bd37938dd5b9ae65957ada0717de5c74c Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Tue, 23 Oct 2018 09:49:45 -0700 Subject: [PATCH] Fix handling of MP3s with appended data Issue: #4954 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=218357113 --- RELEASENOTES.md | 2 ++ .../extractor/mp3/ConstantBitrateSeeker.java | 5 +++ .../exoplayer2/extractor/mp3/MlltSeeker.java | 4 +++ .../extractor/mp3/Mp3Extractor.java | 32 +++++++++++++++---- .../exoplayer2/extractor/mp3/VbriSeeker.java | 10 ++++-- .../exoplayer2/extractor/mp3/XingSeeker.java | 25 +++++++++++---- 6 files changed, 63 insertions(+), 15 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 84864a35c46..df1423e6887 100644 --- a/RELEASENOTES.md +++ b/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)). diff --git a/library/core/src/main/java/com/google/android/exoplayer2/extractor/mp3/ConstantBitrateSeeker.java b/library/core/src/main/java/com/google/android/exoplayer2/extractor/mp3/ConstantBitrateSeeker.java index bffc43a540f..f4007207728 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/extractor/mp3/ConstantBitrateSeeker.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/extractor/mp3/ConstantBitrateSeeker.java @@ -39,4 +39,9 @@ public ConstantBitrateSeeker( public long getTimeUs(long position) { return getTimeUsAtPosition(position); } + + @Override + public long getDataEndPosition() { + return C.POSITION_UNSET; + } } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/extractor/mp3/MlltSeeker.java b/library/core/src/main/java/com/google/android/exoplayer2/extractor/mp3/MlltSeeker.java index ff607b9482e..868c1d9fbff 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/extractor/mp3/MlltSeeker.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/extractor/mp3/MlltSeeker.java @@ -118,4 +118,8 @@ private static Pair linearlyInterpolate( } } + @Override + public long getDataEndPosition() { + return C.POSITION_UNSET; + } } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/extractor/mp3/Mp3Extractor.java b/library/core/src/main/java/com/google/android/exoplayer2/extractor/mp3/Mp3Extractor.java index 84f620734b8..e8848bf9837 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/extractor/mp3/Mp3Extractor.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/extractor/mp3/Mp3Extractor.java @@ -223,7 +223,7 @@ public int read(ExtractorInput input, PositionHolder seekPosition) 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,9 +285,12 @@ private boolean synchronize(ExtractorInput input, boolean sniffing) } } while (true) { - if (!input.peekFully(scratch.data, 0, 4, validFrameCount > 0)) { - // We reached the end of the stream but found at least one valid frame. - break; + 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(); @@ -332,6 +335,17 @@ private boolean synchronize(ExtractorInput input, boolean sniffing) 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 @@ private static MlltSeeker maybeHandleSeekMetadata(Metadata metadata, long firstF } /** - * {@link SeekMap} that also allows mapping from position (byte offset) back to time, which can be - * used to work out the new sample basis timestamp after seeking and resynchronization. + * {@link SeekMap} that provides the end position of audio data and also allows mapping from + * 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 @@ private static MlltSeeker maybeHandleSeekMetadata(Metadata metadata, long firstF */ 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(); } } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/extractor/mp3/VbriSeeker.java b/library/core/src/main/java/com/google/android/exoplayer2/extractor/mp3/VbriSeeker.java index 774505f622c..15e778115d1 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/extractor/mp3/VbriSeeker.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/extractor/mp3/VbriSeeker.java @@ -89,17 +89,19 @@ 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 @@ public long getDurationUs() { return durationUs; } + @Override + public long getDataEndPosition() { + return dataEndPosition; + } } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/extractor/mp3/XingSeeker.java b/library/core/src/main/java/com/google/android/exoplayer2/extractor/mp3/XingSeeker.java index c2971e47ce1..42752e55fb4 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/extractor/mp3/XingSeeker.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/extractor/mp3/XingSeeker.java @@ -75,17 +75,17 @@ 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, - tableOfContents); + return new XingSeeker( + 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 @@ 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 @@ private XingSeeker( 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 @@ public long getDurationUs() { return durationUs; } + @Override + public long getDataEndPosition() { + return dataEndPosition; + } + /** * Returns the time in microseconds for a given table index. *