diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 2757b9d681..d558de8583 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -28,6 +28,8 @@ * Fix seeking issues in AC4 streams caused by not identifying decode-only samples correctly ([#11000](https://github.com/google/ExoPlayer/issues/11000)). + * Fix issue in `PlaybackStatsListener` where spurious `PlaybackStats` are + created after the playlist is cleared. * Transformer: * Parse EXIF rotation data for image inputs. * Track Selection: diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/DefaultPlaybackSessionManager.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/DefaultPlaybackSessionManager.java index b0af78bf61..cc4b7792c5 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/DefaultPlaybackSessionManager.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/DefaultPlaybackSessionManager.java @@ -15,6 +15,7 @@ */ package androidx.media3.exoplayer.analytics; +import static androidx.media3.common.util.Assertions.checkNotNull; import static java.lang.Math.max; import android.util.Base64; @@ -23,7 +24,6 @@ import androidx.media3.common.C; import androidx.media3.common.Player; import androidx.media3.common.Player.DiscontinuityReason; import androidx.media3.common.Timeline; -import androidx.media3.common.util.Assertions; import androidx.media3.common.util.UnstableApi; import androidx.media3.common.util.Util; import androidx.media3.exoplayer.analytics.AnalyticsListener.EventTime; @@ -59,6 +59,7 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag private @MonotonicNonNull Listener listener; private Timeline currentTimeline; @Nullable private String currentSessionId; + private long lastRemovedCurrentWindowSequenceNumber; /** * Creates session manager with a {@link #DEFAULT_SESSION_ID_GENERATOR} to generate session ids. @@ -79,6 +80,7 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag period = new Timeline.Period(); sessions = new HashMap<>(); currentTimeline = Timeline.EMPTY; + lastRemovedCurrentWindowSequenceNumber = -1; } @Override @@ -105,22 +107,22 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag @Override public synchronized void updateSessions(EventTime eventTime) { - Assertions.checkNotNull(listener); + 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 - // the current window if it's the first created media period, or a window that will be played - // in the future. Otherwise, we know that it belongs to a session that was already finished - // and we can ignore the event. - boolean isAlreadyFinished = - currentSession.windowSequenceNumber == C.INDEX_UNSET - ? currentSession.windowIndex != eventTime.windowIndex - : eventTime.mediaPeriodId.windowSequenceNumber < currentSession.windowSequenceNumber; - if (isAlreadyFinished) { + if (eventTime.mediaPeriodId != null) { + if (eventTime.mediaPeriodId.windowSequenceNumber < getMinWindowSequenceNumber()) { + // Ignore event because it is part of a past window that has already been finished. + return; + } + @Nullable SessionDescriptor currentSession = sessions.get(currentSessionId); + if (currentSession != null + && currentSession.windowSequenceNumber == C.INDEX_UNSET + && currentSession.windowIndex != eventTime.windowIndex) { + // Ignore events for anything other than the current window before the first media period + // has been created. return; } } @@ -173,7 +175,7 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag @Override public synchronized void updateSessionsWithTimelineChange(EventTime eventTime) { - Assertions.checkNotNull(listener); + checkNotNull(listener); Timeline previousTimeline = currentTimeline; currentTimeline = eventTime.timeline; Iterator iterator = sessions.values().iterator(); @@ -184,7 +186,7 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag iterator.remove(); if (session.isCreated) { if (session.sessionId.equals(currentSessionId)) { - currentSessionId = null; + clearCurrentSession(session); } listener.onSessionFinished( eventTime, session.sessionId, /* automaticTransitionToNextPlayback= */ false); @@ -197,7 +199,7 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag @Override public synchronized void updateSessionsWithDiscontinuity( EventTime eventTime, @DiscontinuityReason int reason) { - Assertions.checkNotNull(listener); + checkNotNull(listener); boolean hasAutomaticTransition = reason == Player.DISCONTINUITY_REASON_AUTO_TRANSITION; Iterator iterator = sessions.values().iterator(); while (iterator.hasNext()) { @@ -209,7 +211,7 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag boolean isAutomaticTransition = hasAutomaticTransition && isRemovingCurrentSession && session.isActive; if (isRemovingCurrentSession) { - currentSessionId = null; + clearCurrentSession(session); } listener.onSessionFinished(eventTime, session.sessionId, isAutomaticTransition); } @@ -226,7 +228,9 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag @Override public synchronized void finishAllSessions(EventTime eventTime) { - currentSessionId = null; + if (currentSessionId != null) { + clearCurrentSession(checkNotNull(sessions.get(currentSessionId))); + } Iterator iterator = sessions.values().iterator(); while (iterator.hasNext()) { SessionDescriptor session = iterator.next(); @@ -242,7 +246,9 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag private void updateCurrentSession(EventTime eventTime) { if (eventTime.timeline.isEmpty()) { // Clear current session if the Timeline is empty. - currentSessionId = null; + if (currentSessionId != null) { + clearCurrentSession(checkNotNull(sessions.get(currentSessionId))); + } return; } @Nullable SessionDescriptor previousSessionDescriptor = sessions.get(currentSessionId); @@ -271,6 +277,20 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag } } + private void clearCurrentSession(SessionDescriptor currentSession) { + if (currentSession.windowSequenceNumber != C.INDEX_UNSET) { + lastRemovedCurrentWindowSequenceNumber = currentSession.windowSequenceNumber; + } + currentSessionId = null; + } + + private long getMinWindowSequenceNumber() { + @Nullable SessionDescriptor currentSession = sessions.get(currentSessionId); + return currentSession != null && currentSession.windowSequenceNumber != C.INDEX_UNSET + ? currentSession.windowSequenceNumber + : lastRemovedCurrentWindowSequenceNumber + 1; + } + private SessionDescriptor getOrAddSession( int windowIndex, @Nullable MediaPeriodId mediaPeriodId) { // There should only be one matching session if mediaPeriodId is non-null. If mediaPeriodId is @@ -375,7 +395,8 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag int eventWindowIndex, @Nullable MediaPeriodId eventMediaPeriodId) { if (windowSequenceNumber == C.INDEX_UNSET && eventWindowIndex == windowIndex - && eventMediaPeriodId != null) { + && eventMediaPeriodId != null + && eventMediaPeriodId.windowSequenceNumber >= getMinWindowSequenceNumber()) { // Set window sequence number for this session as soon as we have one. windowSequenceNumber = eventMediaPeriodId.windowSequenceNumber; } diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/analytics/DefaultPlaybackSessionManagerTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/analytics/DefaultPlaybackSessionManagerTest.java index 5c6d0d16c5..ba045002f7 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/analytics/DefaultPlaybackSessionManagerTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/analytics/DefaultPlaybackSessionManagerTest.java @@ -482,6 +482,110 @@ public final class DefaultPlaybackSessionManagerTest { verify(mockListener, never()).onSessionActive(any(), eq(adSessionId2)); } + @Test + public void + updateSession_withNewMediaPeriodIdOfSameWindowAfterTimelineUpdateToEmpty_createsNewSession() { + Timeline timeline = new FakeTimeline(/* windowCount= */ 2); + EventTime eventTimeBeforeClear = + createEventTime( + timeline, + /* windowIndex= */ 0, + new MediaPeriodId( + timeline.getUidOfPeriod(/* periodIndex= */ 0), /* windowSequenceNumber= */ 0)); + sessionManager.updateSessions(eventTimeBeforeClear); + EventTime eventTimeWithEmptyTimeline = + createEventTime(Timeline.EMPTY, /* windowIndex= */ 0, /* mediaPeriodId= */ null); + sessionManager.updateSessionsWithTimelineChange(eventTimeWithEmptyTimeline); + EventTime newEventTime = + createEventTime( + timeline, + /* windowIndex= */ 0, + new MediaPeriodId( + timeline.getUidOfPeriod(/* periodIndex= */ 0), /* windowSequenceNumber= */ 1)); + + sessionManager.updateSessions(newEventTime); + + ArgumentCaptor sessionId1 = ArgumentCaptor.forClass(String.class); + ArgumentCaptor sessionId2 = ArgumentCaptor.forClass(String.class); + verify(mockListener).onSessionCreated(eq(eventTimeBeforeClear), sessionId1.capture()); + verify(mockListener).onSessionActive(eventTimeBeforeClear, sessionId1.getValue()); + verify(mockListener) + .onSessionFinished( + eventTimeWithEmptyTimeline, + sessionId1.getValue(), + /* automaticTransitionToNextPlayback= */ false); + verify(mockListener).onSessionCreated(eq(newEventTime), sessionId2.capture()); + verify(mockListener).onSessionActive(newEventTime, sessionId2.getValue()); + verifyNoMoreInteractions(mockListener); + } + + @Test + public void + updateSession_withOldMediaPeriodIdOfSameWindowAfterTimelineUpdateToEmpty_doesNotCreateNewSession() { + Timeline timeline = new FakeTimeline(/* windowCount= */ 2); + EventTime eventTimeBeforeClear = + createEventTime( + timeline, + /* windowIndex= */ 0, + new MediaPeriodId( + timeline.getUidOfPeriod(/* periodIndex= */ 0), /* windowSequenceNumber= */ 0)); + sessionManager.updateSessions(eventTimeBeforeClear); + EventTime eventTimeWithEmptyTimeline = + createEventTime(Timeline.EMPTY, /* windowIndex= */ 0, /* mediaPeriodId= */ null); + sessionManager.updateSessionsWithTimelineChange(eventTimeWithEmptyTimeline); + EventTime newEventTime = + createEventTime( + timeline, + /* windowIndex= */ 0, + new MediaPeriodId( + timeline.getUidOfPeriod(/* periodIndex= */ 0), /* windowSequenceNumber= */ 0)); + + sessionManager.updateSessions(newEventTime); + + ArgumentCaptor sessionId = ArgumentCaptor.forClass(String.class); + verify(mockListener).onSessionCreated(eq(eventTimeBeforeClear), sessionId.capture()); + verify(mockListener).onSessionActive(eventTimeBeforeClear, sessionId.getValue()); + verify(mockListener) + .onSessionFinished( + eventTimeWithEmptyTimeline, + sessionId.getValue(), + /* automaticTransitionToNextPlayback= */ false); + verifyNoMoreInteractions(mockListener); + } + + @Test + public void + updateSession_ofSameWindowWithoutMediaPeriodIdAfterTimelineUpdateToEmpty_createsNewSession() { + Timeline timeline = new FakeTimeline(/* windowCount= */ 2); + EventTime eventTimeBeforeClear = + createEventTime( + timeline, + /* windowIndex= */ 0, + new MediaPeriodId( + timeline.getUidOfPeriod(/* periodIndex= */ 0), /* windowSequenceNumber= */ 0)); + sessionManager.updateSessions(eventTimeBeforeClear); + EventTime eventTimeWithEmptyTimeline = + createEventTime(Timeline.EMPTY, /* windowIndex= */ 0, /* mediaPeriodId= */ null); + sessionManager.updateSessionsWithTimelineChange(eventTimeWithEmptyTimeline); + EventTime newEventTime = + createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId= */ null); + + sessionManager.updateSessions(newEventTime); + + ArgumentCaptor sessionId1 = ArgumentCaptor.forClass(String.class); + ArgumentCaptor sessionId2 = ArgumentCaptor.forClass(String.class); + verify(mockListener).onSessionCreated(eq(eventTimeBeforeClear), sessionId1.capture()); + verify(mockListener).onSessionActive(eventTimeBeforeClear, sessionId1.getValue()); + verify(mockListener) + .onSessionFinished( + eventTimeWithEmptyTimeline, + sessionId1.getValue(), + /* automaticTransitionToNextPlayback= */ false); + verify(mockListener).onSessionCreated(eq(newEventTime), sessionId2.capture()); + verify(mockListener).onSessionActive(newEventTime, sessionId2.getValue()); + verifyNoMoreInteractions(mockListener); + } + @Test public void getSessionForMediaPeriodId_returnsValue_butDoesNotCreateSession() { Timeline timeline = new FakeTimeline();