Skip to content

command: add sub-snap flag to the seek command - #18409

Open
feldgendler wants to merge 1 commit into
mpv-player:masterfrom
feldgendler:sub_snap
Open

command: add sub-snap flag to the seek command#18409
feldgendler wants to merge 1 commit into
mpv-player:masterfrom
feldgendler:sub_snap

Conversation

@feldgendler

@feldgendler feldgendler commented Aug 25, 2026

Copy link
Copy Markdown

seek <amount> sub-snap is a relative seek that lands on the start of the next or previous primary subtitle event when one falls within <amount> seconds, and does an ordinary relative seek otherwise. It is meant to replace the plain relative seek on the arrow keys rather than to sit beside it as one more binding: because it never travels farther than the amount asked for, it does exactly what the seek it replaces would have done wherever no subtitle is in range.

Seeking back a few seconds during dialogue is nearly always an attempt to hear or read one line again. A plain seek -5 lands mid-sentence, so the line has to come round a second time before it is heard whole, and sub-seek -1 jumps to whatever event happens to be previous, which across a silent stretch can be a minute away — which is also why it cannot serve as the everyday seek key. With sub-snap bound to left and right there is nothing to decide before pressing: in dialogue it replays the line from its start, elsewhere it seeks. My own arrow keys have been bound to it since I wrote it and I have not wanted the plain seek back. Language learners are the obvious audience, since repeating individual lines is most of the activity, but this is what anyone rewinding to catch a mumbled line is reaching for.

A script can approximate this with sub-seek followed by a correction seek, but the correction is a second visible jump, and it cannot tell "there is no event within the amount" from "the decoder has not read that far yet". Only the latter is worth waiting for; conflating them makes holding the key down in a gap stall instead of seeking. SD_CTRL_SUB_STEP sees only what has been decoded, and how far that reaches is not exposed to scripts. Key repeat is the other half of it: input is drained in batches, so a script doing a property round-trip per press collapses a burst into one step.

The wait is driven from the playloop by handle_sub_snap() rather than blocking the core. A backward snap needs the region behind the play position, which cannot be read ahead, so the demuxer alone is rewound while playback stays put, and only while paused, since it is shared with the other streams. Repeated presses accumulate into one chain, and the chain starts from the target of a pending seek instead of the play position, so it composes with seeks issued in the same batch rather than replacing them.

The manual gives the limits. Like sub-seek, this only works with events already displayed or within the prefetch range, except that a backward snap also looks behind the play position while paused. It is ignored for the other seek modes and with --play-dir=backward. Whenever the event is not available it falls back to an ordinary relative seek, so it never blocks or waits visibly — it just stops snapping.

Tested with external SRT, embedded ASS and PGS from a Blu-ray rip: bursts of one to four presses from several start positions, forward and backward, on a short file and a ten-minute one; --sub-delay both signs and --sub-speed at 0.5 and 2; subtitles hidden, disabled, and present only as a secondary track; --play-dir=backward; twenty-five presses of key-repeat scrubbing through a gap, compared against a plain seek; and unrelated seeks queued in the same input batch, checked against the result without the flag.

Written with AI assistance, and reviewed with AI tooling. I take full responsibility for the code: I understand what it changes and why, I have tested it myself, I will respond to review in my own words, and it can be submitted under the same license as the files it touches.

@na-na-hi

Copy link
Copy Markdown
Contributor

A script can approximate this with sub-seek followed by a correction seek, but the correction is a second visible jump, and it cannot tell "there is no event within the amount" from "the decoder has not read that far yet". Only the latter is worth waiting for; conflating them makes holding the key down in a gap stall instead of seeking. SD_CTRL_SUB_STEP sees only what has been decoded, and how far that reaches is not exposed to scripts.

This PR has the wrong approach. Scripts already know the exact timings of text subtitle decoded from the sub-lines property. They can caclulate the exact timestamp to seek to. The only thing needs to be done is to make timestamps available for image subtitles too.

On a relative seek, snap to the start of the next or previous primary
subtitle event if it is within the requested amount; otherwise, or when
subtitles are hidden or disabled, do an ordinary relative seek. Unlike
sub-seek, it never jumps farther than the given amount.

The adjacent event is found with SD_CTRL_SUB_STEP, which only sees what
the decoder has already read, so "no event within the amount" has to be
told apart from "not decoded yet"; only the latter is waited for.
handle_sub_snap() drives the decoder from the playloop instead of
blocking the core, and counts the window as read once the demuxer has
read past it. The decoder running dry does not mean the same thing: a
sparse stream reports EOF whenever nothing is queued for it, which in a
stretch without subtitles is most of the time.

A backward snap needs the region behind the play position, which cannot
be read ahead, so the demuxer alone is rewound while playback stays put.
That is done only while paused, since the demuxer is shared with the
other streams. The region decodes from its far end towards the play
position, so the first event it turns up is the farthest one, and only a
window that has been read out can tell a backward chain that there is
nothing in range.

Repeated presses accumulate into one chain instead of each recomputing a
target, because input is drained in batches and a seek queued with
MPSEEK_FLAG_DELAY can still be pending. The chain starts from the target
of a pending seek rather than from the play position, so that it
composes with seeks issued in the same batch instead of replacing them.

Tested with SRT, embedded ASS and PGS, snapping both ways from inside
events and from gaps, into parts of the file that had not been played,
with --sub-delay and --sub-speed set both ways, and with unrelated seeks
queued in the same batch.
@feldgendler

Copy link
Copy Markdown
Author

This PR has the wrong approach. Scripts already know the exact timings of text subtitle decoded from the sub-lines property. They can caclulate the exact timestamp to seek to. The only thing needs to be done is to make timestamps available for image subtitles too.

@na-na-hi You're right about the paragraph you quoted, and I've stopped making that argument: sub-lines does give exact timings, so a script computes the target and issues one seek — there is no correction seek and no second jump. To check the rest of it I wrote the script and used it as a benchmark against this PR. It gets close, and on ordinary linear watching I can't tell them apart. The differences show up in four places.

  1. Backward into a part of the file that hasn't been played. This is the resume case, and it's an everyday one: watch later drops you into the middle of a film, you wonder what was said just before that, and press left. Nothing behind the play position is in memory and no script can fetch it; this PR rewinds the demuxer while paused and reads that region. Since resuming is just start written into the watch-later config, the measurement is that scenario exactly — opening the PGS track of a Blu-ray rip at 1547.5, inside a line, with the previous line running 1540.790-1543.042: the script falls back to a plain seek, the command lands on 1543.167.
  2. Forward with image subtitles. sub_read_packets() makes the demuxer read to the far end of the seek window, past its normal read-ahead, so the command snaps forward at the default --demuxer-readahead-secs=1. A script only sees what has already been decoded, which is about a second ahead, so it has to raise that option globally to compensate.
  3. Key repeat. Presses that arrive in the same input batch accumulate into one seek, rather than a property read plus a seek each — a sub-lines snapshot measures about 0.9 ms for 586 lines and scales linearly, so it isn't free on a heavily typeset file.
  4. --sub-fps is folded into the decoder's speed factor but isn't visible to scripts, so converting sub-lines timings with sub-speed and sub-delay is wrong when it's set.

Doing this turned up a bug, now fixed and pushed. A backward snap silently fell back to a plain seek whenever the window wasn't fully decoded — always, for image subtitles — and while paused, a backward press in a stretch without subtitles took about 550 ms to move. Both came from asking the decoder whether the window had been read: a sparse stream reports EOF whenever nothing is queued for it, so "nothing here" and "not read yet" looked the same. It now goes by how far the demuxer has read, which is what it keeps reading against anyway. That also fixed forward snapping on image subtitles at the default read-ahead. The series is one commit now, since the dec_sub helper it needed is gone with the old logic.

Separately: making image subtitle timings available through sub-lines is worth having whatever happens here, and it's small — sd_lavc already keeps them to serve sub-seek, so it's about twenty lines to report them with no text. I'll send that as its own PR either way.

If the view is still that this belongs in a script rather than the core, I'd rather it be decided on the four cases above than on the paragraph I got wrong, but I'll take the answer.

@feldgendler

Copy link
Copy Markdown
Author

The image subtitle timings are up separately as #18433, since they are useful whatever happens here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants