Exceptions while destroy() #151

jozefchutka · 2015-08-09T20:24:04Z

Using shaka-player.compiled.js v1.4.0 I can observe two exceptions when destroying player instance. Here is my test player - clicking in the document a current instance of player is disposed and a new one created:

<!DOCTYPE html>
<html>
<head>
<meta charset='utf-8'/>
<title>Shaka Player</title>
<style>video {display: block;}</style>
<script src="http://shaka-player-demo.appspot.com/shaka-player.compiled.js"></script>
</head>
<body>
    <script>
        (function () { "use strict";
        var Main = function() {
            var _g = this;
            window.document.body.addEventListener("click",function(event) {
                _g.reloadPlayer();
            });
            this.initPlayer();
        };
        Main.main = function() {
            new Main();
        };
        Main.prototype = {
            initPlayer: function() {
                console.log("initPlayer");
                shaka.polyfill.installAll();
                this.createPlayer();
            }
            ,createPlayer: function() {
                this.element = window.document.createElement("video");
                this.element.autoplay = true;
                window.document.body.appendChild(this.element);
                var mpdUrl = "http://shaka-player-demo.appspot.com/assets/angel_one.mpd";
                var estimator = new shaka.util.EWMABandwidthEstimator();
                var source = new shaka.player.DashVideoSource(mpdUrl,null,estimator,null);
                this.player = new shaka.player.Player(this.element);
                this.player.load(source);
            }
            ,disposePlayer: function() {
                this.player.unload();
                this.player.destroy();
            }
            ,reloadPlayer: function() {
                this.disposePlayer();
                this.createPlayer();
                this.disposePlayer();
                this.createPlayer();
            }
        };
        Main.main();
        })();
    </script>
</body>
</html>

First exception seems to be related to <style>video {display: block;}</style>:

Uncaught (in promise) TypeError: Cannot read property 'reject' of null
    at Be.<anonymous> (http://shaka-player-demo.appspot.com/shaka-player.compiled.js:108:133)(anonymous function) @ shaka-player.compiled.js:108

Second one seems to be related to quickly destroying and recreating player. It is easily replicated by creating and destroying the player twice in a row (see dispose/create/dispose/create sequence)

Uncaught (in promise) TypeError: Cannot read property 'addEventListener' of null
    at new Ua (http://shaka-player-demo.appspot.com/shaka-player.compiled.js:16:868)
    at J (http://shaka-player-demo.appspot.com/shaka-player.compiled.js:16:563)
    at Q.<anonymous> (http://shaka-player-demo.appspot.com/shaka-player.compiled.js:76:328)Ua @ shaka-player.compiled.js:16J @ shaka-player.compiled.js:16(anonymous function) @ shaka-player.compiled.js:76

Its also worth mentioning that unload() seems to be required before destroy(). I would suggest to have that call inside destroy() so memory can be released easily by calling just one master method.

The text was updated successfully, but these errors were encountered:

tdrews · 2015-08-10T16:39:46Z

Player.destroy() does call Player.unload(), but both of these methods return a promise (it's unfortunate that they have to return a promise, but it's required to reset encrypted media extensions). So, you should be able to avoid the exceptions by doing something like this,

var reload = function(newMpdUrl) {
  return player.unload().then(function() {
    return player.load(newMpdUrl, null, sameEstimator, null);
  });
};

jozefchutka · 2015-08-12T08:52:28Z

would it be possible to implement fix in the shaka player so it would not throw these exceptions?

joeyparrish · 2015-08-17T21:43:14Z

@jozefchutka Do you still get these exceptions when you follow advice from @tdrews about the unload promise?

jozefchutka · 2015-08-19T19:08:13Z

hi joeyparrish, the async solution is not ideal in my case as I need to destroy old instance / change source synchronously. Is it preffered the Player instance to be reused or new one to be recreated for every new source?

joeyparrish · 2015-08-19T20:21:45Z

@jozefchutka

For what it's worth, the Player is designed to be reusable. You are welcome to create new Player instances if you like, but if there are still async processes happening with your video tag, attaching a new Player to the same tag before that completes may cause problems for you.

If you intend to create new Players, you should either wait for the old one to be destroyed or add remove the old video tag from the DOM and create a new one for the new Player. However, given that we designed Player to be reusable, I really can't see any reason to do it. Plus, this process would still be async.

The asynchronicity of unload() cannot be avoided due to the asynchronicity of EME APIs that must be used during unload().

My recommendation is to call load() with a first source, then when you want to change sources, just call load() again. load(), which is async, will call unload() internally if need be and wait for the result. If you need to destroy the player, just call destroy() instead of unload() and destroy(). The destroy() method, like load(), will call unload() internally if needed.

Does this work for you? If not, can you explain your use case? I don't understand why a real application would do the things you are doing in your sample above.

jozefchutka · 2015-08-24T08:14:29Z

@joeyparrish you can see I try to create a new video instance for each Player insntance so that should not be an issue. I have also tried to remove

In my realworld application I need to navigate around a singe page app and there is

joeyparrish · 2015-08-24T15:44:52Z

@jozefchutka, I see. Well, we did not consider rapid creation and destruction as a use case, but it seems reasonable to expect destruction to interrupt async processes and cause their failure. Any Promises returned to you would be rejected, and your application would then have to avoid calling methods on that instance from any handlers you attach to that Promise.

For what it's worth, I would still recommend against calling unload() and then destroy() synchronously. Just destroy() should suffice, and will trigger the same logic as unload() during the destruction process.

I notice you say that these exceptions could "easily be avoided". Does that mean you have already looked into it and worked out the fix? Do you have a patch ready? Or is this something you would still like us to work on?

Thanks,
Joey

jozefchutka · 2015-08-25T07:56:41Z

@joeyparrish, I can not recall why I needed to call unload() before destroy(), but I can confirm that even without unload() being called it does work the same in my test. So I can see the same exception while new video being created correctly and old one is gone. If I recall the reason, I will report it as a separate issue.

By saying easily avoided I referred to the fact that apart from the exception in the console the player works as expected, and as that one seems to be a null pointer it can be avoided by having a null check in place. I do not have a patch or fix on my side.

joeyparrish · 2015-08-31T22:44:47Z

I have a fix for this in review, including new Player tests to cover your use case. Thanks for the report!

joeyparrish · 2015-09-01T16:32:28Z

Found one more case we missed.

jozefchutka · 2015-09-01T19:16:58Z

great job. thanks @joeyparrish

joeyparrish added the type: question A question from the community label Aug 17, 2015

joeyparrish self-assigned this Aug 17, 2015

joeyparrish added type: bug Something isn't working correctly and removed type: question A question from the community labels Aug 31, 2015

joeyparrish closed this as completed in 97769bd Sep 1, 2015

joeyparrish reopened this Sep 1, 2015

joeyparrish added this to the v1.5.0 milestone Sep 1, 2015

joeyparrish closed this as completed in b22a384 Sep 1, 2015

carcharodoncarcharias511 mentioned this issue Sep 15, 2015

Exception after destroying shaka player #183

Closed

TheModMaker added the archived label Mar 22, 2018

shaka-project locked and limited conversation to collaborators Mar 22, 2018

shaka-bot added the status: archived Archived and locked; will not be updated label Apr 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Exceptions while destroy() #151

Exceptions while destroy() #151

jozefchutka commented Aug 9, 2015

tdrews commented Aug 10, 2015

jozefchutka commented Aug 12, 2015

joeyparrish commented Aug 17, 2015

jozefchutka commented Aug 19, 2015

joeyparrish commented Aug 19, 2015

jozefchutka commented Aug 24, 2015

joeyparrish commented Aug 24, 2015

jozefchutka commented Aug 25, 2015

joeyparrish commented Aug 31, 2015

joeyparrish commented Sep 1, 2015

jozefchutka commented Sep 1, 2015

Exceptions while destroy() #151

Exceptions while destroy() #151

Comments

jozefchutka commented Aug 9, 2015

tdrews commented Aug 10, 2015

jozefchutka commented Aug 12, 2015

joeyparrish commented Aug 17, 2015

jozefchutka commented Aug 19, 2015

joeyparrish commented Aug 19, 2015

jozefchutka commented Aug 24, 2015

joeyparrish commented Aug 24, 2015

jozefchutka commented Aug 25, 2015

joeyparrish commented Aug 31, 2015

joeyparrish commented Sep 1, 2015

jozefchutka commented Sep 1, 2015