Adds load info to `DrmSessionEventListeneronDrmKeysLoaded()`

[stevemayhew]

This is a first pass at what we talked about in issue https://github.com/androidx/media/issues/1001

• added the wrapper class KeyLoadInfo to embody the requests required to fetch the key
• added KeyLoadInfo to the DrmSessionEventListener.drmKeysLoaded() method
• Deprecated DrmSessionEventListener.onDrmKeysLoaded(int, MediaPeriodId)
• added new onDrmKeysLoaded() with the KeyLoadInfo, setup so both method are called

Next steps are to add it to AnalyticsListener and update any broken test cases possibly add some new ones

[stevemayhew]

Unless we want to go for renaming KeyLoadInfo to KeyRequestInfo and moving forward with the drmKeysRemoved() update too... I would squash the commits and run the code beautifier on what we have.

I'm good either way.

[stevemayhew]

No problem, we both have day jobs too!

I’ve assigned some tasks here internally to move our most important pull requests over from ExoPlayer to AndroidX

On Tue, Feb 27, 2024 at 1:05 AM Ian Baker @.***> wrote:

@.**** requested changes on this pull request.

In libraries/exoplayer/src/main/java/androidx/media3/exoplayer/drm/KeyLoadInfo.java https://github.com/androidx/media/pull/1134#discussion_r1503840234:

@@ -0,0 +1,26 @@ +package androidx.media3.exoplayer.drm;

+import androidx.media3.exoplayer.source.LoadEventInfo; +import java.util.ArrayList; +import java.util.List; + +/**

• • Encapsulates info for the sequence of load requests @.*** LoadEventInfo}, which were required
• • to complete loading a DRM key
• */ +public class KeyLoadInfo {

I think we should make this immutable. If we need the addRetry accumulation logic here then it should be part of a KeyLoadInfo.Builder, but maybe we don't need that and can just accumulate in a local in DefaultDrmSession.

In libraries/exoplayer/src/main/java/androidx/media3/exoplayer/drm/DefaultDrmSession.java https://github.com/androidx/media/pull/1134#discussion_r1503864725:

@@ -644,6 +651,19 @@ public void handleMessage(Message msg) { break; case MSG_KEYS: response = callback.executeKeyRequest(uuid, (KeyRequest) requestTask.request);

•        if (currentKeyLoadInfo != null) {
  

•          String requestUrl = ((KeyRequest) requestTask.request).getLicenseServerUrl();
  

•          LoadEventInfo loadEventInfo =
  

•              new LoadEventInfo(
  

•                  requestTask.taskId,
  

•                  new DataSpec.Builder().setUri(requestUrl).build(),
  

•                  Uri.parse(requestUrl),
  

This is also meant to the be the URI after redirects, so we need to be careful here too.

In libraries/exoplayer/src/main/java/androidx/media3/exoplayer/drm/DefaultDrmSession.java https://github.com/androidx/media/pull/1134#discussion_r1503874316:

@@ -644,6 +651,19 @@ public void handleMessage(Message msg) { break; case MSG_KEYS: response = callback.executeKeyRequest(uuid, (KeyRequest) requestTask.request);

•        if (currentKeyLoadInfo != null) {
  

•          String requestUrl = ((KeyRequest) requestTask.request).getLicenseServerUrl();
  

Hmmm, i wonder if there's another approach here where we add plumbing to pass a TransferListener into HttpMediaDrmCallback which then passes it on to the underlying DataSource impl. That would give you access to the 'real' URL (including redirects handled inside the DataSource as I flagged below).

Downsides of this approach:

1. Not (easily) compatible with LocalMediaDrmCallback and other MediaDrmCallback impls. So what would we do if we didn't have an instance of HttpMediaDrmCallback?
2. It leaks the DataSource instance back out to DefaultDrmSession (probably not that big a deal).

---

Our use case does not care about the URL, simply the load times and retries. So either way is good.

Or maybe we can skip URL for now then (in the spirit of YAGNI https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it). That means we can't use LoadEventInfo. What is the minimal set of fields you need? I feel we should include some way to identify which DRM keys are being loaded - maybe the @Nullable List<SchemeData> that we pass into ExoMediaDrm.getKeyRequest to generate the request bytes?

In libraries/exoplayer/src/main/java/androidx/media3/exoplayer/drm/DefaultDrmSession.java https://github.com/androidx/media/pull/1134#discussion_r1503851859:

@@ -699,6 +719,7 @@ private boolean maybeRetryRequest(Message originalMsg, MediaDrmCallbackException // The error is fatal. return false; }

•  currentKeyLoadInfo.addRetryLoadRequest(loadEventInfo);
   synchronized (this) {
  

I don't think this will report all (most? any?) redirects - because at least some types of redirect will be 'silently' handled inside the HttpDataSource implementation underneath HttpMediaDrmCallback, e.g. https://github.com/androidx/media/blob/0480bc31a8fbec2aed354bee14e8f15e7fe9012a/libraries/datasource/src/main/java/androidx/media3/datasource/DefaultHttpDataSource.java#L594

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 DefaultDrmSession layer.

In libraries/exoplayer/src/main/java/androidx/media3/exoplayer/drm/DefaultDrmSession.java https://github.com/androidx/media/pull/1134#discussion_r1503841193:

@@ -489,6 +492,9 @@ private long getLicenseDurationRemainingSec() {

private void postKeyRequest(byte[] scope, int type, boolean allowRetry) { try {

•  if (type == ExoMediaDrm.KEY_TYPE_STREAMING) {
  

•    currentKeyLoadInfo = new KeyLoadInfo();
  

•  }
  

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 on DefaultDrmSessionManager so we can't really reference it from a 'generic' DRM context. Maybe @ExoMediaDrm.KeyType would work instead (though it feels kinda low-level).

— Reply to this email directly, view it on GitHub https://github.com/androidx/media/pull/1134#pullrequestreview-1902860335, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADBF6GK7DWFHTJHJRPHCJDYVWOVZAVCNFSM6AAAAABD24S3JCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSMBSHA3DAMZTGU . You are receiving this because you authored the thread.Message ID: @.***>

[stevemayhew]

I'm on vacation next week @icbaker so it will take a bit to get to these changes. Only one I need your input on is best path for the taskId