Commit 3b277b28 by tonihei Committed by Christos Tsilopoulos

Fix unintentional new sessions after player release in stats listener.

PlaybackStatsListener has a method whose original intention was to be
called when the player is releaed to finish all pending sessions.
However, this also meant that later events (e.g. onVideoDecoderDisabled)
could create new sessions because the old one was already finished.

Use the new onPlayerReleased callback to implement this properly and to
fix the unintentional new session creation.

PiperOrigin-RevId: 347809527
parent e95e2e0f
......@@ -147,28 +147,6 @@ public final class PlaybackStatsListener
return activeStatsTracker == null ? null : activeStatsTracker.build(/* isFinal= */ false);
}
/**
* Finishes all pending playback sessions. Should be called when the listener is removed from the
* player or when the player is released.
*/
public void finishAllSessions() {
// TODO: Add AnalyticsListener.onAttachedToPlayer and onDetachedFromPlayer to auto-release with
// an actual EventTime. Should also simplify other cases where the listener needs to be released
// separately from the player.
sessionManager.finishAllSessions(
new EventTime(
SystemClock.elapsedRealtime(),
Timeline.EMPTY,
/* windowIndex= */ 0,
/* mediaPeriodId= */ null,
/* eventPlaybackPositionMs= */ 0,
Timeline.EMPTY,
/* currentWindowIndex= */ 0,
/* currentMediaPeriodId= */ null,
/* currentPlaybackPositionMs= */ 0,
/* totalBufferedDurationMs= */ 0));
}
// PlaybackSessionManager.Listener implementation.
@Override
......@@ -309,6 +287,9 @@ public final class PlaybackStatsListener
onSeekStartedEventTime = null;
videoFormat = null;
audioFormat = null;
if (events.contains(AnalyticsListener.EVENT_PLAYER_RELEASED)) {
sessionManager.finishAllSessions(events.getEventTime(EVENT_PLAYER_RELEASED));
}
}
private void maybeAddSessions(Player player, Events events) {
......
......@@ -152,7 +152,7 @@ public final class PlaybackStatsListenerTest {
}
@Test
public void finishAllSessions_callsAllPendingCallbacks() throws Exception {
public void playerRelease_callsAllPendingCallbacks() throws Exception {
PlaybackStatsListener.Callback callback = mock(PlaybackStatsListener.Callback.class);
PlaybackStatsListener playbackStatsListener =
new PlaybackStatsListener(/* keepHistory= */ true, callback);
......@@ -167,7 +167,8 @@ public final class PlaybackStatsListenerTest {
TestPlayerRunHelper.playUntilPosition(
player, /* windowIndex= */ 0, /* positionMs= */ player.getDuration());
runMainLooperToNextTask();
playbackStatsListener.finishAllSessions();
player.release();
runMainLooperToNextTask();
ArgumentCaptor<AnalyticsListener.EventTime> eventTimeCaptor =
ArgumentCaptor.forClass(AnalyticsListener.EventTime.class);
......@@ -178,23 +179,4 @@ public final class PlaybackStatsListenerTest {
.collect(Collectors.toList()))
.containsExactly(0, 1);
}
@Test
public void finishAllSessions_doesNotCallCallbackAgainWhenSessionWouldBeAutomaticallyFinished() {
PlaybackStatsListener.Callback callback = mock(PlaybackStatsListener.Callback.class);
PlaybackStatsListener playbackStatsListener =
new PlaybackStatsListener(/* keepHistory= */ true, callback);
player.addAnalyticsListener(playbackStatsListener);
player.setMediaItem(MediaItem.fromUri("http://test.org"));
runMainLooperToNextTask();
playbackStatsListener.finishAllSessions();
// Simulate removing the playback item to ensure the session would finish if it hadn't already.
player.clearMediaItems();
runMainLooperToNextTask();
// Verify the callback was called once only.
verify(callback).onPlaybackStatsReady(any(), any());
}
}
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