From 69af3897309efababfe4d7bacc03497c9fb9ede8 Mon Sep 17 00:00:00 2001 From: olly Date: Mon, 3 Oct 2016 09:38:40 -0700 Subject: [PATCH] Trim allocator on stop/reset by default This prevents a large amount of memory from being held in the case that a player instance is released, but the application is holding dangling references to the player that are preventing it from being garbage collected. Issue: #1855 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=134992945 --- .../exoplayer2/DefaultLoadControl.java | 25 ++++++++++++++++--- .../android/exoplayer2/ExoPlayerImpl.java | 2 +- .../exoplayer2/ExoPlayerImplInternal.java | 16 ++++++------ .../android/exoplayer2/LoadControl.java | 14 +++++++++-- .../exoplayer2/upstream/DefaultAllocator.java | 19 +++++++++++--- 5 files changed, 59 insertions(+), 17 deletions(-) diff --git a/library/src/main/java/com/google/android/exoplayer2/DefaultLoadControl.java b/library/src/main/java/com/google/android/exoplayer2/DefaultLoadControl.java index 1416dd0ae4..e7a0f8b1b8 100644 --- a/library/src/main/java/com/google/android/exoplayer2/DefaultLoadControl.java +++ b/library/src/main/java/com/google/android/exoplayer2/DefaultLoadControl.java @@ -68,7 +68,7 @@ public final class DefaultLoadControl implements LoadControl { * Constructs a new instance, using the {@code DEFAULT_*} constants defined in this class. */ public DefaultLoadControl() { - this(new DefaultAllocator(C.DEFAULT_BUFFER_SEGMENT_SIZE)); + this(new DefaultAllocator(true, C.DEFAULT_BUFFER_SEGMENT_SIZE)); } /** @@ -104,6 +104,11 @@ public final class DefaultLoadControl implements LoadControl { bufferForPlaybackAfterRebufferUs = bufferForPlaybackAfterRebufferMs * 1000L; } + @Override + public void onPrepared() { + reset(false); + } + @Override public void onTracksSelected(Renderer[] renderers, TrackGroupArray trackGroups, TrackSelections trackSelections) { @@ -117,9 +122,13 @@ public final class DefaultLoadControl implements LoadControl { } @Override - public void onTracksDisabled() { - targetBufferSize = 0; - isBuffering = false; + public void onStopped() { + reset(true); + } + + @Override + public void onReleased() { + reset(true); } @Override @@ -147,4 +156,12 @@ public final class DefaultLoadControl implements LoadControl { : (bufferedDurationUs < minBufferUs ? BELOW_LOW_WATERMARK : BETWEEN_WATERMARKS); } + private void reset(boolean resetAllocator) { + targetBufferSize = 0; + isBuffering = false; + if (resetAllocator) { + allocator.reset(); + } + } + } diff --git a/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java b/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java index 6ced2315f9..d1f72ed059 100644 --- a/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java +++ b/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java @@ -106,7 +106,7 @@ import java.util.concurrent.CopyOnWriteArraySet; @Override public void prepare(MediaSource mediaSource, boolean resetPosition) { timeline = null; - internalPlayer.setMediaSource(mediaSource, resetPosition); + internalPlayer.prepare(mediaSource, resetPosition); } @Override diff --git a/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java b/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java index 2a9360337a..039d6dd810 100644 --- a/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java +++ b/library/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java @@ -75,7 +75,7 @@ import java.io.IOException; public static final int MSG_ERROR = 6; // Internal messages - private static final int MSG_SET_MEDIA_SOURCE = 0; + private static final int MSG_PREPARE = 0; private static final int MSG_SET_PLAY_WHEN_READY = 1; private static final int MSG_DO_SOME_WORK = 2; private static final int MSG_SEEK_TO = 3; @@ -164,8 +164,8 @@ import java.io.IOException; handler = new Handler(internalPlaybackThread.getLooper(), this); } - public void setMediaSource(MediaSource mediaSource, boolean resetPosition) { - handler.obtainMessage(MSG_SET_MEDIA_SOURCE, resetPosition ? 1 : 0, 0, mediaSource) + public void prepare(MediaSource mediaSource, boolean resetPosition) { + handler.obtainMessage(MSG_PREPARE, resetPosition ? 1 : 0, 0, mediaSource) .sendToTarget(); } @@ -253,8 +253,8 @@ import java.io.IOException; public boolean handleMessage(Message msg) { try { switch (msg.what) { - case MSG_SET_MEDIA_SOURCE: { - setMediaSourceInternal((MediaSource) msg.obj, msg.arg1 != 0); + case MSG_PREPARE: { + prepareInternal((MediaSource) msg.obj, msg.arg1 != 0); return true; } case MSG_SET_PLAY_WHEN_READY: { @@ -335,9 +335,10 @@ import java.io.IOException; } } - private void setMediaSourceInternal(MediaSource mediaSource, boolean resetPosition) + private void prepareInternal(MediaSource mediaSource, boolean resetPosition) throws ExoPlaybackException { resetInternal(); + loadControl.onPrepared(); if (resetPosition) { playbackInfo = new PlaybackInfo(0, C.TIME_UNSET); } @@ -597,11 +598,13 @@ import java.io.IOException; private void stopInternal() { resetInternal(); + loadControl.onStopped(); setState(ExoPlayer.STATE_IDLE); } private void releaseInternal() { resetInternal(); + loadControl.onReleased(); setState(ExoPlayer.STATE_IDLE); synchronized (this) { released = true; @@ -638,7 +641,6 @@ import java.io.IOException; loadingPeriodHolder = null; timeline = null; bufferAheadPeriodCount = 0; - loadControl.onTracksDisabled(); setIsLoading(false); } diff --git a/library/src/main/java/com/google/android/exoplayer2/LoadControl.java b/library/src/main/java/com/google/android/exoplayer2/LoadControl.java index 7842ada276..6176c6085b 100644 --- a/library/src/main/java/com/google/android/exoplayer2/LoadControl.java +++ b/library/src/main/java/com/google/android/exoplayer2/LoadControl.java @@ -25,6 +25,11 @@ import com.google.android.exoplayer2.upstream.Allocator; */ public interface LoadControl { + /** + * Called by the player when prepared with a new source. + */ + void onPrepared(); + /** * Called by the player when a track selection occurs. * @@ -36,9 +41,14 @@ public interface LoadControl { TrackSelections trackSelections); /** - * Called by the player when all tracks are disabled. + * Called by the player when stopped. */ - void onTracksDisabled(); + void onStopped(); + + /** + * Called by the player when released. + */ + void onReleased(); /** * Returns the {@link Allocator} that should be used to obtain media buffer allocations. diff --git a/library/src/main/java/com/google/android/exoplayer2/upstream/DefaultAllocator.java b/library/src/main/java/com/google/android/exoplayer2/upstream/DefaultAllocator.java index 5ccfa5f899..d9bd5873f0 100644 --- a/library/src/main/java/com/google/android/exoplayer2/upstream/DefaultAllocator.java +++ b/library/src/main/java/com/google/android/exoplayer2/upstream/DefaultAllocator.java @@ -26,6 +26,7 @@ public final class DefaultAllocator implements Allocator { private static final int AVAILABLE_EXTRA_CAPACITY = 100; + private final boolean trimOnReset; private final int individualAllocationSize; private final byte[] initialAllocationBlock; private final Allocation[] singleAllocationReleaseHolder; @@ -38,10 +39,12 @@ public final class DefaultAllocator implements Allocator { /** * Constructs an instance without creating any {@link Allocation}s up front. * + * @param trimOnReset Whether memory is freed when the allocator is reset. Should be true unless + * the allocator will be re-used by multiple player instances. * @param individualAllocationSize The length of each individual {@link Allocation}. */ - public DefaultAllocator(int individualAllocationSize) { - this(individualAllocationSize, 0); + public DefaultAllocator(boolean trimOnReset, int individualAllocationSize) { + this(trimOnReset, individualAllocationSize, 0); } /** @@ -49,12 +52,16 @@ public final class DefaultAllocator implements Allocator { *

* Note: {@link Allocation}s created up front will never be discarded by {@link #trim()}. * + * @param trimOnReset Whether memory is freed when the allocator is reset. Should be true unless + * the allocator will be re-used by multiple player instances. * @param individualAllocationSize The length of each individual {@link Allocation}. * @param initialAllocationCount The number of allocations to create up front. */ - public DefaultAllocator(int individualAllocationSize, int initialAllocationCount) { + public DefaultAllocator(boolean trimOnReset, int individualAllocationSize, + int initialAllocationCount) { Assertions.checkArgument(individualAllocationSize > 0); Assertions.checkArgument(initialAllocationCount >= 0); + this.trimOnReset = trimOnReset; this.individualAllocationSize = individualAllocationSize; this.availableCount = initialAllocationCount; this.availableAllocations = new Allocation[initialAllocationCount + AVAILABLE_EXTRA_CAPACITY]; @@ -70,6 +77,12 @@ public final class DefaultAllocator implements Allocator { singleAllocationReleaseHolder = new Allocation[1]; } + public synchronized void reset() { + if (trimOnReset) { + setTargetBufferSize(0); + } + } + public synchronized void setTargetBufferSize(int targetBufferSize) { boolean targetBufferSizeReduced = targetBufferSize < this.targetBufferSize; this.targetBufferSize = targetBufferSize;