[node-mpv-2] Loading subtitles after loading and playing a file

[AxelTerizaki]

With the 1.3 version, here's the code I use to load a file, play it, and add subtitles :

module.exports._player is an instance of node-mpv

module.exports._player.command('loadfile',[video,'replace','replaygain-fallback='+gain]);           
module.exports._player.play();
var timeout = 1500;
setTimeout(function(){
   module.exports._player.addSubtitles('memory://'+subtitle);                       
},timeout);

I have to do it this way because the video file takes a bit of time to load. if I don't use a setTimeout, in some cases the subtitles are loaded before the video plays, and thus don't display at all on screen.

In version 2, I'd need the new options parameter added to load() in 1.4 to be able to use the function and its new promsified version.

I could write something like this :

module.exports._player.load(...)
  .then(() => {
     module.exports._player.play();
     var timeout = 1500;
     setTimeout(function(){
        module.exports._player.addSubtitles('memory://'+subtitle);                      
     },timeout);
  });

The thing is, I cannot tell if what takes time is the load() function or the play() function. In any case, I think play() should be promsified, so I can add subtitles right after play() returns a promise. I could then get rid of the timeout and achieve something like this :

module.exports._player.load(...)
  .then(() => {
     module.exports._player.play()
  .then(() => {
     module.exports._player.addSubtitles('memory://'+subtitle);                     
  });
  });

Could it be possible to have the improvements from 1.4 ported to 2.0, at least the options argument of load() which would allow me to use it instead of command() ? Also, have play() return a promise ?

Having command() and freeCommand() return a promise could be helpful as well, I believe.

As for other promised functions, the problem is that I can't think of many use cases where some of them should be promsified. On the other hand, having only some functions returning promises and not others could be quite confusing. I realize it's a lot of code to add especially given all the functions in mpv. To me, a first step would be to add play(), command() and freeCommand() and see if other people might find it interesting to add other promsified functions :)

[AxelTerizaki]

Also, using 2.0 right now, and it seems to work like a charm. I'll have to test it out a bit more, and wait for your update before working on it further :)

[j-holub]

I've added the options argument to load() and append() in Version 2. It doesn't throw an error when you pass an invalid option, because mpv makes it very difficult to check that over the IPC socket. It gives this information only over stdout, which I'm usually not listening to and is set to silent anyway.

The point is, that for every method I have listen to different things to resolve the promise. For example load() is resolved when the playback-restart event is sent over the IPC socket, other methods for different reason.

I haven't found a general way to check if a command was successful or not, that makes it very difficult to promisiy command() and freeCommand().

[AxelTerizaki]

Good thing :) I'll make some tests this evening to see if I have a problem displaying subtitles without the setTimeout.

I'll then get back to you so I can say if it solved my issue or not.

[j-holub]

Sorry I completely forgot to address your play() problem. load() already starts playing the song and should be enough. But yes I totally agree, play() should be promisified. I'll work on that

[j-holub]

Quick update (not sure if it is still relevant though). I have a local version where the method handling the communication itself is promisified, which basically promisifies the whole module in one bunch. I like it.

The problem so far is, to get some good error handling, such that it fails the user knows why and what failed

[AxelTerizaki]

Hello.

In 5 months I've greatly reworked the way mpv is handled in our app. Rewriting it in ES2015 with Babel greatly helped simplify the code (see here at about line 300) and I removed the setTimeout and haven't noticed any problems. Even so, it would be nice to have some kind of promise functions to make sure the file is loaded and in play mode before loading any subtitles.

I'm still using version 2 so far. I've seen the mpv version issue you fixed recently, but we haven't updated our mpv so far (still at 0.27)

Our karaoke party app is really working well, and it's partly thanks to your mpv module :)

[j-holub]

Wow 🙂 Thank you so much, feels nice to hear something like that!

I'm further working on a fully promisified chain. It works pretty well, problem is just the error handling. MPV itself doesn't give a lot of error information via the IPC interface but only via stdout and that makes it difficult to react to specific errors of specific methods.

What you linked me to looks nice, I haven't really worked with the await keyword yet, I might have to look into that.

load() itself should already be promisified for quite a while now with pretty sophisticated error handling (I think at least). Have you had a look? This is the relevant commit.

next(), prev() and skip() return promises as well :)

[AxelTerizaki]

await/async is a feature in ES2015. I don't know if it's supported very well in Node yet (we're still at Node 8 for now for our project anyway) so we use Babel to have it work and it's pretty neat. It makes much less indentation in your code since you can write async functions in a synchronous way for better reading.

Something like

myfunc(args)
    .then(() => {
        doSomethingElse()
    })

becomes

await myfunc(args);
doSomethingElse();

Since it relies on babel I'm not sure if it's a good idea to put it in your code since it's a library aimed at being used by other projects and it would add babel to their dependencies. You should see if/when it's supported in Node, at least.

I'll take a look later at what you pointed out : I have other priorities on the project at the moment and the code we have so far works, but a general cleanup is planned anyway, so when we'll look at our mpv code, we'll be able to optimize it a bit better :)

[j-holub]

Thanks for the info 🙂

I agree, I should aim at supporting older node versions and I don't mind the old syntax (yet). But I'll have a look into await, it really seems to make code a lot cleaner, especially because Version 2 makes heavy use of promises.

I'll close this issue for now since I think it is already fixed in Version 2 (and has been for quite some weeks) since load() is promisified and the promise does not resolve before the song or video is playing.

Feel free to reopen this issue if further problems arise and good luck with your projects :)