command: extend subprocess command stdin, change behavior

Make it possible to feed a string to stdin of a subprocess. Out of
laziness, it can't be an arbitrary byte string. (Would require adding an
option type that takes in a Lua byte string.)

Do not set stdin of a subprocess to fd 0 (i.e. mpv's stdin) anymore,
because it makes things more consistent. Enabling stdin didn't make too
much sense in the first place, so this behavior change seems
justifiable.

win32 support missing.

Fixes: mpv-player#8003

DOCS/interface-changes.rst

@@ -74,6 +74,9 @@ Interface changes
     - undeprecate --video-sync=display-adrop
     - deprecate legacy auto profiles (profiles starting with "extension." and
       "protocol."). Use conditional auto profiles instead.
+    - the "subprocess" command does not connect spawned processes' stdin to
+      mpv's stdin anymore. Instead, stdin is connected to /dev/null by default.
+      To get the old behavior, set the "passthrough_stdin" argument to true.
  --- mpv 0.32.0 ---
     - change behavior when using legacy option syntax with options that start
       with two dashes (``--`` instead of a ``-``). Now, using the recommended

DOCS/man/input.rst

@@ -547,6 +547,17 @@ Remember to quote string arguments in input.conf (see `Flat command syntax`_).
         On Lua, you may use ``utils.get_env_list()`` to retrieve the current
         environment if you e.g. simply want to add a new variable.
 
+    ``stdin_data`` (``MPV_FORMAT_STRING``)
+        Feed the given string to the new process' stdin. Since this is a string,
+        you cannot pass arbitrary binary data. If the process terminates or
+        closes the pipe before all data is written, the remaining data is
+        silently discarded. Probably does not work on win32.
+
+    ``passthrough_stdin`` (``MPV_FORMAT_FLAG``)
+        If enabled, wire the new process' stdin to mpv's stdin (default: no).
+        Before mpv 0.33.0, this argument did not exist, but the default was if
+        it was set to ``yes``.
+
     The command returns the following result (as ``MPV_FORMAT_NODE_MAP``):
 
     ``status`` (``MPV_FORMAT_INT64``)

TOOLS/lua/command-test.lua

@@ -76,6 +76,25 @@ mp.observe_property("vo-configured", "bool", function(_, v)
     end)
 
 
+    mp.command_native_async({name = "subprocess", args = {"wc", "-c"},
+                             stdin_data = "hello", capture_stdout = true},
+        function(res, val, err)
+            print("Should be '5': " .. val.stdout)
+        end)
+    -- blocking stdin by default
+    mp.command_native_async({name = "subprocess", args = {"cat"},
+                             capture_stdout = true},
+        function(res, val, err)
+            print("Should be 0: " .. #val.stdout)
+        end)
+    -- stdin + detached
+    mp.command_native_async({name = "subprocess",
+                             args = {"bash", "-c", "(sleep 5s ; cat)"},
+                             stdin_data = "this should appear after 5s.\n",
+                             detach = true},
+        function(res, val, err)
+            print("5s test: " .. val.status)
+        end)
 
     -- This should get killed on script exit.
     mp.command_native_async({name = "subprocess", playback_only = false,

libmpv/client.h

@@ -141,7 +141,8 @@ extern "C" {
  * - In certain cases, mpv may start sub processes (such as with the ytdl
  *   wrapper script).
  * - Using UNIX IPC (off by default) will override the SIGPIPE signal handler,
- *   and set it to SIG_IGN.
+ *   and set it to SIG_IGN. Some invocations of the "subprocess" command will
+ *   also do that.
  * - mpv will reseed the legacy C random number generator by calling srand() at
  *   some random point once.
  * - mpv may start sub processes, so overriding SIGCHLD, or waiting on all PIDs

osdep/subprocess-posix.c

@@ -165,8 +165,21 @@ void mp_subprocess2(struct mp_subprocess_opts *opts,
     }
 
     for (int n = 0; n < opts->num_fds; n++) {
+        assert(!(opts->fds[n].on_read && opts->fds[n].on_write));
+
         if (opts->fds[n].on_read && mp_make_cloexec_pipe(comm_pipe[n]) < 0)
             goto done;
+
+        if (opts->fds[n].on_write || opts->fds[n].write_buf) {
+            assert(opts->fds[n].on_write && opts->fds[n].write_buf);
+            if (mp_make_cloexec_pipe(comm_pipe[n]) < 0)
+                goto done;
+            MPSWAP(int, comm_pipe[n][0], comm_pipe[n][1]);
+
+            struct sigaction sa = {.sa_handler = SIG_IGN, .sa_flags = SA_RESTART};
+            sigfillset(&sa.sa_mask);
+            sigaction(SIGPIPE, &sa, NULL);
+        }
     }
 
     devnull = open("/dev/null", O_RDONLY | O_CLOEXEC);
@@ -225,7 +238,7 @@ void mp_subprocess2(struct mp_subprocess_opts *opts,
             if (comm_pipe[n][0] >= 0) {
                 map_fds[num_fds] = n;
                 fds[num_fds++] = (struct pollfd){
-                    .events = POLLIN,
+                    .events = opts->fds[n].on_read ? POLLIN : POLLOUT,
                     .fd = comm_pipe[n][0],
                 };
             }
@@ -249,15 +262,35 @@ void mp_subprocess2(struct mp_subprocess_opts *opts,
                         kill(pid, SIGKILL);
                     killed_by_us = true;
                     break;
-                } else {
+                }
+                struct mp_subprocess_fd *fd = &opts->fds[n];
+                if (fd->on_read) {
                     char buf[4096];
                     ssize_t r = read(comm_pipe[n][0], buf, sizeof(buf));
                     if (r < 0 && errno == EINTR)
                         continue;
-                    if (r > 0 && opts->fds[n].on_read)
-                        opts->fds[n].on_read(opts->fds[n].on_read_ctx, buf, r);
+                    fd->on_read(fd->on_read_ctx, buf, MPMAX(r, 0));
                     if (r <= 0)
                         SAFE_CLOSE(comm_pipe[n][0]);
+                } else if (fd->on_write) {
+                    if (!fd->write_buf->len) {
+                        fd->on_write(fd->on_write_ctx);
+                        if (!fd->write_buf->len) {
+                            SAFE_CLOSE(comm_pipe[n][0]);
+                            continue;
+                        }
+                    }
+                    ssize_t r = write(comm_pipe[n][0], fd->write_buf->start,
+                                      fd->write_buf->len);
+                    if (r < 0 && errno == EINTR)
+                        continue;
+                    if (r < 0) {
+                        // Let's not signal an error for now - caller can check
+                        // whether all buffer was written.
+                        SAFE_CLOSE(comm_pipe[n][0]);
+                        continue;
+                    }
+                    *fd->write_buf = bstr_cut(*fd->write_buf, r);
                 }
             }
         }

osdep/subprocess.h

@@ -22,9 +22,18 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#include "misc/bstr.h"
+
 struct mp_cancel;
 
+// Incrementally called with data that was read. Buffer valid only during call.
+// size==0 means EOF.
 typedef void (*subprocess_read_cb)(void *ctx, char *data, size_t size);
+// Incrementally called to refill *mp_subprocess_fd.write_buf, whenever write_buf
+// has length 0 and the pipe is writable. While writing, *write_buf is adjusted
+// to contain only the not yet written data.
+// Not filling the buffer means EOF.
+typedef void (*subprocess_write_cb)(void *ctx);
 
 void mp_devnull(void *ctx, char *data, size_t size);
 
@@ -37,6 +46,9 @@ struct mp_subprocess_fd {
     // Note: "neutral" initialization requires setting src_fd=-1.
     subprocess_read_cb on_read;     // if not NULL, serve reads
     void *on_read_ctx;              // for on_read(on_read_ctx, ...)
+    subprocess_write_cb on_write;   // if not NULL, serve writes
+    void *on_write_ctx;             // for on_write(on_write_ctx, ...)
+    bstr *write_buf;                // must be !=NULL if on_write is set
     int src_fd;                     // if >=0, dup this FD to target FD
 };
 

player/command.c

@@ -5343,6 +5343,11 @@ static void subprocess_read(void *p, char *data, size_t size)
     }
 }
 
+static void subprocess_write(void *p)
+{
+    // Unused; we write a full buffer.
+}
+
 static void cmd_subprocess(void *p)
 {
     struct mp_cmd_ctx *cmd = p;
@@ -5351,6 +5356,8 @@ static void cmd_subprocess(void *p)
     bool playback_only = cmd->args[1].v.i;
     bool detach = cmd->args[5].v.i;
     char **env = cmd->args[6].v.str_list;
+    bstr stdin_data = bstr0(cmd->args[7].v.s);
+    bool passthrough_stdin = cmd->args[8].v.i;
 
     if (env && !env[0])
         env = NULL; // do not actually set an empty environment
@@ -5361,6 +5368,12 @@ static void cmd_subprocess(void *p)
         return;
     }
 
+    if (stdin_data.len && passthrough_stdin) {
+        MP_ERR(mpctx, "both stdin_data and passthrough_stdin set\n");
+        cmd->success = false;
+        return;
+    }
+
     void *tmp = talloc_new(NULL);
 
     struct mp_log *fdlog = mp_log_new(tmp, mpctx->log, cmd->cmd->sender);
@@ -5392,7 +5405,7 @@ static void cmd_subprocess(void *p)
         .fds = {
             {
                 .fd = 0, // stdin
-                .src_fd = 0,
+                .src_fd = passthrough_stdin ? 0 : -1,
             },
         },
         .num_fds = 1,
@@ -5408,6 +5421,16 @@ static void cmd_subprocess(void *p)
             .on_read_ctx = &fdctx[fd],
         };
     }
+    // stdin
+    if (stdin_data.len) {
+        opts.fds[0] = (struct mp_subprocess_fd){
+            .fd = 0,
+            .src_fd = -1,
+            .on_write = subprocess_write,
+            .on_write_ctx = &fdctx[0],
+            .write_buf = &stdin_data,
+        };
+    }
 
     struct mp_subprocess_result sres;
     mp_subprocess2(&opts, &sres);
@@ -6078,6 +6101,8 @@ const struct mp_cmd_def mp_cmds[] = {
             {"capture_stderr", OPT_FLAG(v.i), .flags = MP_CMD_OPT_ARG},
             {"detach", OPT_FLAG(v.i), .flags = MP_CMD_OPT_ARG},
             {"env", OPT_STRINGLIST(v.str_list), .flags = MP_CMD_OPT_ARG},
+            {"stdin_data", OPT_STRING(v.s), .flags = MP_CMD_OPT_ARG},
+            {"passthrough_stdin", OPT_FLAG(v.i), .flags = MP_CMD_OPT_ARG},
         },
         .spawn_thread = true,
         .can_abort = true,