Skip to content

select.lua: refactor subtitle parse - #15936

Merged
kasper93 merged 1 commit into
mpv-player:masterfrom
dyphire:select
Jul 6, 2025
Merged

select.lua: refactor subtitle parse#15936
kasper93 merged 1 commit into
mpv-player:masterfrom
dyphire:select

Conversation

@dyphire

@dyphire dyphire commented Feb 22, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Avoid repeated display of the same content at the same subtitle time

Keep blank lines for both lrc and srt formats

@github-actions

github-actions Bot commented Feb 22, 2025 •
edited
Loading

Copy link
Copy Markdown

Download the artifacts for this pull request:

Windows
macOS

guidocella
guidocella reviewed Feb 22, 2025
View reviewed changes
Comment thread player/lua/select.lua Outdated
guidocella
guidocella reviewed Feb 22, 2025
View reviewed changes
Comment thread player/lua/select.lua Outdated
@dyphire
dyphire force-pushed the select branch 2 times, most recently from c024695 to d154fc1 Compare February 22, 2025 10:23
guidocella
guidocella reviewed Feb 22, 2025
View reviewed changes
Comment thread player/lua/select.lua Outdated
@dyphire
dyphire force-pushed the select branch 2 times, most recently from 86a9123 to 7256e6d Compare February 22, 2025 10:35
guidocella
guidocella reviewed Feb 22, 2025
View reviewed changes
Comment thread player/lua/select.lua Outdated
guidocella
guidocella reviewed Feb 22, 2025
View reviewed changes
Comment thread player/lua/select.lua Outdated
guidocella
guidocella reviewed Feb 22, 2025
View reviewed changes
Comment thread player/lua/select.lua
@dyphire

dyphire commented Feb 22, 2025

Copy link
Copy Markdown
Contributor Author

I noticed that the original subtitle content parsing didn't address possible ass special content, resulting in a lot of content duplication
image
So I refactored the parsing of the subtitle content and now it will avoid repeated display of the same content at the same time. This reduced the number of subtitles displayed from 6000+ to 1366
image

@avih

avih commented Feb 22, 2025

Copy link
Copy Markdown
Member

The async change should probably be in its own commit (unless you think it's tied closely with the other changes).

@dyphire

dyphire commented Feb 22, 2025

Copy link
Copy Markdown
Contributor Author

The async change should probably be in its own commit (unless you think it's tied closely with the other changes).

I was refactoring while working on other problem issues and forgot to split the commit. Will redo it later.

@dyphire
dyphire force-pushed the select branch 3 times, most recently from d61d1ab to 1d842d5 Compare February 22, 2025 13:46
guidocella
guidocella reviewed Feb 22, 2025
View reviewed changes
Comment thread player/lua/select.lua Outdated
guidocella
guidocella reviewed Feb 22, 2025
View reviewed changes
Comment thread player/lua/select.lua Outdated
@dyphire
dyphire force-pushed the select branch 6 times, most recently from ce1cd29 to f8be4d4 Compare February 23, 2025 04:09
@dyphire
dyphire force-pushed the select branch 2 times, most recently from fb3f8fb to 30921db Compare February 28, 2025 02:07
@kasper93 kasper93 added this to the Release v0.40.0 milestone Feb 28, 2025
kasper93
kasper93 reviewed Mar 4, 2025
View reviewed changes
Comment thread player/lua/select.lua Outdated
@kasper93 kasper93 removed this from the Release v0.40.0 milestone Mar 4, 2025
@dyphire

dyphire commented Jul 6, 2025

Copy link
Copy Markdown
Contributor Author

Any suggestions here? I noticed it has been staying for a long time.

@dyphire
dyphire requested review from guidocella and kasper93 July 6, 2025 07:05
kasper93
kasper93 reviewed Jul 6, 2025
View reviewed changes
Comment thread player/lua/select.lua Outdated
@dyphire
select.lua: refactor subtitle parse
b0efbe5 
Avoid repeated display of the same content at the same subtitle time

Keep blank lines for both lrc and srt formats
@dyphire dyphire changed the title select.lua: asynchronous execution of ffmpeg subprocesses select.lua: refactor subtitle parse Jul 6, 2025
@kasper93
kasper93 merged commit 3f6038f into mpv-player:master Jul 6, 2025
28 checks passed
@verygoodlee verygoodlee mentioned this pull request Dec 10, 2025
Merged
@guidocella guidocella mentioned this pull request May 6, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kasper93 kasper93 kasper93 approved these changes
+1 more reviewer
@guidocella guidocella guidocella approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dyphire @avih @kasper93 @guidocella