jellyfin-web icon indicating copy to clipboard operation
jellyfin-web copied to clipboard

Trickplay functionality

Open nicknsy opened this issue 1 year ago • 11 comments

This is the associated web code required to get trickplay (https://github.com/jellyfin/jellyfin/pull/9554) working on the frontend.

The main additions are the updateBubbleHtml function in the video player, as well as all the corresponding trickplay configuration & library options.

nicknsy avatar Feb 12 '24 06:02 nicknsy

@nicknsy Given it was mentioned in the server PR description (but not during the review anymore) what's the state of them being included in the hls stream? Did you check if it works with hls.js alone? Because having them in the hls stream is the most correct approach imo.

The endpoint it's still useful ofc for other clients or other users (i.e posters in resume section maybe?)

Reviewing this on phone, so sorry if I missed something

ferferga avatar Feb 12 '24 07:02 ferferga

Given it was mentioned in the server PR description (but not during the review anymore) what's the state of them being included in the hls stream? Did you check if it works with hls.js alone? Because having them in the hls stream is the most correct approach imo.

The endpoint it's still useful ofc for other clients or other users (i.e posters in resume section maybe?)

Reviewing this on phone, so sorry if I missed something

Doesn't look like hls.js supports trickplay natively: https://github.com/video-dev/hls.js/issues/5392. Because of that, I haven't personally been able to test if the HLS playlist file generated by the server actually works or not (it should though). I believe Roku should support HLS trickplay playlists natively so it might be trivial to implement / test on there?

nicknsy avatar Feb 12 '24 07:02 nicknsy

Google's Shaka-player supports EXT-X-IMAGE-STREAM-INF for thumbnails. They have a demo page for easy debugging.

nyanmisaka avatar Feb 12 '24 10:02 nyanmisaka

Google's Shaka-player supports EXT-X-IMAGE-STREAM-INF for thumbnails. They have a demo page for easy debugging.

Thanks, looks like there was an issue with the playlist not doing relative paths correctly, however after a one-line fix it works great with Shaka. Is Shaka-player something currently supported by jellyfin-web?

nicknsy avatar Feb 12 '24 17:02 nicknsy

Google's Shaka-player supports EXT-X-IMAGE-STREAM-INF for thumbnails. They have a demo page for easy debugging.

Thanks, looks like there was an issue with the playlist not doing relative paths correctly, however after a one-line fix it works great with Shaka. Is Shaka-player something currently supported by jellyfin-web?

Great! I've used shaka in the past as a replacement for our hls.js, but when hls.js fixed their code shaka was unnecessary. If you're interested, you can pick it up and add it as an alternate option. https://github.com/jellyfin/jellyfin-web/pull/4041

nyanmisaka avatar Feb 12 '24 17:02 nyanmisaka

@nicknsy Just to clarify, so server-side's hls implementation is working right, correct?

ferferga avatar Feb 12 '24 22:02 ferferga

@nicknsy Just to clarify, so server-side's hls implementation is working right, correct?

Once https://github.com/jellyfin/jellyfin/pull/11000 is merged the server-side hls implementation will be working, as tested with Shaka-player.

nicknsy avatar Feb 12 '24 23:02 nicknsy

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

jellyfin-bot avatar Feb 21 '24 17:02 jellyfin-bot

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

jellyfin-bot avatar Feb 26 '24 22:02 jellyfin-bot

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

jellyfin-bot avatar Mar 05 '24 07:03 jellyfin-bot

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

jellyfin-bot avatar Mar 17 '24 07:03 jellyfin-bot

Looks good overall. I don't have any trickplay images generated yet to test it in action though. I would have expected some overlap between this and the existing chapter bubble functionality, but I don't see any evidence of that in these changes?

I think the main place there would be overlap would be the getBubbleHtml function however it was problematic for trickplay images (really it is even for chapter images) since it deletes the image container from the DOM on every update causing bad pop-in/out effects. I think ideally the chapter images could also be migrated to updateBubbleHtml which is the function I made for trickplay but initially I didn't want to touch that and potentially break something.

Besides that, trickplay generation takes a lot longer and has more options for things like HW accel and tonemapping to the point where it needs a separate config page from chapter images IMO.

nicknsy avatar Mar 19 '24 01:03 nicknsy

I was mainly questioning the potential overlap of the "bubble" code, so that explanation makes sense. :+1:

thornbill avatar Mar 22 '24 12:03 thornbill

Cloudflare Pages deployment

Latest commit 5a93780
Status ✅ Deployed!
Preview URL https://7bfde35d.jellyfin-web.pages.dev
Type 🔀 Preview

View build logs View bot logs

jellyfin-bot avatar Mar 23 '24 16:03 jellyfin-bot