feat: allow reuse of persistent license sessions by valotvince · Pull Request #4461 · shaka-project/shaka-player

valotvince · 2022-08-31T15:54:50Z

Add capability to re-use persistent license sessions across sessions.

DrmEngine will now always:

try to start stored persistent sessions before trying to fetch a license, as-to be able to check if all needed keys are already loaded.
ask for a new license when the persistent session doesn't have the needed keys for playback,

Given the flag persistentSessionOnlinePlayback is true, DrmEngine:

won't remove the persistent session from the device at the end of the playback,
won't throw an error when the persistent session isn't found on the device,

For now, it needs Shaka's users to persist session information by themselves (localStorage, IndexDB, ...) before giving it back for the next session. Still, it lays foundation to develop the feature to fully handling it on Shaka's side.

TBD:

Rename all offlineSessionIds_ in DrmEngine to storedPersistentSessionIds_ because we now use those in two different contexts, online / offline (and in comments too) (after full review to avoid noise over the feature)
Rename loadOfflineSession_ in DrmEngine to loadStoredPersistentSession_ (after full review to avoid noise over the feature)
Add a public API to retrieve the session (ids and expiration) from DRM Engine if not already available
Refactor / Set up another way to give persistentSessionIds to also give associated initData with it
Test full offline to ensure no regressions
Document feature usage

Related to #1956

valotvince · 2022-08-31T15:59:01Z

+        }
      }

-      // TODO: We should get a key status change event.  Remove once Chrome CDM


ℹ️ Worked like a charm on Chrome, I might need to test the full offline capabilities

Downloaded Widevine DRM content worked great with Chrome ;)

What happens in old TV browsers?

Haven't tested yet the persistent licenses on TV but will do

Tizen 6.5

It correctly starts the persistent-license session at first launch but fails on the session load at second launch, so my guess is that this isn't supported.
The sessionId is always the same initialdrmsessionid so I don't think it could work at all.

Shaka Error DRM.FAILED_TO_CREATE_SESSION (Failed to execute 'load' on 'MediaKeySession': Loading session failed. . (2))

webOS 5.0

It fails to request a media key system access when asking for a persistent-license with the error REQUESTED_KEY_SYSTEM_CONFIG_UNAVAILABLE, so it seems it doesn't support persistent license at all

So my overall conclusion is persistent license isn't supported on TV devices, so I don't think we need to go further into making this feature working on those devices.

Error Recoverability

It pointed me out on an issue with my code where fails to load a persistent license will stop the player, rather than trying to fetch a new one (intended behaviour).

I have two options for that, let me know what you prefer (my pref is option 1):

Option 1: Send a RECOVERABLE error

this.onError_(new shaka.util.Error( this.config_.persistentSessionOnlinePlayback ? shaka.util.Error.Severity.RECOVERABLE : shaka.util.Error.Severity.CRITICAL; shaka.util.Error.Category.DRM, shaka.util.Error.Code.FAILED_TO_CREATE_SESSION, error.message));

Option 2: Bypass the error, only log something:

if (this.config_.persistentSessionOnlinePlayback) { shaka.log.warning('Persistent session', sessionId, 'not available'); } else { this.onError_(new shaka.util.Error( shaka.util.Error.Severity.CRITICAL, shaka.util.Error.Category.DRM, shaka.util.Error.Code.OFFLINE_SESSION_REMOVED)); return Promise.resolve(); }

github-actions · 2022-09-01T09:05:10Z

Incremental code coverage: 90.85%

avelad · 2022-09-01T11:10:05Z

Related to #1956

caridley · 2022-09-01T15:07:38Z

   */
-  createOrLoad() {
-    // Create temp sessions.
+  async createOrLoad() {


I noticed that the attach(video) function calls createOrLoad() but does not wait for the returned promise to resolve. Should it wait?

I think it was done on purpose so that creating / loading sessions could be done in parallel of fetching manifest information, segments and more.

avelad

In general, LGTM, but I think we should give the possibility (player configuration) that ShakaPlayer itself stores this information in localStorage for example.

avelad · 2022-10-09T07:14:04Z

@valotvince it seems that there are some conflict with the main branch, can you rebase? Thanks!

valotvince · 2022-11-14T09:21:46Z

👋 @avelad @joeyparrish I've rebased on main & added documentation about the feature, could you review it again ? Thanks :)

I think we should give the possibility (player configuration) that ShakaPlayer itself stores this information in localStorage for example.

If that's possible I'd like it to be done in another PR, as localStorage (or Indexeddb) is not necessarily the best on some devices. It might take more reflexion on the whole storage capabilities of Shaka, like allowing to give a StorageManager like the ABRManager which would handle the storage, without Shaka's knowing what the storage is.

valotvince · 2022-12-15T06:42:36Z

Hi @avelad @joeyparrish, any comments to share on this PR ? Thanks !

theodab · 2023-01-18T08:02:32Z

+ * }}
+ *
+ * @description
+ * DRM Session Metadata for as session


The JSDoc for this is the same as DrmSessionMetadata, but the two represent different types of data.

From what I gather, PersistentSessionMetadata is the type that the user passes in, representing an existing persistent session. Meanwhile, DrmSessionMetadata represents an active session, which may have come from this playback or have been loaded from a persistent session.

Correct?

You are correct ! :)

theodab · 2023-01-18T08:56:14Z

-            shaka.util.Error.Severity.CRITICAL,
+            severity,
            shaka.util.Error.Category.DRM,
            shaka.util.Error.Code.OFFLINE_SESSION_REMOVED));


The JSDoc for OFFLINE_SESSION_REMOVED should probably be updated. Right now it specifically says "The content is not playable", which is no longer going to be true if this error will sometimes be recoverable.

I'll change the description of the error to:

A required offline session was removed. The content might not be playable depending of the playback context.

theodab · 2023-01-18T09:06:08Z

-      metadata.loaded = true;
-      if (this.areAllSessionsLoaded_()) {
-        this.allSessionsLoaded_.resolve();
+        metadata.loaded = true;


Do we really need to set metadata.loaded to true here?
All that value does is determine when areAllSessionsLoaded_ returns true. So if we are manually setting this to true for each stream we call loadOfflineSession_ on, and we call createOrLoad on each session at once inside createOrLoad, doesn't that mean that we will just immediately resolve this.allSessionsLoaded_ when the last session begins to load? That seems like it defeats the purpose of waiting for the key status change event, if we're still resolving before the event fires.

We only set metadata.loaded when the session is not present anymore (we will never receive a keystatuschange event), and send a OFFLINE_SESSION_REMOVED event.

If other sessions are present & correctly loaded, we wait for the keystatuschange to set the session as loaded.

theodab · 2023-01-18T09:08:22Z

+      expect(session2.generateRequest).not.toHaveBeenCalled();
+    });
+
+    it(


I think this would fit into one line. No need to do the weird indenting.

The whole line

it('tries persistent session ids before requesting a license', async () => {

is 81 characters long 😞

avelad

LGTM, can you rebase and resolve the conflicts? Thanks!

valotvince · 2023-04-13T11:47:16Z

@avelad 👋 Rebase done !

joeyparrish · 2023-04-19T17:06:06Z

+});
+```
+
+NB: Shaka doesn't provide a out-of-the-box storage mechanism for the sessions


There is such a mechanism in the offline/ codebase. We could discuss a follow-up PR to use that mechanism if/when offline support is included.

stale

joeyparrish · 2023-04-19T17:11:03Z

Failed tests look like unrelated flakiness, but I'm re-running them just to be sure.

valotvince commented Aug 31, 2022

View reviewed changes

avelad added type: enhancement New feature or request component: EME The issue involves the Encrypted Media Extensions web API labels Aug 31, 2022

avelad requested review from avelad, joeyparrish and theodab and removed request for joeyparrish August 31, 2022 16:53

valotvince marked this pull request as ready for review September 1, 2022 09:54

caridley reviewed Sep 1, 2022

View reviewed changes

avelad requested changes Sep 6, 2022

View reviewed changes

Comment thread externs/shaka/manifest.js

avelad added the priority: P2 Smaller impact or easy workaround label Sep 26, 2022

valotvince force-pushed the drm-online-persistent-license branch from da2d158 to e00ec59 Compare October 11, 2022 07:14

RoLfBOT mentioned this pull request Oct 13, 2022

Persistent license w/ streaming content #1956

Closed

valotvince force-pushed the drm-online-persistent-license branch from c1880a2 to 8ee4eba Compare November 14, 2022 08:47

theodab previously requested changes Jan 18, 2023

View reviewed changes

valotvince requested review from theodab and removed request for joeyparrish January 23, 2023 10:00

avelad requested review from avelad and joeyparrish February 3, 2023 11:15

avelad requested changes Mar 21, 2023

View reviewed changes

valotvince added 4 commits April 13, 2023 10:52

feat: allow reuse of persistent license sessions

6462f45

add public api to get drm sessions metadata

1b9740e

add demo config exceptions

87bcc43

silence createOrLoad unhandled rejections in attach

c5fa38d

valotvince added 4 commits April 13, 2023 10:52

revert line removal

f235dac

add documentation

3170832

recoverable errors

fc8cd6c

review

5f8bf6f

valotvince force-pushed the drm-online-persistent-license branch from f0e9fec to 5f8bf6f Compare April 13, 2023 08:53

remove useless typedef

934cd66

avelad approved these changes Apr 18, 2023

View reviewed changes

joeyparrish approved these changes Apr 19, 2023

View reviewed changes

joeyparrish merged commit cc97da1 into shaka-project:main Apr 19, 2023

github-actions Bot mentioned this pull request Apr 19, 2023

chore(main): release 4.4.0 #4692

Merged

valotvince deleted the drm-online-persistent-license branch June 23, 2023 14:04

github-actions Bot added the status: archived Archived and locked; will not be updated label Jul 25, 2023

github-actions Bot locked as resolved and limited conversation to collaborators Jul 25, 2023

Conversation

valotvince commented Aug 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Tizen 6.5

webOS 5.0

Error Recoverability

Uh oh!

Uh oh!

github-actions Bot commented Sep 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

avelad commented Sep 1, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avelad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

avelad commented Oct 9, 2022

Uh oh!

valotvince commented Nov 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

valotvince commented Dec 15, 2022

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

avelad left a comment

Choose a reason for hiding this comment

Uh oh!

valotvince commented Apr 13, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joeyparrish commented Apr 19, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

valotvince commented Aug 31, 2022 •

edited

Loading

github-actions Bot commented Sep 1, 2022 •

edited

Loading

valotvince commented Nov 14, 2022 •

edited

Loading