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

ExoPlayer PGSSUB implementation doesn't correctly render multiple on-screen captions #1045

Open
1 of 3 tasks
silentTeee opened this issue May 15, 2023 · 5 comments
Open
1 of 3 tasks
Labels
bug Something isn't working confirmed Confirmed bug reports that shouldn't go stale exoplayer Related to the ExoPlayer integration upstream An issue that is not related to the app, but an upstream component (dependency, backend, web UI)

Comments

@silentTeee
Copy link

Describe the bug

Unfortunately, there seems to be a major weakness in the ExoPlayer PGSSUB implementation: when more than one chunk of text is supposed to be on-screen at a time, (imagine one chunk for dialogue at the bottom, and another to caption a foreign billboard at the top), it will only display the second caption after removing the first. This can be extremely jarring when watching foreign films or shows, and depending on the timing for the text display, make following along impossible.

Here is a correct example of the rendering in ExoPlayer from 2.5.0-beta.6:
https://github.com/jellyfin/jellyfin-android/assets/20748060/e5cf3c23-1ee1-4e1b-9015-b152a295acdd

...and here is what happens when native PGSSUB rendering is enabled in ExoPlayer in the 2.5.0 official proprietary release:
https://github.com/jellyfin/jellyfin-android/assets/20748060/9c5a44bc-60c0-496e-a3cd-078bb3c87205

Keep in mind, this is a very mild example. When a chunk of dialogue goes on for 10 seconds but some foreign on-screen text has a translation pop up just before the dialogue text does, a lot of information can be lost.

Logs

No response

Application version

2.5.0

Where did you install the app from?

Sideloaded APK (proprietary build)

Device information

Google Pixel 7

Android version

Android 13

Jellyfin server version

10.8.10

Which video player implementations does this bug apply to?

  • Web player (default)
  • Integrated player (ExoPlayer)
  • External player (VLC, mpv, MX Player)
@silentTeee silentTeee added the bug Something isn't working label May 15, 2023
@nielsvanvelzen
Copy link
Member

Probably this upstream issue google/ExoPlayer#7458

@nielsvanvelzen nielsvanvelzen added upstream An issue that is not related to the app, but an upstream component (dependency, backend, web UI) exoplayer Related to the ExoPlayer integration confirmed Confirmed bug reports that shouldn't go stale labels May 15, 2023
@silentTeee
Copy link
Author

Yeah, based on the description that issue certainly sounds like what I was experiencing. Okay, so I can create a ticket in the AndroidTV repo sometime in the next 12 hours for this issue, since it happens on that app as well.

As for a solution, would it be better to simply revert #1043 for now? It's been three years since the upstream issue was reported and it hasn't been resolved or shown any additional progress.

@Maxr1998
Copy link
Member

We could make it configurable just like the ASS subtitles and explain the issue there. No activity in 3 years is certainly discouraging, you're right. Unfortunately the ExoPlayer codebase isn't the most comprehensible either, so I'm uncertain whether I could fix it myself without (or even with) throwing a lot of time at it..

@silentTeee
Copy link
Author

silentTeee commented May 16, 2023

I suppose it could be hidden behind a configuration option, though IMO it should default to being disabled.

I am admittedly a bit wary of keeping this feature for two reasons:

  • Simply because I get an icky feeling whenever I release a feature that I am fully aware is buggy, especially if I know there is no practical fix.
  • In it's current state, how many people will such a feature actually benefit?

My rationale for the second reason is this: so far, with every official release of media I have seen the commonly used subtitle format seems to be PGS. This includes foreign productions. Some directors go out of their way to exclude shots of things with written text to avoid localization issues, but often they don't.

And unless they are re-watching something, users won't know what's going to get captions and what won't, and as such can't predict what their experience will be.

Ultimately, it's the decision of you lovely devs who are contributing your time on whether to keep this as an optional feature. I am merely sharing my opinion as a devoted user and fellow software developer.

Edit: formatting and correcting the wording of my rationale.

@Maxr1998
Copy link
Member

I see, thanks for the insight and explanation. I believe it depends a bit on the media you watch. PGSSUB is used on most DVDs and Blu-ray discs, so very common for direct rips of those. Movies usually have a single dialog line at the same time (at least in my experience), so the overlap issue rarely occurs for those. Transcoding them by default would unnecessarily waste resources in that case, or not even be possible for some users. My server doesn't have a dedicated GPU, and while CPU encoding works for some files, I wouldn't be able to transcode a high bit rate 4K HDR Blu-ray rip in real time. Because the client settings are unfortunately not very discoverable, many people might not even know they exist. I think the default should always be attempting to direct play, and only transcode (and use extra processing power) if the user explicitly requests it.

To be fair, for ASS subtitles we currently don't do that, but ASS is even more broken in ExoPlayer (quietly dropping styling) and commonly used for anime where that styling is specifically used.
Furthermore, text only subtitles are easier to convert, so if a user wants a text only subtitle they could instead use SRT or VTT.
PGSSUB uses bitmaps under the hood and can only be converted manually or with OCR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working confirmed Confirmed bug reports that shouldn't go stale exoplayer Related to the ExoPlayer integration upstream An issue that is not related to the app, but an upstream component (dependency, backend, web UI)
Projects
None yet
Development

No branches or pull requests

3 participants