Add new VideoRangeTypes to fully support DoVi on webOS #10469
Conversation
|
The build pipeline does not like the name
|
viktory36
changed the title
Maybe just it's more difficult since both are acronyms.. |
Yup. I was thinking more of what the end user might see - if they navigate over to the playback info screen and find "DOVIHDR10" or "DOVIProfile8" I think that would be more confusing than the current. DOVIwithHDR10 DoViHDR10 DolbyVisionWithHDR10Fallback I think I like this last one Would you be opposed to disabling this warning CA1707? Would throwing that into the jellyfin.ruleset work for the build pipeline to pass? |
|
Changed it to I also reverted the "Dont assume HLG/HDR10 tonemapper can support DOVI+HLG/HDR10" commit - it makes sense that if the DOVI+HDR10 media were previously reported as HDR10 and had no issues with any HwTonemap, the new range types representing this media should fall in the same bucket. |
|
Dear @nyanmisaka , please let me know your thoughts on this? |
JPVenson
added
good first issue
|
@nyanmisaka I have some changes locally that implement the suggestions above, but I can't push to the pull request branch. Should I open a new pull request, as the author seems inactive? @viktory36 I think there is an option to permit that, if you would be so kind as to enable it. |
If the OP still doesn't respond, you can make your branch based on his commits and open a new PR. |
|
Hey @GeorgeH005, I've enabled "allow edits by maintainers" to this PR, and added you as a collaborator to my fork if direct commits to the pr doesn't work. Thanks and excited to have this on main! |
|
@viktory36 Thank you very much! |
GeorgeH005
force-pushed
the
support-dovi-hdr-fallback
branch
from
c786ff2 to
4780d22
Compare
|
I pushed the changes and rebased with upstream master. I have a suspicion that more changes are needed, but I am not familiar enough with the codebase to judge that. I will try my best but any guidance is appreciated! |
GeorgeH005
force-pushed
the
support-dovi-hdr-fallback
branch
from
5fa958b to
bbd9628
Compare
|
@nyanmisaka I know I have bothered you enough today, but I would like your opinion on this, whenever possible. Thanks! |
|
I will take a look at them when I have some time. I have made some changes on jellyfin-web but I haven't opened a pull request yet. |
|
@GeorgeH005 , there's a PR open for web here with an open review comment: jellyfin/jellyfin-web#4916 Let me take care of that one |
|
@viktory36 I'll be sure to check out your work when I complete this. Thanks for helping! |
|
Done! @viktory36 You may want to add yourself to the CONTRIBUTORS file. |
GeorgeH005
force-pushed
the
support-dovi-hdr-fallback
branch
from
855b351 to
99d5110
Compare
|
@nyanmisaka Done! |
|
Just want to double check, have you also tested these PRs on real webOS hardware and Chrome with fMP4-HLS enabled? |
|
@nyanmisaka I have on an LG C3 (webOS 23) and Dolby Vision is triggered as expected (with the patched jellyfin-web ofc), and Safari, with at least some sort of HDR going on for sure. I will test Google Chrome now. |
|
I should note that the file was already a Profile 8.1 mp4 manually tagged as dvh1 via ffmpeg6. Haven't tested the remuxing that should happen when jellyfin/jellyfin-web#4916 is applied. I do not think that it triggered on a separate hev1 mp4 file on the LG via jellyfin, but it did using the same file via its built in media player. But that could be an error with the remuxing parameters on my part |
|
The hev1 mp4 might be something to look into. When converting that I did not specify a "-tag:v:0 hev1" argument I just did a "-i 12345.mkv -c:v copy -c:a copy -sn 12345.mp4". Given that the checks in MediaStream.cs are not solely based on codec tags but also on what I presume is MediaInfo output, our code should not be the problem. I just find it curious that it did trigger when using the TV's built in media player with a usb drive. |
|
@nyanmisaka Any insight is appreciated |
Are the source files mentioned here DV P8? What results did you expect? |
|
The source file was an mkv with Dolby Vision Profile 8 but I did not specify a tag when converting the file, so it might have been my fault |
|
@nyanmisaka Will post the Chrome results in a bit, didn't have the time. The takeaway from the above should be that a correctly formatted Profile 8 file plays just fine on a Dolby Vision capable player, which is an improvement |
|
@GeorgeH005 nitpicked on the DOVIWithSDR enum description a little bit =] I reasoned SDR can commonly appear in 8bit or 10bit color depth, and if DV will be at 10bit depth, then (8bit / 10bit) is a fair statement to make -- though I haven't found a file that has DV with 8bit SDR fallback happening on the same media. |
ffmpeg only passes DV metadata from demuxer to muxer when |
|
I think that I did specify that, but I will run the test again just to be sure. |
|
@nyanmisaka Regardless here is the Chrome DevTools Media page on an HDR capable client. 
This seems correct and the colors are as expected. Again, this is a dvh1 tagged Profile8.1 mp4 file confirmed to work both via the built in LG media player, and via the patched jellyfin build on a DV supporting TV |
@nyanmisaka Possibly but it is an improvement! |
Agreed |
|
@nyanmisaka The jellyfin-web team believes this should be merged before theirs. |
All right. Let's merge this first. cc @crobibero |
This commit adds two new values to the VideoRangeType enum to talk about media that have both DOVI and HDR10 or HLG data (DoVi Profile 8) to enable Dolby Vision playback on those browsers.
Changes
Introduced
DOVI_with_HDR10_FallbackandDOVI_with_HLG_FallbackVideoRangeType enums, and added references to these new types wherever I foundVideoRangeType.HDR10 / HLG.MediaStream.cs will now test both of it's HDR10/HLG and DOVI conditions before returning the video range type.
Issues
Addresses #10468