running a program with a keystroke - passing playing filename to it
Hello, this is my first post. I want to run a program with a keystroke, and pass the playing filename to it (also the film length, current position..). I see that there is a run command. so putting run "mycommand" at input.conf works. But I don't see how to pass the current filename to it, or other data. Is it possible? (I have a playlist, so the current filename changes with > and <. If it were just one film, I could do something like FILE="$file" mplayer "$file" and mycommand would see FILE on its environment) I see that some commands (like osd_show_property_text) expand ${filename}. But run runs the command as a shell script, and inspecting run "env" I don't see a filename variable defined there. I think mplayer could define some variables for the execing proccess. Like MPLAYER_FILENAME, MPLAYER_POS.. I am about to implement it (unless there is another way to do what I want). Does this seem reasonable?
On Fri, Dec 17, 2010 at 12:44:22PM -0300, Elias Gabriel Amaral da Silva wrote:
I think mplayer could define some variables for the execing proccess. Like MPLAYER_FILENAME, MPLAYER_POS.. I am about to implement it (unless there is another way to do what I want). Does this seem reasonable?
If it was just one thing it might be reasonable, but it sounds like your program might want all kinds of information. You should probably just use slave mode, though the issue is that replies will be sent to stdout which will be hard to access from a child process. Still, since I don't know the purpose I can't say, but I think there's a good chance you should implement this as a slave-mode wrapper instead. However being able to use arbitrary properties in commands might be a useful feature.
On Fri, 17 Dec 2010 19:32:28 +0100, Reimar Döffinger wrote:
On Fri, Dec 17, 2010 at 12:44:22PM -0300, Elias Gabriel Amaral da Silva wrote:
I think mplayer could define some variables for the execing proccess. Like MPLAYER_FILENAME, MPLAYER_POS.. I am about to implement it (unless there is another way to do what I want). Does this seem reasonable?
If it was just one thing it might be reasonable, but it sounds like your program might want all kinds of information. You should probably just use slave mode, though the issue is that replies will be sent to stdout which will be hard to access from a child process. Still, since I don't know the purpose I can't say, but I think there's a good chance you should implement this as a slave-mode wrapper instead. However being able to use arbitrary properties in commands might be a useful feature.
there is a slave wrapper called 'libplayer' which also can wrap vlc. -compn
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 12/17/2010 12:23 PM, compn wrote:
On Fri, 17 Dec 2010 19:32:28 +0100, Reimar Döffinger wrote:
On Fri, Dec 17, 2010 at 12:44:22PM -0300, Elias Gabriel Amaral da Silva wrote:
I think mplayer could define some variables for the execing proccess. Like MPLAYER_FILENAME, MPLAYER_POS.. I am about to implement it (unless there is another way to do what I want). Does this seem reasonable?
If it was just one thing it might be reasonable, but it sounds like your program might want all kinds of information. You should probably just use slave mode, though the issue is that replies will be sent to stdout which will be hard to access from a child process. Still, since I don't know the purpose I can't say, but I think there's a good chance you should implement this as a slave-mode wrapper instead. However being able to use arbitrary properties in commands might be a useful feature.
there is a slave wrapper called 'libplayer' which also can wrap vlc.
-compn
Also, I'm getting close to having a fully functional GTK widget that wraps up mplayer. Some functionality is still not present but the basics work. So I can send you a copy of that as well. Kevin - -- Get my public GnuPG key from http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x7D0BD5D1 -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAk0Q7bsACgkQ6w2kMH0L1dGlBwCeK1dU+zk1AB0+PHkZx25lmPF/ Z5gAnRpoy1gtipAy03OL59VOixch+6t5 =Ad8I -----END PGP SIGNATURE-----
2010/12/21 Kevin DeKorte <kdekorte@gmail.com>:
Also, I'm getting close to having a fully functional GTK widget that wraps up mplayer. Some functionality is still not present but the basics work. So I can send you a copy of that as well.
A mplayer widget looks cool :) But what are you proposing, that I write a gtk app? In order to solve my actual problem, I would have to reimplement all the UI too, right? I'm not always at X when I run mplayer. I may just be playing music, on a linux console
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 12/21/2010 12:19 PM, Elias Gabriel Amaral da Silva wrote:
2010/12/21 Kevin DeKorte <kdekorte@gmail.com>:
Also, I'm getting close to having a fully functional GTK widget that wraps up mplayer. Some functionality is still not present but the basics work. So I can send you a copy of that as well.
A mplayer widget looks cool :)
But what are you proposing, that I write a gtk app? In order to solve my actual problem, I would have to reimplement all the UI too, right?
I'm not always at X when I run mplayer. I may just be playing music, on a linux console
If you are running at the console, obviously a GTK widget is not what you want. Since we do not know your use case, we don't know what the best solution would be. I do find some of your patches interesting, but all the information you want is available via the slave interface. Sounds like you are just adding code to mplayer that may not have any long term benefits. Kevin - -- Get my public GnuPG key from http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x7D0BD5D1 -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAk0RGqwACgkQ6w2kMH0L1dFGygCfeYTW+1j02uvC7NCxs9YNspe6 P5oAn0NjGxaeHr4PgjtkrFpMguqdFnHE =YRZ+ -----END PGP SIGNATURE-----
2010/12/17 Reimar Döffinger <Reimar.Doeffinger@gmx.de>:
On Fri, Dec 17, 2010 at 12:44:22PM -0300, Elias Gabriel Amaral da Silva wrote:
I think mplayer could define some variables for the execing proccess. Like MPLAYER_FILENAME, MPLAYER_POS.. I am about to implement it (unless there is another way to do what I want). Does this seem reasonable?
If it was just one thing it might be reasonable, but it sounds like your program might want all kinds of information.
I use mplayer at command line to handle audio or video file. I don't actually want to use a front-end. It's my music player (it has an equalizer, with find I can have shuffled regex-based playlists..), and so on. My use cases: a) mplayer as an editor sometimes I'm hearing a music that I would like to cut. Maybe I want to discard a piece, maybe I want to split in 2 or more pieces (I don't need to rewrite the file mplayer is playing, I'm satisfied to write new files and eventually unlinking the original one). I am too lazy to open an actual editor, so I just forget, and next time this file pops up, it will bother me again (a GUI editor such like audacity would be inconvenient. I would need to find the offset. And to find I need to hear it again..). I just wanted to press a key to "mark" a point and press another to "cut" the file in two files (or three, etc. if I had more marks). I can move back with arrows and pause for better accuracy. this would be really convenient. mplayer has no business in doing that, but I figured that it could run arbitrary external commands. If only it could pass filename, current point in seconds, and maybe the total length.. b) mplayer as an tagger / rater / organizer sometimes I separate files in categories. sometimes I view a file that is in the wrong category. I usually have few categories, so I could press a key A to signal that it should be in category A. this time I don't need to see the position. also, I might want to rate files as good / junk (I might then run a gc script to discard junk, etc). etc
You should probably just use slave mode, though the issue is that replies will be sent to stdout which will be hard to access from a child process. Still, since I don't know the purpose I can't say, but I think there's a good chance you should implement this as a slave-mode wrapper instead.
I didn't know about slave mode. Compn suggested it at #mplayerdev. It is nice. With rlwrap it seems a nice command prompt. I don't know how to build a layer on top slave that is like regular mplayer, but with this added feature.. (maybe that libplayer would help with this?)
However being able to use arbitrary properties in commands might be a useful feature.
I implemented it, just for filename. (After a lot of time figuring out where to change.. and understanding how to get the filename.. it ended up being quite simple, but I still don't know where this filename variable is defined) I am not sure if I can use the variable filename directly, and not, say, mp_property_path or some other wrapper. I think that mp_property_path gets used only while expanding ${filename} somewhere. I'm puzzled, I think I've never edited a file with 3.5k+ lines.. I see that filename might be null (it is checked for null at mp_property_filename. But not at mp_property_path - maybe m_property_string_ro checks for it?), so I checked it for null. I see that mp_property_stream_time_pos and mp_property_length is used for returning current / total time. But it expects an.. option? I think I need to call mp_property_do to get it. Or something like that. It gets called across that function multiple times, why isn't it defined on top of the function? I think that after getting this option I would call mp_property_length(option, M_PROPERTY_GET, str, mpctx); setenv("MPLAYER_LEN", str); But not entirely sure, will try later. Index: DOCS/tech/slave.txt =================================================================== --- DOCS/tech/slave.txt (revision 32718) +++ DOCS/tech/slave.txt (working copy) @@ -508,7 +508,9 @@ Run <value> as shell command. In OSD menu console mode stdout and stdin are through the video output driver. + It sets the variable MPLAYER_FILENAME to the current playing filename. + Available properties: name type min max get set step comment Index: command.c =================================================================== --- command.c (revision 32718) +++ command.c (working copy) @@ -3375,7 +3375,10 @@ case MP_CMD_RUN: #ifndef __MINGW32__ if (!fork()) { + setenv("MPLAYER_FILENAME", + filename != NULL ? filename : "", 1); execl("/bin/sh", "sh", "-c", cmd->args[0].v.s, NULL); + exit(0); } #endif
2010/12/19 Elias Gabriel Amaral da Silva <tolkiendili@gmail.com>:
2010/12/17 Reimar Döffinger <Reimar.Doeffinger@gmx.de>:
However being able to use arbitrary properties in commands might be a useful feature.
I implemented it, just for filename.
Attached is an implementation for MPLAYER_POS and MPLAYER_LEN as well. I talked with some people at #mplayerdev about it. It was suggested that I should avoid putting complexity at the code and do it with a slave mode wrapper. In my understanding, it would have to open some IPC channel (such as an unix socket), and the command run would have to ask it for such data through this channel. Also, the wrapper would have to handle UI. This would too complex. Also uau suggested that instead of keeping hard-coded environment variables (since the list of them could grow), one should expand mplayer variables on run parameter, like osd_show_property_text does. An issue would be how to handle multiple parameters, but it seems that string parameters already get " escaped, so one could write this command: run "my_script \"${filename}\" \"${length}\" \"${time_pos}\"" Where ${filename} would (deceptively) be expanded by mplayer, not by the shell. In any case, it seems this feature haven't attracted much interest, since it does something that could ultimately be done by slave mode. Uau seems unsure whether my feature is worth enough to be merged, even if I implement it in a more general fashion like he suggested. And I admit that may be the case that it will be useful just for me. Thoughts?
On Tue, Dec 21, 2010 at 09:33:40AM -0300, Elias Gabriel Amaral da Silva wrote: [...]
Also uau suggested that instead of keeping hard-coded environment variables (since the list of them could grow), one should expand mplayer variables on run parameter, like osd_show_property_text does.
Sounds like the correct thing to do indeed.
diff --git a/command.c b/command.c index b95b750..2f8d189 100644 --- a/command.c +++ b/command.c @@ -3375,7 +3375,24 @@ int run_command(MPContext *mpctx, mp_cmd_t *cmd) case MP_CMD_RUN: #ifndef __MINGW32__ if (!fork()) { + char* media_len = malloc(32); + char* media_pos = malloc(32); + + if(mp_property_do("length", M_PROPERTY_TO_STRING, + &media_len, mpctx) <= 0 || + mp_property_do("time_pos", M_PROPERTY_TO_STRING, + &media_pos, mpctx) <= 0) { +
Huh? Isn't the allocations useless? If it is needed, are you sure there is some kind of snprintf with a size limit to 32? And where do you free them? -- Clément B. Not sent from a jesusPhone.
2010/12/21 Clément Bœsch <ubitux@gmail.com>:
On Tue, Dec 21, 2010 at 09:33:40AM -0300, Elias Gabriel Amaral da Silva wrote: [...]
Also uau suggested that instead of keeping hard-coded environment variables (since the list of them could grow), one should expand mplayer variables on run parameter, like osd_show_property_text does.
Sounds like the correct thing to do indeed.
Thank you for your input. It seems that implementing it that way would not be too complicated. It would just amount to a call to m_properties_expand_string and I'm done.. But this interface, expanding things that looks like shell parameters but aren't, seems a bit confusing.
diff --git a/command.c b/command.c index b95b750..2f8d189 100644 --- a/command.c +++ b/command.c @@ -3375,7 +3375,24 @@ int run_command(MPContext *mpctx, mp_cmd_t *cmd) case MP_CMD_RUN: #ifndef __MINGW32__ if (!fork()) { + char* media_len = malloc(32); + char* media_pos = malloc(32); + + if(mp_property_do("length", M_PROPERTY_TO_STRING, + &media_len, mpctx) <= 0 || + mp_property_do("time_pos", M_PROPERTY_TO_STRING, + &media_pos, mpctx) <= 0) { +
Huh? Isn't the allocations useless? If it is needed, are you sure there is some kind of snprintf with a size limit to 32? And where do you free them?
Yes, I had no idea on a proper size. The code as such might have some kind of buffer overrun. It seems that I writing just char* media_len; with no allocation works. I'm attaching a new patch.
2010/12/21 Clément Bœsch <ubitux@gmail.com>:
On Tue, Dec 21, 2010 at 09:33:40AM -0300, Elias Gabriel Amaral da Silva wrote: [...]
Also uau suggested that instead of keeping hard-coded environment variables (since the list of them could grow), one should expand mplayer variables on run parameter, like osd_show_property_text does.
Sounds like the correct thing to do indeed.
Sending this attached. This feels cleaner now.
On Tue, Dec 21, 2010 at 10:51:10AM -0300, Elias Gabriel Amaral da Silva wrote:
2010/12/21 Clément Bœsch <ubitux@gmail.com>:
On Tue, Dec 21, 2010 at 09:33:40AM -0300, Elias Gabriel Amaral da Silva wrote: [...]
Also uau suggested that instead of keeping hard-coded environment variables (since the list of them could grow), one should expand mplayer variables on run parameter, like osd_show_property_text does.
Sounds like the correct thing to do indeed.
Sending this attached. This feels cleaner now.
This is much better, I like this.
[...] diff --git a/DOCS/tech/slave.txt b/DOCS/tech/slave.txt index e31a9f4..5bcf074 100644 --- a/DOCS/tech/slave.txt +++ b/DOCS/tech/slave.txt @@ -508,6 +508,11 @@ run <value> Run <value> as shell command. In OSD menu console mode stdout and stdin are through the video output driver.
+ It expands properties inside the command. Due to an unfortunate + syntax clash, it *looks* like those properties are shell variables, + but then aren't. (for example, in run "echo ${filename}", ^^^^ they?
-- Clément B. Not sent from a jesusPhone.
2010/12/21 Clément Bœsch <ubitux@gmail.com>:
[...] diff --git a/DOCS/tech/slave.txt b/DOCS/tech/slave.txt index e31a9f4..5bcf074 100644 --- a/DOCS/tech/slave.txt +++ b/DOCS/tech/slave.txt @@ -508,6 +508,11 @@ run <value> Run <value> as shell command. In OSD menu console mode stdout and stdin are through the video output driver.
+ It expands properties inside the command. Due to an unfortunate + syntax clash, it *looks* like those properties are shell variables, + but then aren't. (for example, in run "echo ${filename}", ^^^^ they?
oh yes, that was a typo, thanks
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 12/21/2010 12:00 PM, Elias Gabriel Amaral da Silva wrote:
2010/12/21 Clément Bœsch <ubitux@gmail.com>:
[...] diff --git a/DOCS/tech/slave.txt b/DOCS/tech/slave.txt index e31a9f4..5bcf074 100644 --- a/DOCS/tech/slave.txt +++ b/DOCS/tech/slave.txt @@ -508,6 +508,11 @@ run <value> Run <value> as shell command. In OSD menu console mode stdout and stdin are through the video output driver.
+ It expands properties inside the command. Due to an unfortunate + syntax clash, it *looks* like those properties are shell variables, + but then aren't. (for example, in run "echo ${filename}", ^^^^ they?
oh yes, that was a typo, thanks
I'm a little concerned that this patch allows mplayer to execute pretty much any command available at the OS level. I know it is not running as root, but it still concerns me a bit. Kevin - -- Get my public GnuPG key from http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x7D0BD5D1 -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAk0RGdcACgkQ6w2kMH0L1dEiJwCeLrBxLCNkCXUfxMWA3OTTKyOE qq4An0vX6a9jjI83THTZwBMPYfO9QIbx =V1M2 -----END PGP SIGNATURE-----
On Tue, 21 Dec 2010 14:19:19 -0700, Kevin DeKorte wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 12/21/2010 12:00 PM, Elias Gabriel Amaral da Silva wrote:
2010/12/21 Clément Bœsch <ubitux@gmail.com>:
[...] diff --git a/DOCS/tech/slave.txt b/DOCS/tech/slave.txt index e31a9f4..5bcf074 100644 --- a/DOCS/tech/slave.txt +++ b/DOCS/tech/slave.txt @@ -508,6 +508,11 @@ run <value> Run <value> as shell command. In OSD menu console mode stdout and stdin are through the video output driver.
+ It expands properties inside the command. Due to an unfortunate + syntax clash, it *looks* like those properties are shell variables, + but then aren't. (for example, in run "echo ${filename}", ^^^^ they?
oh yes, that was a typo, thanks
I'm a little concerned that this patch allows mplayer to execute pretty much any command available at the OS level. I know it is not running as root, but it still concerns me a bit.
you are a few years too late to voice that concern. mplayer has had the 'run' parameter since 2003 (or earlier). this patch just creates a variable to pass the current filename to 'run'. :) now you should wonder how many boxes have been rooted using mplayer's run command, or why mplayer devels would add a backdoor like this. -compn
On Tue, Dec 21, 2010 at 04:36:00PM -0500, compn wrote:
On Tue, 21 Dec 2010 14:19:19 -0700, Kevin DeKorte wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 12/21/2010 12:00 PM, Elias Gabriel Amaral da Silva wrote:
2010/12/21 Clément Bœsch <ubitux@gmail.com>:
[...] diff --git a/DOCS/tech/slave.txt b/DOCS/tech/slave.txt index e31a9f4..5bcf074 100644 --- a/DOCS/tech/slave.txt +++ b/DOCS/tech/slave.txt @@ -508,6 +508,11 @@ run <value> Run <value> as shell command. In OSD menu console mode stdout and stdin are through the video output driver.
+ It expands properties inside the command. Due to an unfortunate + syntax clash, it *looks* like those properties are shell variables, + but then aren't. (for example, in run "echo ${filename}", ^^^^ they?
oh yes, that was a typo, thanks
I'm a little concerned that this patch allows mplayer to execute pretty much any command available at the OS level. I know it is not running as root, but it still concerns me a bit.
you are a few years too late to voice that concern. mplayer has had the 'run' parameter since 2003 (or earlier). this patch just creates a variable to pass the current filename to 'run'. :)
now you should wonder how many boxes have been rooted using mplayer's run command, or why mplayer devels would add a backdoor like this.
Shall we apply this then? It's a pity a lot of interesting patches get lost like this, and more importantly, potential contributors… -- Clément B. Not sent from a jesusPhone.
2011/1/3 Clément Bœsch <ubitux@gmail.com>:
Shall we apply this then? It's a pity a lot of interesting patches get lost like this, and more importantly, potential contributors…
Maybe I got things confused by sending many patches in the same thread. The property expansion patch seems cleaner, but I don't know a property to return exactly what I need (${time_pos} returns with resolution up to a second, like 1:21, and I want a resolution like 81.323312). I also had quoting problems for filenames that has either ' or " (since run command runs /bin/sh, and it has its own quoting quirks..), that maybe could be solved with some trick (the way I see is to pass the filename as an additional parameter, and refer to it with "$1" - in another patch). So although I sent both, property expansion doesn't seem to do what I need. So I'm using the environment variable patch myself.
participants (5)
-
Clément Bœsch -
compn -
Elias Gabriel Amaral da Silva -
Kevin DeKorte -
Reimar Döffinger