Skip to content
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

Custom subtitle outline width #1834

Open
LagradOst opened this issue Oct 27, 2024 · 1 comment · May be fixed by #1840
Open

Custom subtitle outline width #1834

LagradOst opened this issue Oct 27, 2024 · 1 comment · May be fixed by #1840

Comments

@LagradOst
Copy link

Use case description

Subtitle edge (text outline) is an important part of accessibility to make text readable. However the current implementation hardcodes this to twoDpInPx and on some devices like some TV screens, this might be very small and make text unreadable. There is currently no API surface nor internal method accessible via reflection to change the subtitle outline width.

Proposed solutions/Alternatives considered

  1. Allow for some sort of modification of this via CaptionStyleCompat. I know this might not be possible due to android.view.accessibility.CaptioningManager.CaptionStyle. But honestly, this restriction is makes no sense if I as an app developer want to deliver an specific experience.
  2. Change this to Sp. Why is it even in Dp to begin with? The android docs specifically calls Sp when text is involved. https://developer.android.com/training/multiscreen/screendensities
  3. Allow for cues to modify this so we can modify the text style in SubtitleParser.
  4. Allow for custom subtitle painters/subtitle views. Right now the subtitle view is directly set to R.id.exo_subtitles and is a final SubtitleView, meaning that there is no possible way to modify the subtitle rendering after parsing it.
  5. Insert a hidden reflection method/mark as non final. Even with reflection this property is currently impossible to modify as it is marked as private + final. Just adding a hidden method or marking this as non final would fix this. It is impossible to modify this one variable without forking the entire repository.

Note, this is a resubmission of google/ExoPlayer#10507 but exoplayer has been marked as deprecated.

@icbaker
Copy link
Collaborator

icbaker commented Oct 28, 2024

Thanks - I think my comment here about the 'right' way to implement a fully custom outline (by adding it to Cue) probably still stands (i.e. your 3) suggestion): google/ExoPlayer#10507 (comment)

We don't have plans to do this soon, but would consider a high quality PR implementing this (including rendering support in both CanvasSubtitleOutput and WebViewSubtitleOutput).


2. Change this to Sp. Why is it even in Dp to begin with? The android docs specifically calls Sp when text is involved. https://developer.android.com/training/multiscreen/screendensities

This is interesting - looking at the blame layer, it's been this way for a long time. Since this code was first published to GitHub in 2017, but internally I can see this line was added in the very first version of the SubtitleView file in 2014. So it's not something we've deliberately decided to do recently.

However, the link you posted doesn't seem to explicitly say that "text related styling" should use sp, just text size itself (which isn't quite what we're discussing here):

When defining text sizes, you can instead use scalable pixels (sp) as your units. The sp unit is the same size as a dp, by default, but it resizes based on the user's preferred text size. Never use sp for layout sizes.

@LagradOst LagradOst linked a pull request Oct 28, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants