silvermine / videojs-chromecast

MIT License
148 stars 75 forks source link

Correctly disposing of the plugin after calling player.dispose() ? #97

Closed dwassing closed 3 years ago

dwassing commented 3 years ago

Hello! Thanks for writing this lovely plugin, it has saved me quite a bit of headache for integrating chromecast with videoJS. That being said, I still have one small issue that bothers me.

In my application, which uses videoJS with angular, I initiate the plugin with

import videojs from 'video.js';
import chromecast from '@silvermine/videojs-chromecast';
{...}

// add plugin
chromecast(videojs);

I spawn players and destroy them when I'm done with them (callback to ngOnDestroy), which calls player.dispose(). This however causes the cast framework to behave oddly, because I am guessing that the plugin does not unregister properly and throws errors because it cannot find the DOM object anymore. See error log:

ChromecastDisposeErrorLog

According to Google, the button (the plugin) should also dispose when the player is torn down, but I just do not see this happening.

If I keep the player alive by not calling dispose() on it, then I get no errors, and for now my application seems to behave correctly, but I do not feel like this is a good idea long term.

A possibility would maybe be to add a plugin.dispose() functionality. Has this ever been considered?

Thank you so much for any response on the matter. Best regards, Daniel

Edit 26-05-2021: I have tried by running it through webpack with

import videojs from 'video.js';
require('@silvermine/videojs-chromecast')(videojs);

It seems to save me some trouble with registering plugins, and it works, but I still get the same error log. Is it possible that the eventListeners defined in the SessionManager do not correctly de-couple?

dwassing commented 3 years ago

After a two week hiatus and returning to this, I believe this issue seems to stem from the cast framework asking for a player instance when the player is in a disposed state (removed from the DOM) and video js accurately says that the player is disposed. This chromecast plugin is merely an intermediate in the information exchange (as is evident in the error log).

What likely needs to happen is a callback to dispose the connection to the cast framework as the player disposes. I will investigate further into this, and if I find anything, will share it here.

(Maybe it is incorrect to assume that the cast framework should disconnect from a disposed player - but in that case, then the event listeners defined in the ChromecastSessionManager.js file do not get correctly removed upon dispose).

dwassing commented 3 years ago

Edit: Originally thought the problem was adressed, turns out I was wrong.

After creating a minimal reproducible example (use the mute button to kill (dispose) the player) I still find that the event listeners (defined in ChromecastSessionManager) do not correctly detach upon disposing (tab out and back in to the application to see the failed callback as the PresentationAvailability changes).

Tested on chrome 91.0.4472.101 (windows).

Would love to get some developer insights on this. Thanks.

dwassing commented 3 years ago

Update: I finally found the reason, I believe. source

Event listener removals do not find references to their old functions, thus not properly removing them. I fixed this temporarily by editing the ChromecastSessionManager to:

This way, references are retained, and eventListeners are correctly de-coupled. Code aint pretty, but at least it works. Will fork and submit a pull request next week sometime.