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

silentTeee commented 1 year ago

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)
[X] Integrated player (ExoPlayer)
[ ] External player (VLC, mpv, MX Player)

nielsvanvelzen commented 1 year ago

Probably this upstream issue google/ExoPlayer#7458

silentTeee commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.

jellyfin / jellyfin-android