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 support for native PGS subtitle rendering without transcoding #5688

Merged
merged 7 commits into from
Sep 20, 2024

Conversation

Arcus92
Copy link
Contributor

@Arcus92 Arcus92 commented Jun 9, 2024

Changes
This PR allows the client to request and render native PGS subtitle streams without transcoding. PGS is a graphical subtitle format. The subtitles are rendered locally by the client on-top of the video element.

This adds a new dependency libpgs-js. I just created this library for the Jellyfin project. If desired, you can fork it into the @jellyfin domain.

Tasks

  • Disable native PGS support if 'Subtitle burn-in' is not set to 'auto'.
  • Improve performance by using WebWorkers to render subtitles in a separate thread.

Related Pull Request (backend): jellyfin/jellyfin#12056
Related Pull Request (vue): jellyfin/jellyfin-vue#2404

Copy link

sonarcloud bot commented Jun 15, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@Arcus92
Copy link
Contributor Author

Arcus92 commented Jun 20, 2024

I might need a little help over at libpgs-js on how to get the web-worker output ES compatible.

@ferferga
Copy link
Member

@Arcus92 May I ask you why webpack over rollup or Vite?

@Arcus92
Copy link
Contributor Author

Arcus92 commented Jun 21, 2024

@ferferga To be honest ... I haven't worked on web projects in a long time and it is the only bundler I kinda know. I'll look into other tools over the weekend.

@ferferga
Copy link
Member

ferferga commented Jun 21, 2024

I know the basics of rollup, but I know that Vite has this option, which I use in Vue. I'm not sure how good Vite is for libraries though, and that's why I suggested rollup (since Vite is based on it). Your library is rather simple in terms of building, so I believe it shouldn't be too problematic with either bundler, but you'll likely have much more luck with both instead of webpack.

Don't hesitate in pinging me in Matrix if I can be of some help

@thornbill thornbill added feature New feature or request backend Requires work on the server to finish playback This PR or issue mainly concerns playback labels Jul 10, 2024
@Arcus92
Copy link
Contributor Author

Arcus92 commented Jul 10, 2024

As suggested in the backend PR, there is now an opt-in setting to enable experimental Pgs rendering. I'm working on a few performance and load improvements.

@thornbill thornbill added this to the v10.10.0 milestone Aug 13, 2024
@thornbill thornbill added the release-highlight A major new feature for the next release label Aug 13, 2024
@Sab44
Copy link

Sab44 commented Aug 15, 2024

I just found this PR, thanks for putting in the effort for this!
One question, once this is merged will we also be able to adjust vertical subtitle position for PGS subtitles (similar to SRT subtitles)?
If not, what would need to be done to make that possible?
Thanks!

@dmitrylyzo
Copy link
Contributor

@Sab44 PGS subtitles are image-based. Changing their position will result in markers/labels being displayed in the wrong place. The same goes for SSA/ASS.

@dmitrylyzo
Copy link
Contributor

Regarding Tizen and webOS:
transferControlToOffscreen is supported since Chrome 69. So libpgs-js will only work on Tizen 5.5+ and webOS 6+.

webOS 5 emulator:

Uncaught (in promise) TypeError: this.canvas.transferControlToOffscreen is not a function

@Arcus92
Copy link
Contributor Author

Arcus92 commented Aug 15, 2024

Regarding Tizen and webOS: transferControlToOffscreen is supported since Chrome 69. So libpgs-js will only work on Tizen 5.5+ and webOS 6+.

@dmitrylyzo Thanks for testing this on webOS! I could add a fallback in libpgs-js and use single thread rendering if web worker transfer is not supported. But I don't think rendering on the main thread on these devices is a good idea. It could lead to playback stuttering.
For now I'll exclude pgs format support if transferControlToOffscreen isn't available.

@dmitrylyzo
Copy link
Contributor

@dmitrylyzo Thanks for testing this on webOS! I could add a fallback in libpgs-js and use single thread rendering if web worker transfer is not supported. But I don't think rendering on the main thread on these devices is a good idea. It could lead to playback stuttering. For now I'll exclude pgs format support if transferControlToOffscreen isn't available.

JSO renders in the worker, preparing images to be put on the canvas in the main thread. So the main thread only copies the image buffer from the worker and overlays it onto the canvas.

You probably can do the same: parse PGS in the worker and produce a set of images.

@Arcus92
Copy link
Contributor Author

Arcus92 commented Aug 16, 2024

I should be able to refactor the lib to support both render paths: Rendering in worker by default and rendering in main thread as a fallback. I also try to setup a webOS emulator to verify the changes. Sounds like a fun weekend project 😄

@Arcus92
Copy link
Contributor Author

Arcus92 commented Aug 17, 2024

I haven't tested it in Tizen yet but it's now working in webOS 5.
grafik

@Copper-Bot
Copy link

First, thanks for this PR, it will be really great to have native PGS sub support on Tizen without transcoding 🎉
I was wondering if this work can be transposed for DVD sub (or VobSub) as well, as they are image based subtitles like PGS ?
This will probably require opening another PR, but I was just curious if it could easily be done or not.
I don't fully understand the difference between VobSub and PGS, so maybe it's a dumb question (and off-topic).

@dmitrylyzo
Copy link
Contributor

I know console isn't available in that Chromium version

Have you tried using Chrome 79 (or older)? AFAIK, Chrome 80+ doesn't work.

@Arcus92
Copy link
Contributor Author

Arcus92 commented Sep 3, 2024

Have you tried using Chrome 79 (or older)? AFAIK, Chrome 80+ doesn't work.

@dmitrylyzo WebOS 5 is using Chromium 68. Workers should work since version 4.

@dmitrylyzo
Copy link
Contributor

@dmitrylyzo WebOS 5 is using Chromium 68. Workers should work since version 4.

I mean, you need to use Chrome 79 (and older) for debugging webOS 5 because Google removed some API.

@Arcus92
Copy link
Contributor Author

Arcus92 commented Sep 4, 2024

I mean, you need to use Chrome 79 (and older) for debugging webOS 5 because Google removed some API.

Oh, now I got you. Thanks for the hint. The oldest working build I could find is 75 but debugging doesn't change. However, the web worker is working on Desktop. 68 won't boot up on my desktop.

@dmitrylyzo
Copy link
Contributor

dmitrylyzo commented Sep 6, 2024

@Arcus92 I got it working on webOS 5. We need to polyfill Array.flatMap:

this.displaySets[a].windowDefinitions.flatMap is not a function

I'm debugging using Chrome 79.

@Arcus92
Copy link
Contributor Author

Arcus92 commented Sep 6, 2024

@dmitrylyzo Already fixed this in the webos branch. Haven't merged it yet because the worker is not ... working.

@dmitrylyzo
Copy link
Contributor

@dmitrylyzo Already fixed this in the webos branch. Haven't merged it yet because the worker is not ... working.

I'm testing with libpgs-0.5.0, and I managed to load the worker even on webOS 1.2.
At a minimum, it requires polyfilling of Promise and fetch. I did't get it to parse PGS (it's too late currently 🛌).

FIY, you can get console in the worker on webOS 1.2:

@dmitrylyzo
Copy link
Contributor

dmitrylyzo commented Sep 7, 2024

@Arcus92 My test branch with your webos branch:

  • No-offscreen worker starts on webOS 1.2, but fails on Can't find variable: ImageData (no ImageData in the worker scope?).
  • Render in the main thread works on webOS 1.2.

@jellyfin-bot jellyfin-bot added the merge conflict Conflicts prevent merging label Sep 11, 2024
@jellyfin-bot
Copy link
Collaborator

This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged.

@Arcus92
Copy link
Contributor Author

Arcus92 commented Sep 11, 2024

@dmitrylyzo Sorry I was busy for a few days. Thanks for your contribution. I'll have a look at your commits right now.

@Arcus92
Copy link
Contributor Author

Arcus92 commented Sep 13, 2024

I finally fixed the issues by adding a browser detection in libpgs. WebOS with a Chrome version below 68 will now use the main-thread renderer. Firefox and Chrome version without ImageData-in-worker support will also fallback to the main-thread.

Normally I wouldn't check feature support via the userAgent, but in this case some of these legacy browser (like WebOS 5.0) have a broken worker implementation. I may need to add exceptions for other platforms if future issues are reported.

Copy link
Contributor

@dmitrylyzo dmitrylyzo left a comment

Choose a reason for hiding this comment

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

PR works on:

  • webOS 1.2 (main thread)
  • webOS 5 (main thread)
  • Chrome 128 (worker)
  • Firefox 130 (worker)

I think we can use worker even on webOS 1.2, but it can be implemented separately.

Arcus92 and others added 3 commits September 18, 2024 19:18
Removed extra line breaks and using `classList.toggle` instead of `add` and `remove` to simplify code.

Co-authored-by: Bill Thornton <[email protected]>
…as` or `Worker` support like webOS 1.2 by updating `libpgs`.
Copy link

sonarcloud bot commented Sep 18, 2024

@jellyfin-bot
Copy link
Collaborator

Cloudflare Pages deployment

Latest commit e560d37f99951c8be9d828d065fb2c146a3107d6
Status ✅ Deployed!
Preview URL https://97f6818e.jellyfin-web.pages.dev
Type 🔀 Preview

@thornbill thornbill merged commit 6d0f0e8 into jellyfin:master Sep 20, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request playback This PR or issue mainly concerns playback release-highlight A major new feature for the next release
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants