这是indexloc提供的服务,不要输入任何密码
Skip to content

Fix RTL punctuation marks issues with mixed BiDi subtitles #2553

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

orenskl
Copy link

@orenskl orenskl commented Jun 21, 2025

This PR fixes the punctuation marks issues with subtitles text containing a mix of RTL and LTR languages.

google/ExoPlayer#11214
jellyfin/jellyfin-androidtv#4296

Copy link

google-cla bot commented Jun 21, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@orenskl orenskl changed the title Fix RTL punctuation issues with mixed BiDi subtitles Fix RTL punctuation marks issues with mixed BiDi subtitles Jun 21, 2025
@tonihei tonihei assigned tonihei and icbaker and unassigned tonihei Jun 27, 2025
Copy link
Collaborator

@icbaker icbaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I've added some comments. I was also wondering if you'd be able to provide an example stream or subtitle file that demonstrates the problem you're fixing here? So I can run the demo app without this patch, see the problem, then run it again and see it fixed.

I would also be interested to see what happens with span styling when this code is triggered - because atm I'm worried it might all be lost.

@orenskl
Copy link
Author

orenskl commented Jul 7, 2025

Hi, thanks for the comments.

This is a standard SRT file that can be used to reproduce this issue (just rename to srt if needed):

subtitles.txt

Here are some examples:

These lines are displayed as:

Screenshot 2025-07-07 at 22 21 24

should be displayed as:

Screenshot 2025-07-07 at 22 51 06

These lines are displayed as:

Screenshot 2025-07-07 at 22 21 47

Should be displayed as:

Screenshot 2025-07-07 at 22 50 38

You can notice that the punctuation marks are displayed to the right of the sentences and should be displayed to the left (as in all RTL languages). Hope you are good with Hebrew :-)

@icbaker
Copy link
Collaborator

icbaker commented Jul 8, 2025

Thank you for the test file and screenshots, that's really helpful.

I took your SRT file and adapted it to WebVTT so I could add some styling (borrowed from one of our test files), and confirmed that this PR currently strips styling info.

Using this file (just rename to .vtt suffix):
vtt-2553.txt

Compare before your change (styling visible, punctuation in the wrong place):

android-screenshot-2025-07-08T113424

android-screenshot-2025-07-08T113602

With after (styling lost, punctuation in the correct place):

android-screenshot-2025-07-08T113753

android-screenshot-2025-07-08T113803


Please take a look and see if you can tweak this PR so it puts the punctuation in the correct place while preserving (or restoring) the span styling.

@orenskl
Copy link
Author

orenskl commented Jul 8, 2025

I was able to reproduce the styling issues with the VTT file on my side, I think the best option here will be to manipulate the cues and then restore the original spans with the indices fixed. I am working on this.

orenskl added 2 commits July 21, 2025 13:53
- Added support for non Spanned cues
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants