Skip to content

Commit

Permalink
Check if keepalive is enabled before releasing sessions in DDSM.release
Browse files Browse the repository at this point in the history
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
  • Loading branch information
icbaker authored and ojw28 committed Feb 12, 2021
1 parent de359cd commit 6b5b3d8
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 6 deletions.
3 changes: 3 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -457,12 +457,14 @@ public final void release() {
if (--prepareCallsCount != 0) {
return;
}
// Make a local copy, because sessions are removed from this.sessions during release (via
// callback).
List<DefaultDrmSession> 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<DefaultDrmSession> 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,32 @@ public void managerRelease_allKeepaliveSessionsImmediatelyReleased() throws Exce
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<DrmInitData.SchemeData> secondSchemeDatas =
Expand Down

0 comments on commit 6b5b3d8

Please sign in to comment.