feat: support in-manifest DRM data #60
Conversation
src/videojs-http-streaming.js
Outdated
| @@ -168,11 +175,11 @@ const setupEmeOptions = (hlsHandler) => { | |||
| const player = videojs.players[hlsHandler.tech_.options_.playerId]; | |||
|
|
|||
| if (player.eme) { | |||
| player.eme.options = videojs.mergeOptions(player.eme.options, emeOptions( | |||
| player.eme.options = emeOptions( | |||
There was a problem hiding this comment.
Removed this because of the following scenario: given a playlist with two DRM sources, the first has in-manifest DRM info and the second does not, the first source's data would carry over into the second and prevent playback.
There was a problem hiding this comment.
In the long term we'll look into making contrib-eme a middleware
There was a problem hiding this comment.
I know we discussed the possibility of removing player.eme.options entirely, and I think we should go through with that on a major version of videojs-contrib-eme. However, I think we should be careful about completely replacing player.eme.options right now. With this behavior if someone had used the top level eme plugin options, they will all be overwritten. I think instead we should find a way to pass along the new options from VHS as part of the source. Maybe via modifying the source object itself.
There was a problem hiding this comment.
The source options merge with the plugin options in contrib-eme so it would behave the same way it was before where we merge them here
There was a problem hiding this comment.
Instead I'll default pssh: '' and do the merge
src/videojs-http-streaming.js
Outdated
| @@ -146,9 +146,17 @@ const emeOptions = (keySystemOptions, videoPlaylist, audioPlaylist) => { | |||
| for (let keySystem in keySystemOptions) { | |||
| keySystemContentTypes[keySystem] = { | |||
| audioContentType: `audio/mp4; codecs="${audioPlaylist.attributes.CODECS}"`, | |||
| videoContentType: `video/mp4; codecs="${videoPlaylist.attributes.CODECS}"` | |||
| videoContentType: `video/mp4; codecs="${videoPlaylist.attributes.CODECS}"`, | |||
| pssh: '' | |||
There was a problem hiding this comment.
Did we still want to default to an empty string for pssh now that we're overwriting the source options?
src/videojs-http-streaming.js
Outdated
| )); | ||
| ); | ||
|
|
||
| player.currentSource().keySystems = sourceOptions.keySystems; |
There was a problem hiding this comment.
This throws an error if hlsHandler.source_.keySystem is undefined. Might be good for us to check sourceOptions and add a test for the case where no keySystems options are passed. The emeOptions function can also return the keySystems on their own now (and we can probably rename the function to emeKeySystems, or something like that, if we want).
There was a problem hiding this comment.
👍 will make some updates
test/videojs-http-streaming.test.js
Outdated
| }, 'set source eme options'); | ||
| }); | ||
|
|
||
| QUnit.test('ignores configures eme if not provided by source', function(assert) { |
There was a problem hiding this comment.
minor: should probably read something like "does not set source keySystems if keySystems not provided by source"
This reverts commit add4e66.
Description
Added support for detecting in-manifest DRM data.
Requirements Checklist