-
Notifications
You must be signed in to change notification settings - Fork 426
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
feat: setup EME key systems for HLS as well as DASH #629
Conversation
…, change var names
return; | ||
} | ||
const mainSegmentLoader = hlsHandler.masterPlaylistController_.mainSegmentLoader_; | ||
const audioSegmentLoader = hlsHandler.masterPlaylistController_.audioSegmentLoader_; |
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.
Does this work for both muxed and umuxed content?
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.
What do you mean by "this"?
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.
The current changes really. Specifically, with muxed audio, we don't have a separate audio segment loader. We should make sure that this works in either case, or at least, doesn't break existing streams.
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.
Oh, I guess it's unlikely to break existing streams because dash is always unmuxed.
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.
If the content is muxed, then audioSegmentLoader
is null
and emeKeySystems()
will pull the relevant audio/video codec info from the mainSegmentLoader.mimeType_
and format it into separate audio/video mime types accordingly. If it is unmuxed, then the audio/video codec info has already separated out and formatted by configureLoaderMimeTypes_()
into the exact format we need to setup eme.
@@ -3109,7 +3109,7 @@ QUnit.skip('populates quality levels list when available', function(assert) { | |||
assert.ok(this.player.tech_.hls.qualityLevels_, 'added quality levels from video with source'); | |||
}); | |||
|
|||
QUnit.test('configures eme if present on selectedinitialmedia', function(assert) { | |||
QUnit.test('configures eme for DASH if present on selectedinitialmedia', function(assert) { |
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.
the changes in this test is due to how we're setting up EME now? Is it possible to make it so that the test doesn't change?
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.
I think the mocking in this test does need to be updated since the old one mocked objects that are no longer used to setup eme
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.
approved pending testing
@@ -140,18 +141,54 @@ Hls.canPlaySource = function() { | |||
'your player\'s techOrder.'); | |||
}; | |||
|
|||
const emeKeySystems = (keySystemOptions, videoPlaylist, audioPlaylist) => { | |||
const emeKeySystems = (keySystemOptions, mainSegmentLoader, audioSegmentLoader) => { |
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.
Since mainSegmentLoader and audioSegmentLoader are only used for their mimeType_ properties and the mainSegmentLoader's playlist for contentProtection, it might simplify the function to just accept those as parameters (audioMimeType, videoMimeType, and contentProtection). Can even make a function with the first part of this function's logic to do the initial work of getting the mime types from the loaders and then pass the result in directly.
// from the mainSegmentLoader mimeType (ex. 'video/mp4; codecs="mp4, avc1"' --> | ||
// 'video/mp4; codecs="avc1"' and 'audio/mp4; codecs="mp4"') | ||
} else { | ||
const parsedMimeType = parseContentType(mainSegmentLoader.mimeType_); |
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.
I don't know if we will see this behavior in the wild, but it is possible that, with setupEmeOptions
called on selectedinitialmedia
, if the source buffer's readyState
is not yet open
, then the mime types for the loaders will not be configured, and we may get an exception.
}); | ||
|
||
videoMimeType = `${parsedMimeType.type}; codecs="${videoCodec}"`; | ||
audioMimeType = `${parsedMimeType.type.replace('video', 'audio')}; codecs="${audioCodec}"`; |
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.
We must have gotten away with it before because we haven't seen DRM protected audio/video only streams, but we may want to consider that possibility.
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, we did just recently get a question about that. I think, for now, we can continue not supporting it, but definitely something we should fix eventually.
} | ||
}, | ||
'added content types'); | ||
|
||
// unmuxed content |
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.
minor, but may want to use "demuxed"
Description
Currently, VHS augments the EME key systems of DASH sources with DRM-related information from the manifest. In order to support Widevine HLS content, we will need to expand this key system handling to work for HLS content as well. Since the existing logic is based on the assumption that we are dealing with DASH, and thus unmuxed video and audio, some tweaks are needed in order to account for the fact that HLS can be either muxed or unmuxed.
Specific Changes proposed
mainSegmentLoader.mimeType_
andaudioSegmentLoader.mimeType_
instead of playlist attributes, since we already do some of the work needed to format those strings for EME when we initially setup the source buffers. The one exception to that is muxed HLS, where some string manipulation is still required to parse out the individual codecs from a comma-delimited list.emeKeySystems()
functionopenMediaSource()
call to one eme integration test so segment loader mimeTypes will be set