Following the discussion in #1253.
The events won't be removed for a while, though. (Or maybe never, unless
we run out of bits for the uint64_t event mask.)
This is not a real change (the events still work, and the alternative
mechanisms were established a few API revisions earlier), but for the
sake of notifying API users, update DOCS/client-api-changes.rst.
Add a comment about this to avoid confusing users of this function. The
parameter is essentially unused, but exists so that we don't need to
add extra APIs if the need for it arises.
Using magic integer values was an attempt to keep the API less verbose.
But it was probably not a good idea.
Reason 1 (restart) is not made explicit, because it is not used anymore
starting with the previous commit. For ABI compatibility, the value is
left as a hole in the enum.
The only reason for mpv_wait_event() not being thread-safe is that it
returns a pointer to a mpv_event struct member in the mpv_handle
context, which in turn is done for ABI-safety (user doesn't allocate or
manage the struct), and to avoid the need additional memory management.
Some users were interpreting this incorrectly.
This provides some helper functions and classes for C++/Qt. As the top
of qthelper.hpp says, this is built on top of the client API, and is a
mere helper provided for convenience.
Maybe this should be a separate library, but on the other hand I don't
see much of a point in that. It's also header-only, but C++ people like
such things. This makes it easier for us, because we don't need to care
about ABI compatibility.
The client API doesn't change, but bump it so that those who are using
this header can declare a proper dependency.
Allows passing native types as arguments.
Also some minor doc improvements, including giving some (natural)
improvements to mpv_free_node_contents().
Note: mpv_command_node_async() is completely untested.
Maybe using strings for log levels was a mistake (too broad and too
impractical), so I'm adding numeric log level at least for the receiver
side. This makes it easier to map mpv log levels to other logging
systems.
I'm still too stingy to add a function to set the log level by a numeric
value, though.
The numeric values are not directly mapped to the internal mpv values,
because then almost every file in mpv would have to include the client
API header.
Coalesce this into API version 1.6, since 1.6 was bumped just yesterday.
The API could return partial lines, meaning the message could stop
in the middle of a line, and the next message would have the rest of
it (or just the next part of it). This was a pain for the user, so do
the nasty task of buffering the lines ourselves.
Now only complete lines are sent. To make things even easier for the
API user, don't put multiple lines into a single event, but split them.
The terminal output code needed something similar (inserting a prefix
header on start of each line). To avoid code duplication, this commit
refactors the terminal output so that lines are split in a single
place.
So client API users don't have to worry about this specifically.
Also document the overflow case. (Not sure if we really need to do
this; maybe it'd be better not to, since this just adds more noise
to the docs.)
Document the "normal" behavior (if MPV_FORMAT_NONE is not used) first,
and then introduce MPV_FORMAT_NONE as exception.
The actual semantics didn't change in mpv; this is only clarification.
Whether you consider the semantics weird or not depends on your use
case, but I suppose it's a bit confusing anyway. At this point, we keep
MPV_EVENT_PAUSE/UNPAUSE for compatibility only.
Make the "core-idle" property somewhat more useful in this context.
Commit 64b7811c tried to do the "right thing" with respect to whether
keyboard input should be enabled or not. It turns out that X11 does
something stupid by design. All modern toolkits work around this native
X11 behavior, but embedding breaks these workarounds.
The only way to handle this correctly is the XEmbed protocol. It needs
to be supported by the toolkit, and probably also some mpv support. But
Qt has inconsistent support for it. In Qt 4, a X11 specific embedding
widget was needed. Qt 5.0 doesn't support it at all. Qt 5.1 apparently
supports it via QWindow, but if it really does, I couldn't get it to
work.
So add a hack instead. The new --input-x11-keyboard option controls
whether mpv should enable keyboard input on the X11 window or not. In
the command line player, it's enabled by default, but in libmpv it's
disabled.
This hack has the same problem as all previous embedding had: move the
mouse outside of the window, and you don't get keyboard input anymore.
Likewise, mpv will steal all keyboard input from the parent application
as long as the mouse is inside of the mpv window.
Also see issue #1090.
When embedding a X window, it's hard to control whether it receives
mouse/keyboard input or not. It seems the X protocol itself makes this
hard (basically due to the outdated design mismatching with modern
toolkits), and we have to take care of these things explicitly.
Simply do this by manually querying and using the parent window event
flags.
This restores some MPlayer behavior (it doesn't add back exactly the
same code, but it's very similar).
This probably has some potential to interfere with libmpv embedding, so
bump the client API minor.
CC: @mpv-player/stable (if applied, client-api-changes.rst has to be
adjusted to include the 0.5.2 release)
It's not true anymore that the core will stop replying for 50ms
(waiting for video) without calling this function. Simplify the
documentation accordingly. Accessing properties that go through
the VO still have this problem, though.
This is probably nicer. The actual version number doesn't change (other
than the minor being incremented).
The "| 0UL" is to make the type unsigned long int, like it was before.
Handle --term-playing-msg at a better place.
Move MPV_EVENT_TICK hack into a separate function. Also add some words
to the client API that you shouldn't use it. (But better leave breaking
it for later.)
Handle --frames and frame_step differently. Remove the mess from the
playloop, and do it after frame display. Give up on the weird semantics
for audio-only mode (they didn't make sense anyway), and adjust the
manpage accordingly.
Achieve this by polling. Will be used by the OSC. Basically a bad hack -
but the point is that the mpv core itself is in the best position to
improve this later.
It is reasonably stable, so all further changes will be versioned.
Also change how the libmpv version number is generated. Fix the patch
version number to 0; I don't think we have a use for this. In
particular, the version doesn't version mpv, just the client API.
Until now, availability of new log messages (through the mechanism
associated with mpv_request_log_messages()) did not wakeup the client
API properly. Commit 3b7402b5 was basically a hack to improve that
somewhat, but it wasn't a solution.
The main problem is that the client API itself is producing messages, so
the message callback would attempt to lock the client API lock,
resulting in a deadlock. Even if the lock was recursive, we'd run into
lock-order issues.
Solve this by using a separate lock for waiting and wakeup. Also, since
it's a natural addition, avoid redundant wakeups. This means the wakeup
callback as well as the wakeup pipe will be triggered only once until
the next mpv_wait_event() call happens.
This might make the wakeup callback be invoked in a reentrant way for
the first time, for example if a mpv_* function prints to a log. Adjust
the docs accordingly. (Note that non-reentrant beheavior was never
guaranteed - basically the wakeup callback is somewhat dangerous and
inconvenient.)
Also remove some traces of unneeded code. ctx->shutdown for one was
never set, and probably a leftover of an abandoned idea.
A bit verbose, but less misleading. In most cases, the API user probably
actually wants mpv_terminate_destroy() instead, so the less-useful
function shouldn't have a simnpler name anyway.
It wasn't necessarily clear how this works. Especially make clear that
the API user shouldn't expect that there's one byte per readable event
in the wakeup pipe. Actually, the reason why this currently won't work
is because property notifications can generate more events than wakeups.
The limit of 4096 is a more fundamental issue, but the event ringbuffer
is currently limited to 1000 entries anyway.
Also add some important comments to mpv_set_wakeup_callback.
mpv_destroy() should perhaps better be called mpv_detach(), because it
destroys only the handle, not necessarily the player. The player is only
terminated if a quit command is sent.
This function quits automatically, and additionally waits until the
player is completely destroyed. It removes the possibility that the
player core is still uninitializing, while all client handles are
already destroyed. (Although in practice, the difference is usually not
important.)
Now a negative timeout mean an infinite timeout. This is similar to the
poll() system call. Apparently this is more intuitive and less confusing
than specifying a "very high" value as timeout if you want to wait
forever.
For callers that never pass negative timeouts, nothing changes.
Until now, an error was reported only if the command couldn't be parsed.
Attempt to do more fine-grained reporting. This is not necessarily
perfect, but it's an improvement.
This is probably a good idea, because it would make it easier for
software embedding mpv to configure the mpv parts, without requiring the
host program to provide explicit mechanisms for this (other than calling
mpv_load_config_file()).
And slightly adjust the semantics of MPV_EVENT_PAUSE/MPV_EVENT_UNPAUSE.
The real pause state can now be queried with the "core-idle" property,
the user pause state with the "pause" property, whether the player is
paused due to cache with "paused-for-cache", and the keep open event can
be guessed with the "eof-reached" property.
Otherwise, the client API user could not know why playback was stopped.
Regarding the fact that 0 is used both for normal EOF and EOF on error:
this is because mplayer traditionally did not distinguish these, and in
general it's hard to tell the real reason. (There are various weird
corner cases which make it hard.)
This turned out ridiculously complex. I think it will have to be
simplified some day. Main reason for the complexity are:
- filtering properties by forcing clients to observe individual
properties explicitly
(to avoid spamming clients with changes they don't want)
- optional retrieval of property value with the notification
(the basic idea was that this is more user friendly)
- allowing to the client to specify a format in which the value
should be retrieved
(because if a property changes its type, the client API couldn't
convert it properly, and compatibility would break)
I don't know yet which of these are important, and everything could
change. In particular, the interface and semantics should be adjusted
to reduce the implementation complexity.
While I consider the API complete, there could (and probably will) be
bugs left. Also while the implementation is complete, it's inefficient.
The complexity of the property matching is O(a*b*c) with a clients,
b observed properties, and c properties changing at once. I threw away
an earlier implementation using bitmasks, because it was too unwieldy.
wm4
James Ross-Gowan
Alessandro Ghedini
xylosper