diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 4970515cd9..47f8716114 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -11,6 +11,9 @@ `ExoPlayer.Builder`. * Fix bug where prepare errors in the content of `AdsMediaSource` may be never reported ([#2337](https://github.com/androidx/media/issues/2337)). + * Fix memory leak in `MergingMediaSource`, for example used when + sideloading subtitles + ([#2338](https://github.com/androidx/media/issues/2338)). * Transformer: * Filling an initial gap (added via `addGap()`) with silent audio now requires explicitly setting `experimentalSetForceAudioTrack(true)` in diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/MergingMediaSource.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/MergingMediaSource.java index d61a5b2319..d8f136ad57 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/MergingMediaSource.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/MergingMediaSource.java @@ -245,8 +245,9 @@ public final class MergingMediaSource extends CompositeMediaSource { MergingMediaPeriod mergingPeriod = (MergingMediaPeriod) mediaPeriod; for (int i = 0; i < mediaSources.length; i++) { List mediaPeriodsForSource = mediaPeriods.get(i); + MediaPeriod childPeriod = mergingPeriod.getChildPeriod(i); for (int j = 0; j < mediaPeriodsForSource.size(); j++) { - if (mediaPeriodsForSource.get(j).mediaPeriod.equals(mediaPeriod)) { + if (mediaPeriodsForSource.get(j).mediaPeriod.equals(childPeriod)) { mediaPeriodsForSource.remove(j); break; } diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/MergingMediaSourceTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/MergingMediaSourceTest.java index 5630e97945..7335fa02bd 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/MergingMediaSourceTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/MergingMediaSourceTest.java @@ -16,6 +16,7 @@ package androidx.media3.exoplayer.source; import static androidx.media3.test.utils.robolectric.RobolectricUtil.DEFAULT_TIMEOUT_MS; +import static androidx.media3.test.utils.robolectric.RobolectricUtil.runMainLooperUntil; import static com.google.common.truth.Truth.assertThat; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.junit.Assert.assertThrows; @@ -24,8 +25,11 @@ import androidx.annotation.Nullable; import androidx.media3.common.C; import androidx.media3.common.Timeline; import androidx.media3.common.util.Util; +import androidx.media3.exoplayer.analytics.PlayerId; import androidx.media3.exoplayer.source.MediaSource.MediaPeriodId; import androidx.media3.exoplayer.source.MergingMediaSource.IllegalMergeException; +import androidx.media3.exoplayer.upstream.Allocator; +import androidx.media3.exoplayer.upstream.DefaultAllocator; import androidx.media3.test.utils.FakeMediaPeriod; import androidx.media3.test.utils.FakeMediaSource; import androidx.media3.test.utils.FakeTimeline; @@ -36,7 +40,10 @@ import com.google.common.collect.ConcurrentHashMultiset; import com.google.common.collect.ImmutableList; import com.google.common.collect.Multiset; import java.io.IOException; +import java.lang.ref.WeakReference; +import java.util.ArrayList; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicReference; import org.junit.Test; import org.junit.runner.RunWith; @@ -219,6 +226,57 @@ public class MergingMediaSourceTest { } } + @Test + public void createAndReleasePeriods_cleansUpReferences() throws Exception { + ArrayList> createdChildPeriods = new ArrayList<>(); + FakeMediaSource childSource = + new FakeMediaSource() { + @Override + public MediaPeriod createPeriod( + MediaPeriodId id, Allocator allocator, long startPositionUs) { + MediaPeriod period = super.createPeriod(id, allocator, startPositionUs); + createdChildPeriods.add(new WeakReference<>(period)); + return period; + } + }; + Allocator allocator = + new DefaultAllocator(/* trimOnReset= */ true, /* individualAllocationSize= */ 1024); + MergingMediaSource mergingMediaSource = + new MergingMediaSource( + /* adjustPeriodTimeOffsets= */ true, + /* clipDurations= */ true, + childSource, + childSource); + AtomicReference timelineReference = new AtomicReference<>(); + mergingMediaSource.prepareSource( + (source, timeline) -> timelineReference.set(timeline), + /* mediaTransferListener= */ null, + PlayerId.UNSET); + runMainLooperUntil(() -> timelineReference.get() != null); + Object periodUid = timelineReference.get().getUidOfPeriod(/* periodIndex= */ 0); + + // Create 20 periods. + ArrayList createdMergedPeriods = new ArrayList<>(); + for (int i = 0; i < 20; i++) { + createdMergedPeriods.add( + mergingMediaSource.createPeriod( + new MediaPeriodId(periodUid, /* windowSequenceNumber= */ i), + allocator, + /* startPositionUs= */ 0)); + } + // Release 19 periods. + for (int i = 0; i < 19; i++) { + mergingMediaSource.releasePeriod(createdMergedPeriods.remove(0)); + } + // Ensure all pending references are garbage collected + Runtime.getRuntime().gc(); + + // Assert that only 2 child periods remain in memory. + long nonNullChildPeriodReferences = + createdChildPeriods.stream().filter(reference -> reference.get() != null).count(); + assertThat(nonNullChildPeriodReferences).isEqualTo(2); + } + /** * Wraps the specified timelines in a {@link MergingMediaSource}, prepares it and returns the * merged timeline.