Fix memory leak caused by removing wrong period from List

MergingMediaSource started keeping references to all MediaPeriods
it ever created since 191bc094a5 as the cleanup step attempted
to remove the outer merged period from the list instead of the
inner period of child source.

Issue: androidx/media#2338
PiperOrigin-RevId: 750566825
This commit is contained in:
tonihei 2025-04-23 06:19:36 -07:00 committed by Copybara-Service
parent a7c897003c
commit 50345b03aa
3 changed files with 63 additions and 1 deletions

View File

@ -11,6 +11,9 @@
`ExoPlayer.Builder`. `ExoPlayer.Builder`.
* Fix bug where prepare errors in the content of `AdsMediaSource` may be * Fix bug where prepare errors in the content of `AdsMediaSource` may be
never reported ([#2337](https://github.com/androidx/media/issues/2337)). 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: * Transformer:
* Filling an initial gap (added via `addGap()`) with silent audio now * Filling an initial gap (added via `addGap()`) with silent audio now
requires explicitly setting `experimentalSetForceAudioTrack(true)` in requires explicitly setting `experimentalSetForceAudioTrack(true)` in

View File

@ -245,8 +245,9 @@ public final class MergingMediaSource extends CompositeMediaSource<Integer> {
MergingMediaPeriod mergingPeriod = (MergingMediaPeriod) mediaPeriod; MergingMediaPeriod mergingPeriod = (MergingMediaPeriod) mediaPeriod;
for (int i = 0; i < mediaSources.length; i++) { for (int i = 0; i < mediaSources.length; i++) {
List<MediaPeriodAndId> mediaPeriodsForSource = mediaPeriods.get(i); List<MediaPeriodAndId> mediaPeriodsForSource = mediaPeriods.get(i);
MediaPeriod childPeriod = mergingPeriod.getChildPeriod(i);
for (int j = 0; j < mediaPeriodsForSource.size(); j++) { 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); mediaPeriodsForSource.remove(j);
break; break;
} }

View File

@ -16,6 +16,7 @@
package androidx.media3.exoplayer.source; package androidx.media3.exoplayer.source;
import static androidx.media3.test.utils.robolectric.RobolectricUtil.DEFAULT_TIMEOUT_MS; 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 com.google.common.truth.Truth.assertThat;
import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertThrows;
@ -24,8 +25,11 @@ import androidx.annotation.Nullable;
import androidx.media3.common.C; import androidx.media3.common.C;
import androidx.media3.common.Timeline; import androidx.media3.common.Timeline;
import androidx.media3.common.util.Util; import androidx.media3.common.util.Util;
import androidx.media3.exoplayer.analytics.PlayerId;
import androidx.media3.exoplayer.source.MediaSource.MediaPeriodId; import androidx.media3.exoplayer.source.MediaSource.MediaPeriodId;
import androidx.media3.exoplayer.source.MergingMediaSource.IllegalMergeException; 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.FakeMediaPeriod;
import androidx.media3.test.utils.FakeMediaSource; import androidx.media3.test.utils.FakeMediaSource;
import androidx.media3.test.utils.FakeTimeline; 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.ImmutableList;
import com.google.common.collect.Multiset; import com.google.common.collect.Multiset;
import java.io.IOException; import java.io.IOException;
import java.lang.ref.WeakReference;
import java.util.ArrayList;
import java.util.concurrent.CountDownLatch; import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicReference;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
@ -219,6 +226,57 @@ public class MergingMediaSourceTest {
} }
} }
@Test
public void createAndReleasePeriods_cleansUpReferences() throws Exception {
ArrayList<WeakReference<MediaPeriod>> 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<Timeline> 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<MediaPeriod> 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 * Wraps the specified timelines in a {@link MergingMediaSource}, prepares it and returns the
* merged timeline. * merged timeline.