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

Pull request for issue #780 - Support for VDMS Streams in demo app #781

Merged
merged 3 commits into from
May 9, 2017

Conversation

TheJohnBowers
Copy link
Contributor

I have added a response filter for VDMS manifest responses, as well as a license request filter for VDMS license requests. I then add a couple of test assets that utilize these filters and point to the VDMS widevine and clearkey license proxies.

license proxy servers.  Add two test assets that use these license
proxies.  The first is simple single period test asset.  The 2nd
is a multi period asset that is interspersed with unencrypted ad
periods.
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@TheJohnBowers
Copy link
Contributor Author

The corporate CLA has been signed according to my knowledge, but it was initiated by Kris Varro in March. I believe our corporate CLA should have been signed and accepted as of March 19th.

@joeyparrish
Copy link
Member

@TheJohnBowers, I can see that there is a corporate CLA on file for Verizon Digital Media Services. So the CLA bot must be mistaken. I will report this to the maintainers of that bot and we can proceed.

@joeyparrish joeyparrish self-assigned this May 5, 2017
@joeyparrish joeyparrish added the type: enhancement New feature or request label May 5, 2017
@joeyparrish joeyparrish added this to the v2.2.0 milestone May 5, 2017
@joeyparrish
Copy link
Member

The CLA bot maintainers have informed me that you need to get yourself added to a group that associates you with the corporate CLA. The maintainer of the group at Verizon Digital Media Services is Daniel Sanders. When you have been added, just comment on the PR and the bot will re-check the group.

@TheJohnBowers
Copy link
Contributor Author

Ok, I believe I have been added to the corporate CLA now.

@googlebot
Copy link

CLAs look good, thanks!

demo/assets.js Outdated
@@ -1097,6 +1141,46 @@ shakaAssets.testAssets = [
shakaAssets.Feature.MP4,
shakaAssets.Feature.SEGMENT_TEMPLATE_DURATION
]
},
{
Copy link
Member

Choose a reason for hiding this comment

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

Please enclose these assets in their own group. For example, see line 1038 above, where we start a group for GPAC-sourced assets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

demo/assets.js Outdated

source: shakaAssets.Source.UPLYNK,
drm: [
shakaAssets.KeySystem.WIDEVINE
Copy link
Member

Choose a reason for hiding this comment

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

Add shakaAssets.KeySystem.CLEAR_KEY to this list, as well. Same below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

demo/assets.js Outdated
name: 'Big Buck Bunny',
manifestUri: 'https://content.uplynk.com/224ac8717e714b68831997ab6cea4015.mpd', // gjslint: disable=110

source: shakaAssets.Source.UPLYNK,
Copy link
Member

Choose a reason for hiding this comment

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

Please specify which encoder was used in the encoder field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done -- Added the "UPLYNK" encoder so I could specify that in these assets.

demo/assets.js Outdated
],
features: [
shakaAssets.Feature.MP4,
shakaAssets.Feature.SEGMENT_LIST_DURATION
Copy link
Member

Choose a reason for hiding this comment

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

Please add Feature.PSSH, Feature.HIGH_DEFINITION.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

demo/assets.js Outdated
],
features: [
shakaAssets.Feature.MP4,
shakaAssets.Feature.SEGMENT_LIST_DURATION
Copy link
Member

Choose a reason for hiding this comment

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

Please add Feature.PSSH, Feature.HIGH_DEFINITION, Feature.MULTIPERIOD.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done -- I also added shakaAssets.Feature.MULTIPLE_LANGUAGES as I was reviewing the features. This asset has a second Portuguese audio track. We could add a new feature for assets like this that have completely clear periods interspersed with encrypted periods too. I believe you haven't had many tests like this up to this point because of Chrome's issues supporting it until 58.

Add KeySystem.CLEAR_KEY to VDMS assets
Update VDMS assets to have an encoder
Update VDMS assets to list all relevant features
Copy link
Member

@joeyparrish joeyparrish left a comment

Choose a reason for hiding this comment

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

Looks good to me. If you want to add a Feature constant for mixing encrypted & unencrypted, I would be fine with that.

@shaka-bot
Copy link
Collaborator

Testing in progress...

@shaka-bot
Copy link
Collaborator

All tests passed!

@shaka-bot
Copy link
Collaborator

Testing in progress...

@shaka-bot
Copy link
Collaborator

All tests passed!

@joeyparrish joeyparrish merged commit 78059eb into shaka-project:master May 9, 2017
@joeyparrish
Copy link
Member

Thanks!

joeyparrish pushed a commit that referenced this pull request May 10, 2017
joeyparrish pushed a commit that referenced this pull request May 10, 2017
Closes #780

Backported to v2.0.x
@joeyparrish
Copy link
Member

Cherry-picked to v2.1.1 and v2.0.9.

@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 type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants