search

videojs / videojs-contrib-eme

Supports Encrypted Media Extensions for playback of encrypted content in Video.js
Other
199 stars 71 forks source link

Handle player disposal gracefully #69

Open forbesjo opened 5 years ago

forbesjo commented 5 years ago

Previously a player could have leftover contrib-eme data because this plugin mutates the Player prototype. This was "fixed" in https://github.com/videojs/videojs-contrib-eme/pull/61/files#diff-a4f485aa834ca5b3c24819fceb39c56fR243 in that the player.eme is overwritten, cleaning up activeSrc and sessions. However we should have a plugin dispose mechanism that cleans up handlers and data.

gkatsev commented 5 years ago

A simple way to add it is to listen to the dispose event on the player. Though, you definitely could migrate to the Plugin class.

gesinger commented 5 years ago

Note also there's a bit of conversation around dispose in an active PR: https://github.com/videojs/videojs-contrib-eme/pull/47/files#diff-a4f485aa834ca5b3c24819fceb39c56fR257

squarebracket commented 5 years ago

There's a couple more things in the pipeline that tie in with disposal, which may be worth mentioning.

  1. Using loadstart to wipe/initialize sessions on source changes instead of the current method. We talked about it briefly on slack; it seems to be safe, and would fix an everlasting spinner bug after reloading the same source. That handler should be removed on player disposal.

  2. Key rotation. It registers handlers for a couple events on player.tech_ (dunno if tech disposal is a thing that's used). It also uses a single handler function for the events fired by tech_.el_, which would make removing them easier, though I imagine the video element is destroyed during disposal and would thus be unnecessary?

It would probably make sense to do 1 in the same PR as cleaning up the handlers/data.