From 6b5b3d814196fcf2336ea75995249d916b87a55f Mon Sep 17 00:00:00 2001 From: ibaker Date: Thu, 11 Feb 2021 13:47:43 +0000 Subject: [PATCH] Check if keepalive is enabled before releasing sessions in DDSM.release If keepalive is disabled the existing code over-eagerly releases DrmSession instances. This is arguably OK since a (Default)DrmSession should be released before its (Default)Manager is released (since the underlying MediaDrm instance might be released when the manager is released). And if all sessions are released before the manager is released then `sessions` is empty, so the loop is a no-op. Issue: #8576 #minor-release PiperOrigin-RevId: 356955308 --- RELEASENOTES.md | 3 +++ .../drm/DefaultDrmSessionManager.java | 14 +++++----- .../drm/DefaultDrmSessionManagerTest.java | 26 +++++++++++++++++++ 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 4747f0f795..d0407b4c02 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -25,6 +25,9 @@ ([#8523](https://github.com/google/ExoPlayer/issues/8523)). * Propagate DRM configuration when creating media sources for ad content ([#8568](https://github.com/google/ExoPlayer/issues/8568)). + * Only release 'keepalive' references to `DrmSession` in + `DefaultDrmSessionManager#release()` if keepalive is enabled + ([#8576](https://github.com/google/ExoPlayer/issues/8576)). * Remove deprecated symbols: * Remove `Player.DefaultEventListener`. Use `Player.EventListener` instead. diff --git a/library/core/src/main/java/com/google/android/exoplayer2/drm/DefaultDrmSessionManager.java b/library/core/src/main/java/com/google/android/exoplayer2/drm/DefaultDrmSessionManager.java index 67cb095b8d..d24d5ce847 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/drm/DefaultDrmSessionManager.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/drm/DefaultDrmSessionManager.java @@ -457,12 +457,14 @@ public class DefaultDrmSessionManager implements DrmSessionManager { if (--prepareCallsCount != 0) { return; } - // Make a local copy, because sessions are removed from this.sessions during release (via - // callback). - List sessions = new ArrayList<>(this.sessions); - for (int i = 0; i < sessions.size(); i++) { - // Release all the keepalive acquisitions. - sessions.get(i).release(/* eventDispatcher= */ null); + // Release all keepalive acquisitions if keepalive is enabled. + if (sessionKeepaliveMs != C.TIME_UNSET) { + // Make a local copy, because sessions are removed from this.sessions during release (via + // callback). + List sessions = new ArrayList<>(this.sessions); + for (int i = 0; i < sessions.size(); i++) { + sessions.get(i).release(/* eventDispatcher= */ null); + } } Assertions.checkNotNull(exoMediaDrm).release(); exoMediaDrm = null; diff --git a/library/core/src/test/java/com/google/android/exoplayer2/drm/DefaultDrmSessionManagerTest.java b/library/core/src/test/java/com/google/android/exoplayer2/drm/DefaultDrmSessionManagerTest.java index 5ac26a76b3..c0b83e7a65 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/drm/DefaultDrmSessionManagerTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/drm/DefaultDrmSessionManagerTest.java @@ -147,6 +147,32 @@ public class DefaultDrmSessionManagerTest { assertThat(drmSession.getState()).isEqualTo(DrmSession.STATE_RELEASED); } + @Test(timeout = 10_000) + public void managerRelease_keepaliveDisabled_doesntReleaseAnySessions() throws Exception { + FakeExoMediaDrm.LicenseServer licenseServer = + FakeExoMediaDrm.LicenseServer.allowingSchemeDatas(DRM_SCHEME_DATAS); + DrmSessionManager drmSessionManager = + new DefaultDrmSessionManager.Builder() + .setUuidAndExoMediaDrmProvider(DRM_SCHEME_UUID, uuid -> new FakeExoMediaDrm()) + .setSessionKeepaliveMs(C.TIME_UNSET) + .build(/* mediaDrmCallback= */ licenseServer); + + drmSessionManager.prepare(); + DrmSession drmSession = + checkNotNull( + drmSessionManager.acquireSession( + /* playbackLooper= */ checkNotNull(Looper.myLooper()), + /* eventDispatcher= */ null, + FORMAT_WITH_DRM_INIT_DATA)); + waitForOpenedWithKeys(drmSession); + assertThat(drmSession.getState()).isEqualTo(DrmSession.STATE_OPENED_WITH_KEYS); + + // Release the manager, the session should still be open (though it's unusable because + // the underlying ExoMediaDrm is released). + drmSessionManager.release(); + assertThat(drmSession.getState()).isEqualTo(DrmSession.STATE_OPENED_WITH_KEYS); + } + @Test(timeout = 10_000) public void maxConcurrentSessionsExceeded_allKeepAliveSessionsEagerlyReleased() throws Exception { ImmutableList secondSchemeDatas =