Apply suggestions from code review

Co-authored-by: BaronGreenback <jimcartlidge@yahoo.co.uk>
This commit is contained in:
Nyanmisaka 2020-11-11 02:03:53 +00:00 committed by GitHub
parent babb298b90
commit 0b01acbe91
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 26 additions and 25 deletions

View File

@ -1565,7 +1565,7 @@ namespace Jellyfin.Api.Controllers
args += " " + _encodingHelper.GetVideoQualityParam(state, codec, encodingOptions, "veryfast"); args += " " + _encodingHelper.GetVideoQualityParam(state, codec, encodingOptions, "veryfast");
// Unable to force key frames using these encoders, set key frames by GOP // Unable to force key frames using these encoders, set key frames by GOP.
if (string.Equals(codec, "h264_qsv", StringComparison.OrdinalIgnoreCase) if (string.Equals(codec, "h264_qsv", StringComparison.OrdinalIgnoreCase)
|| string.Equals(codec, "h264_nvenc", StringComparison.OrdinalIgnoreCase) || string.Equals(codec, "h264_nvenc", StringComparison.OrdinalIgnoreCase)
|| string.Equals(codec, "h264_amf", StringComparison.OrdinalIgnoreCase) || string.Equals(codec, "h264_amf", StringComparison.OrdinalIgnoreCase)
@ -1587,7 +1587,7 @@ namespace Jellyfin.Api.Controllers
args += " " + keyFrameArg + gopArg; args += " " + keyFrameArg + gopArg;
} }
// Currenly b-frames in libx265 breaks the FMP4-HLS playback on iOS, disable it for now // Currenly b-frames in libx265 breaks the FMP4-HLS playback on iOS, disable it for now.
if (string.Equals(codec, "libx265", StringComparison.OrdinalIgnoreCase)) if (string.Equals(codec, "libx265", StringComparison.OrdinalIgnoreCase))
{ {
args += " -bf 0"; args += " -bf 0";

View File

@ -206,7 +206,7 @@ namespace Jellyfin.Api.Helpers
if (state.VideoStream != null && state.VideoRequest != null) if (state.VideoStream != null && state.VideoRequest != null)
{ {
// Provide SDR HEVC entrance for backward compatibility // Provide SDR HEVC entrance for backward compatibility.
if (EncodingHelper.IsCopyCodec(state.OutputVideoCodec) if (EncodingHelper.IsCopyCodec(state.OutputVideoCodec)
&& !string.IsNullOrEmpty(state.VideoStream.VideoRange) && !string.IsNullOrEmpty(state.VideoStream.VideoRange)
&& string.Equals(state.VideoStream.VideoRange, "HDR", StringComparison.OrdinalIgnoreCase) && string.Equals(state.VideoStream.VideoRange, "HDR", StringComparison.OrdinalIgnoreCase)
@ -215,7 +215,7 @@ namespace Jellyfin.Api.Helpers
var requestedVideoProfiles = state.GetRequestedProfiles("hevc"); var requestedVideoProfiles = state.GetRequestedProfiles("hevc");
if (requestedVideoProfiles != null && requestedVideoProfiles.Length > 0) if (requestedVideoProfiles != null && requestedVideoProfiles.Length > 0)
{ {
// Force HEVC Main Profile and disable video stream copy // Force HEVC Main Profile and disable video stream copy.
state.OutputVideoCodec = "hevc"; state.OutputVideoCodec = "hevc";
var sdrVideoUrl = ReplaceProfile(playlistUrl, "hevc", string.Join(",", requestedVideoProfiles), "main"); var sdrVideoUrl = ReplaceProfile(playlistUrl, "hevc", string.Join(",", requestedVideoProfiles), "main");
sdrVideoUrl += "&AllowVideoStreamCopy=false"; sdrVideoUrl += "&AllowVideoStreamCopy=false";
@ -232,7 +232,7 @@ namespace Jellyfin.Api.Helpers
} }
} }
// Provide Level 5.0 entrance for backward compatibility // Provide Level 5.0 entrance for backward compatibility.
// e.g. Apple A10 chips refuse the master playlist containing SDR HEVC Main Level 5.1 video, // e.g. Apple A10 chips refuse the master playlist containing SDR HEVC Main Level 5.1 video,
// but in fact it is capable of playing videos up to Level 6.1. // but in fact it is capable of playing videos up to Level 6.1.
if (EncodingHelper.IsCopyCodec(state.OutputVideoCodec) if (EncodingHelper.IsCopyCodec(state.OutputVideoCodec)
@ -245,13 +245,13 @@ namespace Jellyfin.Api.Helpers
var playlistCodecsField = new StringBuilder(); var playlistCodecsField = new StringBuilder();
AppendPlaylistCodecsField(playlistCodecsField, state); AppendPlaylistCodecsField(playlistCodecsField, state);
// Force the video level to 5.0 // Force the video level to 5.0.
var originalLevel = state.VideoStream.Level; var originalLevel = state.VideoStream.Level;
state.VideoStream.Level = 150; state.VideoStream.Level = 150;
var newPlaylistCodecsField = new StringBuilder(); var newPlaylistCodecsField = new StringBuilder();
AppendPlaylistCodecsField(newPlaylistCodecsField, state); AppendPlaylistCodecsField(newPlaylistCodecsField, state);
// Restore the video level // Restore the video level.
state.VideoStream.Level = originalLevel; state.VideoStream.Level = originalLevel;
var newPlaylist = ReplacePlaylistCodecsField(basicPlaylist, playlistCodecsField, newPlaylistCodecsField); var newPlaylist = ReplacePlaylistCodecsField(basicPlaylist, playlistCodecsField, newPlaylistCodecsField);
builder.Append(newPlaylist); builder.Append(newPlaylist);
@ -333,7 +333,7 @@ namespace Jellyfin.Api.Helpers
} }
else else
{ {
// Currently we only encode to SDR // Currently we only encode to SDR.
builder.Append(",VIDEO-RANGE=SDR"); builder.Append(",VIDEO-RANGE=SDR");
} }
} }
@ -693,9 +693,10 @@ namespace Jellyfin.Api.Helpers
private string ReplaceProfile(string url, string codec, string oldValue, string newValue) private string ReplaceProfile(string url, string codec, string oldValue, string newValue)
{ {
string profileStr = codec + "-profile=";
return url.Replace( return url.Replace(
codec + "-profile=" + oldValue, profileStr + oldValue,
codec + "-profile=" + newValue, profileStr + newValue,
StringComparison.OrdinalIgnoreCase); StringComparison.OrdinalIgnoreCase);
} }

View File

@ -662,10 +662,10 @@ namespace MediaBrowser.Controller.MediaEncoding
if (string.Equals(state.ActualOutputVideoCodec, "hevc", StringComparison.OrdinalIgnoreCase) if (string.Equals(state.ActualOutputVideoCodec, "hevc", StringComparison.OrdinalIgnoreCase)
|| string.Equals(state.ActualOutputVideoCodec, "h265", StringComparison.OrdinalIgnoreCase)) || string.Equals(state.ActualOutputVideoCodec, "h265", StringComparison.OrdinalIgnoreCase))
{ {
// Transcode to level 5.0 and lower for maximum compatibility // Transcode to level 5.0 and lower for maximum compatibility.
// level 5.0 is suitable for up to 4k 30fps hevc encoding, otherwise let the encoder to handle it // Level 5.0 is suitable for up to 4k 30fps hevc encoding, otherwise let the encoder to handle it.
// https://en.wikipedia.org/wiki/High_Efficiency_Video_Coding_tiers_and_levels // https://en.wikipedia.org/wiki/High_Efficiency_Video_Coding_tiers_and_levels
// MaxLumaSampleRate = 3840*2160*30 = 248832000 < 267386880, // MaxLumaSampleRate = 3840*2160*30 = 248832000 < 267386880.
if (requestLevel >= 150) if (requestLevel >= 150)
{ {
return "150"; return "150";
@ -673,7 +673,7 @@ namespace MediaBrowser.Controller.MediaEncoding
} }
else if (string.Equals(state.ActualOutputVideoCodec, "h264", StringComparison.OrdinalIgnoreCase)) else if (string.Equals(state.ActualOutputVideoCodec, "h264", StringComparison.OrdinalIgnoreCase))
{ {
// Clients may direct play higher than level 41, but there's no reason to transcode higher // Clients may direct play higher than level 41, but there's no reason to transcode higher.
if (requestLevel >= 41) if (requestLevel >= 41)
{ {
return "41"; return "41";
@ -843,7 +843,7 @@ namespace MediaBrowser.Controller.MediaEncoding
else if (string.Equals(videoEncoder, "h264_nvenc", StringComparison.OrdinalIgnoreCase) // h264 (h264_nvenc) else if (string.Equals(videoEncoder, "h264_nvenc", StringComparison.OrdinalIgnoreCase) // h264 (h264_nvenc)
|| string.Equals(videoEncoder, "hevc_nvenc", StringComparison.OrdinalIgnoreCase)) // hevc (hevc_nvenc) || string.Equals(videoEncoder, "hevc_nvenc", StringComparison.OrdinalIgnoreCase)) // hevc (hevc_nvenc)
{ {
// following preset will be deprecated in ffmpeg 4.4, use p1~p7 instead // following preset will be deprecated in ffmpeg 4.4, use p1~p7 instead.
switch (encodingOptions.EncoderPreset) switch (encodingOptions.EncoderPreset)
{ {
case "veryslow": case "veryslow":
@ -976,7 +976,7 @@ namespace MediaBrowser.Controller.MediaEncoding
var profile = state.GetRequestedProfiles(targetVideoCodec).FirstOrDefault(); var profile = state.GetRequestedProfiles(targetVideoCodec).FirstOrDefault();
profile = Regex.Replace(profile, @"\s+", String.Empty); profile = Regex.Replace(profile, @"\s+", String.Empty);
// only libx264 support encoding H264 High 10 Profile, otherwise force High Profile // Only libx264 support encoding H264 High 10 Profile, otherwise force High Profile.
if (!string.Equals(videoEncoder, "libx264", StringComparison.OrdinalIgnoreCase) if (!string.Equals(videoEncoder, "libx264", StringComparison.OrdinalIgnoreCase)
&& profile != null && profile != null
&& profile.IndexOf("high 10", StringComparison.OrdinalIgnoreCase) != -1) && profile.IndexOf("high 10", StringComparison.OrdinalIgnoreCase) != -1)
@ -985,7 +985,7 @@ namespace MediaBrowser.Controller.MediaEncoding
} }
// h264_vaapi does not support Baseline profile, force Constrained Baseline in this case, // h264_vaapi does not support Baseline profile, force Constrained Baseline in this case,
// which is compatible (and ugly) // which is compatible (and ugly).
if (string.Equals(videoEncoder, "h264_vaapi", StringComparison.OrdinalIgnoreCase) if (string.Equals(videoEncoder, "h264_vaapi", StringComparison.OrdinalIgnoreCase)
&& profile != null && profile != null
&& profile.IndexOf("baseline", StringComparison.OrdinalIgnoreCase) != -1) && profile.IndexOf("baseline", StringComparison.OrdinalIgnoreCase) != -1)
@ -993,7 +993,7 @@ namespace MediaBrowser.Controller.MediaEncoding
profile = "constrained_baseline"; profile = "constrained_baseline";
} }
// libx264, h264_qsv and h264_nvenc does not support Constrained Baseline profile, force Baseline in this case // libx264, h264_qsv and h264_nvenc does not support Constrained Baseline profile, force Baseline in this case.
if ((string.Equals(videoEncoder, "libx264", StringComparison.OrdinalIgnoreCase) if ((string.Equals(videoEncoder, "libx264", StringComparison.OrdinalIgnoreCase)
|| string.Equals(videoEncoder, "h264_qsv", StringComparison.OrdinalIgnoreCase) || string.Equals(videoEncoder, "h264_qsv", StringComparison.OrdinalIgnoreCase)
|| string.Equals(videoEncoder, "h264_nvenc", StringComparison.OrdinalIgnoreCase)) || string.Equals(videoEncoder, "h264_nvenc", StringComparison.OrdinalIgnoreCase))
@ -1003,7 +1003,7 @@ namespace MediaBrowser.Controller.MediaEncoding
profile = "baseline"; profile = "baseline";
} }
// Currently hevc_amf only support encoding HEVC Main Profile, otherwise force Main Profile // Currently hevc_amf only support encoding HEVC Main Profile, otherwise force Main Profile.
if (!string.Equals(videoEncoder, "hevc_amf", StringComparison.OrdinalIgnoreCase) if (!string.Equals(videoEncoder, "hevc_amf", StringComparison.OrdinalIgnoreCase)
&& profile != null && profile != null
&& profile.IndexOf("main 10", StringComparison.OrdinalIgnoreCase) != -1) && profile.IndexOf("main 10", StringComparison.OrdinalIgnoreCase) != -1)
@ -1027,7 +1027,7 @@ namespace MediaBrowser.Controller.MediaEncoding
{ {
level = NormalizeTranscodingLevel(state, level); level = NormalizeTranscodingLevel(state, level);
// libx264, QSV, AMF, VAAPI can adjust the given level to match the output // libx264, QSV, AMF, VAAPI can adjust the given level to match the output.
if (string.Equals(videoEncoder, "h264_qsv", StringComparison.OrdinalIgnoreCase) if (string.Equals(videoEncoder, "h264_qsv", StringComparison.OrdinalIgnoreCase)
|| string.Equals(videoEncoder, "libx264", StringComparison.OrdinalIgnoreCase)) || string.Equals(videoEncoder, "libx264", StringComparison.OrdinalIgnoreCase))
{ {
@ -1035,7 +1035,7 @@ namespace MediaBrowser.Controller.MediaEncoding
} }
else if (string.Equals(videoEncoder, "hevc_qsv", StringComparison.OrdinalIgnoreCase)) else if (string.Equals(videoEncoder, "hevc_qsv", StringComparison.OrdinalIgnoreCase))
{ {
// hevc_qsv use -level 51 instead of -level 153 // hevc_qsv use -level 51 instead of -level 153.
if (double.TryParse(level, NumberStyles.Any, _usCulture, out double hevcLevel)) if (double.TryParse(level, NumberStyles.Any, _usCulture, out double hevcLevel))
{ {
param += " -level " + hevcLevel / 3; param += " -level " + hevcLevel / 3;
@ -1066,10 +1066,10 @@ namespace MediaBrowser.Controller.MediaEncoding
if (string.Equals(videoEncoder, "libx265", StringComparison.OrdinalIgnoreCase)) if (string.Equals(videoEncoder, "libx265", StringComparison.OrdinalIgnoreCase))
{ {
// libx265 only accept level option in -x265-params // libx265 only accept level option in -x265-params.
// level option may cause libx265 to fail // level option may cause libx265 to fail.
// libx265 cannot adjust the given level, just throw an error // libx265 cannot adjust the given level, just throw an error.
// TODO: set fine tuned params // TODO: set fine tuned params.
param += " -x265-params:0 no-info=1"; param += " -x265-params:0 no-info=1";
} }