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
This commit is contained in:
olly 2019-04-08 13:20:03 +01:00 committed by Oliver Woodman
parent 0daaba443c
commit 66e416b615
2 changed files with 64 additions and 33 deletions

View file

@ -95,17 +95,21 @@ public final class CacheUtil {
@Nullable CacheKeyFactory cacheKeyFactory, @Nullable CacheKeyFactory cacheKeyFactory,
CachingCounters counters) { CachingCounters counters) {
String key = buildCacheKey(dataSpec, cacheKeyFactory); String key = buildCacheKey(dataSpec, cacheKeyFactory);
long start = dataSpec.absoluteStreamPosition; long position = dataSpec.absoluteStreamPosition;
long left = long bytesLeft;
dataSpec.length != C.LENGTH_UNSET if (dataSpec.length != C.LENGTH_UNSET) {
? dataSpec.length bytesLeft = dataSpec.length;
: ContentMetadata.getContentLength(cache.getContentMetadata(key)); } else {
counters.contentLength = left; long contentLength = ContentMetadata.getContentLength(cache.getContentMetadata(key));
bytesLeft = contentLength == C.LENGTH_UNSET ? C.LENGTH_UNSET : contentLength - position;
}
counters.contentLength = bytesLeft;
counters.alreadyCachedBytes = 0; counters.alreadyCachedBytes = 0;
counters.newlyCachedBytes = 0; counters.newlyCachedBytes = 0;
while (left != 0) { while (bytesLeft != 0) {
long blockLength = 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) { if (blockLength > 0) {
counters.alreadyCachedBytes += blockLength; counters.alreadyCachedBytes += blockLength;
} else { } else {
@ -114,8 +118,8 @@ public final class CacheUtil {
return; return;
} }
} }
start += blockLength; position += blockLength;
left -= left == C.LENGTH_UNSET ? 0 : blockLength; bytesLeft -= bytesLeft == C.LENGTH_UNSET ? 0 : blockLength;
} }
counters.updatePercentage(); counters.updatePercentage();
} }
@ -203,15 +207,19 @@ public final class CacheUtil {
} }
String key = buildCacheKey(dataSpec, cacheKeyFactory); String key = buildCacheKey(dataSpec, cacheKeyFactory);
long start = dataSpec.absoluteStreamPosition; long position = dataSpec.absoluteStreamPosition;
long left = long bytesLeft;
dataSpec.length != C.LENGTH_UNSET if (dataSpec.length != C.LENGTH_UNSET) {
? dataSpec.length bytesLeft = dataSpec.length;
: ContentMetadata.getContentLength(cache.getContentMetadata(key)); } else {
while (left != 0) { long contentLength = ContentMetadata.getContentLength(cache.getContentMetadata(key));
bytesLeft = contentLength == C.LENGTH_UNSET ? C.LENGTH_UNSET : contentLength - position;
}
while (bytesLeft != 0) {
throwExceptionIfInterruptedOrCancelled(isCanceled); throwExceptionIfInterruptedOrCancelled(isCanceled);
long blockLength = 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) { if (blockLength > 0) {
// Skip already cached data. // Skip already cached data.
} else { } else {
@ -220,7 +228,7 @@ public final class CacheUtil {
long read = long read =
readAndDiscard( readAndDiscard(
dataSpec, dataSpec,
start, position,
blockLength, blockLength,
dataSource, dataSource,
buffer, buffer,
@ -230,14 +238,14 @@ public final class CacheUtil {
isCanceled); isCanceled);
if (read < blockLength) { if (read < blockLength) {
// Reached to the end of the data. // Reached to the end of the data.
if (enableEOFException && left != C.LENGTH_UNSET) { if (enableEOFException && bytesLeft != C.LENGTH_UNSET) {
throw new EOFException(); throw new EOFException();
} }
break; break;
} }
} }
start += blockLength; position += blockLength;
left -= left == C.LENGTH_UNSET ? 0 : blockLength; bytesLeft -= bytesLeft == C.LENGTH_UNSET ? 0 : blockLength;
} }
} }
@ -269,6 +277,7 @@ public final class CacheUtil {
CachingCounters counters, CachingCounters counters,
AtomicBoolean isCanceled) AtomicBoolean isCanceled)
throws IOException, InterruptedException { throws IOException, InterruptedException {
long positionOffset = absoluteStreamPosition - dataSpec.absoluteStreamPosition;
while (true) { while (true) {
if (priorityTaskManager != null) { if (priorityTaskManager != null) {
// Wait for any other thread with higher priority to finish its job. // Wait for any other thread with higher priority to finish its job.
@ -284,31 +293,35 @@ public final class CacheUtil {
dataSpec.httpMethod, dataSpec.httpMethod,
dataSpec.httpBody, dataSpec.httpBody,
absoluteStreamPosition, absoluteStreamPosition,
dataSpec.position + absoluteStreamPosition - dataSpec.absoluteStreamPosition, /* position= */ dataSpec.position + positionOffset,
C.LENGTH_UNSET, C.LENGTH_UNSET,
dataSpec.key, dataSpec.key,
dataSpec.flags); dataSpec.flags);
long resolvedLength = dataSource.open(dataSpec); long resolvedLength = dataSource.open(dataSpec);
if (counters.contentLength == C.LENGTH_UNSET && resolvedLength != C.LENGTH_UNSET) { if (counters.contentLength == C.LENGTH_UNSET && resolvedLength != C.LENGTH_UNSET) {
counters.contentLength = dataSpec.absoluteStreamPosition + resolvedLength; counters.contentLength = positionOffset + resolvedLength;
} }
long totalRead = 0; long totalBytesRead = 0;
while (totalRead != length) { while (totalBytesRead != length) {
throwExceptionIfInterruptedOrCancelled(isCanceled); throwExceptionIfInterruptedOrCancelled(isCanceled);
int read = dataSource.read(buffer, 0, int bytesRead =
length != C.LENGTH_UNSET ? (int) Math.min(buffer.length, length - totalRead) dataSource.read(
: buffer.length); buffer,
if (read == C.RESULT_END_OF_INPUT) { 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) { if (counters.contentLength == C.LENGTH_UNSET) {
counters.contentLength = dataSpec.absoluteStreamPosition + totalRead; counters.contentLength = positionOffset + totalBytesRead;
} }
break; break;
} }
totalRead += read; totalBytesRead += bytesRead;
counters.newlyCachedBytes += read; counters.newlyCachedBytes += bytesRead;
counters.updatePercentage(); counters.updatePercentage();
} }
return totalRead; return totalBytesRead;
} catch (PriorityTaskManager.PriorityTooLowException exception) { } catch (PriorityTaskManager.PriorityTooLowException exception) {
// catch and try again // catch and try again
} finally { } finally {

View file

@ -176,6 +176,24 @@ public final class CacheUtilTest {
assertCounters(counters, 300, 0, 1000); 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 @Test
public void testCache() throws Exception { public void testCache() throws Exception {
FakeDataSet fakeDataSet = new FakeDataSet().setRandomData("test_data", 100); FakeDataSet fakeDataSet = new FakeDataSet().setRandomData("test_data", 100);