diff --git a/library/core/src/main/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManager.java b/library/core/src/main/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManager.java index 2cc8456e12..7d4d6087dc 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManager.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManager.java @@ -105,6 +105,10 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag @Override public synchronized void updateSessions(EventTime eventTime) { Assertions.checkNotNull(listener); + if (eventTime.timeline.isEmpty()) { + // Don't try to create new sessions for empty timelines. + return; + } @Nullable SessionDescriptor currentSession = sessions.get(currentSessionId); if (eventTime.mediaPeriodId != null && currentSession != null) { // If we receive an event associated with a media period, then it needs to be either part of @@ -234,6 +238,11 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag @RequiresNonNull("listener") private void updateCurrentSession(EventTime eventTime) { + if (eventTime.timeline.isEmpty()) { + // Clear current session if the Timeline is empty. + currentSessionId = null; + return; + } @Nullable SessionDescriptor previousSessionDescriptor = sessions.get(currentSessionId); SessionDescriptor currentSessionDescriptor = getOrAddSession(eventTime.windowIndex, eventTime.mediaPeriodId); diff --git a/library/core/src/main/java/com/google/android/exoplayer2/analytics/PlaybackStatsListener.java b/library/core/src/main/java/com/google/android/exoplayer2/analytics/PlaybackStatsListener.java index 8b9967addb..9465bd2387 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/analytics/PlaybackStatsListener.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/analytics/PlaybackStatsListener.java @@ -240,7 +240,7 @@ public final class PlaybackStatsListener if (events.size() == 0) { return; } - maybeAddSessions(player, events); + maybeAddSessions(events); for (String session : playbackStatsTrackers.keySet()) { Pair eventTimeAndBelongsToPlayback = findBestEventTime(events, session); PlaybackStatsTracker tracker = playbackStatsTrackers.get(session); @@ -281,17 +281,15 @@ public final class PlaybackStatsListener } } - private void maybeAddSessions(Player player, Events events) { - boolean isCompletelyIdle = - player.getCurrentTimeline().isEmpty() && player.getPlaybackState() == Player.STATE_IDLE; + private void maybeAddSessions(Events events) { for (int i = 0; i < events.size(); i++) { @EventFlags int event = events.get(i); EventTime eventTime = events.getEventTime(event); if (event == EVENT_TIMELINE_CHANGED) { sessionManager.updateSessionsWithTimelineChange(eventTime); - } else if (!isCompletelyIdle && event == EVENT_POSITION_DISCONTINUITY) { + } else if (event == EVENT_POSITION_DISCONTINUITY) { sessionManager.updateSessionsWithDiscontinuity(eventTime, discontinuityReason); - } else if (!isCompletelyIdle) { + } else { sessionManager.updateSessions(eventTime); } } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManagerTest.java b/library/core/src/test/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManagerTest.java index 72998db0e0..930a835cec 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManagerTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManagerTest.java @@ -59,6 +59,16 @@ public final class DefaultPlaybackSessionManagerTest { sessionManager.setListener(mockListener); } + @Test + public void updatesSession_withEmptyTimeline_doesNotCreateNewSession() { + EventTime eventTime = + createEventTime(Timeline.EMPTY, /* windowIndex= */ 0, /* mediaPeriodId */ null); + + sessionManager.updateSessions(eventTime); + + verifyNoMoreInteractions(mockListener); + } + @Test public void updateSessions_withoutMediaPeriodId_createsNewSession() { Timeline timeline = new FakeTimeline(); @@ -486,8 +496,6 @@ public final class DefaultPlaybackSessionManagerTest { @Test public void belongsToSession_withSameWindowIndex_returnsTrue() { - EventTime eventTime = - createEventTime(Timeline.EMPTY, /* windowIndex= */ 0, /* mediaPeriodId= */ null); Timeline timeline = new FakeTimeline(); EventTime eventTimeWithTimeline = createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId= */ null); @@ -496,11 +504,10 @@ public final class DefaultPlaybackSessionManagerTest { timeline.getUidOfPeriod(/* periodIndex= */ 0), /* windowSequenceNumber= */ 0); EventTime eventTimeWithMediaPeriodId = createEventTime(timeline, /* windowIndex= */ 0, mediaPeriodId); - sessionManager.updateSessions(eventTime); + sessionManager.updateSessions(eventTimeWithTimeline); ArgumentCaptor sessionId = ArgumentCaptor.forClass(String.class); - verify(mockListener).onSessionCreated(eq(eventTime), sessionId.capture()); - assertThat(sessionManager.belongsToSession(eventTime, sessionId.getValue())).isTrue(); + verify(mockListener).onSessionCreated(eq(eventTimeWithTimeline), sessionId.capture()); assertThat(sessionManager.belongsToSession(eventTimeWithTimeline, sessionId.getValue())) .isTrue(); assertThat(sessionManager.belongsToSession(eventTimeWithMediaPeriodId, sessionId.getValue())) @@ -509,11 +516,11 @@ public final class DefaultPlaybackSessionManagerTest { @Test public void belongsToSession_withOtherWindowIndex_returnsFalse() { - EventTime eventTime = - createEventTime(Timeline.EMPTY, /* windowIndex= */ 0, /* mediaPeriodId= */ null); - EventTime eventTimeOtherWindow = - createEventTime(Timeline.EMPTY, /* windowIndex= */ 1, /* mediaPeriodId= */ null); Timeline timeline = new FakeTimeline(/* windowCount= */ 2); + EventTime eventTime = + createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId= */ null); + EventTime eventTimeOtherWindow = + createEventTime(timeline, /* windowIndex= */ 1, /* mediaPeriodId= */ null); MediaPeriodId mediaPeriodId = new MediaPeriodId( timeline.getUidOfPeriod(/* periodIndex= */ 1), /* windowSequenceNumber= */ 1); @@ -585,18 +592,18 @@ public final class DefaultPlaybackSessionManagerTest { } @Test - public void initialTimelineUpdate_finishesAllSessionsOutsideTimeline() { + public void timelineUpdate_toEmpty_finishesAllSessionsAndDoesNotCreateNewSessions() { + Timeline timeline = new FakeTimeline(/* windowCount= */ 2); EventTime eventTime1 = - createEventTime(Timeline.EMPTY, /* windowIndex= */ 0, /* mediaPeriodId= */ null); + createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId= */ null); EventTime eventTime2 = - createEventTime(Timeline.EMPTY, /* windowIndex= */ 1, /* mediaPeriodId= */ null); + createEventTime(timeline, /* windowIndex= */ 1, /* mediaPeriodId= */ null); sessionManager.updateSessions(eventTime1); sessionManager.updateSessions(eventTime2); - Timeline timeline = new FakeTimeline(); - EventTime newTimelineEventTime = - createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId= */ null); - sessionManager.updateSessionsWithTimelineChange(newTimelineEventTime); + EventTime eventTimeWithEmptyTimeline = + createEventTime(Timeline.EMPTY, /* windowIndex= */ 0, /* mediaPeriodId= */ null); + sessionManager.updateSessionsWithTimelineChange(eventTimeWithEmptyTimeline); ArgumentCaptor sessionId1 = ArgumentCaptor.forClass(String.class); ArgumentCaptor sessionId2 = ArgumentCaptor.forClass(String.class); @@ -605,14 +612,19 @@ public final class DefaultPlaybackSessionManagerTest { verify(mockListener).onSessionActive(eventTime1, sessionId1.getValue()); verify(mockListener) .onSessionFinished( - newTimelineEventTime, + eventTimeWithEmptyTimeline, + sessionId1.getValue(), + /* automaticTransitionToNextPlayback= */ false); + verify(mockListener) + .onSessionFinished( + eventTimeWithEmptyTimeline, sessionId2.getValue(), /* automaticTransitionToNextPlayback= */ false); verifyNoMoreInteractions(mockListener); } @Test - public void dynamicTimelineUpdate_resolvesWindowIndices() { + public void timelineUpdate_resolvesWindowIndices() { Timeline initialTimeline = new FakeTimeline( new TimelineWindowDefinition(/* periodCount= */ 2, /* id= */ 100), diff --git a/library/core/src/test/java/com/google/android/exoplayer2/analytics/PlaybackStatsListenerTest.java b/library/core/src/test/java/com/google/android/exoplayer2/analytics/PlaybackStatsListenerTest.java index 0fa1a4a85b..668ed5f556 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/analytics/PlaybackStatsListenerTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/analytics/PlaybackStatsListenerTest.java @@ -22,6 +22,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import androidx.annotation.Nullable; import androidx.test.core.app.ApplicationProvider; @@ -77,15 +78,18 @@ public final class PlaybackStatsListenerTest { } @Test - public void stateChangeEvent_toNonIdle_createsInitialPlaybackStats() throws Exception { + public void stateChangeEvent_toEndedWithEmptyTimeline_doesNotCreateInitialPlaybackStats() + throws Exception { + PlaybackStatsListener.Callback callback = mock(PlaybackStatsListener.Callback.class); PlaybackStatsListener playbackStatsListener = - new PlaybackStatsListener(/* keepHistory= */ true, /* callback= */ null); + new PlaybackStatsListener(/* keepHistory= */ true, callback); player.addAnalyticsListener(playbackStatsListener); player.prepare(); runUntilPendingCommandsAreFullyHandled(player); - assertThat(playbackStatsListener.getPlaybackStats()).isNotNull(); + assertThat(playbackStatsListener.getPlaybackStats()).isNull(); + verifyNoMoreInteractions(callback); } @Test