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

Caching and other efficiency improvements for mcap polyfill #4574

Closed
sky-hugolima opened this issue Oct 13, 2022 · 7 comments · Fixed by #4708
Closed

Caching and other efficiency improvements for mcap polyfill #4574

sky-hugolima opened this issue Oct 13, 2022 · 7 comments · Fixed by #4708
Assignees
Labels
priority: P2 Smaller impact or easy workaround status: archived Archived and locked; will not be updated type: enhancement New feature or request
Milestone

Comments

@sky-hugolima
Copy link

Hey !

We've recently changed from Shaka 3.0.5 to 3.3.10, and I noticed that with the new decoding capabilities detection logic, that uses MediaCapabilities API on devices where a shim/polyfill is needed (e.g. WebOS, PS5), there's a non residual time spent doing repetitive calls to requestMediaKeySystemAccess (on lower end LG I saw ~300ms), where previously (3.0.5) we only had to make one call for each key system (Playready, Widevine).

I considered implementing a cache strategy that would store, during the app life cycle, the MediaKeySystemAccess results for each "key system" / codecs, which does improve the performance on subsequent playback sessions but not for the 1st ones.

That got me thinking about alternative ways to do decoding capabilities detection for devices/models where the capabilities are known at compile time and are never expected to change (typically on consoles and SmartTVs)

For example on PS5 (MediaSDK) we know it supports Playready with ec-3, aac, h264 and h265, so instead of doing capability detection for that device we could get a MediaKeySystemAccess instance for Playready with a common media key system config (e.g. ec3 & h265), at the MediaCapabilities polyfill install stage, and then on the decodingInfo call we would return the decoding info without any additional calls to either requestMediaKeySystemAccess or video.canPlayType.

What do you think ? Do you have any concerns with this approach ?

@sky-hugolima sky-hugolima added the type: question A question from the community label Oct 13, 2022
@avelad
Copy link
Collaborator

avelad commented Oct 13, 2022

@sky-hugolima are you interested on send a PR for it (we can discuss the implementation in your PR)? Thanks! It makes a lot of sense to have a cache in some cases.

@github-actions
Copy link
Contributor

@sky-hugolima Does this answer all your questions? If so, would you please close the issue?

@github-actions github-actions bot added the status: waiting on response Waiting on a response from the reporter(s) of the issue label Oct 17, 2022
@avelad avelad removed the status: waiting on response Waiting on a response from the reporter(s) of the issue label Oct 17, 2022
@github-actions
Copy link
Contributor

@sky-hugolima Does this answer all your questions? If so, would you please close the issue?

@github-actions github-actions bot added the status: waiting on response Waiting on a response from the reporter(s) of the issue label Oct 21, 2022
@github-actions
Copy link
Contributor

Closing due to inactivity. If this is still an issue for you or if you have further questions, the OP can ask shaka-bot to reopen it by including @shaka-bot reopen in a comment.

@github-actions github-actions bot removed the status: waiting on response Waiting on a response from the reporter(s) of the issue label Oct 28, 2022
@joeyparrish joeyparrish reopened this Oct 28, 2022
@joeyparrish
Copy link
Member

It sounds like you're proposing to make the mcap polyfill more efficient with caching and/or device-specific details hard-coded. I think that's a fantastic idea. I'll convert this to a feature request and assign it to @sky-hugolima for a PR. Thanks!

@joeyparrish joeyparrish added type: enhancement New feature or request and removed type: question A question from the community labels Oct 28, 2022
@joeyparrish joeyparrish changed the title Device capabilities detection alternatives Caching and other efficiency improvements for mcap polyfill Oct 28, 2022
@joeyparrish joeyparrish added the priority: P2 Smaller impact or easy workaround label Oct 28, 2022
@github-actions github-actions bot added this to the Backlog milestone Oct 28, 2022
@avelad avelad added this to To do in v5 wish list via automation Oct 31, 2022
@sky-hugolima
Copy link
Author

Hey !
Seems like we're aligned then :)
I think we're going to have some time next week to work on this.

@avelad
Copy link
Collaborator

avelad commented Nov 3, 2022

Thanks @sky-hugolima !

v5 wish list automation moved this from To do to Done Nov 18, 2022
joeyparrish pushed a commit that referenced this issue Nov 18, 2022
)

This PR caches the result of `requestMediaKeySystemAccess` saving time
on subsequent calls.

It also makes the calls to `decodingInfo` synchronous. The reason for
this, is the result of testing on multiple devices that the behaviour of
`requestMediaKeySystemAccess` appears to be synchronous, making this
synchronous can save over a second on older TVs.

Closes #4574
@avelad avelad modified the milestones: Backlog, v4.4 Nov 18, 2022
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jan 17, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 17, 2023
@joeyparrish joeyparrish removed this from Done in v5 wish list Oct 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority: P2 Smaller impact or easy workaround status: archived Archived and locked; will not be updated type: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants