Skip to content

SDK / exoplayer

Go to a project
Commit 92e2581e by eguven Committed by Oliver Woodman
Options

Fix CacheUtil.cache() use too much data 

cache() opens all connections with unset length to avoid position errors.
This makes more data then needed to be downloading by the underlying
network stack.

This fix makes makes it open connections for only required length.

Issue:#5927
PiperOrigin-RevId: 250546175
parent 25e93a17
Inline Side-by-side
Showing with 69 additions and 47 deletions
RELEASENOTES.md
...@@ -10,17 +10,26 @@ ...@@ -10,17 +10,26 @@
`PlayerControlView`. `PlayerControlView`.
* Change playback controls toggle from touch down to touch up events * Change playback controls toggle from touch down to touch up events
([#5784](https://github.com/google/ExoPlayer/issues/5784)). ([#5784](https://github.com/google/ExoPlayer/issues/5784)).
<<<<<<< HEAD
* Fix issue where playback controls were not kept visible on key presses * Fix issue where playback controls were not kept visible on key presses
([#5963](https://github.com/google/ExoPlayer/issues/5963)). ([#5963](https://github.com/google/ExoPlayer/issues/5963)).
=======
* Add a workaround for broken raw audio decoding on Oppo R9
([#5782](https://github.com/google/ExoPlayer/issues/5782)).
* Offline:
* Add Scheduler implementation which uses WorkManager.
* Prevent unexpected `DownloadHelper.Callback.onPrepared` callbacks after the
preparation of the `DownloadHelper` failed
([#5915](https://github.com/google/ExoPlayer/issues/5915)).
* Fix CacheUtil.cache() use too much data
([#5927](https://github.com/google/ExoPlayer/issues/5927)).
>>>>>>> 42ba6abf5... Fix CacheUtil.cache() use too much data
* Add a playWhenReady flag to MediaSessionConnector.PlaybackPreparer methods * Add a playWhenReady flag to MediaSessionConnector.PlaybackPreparer methods
to indicate whether a controller sent a play or only a prepare command. This to indicate whether a controller sent a play or only a prepare command. This
allows to take advantage of decoder reuse with the MediaSessionConnector allows to take advantage of decoder reuse with the MediaSessionConnector
([#5891](https://github.com/google/ExoPlayer/issues/5891)). ([#5891](https://github.com/google/ExoPlayer/issues/5891)).
* Add ProgressUpdateListener to PlayerControlView * Add ProgressUpdateListener to PlayerControlView
([#5834](https://github.com/google/ExoPlayer/issues/5834)). ([#5834](https://github.com/google/ExoPlayer/issues/5834)).
* Prevent unexpected `DownloadHelper.Callback.onPrepared` callbacks after the
preparation of the `DownloadHelper` failed
([#5915](https://github.com/google/ExoPlayer/issues/5915)).
* Allow enabling decoder fallback with `DefaultRenderersFactory` * Allow enabling decoder fallback with `DefaultRenderersFactory`
([#5942](https://github.com/google/ExoPlayer/issues/5942)). ([#5942](https://github.com/google/ExoPlayer/issues/5942)).
* Fix bug caused by parallel adaptive track selection using `Format`s without * Fix bug caused by parallel adaptive track selection using `Format`s without
......
library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheDataSource.java
...@@ -134,9 +134,9 @@ public final class CacheDataSource implements DataSource { ...@@ -134,9 +134,9 @@ public final class CacheDataSource implements DataSource {
private @Nullable DataSource currentDataSource; private @Nullable DataSource currentDataSource;
private boolean currentDataSpecLengthUnset; private boolean currentDataSpecLengthUnset;
private @Nullable Uri uri; @Nullable private Uri uri;
private @Nullable Uri actualUri; @Nullable private Uri actualUri;
private @HttpMethod int httpMethod; @HttpMethod private int httpMethod;
private int flags; private int flags;
private @Nullable String key; private @Nullable String key;
private long readPosition; private long readPosition;
...@@ -319,7 +319,7 @@ public final class CacheDataSource implements DataSource { ...@@ -319,7 +319,7 @@ public final class CacheDataSource implements DataSource {
} }
return bytesRead; return bytesRead;
} catch (IOException e) { } catch (IOException e) {
if (currentDataSpecLengthUnset && isCausedByPositionOutOfRange(e)) { if (currentDataSpecLengthUnset && CacheUtil.isCausedByPositionOutOfRange(e)) {
setNoBytesRemainingAndMaybeStoreLength(); setNoBytesRemainingAndMaybeStoreLength();
return C.RESULT_END_OF_INPUT; return C.RESULT_END_OF_INPUT;
} }
...@@ -484,20 +484,6 @@ public final class CacheDataSource implements DataSource { ...@@ -484,20 +484,6 @@ public final class CacheDataSource implements DataSource {
return redirectedUri != null ? redirectedUri : defaultUri; return redirectedUri != null ? redirectedUri : defaultUri;
} }
private static boolean isCausedByPositionOutOfRange(IOException e) {
Throwable cause = e;
while (cause != null) {
if (cause instanceof DataSourceException) {
int reason = ((DataSourceException) cause).reason;
if (reason == DataSourceException.POSITION_OUT_OF_RANGE) {
return true;
}
}
cause = cause.getCause();
}
return false;
}
private boolean isReadingFromUpstream() { private boolean isReadingFromUpstream() {
return !isReadingFromCache(); return !isReadingFromCache();
} }
......
library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheUtil.java
...@@ -20,6 +20,7 @@ import androidx.annotation.Nullable; ...@@ -20,6 +20,7 @@ import androidx.annotation.Nullable;
import android.util.Pair; import android.util.Pair;
import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.C;
import com.google.android.exoplayer2.upstream.DataSource; import com.google.android.exoplayer2.upstream.DataSource;
import com.google.android.exoplayer2.upstream.DataSourceException;
import com.google.android.exoplayer2.upstream.DataSpec; import com.google.android.exoplayer2.upstream.DataSpec;
import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Assertions;
import com.google.android.exoplayer2.util.PriorityTaskManager; import com.google.android.exoplayer2.util.PriorityTaskManager;
...@@ -195,37 +196,42 @@ public final class CacheUtil { ...@@ -195,37 +196,42 @@ public final class CacheUtil {
long contentLength = ContentMetadata.getContentLength(cache.getContentMetadata(key)); long contentLength = ContentMetadata.getContentLength(cache.getContentMetadata(key));
bytesLeft = contentLength == C.LENGTH_UNSET ? C.LENGTH_UNSET : contentLength - position; bytesLeft = contentLength == C.LENGTH_UNSET ? C.LENGTH_UNSET : contentLength - position;
} }
boolean lengthUnset = bytesLeft == C.LENGTH_UNSET;
while (bytesLeft != 0) { while (bytesLeft != 0) {
throwExceptionIfInterruptedOrCancelled(isCanceled); throwExceptionIfInterruptedOrCancelled(isCanceled);
long blockLength = long blockLength =
cache.getCachedLength( cache.getCachedLength(key, position, lengthUnset ? Long.MAX_VALUE : bytesLeft);
key, position, bytesLeft != C.LENGTH_UNSET ? bytesLeft : Long.MAX_VALUE);
if (blockLength > 0) { if (blockLength > 0) {
// Skip already cached data. // Skip already cached data.
} else { } else {
// There is a hole in the cache which is at least "-blockLength" long. // There is a hole in the cache which is at least "-blockLength" long.
blockLength = -blockLength; blockLength = -blockLength;
long length = blockLength == Long.MAX_VALUE ? C.LENGTH_UNSET : blockLength;
boolean isLastBlock = length == bytesLeft;
long read = long read =
readAndDiscard( readAndDiscard(
dataSpec, dataSpec,
position, position,
blockLength, length,
dataSource, dataSource,
buffer, buffer,
priorityTaskManager, priorityTaskManager,
priority, priority,
progressNotifier, progressNotifier,
isLastBlock,
isCanceled); isCanceled);
if (read < blockLength) { if (read < blockLength) {
// Reached to the end of the data. // Reached to the end of the data.
if (enableEOFException && bytesLeft != C.LENGTH_UNSET) { if (enableEOFException && !lengthUnset) {
throw new EOFException(); throw new EOFException();
} }
break; break;
} }
} }
position += blockLength; position += blockLength;
bytesLeft -= bytesLeft == C.LENGTH_UNSET ? 0 : blockLength; if (!lengthUnset) {
bytesLeft -= blockLength;
}
} }
} }
...@@ -242,6 +248,7 @@ public final class CacheUtil { ...@@ -242,6 +248,7 @@ public final class CacheUtil {
* caching. * caching.
* @param priority The priority of this task. * @param priority The priority of this task.
* @param progressNotifier A notifier through which to report progress updates, or {@code null}. * @param progressNotifier A notifier through which to report progress updates, or {@code null}.
* @param isLastBlock Whether this read block is the last block of the content.
* @param isCanceled An optional flag that will interrupt caching if set to true. * @param isCanceled An optional flag that will interrupt caching if set to true.
* @return Number of read bytes, or 0 if no data is available because the end of the opened range * @return Number of read bytes, or 0 if no data is available because the end of the opened range
* has been reached. * has been reached.
...@@ -255,6 +262,7 @@ public final class CacheUtil { ...@@ -255,6 +262,7 @@ public final class CacheUtil {
PriorityTaskManager priorityTaskManager, PriorityTaskManager priorityTaskManager,
int priority, int priority,
@Nullable ProgressNotifier progressNotifier, @Nullable ProgressNotifier progressNotifier,
boolean isLastBlock,
AtomicBoolean isCanceled) AtomicBoolean isCanceled)
throws IOException, InterruptedException { throws IOException, InterruptedException {
long positionOffset = absoluteStreamPosition - dataSpec.absoluteStreamPosition; long positionOffset = absoluteStreamPosition - dataSpec.absoluteStreamPosition;
...@@ -263,22 +271,23 @@ public final class CacheUtil { ...@@ -263,22 +271,23 @@ public final class CacheUtil {
// Wait for any other thread with higher priority to finish its job. // Wait for any other thread with higher priority to finish its job.
priorityTaskManager.proceed(priority); priorityTaskManager.proceed(priority);
} }
try {
throwExceptionIfInterruptedOrCancelled(isCanceled); throwExceptionIfInterruptedOrCancelled(isCanceled);
// Create a new dataSpec setting length to C.LENGTH_UNSET to prevent getting an error in try {
// case the given length exceeds the end of input. long resolvedLength;
dataSpec = try {
new DataSpec( resolvedLength = dataSource.open(dataSpec.subrange(positionOffset, length));
dataSpec.uri, } catch (IOException exception) {
dataSpec.httpMethod, if (length == C.LENGTH_UNSET
dataSpec.httpBody, || !isLastBlock
absoluteStreamPosition, || !isCausedByPositionOutOfRange(exception)) {
/* position= */ dataSpec.position + positionOffset, throw exception;
C.LENGTH_UNSET, }
dataSpec.key, Util.closeQuietly(dataSource);
dataSpec.flags); // Retry to open the data source again, setting length to C.LENGTH_UNSET to prevent
long resolvedLength = dataSource.open(dataSpec); // getting an error in case the given length exceeds the end of input.
if (progressNotifier != null && resolvedLength != C.LENGTH_UNSET) { resolvedLength = dataSource.open(dataSpec.subrange(positionOffset, C.LENGTH_UNSET));
}
if (isLastBlock && progressNotifier != null && resolvedLength != C.LENGTH_UNSET) {
progressNotifier.onRequestLengthResolved(positionOffset + resolvedLength); progressNotifier.onRequestLengthResolved(positionOffset + resolvedLength);
} }
long totalBytesRead = 0; long totalBytesRead = 0;
...@@ -340,6 +349,20 @@ public final class CacheUtil { ...@@ -340,6 +349,20 @@ public final class CacheUtil {
} }
} }
/*package*/ static boolean isCausedByPositionOutOfRange(IOException e) {
Throwable cause = e;
while (cause != null) {
if (cause instanceof DataSourceException) {
int reason = ((DataSourceException) cause).reason;
if (reason == DataSourceException.POSITION_OUT_OF_RANGE) {
return true;
}
}
cause = cause.getCause();
}
return false;
}
private static String buildCacheKey( private static String buildCacheKey(
DataSpec dataSpec, @Nullable CacheKeyFactory cacheKeyFactory) { DataSpec dataSpec, @Nullable CacheKeyFactory cacheKeyFactory) {
return (cacheKeyFactory != null ? cacheKeyFactory : DEFAULT_CACHE_KEY_FACTORY) return (cacheKeyFactory != null ? cacheKeyFactory : DEFAULT_CACHE_KEY_FACTORY)
......
testutils_robolectric/src/main/java/com/google/android/exoplayer2/testutil/CacheAsserts.java
...@@ -83,7 +83,8 @@ public final class CacheAsserts { ...@@ -83,7 +83,8 @@ public final class CacheAsserts {
* @throws IOException If an error occurred reading from the Cache. * @throws IOException If an error occurred reading from the Cache.
*/ */
public static void assertDataCached(Cache cache, Uri uri, byte[] expected) throws IOException { public static void assertDataCached(Cache cache, Uri uri, byte[] expected) throws IOException {
DataSpec dataSpec = new DataSpec(uri); // TODO Make tests specify if the content length is stored in cache metadata.
DataSpec dataSpec = new DataSpec(uri, 0, expected.length, null, 0);
assertDataCached(cache, dataSpec, expected); assertDataCached(cache, dataSpec, expected);
} }
...@@ -95,15 +96,18 @@ public final class CacheAsserts { ...@@ -95,15 +96,18 @@ public final class CacheAsserts {
public static void assertDataCached(Cache cache, DataSpec dataSpec, byte[] expected) public static void assertDataCached(Cache cache, DataSpec dataSpec, byte[] expected)
throws IOException { throws IOException {
DataSource dataSource = new CacheDataSource(cache, DummyDataSource.INSTANCE, 0); DataSource dataSource = new CacheDataSource(cache, DummyDataSource.INSTANCE, 0);
dataSource.open(dataSpec); byte[] bytes;
try { try {
byte[] bytes = TestUtil.readToEnd(dataSource); dataSource.open(dataSpec);
assertWithMessage("Cached data doesn't match expected for '" + dataSpec.uri + "',") bytes = TestUtil.readToEnd(dataSource);
.that(bytes) } catch (IOException e) {
.isEqualTo(expected); throw new IOException("Opening/reading cache failed: " + dataSpec, e);
} finally { } finally {
dataSource.close(); dataSource.close();
} }
assertWithMessage("Cached data doesn't match expected for '" + dataSpec.uri + "',")
.that(bytes)
.isEqualTo(expected);
} }
/** /**
......
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