From b77cc7c621bc6d7fe432ed0aef041829a41120c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20Wr=C3=B3tniak?= Date: Sat, 17 Jun 2017 16:18:43 +0200 Subject: [PATCH 1/4] Introduced failing unit test for ContentDataSource --- .../core/src/androidTest/AndroidManifest.xml | 3 + .../upstream/AndroidDataSourceTest.java | 31 +++++++++ .../exoplayer2/upstream/TestDataProvider.java | 64 +++++++++++++++++++ 3 files changed, 98 insertions(+) create mode 100644 library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AndroidDataSourceTest.java create mode 100644 library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/TestDataProvider.java diff --git a/library/core/src/androidTest/AndroidManifest.xml b/library/core/src/androidTest/AndroidManifest.xml index 2634152c98..0a90b071d2 100644 --- a/library/core/src/androidTest/AndroidManifest.xml +++ b/library/core/src/androidTest/AndroidManifest.xml @@ -24,6 +24,9 @@ android:allowBackup="false" tools:ignore="MissingApplicationIcon,HardcodedDebugMode"> + Date: Sat, 17 Jun 2017 16:26:29 +0200 Subject: [PATCH 2/4] InputStream creation for ContentDataSource changed --- .../google/android/exoplayer2/upstream/ContentDataSource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 f806f47410..5d0d9a80e9 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 @@ -71,7 +71,7 @@ public final class ContentDataSource implements DataSource { try { uri = dataSpec.uri; assetFileDescriptor = resolver.openAssetFileDescriptor(uri, "r"); - inputStream = new FileInputStream(assetFileDescriptor.getFileDescriptor()); + inputStream = assetFileDescriptor.createInputStream(); long skipped = inputStream.skip(dataSpec.position); if (skipped < dataSpec.position) { // We expect the skip to be satisfied in full. If it isn't then we're probably trying to From 50da6d870c1613c0e1bbcf67575ad30383f47593 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20Wr=C3=B3tniak?= Date: Mon, 19 Jun 2017 12:46:09 +0200 Subject: [PATCH 3/4] Comments from https://github.com/google/ExoPlayer/pull/2963#discussion_r122669328 applied --- .../upstream/AndroidDataSourceConstants.java | 8 +++++ .../upstream/AndroidDataSourceTest.java | 31 ------------------- .../upstream/AssetDataSourceTest.java | 21 +++++++++++++ .../upstream/ContentDataSourceTest.java | 21 +++++++++++++ .../upstream/ContentDataSource.java | 18 ++++++----- 5 files changed, 61 insertions(+), 38 deletions(-) create mode 100644 library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AndroidDataSourceConstants.java delete mode 100644 library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AndroidDataSourceTest.java create mode 100644 library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AssetDataSourceTest.java create mode 100644 library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/ContentDataSourceTest.java diff --git a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AndroidDataSourceConstants.java b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AndroidDataSourceConstants.java new file mode 100644 index 0000000000..ad19b7a824 --- /dev/null +++ b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AndroidDataSourceConstants.java @@ -0,0 +1,8 @@ +package com.google.android.exoplayer2.upstream; + +final class AndroidDataSourceConstants { + static final long SAMPLE_MP4_BYTES = 101597; + static final String SAMPLE_MP4_PATH = "/mp4/sample.mp4"; + + private AndroidDataSourceConstants() {} +} diff --git a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AndroidDataSourceTest.java b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AndroidDataSourceTest.java deleted file mode 100644 index 42dadaf379..0000000000 --- a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AndroidDataSourceTest.java +++ /dev/null @@ -1,31 +0,0 @@ -package com.google.android.exoplayer2.upstream; - -import android.content.Context; -import android.net.Uri; -import android.test.InstrumentationTestCase; - -public class AndroidDataSourceTest extends InstrumentationTestCase { - - private static final long SAMPLE_MP4_BYTES = 101597; - private static final String SAMPLE_MP4_PATH = "/mp4/sample.mp4"; - - public void testAssetDataSource() throws Exception { - final Context context = getInstrumentation().getContext(); - AssetDataSource dataSource = new AssetDataSource(context); - Uri assetUri = Uri.parse("file:///android_asset" + SAMPLE_MP4_PATH); - DataSpec dataSpec = new DataSpec(assetUri); - long sourceLengthBytes = dataSource.open(dataSpec); - - assertEquals(SAMPLE_MP4_BYTES, sourceLengthBytes); - } - - public void testContentDataSource() throws Exception { - Context context = getInstrumentation().getContext(); - ContentDataSource dataSource = new ContentDataSource(context); - Uri contentUri = Uri.parse("content://exoplayer" + SAMPLE_MP4_PATH); - DataSpec dataSpec = new DataSpec(contentUri); - long sourceLengthBytes = dataSource.open(dataSpec); - - assertEquals(SAMPLE_MP4_BYTES, sourceLengthBytes); - } -} 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 new file mode 100644 index 0000000000..178842bd4d --- /dev/null +++ b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AssetDataSourceTest.java @@ -0,0 +1,21 @@ +package com.google.android.exoplayer2.upstream; + +import android.content.Context; +import android.net.Uri; +import android.test.InstrumentationTestCase; + +import static com.google.android.exoplayer2.upstream.AndroidDataSourceConstants.SAMPLE_MP4_BYTES; +import static com.google.android.exoplayer2.upstream.AndroidDataSourceConstants.SAMPLE_MP4_PATH; + +public class AssetDataSourceTest extends InstrumentationTestCase { + + public void testAssetDataSource() throws Exception { + final Context context = getInstrumentation().getContext(); + AssetDataSource dataSource = new AssetDataSource(context); + Uri assetUri = Uri.parse("file:///android_asset" + SAMPLE_MP4_PATH); + DataSpec dataSpec = new DataSpec(assetUri); + long sourceLengthBytes = dataSource.open(dataSpec); + + assertEquals(SAMPLE_MP4_BYTES, sourceLengthBytes); + } +} 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 new file mode 100644 index 0000000000..b2edeea0cc --- /dev/null +++ b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/ContentDataSourceTest.java @@ -0,0 +1,21 @@ +package com.google.android.exoplayer2.upstream; + +import android.content.Context; +import android.net.Uri; +import android.test.InstrumentationTestCase; + +import static com.google.android.exoplayer2.upstream.AndroidDataSourceConstants.SAMPLE_MP4_BYTES; +import static com.google.android.exoplayer2.upstream.AndroidDataSourceConstants.SAMPLE_MP4_PATH; + +public class ContentDataSourceTest extends InstrumentationTestCase { + + public void testContentDataSource() throws Exception { + Context context = getInstrumentation().getContext(); + ContentDataSource dataSource = new ContentDataSource(context); + Uri contentUri = Uri.parse("content://exoplayer" + SAMPLE_MP4_PATH); + DataSpec dataSpec = new DataSpec(contentUri); + long sourceLengthBytes = dataSource.open(dataSpec); + + assertEquals(SAMPLE_MP4_BYTES, sourceLengthBytes); + } +} 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 5d0d9a80e9..9421b7ba03 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 @@ -71,7 +71,7 @@ public final class ContentDataSource implements DataSource { try { uri = dataSpec.uri; assetFileDescriptor = resolver.openAssetFileDescriptor(uri, "r"); - inputStream = assetFileDescriptor.createInputStream(); + inputStream = new FileInputStream(assetFileDescriptor.getFileDescriptor()); long skipped = inputStream.skip(dataSpec.position); if (skipped < dataSpec.position) { // We expect the skip to be satisfied in full. If it isn't then we're probably trying to @@ -81,12 +81,16 @@ public final class ContentDataSource implements DataSource { if (dataSpec.length != C.LENGTH_UNSET) { bytesRemaining = dataSpec.length; } else { - 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; + bytesRemaining = assetFileDescriptor.getLength(); + if (bytesRemaining == 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; + } } } } catch (IOException e) { From 86ff19b55bd4b028b4b5215ee120b04c10b9c351 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20Wr=C3=B3tniak?= Date: Mon, 19 Jun 2017 16:09:54 +0200 Subject: [PATCH 4/4] null AssetFileDescriptors support added in `ContentDataSource` --- .../upstream/AndroidDataSourceConstants.java | 8 +++++++ .../upstream/ContentDataSourceTest.java | 23 +++++++++++++++++-- .../exoplayer2/upstream/TestDataProvider.java | 4 ++++ .../upstream/ContentDataSource.java | 4 ++++ 4 files changed, 37 insertions(+), 2 deletions(-) diff --git a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AndroidDataSourceConstants.java b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AndroidDataSourceConstants.java index ad19b7a824..d11202ccf2 100644 --- a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AndroidDataSourceConstants.java +++ b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/AndroidDataSourceConstants.java @@ -1,8 +1,16 @@ package com.google.android.exoplayer2.upstream; +import android.content.ContentResolver; +import android.net.Uri; + final class AndroidDataSourceConstants { static final long SAMPLE_MP4_BYTES = 101597; static final String SAMPLE_MP4_PATH = "/mp4/sample.mp4"; + static final String TEST_DATA_PROVIDER_AUTHORITY = "exoplayer"; + static final Uri NULL_DESCRIPTOR_URI = new Uri.Builder() + .scheme(ContentResolver.SCHEME_CONTENT) + .authority(TEST_DATA_PROVIDER_AUTHORITY) + .build(); private AndroidDataSourceConstants() {} } 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 b2edeea0cc..1cd14b45e1 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 @@ -1,21 +1,40 @@ package com.google.android.exoplayer2.upstream; +import android.content.ContentResolver; import android.content.Context; import android.net.Uri; import android.test.InstrumentationTestCase; +import static com.google.android.exoplayer2.upstream.AndroidDataSourceConstants.NULL_DESCRIPTOR_URI; import static com.google.android.exoplayer2.upstream.AndroidDataSourceConstants.SAMPLE_MP4_BYTES; import static com.google.android.exoplayer2.upstream.AndroidDataSourceConstants.SAMPLE_MP4_PATH; +import static com.google.android.exoplayer2.upstream.AndroidDataSourceConstants.TEST_DATA_PROVIDER_AUTHORITY; public class ContentDataSourceTest extends InstrumentationTestCase { - public void testContentDataSource() throws Exception { + public void testValidContentDataSource() throws Exception { Context context = getInstrumentation().getContext(); ContentDataSource dataSource = new ContentDataSource(context); - Uri contentUri = Uri.parse("content://exoplayer" + SAMPLE_MP4_PATH); + Uri contentUri = new Uri.Builder() + .scheme(ContentResolver.SCHEME_CONTENT) + .authority(TEST_DATA_PROVIDER_AUTHORITY) + .path(SAMPLE_MP4_PATH).build(); DataSpec dataSpec = new DataSpec(contentUri); long sourceLengthBytes = dataSource.open(dataSpec); assertEquals(SAMPLE_MP4_BYTES, sourceLengthBytes); } + + public void testNullContentDataSource() throws Exception { + Context context = getInstrumentation().getContext(); + ContentDataSource dataSource = new ContentDataSource(context); + DataSpec dataSpec = new DataSpec(NULL_DESCRIPTOR_URI); + + try { + dataSource.open(dataSpec); + fail("Expected exception not thrown."); + } catch (ContentDataSource.ContentDataSourceException e) { + // Expected. + } + } } diff --git a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/TestDataProvider.java b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/TestDataProvider.java index 851e7b4b0c..f6e09a7067 100644 --- a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/TestDataProvider.java +++ b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/TestDataProvider.java @@ -29,6 +29,10 @@ public class TestDataProvider extends ContentProvider { @Nullable @Override public AssetFileDescriptor openAssetFile(@NonNull final Uri uri, @NonNull final String mode) throws FileNotFoundException { + if (uri.equals(AndroidDataSourceConstants.NULL_DESCRIPTOR_URI)) { + return null; + } + try { Context context = getContext(); assertNotNull(context); 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 9421b7ba03..3a9be552d2 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 @@ -22,6 +22,7 @@ import android.net.Uri; import com.google.android.exoplayer2.C; import java.io.EOFException; import java.io.FileInputStream; +import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; @@ -71,6 +72,9 @@ public final class ContentDataSource implements DataSource { try { uri = dataSpec.uri; assetFileDescriptor = resolver.openAssetFileDescriptor(uri, "r"); + if (assetFileDescriptor == null) { + throw new FileNotFoundException("Could not open file descriptor for: " + uri); + } inputStream = new FileInputStream(assetFileDescriptor.getFileDescriptor()); long skipped = inputStream.skip(dataSpec.position); if (skipped < dataSpec.position) {