client API: report success status when running commands

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.

4 files changed, 52 insertions, 35 deletions

diff --git a/libmpv/client.h b/libmpv/client.h
index 673586e169..7e2dc1b98a 100644
--- a/libmpv/client.h
+++ b/libmpv/client.h
@@ -207,7 +207,7 @@ typedef enum mpv_error {
     /**
      * Trying to set or get a property using an unsupported MPV_FORMAT.
      */
-    MPV_ERROR_PROPERTY_FORMAT    = -9,
+    MPV_ERROR_PROPERTY_FORMAT   = -9,
     /**
      * The property exists, but is not available. This usually happens when the
      * associated subsystem is not active, e.g. querying audio parameters while
@@ -217,7 +217,11 @@ typedef enum mpv_error {
     /**
      * Error setting or getting a property.
      */
-    MPV_ERROR_PROPERTY_ERROR    = -11
+    MPV_ERROR_PROPERTY_ERROR    = -11,
+    /**
+     * General error when running a command with mpv_command and similar.
+     */
+    MPV_ERROR_COMMAND           = -12
 } mpv_error;
 
 /**
@@ -620,10 +624,6 @@ int mpv_set_option_string(mpv_handle *ctx, const char *name, const char *data);
  *
  * The commands and their parameters are documented in input.rst.
  *
- * Caveat: currently, commands do not report whether they run successfully. If
- *         the command exists and its arguments are not broken, always success
- *         will be returned.
- *
  * @param[in] args NULL-terminated list of strings. Usually, the first item
  *                 is the command, and the following items are arguments.
  * @return error code
diff --git a/player/client.c b/player/client.c
index 0a71e5020d..ba3705a9af 100644
--- a/player/client.c
+++ b/player/client.c
@@ -717,8 +717,8 @@ struct cmd_request {
 static void cmd_fn(void *data)
 {
     struct cmd_request *req = data;
-    run_command(req->mpctx, req->cmd);
-    req->status = 0;
+    int r = run_command(req->mpctx, req->cmd);
+    req->status = r >= 0 ? 0 : MPV_ERROR_COMMAND;
     talloc_free(req->cmd);
     if (req->reply_ctx) {
         status_reply(req->reply_ctx, MPV_EVENT_COMMAND_REPLY,
@@ -1305,6 +1305,7 @@ static const char *err_table[] = {
     [-MPV_ERROR_PROPERTY_FORMAT] = "unsupported format for accessing property",
     [-MPV_ERROR_PROPERTY_UNAVAILABLE] = "property unavailable",
     [-MPV_ERROR_PROPERTY_ERROR] = "error accessing property",
+    [-MPV_ERROR_COMMAND] = "error running command",
 };
 
 const char *mpv_error_string(int error)
diff --git a/player/command.c b/player/command.c
index a7e80ab4b0..35738e1efb 100644
--- a/player/command.c
+++ b/player/command.c
@@ -3045,7 +3045,7 @@ static bool check_property_autorepeat(char *property,  struct MPContext *mpctx)
     return true;
 }
 
-void run_command(MPContext *mpctx, mp_cmd_t *cmd)
+int run_command(MPContext *mpctx, mp_cmd_t *cmd)
 {
     struct command_ctx *cmdctx = mpctx->command_ctx;
     struct MPOpts *opts = mpctx->opts;
@@ -3062,7 +3062,7 @@ void run_command(MPContext *mpctx, mp_cmd_t *cmd)
             if (cmd->args[n].type->type == CONF_TYPE_STRING) {
                 char *s = mp_property_expand_string(mpctx, cmd->args[n].v.s);
                 if (!s)
-                    return;
+                    return -1;
                 talloc_free(cmd->args[n].v.s);
                 cmd->args[n].v.s = s;
             }
@@ -3103,6 +3103,8 @@ void run_command(MPContext *mpctx, mp_cmd_t *cmd)
                 mpctx->add_osd_seek_info |= OSD_SEEK_INFO_BAR;
             if (msg_or_nobar_osd)
                 mpctx->add_osd_seek_info |= OSD_SEEK_INFO_TEXT;
+        } else {
+            return -1;
         }
         break;
     }
@@ -3115,10 +3117,12 @@ void run_command(MPContext *mpctx, mp_cmd_t *cmd)
         } else if (r == M_PROPERTY_UNKNOWN) {
             set_osd_msg(mpctx, osdl, osd_duration,
                         "Unknown property: '%s'", cmd->args[0].v.s);
+            return -1;
         } else if (r <= 0) {
             set_osd_msg(mpctx, osdl, osd_duration,
                         "Failed to set property '%s' to '%s'",
                         cmd->args[0].v.s, cmd->args[1].v.s);
+            return -1;
         }
         break;
     }
@@ -3144,10 +3148,12 @@ void run_command(MPContext *mpctx, mp_cmd_t *cmd)
         } else if (r == M_PROPERTY_UNKNOWN) {
             set_osd_msg(mpctx, osdl, osd_duration,
                         "Unknown property: '%s'", property);
+            return -1;
         } else if (r <= 0) {
             set_osd_msg(mpctx, osdl, osd_duration,
                         "Failed to increment property '%s' by %g",
                         property, s.inc);
+            return -1;
         }
         break;
     }
@@ -3162,9 +3168,11 @@ void run_command(MPContext *mpctx, mp_cmd_t *cmd)
         } else if (r == M_PROPERTY_UNKNOWN) {
             set_osd_msg(mpctx, osdl, osd_duration,
                         "Unknown property: '%s'", property);
+            return -1;
         } else if (r <= 0) {
             set_osd_msg(mpctx, osdl, osd_duration,
                         "Failed to multiply property '%s' by %g", property, f);
+            return -1;
         }
         break;
     }
@@ -3195,10 +3203,12 @@ void run_command(MPContext *mpctx, mp_cmd_t *cmd)
             } else if (r == M_PROPERTY_UNKNOWN) {
                 set_osd_msg(mpctx, osdl, osd_duration,
                             "Unknown property: '%s'", property);
+                return -1;
             } else if (r <= 0) {
                 set_osd_msg(mpctx, osdl, osd_duration,
                             "Failed to set property '%s' to '%s'",
                             property, value);
+                return -1;
             }
         }
         break;
@@ -3212,7 +3222,7 @@ void run_command(MPContext *mpctx, mp_cmd_t *cmd)
             MP_WARN(mpctx, "Failed to get value of property '%s'.\n",
                     cmd->args[0].v.s);
             MP_INFO(mpctx, "ANS_ERROR=%s\n", property_error_string(r));
-            break;
+            return -1;
         }
         MP_INFO(mpctx, "ANS_%s=%s\n", cmd->args[0].v.s, tmp);
         talloc_free(tmp);
@@ -3244,7 +3254,7 @@ void run_command(MPContext *mpctx, mp_cmd_t *cmd)
 
         struct playlist_entry *e = mp_next_file(mpctx, dir, force);
         if (!e && !force)
-            break;
+            return -1;
         mpctx->playlist->current = e;
         mpctx->playlist->current_was_replaced = false;
         mpctx->stop_play = PT_CURRENT_ENTRY;
@@ -3352,6 +3362,7 @@ void run_command(MPContext *mpctx, mp_cmd_t *cmd)
             }
         } else {
             MP_ERR(mpctx, "Unable to load playlist %s.\n", filename);
+            return -1;
         }
         break;
     }
@@ -3377,12 +3388,12 @@ void run_command(MPContext *mpctx, mp_cmd_t *cmd)
                                                              cmd->args[0].v.i);
         if (cmd->args[0].v.i < 0)
             e = mpctx->playlist->current;
-        if (e) {
-            // Can't play a removed entry
-            if (mpctx->playlist->current == e)
-                mpctx->stop_play = PT_CURRENT_ENTRY;
-            playlist_remove(mpctx->playlist, e);
-        }
+        if (!e)
+            return -1;
+        // Can't play a removed entry
+        if (mpctx->playlist->current == e)
+            mpctx->stop_play = PT_CURRENT_ENTRY;
+        playlist_remove(mpctx->playlist, e);
         break;
     }
 
@@ -3391,9 +3402,9 @@ void run_command(MPContext *mpctx, mp_cmd_t *cmd)
                                                               cmd->args[0].v.i);
         struct playlist_entry *e2 = playlist_entry_from_index(mpctx->playlist,
                                                               cmd->args[1].v.i);
-        if (e1) {
-            playlist_move(mpctx->playlist, e1, e2);
-        }
+        if (!e1)
+            return -1;
+        playlist_move(mpctx->playlist, e1, e2);
         break;
     }
 
@@ -3547,17 +3558,18 @@ void run_command(MPContext *mpctx, mp_cmd_t *cmd)
 
     case MP_CMD_SUB_ADD: {
         struct track *sub = mp_add_subtitles(mpctx, cmd->args[0].v.s);
-        if (sub) {
-            mp_switch_track(mpctx, sub->type, sub);
-            mp_mark_user_track_selection(mpctx, 0, sub->type);
-        }
+        if (!sub)
+            return -1;
+        mp_switch_track(mpctx, sub->type, sub);
+        mp_mark_user_track_selection(mpctx, 0, sub->type);
         break;
     }
 
     case MP_CMD_SUB_REMOVE: {
         struct track *sub = mp_track_by_tid(mpctx, STREAM_SUB, cmd->args[0].v.i);
-        if (sub)
-            mp_remove_track(mpctx, sub);
+        if (!sub)
+            return -1;
+        mp_remove_track(mpctx, sub);
         break;
     }
 
@@ -3568,9 +3580,10 @@ void run_command(MPContext *mpctx, mp_cmd_t *cmd)
             if (nsub) {
                 mp_remove_track(mpctx, sub);
                 mp_switch_track(mpctx, nsub->type, nsub);
+                return 0;
             }
         }
-        break;
+        return -1;
     }
 
     case MP_CMD_SCREENSHOT:
@@ -3628,19 +3641,18 @@ void run_command(MPContext *mpctx, mp_cmd_t *cmd)
                 set_osd_msg(mpctx, osdl, osd_duration, "vo='%s'", s);
             } else {
                 set_osd_msg(mpctx, osdl, osd_duration, "Failed!");
+                return -1;
             }
         }
         break;
 
     case MP_CMD_AF:
-        edit_filters_osd(mpctx, STREAM_AUDIO, cmd->args[0].v.s,
-                         cmd->args[1].v.s, msg_osd);
-        break;
+        return edit_filters_osd(mpctx, STREAM_AUDIO, cmd->args[0].v.s,
+                                cmd->args[1].v.s, msg_osd);
 
     case MP_CMD_VF:
-        edit_filters_osd(mpctx, STREAM_VIDEO, cmd->args[0].v.s,
-                         cmd->args[1].v.s, msg_osd);
-        break;
+        return edit_filters_osd(mpctx, STREAM_VIDEO, cmd->args[0].v.s,
+                                cmd->args[1].v.s, msg_osd);
 
     case MP_CMD_SCRIPT_DISPATCH: {
         mpv_event_script_input_dispatch *event = talloc_ptrtype(NULL, event);
@@ -3653,6 +3665,7 @@ void run_command(MPContext *mpctx, mp_cmd_t *cmd)
         {
             MP_VERBOSE(mpctx, "Can't find script '%s' when handling input.\n",
                        cmd->args[0].v.s);
+            return -1;
         }
         break;
     }
@@ -3669,6 +3682,7 @@ void run_command(MPContext *mpctx, mp_cmd_t *cmd)
         {
             MP_VERBOSE(mpctx, "Can't find script '%s' for %s.\n",
                        cmd->args[0].v.s, cmd->name);
+            return -1;
         }
         break;
     }
@@ -3710,7 +3724,9 @@ void run_command(MPContext *mpctx, mp_cmd_t *cmd)
 
     default:
         MP_VERBOSE(mpctx, "Received unknown cmd %s\n", cmd->name);
+        return -1;
     }
+    return 0;
 }
 
 void command_uninit(struct MPContext *mpctx)
diff --git a/player/command.h b/player/command.h
index 258eada148..7ceeda22ff 100644
--- a/player/command.h
+++ b/player/command.h
@@ -26,7 +26,7 @@ struct mp_log;
 void command_init(struct MPContext *mpctx);
 void command_uninit(struct MPContext *mpctx);
 
-void run_command(struct MPContext *mpctx, struct mp_cmd *cmd);
+int run_command(struct MPContext *mpctx, struct mp_cmd *cmd);
 char *mp_property_expand_string(struct MPContext *mpctx, const char *str);
 char *mp_property_expand_escaped_string(struct MPContext *mpctx, const char *str);
 void property_print_help(struct mp_log *log);