feat: include future secondary subtitle overlaps

[kuator]

Why

The subtitle observer only knows secondary cues that mpv has already displayed. If the selected primary subtitle window extends beyond the current playback position, later secondary cues that overlap that window are missing from both the card and its OSD preview.

What changed

• Look ahead on demand with sub-step 1 secondary instead of parsing or extracting the subtitle track.
• Capture each stepped cue's raw timestamps and normalize them with the original secondary-sub-delay and audio-delay.
• Restore secondary-sub-delay after lookahead, including when stepping fails.
• Use the same time-overlap resolver for card export and the OSD preview.
• Require 75% overlap when the shorter cue is under one second, and 50% otherwise.
• Ignore subtitle observer callbacks while lookahead temporarily changes the visible secondary cue.

This performs no file I/O and launches no subprocess. For embedded tracks, lookahead is naturally limited to cues inside mpv's subtitle prefetch range.

Verification

• luajit tests/run.lua
• Real mpv probe with external SRT tracks: repeated stepping exposed all test cues and exact raw timestamps, then restored the original delay.
• Real mpv probe with embedded SRT tracks: stepping exposed the prefetched cues and restored the original delay.
• Regression tests retain the incidental-overlap cases and the 50%/75% thresholds.

[tatsumoto-ren]

Extracting a subtitle file from a video can take a few seconds and requires resources, so until now we didn't want to implement it in mpvacious. Plus, it requires writing a lot of code. Before settling on this solution, let's try a proven method. In our sub-transition script we look ahead by changing sub-delay via mp.commandv("no-osd", "sub-step", 1), which causes the next subtitle line to appear. So you can grab the next subtitle this way, then restore sub-delay with mp.set_property_number("sub-delay", initial_sub_delay).

sub-step <skip> [<flags>]

    Change subtitle timing such, that the subtitle event after the next <skip> subtitle events is displayed. <skip> can be negative to step backwards.

    Secondary argument:

    primary (default)
        Steps through the primary subtitles.
    secondary
        Steps through the secondary subtitles.

for secondary subs, use the secondary argument, then restore secondary-sub-delay instead of sub-delay.

[kuator]

Implemented the sub-step approach in eea9b58. I removed the full-track parser/extractor and now step through the secondary track on demand, capture each cue using its raw timestamps, normalize with the original secondary/audio delays, and restore secondary-sub-delay afterward. The existing 50%/75% overlap filter is unchanged and is applied after cue discovery. luajit tests/run.lua passes. I also probed this in mpv with both external and embedded SRT tracks; state restoration worked in both cases. As expected, embedded lookahead is limited to mpv’s prefetch range.

[kuator]

I think I should try using this for a few days before it becomes reviewable

[kuator]

I think I have to make it handle backwards subtitle overlaps as well, because sometimes I jump into the middle of a primary cue and mpvacious doesn't see a backward secondary cue

[tatsumoto-ren]

Simply rewinding back and forth is enough for mpvacious to memorize subtitle lines. shift+h and shift+l are mentioned in README as aliases for seek back and seek forward.

[kuator]

Simply rewinding back and forth is enough for mpvacious to memorize subtitle lines. shift+h and shift+l are mentioned in README as aliases for seek back and seek forward.

Okay, I'll roll back the change

[kuator]

Removed the backwards seek

[tatsumoto-ren]

Removed the backwards seek

sub-step is a pretty clever hack that can be applied in both directions if you actually need it. My comment was more about whether this is something the users might need or not.

You can probably edit scan_secondary_forward() to include direction as a parameter, so you don't have to duplicate much code.

Also, consider splitting this PR into smaller ones if possible.

Please comment when this PR is ready.

[kuator]

bump

[kuator]

bump @tatsumoto-ren

[tatsumoto-ren]

@kuator I'm trying to understand what's going on, but honestly, it's not smooth. There are some strong hints that this PR was mostly not written by a human. It makes it difficult to review. I'll try to figure it out and maybe come up with some solutions.

[tatsumoto-ren]

get_selected_secondary_subs() is not a getter. It can mutate secondary-sub-delay, run many sub-step commands, and insert subtitles. Every menu update triggers scan. This is likely very expensive.

A function named get_* should normally be read-only.

[tatsumoto-ren]

this function is longer than 21 lines, lacks a docstring, has ambiguous name, and its parameters are vaguely named. Also, it takes more than 4 parameters. usually, if a function takes more than 4 parameters, it should be refactored. Maybe some parameters need to be turned into structs.

• observed is an observed secondary sub_list.
• window is a combined primary subtitle.

[tatsumoto-ren]

Imports inside functions do not match the project's convention.

Prefer:

local mp = require('mp')
local msg = require('mp.msg')

then default to mp / msg inside new.

[tatsumoto-ren]

This function is specifically for testing only. If so, it should be separated from production code visually.

------------------------------------------------------------
-- tests

local function run_scan_case(case)
...
end

[tatsumoto-ren]

Honestly, I don't know what cue means. It is the wrong term here. We already have Subtitle, sub, dialogs, and sub_list. Introducing cue creates a second vocabulary for the same thing.

Subtitle.cues → source_subtitles
cue_index → subtitle_index
current_cue() → current_subtitle()
cues → subtitles
"collect N cues" → "collect N subtitle events"
"a cue is accepted" → "a subtitle event is accepted"

[tatsumoto-ren]

some suggestions:

1. Split functions over the 21 line limit (scan(), run_scan_case() )

2. Don't use imports inside functions

3. Add missing comments and docstrings

4. Use existing project terminology (no "cue")

5. Avoid scanning from Menu:update(). Resolve and cache the secondary selection when primary selection/timing changes or before export.

6. Make the scanner responsible only for discovery. The scanner should discover missing secondary subtitle events.

   • Insert the currently visible secondary subtitle.
   • Step backward and forward.
   • Normalize timestamps.
   • Insert discovered secondary subtitle events.
   • Restore secondary sub delay

   It should not:

   • Apply overlap thresholds.
   • Build flattened text.
   • Return { text, subs }.

7. Subtitle should represent one subtitle span. It should not know which other subtitle events were used to build it. Remove the cues field and remove Subtitle:timing_windows().

8. Introduce an observer-owned primary selection

   {
      subtitle = combined_primary_subtitle,
      windows = {
         Subtitle:from_text('', start_time, end_time),
         ---
      },
   }
   

[kuator]

Okay, I'll try to understand what's happening and maybe there's a way to split this merge request

[kuator]

There's also an option of not merging this mr, I can just keep it in my fork since I understand that any extra code is a maintenance burden

[tatsumoto-ren]

You could probably ask an LLM chatbot to refactor the code. These days they can write code better than humans if you direct them properly, but they can also write shitty code that is unreadable and full of issues.