Bug in play/pause trigger

PetroFrolov commented 11 years ago

Hi! I've founded that if I change source and call player.pause() videojs infinitely call trigger(player, 'pause') / trigger(player_, 'play'); Also event ended doesn't work of this bug.

Please, take a look in next code:

</head><body>

function testPlugin() {
var _p = this;

_p._v = {
    val : {
    'new' : '../static/pepsi_vs_coca.webm',
    'real': ''
    },

    init : function () {
    _p.one('play', videojs.bind(this, this.play));
    _p.one('ended', videojs.bind(this, this.log));
    },

    play : function() {
    _f = videojs.bind(this, this.set);
    setTimeout( function() {_p.one('timeupdate', _f);} , 2000);
    _p.on('play', function() {console.log('play')});
    _p.on('pause', function() {console.log('pause')});
    },

    set : function () {
    this.val.real = _p.currentSrc();
    _p.src(this.val.new);
    _p.load();
    _p.play();
    /* next code produce bug - videojs infinitely call trigger(player_, 'pause' | 'play'); uncomment to see the bug
    _p.pause();
    _p.play();
    */
    _p.one('ended', videojs.bind(this, this.unset));
    },

    unset: function() {
    _p.src(this.val.real);
    _p.load();
    _p.play();
    _p.one('ended', videojs.bind(this, this.log));
    },

    log:  function() {
    console.log( 'ended:' + _p.currentSrc() );
    }
};

_p.ready( function () { _p._v.init() } );
}
videojs.plugin('test', testPlugin);

_V_("video", {'plugins': {'test': true}} );

</script>

</body></html>

Use appropriate video to change the source.

P.S. v.3.2 doesn't have that bug

PetroFrolov commented 11 years ago

I've tested in Chrome v26 && FF v21 under Ubuntu

sentfanwyaerda commented 11 years ago

I belief it is related to #569 and #522 due to a unclean implementation of the myPlayer.src()/load within the API

heff commented 11 years ago

Do you think you could set up an example of this on http://jsbin.com?

PetroFrolov commented 11 years ago

Done )

2013/6/27 Steve Heffernan notifications@github.com

Do you think you could set up an example of this on http://jsbin.com?

— Reply to this email directly or view it on GitHubhttps://github.com/videojs/video.js/issues/573#issuecomment-20087327 .

С уважением, Фролов Петр

heff commented 11 years ago

If you posted a link to it, it didn't come through in the comment.

PetroFrolov commented 11 years ago

http://jsbin.com/owexod/2/

heff commented 11 years ago

I'm having trouble tracking everything. Could you remove anything from that example that's not needed to reproduce the bug?

PetroFrolov commented 11 years ago

What browser have you used? what do you think about chatting in skype? petro.frolov

PetroFrolov commented 11 years ago

I think you can edit http://jsbin.com/owexod/2/ There is correct script for Chrome. If you unkomment marked lines you'll see the bug.

PetroFrolov commented 11 years ago

http://jsbin.com/owexod/2/edit

cameront commented 11 years ago

Looking at that jsbin, I think this is the minimal example of the broken behavior.

http://jsbin.com/owexod/12/edit

If so, it's definitely the same issue as #620

PetroFrolov commented 11 years ago

Hi, my case like your example, but... in my case bug starts after source changed.

http://jsbin.com/owexod/14/edit (you need uncomment some code to see it)

cameront commented 11 years ago

Hmm, I don't think changing the source has anything to do with the observed behavior. Notice that the problem exists even if you don't change the source at all.

See: http://jsbin.com/owexod/16/edit (no uncommenting necessary :) )

The problem is calling _p.pause() from inside the play event handler (or, conversely, "play" from inside the "pause" event handler), as the default action is always getting called, resulting in an endless cycle of play/pause events.

PetroFrolov commented 11 years ago

You're right.

PetroFrolov commented 11 years ago

Hi! What's up? Why did you revert commit?

heff commented 11 years ago

Because the pull request was made against stable instead of master, and I didn't catch that before pulling it in. The change still exists in the master branch and will go out with the next release if it didn't go out with the last.

PetroFrolov commented 11 years ago

Thank you, I've read #620 and I've understood all about default action. But .. the problem still present, I've tested 4.0 and 4.1 CDN-versions. I'll wait for new version.

videojs / video.js

Bug in play/pause trigger #573