Fix spurious sessions created for events after the playlist is cleared
Some events may arrive after the playlist is cleared (e.g. load cancellation). In this case, the DefaultPlaybackSessionManager may create a new session for the already removed item. We already have checks in place that ignore events with old windowSequenceNumbers, but these checks only work if the current session is set (i.e. the playlist is non-empty). The fix is to add the same check for empty playlists by keeping note of the last removed window sequence number. PiperOrigin-RevId: 541870812
This commit is contained in:
parent
71f73229dd
commit
e0191ddded
@ -28,6 +28,8 @@
|
|||||||
* Fix seeking issues in AC4 streams caused by not identifying decode-only
|
* Fix seeking issues in AC4 streams caused by not identifying decode-only
|
||||||
samples correctly
|
samples correctly
|
||||||
([#11000](https://github.com/google/ExoPlayer/issues/11000)).
|
([#11000](https://github.com/google/ExoPlayer/issues/11000)).
|
||||||
|
* Fix issue in `PlaybackStatsListener` where spurious `PlaybackStats` are
|
||||||
|
created after the playlist is cleared.
|
||||||
* Transformer:
|
* Transformer:
|
||||||
* Parse EXIF rotation data for image inputs.
|
* Parse EXIF rotation data for image inputs.
|
||||||
* Track Selection:
|
* Track Selection:
|
||||||
|
@ -15,6 +15,7 @@
|
|||||||
*/
|
*/
|
||||||
package androidx.media3.exoplayer.analytics;
|
package androidx.media3.exoplayer.analytics;
|
||||||
|
|
||||||
|
import static androidx.media3.common.util.Assertions.checkNotNull;
|
||||||
import static java.lang.Math.max;
|
import static java.lang.Math.max;
|
||||||
|
|
||||||
import android.util.Base64;
|
import android.util.Base64;
|
||||||
@ -23,7 +24,6 @@ import androidx.media3.common.C;
|
|||||||
import androidx.media3.common.Player;
|
import androidx.media3.common.Player;
|
||||||
import androidx.media3.common.Player.DiscontinuityReason;
|
import androidx.media3.common.Player.DiscontinuityReason;
|
||||||
import androidx.media3.common.Timeline;
|
import androidx.media3.common.Timeline;
|
||||||
import androidx.media3.common.util.Assertions;
|
|
||||||
import androidx.media3.common.util.UnstableApi;
|
import androidx.media3.common.util.UnstableApi;
|
||||||
import androidx.media3.common.util.Util;
|
import androidx.media3.common.util.Util;
|
||||||
import androidx.media3.exoplayer.analytics.AnalyticsListener.EventTime;
|
import androidx.media3.exoplayer.analytics.AnalyticsListener.EventTime;
|
||||||
@ -59,6 +59,7 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
|
|||||||
private @MonotonicNonNull Listener listener;
|
private @MonotonicNonNull Listener listener;
|
||||||
private Timeline currentTimeline;
|
private Timeline currentTimeline;
|
||||||
@Nullable private String currentSessionId;
|
@Nullable private String currentSessionId;
|
||||||
|
private long lastRemovedCurrentWindowSequenceNumber;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Creates session manager with a {@link #DEFAULT_SESSION_ID_GENERATOR} to generate session ids.
|
* 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();
|
period = new Timeline.Period();
|
||||||
sessions = new HashMap<>();
|
sessions = new HashMap<>();
|
||||||
currentTimeline = Timeline.EMPTY;
|
currentTimeline = Timeline.EMPTY;
|
||||||
|
lastRemovedCurrentWindowSequenceNumber = -1;
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
@ -105,22 +107,22 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
|
|||||||
|
|
||||||
@Override
|
@Override
|
||||||
public synchronized void updateSessions(EventTime eventTime) {
|
public synchronized void updateSessions(EventTime eventTime) {
|
||||||
Assertions.checkNotNull(listener);
|
checkNotNull(listener);
|
||||||
if (eventTime.timeline.isEmpty()) {
|
if (eventTime.timeline.isEmpty()) {
|
||||||
// Don't try to create new sessions for empty timelines.
|
// Don't try to create new sessions for empty timelines.
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@Nullable SessionDescriptor currentSession = sessions.get(currentSessionId);
|
if (eventTime.mediaPeriodId != null) {
|
||||||
if (eventTime.mediaPeriodId != null && currentSession != null) {
|
if (eventTime.mediaPeriodId.windowSequenceNumber < getMinWindowSequenceNumber()) {
|
||||||
// If we receive an event associated with a media period, then it needs to be either part of
|
// Ignore event because it is part of a past window that has already been finished.
|
||||||
// the current window if it's the first created media period, or a window that will be played
|
return;
|
||||||
// in the future. Otherwise, we know that it belongs to a session that was already finished
|
}
|
||||||
// and we can ignore the event.
|
@Nullable SessionDescriptor currentSession = sessions.get(currentSessionId);
|
||||||
boolean isAlreadyFinished =
|
if (currentSession != null
|
||||||
currentSession.windowSequenceNumber == C.INDEX_UNSET
|
&& currentSession.windowSequenceNumber == C.INDEX_UNSET
|
||||||
? currentSession.windowIndex != eventTime.windowIndex
|
&& currentSession.windowIndex != eventTime.windowIndex) {
|
||||||
: eventTime.mediaPeriodId.windowSequenceNumber < currentSession.windowSequenceNumber;
|
// Ignore events for anything other than the current window before the first media period
|
||||||
if (isAlreadyFinished) {
|
// has been created.
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -173,7 +175,7 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
|
|||||||
|
|
||||||
@Override
|
@Override
|
||||||
public synchronized void updateSessionsWithTimelineChange(EventTime eventTime) {
|
public synchronized void updateSessionsWithTimelineChange(EventTime eventTime) {
|
||||||
Assertions.checkNotNull(listener);
|
checkNotNull(listener);
|
||||||
Timeline previousTimeline = currentTimeline;
|
Timeline previousTimeline = currentTimeline;
|
||||||
currentTimeline = eventTime.timeline;
|
currentTimeline = eventTime.timeline;
|
||||||
Iterator<SessionDescriptor> iterator = sessions.values().iterator();
|
Iterator<SessionDescriptor> iterator = sessions.values().iterator();
|
||||||
@ -184,7 +186,7 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
|
|||||||
iterator.remove();
|
iterator.remove();
|
||||||
if (session.isCreated) {
|
if (session.isCreated) {
|
||||||
if (session.sessionId.equals(currentSessionId)) {
|
if (session.sessionId.equals(currentSessionId)) {
|
||||||
currentSessionId = null;
|
clearCurrentSession(session);
|
||||||
}
|
}
|
||||||
listener.onSessionFinished(
|
listener.onSessionFinished(
|
||||||
eventTime, session.sessionId, /* automaticTransitionToNextPlayback= */ false);
|
eventTime, session.sessionId, /* automaticTransitionToNextPlayback= */ false);
|
||||||
@ -197,7 +199,7 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
|
|||||||
@Override
|
@Override
|
||||||
public synchronized void updateSessionsWithDiscontinuity(
|
public synchronized void updateSessionsWithDiscontinuity(
|
||||||
EventTime eventTime, @DiscontinuityReason int reason) {
|
EventTime eventTime, @DiscontinuityReason int reason) {
|
||||||
Assertions.checkNotNull(listener);
|
checkNotNull(listener);
|
||||||
boolean hasAutomaticTransition = reason == Player.DISCONTINUITY_REASON_AUTO_TRANSITION;
|
boolean hasAutomaticTransition = reason == Player.DISCONTINUITY_REASON_AUTO_TRANSITION;
|
||||||
Iterator<SessionDescriptor> iterator = sessions.values().iterator();
|
Iterator<SessionDescriptor> iterator = sessions.values().iterator();
|
||||||
while (iterator.hasNext()) {
|
while (iterator.hasNext()) {
|
||||||
@ -209,7 +211,7 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
|
|||||||
boolean isAutomaticTransition =
|
boolean isAutomaticTransition =
|
||||||
hasAutomaticTransition && isRemovingCurrentSession && session.isActive;
|
hasAutomaticTransition && isRemovingCurrentSession && session.isActive;
|
||||||
if (isRemovingCurrentSession) {
|
if (isRemovingCurrentSession) {
|
||||||
currentSessionId = null;
|
clearCurrentSession(session);
|
||||||
}
|
}
|
||||||
listener.onSessionFinished(eventTime, session.sessionId, isAutomaticTransition);
|
listener.onSessionFinished(eventTime, session.sessionId, isAutomaticTransition);
|
||||||
}
|
}
|
||||||
@ -226,7 +228,9 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
|
|||||||
|
|
||||||
@Override
|
@Override
|
||||||
public synchronized void finishAllSessions(EventTime eventTime) {
|
public synchronized void finishAllSessions(EventTime eventTime) {
|
||||||
currentSessionId = null;
|
if (currentSessionId != null) {
|
||||||
|
clearCurrentSession(checkNotNull(sessions.get(currentSessionId)));
|
||||||
|
}
|
||||||
Iterator<SessionDescriptor> iterator = sessions.values().iterator();
|
Iterator<SessionDescriptor> iterator = sessions.values().iterator();
|
||||||
while (iterator.hasNext()) {
|
while (iterator.hasNext()) {
|
||||||
SessionDescriptor session = iterator.next();
|
SessionDescriptor session = iterator.next();
|
||||||
@ -242,7 +246,9 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
|
|||||||
private void updateCurrentSession(EventTime eventTime) {
|
private void updateCurrentSession(EventTime eventTime) {
|
||||||
if (eventTime.timeline.isEmpty()) {
|
if (eventTime.timeline.isEmpty()) {
|
||||||
// Clear current session if the Timeline is empty.
|
// Clear current session if the Timeline is empty.
|
||||||
currentSessionId = null;
|
if (currentSessionId != null) {
|
||||||
|
clearCurrentSession(checkNotNull(sessions.get(currentSessionId)));
|
||||||
|
}
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@Nullable SessionDescriptor previousSessionDescriptor = sessions.get(currentSessionId);
|
@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(
|
private SessionDescriptor getOrAddSession(
|
||||||
int windowIndex, @Nullable MediaPeriodId mediaPeriodId) {
|
int windowIndex, @Nullable MediaPeriodId mediaPeriodId) {
|
||||||
// There should only be one matching session if mediaPeriodId is non-null. If mediaPeriodId is
|
// 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) {
|
int eventWindowIndex, @Nullable MediaPeriodId eventMediaPeriodId) {
|
||||||
if (windowSequenceNumber == C.INDEX_UNSET
|
if (windowSequenceNumber == C.INDEX_UNSET
|
||||||
&& eventWindowIndex == windowIndex
|
&& eventWindowIndex == windowIndex
|
||||||
&& eventMediaPeriodId != null) {
|
&& eventMediaPeriodId != null
|
||||||
|
&& eventMediaPeriodId.windowSequenceNumber >= getMinWindowSequenceNumber()) {
|
||||||
// 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;
|
||||||
}
|
}
|
||||||
|
@ -482,6 +482,110 @@ public final class DefaultPlaybackSessionManagerTest {
|
|||||||
verify(mockListener, never()).onSessionActive(any(), eq(adSessionId2));
|
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<String> sessionId1 = ArgumentCaptor.forClass(String.class);
|
||||||
|
ArgumentCaptor<String> 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<String> 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<String> sessionId1 = ArgumentCaptor.forClass(String.class);
|
||||||
|
ArgumentCaptor<String> 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
|
@Test
|
||||||
public void getSessionForMediaPeriodId_returnsValue_butDoesNotCreateSession() {
|
public void getSessionForMediaPeriodId_returnsValue_butDoesNotCreateSession() {
|
||||||
Timeline timeline = new FakeTimeline();
|
Timeline timeline = new FakeTimeline();
|
||||||
|
Loading…
x
Reference in New Issue
Block a user