Ensure DownloadHelper doesn't leak unreleased Renderer instances

Issue: #1224 PiperOrigin-RevId: 619935786
androidx · Mar 28, 2024 · c5e894e · c5e894e
1 parent b4e6b9a
commit c5e894e
Show file tree

Hide file tree

Showing 5 changed files with 106 additions and 17 deletions.
diff --git a/RELEASENOTES.md b/RELEASENOTES.md
@@ -121,6 +121,10 @@
  identify a display name
  ([#988](https://github.com/androidx/media/issues/988)).
 * Downloads:
+ * Ensure that `DownloadHelper` doesn't leak unreleased `Renderer`
+ instances, which can eventually result in an app crashing with
+ `IllegalStateException: Too many receivers, total of 1000, registered
+ for pid` ([#1224](https://github.com/androidx/media/issues/1224)).
 * OkHttp Extension:
 * Cronet Extension:
 * RTMP Extension:

diff --git a/...es/exoplayer/src/main/java/androidx/media3/exoplayer/DefaultRendererCapabilitiesList.java b/...es/exoplayer/src/main/java/androidx/media3/exoplayer/DefaultRendererCapabilitiesList.java
@@ -84,6 +84,11 @@ public RendererCapabilities[] getRendererCapabilities() {
  return rendererCapabilities;
  }
 
+ @Override
+ public int size() {
+ return renderers.length;
+ }
+
  @Override
  public void release() {
  for (Renderer renderer : renderers) {

diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/RendererCapabilitiesList.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/RendererCapabilitiesList.java
@@ -31,6 +31,9 @@ interface Factory {
  /** Returns an array of {@link RendererCapabilities}. */
  RendererCapabilities[] getRendererCapabilities();
 
+ /** Returns the number of {@link RendererCapabilities}. */
+ int size();
+
  /** Releases any resources associated with this {@link RendererCapabilitiesList}. */
  void release();
 }
diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/offline/DownloadHelper.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/offline/DownloadHelper.java
@@ -38,10 +38,12 @@
 import androidx.media3.common.util.Util;
 import androidx.media3.datasource.DataSource;
 import androidx.media3.datasource.TransferListener;
+import androidx.media3.exoplayer.DefaultRendererCapabilitiesList;
 import androidx.media3.exoplayer.ExoPlaybackException;
 import androidx.media3.exoplayer.LoadingInfo;
 import androidx.media3.exoplayer.Renderer;
 import androidx.media3.exoplayer.RendererCapabilities;
+import androidx.media3.exoplayer.RendererCapabilitiesList;
 import androidx.media3.exoplayer.RenderersFactory;
 import androidx.media3.exoplayer.analytics.PlayerId;
 import androidx.media3.exoplayer.audio.AudioRendererEventListener;
@@ -144,12 +146,12 @@ public interface Callback {
  public static class LiveContentUnsupportedException extends IOException {}
 
  /**
- * Extracts renderer capabilities for the renderers created by the provided renderers factory.
- *
- * @param renderersFactory A {@link RenderersFactory}.
- * @return The {@link RendererCapabilities} for each renderer created by the {@code
- * renderersFactory}.
+ * @deprecated This method leaks un-released {@link Renderer} instances. There is no direct
+ * replacement. Equivalent functionality can be implemented by constructing the renderer
+ * instances, calling {@link Renderer#getCapabilities()} on each one, then releasing the
+ * renderers when the capabilities are no longer required.
  */
+ @Deprecated
  public static RendererCapabilities[] getRendererCapabilities(RenderersFactory renderersFactory) {
  Renderer[] renderers =
  renderersFactory.createRenderers(
@@ -274,8 +276,9 @@ public static DownloadHelper forMediaItem(
  mediaItem, castNonNull(dataSourceFactory), drmSessionManager),
  trackSelectionParameters,
  renderersFactory != null
- ? getRendererCapabilities(renderersFactory)
- : new RendererCapabilities[0]);
+ ? new DefaultRendererCapabilitiesList.Factory(renderersFactory)
+ .createRendererCapabilitiesList()
+ : new UnreleaseableRendererCapabilitiesList(new RendererCapabilities[0]));
  }
 
  /**
@@ -308,7 +311,7 @@ public static MediaSource createMediaSource(
  private final MediaItem.LocalConfiguration localConfiguration;
  @Nullable private final MediaSource mediaSource;
  private final DefaultTrackSelector trackSelector;
- private final RendererCapabilities[] rendererCapabilities;
+ private final RendererCapabilitiesList rendererCapabilities;
  private final SparseIntArray scratchSet;
  private final Handler callbackHandler;
  private final Timeline.Window window;
@@ -322,6 +325,26 @@ public static MediaSource createMediaSource(
  private List<ExoTrackSelection> @MonotonicNonNull [][]
  immutableTrackSelectionsByPeriodAndRenderer;
 
+ /**
+ * @deprecated The {@link Renderer} instances used to produce {@code rendererCapabilities} must be
+ * kept alive for the lifetime of this {@code DownloadHelper} instance and then released (to
+ * avoid a resource leak). Use {@link DownloadHelper#DownloadHelper(MediaItem, MediaSource,
+ * TrackSelectionParameters, RendererCapabilitiesList)} instead to avoid needing to manually
+ * manage this bookkeeping.
+ */
+ @Deprecated
+ public DownloadHelper(
+ MediaItem mediaItem,
+ @Nullable MediaSource mediaSource,
+ TrackSelectionParameters trackSelectionParameters,
+ RendererCapabilities[] rendererCapabilities) {
+ this(
+ mediaItem,
+ mediaSource,
+ trackSelectionParameters,
+ new UnreleaseableRendererCapabilitiesList(rendererCapabilities));
+ }
+
  /**
  * Creates download helper.
  *
@@ -330,14 +353,14 @@ public static MediaSource createMediaSource(
  * selection needs to be made.
  * @param trackSelectionParameters {@link TrackSelectionParameters} for selecting tracks for
  * downloading.
- * @param rendererCapabilities The {@link RendererCapabilities} of the renderers for which tracks
- * are selected.
+ * @param rendererCapabilities The {@link RendererCapabilitiesList} of the renderers for which
+ * tracks are selected.
  */
  public DownloadHelper(
  MediaItem mediaItem,
  @Nullable MediaSource mediaSource,
  TrackSelectionParameters trackSelectionParameters,
- RendererCapabilities[] rendererCapabilities) {
+ RendererCapabilitiesList rendererCapabilities) {
  this.localConfiguration = checkNotNull(mediaItem.localConfiguration);
  this.mediaSource = mediaSource;
  this.trackSelector =
@@ -371,6 +394,7 @@ public void release() {
  mediaPreparer.release();
  }
  trackSelector.release();
+ rendererCapabilities.release();
  }
 
  /**
@@ -462,7 +486,7 @@ public List<ExoTrackSelection> getTrackSelections(int periodIndex, int rendererI
  */
  public void clearTrackSelections(int periodIndex) {
  assertPreparedWithMedia();
- for (int i = 0; i < rendererCapabilities.length; i++) {
+ for (int i = 0; i < rendererCapabilities.size(); i++) {
  trackSelectionsByPeriodAndRenderer[periodIndex][i].clear();
  }
  }
@@ -521,7 +545,7 @@ public void addAudioLanguagesToSelection(String... languages) {
  // Prefer highest supported bitrate for downloads.
  parametersBuilder.setForceHighestSupportedBitrate(true);
  // Disable all non-audio track types supported by the renderers.
- for (RendererCapabilities capabilities : rendererCapabilities) {
+ for (RendererCapabilities capabilities : rendererCapabilities.getRendererCapabilities()) {
  @C.TrackType int trackType = capabilities.getTrackType();
  parametersBuilder.setTrackTypeDisabled(
  trackType, /* disabled= */ trackType != C.TRACK_TYPE_AUDIO);
@@ -562,7 +586,7 @@ public void addTextLanguagesToSelection(
  // Prefer highest supported bitrate for downloads.
  parametersBuilder.setForceHighestSupportedBitrate(true);
  // Disable all non-text track types supported by the renderers.
- for (RendererCapabilities capabilities : rendererCapabilities) {
+ for (RendererCapabilities capabilities : rendererCapabilities.getRendererCapabilities()) {
  @C.TrackType int trackType = capabilities.getTrackType();
  parametersBuilder.setTrackTypeDisabled(
  trackType, /* disabled= */ trackType != C.TRACK_TYPE_TEXT);
@@ -694,7 +718,7 @@ private void onMediaPrepared() throws ExoPlaybackException {
  checkNotNull(mediaPreparer.mediaPeriods);
  checkNotNull(mediaPreparer.timeline);
  int periodCount = mediaPreparer.mediaPeriods.length;
- int rendererCount = rendererCapabilities.length;
+ int rendererCount = rendererCapabilities.size();
  trackSelectionsByPeriodAndRenderer =
  (List<ExoTrackSelection>[][]) new List<?>[periodCount][rendererCount];
  immutableTrackSelectionsByPeriodAndRenderer =
@@ -762,7 +786,7 @@ private void assertPreparedWithMedia() {
  private TrackSelectorResult runTrackSelection(int periodIndex) throws ExoPlaybackException {
  TrackSelectorResult trackSelectorResult =
  trackSelector.selectTracks(
- rendererCapabilities,
+ rendererCapabilities.getRendererCapabilities(),
  trackGroupArrays[periodIndex],
  new MediaPeriodId(mediaPreparer.timeline.getUidOfPeriod(periodIndex)),
  mediaPreparer.timeline);
@@ -1066,4 +1090,27 @@ public void removeEventListener(EventListener eventListener) {
  // Do nothing.
  }
  }
+
+ private static final class UnreleaseableRendererCapabilitiesList
+ implements RendererCapabilitiesList {
+
+ private final RendererCapabilities[] rendererCapabilities;
+
+ private UnreleaseableRendererCapabilitiesList(RendererCapabilities[] rendererCapabilities) {
+ this.rendererCapabilities = rendererCapabilities;
+ }
+
+ @Override
+ public RendererCapabilities[] getRendererCapabilities() {
+ return rendererCapabilities;
+ }
+
+ @Override
+ public int size() {
+ return rendererCapabilities.length;
+ }
+
+ @Override
+ public void release() {}
+ }
 }
diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/offline/DownloadHelperTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/offline/DownloadHelperTest.java
@@ -29,6 +29,7 @@
 import androidx.media3.common.TrackGroup;
 import androidx.media3.common.TrackSelectionOverride;
 import androidx.media3.common.TrackSelectionParameters;
+import androidx.media3.exoplayer.DefaultRendererCapabilitiesList;
 import androidx.media3.exoplayer.Renderer;
 import androidx.media3.exoplayer.RenderersFactory;
 import androidx.media3.exoplayer.offline.DownloadHelper.Callback;
@@ -39,6 +40,7 @@
 import androidx.media3.exoplayer.trackselection.ExoTrackSelection;
 import androidx.media3.exoplayer.trackselection.MappingTrackSelector.MappedTrackInfo;
 import androidx.media3.exoplayer.upstream.Allocator;
+import androidx.media3.test.utils.FakeDataSource;
 import androidx.media3.test.utils.FakeMediaPeriod;
 import androidx.media3.test.utils.FakeMediaSource;
 import androidx.media3.test.utils.FakeRenderer;
@@ -123,7 +125,8 @@ public void setUp() {
  testMediaItem,
  new TestMediaSource(),
  DownloadHelper.DEFAULT_TRACK_SELECTOR_PARAMETERS_WITHOUT_CONTEXT,
- DownloadHelper.getRendererCapabilities(renderersFactory));
+ new DefaultRendererCapabilitiesList.Factory(renderersFactory)
+ .createRendererCapabilitiesList());
  }
 
  @Test
@@ -439,6 +442,33 @@ public void getDownloadRequest_createsDownloadRequest_withMultipleOverridesOfSam
  new StreamKey(/* periodIndex= */ 0, /* groupIndex= */ 2, /* streamIndex= */ 0));
  }
 
+ // https://github.com/androidx/media/issues/1224
+ @Test
+ public void prepareThenRelease_renderersReleased() throws Exception {
+ // We can't use this.downloadHelper because we need access to the FakeRenderer instances for
+ // later assertions, so we recreate a local DownloadHelper.
+ FakeRenderer videoRenderer = new FakeRenderer(C.TRACK_TYPE_VIDEO);
+ FakeRenderer audioRenderer = new FakeRenderer(C.TRACK_TYPE_AUDIO);
+ FakeRenderer textRenderer = new FakeRenderer(C.TRACK_TYPE_TEXT);
+ RenderersFactory renderersFactory =
+ (handler, videoListener, audioListener, metadata, text) ->
+ new Renderer[] {textRenderer, audioRenderer, videoRenderer};
+
+ DownloadHelper downloadHelper =
+ DownloadHelper.forMediaItem(
+ testMediaItem,
+ DownloadHelper.DEFAULT_TRACK_SELECTOR_PARAMETERS_WITHOUT_CONTEXT,
+ renderersFactory,
+ new FakeDataSource.Factory());
+
+ prepareDownloadHelper(downloadHelper);
+ downloadHelper.release();
+
+ assertThat(videoRenderer.isReleased).isTrue();
+ assertThat(audioRenderer.isReleased).isTrue();
+ assertThat(textRenderer.isReleased).isTrue();
+ }
+
  private static void prepareDownloadHelper(DownloadHelper downloadHelper) throws Exception {
  AtomicReference<Exception> prepareException = new AtomicReference<>(null);
  CountDownLatch preparedLatch = new CountDownLatch(1);