From 66e416b61537ab7af0891c4ecb0b1674eacd7be3 Mon Sep 17 00:00:00 2001 From: olly Date: Mon, 8 Apr 2019 13:20:03 +0100 Subject: [PATCH] Fix bug when caching from non-zero position The position should be subtracted from the total content length retrieved from the cache in this case, both for iteration and for filling out counters.contentLength (for the latter, note not doing it this way is inconsistent with what happens when dataSpec length is set to a known value). PiperOrigin-RevId: 242445034 --- .../exoplayer2/upstream/cache/CacheUtil.java | 79 +++++++++++-------- .../upstream/cache/CacheUtilTest.java | 18 +++++ 2 files changed, 64 insertions(+), 33 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheUtil.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheUtil.java index 96561260ed..f715da118b 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheUtil.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheUtil.java @@ -95,17 +95,21 @@ public final class CacheUtil { @Nullable CacheKeyFactory cacheKeyFactory, CachingCounters counters) { String key = buildCacheKey(dataSpec, cacheKeyFactory); - long start = dataSpec.absoluteStreamPosition; - long left = - dataSpec.length != C.LENGTH_UNSET - ? dataSpec.length - : ContentMetadata.getContentLength(cache.getContentMetadata(key)); - counters.contentLength = left; + long position = dataSpec.absoluteStreamPosition; + long bytesLeft; + if (dataSpec.length != C.LENGTH_UNSET) { + bytesLeft = dataSpec.length; + } else { + long contentLength = ContentMetadata.getContentLength(cache.getContentMetadata(key)); + bytesLeft = contentLength == C.LENGTH_UNSET ? C.LENGTH_UNSET : contentLength - position; + } + counters.contentLength = bytesLeft; counters.alreadyCachedBytes = 0; counters.newlyCachedBytes = 0; - while (left != 0) { + while (bytesLeft != 0) { long blockLength = - cache.getCachedLength(key, start, left != C.LENGTH_UNSET ? left : Long.MAX_VALUE); + cache.getCachedLength( + key, position, bytesLeft != C.LENGTH_UNSET ? bytesLeft : Long.MAX_VALUE); if (blockLength > 0) { counters.alreadyCachedBytes += blockLength; } else { @@ -114,8 +118,8 @@ public final class CacheUtil { return; } } - start += blockLength; - left -= left == C.LENGTH_UNSET ? 0 : blockLength; + position += blockLength; + bytesLeft -= bytesLeft == C.LENGTH_UNSET ? 0 : blockLength; } counters.updatePercentage(); } @@ -203,15 +207,19 @@ public final class CacheUtil { } String key = buildCacheKey(dataSpec, cacheKeyFactory); - long start = dataSpec.absoluteStreamPosition; - long left = - dataSpec.length != C.LENGTH_UNSET - ? dataSpec.length - : ContentMetadata.getContentLength(cache.getContentMetadata(key)); - while (left != 0) { + long position = dataSpec.absoluteStreamPosition; + long bytesLeft; + if (dataSpec.length != C.LENGTH_UNSET) { + bytesLeft = dataSpec.length; + } else { + long contentLength = ContentMetadata.getContentLength(cache.getContentMetadata(key)); + bytesLeft = contentLength == C.LENGTH_UNSET ? C.LENGTH_UNSET : contentLength - position; + } + while (bytesLeft != 0) { throwExceptionIfInterruptedOrCancelled(isCanceled); long blockLength = - cache.getCachedLength(key, start, left != C.LENGTH_UNSET ? left : Long.MAX_VALUE); + cache.getCachedLength( + key, position, bytesLeft != C.LENGTH_UNSET ? bytesLeft : Long.MAX_VALUE); if (blockLength > 0) { // Skip already cached data. } else { @@ -220,7 +228,7 @@ public final class CacheUtil { long read = readAndDiscard( dataSpec, - start, + position, blockLength, dataSource, buffer, @@ -230,14 +238,14 @@ public final class CacheUtil { isCanceled); if (read < blockLength) { // Reached to the end of the data. - if (enableEOFException && left != C.LENGTH_UNSET) { + if (enableEOFException && bytesLeft != C.LENGTH_UNSET) { throw new EOFException(); } break; } } - start += blockLength; - left -= left == C.LENGTH_UNSET ? 0 : blockLength; + position += blockLength; + bytesLeft -= bytesLeft == C.LENGTH_UNSET ? 0 : blockLength; } } @@ -269,6 +277,7 @@ public final class CacheUtil { CachingCounters counters, AtomicBoolean isCanceled) throws IOException, InterruptedException { + long positionOffset = absoluteStreamPosition - dataSpec.absoluteStreamPosition; while (true) { if (priorityTaskManager != null) { // Wait for any other thread with higher priority to finish its job. @@ -284,31 +293,35 @@ public final class CacheUtil { dataSpec.httpMethod, dataSpec.httpBody, absoluteStreamPosition, - dataSpec.position + absoluteStreamPosition - dataSpec.absoluteStreamPosition, + /* position= */ dataSpec.position + positionOffset, C.LENGTH_UNSET, dataSpec.key, dataSpec.flags); long resolvedLength = dataSource.open(dataSpec); if (counters.contentLength == C.LENGTH_UNSET && resolvedLength != C.LENGTH_UNSET) { - counters.contentLength = dataSpec.absoluteStreamPosition + resolvedLength; + counters.contentLength = positionOffset + resolvedLength; } - long totalRead = 0; - while (totalRead != length) { + long totalBytesRead = 0; + while (totalBytesRead != length) { throwExceptionIfInterruptedOrCancelled(isCanceled); - int read = dataSource.read(buffer, 0, - length != C.LENGTH_UNSET ? (int) Math.min(buffer.length, length - totalRead) - : buffer.length); - if (read == C.RESULT_END_OF_INPUT) { + int bytesRead = + dataSource.read( + buffer, + 0, + length != C.LENGTH_UNSET + ? (int) Math.min(buffer.length, length - totalBytesRead) + : buffer.length); + if (bytesRead == C.RESULT_END_OF_INPUT) { if (counters.contentLength == C.LENGTH_UNSET) { - counters.contentLength = dataSpec.absoluteStreamPosition + totalRead; + counters.contentLength = positionOffset + totalBytesRead; } break; } - totalRead += read; - counters.newlyCachedBytes += read; + totalBytesRead += bytesRead; + counters.newlyCachedBytes += bytesRead; counters.updatePercentage(); } - return totalRead; + return totalBytesRead; } catch (PriorityTaskManager.PriorityTooLowException exception) { // catch and try again } finally { diff --git a/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/CacheUtilTest.java b/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/CacheUtilTest.java index 6b58e25444..ba06862385 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/CacheUtilTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/CacheUtilTest.java @@ -176,6 +176,24 @@ public final class CacheUtilTest { assertCounters(counters, 300, 0, 1000); } + @Test + public void testGetCachedFromNonZeroPosition() throws Exception { + mockCache.contentLength = 1000; + mockCache.spansAndGaps = new int[] {100, 100, 200}; + CachingCounters counters = new CachingCounters(); + CacheUtil.getCached( + new DataSpec( + Uri.parse("test"), + /* absoluteStreamPosition= */ 100, + /* length= */ C.LENGTH_UNSET, + /* key= */ null), + mockCache, + /* cacheKeyFactory= */ null, + counters); + + assertCounters(counters, 200, 0, 900); + } + @Test public void testCache() throws Exception { FakeDataSet fakeDataSet = new FakeDataSet().setRandomData("test_data", 100);