command: let frame-step go through multiple frames

This commit gives the frame-step command the ability to accept an
additional argument specifying the amount of frames (forwards or
backwards) to step through. By default, it steps forward 1. The
frame-back-step command is reworked to simply be a call to frame-step
with a -1 value.

An additional flag is added that controls how exactly mpv steps through
frames. By default, "play" is used which behaves like before. mpv will
play through the requested amount of frames before stopping. This is
more robust and reliable but could also take much longer if a high
amount of frames is requested to be skipped through. This mode only
works if mpv is skipping frames forward. Otherwise, the "seek" flag is
used which performs a very exact seek. This working well, depends on pts
values being reliable which may not always be the case. Frame skipping
like this is more of a guess and should be less accurate.

As a minor note, MPSEEK_BACKSTEP is renamed to MPSEEK_FRAMESTEP since
forward seeks can use this as well. Fixes #10128.

DOCS/interface-changes.rst

@@ -41,6 +41,8 @@ Interface changes
     - add `dolbyvision` sub-parameter to `format` video filter
     - `--sub-visibility` no longer has any effect on secondary subtitles
     - add `film-grain` sub-parameter to `format` video filter
+    - add optional `frames` and `flags` arguments to `frame-step` command controlling
+      the direction and amount of frames mpv steps through
  --- mpv 0.34.0 ---
     - deprecate selecting by card number with `--drm-connector`, add
       `--drm-device` which can be used instead

DOCS/man/input.rst

@@ -316,21 +316,40 @@ Remember to quote string arguments in input.conf (see `Flat command syntax`_).
 
     Using it without any arguments gives you the default behavior.
 
-``frame-step``
-    Play one frame, then pause. Does nothing with audio-only playback.
-
-``frame-back-step``
-    Go back by one frame, then pause. Note that this can be very slow (it tries
-    to be precise, not fast), and sometimes fails to behave as expected. How
-    well this works depends on whether precise seeking works correctly (e.g.
-    see the ``--hr-seek-demuxer-offset`` option). Video filters or other video
+``frame-step [<frames>] [<flags>]``
+    Go forward or backwards by a given amount of frames. If ``<frames>`` is
+    omitted, the value is assumed to be ``1``.
+
+    The second argument consists of flags controlling the frameskip mode:
+
+    play (default)
+        Play the video forward by the desired amount of frames and then pause.
+        This only works with a positive value (i.e. frame stepping forwards).
+    seek
+        Perform a very exact seek that attempts to seek by the desired amount
+        of frames.
+
+    Note that the default frameskip mode, play, is more accurate but can be
+    slow depending on how many frames you are skipping (i.e. skipping forward
+    100 frames will play 100 frames of video before stopping). This mode only
+    works when going forwards. Frame stepping back always performs a seek.
+
+    When using seek mode, this can still be very slow (it tries to be precise,
+    not fast), and sometimes fails to behave as expected. How well this works
+    depends on whether precise seeking works correctly (e.g. see the
+    ``--hr-seek-demuxer-offset`` option). Video filters or other video
     post-processing that modifies timing of frames (e.g. deinterlacing) should
-    usually work, but might make backstepping silently behave incorrectly in
+    usually work, but might make framestepping silently behave incorrectly in
     corner cases. Using ``--hr-seek-framedrop=no`` should help, although it
     might make precise seeking slower.
 
     This does not work with audio-only playback.
 
+``frame-back-step``
+    Calls ``frame-step`` with a value of ``-1``.
+
+    This does not work with audio-only playback.
+
 ``set <name> <value>``
     Set the given property or option to the given value.
 

player/command.c

@@ -4970,39 +4970,39 @@ static void cmd_frame_step(void *p)
 {
     struct mp_cmd_ctx *cmd = p;
     struct MPContext *mpctx = cmd->mpctx;
+    int frames = cmd->args[0].v.i;
+    int flags = cmd->args[1].v.i;
 
-    if (!mpctx->playback_initialized) {
+    if (!mpctx->playback_initialized || frames == 0) {
         cmd->success = false;
         return;
     }
 
-    if (cmd->cmd->is_up_down) {
+    if (frames > 0 && !flags && cmd->cmd->is_up_down) {
         if (cmd->cmd->is_up) {
             if (mpctx->step_frames < 1)
                 set_pause_state(mpctx, true);
         } else {
             if (cmd->cmd->repeated) {
                 set_pause_state(mpctx, false);
             } else {
-                add_step_frame(mpctx, 1);
+                add_step_frame(mpctx, frames, flags);
             }
         }
     } else {
-        add_step_frame(mpctx, 1);
+        add_step_frame(mpctx, frames, flags);
     }
 }
 
 static void cmd_frame_back_step(void *p)
 {
     struct mp_cmd_ctx *cmd = p;
-    struct MPContext *mpctx = cmd->mpctx;
-
-    if (!mpctx->playback_initialized) {
-        cmd->success = false;
-        return;
-    }
-
-    add_step_frame(mpctx, -1);
+    struct mp_cmd_arg frames = {.v.i = -1};
+    struct mp_cmd_arg flags = {.v.i = 0};
+    cmd->args = talloc_realloc(cmd, cmd->args, struct mp_cmd_arg, 2);
+    cmd->args[0] = frames;
+    cmd->args[1] = flags;
+    cmd_frame_step(cmd);
 }
 
 static void cmd_quit(void *p)
@@ -6066,8 +6066,17 @@ const struct mp_cmd_def mp_cmds[] = {
     { "stop", cmd_stop,
         { {"flags", OPT_FLAGS(v.i, {"keep-playlist", 1}), .flags = MP_CMD_OPT_ARG} }
     },
-    { "frame-step", cmd_frame_step, .allow_auto_repeat = true,
-        .on_updown = true },
+    { "frame-step", cmd_frame_step,
+        {
+            {"frames", OPT_INT(v.i), OPTDEF_INT(1)},
+            {"flags", OPT_CHOICE(v.i,
+                    {"play", 0},
+                    {"seek", 1}),
+                    .flags = MP_CMD_OPT_ARG},
+        },
+        .allow_auto_repeat = true,
+        .on_updown = true
+    },
     { "frame-back-step", cmd_frame_back_step, .allow_auto_repeat = true },
     { "playlist-next", cmd_playlist_next_prev,
         {

player/core.h

@@ -69,7 +69,7 @@ enum seek_type {
     MPSEEK_RELATIVE,
     MPSEEK_ABSOLUTE,
     MPSEEK_FACTOR,
-    MPSEEK_BACKSTEP,
+    MPSEEK_FRAMESTEP,
 };
 
 enum seek_precision {
@@ -583,7 +583,7 @@ void reset_playback_state(struct MPContext *mpctx);
 void set_pause_state(struct MPContext *mpctx, bool user_pause);
 void update_internal_pause_state(struct MPContext *mpctx);
 void update_core_idle_state(struct MPContext *mpctx);
-void add_step_frame(struct MPContext *mpctx, int dir);
+void add_step_frame(struct MPContext *mpctx, int dir, bool use_seek);
 void queue_seek(struct MPContext *mpctx, enum seek_type type, double amount,
                 enum seek_precision exact, int flags);
 double get_time_length(struct MPContext *mpctx);

player/playloop.c

@@ -203,16 +203,19 @@ void update_screensaver_state(struct MPContext *mpctx)
                                                    : VOCTRL_KILL_SCREENSAVER, NULL);
 }
 
-void add_step_frame(struct MPContext *mpctx, int dir)
+void add_step_frame(struct MPContext *mpctx, int dir, bool use_seek)
 {
     if (!mpctx->vo_chain)
         return;
-    if (dir > 0) {
-        mpctx->step_frames += 1;
+    if (dir > 0 && !use_seek) {
+        mpctx->step_frames += dir;
         set_pause_state(mpctx, false);
-    } else if (dir < 0) {
+    } else {
+        // Offset the frame amount by 1.
+        if (dir < 0)
+            dir += 1;
         if (!mpctx->hrseek_active) {
-            queue_seek(mpctx, MPSEEK_BACKSTEP, 0, MPSEEK_VERY_EXACT, 0);
+            queue_seek(mpctx, MPSEEK_FRAMESTEP, dir, MPSEEK_VERY_EXACT, 0);
             set_pause_state(mpctx, true);
         }
     }
@@ -258,6 +261,17 @@ void reset_playback_state(struct MPContext *mpctx)
     update_core_idle_state(mpctx);
 }
 
+static double calculate_framestep_pts(MPContext *mpctx, double current_time,
+                                      int step_frames)
+{
+    // Crude guess at the pts.
+    int previous_frame = mpctx->num_past_frames - 1;
+    double pts = mpctx->past_frames[previous_frame].approx_duration * step_frames;
+    if (step_frames == 1)
+        pts += 0.001; // arbitrary value so a framestep of 1 actually goes forward
+    return current_time + pts;
+}
+
 static void mp_seek(MPContext *mpctx, struct seek_params seek)
 {
     struct MPOpts *opts = mpctx->opts;
@@ -278,8 +292,9 @@ static void mp_seek(MPContext *mpctx, struct seek_params seek)
     case MPSEEK_ABSOLUTE:
         seek_pts = seek.amount;
         break;
-    case MPSEEK_BACKSTEP:
-        seek_pts = current_time;
+    case MPSEEK_FRAMESTEP:
+        seek_pts = calculate_framestep_pts(mpctx, current_time,
+                                           (int)seek.amount);
         hr_seek_very_exact = true;
         break;
     case MPSEEK_RELATIVE:
@@ -388,7 +403,7 @@ static void mp_seek(MPContext *mpctx, struct seek_params seek)
 
     if (hr_seek) {
         mpctx->hrseek_active = true;
-        mpctx->hrseek_backstep = seek.type == MPSEEK_BACKSTEP;
+        mpctx->hrseek_backstep = seek.type == MPSEEK_FRAMESTEP;
         mpctx->hrseek_pts = seek_pts * mpctx->play_dir;
 
         // allow decoder to drop frames before hrseek_pts
@@ -446,7 +461,7 @@ void queue_seek(struct MPContext *mpctx, enum seek_type type, double amount,
         return;
     case MPSEEK_ABSOLUTE:
     case MPSEEK_FACTOR:
-    case MPSEEK_BACKSTEP:
+    case MPSEEK_FRAMESTEP:
         *seek = (struct seek_params) {
             .type = type,
             .amount = amount,