Sixthhokage2 / remuco

Automatically exported from code.google.com/p/remuco
1 stars 1 forks source link

MPlayer support #21

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
This issue has been migrated from SourceForge.

Title : mplayer adapter
Origin: https://sourceforge.net/support/tracker.php?aid=2806396

==================================================================

When: 2009-06-16 16:45:53
Who : ronald750

Hi,
I have an adapter for mplayer, for now only works playback control (next,
prev, volume, seek, playing toggle). I hope soon to add the functionality
to view the status of playback (if possible).

It was tested in mplayer and gmplayer

-------------------------------------------------------------

When: 2009-06-16 16:45:53
Who : mondai

Sounds great :)

Yep, seeing playback state and currently played item on the client would be
great too.

If you feel your code is ready (or needs a review), attach a patch (svn
diff) to this issue.

Feel free to ask for support if needed.

Regards,
Oben

-------------------------------------------------------------

When: 2009-06-16 16:45:59
Who : mondai

BTW .. is there a special reason why you marked this issue private?

-------------------------------------------------------------

When: 2009-06-16 22:42:01
Who : mondai

Looks good at first glance :)

Two issues:
Why do you use 'fd' and 'rdfifo' as globals? Should be no problem to use
them as instance variables in class MplayerAdapter..
How do you see chances to get information about the currently played item
or to get playback state and volume? I would like to see this working
before including the MPlayer adapter in official releases.

Regards,
Oben

-------------------------------------------------------------

When: 2009-07-07 21:13:29
Who : mondai

Hey Ronald,

do you plan to continue to work on this?

Regards,
Oben

-------------------------------------------------------------

When: 2009-07-07 23:54:00
Who : ronald750

Oben

Sorry for the delay, I have been very busy these days.
I suppose that have enough time the next week.

Regards,
Ronald

-------------------------------------------------------------

When: 2009-07-08 11:42:03
Who : mondai

Didn't want to rush you :) Just wanted to know if there is still live on
this issue. I probably won't have time to work on Remuco the next 3 weeks
anyway. So no need to hurry.

-------------------------------------------------------------

When: 2009-08-13 21:57:30
Who : mondai

Ronald, Remuco version 0.9.1 will be released within the next days. If you
had some time to complete the MPlayer adapter and if you want to see your
contribution to part of the next release, please send me a patch soon.

Thanks and cheers,
Oben

-------------------------------------------------------------

When: 2009-08-14 02:56:03
Who : ronald750

Hi

I need to "add a line" to mplayer configuration file, how I do this from
remuco installer?

Ronald

-------------------------------------------------------------

When: 2009-08-14 09:31:22
Who : mondai

What exactly would you like to add? Is it also possible to add this line at
runtime, e.g. within the player adapter's init method? To add the line
during the installation process setup.py needs to be tweaked, which is
possible, but to keep it clean and simple I would prefer to do it only if
there is no other way.

-------------------------------------------------------------

When: 2009-08-14 13:15:47
Who : ronald750

This line only will be added once, during the instalation.
Mplayer needs a fifo for receive commands, and only works if we add the
name of the fifo at the configuration file
~/.mplayer/config

-------------------------------------------------------------

When: 2009-08-14 14:14:39
Who : nobody

Adding a line to a user specific configuration file during the installation
conceptually is a problem. This won't work in most packaging systems, where
files are installed from a root user perspective. Systems may have multiple
users, should this line be added to the mplayer config of all users? What
if new user are added to a system once the mplayer adapter has been
installed? So either this line should be added to a global mplayer config
file during installation process (is there something like /etc/mplayer?) or
users shoud do it themselves. As the mplayer adapter is a script, you could
provide a parameter like 'init' so that users have to run 'remuco-mplayer
--init' once or you could check if this line is in ~/.mplayer/config at
each startup of the remuco-mplayer and add it if it's not there yet - I
think this check is not to expensive to be done at each adapter startup.

I don't want to make things complicate, but as Remuco is packaged for
ArchLinux, Gentoo and Debian/Ubuntu, this point is quite important.

Original issue reported on code.google.com by obensonne@googlemail.com on 5 Sep 2009 at 10:58

Attachments:

GoogleCodeExporter commented 9 years ago
Any news on this?

I started fiddling with the adapter a couple of minutes ago, basically changed 
the
globals to instance variables and edited setup.py, but couldn't get it to work,
though. The client complained there was a timeout in the hello message, and I 
haven't
had time to debug it
It's forked in http://bitbucket.org/igoralmeida/remuco if you want to take a 
look.
I'll try again tomorrow.

Original comment by igor.con...@gmail.com on 28 Sep 2009 at 2:57

GoogleCodeExporter commented 9 years ago
Thanks for continuing this work.

Original comment by obensonne@googlemail.com on 28 Sep 2009 at 5:42

GoogleCodeExporter commented 9 years ago
No problem. It's also my itch to scratch, anyway.
Well, here's the thing:

Mplayer needs to know from which file should it read commands.
Both are valid:
$ mplayer -input file=/path/to/input/file awesomemovie.avi
$ echo input=file=/tmp/rdfifo > ~/.mplayer/config 

So the adapter creates a FIFO in /tmp/rdfifo and mplayer reads from it. I was 
getting
timeout messages because it seems the FIFO holders know whether their 
counterparts
are "holding their own ends". Maybe someone can shed some light on the issue.
After starting mplayer with the above command, the client starts just peachy. My
current problem is in the update: mplayer only prints to stdout, as far as I 
know, so
I haven't gotten around the read-from-mplayer issue yet.

Ideas?

Original comment by igor.con...@gmail.com on 29 Sep 2009 at 2:49

GoogleCodeExporter commented 9 years ago
This issue was updated by revision c7d0b3b605.

The FIFO file to use to control MPlayer is set in the adapters configuration
file now. As default, the file ~/.cache/remuco/mplayer/fifo is used.
In a next step the MPlayer configuration could be adjusted automatically to
point to that FIFO. However, this should only be done if there is no such
option yet in the MPlayer config - otherwise users may get confused what
happened to their manually configured FIFO.

Original comment by obensonne@googlemail.com on 30 Sep 2009 at 8:50

GoogleCodeExporter commented 9 years ago
This issue was updated by revision 48f34a9539.

Writing commands to the FIFO is now done in a try-catch block to catch the
error which occurs when MPlayer exits. In that case the adapters exits too.
Another option would be to restart the adapter by calling self.stop() and
self.start() subsequently.

Original comment by obensonne@googlemail.com on 30 Sep 2009 at 8:50

GoogleCodeExporter commented 9 years ago
Concerning the update .. would it be possible to redirect the stdout of MPlayer 
to a
file and reading that file for status information?

BTW .. I've merged your changes into the mainline and did some additional 
changes
(see previous 2 posts). So you may want to pull the mainline into your fork 
before
continuing to work on the adapter.

Original comment by obensonne@googlemail.com on 30 Sep 2009 at 9:54

GoogleCodeExporter commented 9 years ago
Great.
I have some changes here but let me add some more functionality before 
uploading it.

A hackish way to read mplayer information is to tee its output to a file. The 
user
would have to do "mplayer /bunch/of/files*.avi | tee /path/to/info_file", or 
alias it
to work, somehow, while still being able to read it from the command-line where
mplayer was originally created, if any.

I'm not an advocate of this solution as it kind of disrupts the principle of 
having
remuco NOT interfere (too much) with the player initialization and such. If we 
go
down this path, we might as well opt for running mplayer as a slave, then it 
would
read from stdin and print to stdout: much easier and cleaner code.

Having a tee'd file, there's still the problem of reading it. It's ugly. A 
bunch of
initialization lines and the last one with millions of carriage returns. I'm 
thinking
of a way to do something similar with 'tac' to the file, then the last line 
would be
printed first and we wouldn't have to search much for 
/[A-Z]+='?somethinghere'?/ lines.

Original comment by igor.con...@gmail.com on 4 Oct 2009 at 3:13

GoogleCodeExporter commented 9 years ago
It's a pity there isn't a FIFO option for the output. Could tee write to a FIFO?

I wonder if there is a nifty solution to get information from mplayer at all, if
stdout is the only interface for that.

Original comment by obensonne@googlemail.com on 4 Oct 2009 at 7:39

GoogleCodeExporter commented 9 years ago
Hope you don't mind that I've set you as the owner of this issue :P

Original comment by obensonne@googlemail.com on 4 Oct 2009 at 7:48

GoogleCodeExporter commented 9 years ago
No problem.

Hope you don't mind that I'll rely on your responses to come up with ideas and
questions se we can fix this :)

So tee can, yes, write to a FIFO, and that fixes our current status problem. 
I'll
write something later today to try to pick up some of the interesting 
responses. An
immediate problem is that poll() won't catch the filename if it is called every 
now
and then. We'd need a separate thread to keep the adapter from running an 
infinite loop.

Original comment by igor.con...@gmail.com on 4 Oct 2009 at 8:06

GoogleCodeExporter commented 9 years ago
The way to go here is to use gobject IO callbacks (no need for an additional 
thread).
Here is an example how the Remuco server uses that to wait for incoming client
connections without using an extra thread for that:
http://code.google.com/p/remuco/source/browse/base/module/remuco/net.py#410 
(BTW ..
Remuco is completely single-threaded)

For the FIFO to read you could do it like this:

def start(self): # player adapter start method
    self.fifo_fd = open("path/to/fifo-from-tee")
    self.watch_id = gobject.io_add_watch(self.fifo_fd, gobject.IO_IN |
                                         gobject.IO_ERR | gobject.IO_HUP,
                                         self.on_fifo_input)

def on_fifo_input(self, fd, condition):
    if condition == gobject.IO_IN:
        # read from fd (fd is self.fifo_fd)
        self.update_...()
    else: # condition is gobject.IO_ERR or gobject.IO_HUP
        # error, close fifo and stop adapter or what else seems appropriate

Original comment by obensonne@googlemail.com on 4 Oct 2009 at 8:38

GoogleCodeExporter commented 9 years ago
Forgot to mention:

The on_fifo_input method must return True if it shall be called again the next 
time
there is input on the FIFO (normal case). On errors it should set self.watch_id 
to
None and return False.

Additionally the player adapter down method should so something like this:

if self.watch_id is not None:
    gobject.source_remove(self.watch_id) 

Original comment by obensonne@googlemail.com on 4 Oct 2009 at 8:43

GoogleCodeExporter commented 9 years ago
This issue was updated by revision 5542515719.

Still incomplete, needs the actual implementation of the string parsing.
Also changed variable naming to cope with both fifos in use now.
User needs to tee the output of mplayer as in the command below:

$ mplayer /path/to/movies*.avi | tee ~/.cache/remuco/mplayer/statusfifo

... or wherever he wants his statusfifo to be.

Original comment by igor.con...@gmail.com on 5 Oct 2009 at 7:56

GoogleCodeExporter commented 9 years ago
This issue was updated by revision ec2df7d474.

AFAIK, there's no sense in waiting for the user to provide a fifo after
mplayer has started, so just issue a warning and keep on loading the adapter.
This means the client will be able to control the player, but it will not
receive messages with information.

Original comment by igor.con...@gmail.com on 9 Oct 2009 at 9:16

GoogleCodeExporter commented 9 years ago
This issue was updated by revision e7d66db30f.

Adding regexes for audio files (not tested) and video files (tested with AVIs,
but should be the same for every video file).
The groups are named, so you can get the matched information with something
like
    matchobj = self.video_regex.match(status_str)
    print matchobj.group('audiopos')

Client information retrieval is *still incomplete*. Ideally, __inputevent()
will modify a class member so that poll()'s next call will send the most
recent information to the client.

Original comment by igor.con...@gmail.com on 9 Oct 2009 at 9:16

GoogleCodeExporter commented 9 years ago
Just skimmed over your last changes .. looks like a good piece of clean code :) 
Here
you'll find some comments:
http://code.google.com/p/remuco/source/browse/adapter/mplayer/remuco-mplayer?r=e
7d66db30fe007d980c4babcdad3e95a4394f6a2

Original comment by obensonne@googlemail.com on 9 Oct 2009 at 10:42

GoogleCodeExporter commented 9 years ago
I was going to write the responses on the code itself, like you did, but I 
think it
is better (organization wise) to keep everything on one place.

* Why not compile the regexes on __init__()?
http://code.google.com/p/remuco/source/browse/adapter/mplayer/remuco-mplayer?r=e
7d66db30fe007d980c4babcdad3e95a4394f6a2#85
I'm assuming start() will be called everytime the instance has gone through a 
stop()
"procedure", so in case mplayer quits and the user turns it back on, the 
adapter will
have to check both fifos again, and it makes sense to have regexes compiled 
only if
you can read from statusfifo.
Though it indeed compiles the same thing everytime start() is called, I think 
it's
cleaner this way. If I were to move the regex compilation to __init__(), I'd add
similar checks for the statusfifo, to maintain "context".
Let me hear your thoughts on this.

* Call update_*() directly in __inputevent()
http://code.google.com/p/remuco/source/browse/adapter/mplayer/remuco-mplayer?r=e
7d66db30fe007d980c4babcdad3e95a4394f6a2#203
I didn't want to do this because mplayer is VERY verbose, but since the core 
does not
blindly send the info, it might be ok.
update_progress() will be called every second or so, since it truncates the 
float
progress to int. How does it compare to other adapters' update period?

Original comment by igor.con...@gmail.com on 10 Oct 2009 at 2:49

GoogleCodeExporter commented 9 years ago
Concerning the start/init thing, I don't care that much. If you have reasons 
for it
to put in start()( instead of __init__(), that's fine, just wanted to know ..

Concerning the update methods, it just would be duplicate work to check for 
changes
as it is done internally anyway.
To prevent sending progress data every second to clients, internally progress 
updates
are rounded up/down to a multiple of 5. Independent how often you update the
progress, it gets sent to clients at most every 5 seconds.

Original comment by obensonne@googlemail.com on 10 Oct 2009 at 10:38

GoogleCodeExporter commented 9 years ago
That's good to know. I noticed the progress were 5 seconds apart from each 
other, but
poll was running 2500ms, so it didn't add up. Thank you for the clarification, 
and to
remind me that I need to take a better look at remuco's code :)

Original comment by igor.con...@gmail.com on 10 Oct 2009 at 2:41

GoogleCodeExporter commented 9 years ago
I plan to make a new release tomorrow, approximately 3pm GMT. Do you think the
MPlayer adapter is ready until then or do you prefer to have some more time and 
to
include it in 0.9.3?

Original comment by obensonne@googlemail.com on 10 Oct 2009 at 5:28

GoogleCodeExporter commented 9 years ago
Sorry, I couldn't answer earlier.
In my fork you will find the adapter with working support for playback status,
pause/unpause, seek, next and previous playlist items, volume control (no 
automatic
mute), fullscreen and filename information in the client. Since apparently I am 
also
the only tester, you must be careful with what I mean by "working support" :)
Despite sending the progress and length information to the client, no 
progress/length
is shown. A really quick look at amarok's adapter tells me there is nothing 
missing,
but I'm clearly wrong.

Concerning the release, I think 635e045a92d6 is ready for public consumption. I 
can
think of a few immediate caveats, though:
* the aforementioned progress issue
* a small lag between volume control and the updated information in the volume 
bar
* the mute key acting as volume down
* O_NONBLOCK means no windows portability (it probably wasn't portable long 
ago, but
whatever)
* 'seek' and 'next/previous item in playlist' use the same keys, so it is kind 
of
annoying having to hold it for a small fast forward (you either fforward too 
much or
you jump to the next item)

Original comment by igor.con...@gmail.com on 11 Oct 2009 at 5:14

GoogleCodeExporter commented 9 years ago
Nop. My plans have been crossed anyway, was busy with other things the last 2 
days.
I'll look into the adapter soon and possibly already include it in the release.

Original comment by obensonne@googlemail.com on 12 Oct 2009 at 6:57

GoogleCodeExporter commented 9 years ago
This issue was updated by revision f78f5c5d8d.

File statusfifo is no longer checked for existence. Instead, it is created as
a FIFO if it does not already exist.
Move regex compilation to __init__(), since there is no longer a check for
statusfifo's existence.
Change statusfifo opening flags to O_RDONLY|O_NONBLOCK, allowing the script to
continue even if mplayer does not start or does not output to statusfifo.
However, according to [1], O_NONBLOCK is only available on unix systems.
Add a self.info dictionary to hold the current item's information.
Make stop() return the instance to its original "state", as far as file
descriptors and watch_id are concerned.
Add update functionality in poll(): item (filename and length)
Change the buffer size for os.read from 100 to 200, again another magic
number, to be able to catch the PAUSE message. I am assuming it will always
have a constant number of '=' signs, followed by a space and a localized
version of "PAUSED". Therefore, no check is performed after the space.
Add pause detection in __inputevent()
Add a more robust "matching regex object detection" using named groups in the
regexes themselves.
Add helper methods __set_info() and __set_default_info() to change and
(re)initialize the self.info dict, respectively.

[1] http://docs.python.org/library/os.html

Original comment by igor.con...@gmail.com on 13 Oct 2009 at 8:57

GoogleCodeExporter commented 9 years ago
This issue was updated by revision 635e045a92.

Mplayer adapter now sends volume information to client.
info_regex updated to comply with get_property response.
poll() now fetches volume information along with filename and length.

Original comment by igor.con...@gmail.com on 13 Oct 2009 at 8:57

GoogleCodeExporter commented 9 years ago
If merged in your changes and applied some additional changes. You'll find
explanations in the commit messages:
http://code.google.com/p/remuco/source/list?path=/adapter/mplayer/remuco-mplayer
 . I
guess its better if you check if my changes also work for you.

Finally, looking at the messy situation when reading from MPlayer's stdout, you 
did a
good job in handling this. It's still a bit tricky to use the adapter, but I 
think
guys using MPlayer aren't afraid of freaky command line actions ;)

BTW .. concerning the "Update issue XY" magic in commit messages, please only 
use it
for (if possible short) comments which contribute to the discussion of an issue 
--
technical details or listings of individual changes should be written _above_ 
the
"Update issue .." line.

Original comment by obensonne@googlemail.com on 13 Oct 2009 at 11:43

GoogleCodeExporter commented 9 years ago
This issue was updated by revision 682d9d7096.

Added file browsing support to the MPlayer adapter. Still not perfect,
but usable (not easy to be perfect when interacting with MPlayer).

Original comment by obensonne@googlemail.com on 14 Oct 2009 at 12:21

GoogleCodeExporter commented 9 years ago
Great. I'll take a look at it tomorrow.

About the 'update issue' thing, well, I was merely copying you :P
Anyway, we should do it different this time. Now that it seems certain that
remuco-mplayer will be included in the next release, we should close this issue 
and
start opening separate usability issues. My last comment (21) already has a few 
of
them, some of which you've already fixed. Thoughts?

It's indeed tricky to work with mplayer-adapter, it only works for a single 
"session"
for now, AND it's still missing DVD information. I'll mess around with it later 
this
week.

Original comment by igor.con...@gmail.com on 14 Oct 2009 at 1:11

GoogleCodeExporter commented 9 years ago
Ok, let's close this issue and handle improvements in new issues. I've some 
ideas how
to improve the fifo handling from a usability point, but this still needs some
testing. I'll write more about this in a new issue.

I'll include the MPlayer adapter in its current state in the next release.

Original comment by obensonne@googlemail.com on 14 Oct 2009 at 6:20