From 0b78837f35f54acc3ef41f5ceeb3b634df1cd142 Mon Sep 17 00:00:00 2001 From: andrewlewis Date: Wed, 30 Aug 2017 10:51:37 -0700 Subject: [PATCH] Fix ContentDataSource bytesRemaining calculation The bytesRemaining didn't always take into account any skipped bytes, which meant that reaching the end of the file was not correctly detected in read(). Issue: #3216 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=167016672 --- .../upstream/ContentDataSourceTest.java | 27 +++++++++++++++++++ .../upstream/ContentDataSource.java | 10 ++++--- .../android/exoplayer2/testutil/TestUtil.java | 2 +- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/ContentDataSourceTest.java b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/ContentDataSourceTest.java index 834e7e1374..2b70c83ca5 100644 --- a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/ContentDataSourceTest.java +++ b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/ContentDataSourceTest.java @@ -23,9 +23,12 @@ import android.database.Cursor; import android.net.Uri; import android.support.annotation.NonNull; import android.test.InstrumentationTestCase; +import android.test.MoreAsserts; +import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.testutil.TestUtil; import java.io.FileNotFoundException; import java.io.IOException; +import java.util.Arrays; /** * Unit tests for {@link ContentDataSource}. @@ -35,6 +38,9 @@ public final class ContentDataSourceTest extends InstrumentationTestCase { private static final String AUTHORITY = "com.google.android.exoplayer2.core.test"; private static final String DATA_PATH = "binary/1024_incrementing_bytes.mp3"; + private static final int TEST_DATA_OFFSET = 1; + private static final int TEST_DATA_LENGTH = 1023; + public void testReadValidUri() throws Exception { ContentDataSource dataSource = new ContentDataSource(getInstrumentation().getContext()); Uri contentUri = new Uri.Builder() @@ -64,6 +70,27 @@ public final class ContentDataSourceTest extends InstrumentationTestCase { } } + public void testReadFromOffsetToEndOfInput() throws Exception { + ContentDataSource dataSource = new ContentDataSource(getInstrumentation().getContext()); + Uri contentUri = new Uri.Builder() + .scheme(ContentResolver.SCHEME_CONTENT) + .authority(AUTHORITY) + .path(DATA_PATH).build(); + try { + DataSpec dataSpec = new DataSpec(contentUri, TEST_DATA_OFFSET, C.LENGTH_UNSET, null); + long length = dataSource.open(dataSpec); + assertEquals(TEST_DATA_LENGTH, length); + byte[] expectedData = Arrays.copyOfRange( + TestUtil.getByteArray(getInstrumentation(), DATA_PATH), TEST_DATA_OFFSET, + TEST_DATA_OFFSET + TEST_DATA_LENGTH); + byte[] readData = TestUtil.readToEnd(dataSource); + MoreAsserts.assertEquals(expectedData, readData); + assertEquals(C.RESULT_END_OF_INPUT, dataSource.read(new byte[1], 0, 1)); + } finally { + dataSource.close(); + } + } + /** * A {@link ContentProvider} for the test. */ diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/ContentDataSource.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/ContentDataSource.java index d118b91378..c37599eccc 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/ContentDataSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/ContentDataSource.java @@ -76,8 +76,8 @@ public final class ContentDataSource implements DataSource { throw new FileNotFoundException("Could not open file descriptor for: " + uri); } inputStream = new FileInputStream(assetFileDescriptor.getFileDescriptor()); - long assertStartOffset = assetFileDescriptor.getStartOffset(); - long skipped = inputStream.skip(assertStartOffset + dataSpec.position) - assertStartOffset; + long assetStartOffset = assetFileDescriptor.getStartOffset(); + long skipped = inputStream.skip(assetStartOffset + dataSpec.position) - assetStartOffset; if (skipped != dataSpec.position) { // We expect the skip to be satisfied in full. If it isn't then we're probably trying to // skip beyond the end of the data. @@ -86,8 +86,8 @@ public final class ContentDataSource implements DataSource { if (dataSpec.length != C.LENGTH_UNSET) { bytesRemaining = dataSpec.length; } else { - bytesRemaining = assetFileDescriptor.getLength(); - if (bytesRemaining == AssetFileDescriptor.UNKNOWN_LENGTH) { + long assetFileDescriptorLength = assetFileDescriptor.getLength(); + if (assetFileDescriptorLength == AssetFileDescriptor.UNKNOWN_LENGTH) { // The asset must extend to the end of the file. bytesRemaining = inputStream.available(); if (bytesRemaining == 0) { @@ -96,6 +96,8 @@ public final class ContentDataSource implements DataSource { // case, so treat as unbounded. bytesRemaining = C.LENGTH_UNSET; } + } else { + bytesRemaining = assetFileDescriptorLength - skipped; } } } catch (IOException e) { diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/TestUtil.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/TestUtil.java index 5819a4b711..2e59b33c0b 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/TestUtil.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/TestUtil.java @@ -181,7 +181,7 @@ public class TestUtil { byte[] expectedData) throws IOException { try { long length = dataSource.open(dataSpec); - Assert.assertEquals(length, expectedData.length); + Assert.assertEquals(expectedData.length, length); byte[] readData = TestUtil.readToEnd(dataSource); MoreAsserts.assertEquals(expectedData, readData); } finally {