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

feat: add serverCertificateUri in DRM advanced config #3358

Merged

Conversation

valotvince
Copy link
Contributor

@valotvince valotvince commented Apr 21, 2021

Description

Fixes #1906

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to
    not work as expected)
  • This change requires a documentation update

Checklist:

  • I have signed the Google CLA https://cla.developers.google.com
  • My code follows the style guidelines of this project
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have verified my change on multiple browsers on different platforms
  • I have run ./build/all.py and the build passes
  • I have run ./build/test.py and all tests pass

Works when running alone but doesn't when running with all the other tests (same thing on master)

Chrome 89.0.4389.128 (Mac OS 10.15.7) Offline stores, plays, and deletes protected content with a persistent license FAILED

@valotvince valotvince marked this pull request as ready for review April 22, 2021 08:29
.addLicenseServer('com.widevine.alpha', 'https://cwip-shaka-proxy.appspot.com/no_auth'),
.addLicenseServer('com.widevine.alpha', 'https://cwip-shaka-proxy.appspot.com/no_auth')
.setExtraConfig({drm: {advanced: {'com.widevine.alpha': {serverCertificateUri:
'https://storage.googleapis.com/wvmedia/cert/cert_license_widevine_com_uat.bin'}}}}),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't really needed as the CDM will request it for us anyway. I'd suggest dropping the demo changes to make things simpler.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, it was just to have a working example with the serverCertificateUri. I could drop it or comment to explain this shouldn't be needed, but it is here to demonstrate the feature, WYT ?

@valotvince valotvince force-pushed the feat-server-certificate-uri branch from 7360ea4 to 74c86a9 Compare April 30, 2021 09:01
@valotvince
Copy link
Contributor Author

@TheModMaker 👋 I've pushed some edits base on your review, thanks !

@shaka-bot
Copy link
Collaborator

Test Failure:

Generating Closure dependencies...
Linting JavaScript...

/var/lib/jenkins/workspace/Manual PR Test (local-tests)/test/media/drm_engine_unit.js
1040:1  error  Expected indentation of 10 spaces but found 8  indent
1041:1  error  Expected indentation of 10 spaces but found 8  indent

✖ 2 problems (2 errors, 0 warnings)
2 errors and 0 warnings potentially fixable with the `--fix` option.

END-BUILD: FAILURE
Build step 'Execute shell' marked build as failure

@shaka-bot
Copy link
Collaborator

All tests passed!

@joeyparrish joeyparrish merged commit 9b4502c into shaka-project:master May 3, 2021
@valotvince valotvince deleted the feat-server-certificate-uri branch May 4, 2021 14:28
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jul 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2023
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change DRM config for serverCertificate to serverCertificateUri
4 participants