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

Using freeCommand() isn't working aka add a method to display ASS subitles #30

Closed AxelTerizaki closed 6 years ago

AxelTerizaki commented 6 years ago

Hello.

I'm not sure if I'm doing it right or wrong, but I'm trying to write a command to mpv for sending text to the OSD during playback of a file.

I've first tried using the command function like this :

mpv.command('show-text',[message,duration,0]);

It worked fine, but then I wanted to position it and wanted to use ASS sequences. However, ASS is escaped since the ${osd-ass-cc/0} is not expanded. Apparently you use the mp.commandv method in mpv's API to send commands with this function. There's probably a good reason behind it and I'm not questioning that.

So I tried to use freeCommand() instead, and tried to set it up like that :

var command = { "command": ["show-text", "test", "10000", "0"] }
_player.freeCommand(command);

Alas, it's not displaying anything, even with the integers without double-quotes around them, I can't get any command to run, not even simple ones like "quit".

Am I doing something wrong with freeCommand?

Or could it be possible to have a command function that calls mp.command on mpv's API instead of mp.commandv ?

Thanks in advance :)

j-holub commented 6 years ago

Hey there,

I have never worked with ASS sequences. As you can see

bildschirmfoto 2017-09-09 um 13 12 52

freeCommand() does nothing but sending exactly what you have entered as a string with an added "\n" to mpv. Your command is a JSON object in this case, this might (not sure though) cause the problem. Have you tried the following?

_player.freeCommand(JSON.stringify(command));

If this should solve the problem, I think something like freeCommandJSON() would actually be a great idea though...

I'm sorry but I don't completely get why command() is not working and what you mean by expanded.

I'm also unsure about command and commandv?

AxelTerizaki commented 6 years ago

I'll try with JSON.stringify, thanks :)

For my problem, see the issue I opened on mpv's github, someone told me your API probably used commandv instead of command :

https://github.com/mpv-player/mpv/issues/4847

mpv's command interface can expand properties so you can use them in your command. A simple one would be like "show-text ${progress}" or something similar to display something that is variable on screen.

j-holub commented 6 years ago

That's actually very valuable information, thank you very much!

I have an exam on monday so I can't look deeply into it this weekend, but be sure I will afterwards :)

AxelTerizaki commented 6 years ago

No worries, I'm in no hurry :)

Thank you for your node-mpv framework, it's actually very useful for my karaoke project :)

AxelTerizaki commented 6 years ago

Okay, freeCommand works better once the JSON is stringified, I should have thought about that.

My problem is that the ${} sequence isn't run, so it displays as is, same problem as what I ran into with command()

If you have some time, I'd really appreciate a way to send commands the way it's discribed in the mpv issue, where I could use ASS to format what I print on screen. :)

Right now I generate .ass files containing what I need and load them with the video file (like the name of the song being played, who requested it, etc.) but if I could display it directly on mpv's OSD once the file is loaded, that would be ideal. :)

Good luck on your exams.

j-holub commented 6 years ago

Glad it worked :)

I will add something like a freeCommandJSON() method and look into that commandv thing.

If I remember correctly, ass files are subtitles right? I used to watch a LOT Japanese dramas and sometimes anime with subtitles and If I remember correctly, ass were that ones that could do fancy things with color and stuff, as opposed to srt files.

By the way, have a taken a look at version 2 of Node-mpv yet? It breaks the API a little but I think it's a lot more robust and stable and already has some more features. You can find it here

AxelTerizaki commented 6 years ago

Version 2 looks nice, I'll look into it more once I have eliminated some bugs and such on my own project :)

ASS files are indeed subtitles files, they provide more flexibility than SRT namely with style and positionning. We use it for karaoke because it allows to color part of the subtitle depending on where you are in the line being sung (so people can follow the colorization to sing)

We don't do many fancy effects apart of this one, but ASS has a lot of possibilities. What I wanted to do was display some info in the bottom-left corner of the video during its first 8 seconds.

j-holub commented 6 years ago

Hey so I checked the documentation of mpv again and didn't find anything about vcommand yet. When I'm using the IPC API I'm using

{"command": ["foo", "bar"]}

so I assume I'm using command not vcommand. Due to lack of time I didn't actually try it yet to just write vcommand there.

But when you used it with JSON.stringify() it worked you said right? My idea would be, to see if I can find out wether the argument was a string or JSON object (I think JavaScript can) and stringy the JSON object. This way you could just use a JSON object as a command, which I think is nicer.

I'd remove the string part in the documentation (but leave it in the code for backwards compatibility) to slowly deprecate the string version, because let's be honest, the JSON version is more pleasant to use any day.

On a different note: this module has a subtitle section. Do you have any requests or suggestions for handy functions, that are not yet there but not too specific, such that they would make a valuable addition to the module.

AxelTerizaki commented 6 years ago

Hmmm, maybe we should ask on the issue I opened on mpv about this then?

In my understanding, one version of the command escapes the mpv options, the other does not.

I should try using the Javascript scripting provided by mpv to see if it works any better. I just need to figure out how to use it first (never connected to mpv directly, hence why I use your module :p)

You can send javascript functions (or lua even) for mpv to execute this way...

As for subtitle functions, honestly we're pretty happy with what node-mpv provides. mpv handles the ASS parsing for the karaoke.

At this moment, we modify the ASS we send to mpv to add information about the song, like singer, title, and who requested it. If we could make mpv display those messages via commands, it could be very useful. Same as allowing the DJ to display messages (formatted via ASS) on the screen for everyone to see, to announce stuff like "Only 5 more songs!" and such.

JSON.stringify did work when using freeCommand but it did escape the mpv options to enable ASS parsing

j-holub commented 6 years ago

Okay I see about the escaping thing. I will have a second look.

Do you have any example files I could work with for debugging?

AxelTerizaki commented 6 years ago

Hmmm, not really.

What you'd have to do would be to load a video (or even a .jpg or .png and put the player on pause) and try to send it show-text commands with the proper sequence.

According to this comment https://github.com/mpv-player/mpv/issues/4847#issuecomment-328176578 the show-text command should expand the parameters provided and not display them, but instead stylize your input text ( {\an1} will put your text in the lower left corner of the screen)

I'll see about trying to make it work via a JS script inside mpv first, or via input.conf tomorrow if I can find some time. Just so I can confirm we've got the right syntax in the guy's comment.

AxelTerizaki commented 6 years ago

For now I've tested this out :

Create a input.conf file in the mpv directory and put this into it :

g show-text "${osd-ass-cc/0}{\\an1}${osd-ass-cc/1}hello"

then launch mpv with mpv --include input.conf and hit the g key, it will run the command and thus show a "hello" in the lower left corner of your mpv window.

So the sequence for using ASS format seems to work fine, we just need to find a way to run it through the command interface too.

AxelTerizaki commented 6 years ago

I did try a few things, and it doesn't seem to work via the IPC server (which you seem to be using.)

Someone suggested we send a script to mpv via the load-script command, but I don't know how to make it listen to commands I could send to it after loading the script and then running them. If you've tried scripting for mpv in LUA/JS before, this could be done as a workaround I suppose.

I reopened the issue on mpv's end to ask for a simpler way to do parameter expansion when simply using the IPC show-text command : it seems it only works via scripting inside mpv or via input.conf file

AxelTerizaki commented 6 years ago

Okay, tried some new things today.

As mentionned in that comment : https://github.com/mpv-player/mpv/issues/4847#issuecomment-329162213

I tried the script provided. You can test it out easily, run mpv like this :

mpv --input-ipc-server=\\.\pipe\mpvsocket --script="H:\\show-text.lua"

I've put my script in H:\ :

mp.register_script_message("show-ass", function(s)
    mp.command(s)  
end)

When I run mpv by hand, the player responds okay to this command :

echo { "command": ["script-message", "show-ass", "show-text ${osd-ass-cc/0}{\\an7}hello 10000"] } >\\.\pipe\mpvsocket

It displays Hello in the upper left corner. You can change it to upper center by replacing an7 by an8.

My problem is when I try launching it through node-mpv with debug on, I see it receives the client-message but the "hello" won't be displayed on screen.

Here's some pice of code I used :

var mpvOptions = [
                '--keep-open=yes',
                '--idle=yes',
                '--fps=60',
                '--no-border',
                '--sub-codepage=UTF-8-BROKEN',
                '--volume=100', 
                '--script="H:\\show-ass.lua"',      
            ];      
var mpvAPI = require('node-mpv');
module.exports._player = new mpvAPI(
{
                    audio_only: false,
                    binary: mpvBinary,
                    socket: '\\\\.\\pipe\\mpvsocket',
                    time_update: 1,
                    verbose: true,
                    debug: true,
                },
                mpvOptions
);

Note that I tried a few things for the sript, like H:\ or simply show-ass.lua (by putting the lua script either in mpv's binary folder or in my root project's source folder)

I'm not sure what I could test more at this point, it looks like it doesn't load the script when I use it through node-mpv.

Any ideas?

j-holub commented 6 years ago

Hey sorry I got back to you so late, really busy with studying at the moment.

Okay one small side note: you don't need idle=yes, it's added internally anyways :)

So it seems, you can't send those commands for the ASS files directly over the IPC socket, right? But I can load them when starting mpv, which is kinda not an option because mpv is running all the time. load-script=yes is also only passed when starting mpv but that's the same time you have to pass the scripts right?

You wanna send those commands at runtime with something like mpv.command(<some ass stuff>) and I think that is how it should be as well. I will enter the conversation in your issue on the mpv side as well and ask the guys over there.

We have to figure some way around their API, gotto figure out some way to pass those commands and I'd like to pack it into an easy to use function, hiding the complicated parts behind that.

I haven't used Windows in like 6 years, so I have no idea about how the pathes are handled there, I'm sure you've already triple checked the path.

Have you tried using the script-message command via mpv.command() or mpv.freeCommand() yet?

AxelTerizaki commented 6 years ago

Thanks for your reply, and don't worry, I'm not in a hurry : for now I'm generating the ASS files I send to mpv on the fly before each song starts. I could avoid this if I could send messages to the screen directly, so it's not a blocking issue.

My problem is not sending messages to the socket, I actually can do it very well.

I tried starting mpv on its own with --script=myscript.lua and it works well : I can send commands to the IPC server via the commandline and it made the ASS in the show-text command work through the script avih sent to me on the mpv issue.

However, when I start it the same way (or so I think) in node-mpv, I can see it receives my commands launched from the commandline, but it looks like the script isn't loading.

And yes, what I'd ultimately like is a command to send text with parameter extension. Avih suggested something interesting though, using expand-text, so I'll try that out.

AxelTerizaki commented 6 years ago

expand-text worked like a charm.

Example :

var infos = '{\\bord0.7}{\\fscx70}{\\fscy70}{\\b1}'+series+'{\\b0}\\N'+__(kara.songtype+'_SHORT')+kara.songorder+kara.title+'\\N{\\i1}{\\fscx50}{\\fscy50}'+__('REQUESTED_BY')+' '+requester+'{\\i0}';

var command = {
    command: [
        'expand-properties',
        'show-text',
        '${osd-ass-cc/0}{\\an1}'+infos,
        8000,
    ]
};              
module.exports._player.freeCommand(JSON.stringify(command));

Will display something like this during the first 8 seconds of playback :

capture d ecran 2017-09-14 11 44 40

Maybe there could be a command in node-mpv to do it better (by using expand-properties as an implicit argument) but other than that, I'm pretty happy with the results, thanks for your help :)

j-holub commented 6 years ago

+1 for Persona 5

Sounds really good. Yes it should be easy to implement a command like this. Should this be specific for subtitles, so something like sendASS() or just as a option for command()? Actually as I'm typing this doing both looks like a good idea right?

I have no idea if this is required for anything else but your use case, but if it is, people can just use command() with the option.

And adding a specific command within the subtitles section will draw more attention to that, if somebody is looking for subtitle things he/she might not check the command section and might not even be aware of the problem.

AxelTerizaki commented 6 years ago

I think adding the commend sendASS() or rather, displayASS() sounds better, because expanding properties might not sound like something people would associate to the ability of sending ASS directly to mpv.

In the description you can explain all it does is use show-text with expand-properties before it, and prefixing anything with ${osd-ass-cc/0}{\\an1}

Here's an example I did to display any message sent by the admin on the center of the screen during playback, to warn people or give useful information :

message: function(message,duration) {               
        var command = {
            command: [
                'expand-properties',
                'show-text',
                '${osd-ass-cc/0}{\\an5}'+message,
                duration,
            ]
        };
        module.exports._player.freeCommand(JSON.stringify(command));
    },

If someone uses the message() function here with message and duration (might want to add minimum osd-level too, to be compliant with the show-text command) it will display message with ASS enabled on the center of the screen.

j-holub commented 6 years ago

Cool.

So the commond is expand-propertiesand show-text is like an option or do you have to use them both all the time? show-text shows text I assume. Are there any other commands that might be useful to you or fellow subtitle makers?

After having a quick look at OSD in the mpv reference I would suggest using osd level 1.

AxelTerizaki commented 6 years ago

show-text is the usual show text command used in mpv.

When using expand-properties beforehand, you give it commands and text as arguments, and it executes them, so yes, show-text is like an argument of expand-properties here.

At least that's how I see it.

As for other features, now that you mention it... :)

To avoid big volume differences when going from one song to another in the playlist, we calculated audio gain through a ffmpeg script beforehand.

We need to pass the gain modifier to mpv when loading a video so it adjusts the volume accordingly.

The load() command in node-mpv doesn't offer any parameters other than append/replace. However, if you use command() this way...

_player.command('loadfile',[video,'replace','replaygain-fallback='+gain]);          

We pass the loadfile mpv command manually to mpv, with the video path as first argument, replace, and then another option which is replaygan-fallback=, with an adjustment variable in decibels, like -5.04 for example.

I think the node-mpv load() function should allow to pass on additional parameters (other than append/replace).

Should I open another issue for this?

j-holub commented 6 years ago

One question: can I always add ${old-ass-cc/0}{\\an5} ? or replace the 0 (position center,right?) by a variable?

So that the method could like displayASS(message, position, duration) and it would become

{command: [
    'expand-properties',
    'show-text',
    `${old-ass-c/${position}}{\\an5}${message}`,
    duration
]};

For the gain thing: I actually have a node repo lying around somewhere, where I tried to build a "gain adjustment module". Haven't worked on it for a while though. Problem is, that it takes very long on weak hardware like the Raspberry Pi (where my MusicServer project was targeted for).

I like the idea actually and I would appreciate taking this to another issue :)

As always thanks for the valuable contribution

AxelTerizaki commented 6 years ago

Yes, gain takes a logn while. We have 4800 videos in our database and it took like 4 hours on my i7 2600K to calculate everything.

A contributor to my app later showed me you could use the -vn option (video null) in ffmpeg to disable video rendering entirely, which sped things a lot for gain calculation, but I haven't retried with the full database yet.

Anyway, back on topic :

${osd-ass-cc/0}{\\an5} are actually two commands

j-holub commented 6 years ago

old was my stupid autocorrection :D

I've seen the numpad part in the issue yes, makes sense actually.

Okay that sounds all really good, so I think we could agree, that both position and duration should be an argument for the method, right?

About that expension, I assume that there might be situations where people might not wand to enable parameter expansion? So should it be optional or do you think people will always want to use that?

AxelTerizaki commented 6 years ago

I think position can be a safe parameter in your function. By default it should be at 7 (upper left of the screen). By safe I mean that people will always want to position their text to display easily, so it's always a plus not having to type {\anX} in your message.

Duration should be arguments too yes.

I can't think of a use case where people would want to disable parameter expansion, but I could be wrong. Note that if you disable it, {\an7} will display as is, obviously.

j-holub commented 6 years ago

I've implemented the function like this and it worked with my tests

bildschirmfoto 2017-09-18 um 11 29 13

I tried using ES 2015 String.raw which would make it possible to use single backslashes without the need to escape them. But since I have to escape the $ sign for the `${osd-ass-cc/0} command, cause it otherwise but be mistaken for a template string it didn't work out.

Could you maybe help me with what I have to pay attention for the documentation? Like special cases and what people should keep in mind when using this? You can also do this via a pull request if you want to or just tell me the important parts and I will do it, whichever you prefer.

AxelTerizaki commented 6 years ago

Sure, I can try a pull request. I'm not much used to github as I have my own gitlab instance (http://lab.shelter.moe) but I'll see what I can do to add to the documentation given all the tests I made.

j-holub commented 6 years ago

If you prefer I can write it as well, I just need some info for things people will have to look out for :)

AxelTerizaki commented 6 years ago

Well, here's what I found out :

Keep in mind these are mpv limitations

And that's all I can think of at the moment :) Hope it helps !

j-holub commented 6 years ago

Thanks a lot :D

j-holub commented 6 years ago

I've added documentation, I hope it's fine that way. I will publish version 1.4.0 and see that I can add this stuff into Version 2, which requires a little more work. I highly encourage you to try out Version2 but keep in mind, that it's yet subject to change.

j-holub commented 6 years ago

And if while working with ASS subtitles you find more things, that you think could enhance this module and make life easier for you, let me know. I'm always happy to add useful features.

AxelTerizaki commented 6 years ago

I've seen that, gratz for releasing it.

How usable is version 2 ? I could actually be very interested because we're running into a problem which could be solved with load() being a promise now (if I understood how version 2 works correctly.)

To explain how it works, it's simply because we need to load external subtitles after loading a video, but if we do it too soon (right after calling load) the subtitle doesn't get loaded. A promise load() would fix that.

I'm not creating an issue because I believe it's fixed in version 2 but I'll have to test it out.

j-holub commented 6 years ago

Yes version 2 would fix exactly this issue.

It is very useable I think. The point is only, that I am not yet sure if every single method should return a promise or only some should. So far the code has blown up pretty much because every method handles the checking for itself using promises and such. And I'm thinking about wether the low level method that sends messages over IPC should already return a promise.

The problem is however, that mpv doesn't tell anything of value over IPC, only {error: 'success'} which even shows up, when the command was not successful.

But for the functional it has so far it runs pretty stable and is pretty useful

mpvPlayer.load('file.mp3)
.then(() => {
    return mpvPlayer.seek(100);
})
.then(() => {
    return mpvPlayer.getPosition()
})
.then((position) => {
   console.log(position);
})
.catch((error) => {
   // catches all errors from load(), seek() and getPosition()
   console.log(error);
})

It blows up the code but it so much more usable and robust. Hope this helps

j-holub commented 6 years ago

Keep in mind, that with version 2 you have to manually start mpv. Because it waits internally for the IPC socket to be created until it connects and stuff. This was the most painful breaking change I had to make (and the reason why I went for version 2 instead of 1.X but it was absolutely necessary.

let mpvPlayer = new mpv();
mpvPlayer.start()
.then(() => {
    return mpvPlayer.load('file.mkv');
})
.then(() => {
    mpvPlayer.displayASS('Hey what's up', 4000, 3);
})
.catch((error) => {
    console.log(error);
});

Though displayASS is not yet present in V2, trying to add that as soon as possible :)