diff --git a/library/core/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java b/library/core/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java index d102d2c637..c4c50eaa03 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java @@ -1785,6 +1785,7 @@ public class SimpleExoPlayer extends BasePlayer wifiLockManager.setStayAwake(false); audioFocusManager.release(); player.release(); + analyticsCollector.release(); removeSurfaceCallbacks(); if (surface != null) { if (ownsSurface) { diff --git a/library/core/src/main/java/com/google/android/exoplayer2/analytics/AnalyticsCollector.java b/library/core/src/main/java/com/google/android/exoplayer2/analytics/AnalyticsCollector.java index e8ab433d55..f09e5db812 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/analytics/AnalyticsCollector.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/analytics/AnalyticsCollector.java @@ -144,6 +144,19 @@ public class AnalyticsCollector }); } + /** + * Releases the collector. Must be called after the player for which data is collected has been + * released. + */ + public void release() { + EventTime eventTime = generateCurrentPlayerMediaPeriodEventTime(); + eventTimes.put(AnalyticsListener.EVENT_PLAYER_RELEASED, eventTime); + // Release listeners lazily so that all events that got triggered as part of player.release() + // are still delivered to all listeners. + listeners.lazyRelease( + AnalyticsListener.EVENT_PLAYER_RELEASED, listener -> listener.onPlayerReleased(eventTime)); + } + /** * Updates the playback queue information used for event association. * diff --git a/library/core/src/main/java/com/google/android/exoplayer2/analytics/AnalyticsListener.java b/library/core/src/main/java/com/google/android/exoplayer2/analytics/AnalyticsListener.java index 6ac6ae5847..6589b98351 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/analytics/AnalyticsListener.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/analytics/AnalyticsListener.java @@ -196,7 +196,8 @@ public interface AnalyticsListener { EVENT_DRM_SESSION_MANAGER_ERROR, EVENT_DRM_KEYS_RESTORED, EVENT_DRM_KEYS_REMOVED, - EVENT_DRM_SESSION_RELEASED + EVENT_DRM_SESSION_RELEASED, + EVENT_PLAYER_RELEASED, }) @interface EventFlags {} /** {@link Player#getCurrentTimeline()} changed. */ @@ -309,6 +310,8 @@ public interface AnalyticsListener { int EVENT_DRM_KEYS_REMOVED = 1034; /** A DRM session has been released. */ int EVENT_DRM_SESSION_RELEASED = 1035; + /** The player was released. */ + int EVENT_PLAYER_RELEASED = 1036; /** Time information of an event. */ final class EventTime { @@ -1013,6 +1016,13 @@ public interface AnalyticsListener { */ default void onDrmSessionReleased(EventTime eventTime) {} + /** + * Called when the {@link Player} is released. + * + * @param eventTime The event time. + */ + default void onPlayerReleased(EventTime eventTime) {} + /** * Called after one or more events occurred. * diff --git a/library/core/src/main/java/com/google/android/exoplayer2/util/ListenerSet.java b/library/core/src/main/java/com/google/android/exoplayer2/util/ListenerSet.java index 24bf624338..d0df7a662e 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/util/ListenerSet.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/util/ListenerSet.java @@ -70,8 +70,9 @@ public final class ListenerSet { } private static final int MSG_ITERATION_FINISHED = 0; + private static final int MSG_LAZY_RELEASE = 1; - private final Handler iterationFinishedHandler; + private final Handler handler; private final Supplier eventFlagsSupplier; private final IterationFinishedEvent iterationFinishedEvent; private final CopyOnWriteArraySet> listeners; @@ -113,8 +114,8 @@ public final class ListenerSet { queuedEvents = new ArrayDeque<>(); // It's safe to use "this" because we don't send a message before exiting the constructor. @SuppressWarnings("methodref.receiver.bound.invalid") - Handler handler = Util.createHandler(looper, this::handleIterationFinished); - iterationFinishedHandler = handler; + Handler handler = Util.createHandler(looper, this::handleMessage); + this.handler = handler; } /** @@ -165,8 +166,8 @@ public final class ListenerSet { /** * Adds an event that is sent to the listeners when {@link #flushEvents} is called. * - * @param eventFlag An integer indicating the type of the event, or {@link C#INDEX_UNSET} to not - * report this event with a flag. + * @param eventFlag An integer indicating the type of the event, or {@link C#INDEX_UNSET} to + * report this event without flag. * @param event The event. */ public void queueEvent(int eventFlag, Event event) { @@ -185,8 +186,8 @@ public final class ListenerSet { if (queuedEvents.isEmpty()) { return; } - if (!iterationFinishedHandler.hasMessages(MSG_ITERATION_FINISHED)) { - iterationFinishedHandler.obtainMessage(MSG_ITERATION_FINISHED).sendToTarget(); + if (!handler.hasMessages(MSG_ITERATION_FINISHED)) { + handler.obtainMessage(MSG_ITERATION_FINISHED).sendToTarget(); } boolean recursiveFlushInProgress = !flushingEvents.isEmpty(); flushingEvents.addAll(queuedEvents); @@ -206,7 +207,7 @@ public final class ListenerSet { * flushes} the event queue to notify all listeners. * * @param eventFlag An integer flag indicating the type of the event, or {@link C#INDEX_UNSET} to - * not report this event with a flag. + * report this event without flag. * @param event The event. */ public void sendEvent(int eventFlag, Event event) { @@ -215,7 +216,7 @@ public final class ListenerSet { } /** - * Releases the set of listeners. + * Releases the set of listeners immediately. * *

This will ensure no events are sent to any listener after this method has been called. */ @@ -227,15 +228,37 @@ public final class ListenerSet { released = true; } - private boolean handleIterationFinished(Message message) { - for (ListenerHolder holder : listeners) { - holder.iterationFinished(eventFlagsSupplier, iterationFinishedEvent); - if (iterationFinishedHandler.hasMessages(MSG_ITERATION_FINISHED)) { - // The invocation above triggered new events (and thus scheduled a new message). We need to - // stop here because this new message will take care of informing every listener about the - // new update (including the ones already called here). - break; + /** + * Releases the set of listeners after all already scheduled {@link Looper} messages were able to + * trigger final events. + * + *

After the specified released callback event, no other events are sent to a listener. + * + * @param releaseEventFlag An integer flag indicating the type of the release event, or {@link + * C#INDEX_UNSET} to report this event without a flag. + * @param releaseEvent The release event. + */ + public void lazyRelease(int releaseEventFlag, Event releaseEvent) { + handler.obtainMessage(MSG_LAZY_RELEASE, releaseEventFlag, 0, releaseEvent).sendToTarget(); + } + + private boolean handleMessage(Message message) { + if (message.what == MSG_ITERATION_FINISHED) { + for (ListenerHolder holder : listeners) { + holder.iterationFinished(eventFlagsSupplier, iterationFinishedEvent); + if (handler.hasMessages(MSG_ITERATION_FINISHED)) { + // The invocation above triggered new events (and thus scheduled a new message). We need + // to stop here because this new message will take care of informing every listener about + // the new update (including the ones already called here). + break; + } } + } else if (message.what == MSG_LAZY_RELEASE) { + int releaseEventFlag = message.arg1; + @SuppressWarnings("unchecked") + Event releaseEvent = (Event) message.obj; + sendEvent(releaseEventFlag, releaseEvent); + release(); } return true; } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/SimpleExoPlayerTest.java b/library/core/src/test/java/com/google/android/exoplayer2/SimpleExoPlayerTest.java index e3a625a3ce..a11961c301 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/SimpleExoPlayerTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/SimpleExoPlayerTest.java @@ -15,14 +15,25 @@ */ package com.google.android.exoplayer2; +import static com.google.android.exoplayer2.robolectric.TestPlayerRunHelper.runUntilPlaybackState; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; +import com.google.android.exoplayer2.analytics.AnalyticsListener; +import com.google.android.exoplayer2.testutil.AutoAdvancingFakeClock; +import com.google.android.exoplayer2.testutil.ExoPlayerTestRunner; +import com.google.android.exoplayer2.testutil.FakeMediaSource; +import com.google.android.exoplayer2.testutil.FakeTimeline; +import com.google.android.exoplayer2.testutil.FakeVideoRenderer; import java.util.concurrent.atomic.AtomicReference; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.annotation.Config; +import org.robolectric.shadows.ShadowLooper; /** Unit test for {@link SimpleExoPlayer}. */ @RunWith(AndroidJUnit4.class) @@ -43,4 +54,29 @@ public class SimpleExoPlayerTest { assertThat(builderThrow.get()).isNull(); } + + @Test + public void release_triggersAllPendingEventsInAnalyticsListeners() throws Exception { + SimpleExoPlayer player = + new SimpleExoPlayer.Builder( + ApplicationProvider.getApplicationContext(), + (handler, videoListener, audioListener, textOutput, metadataOutput) -> + new Renderer[] {new FakeVideoRenderer(handler, videoListener)}) + .setClock(new AutoAdvancingFakeClock()) + .build(); + AnalyticsListener listener = mock(AnalyticsListener.class); + player.addAnalyticsListener(listener); + // Do something that requires clean-up callbacks like decoder disabling. + player.setMediaSource( + new FakeMediaSource(new FakeTimeline(), ExoPlayerTestRunner.VIDEO_FORMAT)); + player.prepare(); + player.play(); + runUntilPlaybackState(player, Player.STATE_READY); + + player.release(); + ShadowLooper.runMainLooperToNextTask(); + + verify(listener).onVideoDisabled(any(), any()); + verify(listener).onPlayerReleased(any()); + } } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/util/ListenerSetTest.java b/library/core/src/test/java/com/google/android/exoplayer2/util/ListenerSetTest.java index 19ffd24899..6526b4db22 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/util/ListenerSetTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/util/ListenerSetTest.java @@ -22,6 +22,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; +import android.os.Handler; import android.os.Looper; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.android.exoplayer2.C; @@ -366,6 +367,34 @@ public class ListenerSetTest { verify(listener, never()).callback1(); } + @Test + public void lazyRelease_stopsForwardingEventsFromNewHandlerMessagesAndCallsReleaseCallback() { + ListenerSet listenerSet = + new ListenerSet<>(Looper.myLooper(), Flags::new, TestListener::iterationFinished); + TestListener listener = mock(TestListener.class); + listenerSet.add(listener); + + // In-line event before release. + listenerSet.sendEvent(EVENT_ID_1, TestListener::callback1); + // Message triggering event sent before release. + new Handler().post(() -> listenerSet.sendEvent(EVENT_ID_1, TestListener::callback1)); + // Lazy release with release callback. + listenerSet.lazyRelease(EVENT_ID_3, TestListener::callback3); + // In-line event after release. + listenerSet.sendEvent(EVENT_ID_1, TestListener::callback1); + // Message triggering event sent after release. + new Handler().post(() -> listenerSet.sendEvent(EVENT_ID_2, TestListener::callback2)); + ShadowLooper.runMainLooperToNextTask(); + + // Verify all events are delivered except for the one triggered by the message sent after the + // lazy release. + verify(listener, times(3)).callback1(); + verify(listener).callback3(); + verify(listener).iterationFinished(Flags.create(EVENT_ID_1)); + verify(listener).iterationFinished(Flags.create(EVENT_ID_1, EVENT_ID_3)); + verifyNoMoreInteractions(listener); + } + private interface TestListener { default void callback1() {}