Feature/restrictions #36
Conversation
| * @return {Object} restrictions Contains the current restrictions | ||
| * @export | ||
| */ | ||
| shaka.player.Player.prototype.getRestrictions = function() { |
There was a problem hiding this comment.
I like that this is a clone of the Restrictions object, but instead of returning a generic Object, let's return an instance of Restrictions instead. To this end, please add a clone() method to Restrictions.prototype.
There was a problem hiding this comment.
I like this idea. But just with respect to the issue below of replacing struct on Restrictions, the way I would like to implement the clone() method would be similar to this because rather than explicitly setting each property because then there is no need to modify this method if new restrictions are added at some point. This will prohibit me from replacing struct, even if the changes below are made. Am I right about this?
|
Overall, looks pretty good. If we change getRestrictions/setRestrictions a bit, the flow to update or add restrictions for the caller becomes: var r = player.getRestrictions(); This won't interfere with or replace any resolution constraints imposed by the license server, and the two restrictions can be set in any order. In fact, you could even have EmeManager request restrictions from the Player in the same way: var restrictions = player.getRestrictions(); Then player.setRestrictions doesn't have to update EmeManager and EmeManager doesn't need to keep a copy of restrictions. |
| @@ -44,7 +46,7 @@ goog.require('shaka.util.Uint8ArrayUtils'); | |||
| * @struct | |||
| * @extends {shaka.util.FakeEventTarget} | |||
| */ | |||
| shaka.media.EmeManager = function(parent, video, videoSource) { | |||
| shaka.media.EmeManager = function(parent, video, videoSource, restrictions) { | |||
There was a problem hiding this comment.
Let's make convert "parent" to "player" (type shaka.player.Player). We're already passing the player as the parent, but let's have EME manager store this.player_ instead of this.restrictions_.
Then, instead of postProcessor(this.restrictions_), this.videoSource_.setRestrictions(this.restrictions_), we can say restrictions = this.player_.getRestrictions(), postProcessor(restrictions), this.player_.setRestrictions(restrictions).
That will make the postProcessor flow and the independent, application-level flow the same.
| @@ -470,7 +473,7 @@ shaka.media.EmeManager.prototype.requestLicense_ = | |||
| function(response) { | |||
| shaka.log.info('onLicenseSuccess_', session); | |||
| if (postProcessor) { | |||
| var restrictions = new shaka.player.DrmSchemeInfo.Restrictions(); | |||
| var restrictions = this.player_.getRestrictions(); | |||
| response = postProcessor(response, restrictions); | |||
| this.videoSource_.setRestrictions(restrictions); | |||
There was a problem hiding this comment.
Should be this.player_.setRestrictions now.
|
Looks great. Thanks for taking the time to work on this! |
This resolves a compile-time conflict between changes to EmeManager (commit a4e9bc3, merge of pull request #36) and OfflineVideoSource (commit 18a843e), merged together in f204c8d to create a compiler error: offline_video_source.js:81: ERROR - actual parameter 1 of EmeManager does not match formal parameter found : shaka.player.OfflineVideoSource required: shaka.player.Player new shaka.media.EmeManager(this, fakeVideoElement, this); Change-Id: I6b263ef0e443bb305f96928b0d0c5019056d7014
github-actions
bot
added
the
status: archived
Updated pull request for #32 based on the discussion in #33
Several things worth discussing: