player.destroy() doesn't work for DASH

[clappr]

player.destroy() doesn't work for DASH

[joeyparrish]

Hrm... 2.0.7-beta isn't a tag in our repo. Is that one of yours in clappr?

Just based on the log Uncaught TypeError: Cannot read property 'abr' of null, I assume this is a bug where the 'abr' member of the configuration object is accessed after the config has been set to null by destroy().

The compiled snippet you mentioned looks like the body of Player.prototype.destroy() itself. What we have there in HEAD is:

  var p = Promise.all([
    this.config_ ? this.config_.abr.manager.stop() : null,

3 weeks ago, before the commit "Overhaul load/unload/destroy", we had only this:

  var p = Promise.all([
    this.config_.abr.manager.stop(),

So prior to that overhaul, multiple calls to destroy() might result in that TypeError.

I think the best solution is to move to a version that includes the overhauled destroy(). Shaka Player v2.0.0-beta3 will have this commit. I can ping this ticket when it's out, if that's helpful.

The next best solution is to make sure that you do not call destroy() more than once on the same Player instance. This should effectively work around the bug until you upgrade to a fixed version of Shaka.

[ibreakstuff93]

@leandromoreira thanks for your awesome work!

[joeyparrish]

JFYI, Shaka Player v2.0.0-beta3 came out recently with the fix mentioned above.

[leandromoreira]

@ibreakstuff93 thanks I can reproduce it here https://jsfiddle.net/hzzurtkh/

[leandromoreira]

@ibreakstuff93 I cloned the shaka project to try to reproduce the same error (aka call destroy) but I couldn't, either on the HEAD or the tag 2.0.7-beta.

@joeyparrish maybe you can help us here, I'm calling the player destroy and then the success promise is executed... but after that a weird error shows up at console Uncaught TypeError: Cannot read property 'abr' of null

    /// _player is an instance of shaka
    this._player.destroy().then(() => console.log('success'), () => console.log('error') )

I saw it was raising this error on the last line of this snippet. (probably because a.a was undefined)

                                function ne(a) {
                                    a.u && a.u.Y(a.A, "sourceopen");
                                    a.c && (a.c.removeAttribute("src"),
                                    a.c.load());
                                    var b = Promise.all([a.a.abr.manager.stop(), a.g ? a.g.s() : null , a.O ? a.O.s() : null , a.h ? a.h.s() : null , a.b ? a.b.s() : null , a.o ? a.o.stop() : null ]);

But again I couldn't reproduce this behavior while on the HEAD or at the tag tag 2.0.7-beta using solely Shaka.

[leandromoreira]

I think I have a clue, we're removing the DOM element (video) after the destroy success, maybe this is not safe.

[leandromoreira]

Hrm... 2.0.7-beta isn't a tag in our repo. Is that one of yours in clappr?

yep, my mistake in fact I checkout to v2.0.0-beta2

Shaka Player v2.0.0-beta3 will have this commit. I can ping this ticket when it's out, if that's helpful.

It'll be! Thanks

By the way, @joeyparrish is right, we're calling destroy twice this is a bug on Clappr side!

[leandromoreira]

@ibreakstuff93 this will be fixed when a newer version will be published

[ibreakstuff93]

@leandromoreira - sorry for being a n00b here, is this tagged as wontfix as the problem lies on the clappr side - and if I go to the latest version of Clappr it will work? Sorry for being a thickie!

[leandromoreira]

@ibreakstuff93 yes, I fixed this at clappr/clappr#1047 and when a new clappr version came out this will be fixed :)

[leandromoreira]

Thanks @joeyparrish I'll try to use the last one now lol