Skip to content

SDK / exoplayer

Go to a project
Commit 09096d6f by olly Committed by Oliver Woodman
Options

Rollback of https://github.com/google/ExoPlayer/commit/e60609e34447553474cdfc86b2dee5462316b10c 

*** Original commit ***

Prevent creation of new sessions if the Timeline is empty.

We currently create sessions based on the placeholder window
index. This shouldn't be needed as we now set a non-empty
timeline as soon as the first MediaItem is added to the playlist.

Once this check is part of the session manager, we can also remove
the equivalent workarounds from the various code integrations.

***

PiperOrigin-RevId: 369452067
parent 32c5ea06
Inline Side-by-side
Showing with 26 additions and 49 deletions
library/core/src/main/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManager.java
...@@ -105,10 +105,6 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag ...@@ -105,10 +105,6 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
@Override @Override
public synchronized void updateSessions(EventTime eventTime) { public synchronized void updateSessions(EventTime eventTime) {
Assertions.checkNotNull(listener); Assertions.checkNotNull(listener);
if (eventTime.timeline.isEmpty()) {
// Don't try to create new sessions for empty timelines.
return;
}
@Nullable SessionDescriptor currentSession = sessions.get(currentSessionId); @Nullable SessionDescriptor currentSession = sessions.get(currentSessionId);
if (eventTime.mediaPeriodId != null && currentSession != null) { if (eventTime.mediaPeriodId != null && currentSession != null) {
// If we receive an event associated with a media period, then it needs to be either part of // 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 ...@@ -238,11 +234,6 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag
@RequiresNonNull("listener") @RequiresNonNull("listener")
private void updateCurrentSession(EventTime eventTime) { 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); @Nullable SessionDescriptor previousSessionDescriptor = sessions.get(currentSessionId);
SessionDescriptor currentSessionDescriptor = SessionDescriptor currentSessionDescriptor =
getOrAddSession(eventTime.windowIndex, eventTime.mediaPeriodId); getOrAddSession(eventTime.windowIndex, eventTime.mediaPeriodId);
......
library/core/src/main/java/com/google/android/exoplayer2/analytics/PlaybackStatsListener.java
...@@ -240,7 +240,7 @@ public final class PlaybackStatsListener ...@@ -240,7 +240,7 @@ public final class PlaybackStatsListener
if (events.size() == 0) { if (events.size() == 0) {
return; return;
} }
maybeAddSessions(events); maybeAddSessions(player, events);
for (String session : playbackStatsTrackers.keySet()) { for (String session : playbackStatsTrackers.keySet()) {
Pair<EventTime, Boolean> eventTimeAndBelongsToPlayback = findBestEventTime(events, session); Pair<EventTime, Boolean> eventTimeAndBelongsToPlayback = findBestEventTime(events, session);
PlaybackStatsTracker tracker = playbackStatsTrackers.get(session); PlaybackStatsTracker tracker = playbackStatsTrackers.get(session);
...@@ -281,15 +281,17 @@ public final class PlaybackStatsListener ...@@ -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++) { for (int i = 0; i < events.size(); i++) {
@EventFlags int event = events.get(i); @EventFlags int event = events.get(i);
EventTime eventTime = events.getEventTime(event); EventTime eventTime = events.getEventTime(event);
if (event == EVENT_TIMELINE_CHANGED) { if (event == EVENT_TIMELINE_CHANGED) {
sessionManager.updateSessionsWithTimelineChange(eventTime); sessionManager.updateSessionsWithTimelineChange(eventTime);
} else if (event == EVENT_POSITION_DISCONTINUITY) { } else if (!isCompletelyIdle && event == EVENT_POSITION_DISCONTINUITY) {
sessionManager.updateSessionsWithDiscontinuity(eventTime, discontinuityReason); sessionManager.updateSessionsWithDiscontinuity(eventTime, discontinuityReason);
} else { } else if (!isCompletelyIdle) {
sessionManager.updateSessions(eventTime); sessionManager.updateSessions(eventTime);
} }
} }
......
library/core/src/test/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManagerTest.java
...@@ -60,16 +60,6 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -60,16 +60,6 @@ public final class DefaultPlaybackSessionManagerTest {
} }
@Test @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() { public void updateSessions_withoutMediaPeriodId_createsNewSession() {
Timeline timeline = new FakeTimeline(); Timeline timeline = new FakeTimeline();
EventTime eventTime = createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId */ null); EventTime eventTime = createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId */ null);
...@@ -496,6 +486,8 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -496,6 +486,8 @@ public final class DefaultPlaybackSessionManagerTest {
@Test @Test
public void belongsToSession_withSameWindowIndex_returnsTrue() { public void belongsToSession_withSameWindowIndex_returnsTrue() {
EventTime eventTime =
createEventTime(Timeline.EMPTY, /* windowIndex= */ 0, /* mediaPeriodId= */ null);
Timeline timeline = new FakeTimeline(); Timeline timeline = new FakeTimeline();
EventTime eventTimeWithTimeline = EventTime eventTimeWithTimeline =
createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId= */ null); createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId= */ null);
...@@ -504,10 +496,11 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -504,10 +496,11 @@ public final class DefaultPlaybackSessionManagerTest {
timeline.getUidOfPeriod(/* periodIndex= */ 0), /* windowSequenceNumber= */ 0); timeline.getUidOfPeriod(/* periodIndex= */ 0), /* windowSequenceNumber= */ 0);
EventTime eventTimeWithMediaPeriodId = EventTime eventTimeWithMediaPeriodId =
createEventTime(timeline, /* windowIndex= */ 0, mediaPeriodId); createEventTime(timeline, /* windowIndex= */ 0, mediaPeriodId);
sessionManager.updateSessions(eventTimeWithTimeline); sessionManager.updateSessions(eventTime);
ArgumentCaptor<String> sessionId = ArgumentCaptor.forClass(String.class); ArgumentCaptor<String> 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())) assertThat(sessionManager.belongsToSession(eventTimeWithTimeline, sessionId.getValue()))
.isTrue(); .isTrue();
assertThat(sessionManager.belongsToSession(eventTimeWithMediaPeriodId, sessionId.getValue())) assertThat(sessionManager.belongsToSession(eventTimeWithMediaPeriodId, sessionId.getValue()))
...@@ -516,11 +509,11 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -516,11 +509,11 @@ public final class DefaultPlaybackSessionManagerTest {
@Test @Test
public void belongsToSession_withOtherWindowIndex_returnsFalse() { public void belongsToSession_withOtherWindowIndex_returnsFalse() {
Timeline timeline = new FakeTimeline(/* windowCount= */ 2);
EventTime eventTime = EventTime eventTime =
createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId= */ null); createEventTime(Timeline.EMPTY, /* windowIndex= */ 0, /* mediaPeriodId= */ null);
EventTime eventTimeOtherWindow = EventTime eventTimeOtherWindow =
createEventTime(timeline, /* windowIndex= */ 1, /* mediaPeriodId= */ null); createEventTime(Timeline.EMPTY, /* windowIndex= */ 1, /* mediaPeriodId= */ null);
Timeline timeline = new FakeTimeline(/* windowCount= */ 2);
MediaPeriodId mediaPeriodId = MediaPeriodId mediaPeriodId =
new MediaPeriodId( new MediaPeriodId(
timeline.getUidOfPeriod(/* periodIndex= */ 1), /* windowSequenceNumber= */ 1); timeline.getUidOfPeriod(/* periodIndex= */ 1), /* windowSequenceNumber= */ 1);
...@@ -592,18 +585,18 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -592,18 +585,18 @@ public final class DefaultPlaybackSessionManagerTest {
} }
@Test @Test
public void timelineUpdate_toEmpty_finishesAllSessionsAndDoesNotCreateNewSessions() { public void initialTimelineUpdate_finishesAllSessionsOutsideTimeline() {
Timeline timeline = new FakeTimeline(/* windowCount= */ 2);
EventTime eventTime1 = EventTime eventTime1 =
createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId= */ null); createEventTime(Timeline.EMPTY, /* windowIndex= */ 0, /* mediaPeriodId= */ null);
EventTime eventTime2 = EventTime eventTime2 =
createEventTime(timeline, /* windowIndex= */ 1, /* mediaPeriodId= */ null); createEventTime(Timeline.EMPTY, /* windowIndex= */ 1, /* mediaPeriodId= */ null);
sessionManager.updateSessions(eventTime1); sessionManager.updateSessions(eventTime1);
sessionManager.updateSessions(eventTime2); sessionManager.updateSessions(eventTime2);
Timeline timeline = new FakeTimeline();
EventTime newTimelineEventTime =
createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId= */ null);
EventTime eventTimeWithEmptyTimeline = sessionManager.updateSessionsWithTimelineChange(newTimelineEventTime);
createEventTime(Timeline.EMPTY, /* windowIndex= */ 0, /* mediaPeriodId= */ null);
sessionManager.updateSessionsWithTimelineChange(eventTimeWithEmptyTimeline);
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);
...@@ -612,19 +605,14 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -612,19 +605,14 @@ public final class DefaultPlaybackSessionManagerTest {
verify(mockListener).onSessionActive(eventTime1, sessionId1.getValue()); verify(mockListener).onSessionActive(eventTime1, sessionId1.getValue());
verify(mockListener) verify(mockListener)
.onSessionFinished( .onSessionFinished(
eventTimeWithEmptyTimeline, newTimelineEventTime,
sessionId1.getValue(),
/* automaticTransitionToNextPlayback= */ false);
verify(mockListener)
.onSessionFinished(
eventTimeWithEmptyTimeline,
sessionId2.getValue(), sessionId2.getValue(),
/* automaticTransitionToNextPlayback= */ false); /* automaticTransitionToNextPlayback= */ false);
verifyNoMoreInteractions(mockListener); verifyNoMoreInteractions(mockListener);
} }
@Test @Test
public void timelineUpdate_resolvesWindowIndices() { public void dynamicTimelineUpdate_resolvesWindowIndices() {
Timeline initialTimeline = Timeline initialTimeline =
new FakeTimeline( new FakeTimeline(
new TimelineWindowDefinition(/* periodCount= */ 2, /* id= */ 100), new TimelineWindowDefinition(/* periodCount= */ 2, /* id= */ 100),
......
library/core/src/test/java/com/google/android/exoplayer2/analytics/PlaybackStatsListenerTest.java
...@@ -22,7 +22,6 @@ import static org.mockito.Mockito.mock; ...@@ -22,7 +22,6 @@ import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never; import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times; import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import androidx.test.core.app.ApplicationProvider; import androidx.test.core.app.ApplicationProvider;
...@@ -78,18 +77,15 @@ public final class PlaybackStatsListenerTest { ...@@ -78,18 +77,15 @@ public final class PlaybackStatsListenerTest {
} }
@Test @Test
public void stateChangeEvent_toEndedWithEmptyTimeline_doesNotCreateInitialPlaybackStats() public void stateChangeEvent_toNonIdle_createsInitialPlaybackStats() throws Exception {
throws Exception {
PlaybackStatsListener.Callback callback = mock(PlaybackStatsListener.Callback.class);
PlaybackStatsListener playbackStatsListener = PlaybackStatsListener playbackStatsListener =
new PlaybackStatsListener(/* keepHistory= */ true, callback); new PlaybackStatsListener(/* keepHistory= */ true, /* callback= */ null);
player.addAnalyticsListener(playbackStatsListener); player.addAnalyticsListener(playbackStatsListener);
player.prepare(); player.prepare();
runUntilPendingCommandsAreFullyHandled(player); runUntilPendingCommandsAreFullyHandled(player);
assertThat(playbackStatsListener.getPlaybackStats()).isNull(); assertThat(playbackStatsListener.getPlaybackStats()).isNotNull();
verifyNoMoreInteractions(callback);
} }
@Test @Test
......
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