Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide libmpv / new slave mode #97

Closed
Nikoli opened this issue May 25, 2013 · 34 comments
Closed

Provide libmpv / new slave mode #97

Nikoli opened this issue May 25, 2013 · 34 comments

Comments

@Nikoli
Copy link
Contributor

Nikoli commented May 25, 2013

It would be nice to convert mpv CLI into libmpv+CLI. All mplayer GUIs use insane parsing of CLI stdout messages, linking to libmpv instead would be much better. There is one attempt to create libmpv and use it in Qt GUI:
https://code.google.com/p/cmplayer/
https://code.google.com/p/cmplayer/source/detail?r=fa6111f2fff70004ae71ac3d8fb592defaff4339

@ghost
Copy link

ghost commented May 25, 2013

Suggestions for exact API welcome, preferably with complete headers. The hardest part is the interface. Of course there's more hard stuff to come, but we can't do much without knowing what the interface should even look like.

@bylee20
Copy link
Contributor

bylee20 commented May 25, 2013

In order to interact with GUI without parsing insane messages, I think some callbacks to notify the state or filter some works are necessary. In CMPlayer, next callbacks are introduced.

  1. filter for run_command like bylee20@a2bf540#diff-0

This callback can be used for monitoring and filtering the commands or insert custom command into decoding thread. Since most modern applications work with multthread, the last purpose can be helpful when some jobs should be in decoding thread.

  1. notifier for playback state

In CMPlayer, I've added a callback to notify the paused state, but It will be great if mpv supports more general callback notifying the playback state like stopped, playing, paused, caching(or buffering), loading(or opening) etc. Maybe it will be more helpful to notify whenever a task(loading file, opening demuxer, opening stream, about to start to play, about to finish to play, clean up ...) is done. Also the frond-end need to be notified whenever any error occurs.

@ghost
Copy link

ghost commented May 25, 2013

filter for run_command

I don't understand this. Commands are generated by keyboard input - so maybe you want a way to get keyboard input instead? But you're replacing the VO completely in cmplayer, so mpv's keyboard handling code isn't even used.

notifier for playback state

OK, there should be a way to check whether properties change, plus some sort of event notification.

@bylee20
Copy link
Contributor

bylee20 commented May 26, 2013

Yes, I handle the keyboard and mouse input by myself, but still I can use the command with the combination of mp_input_parse_cmd() and mp_input_queue_cmd(), e. g., in order to stop the playback.
Even though mpv will provide every function which corresponds each command, I still need some kind of event queue running in decoding thread, especially in the playback loop, and I'm using the command queue as an event queue. For instance, It must be done in mpv's thread to change the mpv's property, and in CMPlayer, an event of setting property is delievered into the mpv's thread in the form of a custom command and the callback filters the custom command. This is a workaround not the best solution maybe.

@lu-zero
Copy link
Contributor

lu-zero commented Jun 26, 2013

https://gist.github.com/lu-zero/5868996

Nikoli asked me to propose something about this. I hope it helps =)

@pigoz
Copy link
Member

pigoz commented Jun 26, 2013

This is roughly similar to something I had in mind. Adding some settable properties would also be beneficial for stuff that changes at runtime (i.e. for volume), as well as adding the possibility to observe changing properties (by installing callbacks). I'd make them a little more typesafe than what they are now though.

What must be decided is what will happen behind all of this. Will it run the player instance inside of a thread or a process? In the first case one should also think about how to handle complete redraws of the current frame + osd issued by the GUI (both OSX and Windows would need this since to make something decent).

@ghost
Copy link

ghost commented Jun 27, 2013

Thanks for the inspiration.

What must be decided is what will happen behind all of this. Will it run the player instance inside of a thread or a process?

Yeah. I don't even know whether it should use a synchronous or asynchronous API. Asynchronous would be closer to what mpv does most of the time internally. (For instance, seeks are not instant; they can take a few playloop iterations.)

There is also someone experimenting with zeromq support for slave mode.

@pigoz
Copy link
Member

pigoz commented Jun 27, 2013

Yeah. I don't even know whether it should use a synchronous or asynchronous API. Asynchronous would be closer to what mpv does most of the time internally. (For instance, seeks are not instant; they can take a few playloop iterations.)

From my experience by using a GUI toolkit (mainly Cocoa), to me it's a no brainer that all should be asynchronous. One should be able to register observers to changes in mpv state. Something along the line of:

typedef int MPVObserverID; // < 0 means error
MPVObserverID mpv_install_observer(MPVPlayer *m, MPVProperty property, void *context, (int)(*observer_cb)(MPVProperty property, void *context, void *value));
int mpv_uninstall_observer(MPVPlayer *m, MPVObserverID id);

Any opinions?

@lu-zero
Copy link
Contributor

lu-zero commented Jun 27, 2013

On Thu, Jun 27, 2013 at 4:34 PM, Stefano Pigozzi
notifications@github.comwrote:

Yeah. I don't even know whether it should use a synchronous or
asynchronous API. Asynchronous would be closer to what mpv does most of the
time internally. (For instance, seeks are not instant; they can take a few
playloop iterations.)

From my experience by using a GUI toolkit (mainly Cocoa), to me it's a no
brainer that all should be asynchronous. One should be able to register
observers to changes in mpv state. Something along the line of:

typedef int MPVObserverID; // < 0 means error
MPVObserverID mpv_install_observer(MPVPlayer _m, MPVProperty property, void *context, (int)(_observer_cb)(MPVProperty property, void *context, void *value));
int mpv_uninstall_observer(MPVPlayer *m, MPVObserverID id);

enum MPVAction;

int mpv_register_callback(MPVPlayer *m, enum MPVAction action, callback);

callback can be something like int cb(MPVPlayer *m, MPVAction action,
MPVState *state)

@ghost
Copy link

ghost commented Jul 1, 2013

We could always add a synchronous API on top of the asynchronous one. But actually I'm not a fan of having 2 sets of separate APIs.

Asynchronous means you can't get the result of an operation immediately. I imagine that would make client code quite awkward. For example, querying a property requires a round trip. It's much like, say, a network protocol. I don't know what magic cocoa provides to make this easier.

Still, asynchronous sounds best to me. It doesn't require messing with the mpv core at all.

Currently, I imagine something like this:


struct mpv;
typedef int reply_id; // used in the callback to associate requests with replies

struct mpv *mpv_create(void);
void mpv_destroy(struct mpv *mpv); // property cleanup requires sending "quit" command first
void mpv_set_event_handler(struct mpv *mpv, void (*event_handler)(void *ctx, struct mpv *mpv, reply_id id, char **props), void *ctx);
reply_id mpv_start(struct mpv *, char **props); // async command to start mpv, I guess?
reply_id mpv_send_command(struct mpv *, char *cmd, char **args); // reuses input commands, see input.rst
reply_id mpv_request_property(struct mpv *, char *property); // reply with property contents
reply_id mpv_observe_property(struct mpv *, char *property); // notify about changes (changes use the reply_id)
reply_id mpv_unobserve_property(struct mpv *, char *property);

All calls that expect a reply return a reply_id, which is a sequence number. This will be passed as argument to the user's event handler callback.

In multiple places, char **props shows up. This is a key value storage. I wonder whether this should use some generic key-value hierarchical storage (think XML, JSON, Lua tables, etc.), but that sounds complicated and I'd rather have something that is more stupid and simple... so char **props is simply a list of strings of key=value entries:

char *props[] = {
    "name1=value1",
    "name2=value2",
    NULL
};

This actually allows values to contain any character (except \0) without escaping or other trouble. This can store lists and hierarchical data too by prefixing names with -. For example:

char *chapters[] = {
    "chapters",
    "-name=chapter 1",
    "-start=0",
    "-end=12.34",
    "", // go back to chapter list level
    "-name=chapter 2",
    "-start=12.34",
    "-end=34.45",
    // Metadata sub-tree nested within the current chapter
    "-metadata",
    "--entry",
    "---name=first metadata entry name",
    "---value=first metadata entry value",
    "--", // go back to metadata list level
    "---name=second metadata entry name",
    "---value=second metadata entry value",
    "", // go back to chapter list level (implicitly 3 levels)
    "-name=chapter 3",
    // ... etc. ...
    0
};

Single values would use an empty name:

char *single_value[] = {
    "=a string value, the first = is not part of the value",
    0
};

This is so that the client can parse this into a dictionary-like data structure without allowing the possibility that parts of the value are interpreted (like a leading =). This also has the good property that you can just check the start of a string to find a named top level value, so the simple case stays simple.

The event handler callback void (*event_handler)(void *ctx, struct mpv *mpv, reply_id id, char **props) is called for all replies. The props parameter describes what the event is about, for example whether a property was changed (and which property). It follows the scheme described above.

Predefined events
-----------------
This goes like:

Event type:
    list of entries that can appear in the "props" parameter

Playback start:
    "type=playback_start",
    "filename=$FILENAME",
        File (or URL) which is about to be loaded.

Playback end:
    "type=playback_end"

Property request:
    "type=property_request",
    "property=$PROPERTY_NAME",
        Property which has been requested with mpv_request_property().
    "value=$CONTENTS",
        The property value, which in most cases will be a simple string, for
        example "value=single value", but in some cases might be much more
        complex (like chapter and track list). In that case, the property
        value is indented by one level below "value=":
            "value=",
            "-chapters",
            "--name=chapter 1",
            "--start=0",

Property change:
    "type=property_change",
    "property=$PROPERTY_NAME",
        Property observed with mpv_observe_property().
    "value=$CONTENTS",
        Property value as in property_request.
        Or maybe empty, and the user has to use mpv_request_property()?

Command reply:
    "type=command_reply",
        Generic reply to mpv_send_command, which returns whether it
        was successful or not.
    "success=yes|no",
    "error_message=$MSG",

It's annoyingly asynchronous, I'd probably faint if I had to use this in C.

Not sure about the property list stuff. Might be a case of trying to be simple that got too complex, or it might be fine.

Opinions?

@ghost
Copy link

ghost commented Jul 1, 2013

divVerent says mpv_unobserve_property should take a reply_id instead, so that it's possible to watch one property multiple times (better for complex programs). mpv_request_property would always return a new ID, even if the property is already watched, and libmpv would send a change event for each registration.

@pigoz
Copy link
Member

pigoz commented Jul 1, 2013

The "trick" is just using a GUI toolkit properly. You setup a data structure in your frontend representing the player's core state and bind the GUI components to it (I'm sure any GUI toolkit supports one-way data binding in 2013). The callbacks would then update this state that is bounded on GUIs. And everything updates like magic.

Your proposal is ok. Two notes:

  • I would have preferred something more typesafe for the properties instead of strings.
  • I think it should use JSON instead of that weird syntax for **props

@pigoz
Copy link
Member

pigoz commented Jul 1, 2013

+1 for divVerent suggestion. In fact I returned an observer ID in my proposal.

@ghost
Copy link

ghost commented Jul 1, 2013

I would have preferred something more typesafe for the properties instead of strings.

This will just lead to misery. For example in the Lua branch, I first wanted to pass properties perfectly to Lua, preserving their data type as far as possible. But this just ended up as fragile and complicated. Strings are an awesome way to represent data if you want loose coupling.

I think it should use JSON instead of that weird syntax for **props

This would require a full JSON parser, whereas you only need strncmp() with the current scheme. JSON also can't deal with invalid UTF-8, which libavformat and thus mpv will return a lot. JSON also requires quoting the strings and inserting escape codes, which is additional code too.

On the other hand, JSON would require only a flat string (instead of a string list), and it's pretty popular and understood by everyone.

Note that we still could provide a function to convert **props into JSON.

@ghost
Copy link

ghost commented Jul 1, 2013

The hardest part is actually managing interaction with other threads. Obviously the user's event_handler has to be called from somewhere.

So there are some possibilities:

  1. It's called asynchronously from a thread created by mpv_create()
  2. There is a mpv_mainloop() function, which does not return as long as mpv is running.
  3. A mpv_process() function, which processes and dispatches mpv pending events. There would be other functions which either wait for new events, or for example return a UNIX FD, which can be used to select() on, or for integration with a GUI library.

IMO it's obvious that 3. is best, but it's also the hardest, as different platforms and GUI libs require special solutions. Suggestions welcome.

@pigoz
Copy link
Member

pigoz commented Jul 1, 2013

1 is easiest from the frontend perspective, from that thread you can then use your GUI toolkit to schedule some operation on the main thread to updated local state.

2 would have been cool to integrate Cocoa inside of mpv's event system (instead of creating a thread), or better integrate mpv's event system inside of Cocoa's event loop.

@ghost
Copy link

ghost commented Jul 1, 2013

All of this would still assume that the mpv playloop runs in its own thread, completely separate from the libmpv client.

@pigoz
Copy link
Member

pigoz commented Jul 1, 2013

Yes and the frontend mainloop in it's own thread (possibly the main thread).

@ghost
Copy link

ghost commented Jul 1, 2013

So, there were concerns about the **props stuff, and that it might be better to keep this opaque instead. You know, something like struct mpv_dict; const char *mpv_dict_get(struct mpv_dist *, const char *key);. Does anyone have a concrete suggestion for an API for this? I tried to make one up, but ended up with a monster that had at least 10 functions. This will probably and up bloated and unwiedly. I suspect generic data structures just suck in C.

Any suggestions?

We could also just stay with *_props that have top-level entries only, and use something like bencode for recursive stuff like chapter data. There aren't really that much things that need structured data (just chapters, tracks, playlist, metadata), so a simple client could get away without requiring a bencode parser. And just for top-level entries, *_prop is really dead-simple, so that nobody can complain, right? I mean even the C main function uses this; it's basically a C idiom.

Bencode format: http://en.wikipedia.org/wiki/Bencode

We could also just expose this stuff as public structs to the client (one struct for chapters, one for the playlist, etc.), but then we enter ABI-compatibility hell.

@ghost
Copy link

ghost commented Jul 2, 2013

If libmpv really becomes real, I'll start with a in-process mpv, because that's simpler. Otherwise you'd need to write code for process management (including redirecting pipes etc.) in a way portable to Windows, which sounds like a complete nightmare.

mpv_start creates a thread, which runs what's inside the current main() function. Thereis a thread-safe queue between mpv and the client (libmpv user). The client is responsible for reading from the queue using an API function.

Example:

struct mpv *player = mpv_create();
mpv_set_event_handler(player, ...);
mpv_start(player, ...);
while (mpv_wait_next_event(player)) { // this call blocks and waits until queue is non-empty
    mpv_process(player); // process any events left in the queue
};
mpv_destroy(player);

It would also provide functions for convenient wakeup for integration into GUI mainloops and stuff like that:

// Returns the read-end of a pipe. Any time a new event is available, a byte will be written
// to this pipe. You can use this with select(), so that you can wake up the next time
// mpv_process() should be called.
int mpv_create_wakup_pipe(struct mpv *);

This would (possibly) allow for GUI toolkit integration, but also makes it possible to "just run" mpv.

Actually, I wonder whether there really should be callbacks. Maybe this is better:

struct mpv_event {
    reply_id id;
    char **props;
};
struct mpv_event *mpv_read_event(struct mpv *);
void mpv_release_event(struct mpv_event *event);

@ghost
Copy link

ghost commented Jul 2, 2013

Oh, for mpv_create_wakup_pipe it should also be clarified that the client must read all data from the pipe, otherwise everything would freeze after 4096 events (when the pipe is full).

Also, instead of pipes, other OS or toolkit specific functions could be provided. For example, on Windows we could send a message to a window on every event.

Note that the mpv playloop would send these notifications. So in the simplest case, there are only two threads. since Windows ruins everything, the playloop will perhaps poll for new libmpv client requests on Windows, while on Linux it'd use a wakeup pipe. (In fact there's already a wakeup pipe in the code.)

@dubhater
Copy link
Contributor

dubhater commented Jul 3, 2013

From irc:

2013-07-03 12:24:07   nodame   So the purpose of libmpv is to allow writing a GUI for it more easily, right?
2013-07-03 12:24:28   pigoz   nodame: yes
2013-07-03 12:24:38   nodame   But how would you integrate mpv's video outputs with a GUI? Or how would you make mpv use your own custom video output?
2013-07-03 12:24:56   nodame   I don't think there was anything about this in the issue, so far.
2013-07-03 12:25:15   nodame   (Maybe because it's very obvious...)
2013-07-03 12:25:54   pigoz   that's still to decide, I think passing a void * pointer to the opengl view / window in your gui would be acceptable
2013-07-03 12:26:01   pigoz   and no it wasn't discussed
2013-07-03 12:26:14   pigoz   also we must decide how to handle resizes.
2013-07-03 12:26:51   pigoz   since the gui has it's own event loop and resizes happen there..
2013-07-03 12:27:24   pigoz   you need for the gui to be able to redraw immediately in those cases..

@ghost
Copy link

ghost commented Jul 8, 2013

This is the header I created some days ago: http://sprunge.us/ETBB

Maybe I'll implement it.

@0xd34df00d
Copy link

The API looks pretty much fine and neat as for me, a C++/Qt programmer :) Much better then all the typical callbacks stuff.

Qt's support for select'ing on file descriptors is easy, it's a matter of creating a QSocketNotifier on a required fd.

@ghost
Copy link

ghost commented Jul 8, 2013

The API looks pretty much fine and neat as for me, a C++/Qt programmer :)

Nice to hear!

Much better then all the typical callbacks stuff.

It's almost the same as callbacks, though. You get the result of any operation as event (i.e. message), and you'll probably dispatch events as callbacks anyway.

Qt's support for select'ing on file descriptors is easy, it's a matter of creating a QSocketNotifier on a required fd.

Won't work on Windows, though.

@0xd34df00d
Copy link

It's better than callbacks as you get all the control and the context as you need it without a little ugly void *userdata. And some APIs forgot the latter, like parts of Pidgin's libpurple.
And seems like you get a little more control of receiving the next message.

Regarding file descriptors and Windows — why won't it? See https://qt-project.org/doc/qt-4.8/qsocketnotifier.html#notes-for-windows-users

@ghost
Copy link

ghost commented Jul 8, 2013

Regarding file descriptors and Windows — why won't it? See https://qt-project.org/doc/qt-4.8/qsocketnotifier.html#notes-for-windows-users

It probably uses select() internally, and then it can handle only sockets on Windows. So unless you create a local network connection instead of real pipes for the wakeup pipe, it won't work.

@ghost
Copy link

ghost commented Aug 11, 2013

OK, I think this should be added once Lua support is working well. Turns out the Lua support might use very similar interfaces and mechanisms.

@ghost
Copy link

ghost commented Feb 2, 2014

There is now some sort of client API (preliminary, in a branch): https://github.com/mpv-player/mpv/blob/client_api3/player/client_api.h

All what's missing is a way to start the player using the API. If that is done, it could be trivially turned into a library.

@giselher
Copy link
Member

giselher commented Feb 3, 2014

Great work 👍

@bylee20
Copy link
Contributor

bylee20 commented Feb 9, 2014

@wm4
Nice work. I have some questions.

May I think all public things are in client_api.h and other headers won't be exposed?
As long as I can see, the header contains only playloop-related things such as commands and properties, and this API is not aimed to give a extensibility of functionality, right?(I can see 'this API is basically equivalent to slave mode.')

None of components such as demuxer, decoder, filter, outputs can be added from application side. Also, hooking or filtering of events are not possible.
I want to know this level of exposure is final decision or just not done yet because noone did. That is, are such patches which expose them acceptable? Or is it against this API's policy?

@ghost
Copy link

ghost commented Feb 9, 2014

In general, it only allows control over playback, that's right. I don't plan to expose internal mpv APIs, because these are not exactly made with concerns like extensibility, API stability and ABI stability in mind. They're clearly made to be internal, rather than some sort of plugin API. Changing that would probably lead to chaos, and would also choke further development (unless we break the API all the time, which would defeat the point of the client API).

But additional hooks for certain things might also be added. These will be on top of existing functionality, like the vf_dlopen filter provides a stable API/ABI to video filters. These can be added as they're needed.

Also, hooking or filtering of events are not possible.

What would that do?

That is, are such patches which expose them acceptable

Probably acceptable. But if you want to do this, wait until the client API is merged into master. Also, API extensions will have to be discussed, because they're a delicate issue. But in principle it's open for extensions.

@ghost
Copy link

ghost commented Feb 10, 2014

Rebased it, changed it a bit around: https://github.com/mpv-player/mpv/blob/client_api4/player/client_api.h

Still not "done", and lacking some fundamental things I'd like to have (ability to retrieve chapter and track list in a nice way, updating and documenting Lua scripting), but I'd say it's ready for merge.

@ghost
Copy link

ghost commented Feb 10, 2014

Merged. Might need some more work to make it less painful. (For example, retrieving properties with their native data types, instead of forcing string conversions.)

@ghost ghost closed this as completed Feb 10, 2014
@Nyx0uf Nyx0uf mentioned this issue Feb 27, 2014
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants