diff --git a/RELEASENOTES.md b/RELEASENOTES.md index eb983e7524..11b493d555 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -2,6 +2,9 @@ ### dev-v2 (not yet released) ### +* Remove the `DataSpec.FLAG_ALLOW_ICY_METADATA` flag. Instead, set the header + `IcyHeaders.REQUEST_HEADER_ENABLE_METADATA_NAME` in the `DataSpec` + `httpRequestHeaders`. * DASH: Support negative @r values in segment timelines ([#1787](https://github.com/google/ExoPlayer/issues/1787)). * Add `allowedCapturePolicy` field to `AudioAttributes` wrapper to allow to diff --git a/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java b/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java index 91bceb9aee..1903e33995 100644 --- a/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java +++ b/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java @@ -22,7 +22,6 @@ import android.text.TextUtils; import androidx.annotation.Nullable; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.ExoPlayerLibraryInfo; -import com.google.android.exoplayer2.metadata.icy.IcyHeaders; import com.google.android.exoplayer2.upstream.BaseDataSource; import com.google.android.exoplayer2.upstream.DataSourceException; import com.google.android.exoplayer2.upstream.DataSpec; @@ -706,11 +705,7 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { if (dataSpec.httpBody != null && !requestHeaders.containsKey(CONTENT_TYPE)) { throw new IOException("HTTP request with non-empty body must set Content-Type"); } - if (dataSpec.isFlagSet(DataSpec.FLAG_ALLOW_ICY_METADATA)) { - requestBuilder.addHeader( - IcyHeaders.REQUEST_HEADER_ENABLE_METADATA_NAME, - IcyHeaders.REQUEST_HEADER_ENABLE_METADATA_VALUE); - } + // Set the Range header. if (dataSpec.position != 0 || dataSpec.length != C.LENGTH_UNSET) { StringBuilder rangeValue = new StringBuilder(); @@ -937,7 +932,8 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { dataSpec.position, dataSpec.length, dataSpec.key, - dataSpec.flags); + dataSpec.flags, + dataSpec.httpRequestHeaders); } else { redirectUrlDataSpec = dataSpec.withUri(Uri.parse(newLocationUrl)); } diff --git a/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java b/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java index 2a8f671577..3053961f49 100644 --- a/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java +++ b/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java @@ -21,7 +21,6 @@ import android.net.Uri; import androidx.annotation.Nullable; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.ExoPlayerLibraryInfo; -import com.google.android.exoplayer2.metadata.icy.IcyHeaders; import com.google.android.exoplayer2.upstream.BaseDataSource; import com.google.android.exoplayer2.upstream.DataSourceException; import com.google.android.exoplayer2.upstream.DataSpec; @@ -361,11 +360,7 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource { if (!dataSpec.isFlagSet(DataSpec.FLAG_ALLOW_GZIP)) { builder.addHeader("Accept-Encoding", "identity"); } - if (dataSpec.isFlagSet(DataSpec.FLAG_ALLOW_ICY_METADATA)) { - builder.addHeader( - IcyHeaders.REQUEST_HEADER_ENABLE_METADATA_NAME, - IcyHeaders.REQUEST_HEADER_ENABLE_METADATA_VALUE); - } + RequestBody requestBody = null; if (dataSpec.httpBody != null) { requestBody = RequestBody.create(null, dataSpec.httpBody); diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ProgressiveMediaPeriod.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ProgressiveMediaPeriod.java index 7cb767854f..c768fe4981 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ProgressiveMediaPeriod.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ProgressiveMediaPeriod.java @@ -55,6 +55,9 @@ import com.google.android.exoplayer2.util.Util; import java.io.EOFException; import java.io.IOException; import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import org.checkerframework.checker.nullness.compatqual.NullableType; /** A {@link MediaPeriod} that extracts data using an {@link Extractor}. */ @@ -86,6 +89,8 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; */ private static final long DEFAULT_LAST_SAMPLE_DURATION_US = 10000; + private static final Map ICY_METADATA_HEADERS = createIcyMetadataHeaders(); + private static final Format ICY_FORMAT = Format.createSampleFormat("icy", MimeTypes.APPLICATION_ICY, Format.OFFSET_SAMPLE_RELATIVE); @@ -1025,9 +1030,8 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; position, C.LENGTH_UNSET, customCacheKey, - DataSpec.FLAG_ALLOW_ICY_METADATA - | DataSpec.FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN - | DataSpec.FLAG_ALLOW_CACHE_FRAGMENTATION); + DataSpec.FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN | DataSpec.FLAG_ALLOW_CACHE_FRAGMENTATION, + ICY_METADATA_HEADERS); } private void setLoadPosition(long position, long timeUs) { @@ -1154,4 +1158,12 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; return 31 * id + (isIcyTrack ? 1 : 0); } } + + private static Map createIcyMetadataHeaders() { + Map headers = new HashMap<>(); + headers.put( + IcyHeaders.REQUEST_HEADER_ENABLE_METADATA_NAME, + IcyHeaders.REQUEST_HEADER_ENABLE_METADATA_VALUE); + return Collections.unmodifiableMap(headers); + } } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DataSpec.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DataSpec.java index f5802bed4e..acf5550427 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DataSpec.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DataSpec.java @@ -35,19 +35,14 @@ public final class DataSpec { /** * The flags that apply to any request for data. Possible flag values are {@link - * #FLAG_ALLOW_GZIP}, {@link #FLAG_ALLOW_ICY_METADATA}, {@link #FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN} - * and {@link #FLAG_ALLOW_CACHE_FRAGMENTATION}. + * #FLAG_ALLOW_GZIP}, {@link #FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN} and {@link + * #FLAG_ALLOW_CACHE_FRAGMENTATION}. */ @Documented @Retention(RetentionPolicy.SOURCE) @IntDef( flag = true, - value = { - FLAG_ALLOW_GZIP, - FLAG_ALLOW_ICY_METADATA, - FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN, - FLAG_ALLOW_CACHE_FRAGMENTATION - }) + value = {FLAG_ALLOW_GZIP, FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN, FLAG_ALLOW_CACHE_FRAGMENTATION}) public @interface Flags {} /** * Allows an underlying network stack to request that the server use gzip compression. @@ -61,17 +56,15 @@ public final class DataSpec { * DataSource#read(byte[], int, int)} will be the decompressed data. */ public static final int FLAG_ALLOW_GZIP = 1; - /** Allows an underlying network stack to request that the stream contain ICY metadata. */ - public static final int FLAG_ALLOW_ICY_METADATA = 1 << 1; // 2 /** Prevents caching if the length cannot be resolved when the {@link DataSource} is opened. */ - public static final int FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN = 1 << 2; // 4 + public static final int FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN = 1 << 1; // 2 /** * Allows fragmentation of this request into multiple cache files, meaning a cache eviction policy * will be able to evict individual fragments of the data. Depending on the cache implementation, * setting this flag may also enable more concurrent access to the data (e.g. reading one fragment * whilst writing another). */ - public static final int FLAG_ALLOW_CACHE_FRAGMENTATION = 1 << 3; // 8 + public static final int FLAG_ALLOW_CACHE_FRAGMENTATION = 1 << 2; // 4 /** * The set of HTTP methods that are supported by ExoPlayer {@link HttpDataSource}s. One of {@link @@ -172,6 +165,36 @@ public final class DataSpec { this(uri, absoluteStreamPosition, absoluteStreamPosition, length, key, flags); } + /** + * Construct a data spec where {@link #position} equals {@link #absoluteStreamPosition} and has + * request headers. + * + * @param uri {@link #uri}. + * @param absoluteStreamPosition {@link #absoluteStreamPosition}, equal to {@link #position}. + * @param length {@link #length}. + * @param key {@link #key}. + * @param flags {@link #flags}. + * @param httpRequestHeaders {@link #httpRequestHeaders} + */ + public DataSpec( + Uri uri, + long absoluteStreamPosition, + long length, + @Nullable String key, + @Flags int flags, + Map httpRequestHeaders) { + this( + uri, + inferHttpMethod(null), + null, + absoluteStreamPosition, + absoluteStreamPosition, + length, + key, + flags, + httpRequestHeaders); + } + /** * Construct a data spec where {@link #position} may differ from {@link #absoluteStreamPosition}. * @@ -216,7 +239,7 @@ public final class DataSpec { @Flags int flags) { this( uri, - /* httpMethod= */ postBody != null ? HTTP_METHOD_POST : HTTP_METHOD_GET, + /* httpMethod= */ inferHttpMethod(postBody), /* httpBody= */ postBody, absoluteStreamPosition, position, @@ -403,4 +426,53 @@ public final class DataSpec { flags, httpRequestHeaders); } + + /** + * Returns a copy of this data spec with the specified request headers. + * + * @param requestHeaders The HTTP request headers. + * @return The copied data spec with the specified request headers. + */ + public DataSpec withRequestHeaders(Map requestHeaders) { + return new DataSpec( + uri, + httpMethod, + httpBody, + absoluteStreamPosition, + position, + length, + key, + flags, + requestHeaders); + } + + /** + * Returns a copy this data spec with additional request headers. + * + *

Note: Values in {@code requestHeaders} will overwrite values with the same header key that + * were previously set in this instance's {@code #httpRequestHeaders}. + * + * @param requestHeaders The additional HTTP request headers. + * @return The copied data with the additional HTTP request headers. + */ + public DataSpec withAdditionalHeaders(Map requestHeaders) { + Map totalHeaders = new HashMap<>(this.httpRequestHeaders); + totalHeaders.putAll(requestHeaders); + + return new DataSpec( + uri, + httpMethod, + httpBody, + absoluteStreamPosition, + position, + length, + key, + flags, + totalHeaders); + } + + @HttpMethod + private static int inferHttpMethod(@Nullable byte[] postBody) { + return postBody != null ? HTTP_METHOD_POST : HTTP_METHOD_GET; + } } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java index 37329a4868..436cad0d64 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java @@ -20,7 +20,6 @@ import android.text.TextUtils; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; import com.google.android.exoplayer2.C; -import com.google.android.exoplayer2.metadata.icy.IcyHeaders; import com.google.android.exoplayer2.upstream.DataSpec.HttpMethod; import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Log; @@ -432,7 +431,6 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou long position = dataSpec.position; long length = dataSpec.length; boolean allowGzip = dataSpec.isFlagSet(DataSpec.FLAG_ALLOW_GZIP); - boolean allowIcyMetadata = dataSpec.isFlagSet(DataSpec.FLAG_ALLOW_ICY_METADATA); if (!allowCrossProtocolRedirects) { // HttpURLConnection disallows cross-protocol redirects, but otherwise performs redirection @@ -444,7 +442,6 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou position, length, allowGzip, - allowIcyMetadata, /* followRedirects= */ true, dataSpec.httpRequestHeaders); } @@ -460,7 +457,6 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou position, length, allowGzip, - allowIcyMetadata, /* followRedirects= */ false, dataSpec.httpRequestHeaders); int responseCode = connection.getResponseCode(); @@ -502,7 +498,6 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou * @param position The byte offset of the requested data. * @param length The length of the requested data, or {@link C#LENGTH_UNSET}. * @param allowGzip Whether to allow the use of gzip. - * @param allowIcyMetadata Whether to allow ICY metadata. * @param followRedirects Whether to follow redirects. * @param requestParameters parameters (HTTP headers) to include in request. */ @@ -513,7 +508,6 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou long position, long length, boolean allowGzip, - boolean allowIcyMetadata, boolean followRedirects, Map requestParameters) throws IOException { @@ -541,14 +535,10 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou } connection.setRequestProperty("User-Agent", userAgent); connection.setRequestProperty("Accept-Encoding", allowGzip ? "gzip" : "identity"); - if (allowIcyMetadata) { - connection.setRequestProperty( - IcyHeaders.REQUEST_HEADER_ENABLE_METADATA_NAME, - IcyHeaders.REQUEST_HEADER_ENABLE_METADATA_VALUE); - } connection.setInstanceFollowRedirects(followRedirects); connection.setDoOutput(httpBody != null); connection.setRequestMethod(DataSpec.getStringForHttpMethod(httpMethod)); + if (httpBody != null) { connection.setFixedLengthStreamingMode(httpBody.length); connection.connect(); diff --git a/library/core/src/test/java/com/google/android/exoplayer2/upstream/DataSpecTest.java b/library/core/src/test/java/com/google/android/exoplayer2/upstream/DataSpecTest.java index f6e30f814a..2323dfe965 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/upstream/DataSpecTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/upstream/DataSpecTest.java @@ -98,31 +98,87 @@ public class DataSpecTest { } @Test - public void copyMethods_copiesHttpRequestHeaders() { - Map httpRequestParameters = new HashMap<>(); - httpRequestParameters.put("key1", "value1"); - httpRequestParameters.put("key2", "value2"); - httpRequestParameters.put("key3", "value3"); - - DataSpec dataSpec = - new DataSpec( - Uri.parse("www.google.com"), - /* httpMethod= */ 0, - /* httpBody= */ new byte[] {0, 0, 0, 0}, - /* absoluteStreamPosition= */ 0, - /* position= */ 0, - /* length= */ 1, - /* key= */ "key", - /* flags= */ 0, - httpRequestParameters); + public void withUri_copiesHttpRequestHeaders() { + Map httpRequestProperties = createRequestProperties(5); + DataSpec dataSpec = createDataSpecWithHeaders(httpRequestProperties); DataSpec dataSpecCopy = dataSpec.withUri(Uri.parse("www.new-uri.com")); - assertThat(dataSpecCopy.httpRequestHeaders).isEqualTo(httpRequestParameters); - dataSpecCopy = dataSpec.subrange(2); - assertThat(dataSpecCopy.httpRequestHeaders).isEqualTo(httpRequestParameters); + assertThat(dataSpecCopy.httpRequestHeaders).isEqualTo(httpRequestProperties); + } - dataSpecCopy = dataSpec.subrange(2, 2); - assertThat(dataSpecCopy.httpRequestHeaders).isEqualTo(httpRequestParameters); + @Test + public void subrange_copiesHttpRequestHeaders() { + Map httpRequestProperties = createRequestProperties(5); + DataSpec dataSpec = createDataSpecWithHeaders(httpRequestProperties); + + DataSpec dataSpecCopy = dataSpec.subrange(2); + + assertThat(dataSpecCopy.httpRequestHeaders).isEqualTo(httpRequestProperties); + } + + @Test + public void subrange_withOffsetAndLength_copiesHttpRequestHeaders() { + Map httpRequestProperties = createRequestProperties(5); + DataSpec dataSpec = createDataSpecWithHeaders(httpRequestProperties); + + DataSpec dataSpecCopy = dataSpec.subrange(2, 2); + + assertThat(dataSpecCopy.httpRequestHeaders).isEqualTo(httpRequestProperties); + } + + @Test + public void withRequestHeaders_setsCorrectHeaders() { + Map httpRequestProperties = createRequestProperties(5); + DataSpec dataSpec = createDataSpecWithHeaders(httpRequestProperties); + + Map newRequestHeaders = createRequestProperties(5, 10); + DataSpec dataSpecCopy = dataSpec.withRequestHeaders(newRequestHeaders); + + assertThat(dataSpecCopy.httpRequestHeaders).isEqualTo(newRequestHeaders); + } + + @Test + public void withAdditionalHeaders_setsCorrectHeaders() { + Map httpRequestProperties = createRequestProperties(5); + DataSpec dataSpec = createDataSpecWithHeaders(httpRequestProperties); + Map additionalHeaders = createRequestProperties(5, 10); + // additionalHeaders may overwrite a header key + String existingKey = httpRequestProperties.keySet().iterator().next(); + additionalHeaders.put(existingKey, "overwritten"); + Map expectedHeaders = new HashMap<>(httpRequestProperties); + expectedHeaders.putAll(additionalHeaders); + + DataSpec dataSpecCopy = dataSpec.withAdditionalHeaders(additionalHeaders); + + assertThat(dataSpecCopy.httpRequestHeaders).isEqualTo(expectedHeaders); + } + + private static Map createRequestProperties(int howMany) { + return createRequestProperties(0, howMany); + } + + private static Map createRequestProperties(int from, int to) { + assertThat(from).isLessThan(to); + + Map httpRequestParameters = new HashMap<>(); + for (int i = from; i < to; i++) { + httpRequestParameters.put("key-" + i, "value-" + i); + } + + return httpRequestParameters; + } + + private static DataSpec createDataSpecWithHeaders(Map httpRequestProperties) { + return new DataSpec( + Uri.parse("www.google.com"), + /* httpMethod= */ 0, + /* httpBody= */ new byte[] {0, 0, 0, 0}, + /* absoluteStreamPosition= */ 0, + /* position= */ 0, + /* length= */ 1, + /* key= */ "key", + /* flags= */ 0, + httpRequestProperties); } }