diff --git a/RELEASENOTES.md b/RELEASENOTES.md index bdc396160c7..c8773b07852 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -19,6 +19,8 @@ is set incorrectly ([#4083](https://github.com/google/ExoPlayer/issues/4083)). Such content is malformed and should be re-encoded. + * Improve support for truncated Ogg streams + ([#7608](https://github.com/google/ExoPlayer/issues/7608)). * HLS: * Fix issue where playback of a live event could become stuck rather than transitioning to `STATE_ENDED` when the event ends diff --git a/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ExtractorUtil.java b/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ExtractorUtil.java index 5ce274b11a1..3eca5c776bd 100644 --- a/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ExtractorUtil.java +++ b/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ExtractorUtil.java @@ -16,10 +16,11 @@ package com.google.android.exoplayer2.extractor; import com.google.android.exoplayer2.C; +import java.io.EOFException; import java.io.IOException; /** Extractor related utility methods. */ -/* package */ final class ExtractorUtil { +public final class ExtractorUtil { /** * Peeks {@code length} bytes from the input peek position, or all the bytes to the end of the @@ -47,5 +48,62 @@ public static int peekToLength(ExtractorInput input, byte[] target, int offset, return totalBytesPeeked; } + /** + * Equivalent to {@link ExtractorInput#readFully(byte[], int, int)} except that it returns {@code + * false} instead of throwing an {@link EOFException} if the end of input is encountered without + * having fully satisfied the read. + */ + public static boolean readFullyQuietly( + ExtractorInput input, byte[] output, int offset, int length) throws IOException { + try { + input.readFully(output, offset, length); + } catch (EOFException e) { + return false; + } + return true; + } + + /** + * Equivalent to {@link ExtractorInput#skipFully(int)} except that it returns {@code false} + * instead of throwing an {@link EOFException} if the end of input is encountered without having + * fully satisfied the skip. + */ + public static boolean skipFullyQuietly(ExtractorInput input, int length) throws IOException { + try { + input.skipFully(length); + } catch (EOFException e) { + return false; + } + return true; + } + + /** + * Peeks data from {@code input}, respecting {@code allowEndOfInput}. Returns true if the peek is + * successful. + * + *
If {@code allowEndOfInput=false} then encountering the end of the input (whether before or + * after reading some data) will throw {@link EOFException}. + * + *
If {@code allowEndOfInput=true} then encountering the end of the input (even after reading + * some data) will return {@code false}. + * + *
This is slightly different to the behaviour of {@link ExtractorInput#peekFully(byte[], int, + * int, boolean)}, where {@code allowEndOfInput=true} only returns false (and suppresses the + * exception) if the end of the input is reached before reading any data. + */ + public static boolean peekFullyQuietly( + ExtractorInput input, byte[] output, int offset, int length, boolean allowEndOfInput) + throws IOException { + try { + return input.peekFully(output, offset, length, /* allowEndOfInput= */ allowEndOfInput); + } catch (EOFException e) { + if (allowEndOfInput) { + return false; + } else { + throw e; + } + } + } + private ExtractorUtil() {} } diff --git a/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ogg/DefaultOggSeeker.java b/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ogg/DefaultOggSeeker.java index b7a80394896..29a0f5932ab 100644 --- a/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ogg/DefaultOggSeeker.java +++ b/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ogg/DefaultOggSeeker.java @@ -15,6 +15,8 @@ */ package com.google.android.exoplayer2.extractor.ogg; +import static com.google.android.exoplayer2.extractor.ExtractorUtil.skipFullyQuietly; + import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; import com.google.android.exoplayer2.C; @@ -229,13 +231,21 @@ long readGranuleOfLastPage(ExtractorInput input) throws IOException { if (!pageHeader.skipToNextPage(input)) { throw new EOFException(); } - do { - pageHeader.populate(input, /* quiet= */ false); - input.skipFully(pageHeader.headerSize + pageHeader.bodySize); - } while ((pageHeader.type & 0x04) != 0x04 + pageHeader.populate(input, /* quiet= */ false); + input.skipFully(pageHeader.headerSize + pageHeader.bodySize); + long granulePosition = pageHeader.granulePosition; + while ((pageHeader.type & 0x04) != 0x04 && pageHeader.skipToNextPage(input) - && input.getPosition() < payloadEndPosition); - return pageHeader.granulePosition; + && input.getPosition() < payloadEndPosition) { + boolean hasPopulated = pageHeader.populate(input, /* quiet= */ true); + if (!hasPopulated || !skipFullyQuietly(input, pageHeader.headerSize + pageHeader.bodySize)) { + // The input file contains a partial page at the end. Ignore it and return the granule + // position of the last complete page. + return granulePosition; + } + granulePosition = pageHeader.granulePosition; + } + return granulePosition; } private final class OggSeekMap implements SeekMap { diff --git a/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ogg/OggPacket.java b/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ogg/OggPacket.java index 14a5fea607d..dcca44b3d83 100644 --- a/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ogg/OggPacket.java +++ b/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ogg/OggPacket.java @@ -15,6 +15,8 @@ */ package com.google.android.exoplayer2.extractor.ogg; +import static com.google.android.exoplayer2.extractor.ExtractorUtil.readFullyQuietly; +import static com.google.android.exoplayer2.extractor.ExtractorUtil.skipFullyQuietly; import static java.lang.Math.max; import com.google.android.exoplayer2.C; @@ -55,7 +57,7 @@ public void reset() { * * @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. + * encountered without reading the whole packet. * @throws IOException If reading from the input fails. */ public boolean populate(ExtractorInput input) throws IOException { @@ -80,7 +82,9 @@ public boolean populate(ExtractorInput input) throws IOException { bytesToSkip += calculatePacketSize(segmentIndex); segmentIndex += segmentCount; } - input.skipFully(bytesToSkip); + if (!skipFullyQuietly(input, bytesToSkip)) { + return false; + } currentSegmentIndex = segmentIndex; } @@ -88,7 +92,9 @@ public boolean populate(ExtractorInput input) throws IOException { int segmentIndex = currentSegmentIndex + segmentCount; if (size > 0) { packetArray.ensureCapacity(packetArray.limit() + size); - input.readFully(packetArray.getData(), packetArray.limit(), size); + if (!readFullyQuietly(input, packetArray.getData(), packetArray.limit(), size)) { + return false; + } packetArray.setLimit(packetArray.limit() + size); populated = pageHeader.laces[segmentIndex - 1] != 255; } diff --git a/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ogg/OggPageHeader.java b/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ogg/OggPageHeader.java index a59eca6c8ce..70a1f22f54e 100644 --- a/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ogg/OggPageHeader.java +++ b/library/extractor/src/main/java/com/google/android/exoplayer2/extractor/ogg/OggPageHeader.java @@ -15,12 +15,13 @@ */ package com.google.android.exoplayer2.extractor.ogg; +import static com.google.android.exoplayer2.extractor.ExtractorUtil.peekFullyQuietly; + import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.ParserException; import com.google.android.exoplayer2.extractor.ExtractorInput; import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.ParsableByteArray; -import java.io.EOFException; import java.io.IOException; /** @@ -106,7 +107,8 @@ public boolean skipToNextPage(ExtractorInput input, long limit) throws IOExcepti Assertions.checkArgument(input.getPosition() == input.getPeekPosition()); scratch.reset(/* limit= */ CAPTURE_PATTERN_SIZE); while ((limit == C.POSITION_UNSET || input.getPosition() + CAPTURE_PATTERN_SIZE < limit) - && peekSafely(input, scratch.getData(), 0, CAPTURE_PATTERN_SIZE, /* quiet= */ true)) { + && peekFullyQuietly( + input, scratch.getData(), 0, CAPTURE_PATTERN_SIZE, /* allowEndOfInput= */ true)) { scratch.setPosition(0); if (scratch.readUnsignedInt() == CAPTURE_PATTERN) { input.resetPeekPosition(); @@ -127,14 +129,13 @@ && peekSafely(input, scratch.getData(), 0, CAPTURE_PATTERN_SIZE, /* quiet= */ tr * @param input The {@link ExtractorInput} to read from. * @param quiet Whether to return {@code false} rather than throwing an exception if the header * cannot be populated. - * @return Whether the read was successful. The read fails if the end of the input is encountered - * without reading data. + * @return Whether the header was entirely populated. * @throws IOException If reading data fails or the stream is invalid. */ public boolean populate(ExtractorInput input, boolean quiet) throws IOException { reset(); scratch.reset(/* limit= */ EMPTY_PAGE_HEADER_SIZE); - if (!peekSafely(input, scratch.getData(), 0, EMPTY_PAGE_HEADER_SIZE, quiet) + if (!peekFullyQuietly(input, scratch.getData(), 0, EMPTY_PAGE_HEADER_SIZE, quiet) || scratch.readUnsignedInt() != CAPTURE_PATTERN) { return false; } @@ -158,7 +159,9 @@ public boolean populate(ExtractorInput input, boolean quiet) throws IOException // calculate total size of header including laces scratch.reset(/* limit= */ pageSegmentCount); - input.peekFully(scratch.getData(), 0, pageSegmentCount); + if (!peekFullyQuietly(input, scratch.getData(), 0, pageSegmentCount, quiet)) { + return false; + } for (int i = 0; i < pageSegmentCount; i++) { laces[i] = scratch.readUnsignedByte(); bodySize += laces[i]; @@ -166,31 +169,4 @@ public boolean populate(ExtractorInput input, boolean quiet) throws IOException return true; } - - /** - * Peek data from {@code input}, respecting {@code quiet}. Return true if the peek is successful. - * - *
If {@code quiet=false} then encountering the end of the input (whether before or after - * reading some data) will throw {@link EOFException}. - * - *
If {@code quiet=true} then encountering the end of the input (even after reading some data) - * will return {@code false}. - * - *
This is slightly different to the behaviour of {@link ExtractorInput#peekFully(byte[], int, - * int, boolean)}, where {@code allowEndOfInput=true} only returns false (and suppresses the - * exception) if the end of the input is reached before reading any data. - */ - private static boolean peekSafely( - ExtractorInput input, byte[] output, int offset, int length, boolean quiet) - throws IOException { - try { - return input.peekFully(output, offset, length, /* allowEndOfInput= */ quiet); - } catch (EOFException e) { - if (quiet) { - return false; - } else { - throw e; - } - } - } } diff --git a/library/extractor/src/test/java/com/google/android/exoplayer2/extractor/ExtractorUtilTest.java b/library/extractor/src/test/java/com/google/android/exoplayer2/extractor/ExtractorUtilTest.java index 9604b48bee4..8959caa3232 100644 --- a/library/extractor/src/test/java/com/google/android/exoplayer2/extractor/ExtractorUtilTest.java +++ b/library/extractor/src/test/java/com/google/android/exoplayer2/extractor/ExtractorUtilTest.java @@ -16,12 +16,14 @@ package com.google.android.exoplayer2.extractor; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; import android.net.Uri; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.testutil.FakeDataSource; import com.google.android.exoplayer2.upstream.DataSpec; +import java.io.EOFException; import java.util.Arrays; import org.junit.Test; import org.junit.runner.RunWith; @@ -34,12 +36,12 @@ public class ExtractorUtilTest { private static final byte[] TEST_DATA = new byte[] {0, 1, 2, 3, 4, 5, 6, 7, 8}; @Test - public void peekToLengthEndNotReached() throws Exception { + public void peekToLength_endNotReached() throws Exception { FakeDataSource testDataSource = new FakeDataSource(); testDataSource .getDataSet() .newDefaultData() - .appendReadData(Arrays.copyOfRange(TEST_DATA, 0, 3)) + .appendReadData(Arrays.copyOf(TEST_DATA, 3)) .appendReadData(Arrays.copyOfRange(TEST_DATA, 3, 6)) .appendReadData(Arrays.copyOfRange(TEST_DATA, 6, 9)); testDataSource.open(new DataSpec(Uri.parse(TEST_URI))); @@ -57,12 +59,12 @@ public void peekToLengthEndNotReached() throws Exception { } @Test - public void peekToLengthEndReached() throws Exception { + public void peekToLength_endReached() throws Exception { FakeDataSource testDataSource = new FakeDataSource(); testDataSource .getDataSet() .newDefaultData() - .appendReadData(Arrays.copyOfRange(TEST_DATA, 0, 3)) + .appendReadData(Arrays.copyOf(TEST_DATA, 3)) .appendReadData(Arrays.copyOfRange(TEST_DATA, 3, 6)) .appendReadData(Arrays.copyOfRange(TEST_DATA, 6, 9)); testDataSource.open(new DataSpec(Uri.parse(TEST_URI))); @@ -77,4 +79,135 @@ public void peekToLengthEndReached() throws Exception { assertThat(input.getPeekPosition()).isEqualTo(TEST_DATA.length); assertThat(target).isEqualTo(TEST_DATA); } + + @Test + public void readFullyQuietly_endNotReached_isTrueAndReadsData() throws Exception { + FakeDataSource testDataSource = new FakeDataSource(); + testDataSource + .getDataSet() + .newDefaultData() + .appendReadData(Arrays.copyOf(TEST_DATA, 3)) + .appendReadData(Arrays.copyOfRange(TEST_DATA, 3, 6)) + .appendReadData(Arrays.copyOfRange(TEST_DATA, 6, 9)); + testDataSource.open(new DataSpec(Uri.parse(TEST_URI))); + ExtractorInput input = new DefaultExtractorInput(testDataSource, 0, C.LENGTH_UNSET); + byte[] target = new byte[TEST_DATA.length]; + int offset = 2; + int length = 4; + + boolean hasRead = ExtractorUtil.readFullyQuietly(input, target, offset, length); + + assertThat(hasRead).isTrue(); + assertThat(input.getPosition()).isEqualTo(length); + assertThat(Arrays.copyOfRange(target, offset, offset + length)) + .isEqualTo(Arrays.copyOf(TEST_DATA, length)); + } + + @Test + public void readFullyQuietly_endReached_isFalse() throws Exception { + FakeDataSource testDataSource = new FakeDataSource(); + testDataSource.getDataSet().newDefaultData().appendReadData(Arrays.copyOf(TEST_DATA, 3)); + testDataSource.open(new DataSpec(Uri.parse(TEST_URI))); + ExtractorInput input = new DefaultExtractorInput(testDataSource, 0, C.LENGTH_UNSET); + byte[] target = new byte[TEST_DATA.length]; + int offset = 0; + int length = TEST_DATA.length + 1; + + boolean hasRead = ExtractorUtil.readFullyQuietly(input, target, offset, length); + + assertThat(hasRead).isFalse(); + assertThat(input.getPosition()).isEqualTo(0); + } + + @Test + public void skipFullyQuietly_endNotReached_isTrueAndSkipsData() throws Exception { + FakeDataSource testDataSource = new FakeDataSource(); + testDataSource + .getDataSet() + .newDefaultData() + .appendReadData(Arrays.copyOf(TEST_DATA, 3)) + .appendReadData(Arrays.copyOfRange(TEST_DATA, 3, 6)) + .appendReadData(Arrays.copyOfRange(TEST_DATA, 6, 9)); + testDataSource.open(new DataSpec(Uri.parse(TEST_URI))); + ExtractorInput input = new DefaultExtractorInput(testDataSource, 0, C.LENGTH_UNSET); + int length = 4; + + boolean hasRead = ExtractorUtil.skipFullyQuietly(input, length); + + assertThat(hasRead).isTrue(); + assertThat(input.getPosition()).isEqualTo(length); + } + + @Test + public void skipFullyQuietly_endReached_isFalse() throws Exception { + FakeDataSource testDataSource = new FakeDataSource(); + testDataSource.getDataSet().newDefaultData().appendReadData(Arrays.copyOf(TEST_DATA, 3)); + testDataSource.open(new DataSpec(Uri.parse(TEST_URI))); + ExtractorInput input = new DefaultExtractorInput(testDataSource, 0, C.LENGTH_UNSET); + int length = TEST_DATA.length + 1; + + boolean hasRead = ExtractorUtil.skipFullyQuietly(input, length); + + assertThat(hasRead).isFalse(); + assertThat(input.getPosition()).isEqualTo(0); + } + + @Test + public void peekFullyQuietly_endNotReached_isTrueAndPeeksData() throws Exception { + FakeDataSource testDataSource = new FakeDataSource(); + testDataSource + .getDataSet() + .newDefaultData() + .appendReadData(Arrays.copyOf(TEST_DATA, 3)) + .appendReadData(Arrays.copyOfRange(TEST_DATA, 3, 6)) + .appendReadData(Arrays.copyOfRange(TEST_DATA, 6, 9)); + testDataSource.open(new DataSpec(Uri.parse(TEST_URI))); + ExtractorInput input = new DefaultExtractorInput(testDataSource, 0, C.LENGTH_UNSET); + byte[] target = new byte[TEST_DATA.length]; + int offset = 2; + int length = 4; + + boolean hasRead = + ExtractorUtil.peekFullyQuietly(input, target, offset, length, /* allowEndOfInput= */ false); + + assertThat(hasRead).isTrue(); + assertThat(input.getPeekPosition()).isEqualTo(length); + assertThat(Arrays.copyOfRange(target, offset, offset + length)) + .isEqualTo(Arrays.copyOf(TEST_DATA, length)); + } + + @Test + public void peekFullyQuietly_endReachedWithEndOfInputAllowed_isFalse() throws Exception { + FakeDataSource testDataSource = new FakeDataSource(); + testDataSource.getDataSet().newDefaultData().appendReadData(Arrays.copyOf(TEST_DATA, 3)); + testDataSource.open(new DataSpec(Uri.parse(TEST_URI))); + ExtractorInput input = new DefaultExtractorInput(testDataSource, 0, C.LENGTH_UNSET); + byte[] target = new byte[TEST_DATA.length]; + int offset = 0; + int length = TEST_DATA.length + 1; + + boolean hasRead = + ExtractorUtil.peekFullyQuietly(input, target, offset, length, /* allowEndOfInput= */ true); + + assertThat(hasRead).isFalse(); + assertThat(input.getPeekPosition()).isEqualTo(0); + } + + @Test + public void peekFullyQuietly_endReachedWithoutEndOfInputAllowed_throws() throws Exception { + FakeDataSource testDataSource = new FakeDataSource(); + testDataSource.getDataSet().newDefaultData().appendReadData(Arrays.copyOf(TEST_DATA, 3)); + testDataSource.open(new DataSpec(Uri.parse(TEST_URI))); + ExtractorInput input = new DefaultExtractorInput(testDataSource, 0, C.LENGTH_UNSET); + byte[] target = new byte[TEST_DATA.length]; + int offset = 0; + int length = TEST_DATA.length + 1; + + assertThrows( + EOFException.class, + () -> + ExtractorUtil.peekFullyQuietly( + input, target, offset, length, /* allowEndOfInput= */ false)); + assertThat(input.getPeekPosition()).isEqualTo(0); + } }