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 7d4d6087dc..2cc8456e12 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,10 +105,6 @@ 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 @@ -238,11 +234,6 @@ 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 9465bd2387..8b9967addb 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(events); + maybeAddSessions(player, events); for (String session : playbackStatsTrackers.keySet()) { Pair eventTimeAndBelongsToPlayback = findBestEventTime(events, session); PlaybackStatsTracker tracker = playbackStatsTrackers.get(session); @@ -281,15 +281,17 @@ public final class PlaybackStatsListener } } - private void maybeAddSessions(Events events) { + private void maybeAddSessions(Player player, Events events) { + boolean isCompletelyIdle = + player.getCurrentTimeline().isEmpty() && player.getPlaybackState() == Player.STATE_IDLE; 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 (event == EVENT_POSITION_DISCONTINUITY) { + } else if (!isCompletelyIdle && event == EVENT_POSITION_DISCONTINUITY) { sessionManager.updateSessionsWithDiscontinuity(eventTime, discontinuityReason); - } else { + } else if (!isCompletelyIdle) { 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 930a835cec..72998db0e0 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,16 +59,6 @@ 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(); @@ -496,6 +486,8 @@ 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); @@ -504,10 +496,11 @@ public final class DefaultPlaybackSessionManagerTest { timeline.getUidOfPeriod(/* periodIndex= */ 0), /* windowSequenceNumber= */ 0); EventTime eventTimeWithMediaPeriodId = createEventTime(timeline, /* windowIndex= */ 0, mediaPeriodId); - sessionManager.updateSessions(eventTimeWithTimeline); + sessionManager.updateSessions(eventTime); ArgumentCaptor sessionId = ArgumentCaptor.forClass(String.class); - verify(mockListener).onSessionCreated(eq(eventTimeWithTimeline), sessionId.capture()); + verify(mockListener).onSessionCreated(eq(eventTime), sessionId.capture()); + assertThat(sessionManager.belongsToSession(eventTime, sessionId.getValue())).isTrue(); assertThat(sessionManager.belongsToSession(eventTimeWithTimeline, sessionId.getValue())) .isTrue(); assertThat(sessionManager.belongsToSession(eventTimeWithMediaPeriodId, sessionId.getValue())) @@ -516,11 +509,11 @@ public final class DefaultPlaybackSessionManagerTest { @Test public void belongsToSession_withOtherWindowIndex_returnsFalse() { - Timeline timeline = new FakeTimeline(/* windowCount= */ 2); EventTime eventTime = - createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId= */ null); + createEventTime(Timeline.EMPTY, /* windowIndex= */ 0, /* mediaPeriodId= */ null); EventTime eventTimeOtherWindow = - createEventTime(timeline, /* windowIndex= */ 1, /* mediaPeriodId= */ null); + createEventTime(Timeline.EMPTY, /* windowIndex= */ 1, /* mediaPeriodId= */ null); + Timeline timeline = new FakeTimeline(/* windowCount= */ 2); MediaPeriodId mediaPeriodId = new MediaPeriodId( timeline.getUidOfPeriod(/* periodIndex= */ 1), /* windowSequenceNumber= */ 1); @@ -592,18 +585,18 @@ public final class DefaultPlaybackSessionManagerTest { } @Test - public void timelineUpdate_toEmpty_finishesAllSessionsAndDoesNotCreateNewSessions() { - Timeline timeline = new FakeTimeline(/* windowCount= */ 2); + public void initialTimelineUpdate_finishesAllSessionsOutsideTimeline() { EventTime eventTime1 = - createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId= */ null); + createEventTime(Timeline.EMPTY, /* windowIndex= */ 0, /* mediaPeriodId= */ null); EventTime eventTime2 = - createEventTime(timeline, /* windowIndex= */ 1, /* mediaPeriodId= */ null); + createEventTime(Timeline.EMPTY, /* windowIndex= */ 1, /* mediaPeriodId= */ null); sessionManager.updateSessions(eventTime1); sessionManager.updateSessions(eventTime2); + Timeline timeline = new FakeTimeline(); + EventTime newTimelineEventTime = + createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId= */ null); - EventTime eventTimeWithEmptyTimeline = - createEventTime(Timeline.EMPTY, /* windowIndex= */ 0, /* mediaPeriodId= */ null); - sessionManager.updateSessionsWithTimelineChange(eventTimeWithEmptyTimeline); + sessionManager.updateSessionsWithTimelineChange(newTimelineEventTime); ArgumentCaptor sessionId1 = ArgumentCaptor.forClass(String.class); ArgumentCaptor sessionId2 = ArgumentCaptor.forClass(String.class); @@ -612,19 +605,14 @@ public final class DefaultPlaybackSessionManagerTest { verify(mockListener).onSessionActive(eventTime1, sessionId1.getValue()); verify(mockListener) .onSessionFinished( - eventTimeWithEmptyTimeline, - sessionId1.getValue(), - /* automaticTransitionToNextPlayback= */ false); - verify(mockListener) - .onSessionFinished( - eventTimeWithEmptyTimeline, + newTimelineEventTime, sessionId2.getValue(), /* automaticTransitionToNextPlayback= */ false); verifyNoMoreInteractions(mockListener); } @Test - public void timelineUpdate_resolvesWindowIndices() { + public void dynamicTimelineUpdate_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 668ed5f556..0fa1a4a85b 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,7 +22,6 @@ 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; @@ -78,18 +77,15 @@ public final class PlaybackStatsListenerTest { } @Test - public void stateChangeEvent_toEndedWithEmptyTimeline_doesNotCreateInitialPlaybackStats() - throws Exception { - PlaybackStatsListener.Callback callback = mock(PlaybackStatsListener.Callback.class); + public void stateChangeEvent_toNonIdle_createsInitialPlaybackStats() throws Exception { PlaybackStatsListener playbackStatsListener = - new PlaybackStatsListener(/* keepHistory= */ true, callback); + new PlaybackStatsListener(/* keepHistory= */ true, /* callback= */ null); player.addAnalyticsListener(playbackStatsListener); player.prepare(); runUntilPendingCommandsAreFullyHandled(player); - assertThat(playbackStatsListener.getPlaybackStats()).isNull(); - verifyNoMoreInteractions(callback); + assertThat(playbackStatsListener.getPlaybackStats()).isNotNull(); } @Test