Skip to content

SDK / exoplayer

Go to a project
Commit c0ee312a by tonihei Committed by Oliver Woodman
Options

Simplify keeping track of current id in DefaultPlaybackSessionManager 

We currently have a currentMediaPeriodId and an activeSessionId that are more
or less tracking the same thing unless the current media period isn't "active" yet.

Simplify this logic by using a single currentSessionId field and the respective
isActive flag of this session. Also move all session creation and activation code in
the same method to make it easier to reason about the code.

This change also fixes a subtle bug where events after a seek to a new window
are not ignored as they should.

PiperOrigin-RevId: 289432181
parent ad7bcab4
Inline Side-by-side
Showing with 61 additions and 45 deletions
library/core/src/main/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManager.java
...@@ -29,7 +29,6 @@ import java.util.HashMap; ...@@ -29,7 +29,6 @@ import java.util.HashMap;
import java.util.Iterator; import java.util.Iterator;
import java.util.Random; import java.util.Random;
import org.checkerframework.checker.nullness.qual.MonotonicNonNull; import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
import org.checkerframework.checker.nullness.qual.RequiresNonNull;
/** /**
* Default {@link PlaybackSessionManager} which instantiates a new session for each window in the * Default {@link PlaybackSessionManager} which instantiates a new session for each window in the
...@@ -48,8 +47,7 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag ...@@ -48,8 +47,7 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
private @MonotonicNonNull Listener listener; private @MonotonicNonNull Listener listener;
private Timeline currentTimeline; private Timeline currentTimeline;
@Nullable private MediaPeriodId currentMediaPeriodId; @Nullable private String currentSessionId;
@Nullable private String activeSessionId;
/** Creates session manager. */ /** Creates session manager. */
public DefaultPlaybackSessionManager() { public DefaultPlaybackSessionManager() {
...@@ -83,22 +81,34 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag ...@@ -83,22 +81,34 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
@Override @Override
public synchronized void updateSessions(EventTime eventTime) { public synchronized void updateSessions(EventTime eventTime) {
boolean isObviouslyFinished = Assertions.checkNotNull(listener);
eventTime.mediaPeriodId != null @Nullable SessionDescriptor currentSession = sessions.get(currentSessionId);
&& currentMediaPeriodId != null if (eventTime.mediaPeriodId != null && currentSession != null) {
&& eventTime.mediaPeriodId.windowSequenceNumber // If we receive an event associated with a media period, then it needs to be either part of
< currentMediaPeriodId.windowSequenceNumber; // the current window if it's the first created media period, or a window that will be played
if (!isObviouslyFinished) { // in the future. Otherwise, we know that it belongs to a session that was already finished
SessionDescriptor descriptor = // and we can ignore the event.
getOrAddSession(eventTime.windowIndex, eventTime.mediaPeriodId); boolean isAlreadyFinished =
if (!descriptor.isCreated) { currentSession.windowSequenceNumber == C.INDEX_UNSET
descriptor.isCreated = true; ? currentSession.windowIndex != eventTime.windowIndex
Assertions.checkNotNull(listener).onSessionCreated(eventTime, descriptor.sessionId); : eventTime.mediaPeriodId.windowSequenceNumber < currentSession.windowSequenceNumber;
if (activeSessionId == null) { if (isAlreadyFinished) {
updateActiveSession(eventTime, descriptor); return;
}
} }
} }
SessionDescriptor eventSession =
getOrAddSession(eventTime.windowIndex, eventTime.mediaPeriodId);
if (currentSessionId == null) {
currentSessionId = eventSession.sessionId;
}
if (!eventSession.isCreated) {
eventSession.isCreated = true;
listener.onSessionCreated(eventTime, eventSession.sessionId);
}
if (eventSession.sessionId.equals(currentSessionId) && !eventSession.isActive) {
eventSession.isActive = true;
listener.onSessionActive(eventTime, eventSession.sessionId);
}
} }
@Override @Override
...@@ -112,8 +122,8 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag ...@@ -112,8 +122,8 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
if (!session.tryResolvingToNewTimeline(previousTimeline, currentTimeline)) { if (!session.tryResolvingToNewTimeline(previousTimeline, currentTimeline)) {
iterator.remove(); iterator.remove();
if (session.isCreated) { if (session.isCreated) {
if (session.sessionId.equals(activeSessionId)) { if (session.sessionId.equals(currentSessionId)) {
activeSessionId = null; currentSessionId = null;
} }
listener.onSessionFinished( listener.onSessionFinished(
eventTime, session.sessionId, /* automaticTransitionToNextPlayback= */ false); eventTime, session.sessionId, /* automaticTransitionToNextPlayback= */ false);
...@@ -136,36 +146,41 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag ...@@ -136,36 +146,41 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
if (session.isFinishedAtEventTime(eventTime)) { if (session.isFinishedAtEventTime(eventTime)) {
iterator.remove(); iterator.remove();
if (session.isCreated) { if (session.isCreated) {
boolean isRemovingActiveSession = session.sessionId.equals(activeSessionId); boolean isRemovingCurrentSession = session.sessionId.equals(currentSessionId);
boolean isAutomaticTransition = hasAutomaticTransition && isRemovingActiveSession; boolean isAutomaticTransition =
if (isRemovingActiveSession) { hasAutomaticTransition && isRemovingCurrentSession && session.isActive;
activeSessionId = null; if (isRemovingCurrentSession) {
currentSessionId = null;
} }
listener.onSessionFinished(eventTime, session.sessionId, isAutomaticTransition); listener.onSessionFinished(eventTime, session.sessionId, isAutomaticTransition);
} }
} }
} }
SessionDescriptor activeSessionDescriptor = @Nullable SessionDescriptor previousSessionDescriptor = sessions.get(currentSessionId);
SessionDescriptor currentSessionDescriptor =
getOrAddSession(eventTime.windowIndex, eventTime.mediaPeriodId); getOrAddSession(eventTime.windowIndex, eventTime.mediaPeriodId);
currentSessionId = currentSessionDescriptor.sessionId;
if (eventTime.mediaPeriodId != null if (eventTime.mediaPeriodId != null
&& eventTime.mediaPeriodId.isAd() && eventTime.mediaPeriodId.isAd()
&& (currentMediaPeriodId == null && (previousSessionDescriptor == null
|| currentMediaPeriodId.windowSequenceNumber || previousSessionDescriptor.windowSequenceNumber
!= eventTime.mediaPeriodId.windowSequenceNumber != eventTime.mediaPeriodId.windowSequenceNumber
|| currentMediaPeriodId.adGroupIndex != eventTime.mediaPeriodId.adGroupIndex || previousSessionDescriptor.adMediaPeriodId == null
|| currentMediaPeriodId.adIndexInAdGroup != eventTime.mediaPeriodId.adIndexInAdGroup)) { || previousSessionDescriptor.adMediaPeriodId.adGroupIndex
!= eventTime.mediaPeriodId.adGroupIndex
|| previousSessionDescriptor.adMediaPeriodId.adIndexInAdGroup
!= eventTime.mediaPeriodId.adIndexInAdGroup)) {
// New ad playback started. Find corresponding content session and notify ad playback started. // New ad playback started. Find corresponding content session and notify ad playback started.
MediaPeriodId contentMediaPeriodId = MediaPeriodId contentMediaPeriodId =
new MediaPeriodId( new MediaPeriodId(
eventTime.mediaPeriodId.periodUid, eventTime.mediaPeriodId.windowSequenceNumber); eventTime.mediaPeriodId.periodUid, eventTime.mediaPeriodId.windowSequenceNumber);
SessionDescriptor contentSession = SessionDescriptor contentSession =
getOrAddSession(eventTime.windowIndex, contentMediaPeriodId); getOrAddSession(eventTime.windowIndex, contentMediaPeriodId);
if (contentSession.isCreated && activeSessionDescriptor.isCreated) { if (contentSession.isCreated && currentSessionDescriptor.isCreated) {
listener.onAdPlaybackStarted( listener.onAdPlaybackStarted(
eventTime, contentSession.sessionId, activeSessionDescriptor.sessionId); eventTime, contentSession.sessionId, currentSessionDescriptor.sessionId);
} }
} }
updateActiveSession(eventTime, activeSessionDescriptor);
} }
private SessionDescriptor getOrAddSession( private SessionDescriptor getOrAddSession(
...@@ -199,18 +214,6 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag ...@@ -199,18 +214,6 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
return bestMatch; return bestMatch;
} }
@RequiresNonNull("listener")
private void updateActiveSession(EventTime eventTime, SessionDescriptor sessionDescriptor) {
currentMediaPeriodId = eventTime.mediaPeriodId;
if (sessionDescriptor.isCreated) {
activeSessionId = sessionDescriptor.sessionId;
if (!sessionDescriptor.isActive) {
sessionDescriptor.isActive = true;
listener.onSessionActive(eventTime, sessionDescriptor.sessionId);
}
}
}
private static String generateSessionId() { private static String generateSessionId() {
byte[] randomBytes = new byte[SESSION_ID_LENGTH]; byte[] randomBytes = new byte[SESSION_ID_LENGTH];
RANDOM.nextBytes(randomBytes); RANDOM.nextBytes(randomBytes);
...@@ -284,8 +287,7 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag ...@@ -284,8 +287,7 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
int eventWindowIndex, @Nullable MediaPeriodId eventMediaPeriodId) { int eventWindowIndex, @Nullable MediaPeriodId eventMediaPeriodId) {
if (windowSequenceNumber == C.INDEX_UNSET if (windowSequenceNumber == C.INDEX_UNSET
&& eventWindowIndex == windowIndex && eventWindowIndex == windowIndex
&& eventMediaPeriodId != null && eventMediaPeriodId != null) {
&& !eventMediaPeriodId.isAd()) {
// Set window sequence number for this session as soon as we have one. // Set window sequence number for this session as soon as we have one.
windowSequenceNumber = eventMediaPeriodId.windowSequenceNumber; windowSequenceNumber = eventMediaPeriodId.windowSequenceNumber;
} }
......
library/core/src/test/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManagerTest.java
...@@ -501,6 +501,7 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -501,6 +501,7 @@ public final class DefaultPlaybackSessionManagerTest {
createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId= */ null); createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId= */ null);
sessionManager.handleTimelineUpdate(newTimelineEventTime); sessionManager.handleTimelineUpdate(newTimelineEventTime);
sessionManager.updateSessions(newTimelineEventTime);
ArgumentCaptor<String> sessionId1 = ArgumentCaptor.forClass(String.class); ArgumentCaptor<String> sessionId1 = ArgumentCaptor.forClass(String.class);
ArgumentCaptor<String> sessionId2 = ArgumentCaptor.forClass(String.class); ArgumentCaptor<String> sessionId2 = ArgumentCaptor.forClass(String.class);
...@@ -657,6 +658,7 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -657,6 +658,7 @@ public final class DefaultPlaybackSessionManagerTest {
sessionManager.handlePositionDiscontinuity( sessionManager.handlePositionDiscontinuity(
eventTime2, Player.DISCONTINUITY_REASON_PERIOD_TRANSITION); eventTime2, Player.DISCONTINUITY_REASON_PERIOD_TRANSITION);
sessionManager.updateSessions(eventTime2);
verify(mockListener).onSessionCreated(eq(eventTime1), anyString()); verify(mockListener).onSessionCreated(eq(eventTime1), anyString());
verify(mockListener).onSessionActive(eq(eventTime1), anyString()); verify(mockListener).onSessionActive(eq(eventTime1), anyString());
...@@ -688,6 +690,7 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -688,6 +690,7 @@ public final class DefaultPlaybackSessionManagerTest {
sessionManager.handlePositionDiscontinuity( sessionManager.handlePositionDiscontinuity(
eventTime2, Player.DISCONTINUITY_REASON_PERIOD_TRANSITION); eventTime2, Player.DISCONTINUITY_REASON_PERIOD_TRANSITION);
sessionManager.updateSessions(eventTime2);
verify(mockListener).onSessionCreated(eventTime1, sessionId1); verify(mockListener).onSessionCreated(eventTime1, sessionId1);
verify(mockListener).onSessionActive(eventTime1, sessionId1); verify(mockListener).onSessionActive(eventTime1, sessionId1);
...@@ -722,6 +725,7 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -722,6 +725,7 @@ public final class DefaultPlaybackSessionManagerTest {
sessionManager.getSessionForMediaPeriodId(timeline, eventTime2.mediaPeriodId); sessionManager.getSessionForMediaPeriodId(timeline, eventTime2.mediaPeriodId);
sessionManager.handlePositionDiscontinuity(eventTime2, Player.DISCONTINUITY_REASON_SEEK); sessionManager.handlePositionDiscontinuity(eventTime2, Player.DISCONTINUITY_REASON_SEEK);
sessionManager.updateSessions(eventTime2);
verify(mockListener).onSessionCreated(eventTime1, sessionId1); verify(mockListener).onSessionCreated(eventTime1, sessionId1);
verify(mockListener).onSessionActive(eventTime1, sessionId1); verify(mockListener).onSessionActive(eventTime1, sessionId1);
...@@ -748,6 +752,7 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -748,6 +752,7 @@ public final class DefaultPlaybackSessionManagerTest {
sessionManager.updateSessions(eventTime2); sessionManager.updateSessions(eventTime2);
sessionManager.handlePositionDiscontinuity(eventTime2, Player.DISCONTINUITY_REASON_SEEK); sessionManager.handlePositionDiscontinuity(eventTime2, Player.DISCONTINUITY_REASON_SEEK);
sessionManager.updateSessions(eventTime2);
verify(mockListener, never()).onSessionFinished(any(), anyString(), anyBoolean()); verify(mockListener, never()).onSessionFinished(any(), anyString(), anyBoolean());
} }
...@@ -790,6 +795,7 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -790,6 +795,7 @@ public final class DefaultPlaybackSessionManagerTest {
sessionManager.getSessionForMediaPeriodId(timeline, eventTime2.mediaPeriodId); sessionManager.getSessionForMediaPeriodId(timeline, eventTime2.mediaPeriodId);
sessionManager.handlePositionDiscontinuity(eventTime3, Player.DISCONTINUITY_REASON_SEEK); sessionManager.handlePositionDiscontinuity(eventTime3, Player.DISCONTINUITY_REASON_SEEK);
sessionManager.updateSessions(eventTime3);
verify(mockListener).onSessionCreated(eventTime1, sessionId1); verify(mockListener).onSessionCreated(eventTime1, sessionId1);
verify(mockListener).onSessionActive(eventTime1, sessionId1); verify(mockListener).onSessionActive(eventTime1, sessionId1);
...@@ -851,6 +857,7 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -851,6 +857,7 @@ public final class DefaultPlaybackSessionManagerTest {
sessionManager.handlePositionDiscontinuity( sessionManager.handlePositionDiscontinuity(
contentEventTime, Player.DISCONTINUITY_REASON_AD_INSERTION); contentEventTime, Player.DISCONTINUITY_REASON_AD_INSERTION);
sessionManager.updateSessions(contentEventTime);
verify(mockListener).onSessionCreated(adEventTime1, adSessionId1); verify(mockListener).onSessionCreated(adEventTime1, adSessionId1);
verify(mockListener).onSessionActive(adEventTime1, adSessionId1); verify(mockListener).onSessionActive(adEventTime1, adSessionId1);
...@@ -858,6 +865,8 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -858,6 +865,8 @@ public final class DefaultPlaybackSessionManagerTest {
verify(mockListener) verify(mockListener)
.onSessionFinished( .onSessionFinished(
contentEventTime, adSessionId1, /* automaticTransitionToNextPlayback= */ true); contentEventTime, adSessionId1, /* automaticTransitionToNextPlayback= */ true);
verify(mockListener).onSessionCreated(eq(contentEventTime), anyString());
verify(mockListener).onSessionActive(eq(contentEventTime), anyString());
verifyNoMoreInteractions(mockListener); verifyNoMoreInteractions(mockListener);
} }
...@@ -908,6 +917,7 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -908,6 +917,7 @@ public final class DefaultPlaybackSessionManagerTest {
sessionManager.handlePositionDiscontinuity( sessionManager.handlePositionDiscontinuity(
adEventTime1, Player.DISCONTINUITY_REASON_AD_INSERTION); adEventTime1, Player.DISCONTINUITY_REASON_AD_INSERTION);
sessionManager.updateSessions(adEventTime1);
verify(mockListener, never()).onSessionFinished(any(), anyString(), anyBoolean()); verify(mockListener, never()).onSessionFinished(any(), anyString(), anyBoolean());
} }
...@@ -964,7 +974,9 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -964,7 +974,9 @@ public final class DefaultPlaybackSessionManagerTest {
sessionManager.handlePositionDiscontinuity( sessionManager.handlePositionDiscontinuity(
adEventTime1, Player.DISCONTINUITY_REASON_AD_INSERTION); adEventTime1, Player.DISCONTINUITY_REASON_AD_INSERTION);
sessionManager.updateSessions(adEventTime1);
sessionManager.handlePositionDiscontinuity(adEventTime2, Player.DISCONTINUITY_REASON_SEEK); sessionManager.handlePositionDiscontinuity(adEventTime2, Player.DISCONTINUITY_REASON_SEEK);
sessionManager.updateSessions(adEventTime2);
verify(mockListener).onSessionCreated(eq(contentEventTime), anyString()); verify(mockListener).onSessionCreated(eq(contentEventTime), anyString());
verify(mockListener).onSessionActive(eq(contentEventTime), anyString()); verify(mockListener).onSessionActive(eq(contentEventTime), anyString());
...@@ -1034,8 +1046,10 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -1034,8 +1046,10 @@ public final class DefaultPlaybackSessionManagerTest {
sessionManager.updateSessions(adEventTime1); sessionManager.updateSessions(adEventTime1);
sessionManager.handlePositionDiscontinuity( sessionManager.handlePositionDiscontinuity(
adEventTime1, Player.DISCONTINUITY_REASON_AD_INSERTION); adEventTime1, Player.DISCONTINUITY_REASON_AD_INSERTION);
sessionManager.updateSessions(adEventTime1);
sessionManager.handlePositionDiscontinuity( sessionManager.handlePositionDiscontinuity(
contentEventTime2, Player.DISCONTINUITY_REASON_AD_INSERTION); contentEventTime2, Player.DISCONTINUITY_REASON_AD_INSERTION);
sessionManager.updateSessions(contentEventTime2);
String adSessionId2 = String adSessionId2 =
sessionManager.getSessionForMediaPeriodId(adTimeline, adEventTime2.mediaPeriodId); sessionManager.getSessionForMediaPeriodId(adTimeline, adEventTime2.mediaPeriodId);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment