j-holub / Node-MPV

A NodeJs Module for MPV Player
https://www.npmjs.com/package/node-mpv
MIT License
116 stars 73 forks source link

handle manual closing of mpv #16

Closed vankasteelj closed 7 years ago

vankasteelj commented 7 years ago

I'm trying to detect when MPV was manually closed by the user.

mpvPlayer.on(event) doesn't seem to work, I tried : close, closed, quit, quitted, end, ended

Is there a way to detect such a move? I see the mpv.js file is getting a new instance spawned, which is fine, but it'd be cool if there was a way to know what the user did

vankasteelj commented 7 years ago

Maybe on https://github.com/00SteinsGate00/Node-MPV/blob/master/lib/mpv/mpv.js#L103

    this.emit('quitted', null);

would something prevent such a line? It works locally, but I don't know the project as a whole

j-holub commented 7 years ago

Funny you say that, I was just discussing something smililar with @LukePulverenti. That auto restarting is actually not a really good idea, but instead just inform the user, that mpv has crashed and let him handle that.

And I agree. I proposed solution was exactly something like you said. Just for information, I'm working on a version 2 of the library, which you can find here.

But since you opened an issue about it, I will fix/add this in the Version 1 as soon as I can (should be within the next few days, got a small university project to hand in on Wednesday).

I introduced a small breaking change in Ver2 on how to start mpv, it would be interesting to get an opinion on that from somebody who's actually using the project :)

vankasteelj commented 7 years ago

It looks like what's used by webtorrent, it does give a little more control over what's going on and allows to better handle errors through the catch.

FYI I'm not using the project yet, but I'm fiddling around while we figure out how to use a custom node binding of MPV. I would prefer to have the player actually embded in the HMTL instead of spawning it, but it still beats VLC which has absolutely no API (and thus sucks for me)

j-holub commented 7 years ago

Especially in Ver2 you can easily start and stop mpv (for example when it crashed).

To be honest, thinking about it now I can find any scenario, where it is good that mpv is respawned automatically. It would lose the playlist and everything. With Ver2 I can make some more drastic changes and probably will change it, that it just crashed, triggers an event and the user can restart it, reload the playlist and what so ever. Maybe I'll leave an autostart option or something.

Embedding the player in HTML sounds tricky, I don't know how to do that honestly. I hope you realized, that the music/video is playing locally on the machine, not in the browser. So even if somebody uses the browser on his phone, it is played on the server machine.

vankasteelj commented 7 years ago

well yes, but we're working on a multimedia player in HTML/JS at https://github.com/butterproject/butter and we're using a nodejs browser called nwjs, a hybrid basically "nodejs in a chrome browser". Thus, if we build a node binding with libmpv, we can embed a mpv player inside a local webpage, as it if was flash, pepper or html5 player.

j-holub commented 7 years ago

Okay that nwjs thing sounds nuts, sounds similar to electron.

Just not very fond of the "streaming movies from torrents you didn't pay for" part ;) Please correct if I'm wrong, it's just what butter sounds like, sorry if I offended anybody with that assumption.

vankasteelj commented 7 years ago

I'm not offended, but we gather content from independant filmmakers such as VODO and Archive.org, so your assumption is wrong^^

j-holub commented 7 years ago

That sounds awesome! I like Independent films, especially Japanese ones. Gotta keep my eye on Butter :)

Sorry I made a wrong judgment :)

j-holub commented 7 years ago

Hey there.

I've implemented it in both Branches (Version1 and 2), but not yet pushed. So far I called the event crashed because it is not called, when the user quits the player using the quit() method. Because you know when you call that function and I think it's not a good idea if you cannot distinguish between a crash and when it was quitted on purpose. Calling it quitted might confuse people that it is also triggered when quit() is called.

What do you think? Another idea would be something like

emit('quitted', <boolean_if_on_purpose>)

Would there be a reason to have an event triggered when the quit() function is called?

vankasteelj commented 7 years ago

There are multiple scenarii:

1) a script opens the player and closes it (no need for an event, the script knows it is closed because it asked it to close)

2) a script opens the player and the player crashes : 'crash' event is a good idea

3) a script opens the player, and someone human closes the player : 'quit' event is absolutely needed

j-holub commented 7 years ago

I see your point yes.

At this point I don't know yet, how to distinguish a crash from a close to be honest. In both cases the close event of node's spawn is triggered.

Node's Socket has an has_error boolean that is passed in the close event of that, I might investigate that.

j-holub commented 7 years ago

Version 2 now has both quit and crashed events.

the close event of the child_process module gives an error_code which I can use to distinguish between a crash and a close on purpose. For some reason it doesn't really work in version 1. It's often null or not consistent. I'm still looking into it.

A crash event is already implemented in Version 1, it just can't differentiate between a crash and a close on purpose yet.

vankasteelj commented 7 years ago

Isn't there a code send back by the terminal? in Bash you have exit codes, somewhat as http://tldp.org/LDP/abs/html/exitcodes.html

j-holub commented 7 years ago

As said, the close or exit listener return exit codes and in version 2 I can use them do distinguish between a crash and a quit on purpose. For some reason it doesn't work in version 1, where the exit code is undefined for the most part, for some reason.

It works in the version I intend to keep around though, so that's something good so far.

I also thought about listening to the bash output directly, but I set mpv to really-quite. I had issues in the past, where I spawned python programs with python, and their console output spammend the buffer and caused them to crash. Since the output is not required anyway I just suppressed it.

Mpv offers msg-levels, I use that to set the ipc socket output to true, to find out when the socket was bound in version 2. But unfortunately I can't find any documentation on this on the mpv side. I can't find anywhere which modules there are, so I could set just that to verbose.

Point is, it works like a charm in version 2, I don't want to spend too much time on version 1, still trying though. I just don't get why the exit code thing works on the one part and does not on the other =/

vankasteelj commented 7 years ago
    play: (file) => {
        Player.handleEvents();

        console.info('Playing:', file)

        Player.mpv.start().then(() => {
            Player.mpv.loadFile(file);
            Player.mpv.observeProperty('percent-pos', 50);
        }).catch(error => {
            console.error('MPV error', error);
        });
    },

    handleEvents: () => {
        if (Player.config.events) return;

        Player.mpv.on('statuschange', states => {
            Player.config.states = states;
        });
        Player.mpv.on('paused', () => {
            console.log('MPV paused at %s%', Player.config.states['percent-pos']);
        });
        Player.mpv.on('resumed', () => {
            console.log('MPV resumed at %s%', Player.config.states['percent-pos']);
        });
        Player.mpv.on('stopped', () => {
            console.log('MPV stopped')
        });
        Player.mpv.on('quit', () => {
            console.log('MPV quitted at %s%', Player.config.states['percent-pos']);
        });

        Player.config.events = true;
    }

this could probably be better, but it works.

j-holub commented 7 years ago

Closing it because it is already fixed in Version2 anyways