Skip to content

SDK / exoplayer

Go to a project
Commit 1be295ab by olly Committed by tonihei
Options

Fix SimpleCache.getCachedLength rollover bug & improve test coverage 

PiperOrigin-RevId: 312266156
parent 6e02a815
Inline Side-by-side
Showing with 87 additions and 24 deletions
library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CachedContent.java
......@@ -15,8 +15,10 @@
*/
package com.google.android.exoplayer2.upstream.cache;
import static com.google.android.exoplayer2.util.Assertions.checkArgument;
import static com.google.android.exoplayer2.util.Assertions.checkState;
import androidx.annotation.Nullable;
import com.google.android.exoplayer2.util.Assertions;
import com.google.android.exoplayer2.util.Log;
import java.io.File;
import java.util.TreeSet;
......@@ -117,12 +119,18 @@ import java.util.TreeSet;
* isn't cached.
*/
public long getCachedBytesLength(long position, long length) {
checkArgument(position >= 0);
checkArgument(length >= 0);
SimpleCacheSpan span = getSpan(position);
if (span.isHoleSpan()) {
// We don't have a span covering the start of the queried region.
return -Math.min(span.isOpenEnded() ? Long.MAX_VALUE : span.length, length);
}
long queryEndPosition = position + length;
if (queryEndPosition < 0) {
// The calculation rolled over (length is probably Long.MAX_VALUE).
queryEndPosition = Long.MAX_VALUE;
}
long currentEndPosition = span.position + span.length;
if (currentEndPosition < queryEndPosition) {
for (SimpleCacheSpan next : cachedSpans.tailSet(span, false)) {
......@@ -153,7 +161,7 @@ import java.util.TreeSet;
*/
public SimpleCacheSpan setLastTouchTimestamp(
SimpleCacheSpan cacheSpan, long lastTouchTimestamp, boolean updateFile) {
Assertions.checkState(cachedSpans.remove(cacheSpan));
checkState(cachedSpans.remove(cacheSpan));
File file = cacheSpan.file;
if (updateFile) {
File directory = file.getParentFile();
......
library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/SimpleCacheTest.java
......@@ -300,38 +300,93 @@ public class SimpleCacheTest {
}
@Test
public void getCachedLength() throws Exception {
public void getCachedLength_noCachedContent_returnsNegativeMaxHoleLength() {
SimpleCache simpleCache = getSimpleCache();
CacheSpan cacheSpan = simpleCache.startReadWrite(KEY_1, 0);
// No cached bytes, returns -'length'
assertThat(simpleCache.getCachedLength(KEY_1, 0, 100)).isEqualTo(-100);
// Position value doesn't affect the return value
assertThat(simpleCache.getCachedLength(KEY_1, 20, 100)).isEqualTo(-100);
addCache(simpleCache, KEY_1, 0, 15);
// Returns the length of a single span
assertThat(simpleCache.getCachedLength(KEY_1, 0, 100)).isEqualTo(15);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 0, /* length= */ 100))
.isEqualTo(-100);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 0, /* length= */ Long.MAX_VALUE))
.isEqualTo(-Long.MAX_VALUE);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 20, /* length= */ 100))
.isEqualTo(-100);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 20, /* length= */ Long.MAX_VALUE))
.isEqualTo(-Long.MAX_VALUE);
}
// Value is capped by the 'length'
assertThat(simpleCache.getCachedLength(KEY_1, 0, 10)).isEqualTo(10);
@Test
public void getCachedLength_returnsNegativeHoleLength() throws Exception {
SimpleCache simpleCache = getSimpleCache();
CacheSpan cacheSpan = simpleCache.startReadWrite(KEY_1, /* position= */ 0);
addCache(simpleCache, KEY_1, /* position= */ 50, /* length= */ 50);
simpleCache.releaseHoleSpan(cacheSpan);
addCache(simpleCache, KEY_1, 15, 35);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 0, /* length= */ 100))
.isEqualTo(-50);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 0, /* length= */ Long.MAX_VALUE))
.isEqualTo(-50);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 20, /* length= */ 100))
.isEqualTo(-30);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 20, /* length= */ Long.MAX_VALUE))
.isEqualTo(-30);
}
// Returns the length of two adjacent spans
assertThat(simpleCache.getCachedLength(KEY_1, 0, 100)).isEqualTo(50);
@Test
public void getCachedLength_returnsCachedLength() throws Exception {
SimpleCache simpleCache = getSimpleCache();
CacheSpan cacheSpan = simpleCache.startReadWrite(KEY_1, /* position= */ 0);
addCache(simpleCache, KEY_1, /* position= */ 0, /* length= */ 50);
simpleCache.releaseHoleSpan(cacheSpan);
addCache(simpleCache, KEY_1, 60, 10);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 0, /* length= */ 100))
.isEqualTo(50);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 0, /* length= */ Long.MAX_VALUE))
.isEqualTo(50);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 20, /* length= */ 100))
.isEqualTo(30);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 20, /* length= */ Long.MAX_VALUE))
.isEqualTo(30);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 20, /* length= */ 15))
.isEqualTo(15);
}
// Not adjacent span doesn't affect return value
assertThat(simpleCache.getCachedLength(KEY_1, 0, 100)).isEqualTo(50);
@Test
public void getCachedLength_withMultipleAdjacentSpans_returnsCachedLength() throws Exception {
SimpleCache simpleCache = getSimpleCache();
CacheSpan cacheSpan = simpleCache.startReadWrite(KEY_1, /* position= */ 0);
addCache(simpleCache, KEY_1, /* position= */ 0, /* length= */ 25);
addCache(simpleCache, KEY_1, /* position= */ 25, /* length= */ 25);
simpleCache.releaseHoleSpan(cacheSpan);
// Returns length of hole up to the next cached span
assertThat(simpleCache.getCachedLength(KEY_1, 55, 100)).isEqualTo(-5);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 0, /* length= */ 100))
.isEqualTo(50);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 0, /* length= */ Long.MAX_VALUE))
.isEqualTo(50);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 20, /* length= */ 100))
.isEqualTo(30);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 20, /* length= */ Long.MAX_VALUE))
.isEqualTo(30);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 20, /* length= */ 15))
.isEqualTo(15);
}
@Test
public void getCachedLength_withMultipleNonAdjacentSpans_returnsCachedLength() throws Exception {
SimpleCache simpleCache = getSimpleCache();
CacheSpan cacheSpan = simpleCache.startReadWrite(KEY_1, /* position= */ 0);
addCache(simpleCache, KEY_1, /* position= */ 0, /* length= */ 10);
addCache(simpleCache, KEY_1, /* position= */ 15, /* length= */ 35);
simpleCache.releaseHoleSpan(cacheSpan);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 0, /* length= */ 100))
.isEqualTo(10);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 0, /* length= */ Long.MAX_VALUE))
.isEqualTo(10);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 20, /* length= */ 100))
.isEqualTo(30);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 20, /* length= */ Long.MAX_VALUE))
.isEqualTo(30);
assertThat(simpleCache.getCachedLength(KEY_1, /* position= */ 20, /* length= */ 15))
.isEqualTo(15);
}
/* Tests https://github.com/google/ExoPlayer/issues/3260 case. */
......
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