From 514edb699f861e2d817da045ef7030195250d4fe Mon Sep 17 00:00:00 2001 From: aquilescanta Date: Fri, 31 Aug 2018 05:30:23 -0700 Subject: [PATCH] Add a type check for OGG files with a single payload page Also make some javadocs more consistent with the rest of the library. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=211071559 --- .../extractor/ogg/DefaultOggSeeker.java | 57 +++++++++++-------- .../exoplayer2/extractor/ogg/OggPacket.java | 10 ++-- .../extractor/ogg/OggPageHeader.java | 14 ++--- .../exoplayer2/extractor/ogg/OpusReader.java | 4 +- .../extractor/ogg/StreamReader.java | 12 +++- .../extractor/ogg/DefaultOggSeekerTest.java | 19 +++++-- .../ogg/DefaultOggSeekerUtilMethodsTest.java | 28 +++++++-- 7 files changed, 91 insertions(+), 53 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/extractor/ogg/DefaultOggSeeker.java b/library/core/src/main/java/com/google/android/exoplayer2/extractor/ogg/DefaultOggSeeker.java index 042ab681f9..93ed00a05b 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/extractor/ogg/DefaultOggSeeker.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/extractor/ogg/DefaultOggSeeker.java @@ -23,9 +23,7 @@ import com.google.android.exoplayer2.util.Assertions; import java.io.EOFException; import java.io.IOException; -/** - * Used to seek in an Ogg stream. - */ +/** Seeks in an Ogg stream. */ /* package */ final class DefaultOggSeeker implements OggSeeker { //@VisibleForTesting @@ -56,19 +54,27 @@ import java.io.IOException; /** * Constructs an OggSeeker. + * * @param startPosition Start position of the payload (inclusive). * @param endPosition End position of the payload (exclusive). - * @param streamReader StreamReader instance which owns this OggSeeker + * @param streamReader The {@link StreamReader} that owns this seeker. * @param firstPayloadPageSize The total size of the first payload page, in bytes. * @param firstPayloadPageGranulePosition The granule position of the first payload page. + * @param firstPayloadPageIsLastPage Whether the first payload page is also the last page in the + * ogg stream. */ - public DefaultOggSeeker(long startPosition, long endPosition, StreamReader streamReader, - int firstPayloadPageSize, long firstPayloadPageGranulePosition) { + public DefaultOggSeeker( + long startPosition, + long endPosition, + StreamReader streamReader, + long firstPayloadPageSize, + long firstPayloadPageGranulePosition, + boolean firstPayloadPageIsLastPage) { Assertions.checkArgument(startPosition >= 0 && endPosition > startPosition); this.streamReader = streamReader; this.startPosition = startPosition; this.endPosition = endPosition; - if (firstPayloadPageSize == endPosition - startPosition) { + if (firstPayloadPageSize == endPosition - startPosition || firstPayloadPageIsLastPage) { totalGranules = firstPayloadPageGranulePosition; state = STATE_IDLE; } else { @@ -240,11 +246,11 @@ import java.io.IOException; * Skips to the next page. * * @param input The {@code ExtractorInput} to skip to the next page. - * @throws IOException thrown if peeking/reading from the input fails. - * @throws InterruptedException thrown if interrupted while peeking/reading from the input. - * @throws EOFException if the next page can't be found before the end of the input. + * @throws IOException If peeking/reading from the input fails. + * @throws InterruptedException If the thread is interrupted. + * @throws EOFException If the next page can't be found before the end of the input. */ - //@VisibleForTesting + // @VisibleForTesting void skipToNextPage(ExtractorInput input) throws IOException, InterruptedException { if (!skipToNextPage(input, endPosition)) { // Not found until eof. @@ -256,21 +262,21 @@ import java.io.IOException; * Skips to the next page. Searches for the next page header. * * @param input The {@code ExtractorInput} to skip to the next page. - * @param until Searches until this position. - * @return true if the next page is found. + * @param limit The limit up to which the search should take place. + * @return Whether the next page was found. * @throws IOException thrown if peeking/reading from the input fails. * @throws InterruptedException thrown if interrupted while peeking/reading from the input. */ - //@VisibleForTesting - boolean skipToNextPage(ExtractorInput input, long until) + // @VisibleForTesting + boolean skipToNextPage(ExtractorInput input, long limit) throws IOException, InterruptedException { - until = Math.min(until + 3, endPosition); + limit = Math.min(limit + 3, endPosition); byte[] buffer = new byte[2048]; int peekLength = buffer.length; while (true) { - if (input.getPosition() + peekLength > until) { + if (input.getPosition() + peekLength > limit) { // Make sure to not peek beyond the end of the input. - peekLength = (int) (until - input.getPosition()); + peekLength = (int) (limit - input.getPosition()); if (peekLength < 4) { // Not found until end. return false; @@ -278,7 +284,9 @@ import java.io.IOException; } input.peekFully(buffer, 0, peekLength, false); for (int i = 0; i < peekLength - 3; i++) { - if (buffer[i] == 'O' && buffer[i + 1] == 'g' && buffer[i + 2] == 'g' + if (buffer[i] == 'O' + && buffer[i + 1] == 'g' + && buffer[i + 2] == 'g' && buffer[i + 3] == 'S') { // Match! Skip to the start of the pattern. input.skipFully(i); @@ -295,13 +303,12 @@ import java.io.IOException; * total number of samples per channel. * * @param input The {@link ExtractorInput} to read from. - * @return the total number of samples of this input. - * @throws IOException thrown if reading from the input fails. - * @throws InterruptedException thrown if interrupted while reading from the input. + * @return The total number of samples of this input. + * @throws IOException If reading from the input fails. + * @throws InterruptedException If the thread is interrupted. */ - //@VisibleForTesting - long readGranuleOfLastPage(ExtractorInput input) - throws IOException, InterruptedException { + // @VisibleForTesting + long readGranuleOfLastPage(ExtractorInput input) throws IOException, InterruptedException { skipToNextPage(input); pageHeader.reset(); while ((pageHeader.type & 0x04) != 0x04 && input.getPosition() < endPosition) { diff --git a/library/core/src/main/java/com/google/android/exoplayer2/extractor/ogg/OggPacket.java b/library/core/src/main/java/com/google/android/exoplayer2/extractor/ogg/OggPacket.java index c7f4e9489b..482f0c5021 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/extractor/ogg/OggPacket.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/extractor/ogg/OggPacket.java @@ -51,11 +51,11 @@ import java.util.Arrays; * can resume properly from an error while reading a continued packet spanned across multiple * pages. * - * @param input the {@link ExtractorInput} to read data from. - * @return {@code true} if the read was successful. {@code false} if the end of the input was - * encountered having read no data. - * @throws IOException thrown if reading from the input fails. - * @throws InterruptedException thrown if interrupted while reading from input. + * @param input The {@link ExtractorInput} to read data from. + * @return {@code true} if the read was successful. The read fails if the end of the input is + * encountered without reading data. + * @throws IOException If reading from the input fails. + * @throws InterruptedException If the thread is interrupted. */ public boolean populate(ExtractorInput input) throws IOException, InterruptedException { Assertions.checkState(input != null); diff --git a/library/core/src/main/java/com/google/android/exoplayer2/extractor/ogg/OggPageHeader.java b/library/core/src/main/java/com/google/android/exoplayer2/extractor/ogg/OggPageHeader.java index e260a72d44..bbf7e2fc6b 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/extractor/ogg/OggPageHeader.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/extractor/ogg/OggPageHeader.java @@ -71,13 +71,13 @@ import java.io.IOException; /** * Peeks an Ogg page header and updates this {@link OggPageHeader}. * - * @param input the {@link ExtractorInput} to read from. - * @param quiet if {@code true} no Exceptions are thrown but {@code false} is return if something - * goes wrong. - * @return {@code true} if the read was successful. {@code false} if the end of the input was - * encountered having read no data. - * @throws IOException thrown if reading data fails or the stream is invalid. - * @throws InterruptedException thrown if thread is interrupted when reading/peeking. + * @param input The {@link ExtractorInput} to read from. + * @param quiet If {@code true}, no exceptions are thrown but {@code false} is returned if + * something goes wrong. + * @return {@code true} if the read was successful. The read fails if the end of the input is + * encountered without reading data. + * @throws IOException If reading data fails or the stream is invalid. + * @throws InterruptedException If the thread is interrupted. */ public boolean populate(ExtractorInput input, boolean quiet) throws IOException, InterruptedException { diff --git a/library/core/src/main/java/com/google/android/exoplayer2/extractor/ogg/OpusReader.java b/library/core/src/main/java/com/google/android/exoplayer2/extractor/ogg/OpusReader.java index ce3b9ea6ba..ff5f115573 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/extractor/ogg/OpusReader.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/extractor/ogg/OpusReader.java @@ -20,7 +20,6 @@ import com.google.android.exoplayer2.Format; import com.google.android.exoplayer2.util.MimeTypes; import com.google.android.exoplayer2.util.ParsableByteArray; import com.google.android.exoplayer2.util.Util; -import java.io.IOException; import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.util.ArrayList; @@ -67,8 +66,7 @@ import java.util.List; } @Override - protected boolean readHeaders(ParsableByteArray packet, long position, SetupData setupData) - throws IOException, InterruptedException { + protected boolean readHeaders(ParsableByteArray packet, long position, SetupData setupData) { if (!headerRead) { byte[] metadata = Arrays.copyOf(packet.data, packet.limit()); int channelCount = metadata[9] & 0xFF; diff --git a/library/core/src/main/java/com/google/android/exoplayer2/extractor/ogg/StreamReader.java b/library/core/src/main/java/com/google/android/exoplayer2/extractor/ogg/StreamReader.java index 8872c6359f..e459ad1e58 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/extractor/ogg/StreamReader.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/extractor/ogg/StreamReader.java @@ -144,9 +144,15 @@ import java.io.IOException; oggSeeker = new UnseekableOggSeeker(); } else { OggPageHeader firstPayloadPageHeader = oggPacket.getPageHeader(); - oggSeeker = new DefaultOggSeeker(payloadStartPosition, input.getLength(), this, - firstPayloadPageHeader.headerSize + firstPayloadPageHeader.bodySize, - firstPayloadPageHeader.granulePosition); + boolean isLastPage = (firstPayloadPageHeader.type & 0x04) != 0; // Type 4 is end of stream. + oggSeeker = + new DefaultOggSeeker( + payloadStartPosition, + input.getLength(), + this, + firstPayloadPageHeader.headerSize + firstPayloadPageHeader.bodySize, + firstPayloadPageHeader.granulePosition, + isLastPage); } setupData = null; diff --git a/library/core/src/test/java/com/google/android/exoplayer2/extractor/ogg/DefaultOggSeekerTest.java b/library/core/src/test/java/com/google/android/exoplayer2/extractor/ogg/DefaultOggSeekerTest.java index 993bb86b48..1b6df9c5a4 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/extractor/ogg/DefaultOggSeekerTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/extractor/ogg/DefaultOggSeekerTest.java @@ -35,7 +35,13 @@ public final class DefaultOggSeekerTest { @Test public void testSetupWithUnsetEndPositionFails() { try { - new DefaultOggSeeker(0, C.LENGTH_UNSET, new TestStreamReader(), 1, 1); + new DefaultOggSeeker( + /* startPosition= */ 0, + /* endPosition= */ C.LENGTH_UNSET, + /* streamReader= */ new TestStreamReader(), + /* firstPayloadPageSize= */ 1, + /* firstPayloadPageGranulePosition= */ 1, + /* firstPayloadPageIsLastPage= */ false); fail(); } catch (IllegalArgumentException e) { // ignored @@ -56,11 +62,12 @@ public final class DefaultOggSeekerTest { TestStreamReader streamReader = new TestStreamReader(); DefaultOggSeeker oggSeeker = new DefaultOggSeeker( - 0, - testFile.data.length, - streamReader, - testFile.firstPayloadPageSize, - testFile.firstPayloadPageGranulePosition); + /* startPosition= */ 0, + /* endPosition= */ testFile.data.length, + /* streamReader= */ streamReader, + /* firstPayloadPageSize= */ testFile.firstPayloadPageSize, + /* firstPayloadPageGranulePosition= */ testFile.firstPayloadPageGranulePosition, + /* firstPayloadPageIsLastPage= */ false); OggPageHeader pageHeader = new OggPageHeader(); while (true) { diff --git a/library/core/src/test/java/com/google/android/exoplayer2/extractor/ogg/DefaultOggSeekerUtilMethodsTest.java b/library/core/src/test/java/com/google/android/exoplayer2/extractor/ogg/DefaultOggSeekerUtilMethodsTest.java index be771ac3b9..ef38282691 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/extractor/ogg/DefaultOggSeekerUtilMethodsTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/extractor/ogg/DefaultOggSeekerUtilMethodsTest.java @@ -85,8 +85,14 @@ public final class DefaultOggSeekerUtilMethodsTest { private static void skipToNextPage(ExtractorInput extractorInput) throws IOException, InterruptedException { - DefaultOggSeeker oggSeeker = new DefaultOggSeeker(0, extractorInput.getLength(), - new FlacReader(), 1, 2); + DefaultOggSeeker oggSeeker = + new DefaultOggSeeker( + /* startPosition= */ 0, + /* endPosition= */ extractorInput.getLength(), + /* streamReader= */ new FlacReader(), + /* firstPayloadPageSize= */ 1, + /* firstPayloadPageGranulePosition= */ 2, + /* firstPayloadPageIsLastPage= */ false); while (true) { try { oggSeeker.skipToNextPage(extractorInput); @@ -157,7 +163,14 @@ public final class DefaultOggSeekerUtilMethodsTest { private void skipToPageOfGranule(ExtractorInput input, long granule, long elapsedSamplesExpected) throws IOException, InterruptedException { - DefaultOggSeeker oggSeeker = new DefaultOggSeeker(0, input.getLength(), new FlacReader(), 1, 2); + DefaultOggSeeker oggSeeker = + new DefaultOggSeeker( + /* startPosition= */ 0, + /* endPosition= */ input.getLength(), + /* streamReader= */ new FlacReader(), + /* firstPayloadPageSize= */ 1, + /* firstPayloadPageGranulePosition= */ 2, + /* firstPayloadPageIsLastPage= */ false); while (true) { try { assertThat(oggSeeker.skipToPageOfGranule(input, granule, -1)) @@ -211,7 +224,14 @@ public final class DefaultOggSeekerUtilMethodsTest { private void assertReadGranuleOfLastPage(FakeExtractorInput input, int expected) throws IOException, InterruptedException { - DefaultOggSeeker oggSeeker = new DefaultOggSeeker(0, input.getLength(), new FlacReader(), 1, 2); + DefaultOggSeeker oggSeeker = + new DefaultOggSeeker( + /* startPosition= */ 0, + /* endPosition= */ input.getLength(), + /* streamReader= */ new FlacReader(), + /* firstPayloadPageSize= */ 1, + /* firstPayloadPageGranulePosition= */ 2, + /* firstPayloadPageIsLastPage= */ false); while (true) { try { assertThat(oggSeeker.readGranuleOfLastPage(input)).isEqualTo(expected);