shaka-project / shaka-player

JavaScript player library / DASH & HLS client / MSE-EME player
Apache License 2.0
7.19k stars 1.34k forks source link

`Player.attach(videoElement)` with the UI uses `SimpleTextDisplayer` instead of `UITextDisplayer` like `new Player(videoElement)` does #5974

Closed absidue closed 11 months ago

absidue commented 11 months ago

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.6.3

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

Can you reproduce the issue with the latest code from main? not tested

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

If custom app, can you reproduce the issue using our demo app? No, the demo uses the page scanning auto-initialization instead of programatically creating a player.

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?

https://storage.googleapis.com/shaka-demo-assets/angel-one/dash.mpd

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

Default

What did you do?

Here is a small example to reproduce the issue based on the UI tutorial: https://shaka-player-demo.appspot.com/docs/api/tutorial-ui.html

<!DOCTYPE html>
<html>
  <head>
    <script src="https://cdn.jsdelivr.net/npm/shaka-player@4.6.3/dist/shaka-player.ui.js"></script>
    <link rel="stylesheet" type="text/css" href="https://cdn.jsdelivr.net/npm/shaka-player@4.6.3/dist/controls.css">
  </head>
  <body>
    <div id="container" style="max-width:40em">
      <video autoplay id="video" style="width:100%;height:100%"></video>
    </div>

    <script>
      (async function () {
        const video = document.getElementById('video');
        const container = document.getElementById('container');

        const localPlayer = new shaka.Player();
        await localPlayer.attach(video);

        // replacing the two lines above with this works correctly:
        // const localPlayer = new shaka.Player(video);

        const ui = new shaka.ui.Overlay(localPlayer, container, video);

        const player = ui.getControls().getPlayer();

        await player.load('https://storage.googleapis.com/shaka-demo-assets/angel-one/dash.mpd', null, 'application/dash+xml');
      })()
    </script>
  </body>
</html>

What did you expect to happen? I was expecting the UITextDisplayer to be used as I am using the UI.

What actually happened?

shaka-player used the SimpleTextDisplayer (notice how the captions are displayed underneath the control bar when the controls are visiable).

The bug only happens when you use attach, passing the video element to the Player constructor makes it work correctly. As the recommendation is to switch to attach and passing the video element to the constructor is deprecated, I think this should probably be considered a regression.

theodab commented 11 months ago

When the shaka.ui.Overlay object is created, it configures the player instance to use the UITextDisplayer instead of the SimpleTextDisplayer. The text displayer is created during attach, so calling attach before making the overlay causes the wrong text displayer to be used.

Attaching the video the old way works properly, meanwhile, because attach is still an asynchronous operation behind the scenes, it's just started during construction if you put that argument in. So you're making the overlay before the attach process gets around to making the text displayer.

I think this is working as intended, but the tutorial should be changed to recommend calling attach after making the overlay.