Before this, we pretty much guaranteed that --mpv-ipc-fd=3 would be
passed. The FD was hardcoded, so scripts started by this mechanism
didn't need to actually parse the argument. Change this to using a
mostly random FD number instead.
I decided to do this because posix_spawnp() and the current replacement
cannot "guarantee" a FD layout. posix_spawn_file_actions_adddup2() just
runs dup2() calls, so it may be hard to set FD 3/4 if they are already
used by something else. For example imagine trying to map:
{.fd = 3, .src_fd = 4},
{.fd = 4, .src_fd = 3},
Then it'd do dup2(4, 3), dup2(3, 4) (reminder: dup2(src, dst)), and the
end result is that FD 4 really maps to the original FD 4.
While this was not a problem in the present code, it's too messy that I
don't want to pretend it can always done this way without an unholy
mess. So my assumption is that FDs 0-2 can be freely assigned because
they're never closed (probably...), while for all other FDs only
pass-through is reasonable.
This has been part of the libmpv for a while, so the implementation in
the IPC code is quite simple: just pass the mpv_node representing the
value of the "command" field without further checks to
mpv_command_node().
The only problem are the IPC-specific commands, which essentially have
their own dispatch mechanism. They expect an array. I'm not going to
rewrite the dispatch mechanism, so these still work only with an array.
I decided make the other case explicit with cmd==NULL. (I could also
have set cmd=="", which would have avoided changing each if condition
since "" matches no existing command, but that felt dirty.)
I decided to make this explicit. The alternative would have been making
all commands asynchronous always, like a small note in the manpage
threatened. I think that could have caused compatibility issues.
As a design decision, this does not send a reply if an async command
started. This could be a good or bad idea, but in any case, it will make
async command look almost like synchronous ones, except they don't block
the IPC protocol.
This is just a more convenient way to start IPC client scripts per mpv
instance.
Does not work on Windows, although it could if the subprocess and IPC
parts are implemented (and I guess .exe/.bat suffixes are required).
Also untested whether it builds on Windows. A lot of other things are
untested too, so don't complain.
I wanted to put all commands through mpv_command_node_async() instead of
mpv_command_node(). Using synchronous commands over a synchronous
transport doesn't make sense anyway.
This would have used the request_id field in IPC requests as reply ID
for the async commands. But the latter need to be [u]int64, while the
former can be any type. To avoid that we need an extra lookup table for
mapping reply IDs to request_id values, we now require that request_id
fields are integers.
Since this would be an incompatible change, just deprecate non-integers
for now, and plan the change for a later time.
The only effective difference is that the former explicitly checks
whether the JSON value type is string, and errors out if not. The rest
is exactly the same (mpv_set_property_string is mpv_set_property with
MPV_FORMAT_STRING).
It seems silly to keep this, so just remove it.
As threatened by the API changes document.
This commit also removes or stubs equivalent calls in IPC and Lua
scripting.
The stubs are left to maintain ABI compatibility. The semantics of the
API functions have been close enough to doing nothing that this probably
won't even break existing API users. Probably.
They're useless, and I have no idea what they're actually supposed to do
(wrt. pending input processing changes).
Also remove their implicit uses from the IPC handlers.
Old-style commands using _ as separator (e.g. show_progress) were still
used in some places, including documentation and configuration files.
This commit updates all such instances to the new style (show-progress)
so that commands are easier to find in the manual.
If the request contains a "request_id", copy it back into the
response. There is no interpretation of the request_id value by mpv; the
only purpose is to make it easier on the requester by providing an
ability to match up responses with requests.
Because the IPC mechanism sends events continously, it's possible for
the response to a request to arrive several events after the request was
made. This can make it very difficult on the requester to determine
which response goes to which request.
Some rationale for the documented/suggested behavior:
It's not really clear what to do with invalid UTF-8, since JSON simply
can't transport this information. Maybe you could transfer such strings
as byte arrays, but that would be very verbose and inconvenient, and
would pose the problem that it's hard to distinguish between strings
encoded in this way and actual arrays.
There are many other ways how this could be handled. For example, you
could replace invalid sequences with '?'. Or you could do it like
Python, and use certain reserved unicode codepoints to "tunnel" through
invalid bytes.
Which of these works really depends on the application. And since this
can be done entirely on the byte level (invalid UTF-8 sequences can
appear only in strings in our case), it's best to leave this to the
receiver.
This is not realy obvious, so I assume this is a helpful hint.
Although the usefulness of such an approach is probably influenced by
the fact that the player might send events that arrive in the short
window while the socket is connected.
It's kind of obvious, since the protocol by design has to allow you to
read (loadfile) and write (screenshot_to) random files, but better
make it explicit so that nobody accidentally does something insecure.
Oxan van Leeuwen
a1346054
wm4
Timotej Lazar
James Ross-Gowan
Martin Herkt
Preston Hunt
Alessandro Ghedini