Skip to content

SDK / exoplayer

Go to a project
Commit 50c46e6b by tonihei Committed by Oliver Woodman
Options

Rollback of https://github.com/google/ExoPlayer/commit/09096d6fbf5532728ab675d2d1ccefc3daa0bb03 

*** Original commit ***

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...

***

PiperOrigin-RevId: 369609523
parent a8cf54e9
Inline Side-by-side
Showing with 49 additions and 26 deletions
library/core/src/main/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManager.java
...@@ -105,6 +105,10 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag ...@@ -105,6 +105,10 @@ 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
...@@ -234,6 +238,11 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag ...@@ -234,6 +238,11 @@ 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(player, events); maybeAddSessions(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,17 +281,15 @@ public final class PlaybackStatsListener ...@@ -281,17 +281,15 @@ public final class PlaybackStatsListener
} }
} }
private void maybeAddSessions(Player player, Events events) { private void maybeAddSessions(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 (!isCompletelyIdle && event == EVENT_POSITION_DISCONTINUITY) { } else if (event == EVENT_POSITION_DISCONTINUITY) {
sessionManager.updateSessionsWithDiscontinuity(eventTime, discontinuityReason); sessionManager.updateSessionsWithDiscontinuity(eventTime, discontinuityReason);
} else if (!isCompletelyIdle) { } else {
sessionManager.updateSessions(eventTime); sessionManager.updateSessions(eventTime);
} }
} }
......
library/core/src/test/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManagerTest.java
...@@ -60,6 +60,16 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -60,6 +60,16 @@ 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);
...@@ -486,8 +496,6 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -486,8 +496,6 @@ 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);
...@@ -496,11 +504,10 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -496,11 +504,10 @@ 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(eventTime); sessionManager.updateSessions(eventTimeWithTimeline);
ArgumentCaptor<String> sessionId = ArgumentCaptor.forClass(String.class); ArgumentCaptor<String> sessionId = ArgumentCaptor.forClass(String.class);
verify(mockListener).onSessionCreated(eq(eventTime), sessionId.capture()); verify(mockListener).onSessionCreated(eq(eventTimeWithTimeline), 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()))
...@@ -509,11 +516,11 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -509,11 +516,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.EMPTY, /* windowIndex= */ 0, /* mediaPeriodId= */ null); createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId= */ null);
EventTime eventTimeOtherWindow = EventTime eventTimeOtherWindow =
createEventTime(Timeline.EMPTY, /* windowIndex= */ 1, /* mediaPeriodId= */ null); createEventTime(timeline, /* 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);
...@@ -585,18 +592,18 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -585,18 +592,18 @@ public final class DefaultPlaybackSessionManagerTest {
} }
@Test @Test
public void initialTimelineUpdate_finishesAllSessionsOutsideTimeline() { public void timelineUpdate_toEmpty_finishesAllSessionsAndDoesNotCreateNewSessions() {
Timeline timeline = new FakeTimeline(/* windowCount= */ 2);
EventTime eventTime1 = EventTime eventTime1 =
createEventTime(Timeline.EMPTY, /* windowIndex= */ 0, /* mediaPeriodId= */ null); createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId= */ null);
EventTime eventTime2 = EventTime eventTime2 =
createEventTime(Timeline.EMPTY, /* windowIndex= */ 1, /* mediaPeriodId= */ null); createEventTime(timeline, /* 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);
sessionManager.updateSessionsWithTimelineChange(newTimelineEventTime); EventTime eventTimeWithEmptyTimeline =
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);
...@@ -605,14 +612,19 @@ public final class DefaultPlaybackSessionManagerTest { ...@@ -605,14 +612,19 @@ public final class DefaultPlaybackSessionManagerTest {
verify(mockListener).onSessionActive(eventTime1, sessionId1.getValue()); verify(mockListener).onSessionActive(eventTime1, sessionId1.getValue());
verify(mockListener) verify(mockListener)
.onSessionFinished( .onSessionFinished(
newTimelineEventTime, eventTimeWithEmptyTimeline,
sessionId1.getValue(),
/* automaticTransitionToNextPlayback= */ false);
verify(mockListener)
.onSessionFinished(
eventTimeWithEmptyTimeline,
sessionId2.getValue(), sessionId2.getValue(),
/* automaticTransitionToNextPlayback= */ false); /* automaticTransitionToNextPlayback= */ false);
verifyNoMoreInteractions(mockListener); verifyNoMoreInteractions(mockListener);
} }
@Test @Test
public void dynamicTimelineUpdate_resolvesWindowIndices() { public void timelineUpdate_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,6 +22,7 @@ import static org.mockito.Mockito.mock; ...@@ -22,6 +22,7 @@ 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;
...@@ -77,15 +78,18 @@ public final class PlaybackStatsListenerTest { ...@@ -77,15 +78,18 @@ public final class PlaybackStatsListenerTest {
} }
@Test @Test
public void stateChangeEvent_toNonIdle_createsInitialPlaybackStats() throws Exception { public void stateChangeEvent_toEndedWithEmptyTimeline_doesNotCreateInitialPlaybackStats()
throws Exception {
PlaybackStatsListener.Callback callback = mock(PlaybackStatsListener.Callback.class);
PlaybackStatsListener playbackStatsListener = PlaybackStatsListener playbackStatsListener =
new PlaybackStatsListener(/* keepHistory= */ true, /* callback= */ null); new PlaybackStatsListener(/* keepHistory= */ true, callback);
player.addAnalyticsListener(playbackStatsListener); player.addAnalyticsListener(playbackStatsListener);
player.prepare(); player.prepare();
runUntilPendingCommandsAreFullyHandled(player); runUntilPendingCommandsAreFullyHandled(player);
assertThat(playbackStatsListener.getPlaybackStats()).isNotNull(); assertThat(playbackStatsListener.getPlaybackStats()).isNull();
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