Skip to content
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

How can we know if the license request was successful, and the CDM accepted the response? #1035

Closed
chrisfillmore opened this issue Sep 27, 2017 · 10 comments
Assignees
Labels
flag: good first issue This might be a relatively easy issue; good for new contributors flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this status: archived Archived and locked; will not be updated type: enhancement New feature or request
Milestone

Comments

@chrisfillmore
Copy link
Contributor

chrisfillmore commented Sep 27, 2017

I have a requirement to signal to our downstream clients that the license request (and CDM update) was successful and we are able to play content.

// drm_engine
return session.update(response.data).then(function() {
  // I need to know this has happened
  if (updatePromise)
    updatePromise.resolve();
});

Could Shaka signal that the call to session.update() was fulfilled?

@chrisfillmore chrisfillmore changed the title How can we know if the license request was successful? How can we know if the license request was successful, and the CDM accepted the response? Sep 28, 2017
@michellezhuogg michellezhuogg added the type: enhancement New feature or request label Sep 29, 2017
@michellezhuogg
Copy link
Contributor

@joeyparrish Could you take a look to see if we should add the feature?

@joeyparrish
Copy link
Member

I see no reason we couldn't add an informative event for this. Adding to v2.3, with the caveat that it will skip to v2.4 if we don't have time.

@joeyparrish joeyparrish added this to the v2.3.0 milestone Sep 30, 2017
@joeyparrish joeyparrish added the flag: good first issue This might be a relatively easy issue; good for new contributors label Sep 30, 2017
@chrisfillmore
Copy link
Contributor Author

chrisfillmore commented Oct 3, 2017

@joeyparrish I can make a PR for this since I'm supposed to deliver this feature, not only for Shaka but also for our Safari player implementation. Here's what I had in mind, can you advise:

  • Provide the player's onEvent callback to new shaka.media.DrmEngine(...)
  • Invoke onEvent with a FakeEvent when the call to session.update() is fulfilled
  • I was thinking of giving the event a type of keysessionupdate

Only thing I noticed was the signature for new shaka.media.DrmEngine(...) is getting a little long, and I see in other places you use a playerInterface. Would you suggest using a playerInterface in this case?

@joeyparrish
Copy link
Member

Yes, absolutely. DrmEngine is not directly accessible to apps through our exported API, so there's no compatibility concerns there. A Player interface object makes sense.

@joeyparrish
Copy link
Member

I'm not sure I like the event name "keysessionupdate", though. It doesn't match current EME terminology, as we don't call them "key sessions".

How about "licenseupdate", "drmsessionupdate", or "drmupdate"?

@joeyparrish joeyparrish added the flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this label Oct 3, 2017
@chrisfillmore
Copy link
Contributor Author

I'm about done the work, and running the tests now I see this fail on master:

Safari 11.0.0 (Mac OS X 10.12.6) MediaSourceEngine removes segments FAILED
	Expected 15 to be close to 10.
	test/media/media_source_engine_integration.js:142:57
	promiseReactionJob@[native code]
	Expected 15 to be close to 20.
	test/media/media_source_engine_integration.js:143:58
	promiseReactionJob@[native code]
	Expected 25 to be 20.
	test/media/media_source_engine_integration.js:146:50
	promiseReactionJob@[native code]
	Expected 5 to be close to 10.
	test/media/media_source_engine_integration.js:147:58
	promiseReactionJob@[native code]

This is without my changes.

On the branch with my changes, in my most recent test run, I see this failure too:

Safari 11.0.0 (Mac OS X 10.12.6) MediaSourceEngine trims content at appendWindowEnd FAILED
	Expected 6.916666666666667 to be close to 10.
	http://localhost:9877test/media/media_source_engine_integration.js:332:57
	promiseReactionJob@[native code]
	Expected 6.916666666666667 to be close to 18, 1.
	http://localhost:9877test/media/media_source_engine_integration.js:335:57
	promiseReactionJob@[native code]

But I've run the tests a few times and I think I see varying behaviour out of some integration tests. Do you usually see 100% success rate across browsers for integration tests?

@chrisfillmore
Copy link
Contributor Author

Nevermind I see you just posted about this in #1048.

@joeyparrish
Copy link
Member

Yeah, I have the Safari 11 issues fixed locally with a new polyfill, but I am still in the process of writing a bug report to Apple about the remove failure.

@joeyparrish
Copy link
Member

New MediaSource bug filed against Safari 11: https://bugs.webkit.org/show_bug.cgi?id=177884

@joeyparrish
Copy link
Member

Cherry-picked to v2.2.3.

@shaka-project shaka-project locked and limited conversation to collaborators Mar 22, 2018
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
flag: good first issue This might be a relatively easy issue; good for new contributors flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this status: archived Archived and locked; will not be updated type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants