[WIP] fix: trigger playback ready event when rendered

[kslimani]

It ensure that "PLAYBACK_READY" event is trigger when plugin is rendered to attempt to be consistent with other playback plugins.

These changes allow the "CORE_READY" event to be trigger before the play() method is called.

I also added some conditions to avoid potential predictable exception(s) on undefined/null properties/method results.

Should fix #74.

I focused on avoiding any backward incompatible changes, but it would be nice to have some code review before merge.

[kslimani]

I added an additionnal commit to fix an issue with Safari on macOS : it store the user interaction provided by clicking on play (when autoplay is not permitted) before setup the Shaka instance.

The main reason is that the load() method will fetch the manifest and therefore will lost the user interaction. (required by core plugins which may require to initalize stuff when user click on play)

It is a workaround, otherwise when the playback "PLAY_INTENT" event is triggered, the user interaction is already lost. (this change make the Shaka playback plugin fully compatible with my IMA SDK ad plugin)

[kslimani]

Added minor fix and updated jsDelivr script urls.

[kslimani]

Added level property to "levels available" playback event object to increase consistency with hls.js playback plugin. (it allow developper to use label callback option in level selector plugin to fully customize label formatting).

[kslimani]

Added a commit which hopefully fix #76 .

[joaopaulovieira]

Hey @kslimani, thanks for the contribution! I suggest one approach to handle the behavior related to issue #74.

Move the code on the _onShakaReady to the _ready and checks if the _setup has occurred. I think the semantic of the ready state is the playback is ready to play and this sentence is not true if the _setup doesn't occur.

Something like:

  _ready () {
    !this._player && this._setup()
    this._isShakaReadyState = true
    this.trigger(DashShakaPlayback.Events.SHAKA_READY)
    super._ready()
  }

  _onShakaReady() {
    this._ready()
  }

With this approach, we handle the call of _ready inside the render call on HTML5Video and when occurs one imperative call of the _setup on the DashShakaPlayback.

What do you think?

[kslimani]

@joaopaulovieira indeed i missed the _ready() call in the HTML5 playback parent class, thank you for pointing this to me. (i think in the current state, this PR may trigger the playback state ready event too soon)

I like your approach because it attempt to create the Shaka instance and load the source manifest at render time. (which will avoid the source manifest ajax request when user click to play)

The only part i think about is that _setup() method perform asynchronous stuff and set _isShakaReadyState when Shaka instance has loaded source. For this reason, maybe we should still override the _ready method without calling super._ready() ?

What do you think ?

PS: i rename this PR as "WIP" until this issue is fixed.

[joaopaulovieira]

I may not have understood. Can you apply your suggested changes to the PR?

[kslimani]

Let's hold this PR until i have the hand again on mobile/desktop devices (covid-19 containment) to ensure it will works.