Skip to content

SDK / exoplayer

Go to a project
Commit 5ce40fa0 by eguven Committed by Oliver Woodman
Options

Fix CacheDataSource trying to read more after EOS 

setBytesRemaining() doesn't work when called after closeCurrentSource()

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=182999254
parent 8b790288
Inline Side-by-side
Showing with 42 additions and 22 deletions
library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheDataSource.java
...@@ -240,15 +240,13 @@ public final class CacheDataSource implements DataSource { ...@@ -240,15 +240,13 @@ public final class CacheDataSource implements DataSource {
if (bytesRemaining != C.LENGTH_UNSET) { if (bytesRemaining != C.LENGTH_UNSET) {
bytesRemaining -= bytesRead; bytesRemaining -= bytesRead;
} }
} else { } else if (currentDataSpecLengthUnset) {
closeCurrentSource();
if (currentDataSpecLengthUnset) {
setBytesRemaining(0); setBytesRemaining(0);
} else if (bytesRemaining > 0 || bytesRemaining == C.LENGTH_UNSET) { } else if (bytesRemaining > 0 || bytesRemaining == C.LENGTH_UNSET) {
closeCurrentSource();
openNextSource(false); openNextSource(false);
return read(buffer, offset, readLength); return read(buffer, offset, readLength);
} }
}
return bytesRead; return bytesRead;
} catch (IOException e) { } catch (IOException e) {
if (currentDataSpecLengthUnset && isCausedByPositionOutOfRange(e)) { if (currentDataSpecLengthUnset && isCausedByPositionOutOfRange(e)) {
......
library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/CacheDataSourceTest.java
...@@ -15,10 +15,8 @@ ...@@ -15,10 +15,8 @@
*/ */
package com.google.android.exoplayer2.upstream.cache; package com.google.android.exoplayer2.upstream.cache;
import static com.google.android.exoplayer2.C.LENGTH_UNSET;
import static com.google.android.exoplayer2.upstream.cache.CacheAsserts.assertCacheEmpty; import static com.google.android.exoplayer2.upstream.cache.CacheAsserts.assertCacheEmpty;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static org.junit.Assert.fail; import static org.junit.Assert.fail;
import android.net.Uri; import android.net.Uri;
...@@ -35,7 +33,6 @@ import java.util.Arrays; ...@@ -35,7 +33,6 @@ import java.util.Arrays;
import java.util.NavigableSet; import java.util.NavigableSet;
import org.junit.After; import org.junit.After;
import org.junit.Before; import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.robolectric.RobolectricTestRunner; import org.robolectric.RobolectricTestRunner;
...@@ -97,10 +94,8 @@ public final class CacheDataSourceTest { ...@@ -97,10 +94,8 @@ public final class CacheDataSourceTest {
assertCacheAndRead(false, true); assertCacheAndRead(false, true);
} }
// Disabled test as we don't support caching of definitely unknown length content
@Ignore
@Test @Test
public void disabledTestCacheAndReadUnboundedRequestUnknownLength() throws Exception { public void testCacheAndReadUnboundedRequestUnknownLength() throws Exception {
assertCacheAndRead(true, true); assertCacheAndRead(true, true);
} }
...@@ -130,7 +125,7 @@ public final class CacheDataSourceTest { ...@@ -130,7 +125,7 @@ public final class CacheDataSourceTest {
// Read partial at EOS but don't cross it so length is unknown // Read partial at EOS but don't cross it so length is unknown
CacheDataSource cacheDataSource = createCacheDataSource(false, true); CacheDataSource cacheDataSource = createCacheDataSource(false, true);
assertReadData(cacheDataSource, true, TEST_DATA.length - 2, 2); assertReadData(cacheDataSource, true, TEST_DATA.length - 2, 2);
assertThat(cache.getContentLength(testDataKey)).isEqualTo(LENGTH_UNSET); assertThat(cache.getContentLength(testDataKey)).isEqualTo(C.LENGTH_UNSET);
// Now do an unbounded request for whole data. This will cause a bounded request from upstream. // Now do an unbounded request for whole data. This will cause a bounded request from upstream.
// End of data from upstream shouldn't be mixed up with EOS and cause length set wrong. // End of data from upstream shouldn't be mixed up with EOS and cause length set wrong.
...@@ -140,21 +135,48 @@ public final class CacheDataSourceTest { ...@@ -140,21 +135,48 @@ public final class CacheDataSourceTest {
// Now the length set correctly do an unbounded request with offset // Now the length set correctly do an unbounded request with offset
assertThat( assertThat(
cacheDataSource.open( cacheDataSource.open(
new DataSpec(testDataUri, TEST_DATA.length - 2, LENGTH_UNSET, testDataKey))) new DataSpec(testDataUri, TEST_DATA.length - 2, C.LENGTH_UNSET, testDataKey)))
.isEqualTo(2); .isEqualTo(2);
// An unbounded request with offset for not cached content // An unbounded request with offset for not cached content
assertThat( assertThat(
cacheDataSource.open( cacheDataSource.open(
new DataSpec(Uri.parse("notCachedUri"), TEST_DATA.length - 2, LENGTH_UNSET, null))) new DataSpec(
.isEqualTo(LENGTH_UNSET); Uri.parse("notCachedUri"), TEST_DATA.length - 2, C.LENGTH_UNSET, null)))
.isEqualTo(C.LENGTH_UNSET);
}
@Test
public void testCantResolvedLengthContentReadInOneConnectionAndLengthIsResolved()
throws Exception {
FakeDataSource upstream = new FakeDataSource();
upstream
.getDataSet()
.newData(testDataUri)
.appendReadData(TEST_DATA)
.setSimulateUnknownLength(true);
CacheDataSource cacheDataSource = new CacheDataSource(cache, upstream, 0);
cacheDataSource.open(new DataSpec(testDataUri, 0, C.LENGTH_UNSET, testDataKey));
TestUtil.readToEnd(cacheDataSource);
cacheDataSource.close();
assertThat(upstream.getAndClearOpenedDataSpecs()).hasLength(1);
assertThat(cache.getContentLength(testDataKey)).isEqualTo(TEST_DATA.length);
} }
@Test @Test
public void testIgnoreCacheForUnsetLengthRequests() throws Exception { public void testIgnoreCacheForUnsetLengthRequests() throws Exception {
CacheDataSource cacheDataSource = createCacheDataSource(false, true, FakeDataSource upstream = new FakeDataSource();
CacheDataSource.FLAG_IGNORE_CACHE_FOR_UNSET_LENGTH_REQUESTS); upstream.getDataSet().setData(testDataUri, TEST_DATA);
assertReadData(cacheDataSource, true, 0, C.LENGTH_UNSET); CacheDataSource cacheDataSource =
new CacheDataSource(
cache, upstream, CacheDataSource.FLAG_IGNORE_CACHE_FOR_UNSET_LENGTH_REQUESTS);
cacheDataSource.open(new DataSpec(testDataUri, 0, C.LENGTH_UNSET, testDataKey));
TestUtil.readToEnd(cacheDataSource);
cacheDataSource.close();
assertThat(cache.getKeys()).isEmpty(); assertThat(cache.getKeys()).isEmpty();
} }
...@@ -347,10 +369,8 @@ public final class CacheDataSourceTest { ...@@ -347,10 +369,8 @@ public final class CacheDataSourceTest {
boolean unboundedRequest, boolean unknownLength) throws IOException { boolean unboundedRequest, boolean unknownLength) throws IOException {
int length = unboundedRequest ? C.LENGTH_UNSET : TEST_DATA.length; int length = unboundedRequest ? C.LENGTH_UNSET : TEST_DATA.length;
assertReadData(cacheDataSource, unknownLength, 0, length); assertReadData(cacheDataSource, unknownLength, 0, length);
assertWithMessage( // If !unboundedRequest, CacheDataSource doesn't reach EOS so shouldn't cache content length
"When the range specified, CacheDataSource doesn't reach EOS so shouldn't " assertThat(cache.getContentLength(testDataKey))
+ "cache content length")
.that(cache.getContentLength(testDataKey))
.isEqualTo(!unboundedRequest ? C.LENGTH_UNSET : TEST_DATA.length); .isEqualTo(!unboundedRequest ? C.LENGTH_UNSET : TEST_DATA.length);
} }
...@@ -360,7 +380,9 @@ public final class CacheDataSourceTest { ...@@ -360,7 +380,9 @@ public final class CacheDataSourceTest {
if (length != C.LENGTH_UNSET) { if (length != C.LENGTH_UNSET) {
testDataLength = Math.min(testDataLength, length); testDataLength = Math.min(testDataLength, length);
} }
DataSpec dataSpec = new DataSpec(testDataUri, position, length, testDataKey); DataSpec dataSpec =
new DataSpec(
testDataUri, position, length, testDataKey, DataSpec.FLAG_ALLOW_CACHING_UNKNOWN_LENGTH);
assertThat(cacheDataSource.open(dataSpec)).isEqualTo(unknownLength ? length : testDataLength); assertThat(cacheDataSource.open(dataSpec)).isEqualTo(unknownLength ? length : testDataLength);
cacheDataSource.close(); cacheDataSource.close();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment