From e103d087d316bc19bf6227b21e8d9dfd091fd6a7 Mon Sep 17 00:00:00 2001 From: Max Git Date: Mon, 1 Jun 2020 07:10:15 +0200 Subject: [PATCH 1/6] Try harder at detecting FFmpeg version and enable the validation --- .../Encoder/EncoderValidator.cs | 120 ++++++++++++++---- .../Encoder/MediaEncoder.cs | 3 - .../EncoderValidatorTests.cs | 7 +- 3 files changed, 97 insertions(+), 33 deletions(-) diff --git a/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs b/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs index 6e036d24c1..d13d6045ea 100644 --- a/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs +++ b/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Globalization; using System.Linq; using System.Text; using System.Text.RegularExpressions; @@ -12,7 +13,7 @@ namespace MediaBrowser.MediaEncoding.Encoder { private const string DefaultEncoderPath = "ffmpeg"; - private static readonly string[] requiredDecoders = new[] + private static readonly string[] _requiredDecoders = new[] { "mpeg2video", "h264_qsv", @@ -33,7 +34,7 @@ namespace MediaBrowser.MediaEncoding.Encoder "hevc" }; - private static readonly string[] requiredEncoders = new[] + private static readonly string[] _requiredEncoders = new[] { "libx264", "libx265", @@ -61,7 +62,19 @@ namespace MediaBrowser.MediaEncoding.Encoder "hevc_amf" }; - // Try and use the individual library versions to determine a FFmpeg version + // These are the library versions that corresponds to our minimum ffmpeg version 4.x according to the version table below + private static readonly IReadOnlyDictionary _ffmpegMinimumLibraryVersions = new Dictionary + { + {"libavutil", 56.14}, + {"libavcodec", 58.18 }, + {"libavformat", 58.12 }, + {"libavdevice", 58.3 }, + {"libavfilter", 7.16 }, + {"libswscale", 5.1 }, + {"libswresample", 3.1}, + {"libpostproc", 55.1 } + }; + // This lookup table is to be maintained with the following command line: // $ ffmpeg -version | perl -ne ' print "$1=$2.$3," if /^(lib\w+)\s+(\d+)\.\s*(\d+)/' private static readonly IReadOnlyDictionary _ffmpegVersionMap = new Dictionary @@ -123,32 +136,36 @@ namespace MediaBrowser.MediaEncoding.Encoder // Work out what the version under test is var version = GetFFmpegVersion(versionOutput); - _logger.LogInformation("Found ffmpeg version {0}", version != null ? version.ToString() : "unknown"); + _logger.LogInformation("Found ffmpeg version {Version}", version != null ? version.ToString() : "unknown"); if (version == null) { - if (MinVersion != null && MaxVersion != null) // Version is unknown + if (MaxVersion != null) // Version is unknown { if (MinVersion == MaxVersion) { - _logger.LogWarning("FFmpeg validation: We recommend ffmpeg version {0}", MinVersion); + _logger.LogWarning("FFmpeg validation: We recommend version {MinVersion}", MinVersion); } else { - _logger.LogWarning("FFmpeg validation: We recommend a minimum of {0} and maximum of {1}", MinVersion, MaxVersion); + _logger.LogWarning("FFmpeg validation: We recommend a minimum of {MinVersion} and maximum of {MaxVersion}", MinVersion, MaxVersion); } } + else + { + _logger.LogWarning("FFmpeg validation: We recommend minimum version {MinVersion}", MinVersion); + } return false; } - else if (MinVersion != null && version < MinVersion) // Version is below what we recommend + else if (version < MinVersion) // Version is below what we recommend { - _logger.LogWarning("FFmpeg validation: The minimum recommended ffmpeg version is {0}", MinVersion); + _logger.LogWarning("FFmpeg validation: The minimum recommended version is {MinVersion}", MinVersion); return false; } else if (MaxVersion != null && version > MaxVersion) // Version is above what we recommend { - _logger.LogWarning("FFmpeg validation: The maximum recommended ffmpeg version is {0}", MaxVersion); + _logger.LogWarning("FFmpeg validation: The maximum recommended version is {MaxVersion}", MaxVersion); return false; } @@ -162,13 +179,12 @@ namespace MediaBrowser.MediaEncoding.Encoder /// /// Using the output from "ffmpeg -version" work out the FFmpeg version. /// For pre-built binaries the first line should contain a string like "ffmpeg version x.y", which is easy - /// to parse. If this is not available, then we try to match known library versions to FFmpeg versions. - /// If that fails then we use one of the main libraries to determine if it's new/older than the latest - /// we have stored. + /// to parse. If this is not available, then we try to match known library versions to FFmpeg versions. + /// If that fails then we test the libraries to determine if they're newer than our minimum versions. /// /// /// - internal static Version GetFFmpegVersion(string output) + internal Version GetFFmpegVersion(string output) { // For pre-built binaries the FFmpeg version should be mentioned at the very start of the output var match = Regex.Match(output, @"^ffmpeg version n?((?:\d+\.?)+)"); @@ -179,37 +195,87 @@ namespace MediaBrowser.MediaEncoding.Encoder } else { - // Create a reduced version string and lookup key from dictionary - var reducedVersion = GetLibrariesVersionString(output); + if (!TryGetFFmpegLibraryVersions(output, out string versionString, out IReadOnlyDictionary versionMap)) + { + _logger.LogError("No ffmpeg library versions found"); - // Try to lookup the string and return Key, otherwise if not found returns null - return _ffmpegVersionMap.TryGetValue(reducedVersion, out Version version) ? version : null; + return null; + } + + // First try to lookup the full version string + if (_ffmpegVersionMap.TryGetValue(versionString, out Version version)) + { + return version; + } + + // Then try to test for minimum library versions + return TestMinimumFFmpegLibraryVersions(versionMap); } } + private Version TestMinimumFFmpegLibraryVersions(IReadOnlyDictionary versionMap) + { + var allVersionsValidated = true; + + foreach (var minimumVersion in _ffmpegMinimumLibraryVersions) + { + if (versionMap.TryGetValue(minimumVersion.Key, out var foundVersion)) + { + if (foundVersion >= minimumVersion.Value) + { + _logger.LogInformation("Found {Library} version {FoundVersion} ({MinimumVersion})", minimumVersion.Key, foundVersion, minimumVersion.Value); + } + else + { + _logger.LogWarning("Found {Library} version {FoundVersion} lower than recommended version {MinimumVersion}", minimumVersion.Key, foundVersion, minimumVersion.Value); + allVersionsValidated = false; + } + } + else + { + _logger.LogError("{Library} version not found", minimumVersion.Key); + allVersionsValidated = false; + } + } + + return allVersionsValidated ? MinVersion : null; + } + /// /// Grabs the library names and major.minor version numbers from the 'ffmpeg -version' output - /// and condenses them on to one line. Output format is "name1=major.minor,name2=major.minor,etc." /// /// + /// + /// /// - private static string GetLibrariesVersionString(string output) + private static bool TryGetFFmpegLibraryVersions(string output, out string versionString, out IReadOnlyDictionary versionMap) { - var rc = new StringBuilder(144); - foreach (Match m in Regex.Matches( + var sb = new StringBuilder(144); + + var map = new Dictionary(); + + foreach (Match match in Regex.Matches( output, @"((?lib\w+)\s+(?\d+)\.\s*(?\d+))", RegexOptions.Multiline)) { - rc.Append(m.Groups["name"]) + sb.Append(match.Groups["name"]) .Append('=') - .Append(m.Groups["major"]) + .Append(match.Groups["major"]) .Append('.') - .Append(m.Groups["minor"]) + .Append(match.Groups["minor"]) .Append(','); + + var str = $"{match.Groups["major"]}.{match.Groups["minor"]}"; + var versionNumber = double.Parse(str, NumberStyles.AllowDecimalPoint, CultureInfo.InvariantCulture); + + map.Add(match.Groups["name"].Value, versionNumber); } - return rc.Length == 0 ? null : rc.ToString(); + versionString = sb.ToString(); + versionMap = map as IReadOnlyDictionary; + + return sb.Length > 0; } private enum Codec @@ -236,7 +302,7 @@ namespace MediaBrowser.MediaEncoding.Encoder return Enumerable.Empty(); } - var required = codec == Codec.Encoder ? requiredEncoders : requiredDecoders; + var required = codec == Codec.Encoder ? _requiredEncoders : _requiredDecoders; var found = Regex .Matches(output, @"^\s\S{6}\s(?[\w|-]+)\s+.+$", RegexOptions.Multiline) diff --git a/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs b/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs index 1377502dd9..2abd31a504 100644 --- a/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs +++ b/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs @@ -183,9 +183,6 @@ namespace MediaBrowser.MediaEncoding.Encoder _logger.LogWarning("FFmpeg: {Location}: Failed version check: {Path}", location, path); } - // ToDo - Enable the ffmpeg validator. At the moment any version can be used. - rc = true; - _ffmpegPath = path; EncoderLocation = location; } diff --git a/tests/Jellyfin.MediaEncoding.Tests/EncoderValidatorTests.cs b/tests/Jellyfin.MediaEncoding.Tests/EncoderValidatorTests.cs index e0f1f236c7..ae389efcd1 100644 --- a/tests/Jellyfin.MediaEncoding.Tests/EncoderValidatorTests.cs +++ b/tests/Jellyfin.MediaEncoding.Tests/EncoderValidatorTests.cs @@ -17,7 +17,7 @@ namespace Jellyfin.MediaEncoding.Tests yield return new object?[] { EncoderValidatorTestsData.FFmpegV42Output, new Version(4, 2) }; yield return new object?[] { EncoderValidatorTestsData.FFmpegV414Output, new Version(4, 1, 4) }; yield return new object?[] { EncoderValidatorTestsData.FFmpegV404Output, new Version(4, 0, 4) }; - yield return new object?[] { EncoderValidatorTestsData.FFmpegGitUnknownOutput, null }; + yield return new object?[] { EncoderValidatorTestsData.FFmpegGitUnknownOutput, new Version(4, 0) }; } IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); @@ -27,7 +27,8 @@ namespace Jellyfin.MediaEncoding.Tests [ClassData(typeof(GetFFmpegVersionTestData))] public void GetFFmpegVersionTest(string versionOutput, Version? version) { - Assert.Equal(version, EncoderValidator.GetFFmpegVersion(versionOutput)); + var val = new EncoderValidator(new NullLogger()); + Assert.Equal(version, val.GetFFmpegVersion(versionOutput)); } [Theory] @@ -35,7 +36,7 @@ namespace Jellyfin.MediaEncoding.Tests [InlineData(EncoderValidatorTestsData.FFmpegV42Output, true)] [InlineData(EncoderValidatorTestsData.FFmpegV414Output, true)] [InlineData(EncoderValidatorTestsData.FFmpegV404Output, true)] - [InlineData(EncoderValidatorTestsData.FFmpegGitUnknownOutput, false)] + [InlineData(EncoderValidatorTestsData.FFmpegGitUnknownOutput, true)] public void ValidateVersionInternalTest(string versionOutput, bool valid) { var val = new EncoderValidator(new NullLogger()); From 480fd0a66a7d8e47542e89b4751b9cf59b5faa9b Mon Sep 17 00:00:00 2001 From: rotvel Date: Mon, 1 Jun 2020 16:00:58 +0200 Subject: [PATCH 2/6] Update MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs Co-authored-by: Cody Robibero --- .../Encoder/EncoderValidator.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs b/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs index d13d6045ea..801479eede 100644 --- a/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs +++ b/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs @@ -65,14 +65,14 @@ namespace MediaBrowser.MediaEncoding.Encoder // These are the library versions that corresponds to our minimum ffmpeg version 4.x according to the version table below private static readonly IReadOnlyDictionary _ffmpegMinimumLibraryVersions = new Dictionary { - {"libavutil", 56.14}, - {"libavcodec", 58.18 }, - {"libavformat", 58.12 }, - {"libavdevice", 58.3 }, - {"libavfilter", 7.16 }, - {"libswscale", 5.1 }, - {"libswresample", 3.1}, - {"libpostproc", 55.1 } + { "libavutil", 56.14 }, + { "libavcodec", 58.18 }, + { "libavformat", 58.12 }, + { "libavdevice", 58.3 }, + { "libavfilter", 7.16 }, + { "libswscale", 5.1 }, + { "libswresample", 3.1 }, + { "libpostproc", 55.1 } }; // This lookup table is to be maintained with the following command line: From 3fbc387257da56885cf5fb00b90125deb889b453 Mon Sep 17 00:00:00 2001 From: Max Git Date: Mon, 15 Jun 2020 12:15:05 +0200 Subject: [PATCH 3/6] Fix stylecop error like on master. --- .../EncoderValidatorTests.cs | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/Jellyfin.MediaEncoding.Tests/EncoderValidatorTests.cs b/tests/Jellyfin.MediaEncoding.Tests/EncoderValidatorTests.cs index ae389efcd1..a9e3cf7f07 100644 --- a/tests/Jellyfin.MediaEncoding.Tests/EncoderValidatorTests.cs +++ b/tests/Jellyfin.MediaEncoding.Tests/EncoderValidatorTests.cs @@ -9,20 +9,6 @@ namespace Jellyfin.MediaEncoding.Tests { public class EncoderValidatorTests { - private class GetFFmpegVersionTestData : IEnumerable - { - public IEnumerator GetEnumerator() - { - yield return new object?[] { EncoderValidatorTestsData.FFmpegV421Output, new Version(4, 2, 1) }; - yield return new object?[] { EncoderValidatorTestsData.FFmpegV42Output, new Version(4, 2) }; - yield return new object?[] { EncoderValidatorTestsData.FFmpegV414Output, new Version(4, 1, 4) }; - yield return new object?[] { EncoderValidatorTestsData.FFmpegV404Output, new Version(4, 0, 4) }; - yield return new object?[] { EncoderValidatorTestsData.FFmpegGitUnknownOutput, new Version(4, 0) }; - } - - IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); - } - [Theory] [ClassData(typeof(GetFFmpegVersionTestData))] public void GetFFmpegVersionTest(string versionOutput, Version? version) @@ -42,5 +28,19 @@ namespace Jellyfin.MediaEncoding.Tests var val = new EncoderValidator(new NullLogger()); Assert.Equal(valid, val.ValidateVersionInternal(versionOutput)); } + + private class GetFFmpegVersionTestData : IEnumerable + { + public IEnumerator GetEnumerator() + { + yield return new object?[] { EncoderValidatorTestsData.FFmpegV421Output, new Version(4, 2, 1) }; + yield return new object?[] { EncoderValidatorTestsData.FFmpegV42Output, new Version(4, 2) }; + yield return new object?[] { EncoderValidatorTestsData.FFmpegV414Output, new Version(4, 1, 4) }; + yield return new object?[] { EncoderValidatorTestsData.FFmpegV404Output, new Version(4, 0, 4) }; + yield return new object?[] { EncoderValidatorTestsData.FFmpegGitUnknownOutput, new Version(4, 0) }; + } + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } } } From 11f3a0dc58386c815b34e0579cb37c3557e366f4 Mon Sep 17 00:00:00 2001 From: Max Git Date: Mon, 15 Jun 2020 15:10:59 +0200 Subject: [PATCH 4/6] Use Version instead of double. Use correct version number for libavdevice. --- .../Encoder/EncoderValidator.cs | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs b/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs index 801479eede..28471b9561 100644 --- a/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs +++ b/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs @@ -63,16 +63,16 @@ namespace MediaBrowser.MediaEncoding.Encoder }; // These are the library versions that corresponds to our minimum ffmpeg version 4.x according to the version table below - private static readonly IReadOnlyDictionary _ffmpegMinimumLibraryVersions = new Dictionary + private static readonly IReadOnlyDictionary _ffmpegMinimumLibraryVersions = new Dictionary { - { "libavutil", 56.14 }, - { "libavcodec", 58.18 }, - { "libavformat", 58.12 }, - { "libavdevice", 58.3 }, - { "libavfilter", 7.16 }, - { "libswscale", 5.1 }, - { "libswresample", 3.1 }, - { "libpostproc", 55.1 } + { "libavutil", new Version(56, 14) }, + { "libavcodec", new Version(58, 18) }, + { "libavformat", new Version(58, 12) }, + { "libavdevice", new Version(58, 3) }, + { "libavfilter", new Version(7, 16) }, + { "libswscale", new Version(5, 1) }, + { "libswresample", new Version(3, 1) }, + { "libpostproc", new Version(55, 1) } }; // This lookup table is to be maintained with the following command line: @@ -195,7 +195,7 @@ namespace MediaBrowser.MediaEncoding.Encoder } else { - if (!TryGetFFmpegLibraryVersions(output, out string versionString, out IReadOnlyDictionary versionMap)) + if (!TryGetFFmpegLibraryVersions(output, out string versionString, out IReadOnlyDictionary versionMap)) { _logger.LogError("No ffmpeg library versions found"); @@ -213,7 +213,7 @@ namespace MediaBrowser.MediaEncoding.Encoder } } - private Version TestMinimumFFmpegLibraryVersions(IReadOnlyDictionary versionMap) + private Version TestMinimumFFmpegLibraryVersions(IReadOnlyDictionary versionMap) { var allVersionsValidated = true; @@ -248,11 +248,11 @@ namespace MediaBrowser.MediaEncoding.Encoder /// /// /// - private static bool TryGetFFmpegLibraryVersions(string output, out string versionString, out IReadOnlyDictionary versionMap) + private static bool TryGetFFmpegLibraryVersions(string output, out string versionString, out IReadOnlyDictionary versionMap) { var sb = new StringBuilder(144); - var map = new Dictionary(); + var map = new Dictionary(); foreach (Match match in Regex.Matches( output, @@ -267,13 +267,14 @@ namespace MediaBrowser.MediaEncoding.Encoder .Append(','); var str = $"{match.Groups["major"]}.{match.Groups["minor"]}"; - var versionNumber = double.Parse(str, NumberStyles.AllowDecimalPoint, CultureInfo.InvariantCulture); - map.Add(match.Groups["name"].Value, versionNumber); + var version = Version.Parse(str); + + map.Add(match.Groups["name"].Value, version); } versionString = sb.ToString(); - versionMap = map as IReadOnlyDictionary; + versionMap = map as IReadOnlyDictionary; return sb.Length > 0; } From ef3200e178e41cede865e0876a6d56171b1f4cce Mon Sep 17 00:00:00 2001 From: Max Git Date: Mon, 15 Jun 2020 19:50:09 +0200 Subject: [PATCH 5/6] Remove redundant cast --- MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs b/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs index 28471b9561..a055e57ec8 100644 --- a/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs +++ b/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs @@ -274,7 +274,7 @@ namespace MediaBrowser.MediaEncoding.Encoder } versionString = sb.ToString(); - versionMap = map as IReadOnlyDictionary; + versionMap = map; return sb.Length > 0; } From e6c197b96991a619d58e816ee56c23644c4a1de1 Mon Sep 17 00:00:00 2001 From: Max Git Date: Tue, 16 Jun 2020 01:09:41 +0200 Subject: [PATCH 6/6] Cleanup --- MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs b/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs index a055e57ec8..be46255053 100644 --- a/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs +++ b/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.Diagnostics; -using System.Globalization; using System.Linq; using System.Text; using System.Text.RegularExpressions;