From be986537ab2939f5ed9be0e7625bbf7ab7df0ef0 Mon Sep 17 00:00:00 2001 From: tonihei Date: Thu, 13 Oct 2022 12:29:51 +0000 Subject: [PATCH] Ensure sessions without MediaPeriodId are ended after seek to new item We already have logic to end all session except the current one if the current one doesn't have a MediaPeriodId yet. This is assuming that this only happens after a seek on the app side where the player doesn't have detailled knowledge about the MediaPeriodIds yet. Currently this logic isn't triggered if the window we are coming from doesn't have its MediaPeriodId either as we run into another check that keeps sessions around until we have a valid windowSequenceNumber. Swapping both conditions fixes this case without breaking any of the other known transition scenarios. Issue: androidx/media#180 PiperOrigin-RevId: 480866465 (cherry picked from commit 409c9f874cbfce4b6c77ee700c0c50caa218cf3d) --- RELEASENOTES.md | 2 ++ .../DefaultPlaybackSessionManager.java | 8 +++--- .../DefaultPlaybackSessionManagerTest.java | 27 ++++++++++++++++++- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 87a11c84a8..ecf4473c6c 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -21,6 +21,8 @@ ([#15](https://github.com/androidx/media/issues/15)). * Prefer other tracks to Dolby Vision if display does not support it. ([#8944](https://github.com/google/ExoPlayer/issues/8944)). + * Fix session tracking problem with fast seeks in `PlaybackStatsListener` + ([#180](https://github.com/androidx/media/issues/180)). * Downloads: * Fix potential infinite loop in `ProgressiveDownloader` caused by simultaneous download and playback with the same `PriorityTaskManager` diff --git a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/DefaultPlaybackSessionManager.java b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/DefaultPlaybackSessionManager.java index 24fc544c32..b0af78bf61 100644 --- a/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/DefaultPlaybackSessionManager.java +++ b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/analytics/DefaultPlaybackSessionManager.java @@ -382,15 +382,15 @@ public void maybeSetWindowSequenceNumber( } public boolean isFinishedAtEventTime(EventTime eventTime) { - if (windowSequenceNumber == C.INDEX_UNSET) { - // Sessions with unspecified window sequence number are kept until we know more. - return false; - } if (eventTime.mediaPeriodId == null) { // For event times without media period id (e.g. after seek to new window), we only keep // sessions of this window. return windowIndex != eventTime.windowIndex; } + if (windowSequenceNumber == C.INDEX_UNSET) { + // Sessions with unspecified window sequence number are kept until we know more. + return false; + } if (eventTime.mediaPeriodId.windowSequenceNumber > windowSequenceNumber) { // All past window sequence numbers are finished. return true; diff --git a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/analytics/DefaultPlaybackSessionManagerTest.java b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/analytics/DefaultPlaybackSessionManagerTest.java index dd74acd379..e331b100db 100644 --- a/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/analytics/DefaultPlaybackSessionManagerTest.java +++ b/libraries/exoplayer/src/test/java/androidx/media3/exoplayer/analytics/DefaultPlaybackSessionManagerTest.java @@ -925,7 +925,8 @@ public void positionDiscontinuity_toNewWindow_withPeriodTransitionReason_finishe } @Test - public void positionDiscontinuity_toNewWindow_withSeekTransitionReason_finishesSession() { + public void + positionDiscontinuity_toNewWindow_withMediaPeriodIds_withSeekTransitionReason_finishesSession() { Timeline timeline = new FakeTimeline(/* windowCount= */ 2); EventTime eventTime1 = createEventTime( @@ -957,6 +958,30 @@ public void positionDiscontinuity_toNewWindow_withSeekTransitionReason_finishesS verifyNoMoreInteractions(mockListener); } + @Test + public void + positionDiscontinuity_toNewWindow_withoutMediaPeriodIds_withSeekTransitionReason_finishesSession() { + Timeline timeline = new FakeTimeline(/* windowCount= */ 2); + EventTime eventTime1 = + createEventTime(timeline, /* windowIndex= */ 0, /* mediaPeriodId= */ null); + EventTime eventTime2 = + createEventTime(timeline, /* windowIndex= */ 1, /* mediaPeriodId= */ null); + sessionManager.updateSessionsWithTimelineChange(eventTime1); + + sessionManager.updateSessionsWithDiscontinuity(eventTime2, Player.DISCONTINUITY_REASON_SEEK); + + ArgumentCaptor sessionId1 = ArgumentCaptor.forClass(String.class); + ArgumentCaptor sessionId2 = ArgumentCaptor.forClass(String.class); + verify(mockListener).onSessionCreated(eq(eventTime1), sessionId1.capture()); + verify(mockListener).onSessionActive(eventTime1, sessionId1.getValue()); + verify(mockListener).onSessionCreated(eq(eventTime2), sessionId2.capture()); + verify(mockListener) + .onSessionFinished( + eventTime2, sessionId1.getValue(), /* automaticTransitionToNextPlayback= */ false); + verify(mockListener).onSessionActive(eventTime2, sessionId2.getValue()); + verifyNoMoreInteractions(mockListener); + } + @Test public void positionDiscontinuity_toSameWindow_withoutMediaPeriodId_doesNotFinishSession() { Timeline timeline = new FakeTimeline();