Commit 5bbe3ae7 by olly Committed by Oliver Woodman

Cache data with unknown length by default

We currently default to not caching data if the content length
cannot be resolved once the DataSource has been open. The
reason for this is to avoid caching progressive live streams.
By doing this we were accidentally not caching in other places
where caching is possible, such as DASH/SS/HLS segments during
playback if the server doesn't include a Content-Length header.
Also HLS encryption key chunks, which were very unlikely to be
cached during playback because we explicitly set FLAG_ALLOW_GZIP
(which normally stops content length from resolving) without
setting FLAG_ALLOW_CACHING_UNKNOWN_LENGTH.

It seems like a good idea to flip the default at this point,
and explicitly disable caching in the one case where we want
that to happen.

PiperOrigin-RevId: 223994110
parent 976a21f1
...@@ -9,6 +9,9 @@ ...@@ -9,6 +9,9 @@
([#3314](https://github.com/google/ExoPlayer/issues/3314)). ([#3314](https://github.com/google/ExoPlayer/issues/3314)).
* Do not retry failed loads whose error is `FileNotFoundException`. * Do not retry failed loads whose error is `FileNotFoundException`.
* Prevent Cea608Decoder from generating Subtitles with null Cues list * Prevent Cea608Decoder from generating Subtitles with null Cues list
* Caching: Cache data with unknown length by default. The previous flag to opt in
to this behavior (`DataSpec.FLAG_ALLOW_CACHING_UNKNOWN_LENGTH`) has been
replaced with an opt out flag (`DataSpec.FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN`).
### 2.9.2 ### ### 2.9.2 ###
......
...@@ -850,6 +850,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; ...@@ -850,6 +850,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType;
private DataSpec dataSpec; private DataSpec dataSpec;
private long length; private long length;
@SuppressWarnings("method.invocation.invalid")
public ExtractingLoadable( public ExtractingLoadable(
Uri uri, Uri uri,
DataSource dataSource, DataSource dataSource,
...@@ -864,7 +865,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; ...@@ -864,7 +865,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType;
this.positionHolder = new PositionHolder(); this.positionHolder = new PositionHolder();
this.pendingExtractorSeek = true; this.pendingExtractorSeek = true;
this.length = C.LENGTH_UNSET; this.length = C.LENGTH_UNSET;
dataSpec = new DataSpec(uri, positionHolder.position, C.LENGTH_UNSET, customCacheKey); dataSpec = buildDataSpec(/* position= */ 0);
} }
// Loadable implementation. // Loadable implementation.
...@@ -881,7 +882,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; ...@@ -881,7 +882,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType;
ExtractorInput input = null; ExtractorInput input = null;
try { try {
long position = positionHolder.position; long position = positionHolder.position;
dataSpec = new DataSpec(uri, position, C.LENGTH_UNSET, customCacheKey); dataSpec = buildDataSpec(position);
length = dataSource.open(dataSpec); length = dataSource.open(dataSpec);
if (length != C.LENGTH_UNSET) { if (length != C.LENGTH_UNSET) {
length += position; length += position;
...@@ -915,6 +916,17 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; ...@@ -915,6 +916,17 @@ import org.checkerframework.checker.nullness.compatqual.NullableType;
// Internal methods. // Internal methods.
private DataSpec buildDataSpec(long position) {
// Disable caching if the content length cannot be resolved, since this is indicative of a
// progressive live stream.
return new DataSpec(
uri,
position,
C.LENGTH_UNSET,
customCacheKey,
DataSpec.FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN);
}
private void setLoadPosition(long position, long timeUs) { private void setLoadPosition(long position, long timeUs) {
positionHolder.position = position; positionHolder.position = position;
seekTimeUs = timeUs; seekTimeUs = timeUs;
......
...@@ -287,8 +287,7 @@ public final class SingleSampleMediaSource extends BaseMediaSource { ...@@ -287,8 +287,7 @@ public final class SingleSampleMediaSource extends BaseMediaSource {
this.durationUs = durationUs; this.durationUs = durationUs;
this.loadErrorHandlingPolicy = loadErrorHandlingPolicy; this.loadErrorHandlingPolicy = loadErrorHandlingPolicy;
this.treatLoadErrorsAsEndOfStream = treatLoadErrorsAsEndOfStream; this.treatLoadErrorsAsEndOfStream = treatLoadErrorsAsEndOfStream;
dataSpec = dataSpec = new DataSpec(uri, DataSpec.FLAG_ALLOW_GZIP);
new DataSpec(uri, DataSpec.FLAG_ALLOW_GZIP | DataSpec.FLAG_ALLOW_CACHING_UNKNOWN_LENGTH);
timeline = timeline =
new SinglePeriodTimeline(durationUs, /* isSeekable= */ true, /* isDynamic= */ false, tag); new SinglePeriodTimeline(durationUs, /* isSeekable= */ true, /* isDynamic= */ false, tag);
} }
......
...@@ -32,32 +32,29 @@ public final class DataSpec { ...@@ -32,32 +32,29 @@ public final class DataSpec {
/** /**
* The flags that apply to any request for data. Possible flag values are {@link #FLAG_ALLOW_GZIP} * The flags that apply to any request for data. Possible flag values are {@link #FLAG_ALLOW_GZIP}
* and {@link #FLAG_ALLOW_CACHING_UNKNOWN_LENGTH}. * and {@link #FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN}.
*/ */
@Documented @Documented
@Retention(RetentionPolicy.SOURCE) @Retention(RetentionPolicy.SOURCE)
@IntDef( @IntDef(
flag = true, flag = true,
value = {FLAG_ALLOW_GZIP, FLAG_ALLOW_CACHING_UNKNOWN_LENGTH}) value = {FLAG_ALLOW_GZIP, FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN})
public @interface Flags {} public @interface Flags {}
/** /**
* Permits an underlying network stack to request that the server use gzip compression. * Allows an underlying network stack to request that the server use gzip compression.
* <p> *
* Should not typically be set if the data being requested is already compressed (e.g. most audio * <p>Should not typically be set if the data being requested is already compressed (e.g. most
* and video requests). May be set when requesting other data. * audio and video requests). May be set when requesting other data.
* <p> *
* When a {@link DataSource} is used to request data with this flag set, and if the * <p>When a {@link DataSource} is used to request data with this flag set, and if the {@link
* {@link DataSource} does make a network request, then the value returned from * DataSource} does make a network request, then the value returned from {@link
* {@link DataSource#open(DataSpec)} will typically be {@link C#LENGTH_UNSET}. The data read from * DataSource#open(DataSpec)} will typically be {@link C#LENGTH_UNSET}. The data read from {@link
* {@link DataSource#read(byte[], int, int)} will be the decompressed data. * DataSource#read(byte[], int, int)} will be the decompressed data.
*/ */
public static final int FLAG_ALLOW_GZIP = 1; public static final int FLAG_ALLOW_GZIP = 1;
/** /** Prevents caching if the length cannot be resolved when the {@link DataSource} is opened. */
* Permits content to be cached even if its length can not be resolved. Typically this's the case public static final int FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN = 1 << 1; // 2
* for progressive live streams and when {@link #FLAG_ALLOW_GZIP} is used.
*/
public static final int FLAG_ALLOW_CACHING_UNKNOWN_LENGTH = 1 << 1; // 2
/** /**
* The set of HTTP methods that are supported by ExoPlayer {@link HttpDataSource}s. One of {@link * The set of HTTP methods that are supported by ExoPlayer {@link HttpDataSource}s. One of {@link
......
...@@ -91,11 +91,7 @@ public final class ParsingLoadable<T> implements Loadable { ...@@ -91,11 +91,7 @@ public final class ParsingLoadable<T> implements Loadable {
* @param parser Parses the object from the response. * @param parser Parses the object from the response.
*/ */
public ParsingLoadable(DataSource dataSource, Uri uri, int type, Parser<? extends T> parser) { public ParsingLoadable(DataSource dataSource, Uri uri, int type, Parser<? extends T> parser) {
this( this(dataSource, new DataSpec(uri, DataSpec.FLAG_ALLOW_GZIP), type, parser);
dataSource,
new DataSpec(uri, DataSpec.FLAG_ALLOW_GZIP | DataSpec.FLAG_ALLOW_CACHING_UNKNOWN_LENGTH),
type,
parser);
} }
/** /**
......
...@@ -121,7 +121,7 @@ public final class CacheDataSink implements DataSink { ...@@ -121,7 +121,7 @@ public final class CacheDataSink implements DataSink {
@Override @Override
public void open(DataSpec dataSpec) throws CacheDataSinkException { public void open(DataSpec dataSpec) throws CacheDataSinkException {
if (dataSpec.length == C.LENGTH_UNSET if (dataSpec.length == C.LENGTH_UNSET
&& !dataSpec.isFlagSet(DataSpec.FLAG_ALLOW_CACHING_UNKNOWN_LENGTH)) { && dataSpec.isFlagSet(DataSpec.FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN)) {
this.dataSpec = null; this.dataSpec = null;
return; return;
} }
......
...@@ -268,7 +268,7 @@ public final class CacheUtil { ...@@ -268,7 +268,7 @@ public final class CacheUtil {
dataSpec.position + absoluteStreamPosition - dataSpec.absoluteStreamPosition, dataSpec.position + absoluteStreamPosition - dataSpec.absoluteStreamPosition,
C.LENGTH_UNSET, C.LENGTH_UNSET,
dataSpec.key, dataSpec.key,
dataSpec.flags | DataSpec.FLAG_ALLOW_CACHING_UNKNOWN_LENGTH); 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 = dataSpec.absoluteStreamPosition + resolvedLength;
......
...@@ -602,7 +602,6 @@ public final class CacheDataSourceTest { ...@@ -602,7 +602,6 @@ public final class CacheDataSourceTest {
} }
private DataSpec buildDataSpec(long position, long length, @Nullable String key) { private DataSpec buildDataSpec(long position, long length, @Nullable String key) {
return new DataSpec( return new DataSpec(testDataUri, position, length, key);
testDataUri, position, length, key, DataSpec.FLAG_ALLOW_CACHING_UNKNOWN_LENGTH);
} }
} }
...@@ -83,7 +83,7 @@ public final class CacheAsserts { ...@@ -83,7 +83,7 @@ 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, DataSpec.FLAG_ALLOW_CACHING_UNKNOWN_LENGTH); DataSpec dataSpec = new DataSpec(uri);
assertDataCached(cache, dataSpec, expected); assertDataCached(cache, dataSpec, 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