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 encoding with libx265 and hevc_nvenc #1433
Conversation
{"mediacodec", "h264_mediacodec"}, | ||
{"vaapi", "h264_vaapi"} | ||
{"qsv", hwEncoder + "_qsv"}, | ||
{hwEncoder + "_qsv", hwEncoder + "_qsv"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are there these _qsv
maps doubled? Can we remove those? And that _v4l2m2m
one also maps to itself. Should probably be a default value or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are there these _qsv maps doubled? Can we remove those?
I'm assuming the original author wanted both those values to map to the correct hw encoder. I think a refactor and using enums would answer/solve these questions (see my response in your other discussion).
And that _v4l2m2m one also maps to itself. Should probably be a default value or something.
I'm not sure I understand as I don't see a problem here. Presumably the mapping to itself is the correct action so that we don't have to do any special casing in the code below that uses the map.
@@ -119,6 +127,11 @@ public string GetVideoEncoder(EncodingJobInfo state, EncodingOptions encodingOpt | |||
|
|||
if (!string.IsNullOrEmpty(codec)) | |||
{ | |||
if (string.Equals(codec, "h265", StringComparison.OrdinalIgnoreCase) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All these strings, h265
, hevc
, 265
and all the others used through out the code, should probably be enums or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely agree. I've never seen so many string comparisons in a chunk of code in my life. I initially went down the road of creating enums, but I quickly realized how much code this was going to touch so I changed my mind. I believe that change should go in its own Issue and PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm gonna give you a little tip... enums are a bad idea... Take it or leave it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you elaborate? It seems like codecs for example internally in ffmpeg are already essentially an enumeration, besides they do not change all that often. And for serialization you can specify the string values if you do not want to deal with numbers (codec IDs for example) in your JSON bodies.
What would be the easiest way of testing this PR? |
I tested it by doing the following:
An example URL that I tested:
This results in the following ffmpeg command:
Note that to test libx265 you need a fairly powerful CPU and to test hevc_nvenc you need a |
NVENC for h265 will work on much older GPUs than a Turing. 10-bit support came with Pascal, but even some Maxwell cards will do 4K, and GM206 even 4:4:4. Reference Table: https://developer.nvidia.com/nvidia-video-codec-sdk#NVENCFeatures |
I was thinking of HEVC with B frame support. |
@fhriley I'm sorry this PR has gotten stale (this is our fault). Can you fix the merge conflicts? |
@fhriley Just a ping on this one, are you able to quickly review and correct that merge conflict?
The Contributors one should just have your name at the bottom of the existing list since it's been updated since. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, but review from @jellyfin/backend members would be appreciated.
} | ||
|
||
private string GetH264OrH265Encoder(string defaultEncoder, string hwEncoder, EncodingJobInfo state, EncodingOptions encodingOptions) | ||
{ | ||
// Only use alternative encoders for video files. | ||
// When using concat with folder rips, if the mfx session fails to initialize, ffmpeg will be stuck retrying and will not exit gracefully | ||
// Since transcoding of folder rips is expiremental anyway, it's not worth adding additional variables such as this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jellyfin fails to user Hardware accleration when trying to stream a BluRay Folder. THis needs to be fixed here.
Hi, thanks for adding hevc support to jellyfin. That is really an helpful feature for me and many others with limited upload speeds. I can confirm that the method works as described by @fhriley grabbing the URL and using vlc. Unfortunately there is no setting in Jellyfin to avoid to having to use external programs to make Jellyfin stream in HEVC. Also trying to change the h264 setting in that url on the fly with an browser addon for example fails in jellyfin with the Error: #2803 In that case the error is probably valid because the browser may not support HEVC decoding. Please add a solution to tell jellyfin to stream using HEVC. Maybe using an external player like vlc, so that we don't have to use the hack method to force Jellyfin to play HEVC. Also I might add, that Jellyfin fails to use Hardware acceleration when trying to stream a Bluray FOlder. Thanks for adding this feature. |
Changes
This implements encoding support for h265 using libx265 or hevc_nvenc. Note that I did not implement it for other hardware encoders because I don't know enough about them, and I do not have that hardware. Also note that this will require changes in clients to actually request h265.
Issues
Fix #1432.