Please add MPV.get_property_osd and MPV.register_event

[McSinyx]

I'm writing a mpv front-end using your module and found this example in Lua. Both of those functions are described here.

Thanks a lot for writing and maintaining this module :bowing_man:

[jaseg]

You could already register event handlers with MPV.register_event_callback, but I have now added a decorator that makes this a little easier.

import mpv
player = mpv.MPV()
@player.event_callback('start-file')
def frob(event):
    print("Starting!")

OSD property values are now accessible (read-only) via MPV.osd.foo, e.g.

import mpv
player = mpv.MPV()
print(player.osd.volume)

[McSinyx]

Thanks for so quick commit!

The osd properties works great, but I don't seem to understand the event callback thing. What I want is something similar to the Lua example (mp.register_event("tick", update_status_line)) because observing time-pos is expensive as it's a float with high precision (it get updated to frequently). So I tried to use the event_callback decorator (also is there an easy to understand doc to that, still I don't quite get how this work?) like this, but it doen't seem to work:

#!/usr/bin/env python3
import mpv

player = mpv.MPV(ytdl=True, input_default_bindings=True, input_vo_keyboard=True)
@player.event_callback('tick')
def foo(event):
    print(player.osd.time_pos)

player.play('https://youtu.be/DLzxrzFCyOs')
player.wait_for_playback()

del player

About your latest commit, is there anything wrong with automated docstring insertion? Also could you remove the trailing crunch in line 18 and Vim config in line 31 (you could use setlocal in your vimrc because each person has his/her own config).

[jaseg]

The TICK event is not subscribed by default and there is no way to subscribe to it through the high-level API. I just pushed a little fix and now what you want to do can be achieved with the following:

import mpv
player = mpv.MPV()

@player.property_observer('time-pos')
def do_foo(_name, _value):
    # Here, _value is either None if nothing is playing or a float containing fractional
    # seconds since the beginning of the file.
    print(player.osd.time_pos)

The property observers are synchronized to MPV's ticks, so you'll only get one time-pos update per frame.

As for the vim modeline, since this python module uses quite a bit of careful manual formatting you cannot automatically retab it. Removing this would just invite people to accidentially screw up the file's formatting.

[McSinyx]

you'll only get one time-pos update per frame

That's still expensive. Can I observe osd property or something that change each second? Or maybe I think I'll come up with a thread that update status every second :stuck_out_tongue:

Still, the trailing crunch and wrong docstrings remains.

[jaseg]

I tried, you can't observe the OSD properties. You could ignore 19 out of 20 calls, they're really not that expensive (they're coming from a python thread reading from a socket). You could also use some other timing mechanism. I would rather recommend using os.signal and SIGALRM (on UNIX) instead of a thread here because threads are expensive.

As for the crunch character... I left it there since I liked it. Open a PR if you don't. What do you mean with the docstring? I don't think I get what's wrong with it :)

[jaseg]

Oh, I just found a hakcy workarounnd. You can observe e.g. time-pos with the format set to MpvFormat.INT64 and you'll only get one update a second. You have to patch observe_property to add that. I'll think of a permanent fix some time later today or tomorrow, as that is not quite as simple since now you can observe a property using different formats and the property name alone is not enough for disambiguation.

[McSinyx]

I'll try to learn about signal handling after dinner, thanks for the recommendation.

About the docstrings, there are 30 functions with the same docstring """ Mapped mpv seek command, see man mpv(1). """. And about styling, do you think no space around the string would look better, i.e. """Mapped mpv [...] see man mpv(1).""". That will look more like a normal quote and match other style in the file with parenthesis and square braces and {} (sorry I don't remember the name for this). Also, I think 79 column limit would be a nice thing (e.g. to have 2 window tile vertically). I can pull a styling request on this if you accept my personal preference.

[jaseg]

I still don't understand what the problem with these docstrings is. They are not automatically generated, I put them there by hand. They IMHO contain the exact right information as I don't want to go ahead copy'n'pasting the detailed descriptions from man mpv(1) since that would be a mess to keep in sync with upstream.

If you're so keen on minor formatting changes, feel free to submit a PR. Otherwise I'll just try to clean up things the next time I touch them. One thing I'm generally against is hard-wrapping stuff to 80 columns just because. If you don't like the wide lines, just turn on soft wrap on your editor. It should be able to do that in a readable way.

[McSinyx]

Well I guess the problem with those docstrings is the word seek, e.g. run, quit, screenshot all have that same description.

Generally I'm not so keen on reformating but I think that'd be a nice thing to learn more about how this module work. I'll try to avoid too hard wrapping. The point of 80-col wrap to me maybe matter most on comments and docstrings, as sometimes one word or two get pushed to the next line.

Also I have created a PR for observe_property. I'm not sure if it break anything though.

[jaseg]

Thank you.

Unfortunately, your solution would break when two handlers are registered to the same property using different types. I just pushed a fix to master. You can now do

@player.property_observer('time-pos', force_fmt=mpv.MpvFormat.UINT64)
def my_print0r(name, value):
    print(name, 'is', value)

Next time please make sure you separate formatting changes from logic changes in both commits and PRs.

[jaseg]

Btw, the docstring is always the same because it's just a pointer to upstream. The functions are just wrappers around upstream and I don't want to copy-paste upstream's more detailed doc here. One possible solution would be to manually put a whole bunch of deeplinks to https://mpv.io/manual/master/#command-interface into all of these docstrings.

[McSinyx]

@jaseg Sorry for using this old issue instead of opening a new one as I think the problem is quite related. Now I can't access osd properties with - (which will be converted to _, e.g. 'time-pos' or mpv.MPV.osd.time_pos) anymore. Also I notice that mpv.MPV.time_pos (this still works somehow) and mpv.MPV.duration have the wrong value (the video is the one in your example and it's played OK):

And thank you for updating the docstrings!

[jaseg]

It seems I broke some stuff and the tests did not catch it. I'll probably push a fix and a new version later today along with fixed unit tests.

[jaseg]

Should be fixed now, including unit tests and a regression test.

[McSinyx]

Thanks a lot for the quick fix, it works for me now!