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

Add pgssub to ExoPlayer subtitle codecs #1043

Merged
merged 1 commit into from
May 13, 2023
Merged

Conversation

Maxr1998
Copy link
Member

@Maxr1998 Maxr1998 commented May 13, 2023

ExoPlayer supports decoding embedded pgssub subtitles, so they should be enabled.

Tested locally and confirmed working.
I also removed the likely invalid pgs specification from the external player profile, since ffprobe lists pgs subtitles as pgssub.

ExoPlayer supports decoding embedded pgssub subtitles, so they should be enabled.
@Maxr1998 Maxr1998 added enhancement New feature or request exoplayer Related to the ExoPlayer integration labels May 13, 2023
@Maxr1998 Maxr1998 added this to the v2.5.0 milestone May 13, 2023
@Maxr1998 Maxr1998 requested a review from nielsvanvelzen May 13, 2023 16:16
Copy link
Member

@nielsvanvelzen nielsvanvelzen left a comment

Choose a reason for hiding this comment

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

Funnily enough I added this to the TV app 3 weeks ago :p

jellyfin/jellyfin-androidtv#2690

@nielsvanvelzen nielsvanvelzen merged commit 8b6e2ae into master May 13, 2023
@nielsvanvelzen nielsvanvelzen deleted the enable-pgssub branch May 13, 2023 18:51
@Maxr1998
Copy link
Member Author

Nice 😄
I actually thought it was either already supported in the app or unsupported by ExoPlayer, but I tried to watch a movie recently and only then realized we simply didn't enable it yet.

@silentTeee
Copy link

So I guess I will place my warning here: I don't think ExoPlayer's implementation is all that solid yet. I've been meaning to capture a video of it happening compared to the web player, but if more than one track is supposed to show up on the screen at the same time, I had issues on the TV app with them not showing up correctly after we switched to native support from transcoding. In about 17 hours I should have free time to properly characterize the TV app, and if I still see issues I'll submit a ticket there. I can characterize the mobile app as well if we have a test image.

@Maxr1998
Copy link
Member Author

Good point, support for multiple subtitles at the same time doesn't really exist yet with the current track selection code. Many movies have two sets of subtitles to provide subtitles for foreign languages spoken in the movie (also known as forced subtitles) in addition to the closed captions that transcribe all the conversations.
However, that's unrelated to the PGS subtitle format, the same would happen if you had two text-based subtitle tracks (like SRT).

I believe it would make sense to create a separate ticket for this issue.

@silentTeee
Copy link

I think we might be referring to two different things (my fault, I worded it poorly). I think it's not so much that there are multiple tracks playing at the same time, rather it seems more like there are multiple, parallel captions of text on the screen, at different positions, but part of the same track...at least, in an MKV container it's only labeled as one track.

Sadly that free time I thought I had is going to be delayed, but I will submit a separate ticket to the TV App repo and this one (assuming the same issue happens) once I have recordings of what it looks like across several clients.

Otherwise, congrats on getting this and 2.5.0 out the door!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request exoplayer Related to the ExoPlayer integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants