From e6e75a536aad6d412dba4d749ea63f91a16726e6 Mon Sep 17 00:00:00 2001 From: olly Date: Mon, 6 Nov 2017 07:03:09 -0800 Subject: [PATCH] Don't use InputStream.available in ContentDataSource Issue: #3426 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=174700804 --- .../upstream/AssetDataSourceTest.java | 4 +- .../upstream/ContentDataSourceTest.java | 110 +++++++++++++----- .../upstream/ContentDataSource.java | 17 ++- .../android/exoplayer2/testutil/TestUtil.java | 19 ++- 4 files changed, 104 insertions(+), 46 deletions(-) diff --git a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AssetDataSourceTest.java b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AssetDataSourceTest.java index 102c89ec2b0..d582d25ab1b 100644 --- a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AssetDataSourceTest.java +++ b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AssetDataSourceTest.java @@ -30,14 +30,14 @@ public void testReadFileUri() throws Exception { AssetDataSource dataSource = new AssetDataSource(getInstrumentation().getContext()); DataSpec dataSpec = new DataSpec(Uri.parse("file:///android_asset/" + DATA_PATH)); TestUtil.assertDataSourceContent(dataSource, dataSpec, - TestUtil.getByteArray(getInstrumentation(), DATA_PATH)); + TestUtil.getByteArray(getInstrumentation(), DATA_PATH), true); } public void testReadAssetUri() throws Exception { AssetDataSource dataSource = new AssetDataSource(getInstrumentation().getContext()); DataSpec dataSpec = new DataSpec(Uri.parse("asset:///" + DATA_PATH)); TestUtil.assertDataSourceContent(dataSource, dataSpec, - TestUtil.getByteArray(getInstrumentation(), DATA_PATH)); + TestUtil.getByteArray(getInstrumentation(), DATA_PATH), true); } } 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 6a85483dd12..e19f7ad033e 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 @@ -15,17 +15,22 @@ */ package com.google.android.exoplayer2.upstream; +import android.app.Instrumentation; import android.content.ContentProvider; import android.content.ContentResolver; import android.content.ContentValues; import android.content.res.AssetFileDescriptor; import android.database.Cursor; import android.net.Uri; +import android.os.Bundle; +import android.os.ParcelFileDescriptor; import android.support.annotation.NonNull; +import android.support.annotation.Nullable; import android.test.InstrumentationTestCase; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.testutil.TestUtil; import java.io.FileNotFoundException; +import java.io.FileOutputStream; import java.io.IOException; import java.util.Arrays; @@ -37,23 +42,33 @@ 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"; - public void testReadValidUri() throws Exception { - ContentDataSource dataSource = new ContentDataSource(getInstrumentation().getContext()); - Uri contentUri = new Uri.Builder() - .scheme(ContentResolver.SCHEME_CONTENT) - .authority(AUTHORITY) - .path(DATA_PATH).build(); - DataSpec dataSpec = new DataSpec(contentUri); - TestUtil.assertDataSourceContent(dataSource, dataSpec, - TestUtil.getByteArray(getInstrumentation(), DATA_PATH)); + public void testRead() throws Exception { + assertData(getInstrumentation(), 0, C.LENGTH_UNSET, false); + } + + public void testReadPipeMode() throws Exception { + assertData(getInstrumentation(), 0, C.LENGTH_UNSET, true); + } + + public void testReadFixedLength() throws Exception { + assertData(getInstrumentation(), 0, 100, false); + } + + public void testReadFromOffsetToEndOfInput() throws Exception { + assertData(getInstrumentation(), 1, C.LENGTH_UNSET, false); + } + + public void testReadFromOffsetToEndOfInputPipeMode() throws Exception { + assertData(getInstrumentation(), 1, C.LENGTH_UNSET, true); + } + + public void testReadFromOffsetFixedLength() throws Exception { + assertData(getInstrumentation(), 1, 100, false); } public void testReadInvalidUri() throws Exception { ContentDataSource dataSource = new ContentDataSource(getInstrumentation().getContext()); - Uri contentUri = new Uri.Builder() - .scheme(ContentResolver.SCHEME_CONTENT) - .authority(AUTHORITY) - .build(); + Uri contentUri = TestContentProvider.buildUri("does/not.exist", false); DataSpec dataSpec = new DataSpec(contentUri); try { dataSource.open(dataSpec); @@ -66,18 +81,16 @@ public void testReadInvalidUri() throws Exception { } } - 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(); + private static void assertData(Instrumentation instrumentation, int offset, int length, + boolean pipeMode) throws IOException { + Uri contentUri = TestContentProvider.buildUri(DATA_PATH, pipeMode); + ContentDataSource dataSource = new ContentDataSource(instrumentation.getContext()); try { - int testOffset = 1; - DataSpec dataSpec = new DataSpec(contentUri, testOffset, C.LENGTH_UNSET, null); - byte[] completeData = TestUtil.getByteArray(getInstrumentation(), DATA_PATH); - byte[] expectedData = Arrays.copyOfRange(completeData, testOffset, completeData.length); - TestUtil.assertDataSourceContent(dataSource, dataSpec, expectedData); + DataSpec dataSpec = new DataSpec(contentUri, offset, length, null); + byte[] completeData = TestUtil.getByteArray(instrumentation, DATA_PATH); + byte[] expectedData = Arrays.copyOfRange(completeData, offset, + length == C.LENGTH_UNSET ? completeData.length : offset + length); + TestUtil.assertDataSourceContent(dataSource, dataSpec, expectedData, !pipeMode); } finally { dataSource.close(); } @@ -86,7 +99,21 @@ public void testReadFromOffsetToEndOfInput() throws Exception { /** * A {@link ContentProvider} for the test. */ - public static final class TestContentProvider extends ContentProvider { + public static final class TestContentProvider extends ContentProvider + implements ContentProvider.PipeDataWriter { + + private static final String PARAM_PIPE_MODE = "pipe-mode"; + + public static Uri buildUri(String filePath, boolean pipeMode) { + Uri.Builder builder = new Uri.Builder() + .scheme(ContentResolver.SCHEME_CONTENT) + .authority(AUTHORITY) + .path(filePath); + if (pipeMode) { + builder.appendQueryParameter(TestContentProvider.PARAM_PIPE_MODE, "1"); + } + return builder.build(); + } @Override public boolean onCreate() { @@ -106,7 +133,14 @@ public AssetFileDescriptor openAssetFile(@NonNull Uri uri, @NonNull String mode) return null; } try { - return getContext().getAssets().openFd(uri.getPath().replaceFirst("/", "")); + String fileName = getFileName(uri); + boolean pipeMode = uri.getQueryParameter(PARAM_PIPE_MODE) != null; + if (pipeMode) { + ParcelFileDescriptor fileDescriptor = openPipeHelper(uri, null, null, null, this); + return new AssetFileDescriptor(fileDescriptor, 0, C.LENGTH_UNSET); + } else { + return getContext().getAssets().openFd(fileName); + } } catch (IOException e) { FileNotFoundException exception = new FileNotFoundException(e.getMessage()); exception.initCause(e); @@ -125,17 +159,33 @@ public Uri insert(@NonNull Uri uri, ContentValues values) { } @Override - public int delete(@NonNull Uri uri, String selection, - String[] selectionArgs) { + public int delete(@NonNull Uri uri, String selection, String[] selectionArgs) { throw new UnsupportedOperationException(); } @Override - public int update(@NonNull Uri uri, ContentValues values, - String selection, String[] selectionArgs) { + public int update(@NonNull Uri uri, ContentValues values, String selection, + String[] selectionArgs) { throw new UnsupportedOperationException(); } + @Override + public void writeDataToPipe(@NonNull ParcelFileDescriptor output, @NonNull Uri uri, + @NonNull String mimeType, @Nullable Bundle opts, @Nullable Object args) { + try { + byte[] data = TestUtil.getByteArray(getContext(), getFileName(uri)); + FileOutputStream outputStream = new FileOutputStream(output.getFileDescriptor()); + outputStream.write(data); + outputStream.close(); + } catch (IOException e) { + throw new RuntimeException("Error writing to pipe", e); + } + } + + private static String getFileName(Uri uri) { + return uri.getPath().replaceFirst("/", ""); + } + } } 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 c37599eccc4..87642e0eba7 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 @@ -24,7 +24,7 @@ import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; -import java.io.InputStream; +import java.nio.channels.FileChannel; /** * A {@link DataSource} for reading from a content URI. @@ -47,7 +47,7 @@ public ContentDataSourceException(IOException cause) { private Uri uri; private AssetFileDescriptor assetFileDescriptor; - private InputStream inputStream; + private FileInputStream inputStream; private long bytesRemaining; private boolean opened; @@ -88,14 +88,11 @@ public long open(DataSpec dataSpec) throws ContentDataSourceException { } else { 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) { - // FileInputStream.available() returns 0 if the remaining length cannot be determined, - // or if it's greater than Integer.MAX_VALUE. We don't know the true length in either - // case, so treat as unbounded. - bytesRemaining = C.LENGTH_UNSET; - } + // The asset must extend to the end of the file. If FileInputStream.getChannel().size() + // returns 0 then the remaining length cannot be determined. + FileChannel channel = inputStream.getChannel(); + long channelSize = channel.size(); + bytesRemaining = channelSize == 0 ? C.LENGTH_UNSET : channelSize - channel.position(); } else { bytesRemaining = assetFileDescriptorLength - skipped; } 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 b5b084fc7bd..61d1ecaeea7 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 @@ -16,6 +16,7 @@ package com.google.android.exoplayer2.testutil; import android.app.Instrumentation; +import android.content.Context; import android.test.MoreAsserts; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.Timeline; @@ -121,12 +122,20 @@ public static byte[] joinByteArrays(byte[]... byteArrays) { public static byte[] getByteArray(Instrumentation instrumentation, String fileName) throws IOException { - return Util.toByteArray(getInputStream(instrumentation, fileName)); + return getByteArray(instrumentation.getContext(), fileName); + } + + public static byte[] getByteArray(Context context, String fileName) throws IOException { + return Util.toByteArray(getInputStream(context, fileName)); } public static InputStream getInputStream(Instrumentation instrumentation, String fileName) throws IOException { - return instrumentation.getContext().getResources().getAssets().open(fileName); + return getInputStream(instrumentation.getContext(), fileName); + } + + public static InputStream getInputStream(Context context, String fileName) throws IOException { + return context.getResources().getAssets().open(fileName); } public static String getString(Instrumentation instrumentation, String fileName) @@ -167,13 +176,15 @@ public synchronized void onSourceInfoRefreshed(MediaSource source, Timeline time * @param dataSource The {@link DataSource} through which to read. * @param dataSpec The {@link DataSpec} to use when opening the {@link DataSource}. * @param expectedData The expected data. + * @param expectKnownLength Whether to assert that {@link DataSource#open} returns the expected + * data length. If false then it's asserted that {@link C#LENGTH_UNSET} is returned. * @throws IOException If an error occurs reading fom the {@link DataSource}. */ public static void assertDataSourceContent(DataSource dataSource, DataSpec dataSpec, - byte[] expectedData) throws IOException { + byte[] expectedData, boolean expectKnownLength) throws IOException { try { long length = dataSource.open(dataSpec); - Assert.assertEquals(expectedData.length, length); + Assert.assertEquals(expectKnownLength ? expectedData.length : C.LENGTH_UNSET, length); byte[] readData = TestUtil.readToEnd(dataSource); MoreAsserts.assertEquals(expectedData, readData); } finally {