Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Player does not emit playing event after buffering event #75

Open
dragantl opened this issue Oct 13, 2016 · 6 comments
Open

Player does not emit playing event after buffering event #75

dragantl opened this issue Oct 13, 2016 · 6 comments

Comments

@dragantl
Copy link

I get the following sequence of events:

  • opening
  • buffering
  • playing
  • buffering

However, I would expect playing to be emitted after the last buffering event, assuming that the two events are reflections of waiting (http://www.w3schools.com/tags/av_event_waiting.asp) and playing (http://www.w3schools.com/tags/av_event_playing.asp) events of HTML5 video.

I remember we had this conversation a while ago. It was mentioned that VLC lib emits a huge number of events and they are hard to sync/report. However, wcjs-player already tracks and syncs end of buffering event. That is seen by it being able to remove the Buffering % UI element. What needs to happen is it should also emit playing event at that time (I do not think buffering state can be entered while in paused state).

@dragantl
Copy link
Author

Looks like this issue can be resolved by addition of the following snippet to onBuffering function:

if (event == 100) {
    vlcs[i].vlc.onPlaying(i);
}

With the final result:

vlcs[newid].vlc.onBuffering = function(i) {
    return function(event) {
        if (vlcs[i].lastState != "buffering") {
            vlcs[i].lastState = "buffering";
            vlcs[i].events.emit('StateChanged','buffering');
            vlcs[i].events.emit('StateChangedInt',2);
        }
        isBuffering.call(players[i],event);
        if (event == 100) {
            vlcs[i].vlc.onPlaying(i);
        }
    }
}(newid);

If you guys are too busy, maybe you could review the change and I will make a PR?

@jaruba
Copy link
Owner

jaruba commented Oct 14, 2016

I'm trying to remember the solution I recommended before.. But it wasn't in github issues, it was on gitter... and a long time ago..

@dragantl
Copy link
Author

Why doesn't the above work? I've applied it to my local wcjs-player package and things seem to be working fine. The code also seems to handle things properly.

@jaruba
Copy link
Owner

jaruba commented Oct 21, 2016

I'm not saying it doesn't work, just that I can't predict all the possible implications of it. For example, keep in mind that the 'playing' event is not the one that's wrong here. The 'buffering' event is wrong. Because in order to prevent what's commonly known as event flood, the 'buffering' events are being queued one after the other. So 'playing' is sent properly, while 'buffering' is being pushed further into the timeline. What you did is correct in fixing it for your requirements, and the perfect / simplest way of avoiding a similar issue for others. But in reality, the 'playing' event should count as 'buffering 100', not the other way around. :)

@jaruba
Copy link
Owner

jaruba commented Oct 21, 2016

Besides, it would require removing the actual 'playing' event for a fake one. If (for whatever reason) there is no 'buffering 100' on a video, which might happen due to a different libVLC version being used and some silly video type / loading circumstances, etc. Then there would be no more 'playing' event at all with this code.. While with the reverse logic, overwriting 'buffering 100' (as absurd as it may be to code) can't create any such issues.

@jaruba
Copy link
Owner

jaruba commented Oct 21, 2016

I'm OK with this solution being added as a parameter to '.addPlayer()' something like 'appendPlaying: true', but definitely not as a permanent solution as it doesn't fix the actual problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants