-
Notifications
You must be signed in to change notification settings - Fork 634
Adds load info to DrmSessionEventListeneronDrmKeysLoaded()
#1134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5340fe0
1e47553
6dcfe13
c767893
b0c6db3
6196760
2183d5d
61aed66
6b1c71a
e19716e
f36f661
5d130cb
9fafcbc
442817f
caaa39b
eff7784
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -137,6 +137,7 @@ public interface ReferenceCountListener { | |||
private final UUID uuid; | ||||
private final Looper playbackLooper; | ||||
private final ResponseHandler responseHandler; | ||||
private final Object keyRequestInfoLock; | ||||
|
||||
private @DrmSession.State int state; | ||||
private int referenceCount; | ||||
|
@@ -148,6 +149,11 @@ public interface ReferenceCountListener { | |||
private byte @MonotonicNonNull [] offlineLicenseKeySetId; | ||||
|
||||
@Nullable private KeyRequest currentKeyRequest; | ||||
|
||||
@GuardedBy("keyRequestInfoLock") | ||||
@Nullable | ||||
private KeyRequestInfo.Builder currentKeyRequestInfo; | ||||
|
||||
@Nullable private ProvisionRequest currentProvisionRequest; | ||||
|
||||
/** | ||||
|
@@ -209,6 +215,7 @@ public DefaultDrmSession( | |||
state = STATE_OPENING; | ||||
this.playbackLooper = playbackLooper; | ||||
responseHandler = new ResponseHandler(playbackLooper); | ||||
keyRequestInfoLock = new Object(); | ||||
} | ||||
|
||||
public boolean hasSessionId(byte[] sessionId) { | ||||
|
@@ -348,6 +355,9 @@ public void release(@Nullable DrmSessionEventListener.EventDispatcher eventDispa | |||
cryptoConfig = null; | ||||
lastException = null; | ||||
currentKeyRequest = null; | ||||
synchronized (keyRequestInfoLock) { | ||||
currentKeyRequestInfo = null; | ||||
} | ||||
currentProvisionRequest = null; | ||||
if (sessionId != null) { | ||||
mediaDrm.closeSession(sessionId); | ||||
|
@@ -489,6 +499,12 @@ private long getLicenseDurationRemainingSec() { | |||
|
||||
private void postKeyRequest(byte[] scope, int type, boolean allowRetry) { | ||||
try { | ||||
synchronized (keyRequestInfoLock) { | ||||
currentKeyRequestInfo = new KeyRequestInfo.Builder(); | ||||
if (schemeDatas != null) { | ||||
currentKeyRequestInfo.setSchemeDatas(schemeDatas); | ||||
} | ||||
} | ||||
currentKeyRequest = mediaDrm.getKeyRequest(scope, schemeDatas, type, keyRequestParameters); | ||||
Util.castNonNull(requestHandler).post(MSG_KEYS, checkNotNull(currentKeyRequest), allowRetry); | ||||
} catch (Exception | NoSuchMethodError e) { | ||||
|
@@ -502,6 +518,13 @@ private void onKeyResponse(Object request, Object response) { | |||
return; | ||||
} | ||||
currentKeyRequest = null; | ||||
KeyRequestInfo keyRequestInfo; | ||||
synchronized (keyRequestInfoLock) { | ||||
// currentKeyRequest and currentKeyRequestInfo are assigned together, and nulled-out together, | ||||
// so it must be non-null here. | ||||
keyRequestInfo = checkNotNull(currentKeyRequestInfo).build(); | ||||
currentKeyRequestInfo = null; | ||||
} | ||||
|
||||
if (response instanceof Exception || response instanceof NoSuchMethodError) { | ||||
onKeysError((Throwable) response, /* thrownByExoMediaDrm= */ false); | ||||
|
@@ -510,8 +533,11 @@ private void onKeyResponse(Object request, Object response) { | |||
|
||||
try { | ||||
byte[] responseData = ((MediaDrmCallback.Response) response).data; | ||||
|
||||
if (mode == DefaultDrmSessionManager.MODE_RELEASE) { | ||||
mediaDrm.provideKeyResponse(Util.castNonNull(offlineLicenseKeySetId), responseData); | ||||
// TODO: http://github.com/androidx/media/issues/1001 - Plumb the KeyLoadInfo up into | ||||
// drmKeysRemoved. | ||||
dispatchEvent(DrmSessionEventListener.EventDispatcher::drmKeysRemoved); | ||||
} else { | ||||
byte[] keySetId = mediaDrm.provideKeyResponse(sessionId, responseData); | ||||
|
@@ -523,7 +549,7 @@ private void onKeyResponse(Object request, Object response) { | |||
offlineLicenseKeySetId = keySetId; | ||||
} | ||||
state = STATE_OPENED_WITH_KEYS; | ||||
dispatchEvent(DrmSessionEventListener.EventDispatcher::drmKeysLoaded); | ||||
dispatchEvent(eventDispatcher -> eventDispatcher.drmKeysLoaded(keyRequestInfo)); | ||||
} | ||||
} catch (Exception | NoSuchMethodError e) { | ||||
onKeysError(e, /* thrownByExoMediaDrm= */ true); | ||||
|
@@ -659,7 +685,17 @@ public void handleMessage(Message msg) { | |||
callback.executeProvisionRequest(uuid, (ProvisionRequest) requestTask.request); | ||||
break; | ||||
case MSG_KEYS: | ||||
response = callback.executeKeyRequest(uuid, (KeyRequest) requestTask.request); | ||||
MediaDrmCallback.Response keyResponse = | ||||
callback.executeKeyRequest(uuid, (KeyRequest) requestTask.request); | ||||
response = keyResponse; | ||||
synchronized (keyRequestInfoLock) { | ||||
if (currentKeyRequestInfo != null && keyResponse.loadEventInfo != null) { | ||||
currentKeyRequestInfo.addLoadInfo( | ||||
keyResponse.loadEventInfo.copyWithTaskIdAndDurationMs( | ||||
requestTask.taskId, | ||||
SystemClock.elapsedRealtime() - requestTask.startTimeMs)); | ||||
} | ||||
} | ||||
break; | ||||
default: | ||||
throw new RuntimeException(); | ||||
|
@@ -715,6 +751,11 @@ private boolean maybeRetryRequest(Message originalMsg, MediaDrmCallbackException | |||
// The error is fatal. | ||||
return false; | ||||
} | ||||
synchronized (keyRequestInfoLock) { | ||||
if (currentKeyRequestInfo != null) { | ||||
currentKeyRequestInfo.addLoadInfo(loadEventInfo); | ||||
} | ||||
} | ||||
synchronized (this) { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might be useful if you want the trails of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this will report all (most? any?) redirects - because at least some types of redirect will be 'silently' handled inside the media/libraries/datasource/src/main/java/androidx/media3/datasource/DefaultHttpDataSource.java Line 594 in 0480bc3
So I think we need to be quite careful we can usefully describe what this field holds, because i think it will only show retries that happened at this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is certainly true for We'll have a look and let you know, definitely preference would be to do it in the FWIW this is the same as how other Definitely something to look into. |
||||
if (!isReleased) { | ||||
sendMessageDelayed(Message.obtain(originalMsg), retryDelayMs); | ||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably just check for
mode != DefaultDrmSessionManager.MODE_RELEASE
. That way we cover download requests too.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I think we should probably include release calls too, since they can theoretically also take time right?
In any case, we should indicate what type of request is being reported in
KeyLoadInfo
. But not sure exactly how to do that.@Mode
is onDefaultDrmSessionManager
so we can't really reference it from a 'generic' DRM context. Maybe@ExoMediaDrm.KeyType
would work instead (though it feels kinda low-level).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that sounds like a good idea. We can start a list for what to add to
KeyLoadInfo
here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is done in the latest commit, except for adding the
KeyLoadInfo
to thedrmKeysRemoved()
method. I'm fine with doing this too, however then we should renameKeyLoadInfo
toKeyRequestInfo
. I'm all for the YAGNI thinking too ;-)But, seems most of the work will be rebasing all the files open for this change so it is probably now or never.
I'll follow your lead on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think we should do this rename and include key releasing - I agree it's going to be easier to do this now than later.