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

Player.addThumbnailsTrack() is slow and freezes the browser for long videos #6065

Closed
absidue opened this issue Jan 9, 2024 · 0 comments · Fixed by #6067
Closed

Player.addThumbnailsTrack() is slow and freezes the browser for long videos #6065

absidue opened this issue Jan 9, 2024 · 0 comments · Fixed by #6067
Assignees
Labels
priority: P1 Big impact or workaround impractical; resolve before feature release status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone

Comments

@absidue
Copy link

absidue commented Jan 9, 2024

Have you read the FAQ and checked for duplicate open issues?
Yes

If the problem is related to FairPlay, have you read the tutorial?

Not applicable

What version of Shaka Player are you using?

4.7.3

Can you reproduce the issue with our latest release version?
Yes

Can you reproduce the issue with the latest code from main?
Yes

Are you using the demo app or your own custom app?
Both

If custom app, can you reproduce the issue using our demo app?
Yes

What browser and OS are you using?
Firefox on Windows 10

For embedded devices (smart TVs, etc.), what model and firmware version are you using?
Not applicable

What are the manifest and license server URIs?

Here is a 10 hour long vtt file, it's long enough to noticably reproduce the issue, but short enough that the browser eventually recovers from being frozen.

GitHub doesn't allow the .vtt file extension, so I had to rename it to .txt before uploading.
long-thumbnails.txt

What configuration are you using? What is the output of player.getConfiguration()?

Default

What did you do?

  1. Load any manifest on the demo page e.g.
  2. Wait until it has loaded
  3. Run this with the contents of the WebVTT file above in the dev tools: await document.getElementById('video').ui.getControls().getPlayer().addThumbnailsTrack(data:text/vtt;charset=utf-8,${encodeURIComponent(contents of file here)}, 'text/vtt')

What did you expect to happen?
The thumbnails to be added quickly and the browser to not freeze.

What actually happened?

The web browser froze for a while, although it did eventually recover.
Clicking on the dropdowns is one good way to notice that the page is frozen.

Doing a performance benchmark while it was frozen (starting it before adding the thumbnails and stopping it when the web browser recovers), showed that in total the majority of time was spent in shaka.util.ManifestParserUtils.resolveUris, which addThumbnailsTrack calls in a for loop.

shaka-player/lib/player.js

Lines 4839 to 4842 in 1b7a54c

for (const cue of cues) {
const imageUri = shaka.util.ManifestParserUtils.resolveUris(
[uri], [cue.payload])[0];
const reference = new shaka.media.SegmentReference(

shaka.util.ManifestParserUtils.resolveUris has a note the indicates that it is a known performance bottleneck.

* Note: This method is slow in SmartTVs and Consoles. It should only be
* called when necessary.

As addThumbnailsTrack is calling it in a loop, that means it creates two new goog.Uri objects every time, one for the base URI (which doesn't change) and one for the thumbnail URI and then resolves the thumbnail URI against the base URI. One minor change that would probably make a major difference is creating the base URI goog.Uri object once and then reusing it, instead of creating a new one for each iteration. Here is a small example of how that could be done:

--- a/lib/player.js
+++ b/lib/player.js
@@ -7,6 +7,7 @@
 goog.provide('shaka.Player');

 goog.require('goog.asserts');
+goog.require('goog.Uri');
 goog.require('shaka.config.AutoShowText');
 goog.require('shaka.Deprecate');
 goog.require('shaka.log');
@@ -4835,10 +4836,10 @@ shaka.Player = class extends shaka.util.FakeEventTarget {
     const data = shaka.util.BufferUtils.toUint8(buffer);
     const cues = TextParser.parseMedia(data, time, uri);

+    const baseUri = new goog.Uri(uri);
     const references = [];
     for (const cue of cues) {
-      const imageUri = shaka.util.ManifestParserUtils.resolveUris(
-          [uri], [cue.payload])[0];
+      const imageUri = baseUri.resolve(new goog.Uri(cue.payload)).toString();
       const reference = new shaka.media.SegmentReference(
           cue.startTime,
           cue.endTime,

As mentioned in a previous issue, I would happily create pull requests, if the CLA didn't exist. Having to doxx myself to contribute to a project is unfortunately a big turn off, I know that other people feel the same way too.

@absidue absidue added the type: bug Something isn't working correctly label Jan 9, 2024
@shaka-bot shaka-bot added this to the v5.0 milestone Jan 9, 2024
@avelad avelad added the priority: P1 Big impact or workaround impractical; resolve before feature release label Jan 10, 2024
@avelad avelad self-assigned this Jan 10, 2024
@avelad avelad closed this as completed in 3a14047 Jan 11, 2024
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Mar 11, 2024
@shaka-project shaka-project locked as resolved and limited conversation to collaborators Mar 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority: P1 Big impact or workaround impractical; resolve before feature release status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants