-
Notifications
You must be signed in to change notification settings - Fork 313
Fire playing on buffer full only when we are not pending a pause. #90
Conversation
As the code is now, a 'playing' event fires off as soon as an hls src is provided and using videojs-contrib-hls plugin. |
I think this makes sense, I'm not sure what the @seniorflexdeveloper, can you add your thoughts on this? |
My only concern with this is if the buffer fills multiple times per playback will this cause a misreport. For example, if during a single playback my buffer empties and then refills, will this fire twice? |
Wouldn't it fire multiple times with the existing code? The intention of this change was to limit the number of times it fires not increase it. Perhaps there should also be a check for !_isPlaying? case "NetStream.Buffer.Full":
_isBuffering = false;
_model.broadcastEventExternally(ExternalEventName.ON_BUFFER_FULL);
_model.broadcastEventExternally(ExternalEventName.ON_CAN_PLAY);
if (!_pausePending && !_isPlaying) {
_pausedSeekValue = -1;
_isPlaying = true;
_model.broadcastEventExternally(ExternalEventName.ON_START);
else {
_pausePending = false;
_ns.pause();
_isPaused = true;
}
break; In any case, I agree with @heff. 'playing' and buffer full event don't seem like they should be tied together conceptually. So maybe that should be addressed as a larger change rather than trying to hack it out here. |
what does the |
I'm not sure where the term 'ON_START' comes from, but the var equals 'playing'. The 'playing' event is supposed to be triggered when paused=false and there's enough buffer to start playing. So not just when play() is called. I guess that kind of explains the buffer full part. |
Thanks. Based on that, I think we should be checking the value of |
Im still working on this. I don't think _isPlaying is the right thing to look at cause it appears to almost always be true under normal circumstances when the Buffer.Full event fires. I've changed it to look at the _isBuffering property since I think that's pretty close to what we want. _isBuffering is set true when: and set false when: I'm still working through this change though. |
OK, changed the code to only fire if we were buffering. I had to add conditionals to the other places where we fire this event to prevent it from firing twice (once on the play request and again when the buffer filled). My manual tests of playback with an mp4 using the contrib-hls plugin look good. Wish there was a bit more automation already in place around these events. |
Fire playing on buffer full only when we are not pending a pause.
No description provided.