Remove DataSpec.FLAG_ALLOW_ICY_METADATA

Remove the flag DataSpec.FLAG_ALLOW_ICY_METADATA. Instead, set the
header IcyHeaders.REQUEST_HEADER_ENABLE_METADATA_NAME in the DataSpec
httpRequestHeaders.

BUG:134034248
PiperOrigin-RevId: 270662676

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

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));
       }

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);

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<String, String> 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
+          ICY_METADATA_HEADERS);
-              | DataSpec.FLAG_ALLOW_CACHE_FRAGMENTATION);
     }
 
     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<String, String> createIcyMetadataHeaders() {
+    Map<String, String> headers = new HashMap<>();
+    headers.put(
+        IcyHeaders.REQUEST_HEADER_ENABLE_METADATA_NAME,
+        IcyHeaders.REQUEST_HEADER_ENABLE_METADATA_VALUE);
+    return Collections.unmodifiableMap(headers);
+  }
 }

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}
+   * #FLAG_ALLOW_GZIP}, {@link #FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN} and {@link
-   * and {@link #FLAG_ALLOW_CACHE_FRAGMENTATION}.
+   * #FLAG_ALLOW_CACHE_FRAGMENTATION}.
    */
   @Documented
   @Retention(RetentionPolicy.SOURCE)
   @IntDef(
       flag = true,
-      value = {
+      value = {FLAG_ALLOW_GZIP, FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN, FLAG_ALLOW_CACHE_FRAGMENTATION})
-        FLAG_ALLOW_GZIP,
-        FLAG_ALLOW_ICY_METADATA,
-        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<String, String> 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<String, String> requestHeaders) {
+    return new DataSpec(
+        uri,
+        httpMethod,
+        httpBody,
+        absoluteStreamPosition,
+        position,
+        length,
+        key,
+        flags,
+        requestHeaders);
+  }
+
+  /**
+   * Returns a copy this data spec with additional request headers.
+   *
+   * <p>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<String, String> requestHeaders) {
+    Map<String, String> 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;
+  }
 }

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<String, String> 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();

library/core/src/test/java/com/google/android/exoplayer2/upstream/DataSpecTest.java

@@ -98,14 +98,79 @@ public class DataSpecTest {
   }
 
   @Test
-  public void copyMethods_copiesHttpRequestHeaders() {
+  public void withUri_copiesHttpRequestHeaders() {
-    Map<String, String> httpRequestParameters = new HashMap<>();
+    Map<String, String> httpRequestProperties = createRequestProperties(5);
-    httpRequestParameters.put("key1", "value1");
+    DataSpec dataSpec = createDataSpecWithHeaders(httpRequestProperties);
-    httpRequestParameters.put("key2", "value2");
-    httpRequestParameters.put("key3", "value3");
 
-    DataSpec dataSpec =
+    DataSpec dataSpecCopy = dataSpec.withUri(Uri.parse("www.new-uri.com"));
-        new DataSpec(
+
+    assertThat(dataSpecCopy.httpRequestHeaders).isEqualTo(httpRequestProperties);
+  }
+
+  @Test
+  public void subrange_copiesHttpRequestHeaders() {
+    Map<String, String> httpRequestProperties = createRequestProperties(5);
+    DataSpec dataSpec = createDataSpecWithHeaders(httpRequestProperties);
+
+    DataSpec dataSpecCopy = dataSpec.subrange(2);
+
+    assertThat(dataSpecCopy.httpRequestHeaders).isEqualTo(httpRequestProperties);
+  }
+
+  @Test
+  public void subrange_withOffsetAndLength_copiesHttpRequestHeaders() {
+    Map<String, String> httpRequestProperties = createRequestProperties(5);
+    DataSpec dataSpec = createDataSpecWithHeaders(httpRequestProperties);
+
+    DataSpec dataSpecCopy = dataSpec.subrange(2, 2);
+
+    assertThat(dataSpecCopy.httpRequestHeaders).isEqualTo(httpRequestProperties);
+  }
+
+  @Test
+  public void withRequestHeaders_setsCorrectHeaders() {
+    Map<String, String> httpRequestProperties = createRequestProperties(5);
+    DataSpec dataSpec = createDataSpecWithHeaders(httpRequestProperties);
+
+    Map<String, String> newRequestHeaders = createRequestProperties(5, 10);
+    DataSpec dataSpecCopy = dataSpec.withRequestHeaders(newRequestHeaders);
+
+    assertThat(dataSpecCopy.httpRequestHeaders).isEqualTo(newRequestHeaders);
+  }
+
+  @Test
+  public void withAdditionalHeaders_setsCorrectHeaders() {
+    Map<String, String> httpRequestProperties = createRequestProperties(5);
+    DataSpec dataSpec = createDataSpecWithHeaders(httpRequestProperties);
+    Map<String, String> additionalHeaders = createRequestProperties(5, 10);
+    // additionalHeaders may overwrite a header key
+    String existingKey = httpRequestProperties.keySet().iterator().next();
+    additionalHeaders.put(existingKey, "overwritten");
+    Map<String, String> expectedHeaders = new HashMap<>(httpRequestProperties);
+    expectedHeaders.putAll(additionalHeaders);
+
+    DataSpec dataSpecCopy = dataSpec.withAdditionalHeaders(additionalHeaders);
+
+    assertThat(dataSpecCopy.httpRequestHeaders).isEqualTo(expectedHeaders);
+  }
+
+  private static Map<String, String> createRequestProperties(int howMany) {
+    return createRequestProperties(0, howMany);
+  }
+
+  private static Map<String, String> createRequestProperties(int from, int to) {
+    assertThat(from).isLessThan(to);
+
+    Map<String, String> httpRequestParameters = new HashMap<>();
+    for (int i = from; i < to; i++) {
+      httpRequestParameters.put("key-" + i, "value-" + i);
+    }
+
+    return httpRequestParameters;
+  }
+
+  private static DataSpec createDataSpecWithHeaders(Map<String, String> httpRequestProperties) {
+    return new DataSpec(
         Uri.parse("www.google.com"),
         /* httpMethod= */ 0,
         /* httpBody= */ new byte[] {0, 0, 0, 0},
@@ -114,15 +179,6 @@ public class DataSpecTest {
         /* length= */ 1,
         /* key= */ "key",
         /* flags= */ 0,
-            httpRequestParameters);
+        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);
-
-    dataSpecCopy = dataSpec.subrange(2, 2);
-    assertThat(dataSpecCopy.httpRequestHeaders).isEqualTo(httpRequestParameters);
   }
 }