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

DrmEngine#fillInDrmInfoDefaults_ error logging is misleading #1284

Closed
chrisfillmore opened this issue Feb 8, 2018 · 5 comments
Closed

DrmEngine#fillInDrmInfoDefaults_ error logging is misleading #1284

chrisfillmore opened this issue Feb 8, 2018 · 5 comments
Assignees
Labels
status: archived Archived and locked; will not be updated type: enhancement New feature or request
Milestone

Comments

@chrisfillmore
Copy link
Contributor

chrisfillmore commented Feb 8, 2018

This is very minor but has come up recently in our work on Tizen.

Tizen supports both Widevine and PlayReady, and Shaka's behaviour is to pick the first supported keySystem based on the order of the ContentProtection blocks in the manifest (if I recall correctly). However, we have a business rule which says "Tizen shall use PlayReady", so in order to force this, we only configure a license server for com.microsoft.playready. This will cause Shaka to log an error, since there is a DrmInfo for Widevine, but no license server configured:

shaka.log.error('No license server configured for ' + keySystem);

I think this logging is misleading since it's not an error. At most it could be a warning but even that seems like a stretch, since this seems like an ordinary use case to me. I imagine that clients would never want to leave the selection of keySystem to chance. They want to know definitively what DRM they're using. (We encountered this problem on Tizen because the order of the ContentProtection blocks is different for our cloud DVR content than the rest of our content.)

I can submit a PR for this but I'd like to know if you think it could be changed to a warning, or removed altogether, or if you disagree with this assessment.

@theodab theodab added needs triage type: enhancement New feature or request and removed needs triage labels Feb 8, 2018
@theodab
Copy link
Collaborator

theodab commented Feb 8, 2018

This sounds reasonable to me. An info log, maybe with an additional warning if the variant has DrmInfos but every single one of them ends up with no licenseServerUri. What's your opinion, @joeyparrish?

@chrisfillmore
Copy link
Contributor Author

an additional warning if the variant has DrmInfos but every single one of them ends up with no licenseServerUri

This sounds like an actual error scenario to me, though I'm not sure if Shaka should fail with an assertion or publish a shaka.util.Error.

But perhaps there are some edge cases where clients know that some content will have ContentProtection blocks in the manifest, but the content itself is clear, so they don't provide license urls?

@TheModMaker
Copy link
Contributor

We already throw an error if we don't have a license server URL for the chosen key system, so maybe the log isn't needed. We also need to change our code so on multi-DRM platforms we pick the one with a license server given. I think we will still choose the first and error out if we don't have a license server for it.

@joeyparrish
Copy link
Member

I believe we try the key systems with license servers before the others. If no license-server-configured key systems work, we only try the others so we can give a more useful error message (no license server vs unsupported config). This is from memory, though. I could be wrong.

I think misleading error messages should be removed. If there is a usable key system with a configured license server, there is no reason for a "no license server" error message to appear.

@joeyparrish
Copy link
Member

Cherry-picked for v2.3.3.

@shaka-project shaka-project locked and limited conversation to collaborators Apr 21, 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
status: archived Archived and locked; will not be updated type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants