Multiple call of start (Node-MPV-2)

[Lukinoh]

Hi,

I am using Node-MPV-2, and I have a few questions.

Several functions does not work if the player is not running. For instance: quit, resume, pause, and load (non-exhausive list). Should Node-MPV-2 be more resilient and checks if isRunning is true, or it should be on my side ? Load even breaks node sh Error: connect ECONNREFUSED /tmp/node-mpv.sock at Object.exports._errnoException (util.js:1020:11) at exports._exceptionWithHostPort (util.js:1043:20) at PipeConnectWrap.afterConnect [as oncomplete] (net.js:1086:14) [19:53:58] [nodemon] app crashed - waiting for file changes before starting... Actually, I came here for a derivation of this issue. If we call several times start method, it will spawn a new mpv process each time, and replace the old one in the mpvPlayer object. Hence, if you do something like:

START -> LOAD a file -> START -> QUIT You will quit the second START, and the other one will be still there and unreferenced.

Thank you for your work, it is really a useful project !

[j-holub]

Hey there,

thank you for the feedback, really.

I am aware of the first issue you describe. I agree with you, that node-mpv should check that for itself, but when I first implemented start() and quit() the only way I saw doing this was by bloating up every function with it's own check code. Now that I'm using more promises, I might find a more elegant way. With you on the "Node-Mpv should check that by itself" thing.

As for the latter issue, that's definitely something that should (and will) be fixed. Thank you a lot. I think I can fix it quite easily. I have an oral exam at university tomorrow so I don't know if I will still make it tonight, but I try to do it as fast as possible.

Thanks a lot :D

[j-holub]

My idea is, that when you call start() and mpv is already running, the promise is rejected with an error message like "MPV already running".

But you can pass it a parameter like "force-restart" or something, the restarts the player. What are your thoughts about that?

[Lukinoh]

If this is the only reason of failure, could be enough.

But if there are other reasons of failures and you return various strings like "MPV already running", "MPV crashes", "MPV xyz", it could be not be the best, because if the developper has to take a different action in function of the error, he would have to comparison of string like "MPV already running".

Maybe you could look at well established librairies for some inspiration. Error handling/generation is not really my cup of tea :p

I don't see a use case where "force-restart" would be used. If someone needs really that, he should do a Quit -> Start. But that's a personel opinion :p

[j-holub]

Regarding your worries about the error message please have a look here, I'm sorry I haven't documented it yet :) But I'm open to look at more sophisticated solutions.

Yeah the force-restart might indeed not be required.

[Lukinoh]

Fair enough :smile:

[j-holub]

Should be fixed in the latest version, worked in my case (I have to implement unit test). I'll see about the thing for calling methods when the player is not running, it's straight forward for methods that return promises, but for other's it would mean to promisify them for that purpose.

Error message over a socket event is of course a possibility....

[Lukinoh]

Thank you for the modification. Just to bounce on your last message, I think that every action that is sent to the mpv player through the IPC should return a promise to know if successful or not. Then, how it is implemented is up to you :p.

Thank you again for your quick responses, and good luck for your exams !