Add tests for EncoderValidator

* Add support for ffmpeg 4.2
* Parse the complete ffmpeg version instead of only the first 2 digits
* Make max and min version optional
* Remove max limitation (for now)
* Style improvements

3 files changed, 148 insertions, 174 deletions

diff --git a/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs b/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs
index b00350875..da0b7693e 100644
--- a/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs
+++ b/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs
@@ -1,111 +1,154 @@
 using System;
 using System.Collections.Generic;
-using System.Collections.ObjectModel;
+using System.Diagnostics;
 using System.Linq;
+using System.Text;
 using System.Text.RegularExpressions;
-using MediaBrowser.Model.Diagnostics;
 using Microsoft.Extensions.Logging;
 
 namespace MediaBrowser.MediaEncoding.Encoder
 {
     public class EncoderValidator
     {
-        private readonly ILogger _logger;
-        private readonly IProcessFactory _processFactory;
+        private const string DefaultEncoderPath = "ffmpeg";
 
-        public EncoderValidator(ILogger logger, IProcessFactory processFactory)
+        private static readonly string[] requiredDecoders = new[]
         {
-            _logger = logger;
-            _processFactory = processFactory;
-        }
+            "mpeg2video",
+            "h264_qsv",
+            "hevc_qsv",
+            "mpeg2_qsv",
+            "vc1_qsv",
+            "h264_cuvid",
+            "hevc_cuvid",
+            "dts",
+            "ac3",
+            "aac",
+            "mp3",
+            "h264",
+            "hevc"
+        };
 
-        public (IEnumerable<string> decoders, IEnumerable<string> encoders) GetAvailableCoders(string encoderPath)
+        private static readonly string[] requiredEncoders = new[]
+        {
+            "libx264",
+            "libx265",
+            "mpeg4",
+            "msmpeg4",
+            "libvpx",
+            "libvpx-vp9",
+            "aac",
+            "libmp3lame",
+            "libopus",
+            "libvorbis",
+            "srt",
+            "h264_nvenc",
+            "hevc_nvenc",
+            "h264_qsv",
+            "hevc_qsv",
+            "h264_omx",
+            "hevc_omx",
+            "h264_vaapi",
+            "hevc_vaapi",
+            "h264_v4l2m2m",
+            "ac3"
+        };
+
+        // Try and use the individual library versions to determine a FFmpeg version
+        // 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<string, Version> _ffmpegVersionMap = new Dictionary<string, Version>
         {
-            _logger.LogInformation("Validating media encoder at {EncoderPath}", encoderPath);
+            { "libavutil=56.31,libavcodec=58.54,libavformat=58.29,libavdevice=58.8,libavfilter=7.57,libswscale=5.5,libswresample=3.5,libpostproc=55.5,", new Version(4, 2) },
+            { "libavutil=56.22,libavcodec=58.35,libavformat=58.20,libavdevice=58.5,libavfilter=7.40,libswscale=5.3,libswresample=3.3,libpostproc=55.3,", new Version(4, 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,", new Version(4, 0) },
+            { "libavutil=55.78,libavcodec=57.107,libavformat=57.83,libavdevice=57.10,libavfilter=6.107,libswscale=4.8,libswresample=2.9,libpostproc=54.7,", new Version(3, 4) },
+            { "libavutil=55.58,libavcodec=57.89,libavformat=57.71,libavdevice=57.6,libavfilter=6.82,libswscale=4.6,libswresample=2.7,libpostproc=54.5,", new Version(3, 3) },
+            { "libavutil=55.34,libavcodec=57.64,libavformat=57.56,libavdevice=57.1,libavfilter=6.65,libswscale=4.2,libswresample=2.3,libpostproc=54.1,", new Version(3, 2) },
+            { "libavutil=54.31,libavcodec=56.60,libavformat=56.40,libavdevice=56.4,libavfilter=5.40,libswscale=3.1,libswresample=1.2,libpostproc=53.3,", new Version(2, 8) }
+        };
 
-            var decoders = GetCodecs(encoderPath, Codec.Decoder);
-            var encoders = GetCodecs(encoderPath, Codec.Encoder);
+        private readonly ILogger _logger;
 
-            _logger.LogInformation("Encoder validation complete");
+        private readonly string _encoderPath;
 
-            return (decoders, encoders);
+        public EncoderValidator(ILogger logger, string encoderPath = DefaultEncoderPath)
+        {
+            _logger = logger;
+            _encoderPath = encoderPath;
         }
 
-        public bool ValidateVersion(string encoderAppPath, bool logOutput)
+        public static Version MinVersion { get; } = new Version(4, 0);
+
+        public static Version MaxVersion { get; } = null;
+
+        public bool ValidateVersion()
         {
             string output = null;
             try
             {
-                output = GetProcessOutput(encoderAppPath, "-version");
+                output = GetProcessOutput(_encoderPath, "-version");
             }
             catch (Exception ex)
             {
-                if (logOutput)
-                {
-                    _logger.LogError(ex, "Error validating encoder");
-                }
+                _logger.LogError(ex, "Error validating encoder");
             }
 
             if (string.IsNullOrWhiteSpace(output))
             {
-                if (logOutput)
-                {
-                    _logger.LogError("FFmpeg validation: The process returned no result");
-                }
+                _logger.LogError("FFmpeg validation: The process returned no result");
                 return false;
             }
 
             _logger.LogDebug("ffmpeg output: {Output}", output);
 
-            if (output.IndexOf("Libav developers", StringComparison.OrdinalIgnoreCase) != -1)
+            return ValidateVersionInternal(output);
+        }
+
+        internal bool ValidateVersionInternal(string versionOutput)
+        {
+            if (versionOutput.IndexOf("Libav developers", StringComparison.OrdinalIgnoreCase) != -1)
             {
-                if (logOutput)
-                {
-                    _logger.LogError("FFmpeg validation: avconv instead of ffmpeg is not supported");
-                }
+                _logger.LogError("FFmpeg validation: avconv instead of ffmpeg is not supported");
                 return false;
             }
 
-            // The min and max FFmpeg versions required to run jellyfin successfully
-            var minRequired = new Version(4, 0);
-            var maxRequired = new Version(4, 0);
-
             // Work out what the version under test is
-            var underTest = GetFFmpegVersion(output);
+            var version = GetFFmpegVersion(versionOutput);
 
-            if (logOutput)
-            {
-                _logger.LogInformation("FFmpeg validation: Found ffmpeg version {0}", underTest != null ? underTest.ToString() : "unknown");
+            _logger.LogInformation("Found ffmpeg version {0}", version != null ? version.ToString() : "unknown");
 
-                if (underTest == null) // Version is unknown
-                {
-                    if (minRequired.Equals(maxRequired))
-                    {
-                        _logger.LogWarning("FFmpeg validation: We recommend ffmpeg version {0}", minRequired.ToString());
-                    }
-                    else
-                    {
-                        _logger.LogWarning("FFmpeg validation: We recommend a minimum of {0} and maximum of {1}", minRequired.ToString(), maxRequired.ToString());
-                    }
-                }
-                else if (underTest.CompareTo(minRequired) < 0) // Version is below what we recommend
-                {
-                    _logger.LogWarning("FFmpeg validation: The minimum recommended ffmpeg version is {0}", minRequired.ToString());
-                }
-                else if (underTest.CompareTo(maxRequired) > 0) // Version is above what we recommend
+            if (version == null && MinVersion != null && MaxVersion != null) // Version is unknown
+            {
+                if (MinVersion == MaxVersion)
                 {
-                    _logger.LogWarning("FFmpeg validation: The maximum recommended ffmpeg version is {0}", maxRequired.ToString());
+                    _logger.LogWarning("FFmpeg validation: We recommend ffmpeg version {0}", MinVersion);
                 }
-                else  // Version is ok
+                else
                 {
-                    _logger.LogInformation("FFmpeg validation: Found suitable ffmpeg version");
+                    _logger.LogWarning("FFmpeg validation: We recommend a minimum of {0} and maximum of {1}", MinVersion, MaxVersion);
                 }
+
+                return false;
+            }
+            else if (MinVersion != null && version < MinVersion) // Version is below what we recommend
+            {
+                _logger.LogWarning("FFmpeg validation: The minimum recommended ffmpeg version is {0}", 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);
+                return false;
             }
 
-            // underTest shall be null if versions is unknown
-            return (underTest == null) ? false : (underTest.CompareTo(minRequired) >= 0 && underTest.CompareTo(maxRequired) <= 0);
+            return true;
         }
 
+        public IEnumerable<string> GetDecoders() => GetCodecs(Codec.Decoder);
+
+        public IEnumerable<string> GetEncoders() => GetCodecs(Codec.Encoder);
+
         /// <summary>
         /// 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
@@ -115,10 +158,10 @@ namespace MediaBrowser.MediaEncoding.Encoder
         /// </summary>
         /// <param name="output"></param>
         /// <returns></returns>
-        static private Version GetFFmpegVersion(string output)
+        internal static 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 (\d+\.\d+)");
+            var match = Regex.Match(output, @"ffmpeg version ((?:\d+\.?)+)");
 
             if (match.Success)
             {
@@ -126,25 +169,11 @@ namespace MediaBrowser.MediaEncoding.Encoder
             }
             else
             {
-                // Try and use the individual library versions to determine a FFmpeg version
-                // This lookup table is to be maintained with the following command line:
-                // $ ./ffmpeg.exe -version | perl -ne ' print "$1=$2.$3," if /^(lib\w+)\s+(\d+)\.\s*(\d+)/'
-                var lut = new ReadOnlyDictionary<Version, string>
-                    (new Dictionary<Version, string>
-                    {
-                        { new Version("4.1"), "libavutil=56.22,libavcodec=58.35,libavformat=58.20,libavdevice=58.5,libavfilter=7.40,libswscale=5.3,libswresample=3.3,libpostproc=55.3," },
-                        { new Version("4.0"), "libavutil=56.14,libavcodec=58.18,libavformat=58.12,libavdevice=58.3,libavfilter=7.16,libswscale=5.1,libswresample=3.1,libpostproc=55.1," },
-                        { new Version("3.4"), "libavutil=55.78,libavcodec=57.107,libavformat=57.83,libavdevice=57.10,libavfilter=6.107,libswscale=4.8,libswresample=2.9,libpostproc=54.7," },
-                        { new Version("3.3"), "libavutil=55.58,libavcodec=57.89,libavformat=57.71,libavdevice=57.6,libavfilter=6.82,libswscale=4.6,libswresample=2.7,libpostproc=54.5," },
-                        { new Version("3.2"), "libavutil=55.34,libavcodec=57.64,libavformat=57.56,libavdevice=57.1,libavfilter=6.65,libswscale=4.2,libswresample=2.3,libpostproc=54.1," },
-                        { new Version("2.8"), "libavutil=54.31,libavcodec=56.60,libavformat=56.40,libavdevice=56.4,libavfilter=5.40,libswscale=3.1,libswresample=1.2,libpostproc=53.3," }
-                    });
-
                 // Create a reduced version string and lookup key from dictionary
-                var reducedVersion = GetVersionString(output);
+                var reducedVersion = GetLibrariesVersionString(output);
 
                 // Try to lookup the string and return Key, otherwise if not found returns null
-                return lut.FirstOrDefault(x => x.Value == reducedVersion).Key;
+                return _ffmpegVersionMap.TryGetValue(reducedVersion, out Version version) ? version : null;
             }
         }
 
@@ -154,76 +183,38 @@ namespace MediaBrowser.MediaEncoding.Encoder
         /// </summary>
         /// <param name="output"></param>
         /// <returns></returns>
-        static private string GetVersionString(string output)
+        private static string GetLibrariesVersionString(string output)
         {
-            string pattern = @"((?<name>lib\w+)\s+(?<major>\d+)\.\s*(?<minor>\d+))";
-            RegexOptions options = RegexOptions.Multiline;
-
-            string rc = null;
-
-            foreach (Match m in Regex.Matches(output, pattern, options))
+            var rc = new StringBuilder(144);
+            foreach (Match m in Regex.Matches(
+                output,
+                @"((?<name>lib\w+)\s+(?<major>\d+)\.\s*(?<minor>\d+))",
+                RegexOptions.Multiline))
             {
-                rc += string.Concat(m.Groups["name"], '=', m.Groups["major"], '.', m.Groups["minor"], ',');
+                rc.Append(m.Groups["name"])
+                    .Append('=')
+                    .Append(m.Groups["major"])
+                    .Append('.')
+                    .Append(m.Groups["minor"])
+                    .Append(',');
             }
 
-            return rc;
+            return rc.Length == 0 ? null : rc.ToString();
         }
 
-        private static readonly string[] requiredDecoders = new[]
-            {
-                "mpeg2video",
-                "h264_qsv",
-                "hevc_qsv",
-                "mpeg2_qsv",
-                "vc1_qsv",
-                "h264_cuvid",
-                "hevc_cuvid",
-                "dts",
-                "ac3",
-                "aac",
-                "mp3",
-                "h264",
-                "hevc"
-            };
-
-        private static readonly string[] requiredEncoders = new[]
-            {
-                "libx264",
-                "libx265",
-                "mpeg4",
-                "msmpeg4",
-                "libvpx",
-                "libvpx-vp9",
-                "aac",
-                "libmp3lame",
-                "libopus",
-                "libvorbis",
-                "srt",
-                "h264_nvenc",
-                "hevc_nvenc",
-                "h264_qsv",
-                "hevc_qsv",
-                "h264_omx",
-                "hevc_omx",
-                "h264_vaapi",
-                "hevc_vaapi",
-                "h264_v4l2m2m",
-                "ac3"
-            };
-
         private enum Codec
         {
             Encoder,
             Decoder
         }
 
-        private IEnumerable<string> GetCodecs(string encoderAppPath, Codec codec)
+        private IEnumerable<string> GetCodecs(Codec codec)
         {
             string codecstr = codec == Codec.Encoder ? "encoders" : "decoders";
             string output = null;
             try
             {
-                output = GetProcessOutput(encoderAppPath, "-" + codecstr);
+                output = GetProcessOutput(_encoderPath, "-" + codecstr);
             }
             catch (Exception ex)
             {
@@ -250,45 +241,25 @@ namespace MediaBrowser.MediaEncoding.Encoder
 
         private string GetProcessOutput(string path, string arguments)
         {
-            IProcess process = _processFactory.Create(new ProcessOptions
+            using (var process = new Process()
             {
-                CreateNoWindow = true,
-                UseShellExecute = false,
-                FileName = path,
-                Arguments = arguments,
-                IsHidden = true,
-                ErrorDialog = false,
-                RedirectStandardOutput = true,
-                // ffmpeg uses stderr to log info, don't show this
-                RedirectStandardError = true
-            });
-
-            _logger.LogDebug("Running {Path} {Arguments}", path, arguments);
-
-            using (process)
+                StartInfo = new ProcessStartInfo(path, arguments)
+                {
+                    CreateNoWindow = true,
+                    UseShellExecute = false,
+                    WindowStyle = ProcessWindowStyle.Hidden,
+                    ErrorDialog = false,
+                    RedirectStandardOutput = true,
+                    // ffmpeg uses stderr to log info, don't show this
+                    RedirectStandardError = true
+                }
+            })
             {
+                _logger.LogDebug("Running {Path} {Arguments}", path, arguments);
+
                 process.Start();
 
-                try
-                {
-                    return process.StandardOutput.ReadToEnd();
-                }
-                catch
-                {
-                    _logger.LogWarning("Killing process {Path} {Arguments}", path, arguments);
-
-                    // Hate having to do this
-                    try
-                    {
-                        process.Kill();
-                    }
-                    catch (Exception ex)
-                    {
-                        _logger.LogError(ex, "Error killing process");
-                    }
-
-                    throw;
-                }
+                return process.StandardOutput.ReadToEnd();
             }
         }
     }
diff --git a/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs b/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs
index 75bb960c3..04ff66991 100644
--- a/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs
+++ b/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs
@@ -114,13 +114,13 @@ namespace MediaBrowser.MediaEncoding.Encoder
                 FFprobePath = Regex.Replace(FFmpegPath, @"[^\/\\]+?(\.[^\/\\\n.]+)?$", @"ffprobe$1");
 
                 // Interrogate to understand what coders are supported
-                var result = new EncoderValidator(_logger, _processFactory).GetAvailableCoders(FFmpegPath);
+                var validator = new EncoderValidator(_logger, FFmpegPath);
 
-                SetAvailableDecoders(result.decoders);
-                SetAvailableEncoders(result.encoders);
+                SetAvailableDecoders(validator.GetDecoders());
+                SetAvailableEncoders(validator.GetEncoders());
             }
 
-            _logger.LogInformation("FFmpeg: {0}: {1}", EncoderLocation.ToString(), FFmpegPath ?? string.Empty);
+            _logger.LogInformation("FFmpeg: {0}: {1}", EncoderLocation, FFmpegPath ?? string.Empty);
         }
 
         /// <summary>
@@ -183,11 +183,11 @@ namespace MediaBrowser.MediaEncoding.Encoder
             {
                 if (File.Exists(path))
                 {
-                    rc = new EncoderValidator(_logger, _processFactory).ValidateVersion(path, true);
+                    rc = new EncoderValidator(_logger, path).ValidateVersion();
 
                     if (!rc)
                     {
-                        _logger.LogWarning("FFmpeg: {0}: Failed version check: {1}", location.ToString(), path);
+                        _logger.LogWarning("FFmpeg: {0}: Failed version check: {1}", location, path);
                     }
 
                     // ToDo - Enable the ffmpeg validator.  At the moment any version can be used.
@@ -198,7 +198,7 @@ namespace MediaBrowser.MediaEncoding.Encoder
                 }
                 else
                 {
-                    _logger.LogWarning("FFmpeg: {0}: File not found: {1}", location.ToString(), path);
+                    _logger.LogWarning("FFmpeg: {0}: File not found: {1}", location, path);
                 }
             }
 
@@ -228,9 +228,9 @@ namespace MediaBrowser.MediaEncoding.Encoder
         /// </summary>
         /// <param name="fileName"></param>
         /// <returns></returns>
-        private string ExistsOnSystemPath(string filename)
+        private string ExistsOnSystemPath(string fileName)
         {
-            string inJellyfinPath = GetEncoderPathFromDirectory(System.AppContext.BaseDirectory, filename);
+            string inJellyfinPath = GetEncoderPathFromDirectory(System.AppContext.BaseDirectory, fileName);
             if (!string.IsNullOrEmpty(inJellyfinPath))
             {
                 return inJellyfinPath;
@@ -239,13 +239,14 @@ namespace MediaBrowser.MediaEncoding.Encoder
 
             foreach (var path in values.Split(Path.PathSeparator))
             {
-                var candidatePath = GetEncoderPathFromDirectory(path, filename);
+                var candidatePath = GetEncoderPathFromDirectory(path, fileName);
 
                 if (!string.IsNullOrEmpty(candidatePath))
                 {
                     return candidatePath;
                 }
             }
+
             return null;
         }
 
diff --git a/MediaBrowser.MediaEncoding/Properties/AssemblyInfo.cs b/MediaBrowser.MediaEncoding/Properties/AssemblyInfo.cs
index a9491374b..7b74cfc89 100644
--- a/MediaBrowser.MediaEncoding/Properties/AssemblyInfo.cs
+++ b/MediaBrowser.MediaEncoding/Properties/AssemblyInfo.cs
@@ -1,5 +1,6 @@
 using System.Reflection;
 using System.Resources;
+using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 
 // General Information about an assembly is controlled through the following
@@ -14,6 +15,7 @@ using System.Runtime.InteropServices;
 [assembly: AssemblyTrademark("")]
 [assembly: AssemblyCulture("")]
 [assembly: NeutralResourcesLanguage("en")]
+[assembly: InternalsVisibleTo("Jellyfin.MediaEncoding.Tests")]
 
 // Setting ComVisible to false makes the types in this assembly not visible
 // to COM components.  If you need to access a type in this assembly from