From 394ab7bcfd53bc8ffb8d2e37a5aeb71fef04d13c Mon Sep 17 00:00:00 2001 From: tonihei Date: Mon, 29 Mar 2021 14:18:13 +0100 Subject: [PATCH] Make use of position discontinuity changes. The extended onPositionDiscontinuity callback can be used to improve some listener classes: - Listening to onTimelineChanged to detect discontinuities is no longer needed. - Listening to onSeekStarted is no longer needed as the start position is part of the onPositionDiscontinuty callback. - The exact old position is also useful for media time history logging. As a side effect, removing onSeekStarted handling from PlaybackStatsListener fixes Issue: #8675 that was caused by the special EventTime handling for onSeekStarted. PiperOrigin-RevId: 365558959 --- .../DefaultPlaybackSessionManager.java | 42 +++++--- .../analytics/PlaybackSessionManager.java | 8 ++ .../analytics/PlaybackStatsListener.java | 97 +++++++++---------- .../android/exoplayer2/util/EventLogger.java | 5 - 4 files changed, 79 insertions(+), 73 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManager.java b/library/core/src/main/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManager.java index c43422ca8d..2cc8456e12 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManager.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/analytics/DefaultPlaybackSessionManager.java @@ -33,6 +33,7 @@ import java.util.HashMap; import java.util.Iterator; import java.util.Random; 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 @@ -184,7 +185,7 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag } } } - updateSessionsWithDiscontinuity(eventTime, Player.DISCONTINUITY_REASON_INTERNAL); + updateCurrentSession(eventTime); } @Override @@ -208,6 +209,31 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag } } } + updateCurrentSession(eventTime); + } + + @Override + @Nullable + public synchronized String getActiveSessionId() { + return currentSessionId; + } + + @Override + public synchronized void finishAllSessions(EventTime eventTime) { + currentSessionId = null; + Iterator iterator = sessions.values().iterator(); + while (iterator.hasNext()) { + SessionDescriptor session = iterator.next(); + iterator.remove(); + if (session.isCreated && listener != null) { + listener.onSessionFinished( + eventTime, session.sessionId, /* automaticTransitionToNextPlayback= */ false); + } + } + } + + @RequiresNonNull("listener") + private void updateCurrentSession(EventTime eventTime) { @Nullable SessionDescriptor previousSessionDescriptor = sessions.get(currentSessionId); SessionDescriptor currentSessionDescriptor = getOrAddSession(eventTime.windowIndex, eventTime.mediaPeriodId); @@ -234,20 +260,6 @@ public final class DefaultPlaybackSessionManager implements PlaybackSessionManag } } - @Override - public void finishAllSessions(EventTime eventTime) { - currentSessionId = null; - Iterator iterator = sessions.values().iterator(); - while (iterator.hasNext()) { - SessionDescriptor session = iterator.next(); - iterator.remove(); - if (session.isCreated && listener != null) { - listener.onSessionFinished( - eventTime, session.sessionId, /* automaticTransitionToNextPlayback= */ false); - } - } - } - private SessionDescriptor getOrAddSession( int windowIndex, @Nullable MediaPeriodId mediaPeriodId) { // There should only be one matching session if mediaPeriodId is non-null. If mediaPeriodId is diff --git a/library/core/src/main/java/com/google/android/exoplayer2/analytics/PlaybackSessionManager.java b/library/core/src/main/java/com/google/android/exoplayer2/analytics/PlaybackSessionManager.java index 1038f3b6e1..786da8c2fc 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/analytics/PlaybackSessionManager.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/analytics/PlaybackSessionManager.java @@ -15,6 +15,7 @@ */ package com.google.android.exoplayer2.analytics; +import androidx.annotation.Nullable; import com.google.android.exoplayer2.Player.DiscontinuityReason; import com.google.android.exoplayer2.Timeline; import com.google.android.exoplayer2.analytics.AnalyticsListener.EventTime; @@ -128,6 +129,13 @@ public interface PlaybackSessionManager { */ void updateSessionsWithDiscontinuity(EventTime eventTime, @DiscontinuityReason int reason); + /** + * Returns the session identifier of the session that is currently actively playing, or {@code + * null} if there no such session. + */ + @Nullable + String getActiveSessionId(); + /** * Finishes all existing sessions and calls their respective {@link * Listener#onSessionFinished(EventTime, String, boolean)} callback. diff --git a/library/core/src/main/java/com/google/android/exoplayer2/analytics/PlaybackStatsListener.java b/library/core/src/main/java/com/google/android/exoplayer2/analytics/PlaybackStatsListener.java index d8f4074c68..afff94cb58 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/analytics/PlaybackStatsListener.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/analytics/PlaybackStatsListener.java @@ -79,19 +79,18 @@ public final class PlaybackStatsListener private final Period period; private PlaybackStats finishedPlaybackStats; - @Nullable private String activeContentPlayback; - @Nullable private String activeAdPlayback; - @Nullable private EventTime onSeekStartedEventTime; - @Player.DiscontinuityReason int discontinuityReason; - int droppedFrames; - @Nullable Exception nonFatalException; - long bandwidthTimeMs; - long bandwidthBytes; - @Nullable Format videoFormat; - @Nullable Format audioFormat; - int videoHeight; - int videoWidth; + @Nullable private String discontinuityFromSession; + private long discontinuityFromPositionMs; + @Player.DiscontinuityReason private int discontinuityReason; + private int droppedFrames; + @Nullable private Exception nonFatalException; + private long bandwidthTimeMs; + private long bandwidthBytes; + @Nullable private Format videoFormat; + @Nullable private Format audioFormat; + private int videoHeight; + private int videoWidth; /** * Creates listener for playback stats. @@ -137,13 +136,10 @@ public final class PlaybackStatsListener */ @Nullable public PlaybackStats getPlaybackStats() { + @Nullable String activeSessionId = sessionManager.getActiveSessionId(); @Nullable PlaybackStatsTracker activeStatsTracker = - activeAdPlayback != null - ? playbackStatsTrackers.get(activeAdPlayback) - : activeContentPlayback != null - ? playbackStatsTrackers.get(activeContentPlayback) - : null; + activeSessionId == null ? null : playbackStatsTrackers.get(activeSessionId); return activeStatsTracker == null ? null : activeStatsTracker.build(/* isFinal= */ false); } @@ -159,11 +155,6 @@ public final class PlaybackStatsListener @Override public void onSessionActive(EventTime eventTime, String session) { checkNotNull(playbackStatsTrackers.get(session)).onForeground(); - if (eventTime.mediaPeriodId != null && eventTime.mediaPeriodId.isAd()) { - activeAdPlayback = session; - } else { - activeContentPlayback = session; - } } @Override @@ -173,14 +164,11 @@ public final class PlaybackStatsListener @Override public void onSessionFinished(EventTime eventTime, String session, boolean automaticTransition) { - if (session.equals(activeAdPlayback)) { - activeAdPlayback = null; - } else if (session.equals(activeContentPlayback)) { - activeContentPlayback = null; - } PlaybackStatsTracker tracker = checkNotNull(playbackStatsTrackers.remove(session)); EventTime startEventTime = checkNotNull(sessionStartEventTimes.remove(session)); - tracker.onFinished(eventTime, automaticTransition); + long discontinuityFromPositionMs = + session.equals(discontinuityFromSession) ? this.discontinuityFromPositionMs : C.TIME_UNSET; + tracker.onFinished(eventTime, automaticTransition, discontinuityFromPositionMs); PlaybackStats playbackStats = tracker.build(/* isFinal= */ true); finishedPlaybackStats = PlaybackStats.merge(finishedPlaybackStats, playbackStats); if (callback != null) { @@ -191,15 +179,18 @@ public final class PlaybackStatsListener // AnalyticsListener implementation. @Override - public void onPositionDiscontinuity(EventTime eventTime, @Player.DiscontinuityReason int reason) { + public void onPositionDiscontinuity( + EventTime eventTime, + Player.PositionInfo oldPositionInfo, + Player.PositionInfo newPositionInfo, + @Player.DiscontinuityReason int reason) { + if (discontinuityFromSession == null) { + discontinuityFromSession = sessionManager.getActiveSessionId(); + discontinuityFromPositionMs = oldPositionInfo.positionMs; + } discontinuityReason = reason; } - @Override - public void onSeekStarted(EventTime eventTime) { - onSeekStartedEventTime = eventTime; - } - @Override public void onDroppedVideoFrames(EventTime eventTime, int droppedFrames, long elapsedMs) { this.droppedFrames = droppedFrames; @@ -253,9 +244,7 @@ public final class PlaybackStatsListener for (String session : playbackStatsTrackers.keySet()) { Pair eventTimeAndBelongsToPlayback = findBestEventTime(events, session); PlaybackStatsTracker tracker = playbackStatsTrackers.get(session); - boolean hasPositionDiscontinuity = - hasEvent(events, session, EVENT_POSITION_DISCONTINUITY) - || hasEvent(events, session, EVENT_TIMELINE_CHANGED); + boolean hasDiscontinuityToPlayback = hasEvent(events, session, EVENT_POSITION_DISCONTINUITY); boolean hasDroppedFrames = hasEvent(events, session, EVENT_DROPPED_VIDEO_FRAMES); boolean hasAudioUnderrun = hasEvent(events, session, EVENT_AUDIO_UNDERRUN); boolean startedLoading = hasEvent(events, session, EVENT_LOAD_STARTED); @@ -270,8 +259,8 @@ public final class PlaybackStatsListener player, /* eventTime= */ eventTimeAndBelongsToPlayback.first, /* belongsToPlayback= */ eventTimeAndBelongsToPlayback.second, - /* seeked= */ onSeekStartedEventTime != null, - hasPositionDiscontinuity, + session.equals(discontinuityFromSession) ? discontinuityFromPositionMs : C.TIME_UNSET, + hasDiscontinuityToPlayback, hasDroppedFrames ? droppedFrames : 0, hasAudioUnderrun, startedLoading, @@ -284,9 +273,9 @@ public final class PlaybackStatsListener hasVideoSize ? videoHeight : Format.NO_VALUE, hasVideoSize ? videoWidth : Format.NO_VALUE); } - onSeekStartedEventTime = null; videoFormat = null; audioFormat = null; + discontinuityFromSession = null; if (events.contains(AnalyticsListener.EVENT_PLAYER_RELEASED)) { sessionManager.finishAllSessions(events.getEventTime(EVENT_PLAYER_RELEASED)); } @@ -311,11 +300,8 @@ public final class PlaybackStatsListener } private Pair findBestEventTime(Events events, String session) { - // Check all event times of the events as well as the event time when a seek started. - @Nullable EventTime eventTime = onSeekStartedEventTime; - boolean belongsToPlayback = - onSeekStartedEventTime != null - && sessionManager.belongsToSession(onSeekStartedEventTime, session); + @Nullable EventTime eventTime = null; + boolean belongsToPlayback = false; for (int i = 0; i < events.size(); i++) { @EventFlags int event = events.get(i); EventTime newEventTime = events.getEventTime(event); @@ -463,15 +449,18 @@ public final class PlaybackStatsListener * @param eventTime The {@link EventTime}. Does not belong to this playback. * @param automaticTransition Whether the playback finished because of an automatic transition * to the next playback item. + * @param discontinuityFromPositionMs The position before the discontinuity from this playback, + * {@link C#TIME_UNSET} if no discontinuity started from this playback. */ - public void onFinished(EventTime eventTime, boolean automaticTransition) { + public void onFinished( + EventTime eventTime, boolean automaticTransition, long discontinuityFromPositionMs) { // Simulate state change to ENDED to record natural ending of playback. @PlaybackState int finalPlaybackState = currentPlaybackState == PlaybackStats.PLAYBACK_STATE_ENDED || automaticTransition ? PlaybackStats.PLAYBACK_STATE_ENDED : PlaybackStats.PLAYBACK_STATE_ABANDONED; - maybeUpdateMediaTimeHistory(eventTime.realtimeMs, /* mediaTimeMs= */ C.TIME_UNSET); + maybeUpdateMediaTimeHistory(eventTime.realtimeMs, discontinuityFromPositionMs); maybeRecordVideoFormatTime(eventTime.realtimeMs); maybeRecordAudioFormatTime(eventTime.realtimeMs); updatePlaybackState(finalPlaybackState, eventTime); @@ -483,8 +472,9 @@ public final class PlaybackStatsListener * @param player The {@link Player}. * @param eventTime The {@link EventTime} of the events. * @param belongsToPlayback Whether the {@code eventTime} belongs to this playback. - * @param seeked Whether a seek occurred. - * @param positionDiscontinuity Whether a position discontinuity occurred for this playback. + * @param discontinuityFromPositionMs The position before the discontinuity from this playback, + * or {@link C#TIME_UNSET} if no discontinuity started from this playback. + * @param hasDiscontinuity Whether a discontinuity to this playback occurred. * @param droppedFrameCount The number of newly dropped frames for this playback. * @param hasAudioUnderun Whether a new audio underrun occurred for this playback. * @param startedLoading Whether this playback started loading. @@ -501,8 +491,8 @@ public final class PlaybackStatsListener Player player, EventTime eventTime, boolean belongsToPlayback, - boolean seeked, - boolean positionDiscontinuity, + long discontinuityFromPositionMs, + boolean hasDiscontinuity, int droppedFrameCount, boolean hasAudioUnderun, boolean startedLoading, @@ -514,7 +504,8 @@ public final class PlaybackStatsListener @Nullable Format audioFormat, int videoHeight, int videoWidth) { - if (seeked) { + if (discontinuityFromPositionMs != C.TIME_UNSET) { + maybeUpdateMediaTimeHistory(eventTime.realtimeMs, discontinuityFromPositionMs); isSeeking = true; } if (player.getPlaybackState() != Player.STATE_BUFFERING) { @@ -523,7 +514,7 @@ public final class PlaybackStatsListener int playerPlaybackState = player.getPlaybackState(); if (playerPlaybackState == Player.STATE_IDLE || playerPlaybackState == Player.STATE_ENDED - || positionDiscontinuity) { + || hasDiscontinuity) { isInterruptedByAd = false; } if (fatalError != null) { diff --git a/library/core/src/main/java/com/google/android/exoplayer2/util/EventLogger.java b/library/core/src/main/java/com/google/android/exoplayer2/util/EventLogger.java index 33b86481a7..513621a6fa 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/util/EventLogger.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/util/EventLogger.java @@ -187,11 +187,6 @@ public class EventLogger implements AnalyticsListener { logd(eventTime, "positionDiscontinuity", builder.toString()); } - @Override - public void onSeekStarted(EventTime eventTime) { - logd(eventTime, "seekStarted"); - } - @Override public void onPlaybackParametersChanged( EventTime eventTime, PlaybackParameters playbackParameters) {