Fix/multiple temporary drm session #1185
Conversation
| const keyId = tenc.subarray(8, 24); | ||
| return isValidKeyId(keyId) | ||
| ? keyId | ||
| : null; |
There was a problem hiding this comment.
is there a resource that tells that a all 0 key id is invalid?
| * @returns {Boolean} True if value equals zero | ||
| */ | ||
| function isZero(value: number) { | ||
| return value === 0; |
There was a problem hiding this comment.
This function's utility looks too small to me to justify creating a function
| @@ -148,8 +148,8 @@ export default class KeySessionRecord { | |||
| if (this._keyIds !== null && areAllKeyIdsContainedIn(keyIds, this._keyIds)) { | |||
| return true; | |||
| } | |||
| if (this._initializationData.keyIds !== undefined) { | |||
| return areAllKeyIdsContainedIn(keyIds, this._initializationData.keyIds); | |||
| if (this._initializationData.keyIds !== undefined && areAllKeyIdsContainedIn(keyIds, this._initializationData.keyIds)) { | |||
There was a problem hiding this comment.
To me, if a wanted key id is not linked to a session, then the license has to be loaded, else we would be breaking legitimate use cases.
| { | ||
| return areAllKeyIdsContainedIn(initializationData.keyIds, | ||
| this._initializationData.keyIds); | ||
| } |
There was a problem hiding this comment.
If we're removing this, we may want to rename the method.
Maybe the best thing would be to inline the method altogether as moreover it seems to be used only once?
There was a problem hiding this comment.
I have some reserves with this PR:
- Is an all
0key-id guaranteed to be invalid? Ignoring some key ids seems dangerous to me. - To me if key ids are known both for the current initialization data and for the one associated to a
KeySessionRecord, then it should be the preferred token to determine if aMediaKeySessionalready handles a key (through the KeySessionRecord). Comparing what here is called initializationData values, which are actually the full pssh, seems less precize to me. If the issue was with the former logic, then it may be a bug that I would prefer to fix instead.
|
Closing this as it is an old stale PR (I'm cleaning them up). Do not hesitate to re-open if you have updates. |
On our live DASH + Widevine streams, the player creates many temporary DRM sessions and so trigger as many license resquest to our Widevine server.
This pull request tend to fix 2 issues.
tencbox to test it against our current DRM session's keyIds. In our streams, this keyId is filled with zero and will not be compatible with our current session's keyIds.KeySessionRecord.isCompatibleWith, theinitializationData.valueswas never tested because testinginitializationData.keysreturnsfalsebefore.Note:
_checkInitializationDataCompatibilityas I am not sure how useful it is.initializationData.keystest, the zero keyId does not fail the DRM session keyId tests. So I am not sure if we still want to key this test or consider the zero keyId as validFix #1183