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

Adding DOVI support on webOS

Open viktory36 opened this issue 1 year ago • 11 comments

-- resubmitting fresh/clean PR --

This change will block non-mp4 containers from direct playing DOVI media on webOS browsers with DOVI support. This, paired with the "Prefer fMP4-HLS container" client playback setting will enable DOVI Profile 5 playback on webOS.

For DOVI Profile 8 (dovi with hdr10/hlg fallback), I think we will require some changes in the server side or in ffprobe. Please see jellyfin/jellyfin#10468#issuecomment-1780029285

Changes Where browser is webOS and has DOVI support, push a CodecProfile that says containers other than mp4 cannot direct play DOVI.

Issues jellyfin/jellyfin-webos#159

-- I've incorporated the latest review comments from dmitrylyzo from the old PR in this one, and happy to know your thoughts here. Should we wait for clarity on dovi-8 related issues before merging this?

viktory36 avatar Oct 25 '23 22:10 viktory36

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

sonarqubecloud[bot] avatar Oct 26 '23 08:10 sonarqubecloud[bot]

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

jellyfin-bot avatar Nov 03 '23 04:11 jellyfin-bot

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

sonarqubecloud[bot] avatar Dec 01 '23 16:12 sonarqubecloud[bot]

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

jellyfin-bot avatar Jan 05 '24 01:01 jellyfin-bot

I decided to try this today - I have an android player and don't need it but I know some people on webos are struggling.

I can't confirm it's working for sure. What I can say is that when playing mkv with DV and HDR layer, it'll play the HDR straight from mkv.

I would expect this to play the DV layer as a preference always. Maybe if that's not covered in this PR, it should be.

Schaka avatar Feb 27 '24 18:02 Schaka

@Schaka, this PR enables playback of DV media that does not have HDR fallback (DV profile 5) by forcing a transmux, like you describe in the recently linked issue. For profile 8, things have to change on the server side (please see linked issue in OP).

@dmitrylyzo hey, I think this PR can be merged in it's current state. While it does not fully enable all dolby vision playback, it will resolve the green-purple issue which still helps a considerable chunk of users.

If a fix for full support is implemented in server side without introducing new videorangetypes, there will be no further changes required in web. If new videorangetypes are introduced in server (like in https://github.com/jellyfin/jellyfin/pull/10469), they will have to be added to the below lines https://github.com/jellyfin/jellyfin-web/blob/713ac0e130bfd452debf1c2a1b25a64b5bcaa01f/src/scripts/browserDeviceProfile.js#L930 https://github.com/jellyfin/jellyfin-web/blob/713ac0e130bfd452debf1c2a1b25a64b5bcaa01f/src/scripts/browserDeviceProfile.js#L936

In any case, the changes tagged to this PR are good to go.

viktory36 avatar Feb 27 '24 19:02 viktory36

Do you have a sample DV5 file I could use to test this? If you guys still need confirmation, that is. On the I'm on the C1.

Schaka avatar Feb 27 '24 19:02 Schaka

There is one here https://kodi.wiki/view/Samples Dolby Vision Mystery Box (Profile 5) Remember to enable fMP4-HLS in playback settings!

viktory36 avatar Feb 27 '24 20:02 viktory36

I tested this PR with the DV5 (in mkv container) and it works perfectly, just transmuxing as expected.

If I wanted to hackfix DV playback for profile 7 and 8, I'd have to remove HDR, HDR10 and HLG from mkv, so that it would force transmux to mp4 as well?

Edit: I actually tried. It will play the HDR10 or HDR fallback when transmuxing to mp4, if I adjust the browserProfile for -mp4 (non-mp4 as I understand) to only accept SDR.

Am I correct in understanding that as of now, transmuxing P7 or P8 loses DV information and thus will only play back HDR/HLG/whatever fallback?

Schaka avatar Feb 27 '24 22:02 Schaka

Am I correct in understanding that as of now, transmuxing P7 or P8 loses DV information and thus will only play back HDR/HLG/whatever fallback?

With jellyfin-ffmpeg5, I am able to remux to MP4 keeping DoVi8. So I suppose the DV info should remain when remuxing.

dmitrylyzo avatar Mar 04 '24 13:03 dmitrylyzo

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 20:03 jellyfin-bot

I'll try to have a look here tomorrow, see if I can help out somehow.

GeorgeH005 avatar Mar 14 '24 22:03 GeorgeH005

Just in case. This PR can be considered as "feature complete" - no need to add more.

My suggestion of using NotEquals is not really necessary, but it might be useful when more video ranges are added in the future. On the other hand, it doesn't work if multiple video ranges are required - need another negation condition that supports a list of values.

DOVI+HDR10/HLG should be implemented in the new PR as it requires changes to the server.

dmitrylyzo avatar Mar 14 '24 23:03 dmitrylyzo

Awesome. @GeorgeH005, do you want to take that one up?

viktory36 avatar Mar 15 '24 01:03 viktory36

@viktory36 Will try to, I will link the pull request here when I open one. Would appreciate your help, given your previous work on the web codebase.

GeorgeH005 avatar Mar 15 '24 10:03 GeorgeH005

No problem - we will just need the new types added in server also added here: https://github.com/jellyfin/jellyfin-web/blob/713ac0e130bfd452debf1c2a1b25a64b5bcaa01f/src/scripts/browserDeviceProfile.js#L930 https://github.com/jellyfin/jellyfin-web/blob/713ac0e130bfd452debf1c2a1b25a64b5bcaa01f/src/scripts/browserDeviceProfile.js#L936

So we will have 3 prs total. This one with no more changes, another one needed in jellyfin-web that deals with with DOVI+HDR10/HLG implementation that modifies above lines + others if necessary, and the one in server that introduces the new types.

With these 3 PRs merged, we will end up with full DOVI support on WebOS, and all other clients will now see "DOVIWithHDR10" in playback info instead of just "HDR10" when they're playing DOVI media.

viktory36 avatar Mar 15 '24 18:03 viktory36

No problem - we will just need the new types added in server also added here:

https://github.com/jellyfin/jellyfin-web/blob/713ac0e130bfd452debf1c2a1b25a64b5bcaa01f/src/scripts/browserDeviceProfile.js#L930

https://github.com/jellyfin/jellyfin-web/blob/713ac0e130bfd452debf1c2a1b25a64b5bcaa01f/src/scripts/browserDeviceProfile.js#L936

My thoughts exactly! I was waiting in order to make sure I wasn't missing anything else that needed changing (frankly I am still checking, but so far so good).

Here is the relevant pull request: jellyfin/jellyfin-web#5282

GeorgeH005 avatar Mar 16 '24 00:03 GeorgeH005

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 09:03 jellyfin-bot

Shouldn't this have already been merged? I will have to make changes to the types mentioned here in #5282, and I suppose it would be better to make them after this has been merged, rather than here directly

GeorgeH005 avatar Mar 17 '24 15:03 GeorgeH005

The PR currently has merge conflicts. I believe it should be good to go once those are resolved.

thornbill avatar Mar 18 '24 02:03 thornbill

@viktory36 Rebase please?

GeorgeH005 avatar Mar 18 '24 11:03 GeorgeH005

@dmitrylyzo @thornbill resolved the conflict. Merge, please?

viktory36 avatar Mar 18 '24 14:03 viktory36

wow this would be great ! would this work on 10.9 then?

iwatch-x avatar Mar 18 '24 19:03 iwatch-x

@iwatch-x Probably in the beta following its merging.

GeorgeH005 avatar Mar 18 '24 19:03 GeorgeH005

Cloudflare Pages deployment

Latest commit 3659355
Status ✅ Deployed!
Preview URL https://8325b07a.jellyfin-web.pages.dev
Type 🔀 Preview

View build logs View bot logs

jellyfin-bot avatar Mar 18 '24 19:03 jellyfin-bot