From 9a23d9a611a85c75a4a8280b990f8516f4a93117 Mon Sep 17 00:00:00 2001
From: ibaker <ibaker@google.com>
Date: Mon, 14 Oct 2024 07:34:13 -0700
Subject: [PATCH] Deprecate `HlsExtractorFactory.DEFAULT`

`HlsExtractorFactory` instances are mutable, so storing one in a static
field is not safe, and can lead to state accidentally/surprisingly being
shared between different player instances.

PiperOrigin-RevId: 685701062
---
 .../exoplayer/hls/HlsExtractorFactory.java    |  7 ++++-
 .../media3/exoplayer/hls/HlsMediaSource.java  | 30 +++++++++++++------
 .../exoplayer/hls/HlsChunkSourceTest.java     |  2 +-
 3 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/libraries/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/HlsExtractorFactory.java b/libraries/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/HlsExtractorFactory.java
index 2f4e832c75..7e301fe2f9 100644
--- a/libraries/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/HlsExtractorFactory.java
+++ b/libraries/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/HlsExtractorFactory.java
@@ -36,7 +36,12 @@ import java.util.Map;
 @UnstableApi
 public interface HlsExtractorFactory {
 
-  HlsExtractorFactory DEFAULT = new DefaultHlsExtractorFactory();
+  /**
+   * @deprecated {@code HlsExtractorFactory} instances are mutable, so sharing one in a static field
+   *     is not safe. Construct a new instance of {@link DefaultHlsExtractorFactory} for each usage
+   *     instead.
+   */
+  @Deprecated HlsExtractorFactory DEFAULT = new DefaultHlsExtractorFactory();
 
   /**
    * Creates an {@link Extractor} for extracting HLS media chunks.
diff --git a/libraries/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/HlsMediaSource.java b/libraries/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/HlsMediaSource.java
index be597fe46f..7985d7211a 100644
--- a/libraries/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/HlsMediaSource.java
+++ b/libraries/exoplayer_hls/src/main/java/androidx/media3/exoplayer/hls/HlsMediaSource.java
@@ -105,7 +105,9 @@ public final class HlsMediaSource extends BaseMediaSource
 
     private final HlsDataSourceFactory hlsDataSourceFactory;
 
-    private HlsExtractorFactory extractorFactory;
+    @Nullable private HlsExtractorFactory extractorFactory;
+    @Nullable private SubtitleParser.Factory subtitleParserFactoryOverride;
+    private boolean parseSubtitlesDuringExtraction;
     private HlsPlaylistParserFactory playlistParserFactory;
     private HlsPlaylistTracker.Factory playlistTrackerFactory;
     private CompositeSequenceableLoaderFactory compositeSequenceableLoaderFactory;
@@ -128,7 +130,7 @@ public final class HlsMediaSource extends BaseMediaSource
      *   <li>{@link DefaultDrmSessionManagerProvider}
      *   <li>{@link DefaultHlsPlaylistParserFactory}
      *   <li>{@link DefaultHlsPlaylistTracker#FACTORY}
-     *   <li>{@link HlsExtractorFactory#DEFAULT}
+     *   <li>{@link DefaultHlsExtractorFactory}
      *   <li>{@link DefaultLoadErrorHandlingPolicy}
      *   <li>{@link DefaultCompositeSequenceableLoaderFactory}
      * </ul>
@@ -150,7 +152,7 @@ public final class HlsMediaSource extends BaseMediaSource
      *   <li>{@link DefaultDrmSessionManagerProvider}
      *   <li>{@link DefaultHlsPlaylistParserFactory}
      *   <li>{@link DefaultHlsPlaylistTracker#FACTORY}
-     *   <li>{@link HlsExtractorFactory#DEFAULT}
+     *   <li>{@link DefaultHlsExtractorFactory}
      *   <li>{@link DefaultLoadErrorHandlingPolicy}
      *   <li>{@link DefaultCompositeSequenceableLoaderFactory}
      * </ul>
@@ -163,7 +165,6 @@ public final class HlsMediaSource extends BaseMediaSource
       drmSessionManagerProvider = new DefaultDrmSessionManagerProvider();
       playlistParserFactory = new DefaultHlsPlaylistParserFactory();
       playlistTrackerFactory = DefaultHlsPlaylistTracker.FACTORY;
-      extractorFactory = HlsExtractorFactory.DEFAULT;
       loadErrorHandlingPolicy = new DefaultLoadErrorHandlingPolicy();
       compositeSequenceableLoaderFactory = new DefaultCompositeSequenceableLoaderFactory();
       metadataType = METADATA_TYPE_ID3;
@@ -174,7 +175,11 @@ public final class HlsMediaSource extends BaseMediaSource
 
     /**
      * Sets the factory for {@link Extractor}s for the segments. The default value is {@link
-     * HlsExtractorFactory#DEFAULT}.
+     * DefaultHlsExtractorFactory}.
+     *
+     * <p>Any values passed to {@link #setSubtitleParserFactory} or {@link
+     * #experimentalParseSubtitlesDuringExtraction} will be forwarded to the provided {@link
+     * HlsExtractorFactory} instance during {@link #createMediaSource}.
      *
      * @param extractorFactory An {@link HlsExtractorFactory} for {@link Extractor}s for the
      *     segments.
@@ -182,8 +187,7 @@ public final class HlsMediaSource extends BaseMediaSource
      */
     @CanIgnoreReturnValue
     public Factory setExtractorFactory(@Nullable HlsExtractorFactory extractorFactory) {
-      this.extractorFactory =
-          extractorFactory != null ? extractorFactory : HlsExtractorFactory.DEFAULT;
+      this.extractorFactory = extractorFactory;
       return this;
     }
 
@@ -202,7 +206,7 @@ public final class HlsMediaSource extends BaseMediaSource
     @CanIgnoreReturnValue
     @Override
     public Factory setSubtitleParserFactory(SubtitleParser.Factory subtitleParserFactory) {
-      extractorFactory.setSubtitleParserFactory(checkNotNull(subtitleParserFactory));
+      this.subtitleParserFactoryOverride = subtitleParserFactory;
       return this;
     }
 
@@ -211,7 +215,7 @@ public final class HlsMediaSource extends BaseMediaSource
     @CanIgnoreReturnValue
     public Factory experimentalParseSubtitlesDuringExtraction(
         boolean parseSubtitlesDuringExtraction) {
-      extractorFactory.experimentalParseSubtitlesDuringExtraction(parseSubtitlesDuringExtraction);
+      this.parseSubtitlesDuringExtraction = parseSubtitlesDuringExtraction;
       return this;
     }
 
@@ -384,6 +388,14 @@ public final class HlsMediaSource extends BaseMediaSource
     @Override
     public HlsMediaSource createMediaSource(MediaItem mediaItem) {
       checkNotNull(mediaItem.localConfiguration);
+      if (extractorFactory == null) {
+        extractorFactory = new DefaultHlsExtractorFactory();
+      }
+      if (subtitleParserFactoryOverride != null) {
+        extractorFactory.setSubtitleParserFactory(subtitleParserFactoryOverride);
+      }
+      extractorFactory.experimentalParseSubtitlesDuringExtraction(parseSubtitlesDuringExtraction);
+      HlsExtractorFactory extractorFactory = this.extractorFactory;
       HlsPlaylistParserFactory playlistParserFactory = this.playlistParserFactory;
       List<StreamKey> streamKeys = mediaItem.localConfiguration.streamKeys;
       if (!streamKeys.isEmpty()) {
diff --git a/libraries/exoplayer_hls/src/test/java/androidx/media3/exoplayer/hls/HlsChunkSourceTest.java b/libraries/exoplayer_hls/src/test/java/androidx/media3/exoplayer/hls/HlsChunkSourceTest.java
index 077ce0cb86..4094acf429 100644
--- a/libraries/exoplayer_hls/src/test/java/androidx/media3/exoplayer/hls/HlsChunkSourceTest.java
+++ b/libraries/exoplayer_hls/src/test/java/androidx/media3/exoplayer/hls/HlsChunkSourceTest.java
@@ -543,7 +543,7 @@ public class HlsChunkSourceTest {
 
   private HlsChunkSource createHlsChunkSource(@Nullable CmcdConfiguration cmcdConfiguration) {
     return new HlsChunkSource(
-        HlsExtractorFactory.DEFAULT,
+        new DefaultHlsExtractorFactory(),
         mockPlaylistTracker,
         new Uri[] {IFRAME_URI, PLAYLIST_URI},
         new Format[] {IFRAME_FORMAT, ExoPlayerTestRunner.VIDEO_FORMAT},