From 6e4508daec142a73c9773f49554e13008eed3231 Mon Sep 17 00:00:00 2001 From: kimvde Date: Tue, 29 Jun 2021 15:35:02 +0100 Subject: [PATCH] Improve support for Ogg truncated content #minor-release Issue:#7608 PiperOrigin-RevId: 382081687 --- RELEASENOTES.md | 2 + .../exoplayer2/extractor/ExtractorUtil.java | 58 +++++++ .../extractor/ogg/DefaultOggSeeker.java | 22 ++- .../exoplayer2/extractor/ogg/OggPacket.java | 12 +- .../extractor/ogg/OggPageHeader.java | 42 ++---- .../extractor/ExtractorUtilTest.java | 141 +++++++++++++++++- 6 files changed, 231 insertions(+), 46 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 5bd2262baf..607c0b4f1c 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -83,6 +83,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 164c991597..54cc61f2ef 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 @@ -18,6 +18,7 @@ package com.google.android.exoplayer2.extractor; import androidx.annotation.Nullable; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.ParserException; +import java.io.EOFException; import java.io.IOException; import org.checkerframework.dataflow.qual.Pure; @@ -62,5 +63,62 @@ public final class ExtractorUtil { 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 b7a8039489..29a0f5932a 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 @@ import java.io.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 e9d0b8b4fc..b31d3769b2 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; @@ -51,7 +53,7 @@ import java.util.Arrays; * * @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 { @@ -76,7 +78,9 @@ import java.util.Arrays; bytesToSkip += calculatePacketSize(segmentIndex); segmentIndex += segmentCount; } - input.skipFully(bytesToSkip); + if (!skipFullyQuietly(input, bytesToSkip)) { + return false; + } currentSegmentIndex = segmentIndex; } @@ -84,7 +88,9 @@ import java.util.Arrays; 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 6e9102f501..306e64f559 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; /** Data object to store header information. */ @@ -102,7 +103,8 @@ import java.io.IOException; 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(); @@ -123,14 +125,13 @@ import java.io.IOException; * @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; } @@ -155,7 +156,9 @@ import java.io.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]; @@ -163,31 +166,4 @@ import java.io.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 df4f05f717..6734face12 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; @@ -36,12 +38,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))); @@ -59,12 +61,12 @@ public class ExtractorUtilTest { } @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))); @@ -79,4 +81,135 @@ public class ExtractorUtilTest { 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); + } }