Simplify the way we choose our ffmpeg

* no longer search $PATH
* no longer require a full path
* don't fall back

6 files changed, 38 insertions, 89 deletions

diff --git a/Emby.Server.Implementations/ApplicationHost.cs b/Emby.Server.Implementations/ApplicationHost.cs
index bf7ddace2..0b5322f39 100644
--- a/Emby.Server.Implementations/ApplicationHost.cs
+++ b/Emby.Server.Implementations/ApplicationHost.cs
@@ -1099,7 +1099,6 @@ namespace Emby.Server.Implementations
                 ServerName = FriendlyName,
                 LocalAddress = GetSmartApiUrl(source),
                 SupportsLibraryMonitor = true,
-                EncoderLocation = _mediaEncoder.EncoderLocation,
                 SystemArchitecture = RuntimeInformation.OSArchitecture,
                 PackageName = _startupOptions.PackageName
             };
diff --git a/MediaBrowser.Controller/MediaEncoding/IMediaEncoder.cs b/MediaBrowser.Controller/MediaEncoding/IMediaEncoder.cs
index ff2456070..63308fa09 100644
--- a/MediaBrowser.Controller/MediaEncoding/IMediaEncoder.cs
+++ b/MediaBrowser.Controller/MediaEncoding/IMediaEncoder.cs
@@ -10,7 +10,6 @@ using MediaBrowser.Model.Dlna;
 using MediaBrowser.Model.Dto;
 using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.MediaInfo;
-using MediaBrowser.Model.System;
 
 namespace MediaBrowser.Controller.MediaEncoding
 {
@@ -20,11 +19,6 @@ namespace MediaBrowser.Controller.MediaEncoding
     public interface IMediaEncoder : ITranscoderSupport
     {
         /// <summary>
-        /// Gets location of the discovered FFmpeg tool.
-        /// </summary>
-        FFmpegLocation EncoderLocation { get; }
-
-        /// <summary>
         /// Gets the encoder path.
         /// </summary>
         /// <value>The encoder path.</value>
diff --git a/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs b/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs
index f782e65bd..ef831ab82 100644
--- a/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs
+++ b/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs
@@ -12,8 +12,6 @@ namespace MediaBrowser.MediaEncoding.Encoder
 {
     public class EncoderValidator
     {
-        private const string DefaultEncoderPath = "ffmpeg";
-
         private static readonly string[] _requiredDecoders = new[]
         {
             "h264",
@@ -106,7 +104,7 @@ namespace MediaBrowser.MediaEncoding.Encoder
 
         private readonly string _encoderPath;
 
-        public EncoderValidator(ILogger logger, string encoderPath = DefaultEncoderPath)
+        public EncoderValidator(ILogger logger, string encoderPath)
         {
             _logger = logger;
             _encoderPath = encoderPath;
diff --git a/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs b/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs
index 412a95321..f8ba78e46 100644
--- a/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs
+++ b/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs
@@ -23,7 +23,6 @@ using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.Globalization;
 using MediaBrowser.Model.IO;
 using MediaBrowser.Model.MediaInfo;
-using MediaBrowser.Model.System;
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.Logging;
 
@@ -69,7 +68,7 @@ namespace MediaBrowser.MediaEncoding.Encoder
 
         private string _ffmpegPath = string.Empty;
         private string _ffprobePath;
-        private int threads;
+        private int _threads;
 
         public MediaEncoder(
             ILogger<MediaEncoder> logger,
@@ -89,9 +88,6 @@ namespace MediaBrowser.MediaEncoding.Encoder
         /// <inheritdoc />
         public string EncoderPath => _ffmpegPath;
 
-        /// <inheritdoc />
-        public FFmpegLocation EncoderLocation { get; private set; }
-
         /// <summary>
         /// Run at startup or if the user removes a Custom path from transcode page.
         /// Sets global variables FFmpegPath.
@@ -100,20 +96,23 @@ namespace MediaBrowser.MediaEncoding.Encoder
         public void SetFFmpegPath()
         {
             // 1) Custom path stored in config/encoding xml file under tag <EncoderAppPath> takes precedence
-            if (!ValidatePath(_configurationManager.GetEncodingOptions().EncoderAppPath, FFmpegLocation.Custom))
+            var ffmpegPath = _configurationManager.GetEncodingOptions().EncoderAppPath;
+            if (string.IsNullOrEmpty(ffmpegPath))
             {
                 // 2) Check if the --ffmpeg CLI switch has been given
-                if (!ValidatePath(_startupOptionFFmpegPath, FFmpegLocation.SetByArgument))
+                ffmpegPath = _startupOptionFFmpegPath;
+                if (string.IsNullOrEmpty(ffmpegPath))
                 {
-                    // 3) Search system $PATH environment variable for valid FFmpeg
-                    if (!ValidatePath(ExistsOnSystemPath("ffmpeg"), FFmpegLocation.System))
-                    {
-                        EncoderLocation = FFmpegLocation.NotFound;
-                        _ffmpegPath = null;
-                    }
+                    // 3) Check "ffmpeg"
+                    ffmpegPath = "ffmpeg";
                 }
             }
 
+            if (!ValidatePath(ffmpegPath))
+            {
+                _ffmpegPath = null;
+            }
+
             // Write the FFmpeg path to the config/encoding.xml file as <EncoderAppPathDisplay> so it appears in UI
             var config = _configurationManager.GetEncodingOptions();
             config.EncoderAppPathDisplay = _ffmpegPath ?? string.Empty;
@@ -131,10 +130,10 @@ namespace MediaBrowser.MediaEncoding.Encoder
                 SetAvailableDecoders(validator.GetDecoders());
                 SetAvailableEncoders(validator.GetEncoders());
                 SetAvailableHwaccels(validator.GetHwaccels());
-                threads = EncodingHelper.GetNumberOfThreads(null, _configurationManager.GetEncodingOptions(), null);
+                _threads = EncodingHelper.GetNumberOfThreads(null, _configurationManager.GetEncodingOptions(), null);
             }
 
-            _logger.LogInformation("FFmpeg: {EncoderLocation}: {FfmpegPath}", EncoderLocation, _ffmpegPath ?? string.Empty);
+            _logger.LogInformation("FFmpeg: {FfmpegPath}", _ffmpegPath ?? string.Empty);
         }
 
         /// <summary>
@@ -153,15 +152,12 @@ namespace MediaBrowser.MediaEncoding.Encoder
             {
                 throw new ArgumentException("Unexpected pathType value");
             }
-            else if (string.IsNullOrWhiteSpace(path))
+
+            if (string.IsNullOrWhiteSpace(path))
             {
                 // User had cleared the custom path in UI
                 newPath = string.Empty;
             }
-            else if (File.Exists(path))
-            {
-                newPath = path;
-            }
             else if (Directory.Exists(path))
             {
                 // Given path is directory, so resolve down to filename
@@ -169,7 +165,7 @@ namespace MediaBrowser.MediaEncoding.Encoder
             }
             else
             {
-                throw new ResourceNotFoundException();
+                newPath = path;
             }
 
             // Write the new ffmpeg path to the xml as <EncoderAppPath>
@@ -184,37 +180,26 @@ namespace MediaBrowser.MediaEncoding.Encoder
 
         /// <summary>
         /// Validates the supplied FQPN to ensure it is a ffmpeg utility.
-        /// If checks pass, global variable FFmpegPath and EncoderLocation are updated.
+        /// If checks pass, global variable FFmpegPath is updated.
         /// </summary>
         /// <param name="path">FQPN to test.</param>
-        /// <param name="location">Location (External, Custom, System) of tool.</param>
         /// <returns><c>true</c> if the version validation succeeded; otherwise, <c>false</c>.</returns>
-        private bool ValidatePath(string path, FFmpegLocation location)
+        private bool ValidatePath(string path)
         {
-            bool rc = false;
-
-            if (!string.IsNullOrEmpty(path))
+            if (string.IsNullOrEmpty(path))
             {
-                if (File.Exists(path))
-                {
-                    rc = new EncoderValidator(_logger, path).ValidateVersion();
-
-                    if (!rc)
-                    {
-                        _logger.LogWarning("FFmpeg: {Location}: Failed version check: {Path}", location, path);
-                    }
+                return false;
+            }
 
-                    _ffmpegPath = path;
-                    EncoderLocation = location;
-                    return true;
-                }
-                else
-                {
-                    _logger.LogWarning("FFmpeg: {Location}: File not found: {Path}", location, path);
-                }
+            bool rc = new EncoderValidator(_logger, path).ValidateVersion();
+            if (!rc)
+            {
+                _logger.LogWarning("FFmpeg: Failed version check: {Path}", path);
+                return false;
             }
 
-            return rc;
+            _ffmpegPath = path;
+            return true;
         }
 
         private string GetEncoderPathFromDirectory(string path, string filename, bool recursive = false)
@@ -235,34 +220,6 @@ namespace MediaBrowser.MediaEncoding.Encoder
             }
         }
 
-        /// <summary>
-        /// Search the system $PATH environment variable looking for given filename.
-        /// </summary>
-        /// <param name="fileName">The filename.</param>
-        /// <returns>The full path to the file.</returns>
-        private string ExistsOnSystemPath(string fileName)
-        {
-            var inJellyfinPath = GetEncoderPathFromDirectory(AppContext.BaseDirectory, fileName, recursive: true);
-            if (!string.IsNullOrEmpty(inJellyfinPath))
-            {
-                return inJellyfinPath;
-            }
-
-            var values = Environment.GetEnvironmentVariable("PATH");
-
-            foreach (var path in values.Split(Path.PathSeparator))
-            {
-                var candidatePath = GetEncoderPathFromDirectory(path, fileName);
-
-                if (!string.IsNullOrEmpty(candidatePath))
-                {
-                    return candidatePath;
-                }
-            }
-
-            return null;
-        }
-
         public void SetAvailableEncoders(IEnumerable<string> list)
         {
             _encoders = list.ToList();
@@ -394,7 +351,7 @@ namespace MediaBrowser.MediaEncoding.Encoder
             var args = extractChapters
                 ? "{0} -i {1} -threads {2} -v warning -print_format json -show_streams -show_chapters -show_format"
                 : "{0} -i {1} -threads {2} -v warning -print_format json -show_streams -show_format";
-            args = string.Format(CultureInfo.InvariantCulture, args, probeSizeArgument, inputPath, threads).Trim();
+            args = string.Format(CultureInfo.InvariantCulture, args, probeSizeArgument, inputPath, _threads).Trim();
 
             var process = new Process
             {
@@ -615,7 +572,7 @@ namespace MediaBrowser.MediaEncoding.Encoder
                 }
             }
 
-            var args = string.Format(CultureInfo.InvariantCulture, "-i {0}{3} -threads {4} -v quiet -vframes 1 {2} -f image2 \"{1}\"", inputPath, tempExtractPath, vf, mapArg, threads);
+            var args = string.Format(CultureInfo.InvariantCulture, "-i {0}{3} -threads {4} -v quiet -vframes 1 {2} -f image2 \"{1}\"", inputPath, tempExtractPath, vf, mapArg, _threads);
 
             if (offset.HasValue)
             {
@@ -728,7 +685,7 @@ namespace MediaBrowser.MediaEncoding.Encoder
             Directory.CreateDirectory(targetDirectory);
             var outputPath = Path.Combine(targetDirectory, filenamePrefix + "%05d.jpg");
 
-            var args = string.Format(CultureInfo.InvariantCulture, "-i {0} -threads {3} -v quiet {2} -f image2 \"{1}\"", inputArgument, outputPath, vf, threads);
+            var args = string.Format(CultureInfo.InvariantCulture, "-i {0} -threads {3} -v quiet {2} -f image2 \"{1}\"", inputArgument, outputPath, vf, _threads);
 
             if (!string.IsNullOrWhiteSpace(container))
             {
diff --git a/MediaBrowser.Model/System/SystemInfo.cs b/MediaBrowser.Model/System/SystemInfo.cs
index e45b2f33a..a82c1c8c0 100644
--- a/MediaBrowser.Model/System/SystemInfo.cs
+++ b/MediaBrowser.Model/System/SystemInfo.cs
@@ -133,6 +133,7 @@ namespace MediaBrowser.Model.System
         [Obsolete("This should be handled by the package manager")]
         public bool HasUpdateAvailable { get; set; }
 
+        [Obsolete("This isn't set correctly anymore")]
         public FFmpegLocation EncoderLocation { get; set; }
 
         public Architecture SystemArchitecture { get; set; }
diff --git a/tests/Jellyfin.MediaEncoding.Tests/EncoderValidatorTests.cs b/tests/Jellyfin.MediaEncoding.Tests/EncoderValidatorTests.cs
index 39fd8afda..cc429b442 100644
--- a/tests/Jellyfin.MediaEncoding.Tests/EncoderValidatorTests.cs
+++ b/tests/Jellyfin.MediaEncoding.Tests/EncoderValidatorTests.cs
@@ -9,12 +9,13 @@ namespace Jellyfin.MediaEncoding.Tests
 {
     public class EncoderValidatorTests
     {
+        private readonly EncoderValidator _encoderValidator = new EncoderValidator(new NullLogger<EncoderValidatorTests>(), "ffmpeg");
+
         [Theory]
         [ClassData(typeof(GetFFmpegVersionTestData))]
         public void GetFFmpegVersionTest(string versionOutput, Version? version)
         {
-            var val = new EncoderValidator(new NullLogger<EncoderValidatorTests>());
-            Assert.Equal(version, val.GetFFmpegVersion(versionOutput));
+            Assert.Equal(version, _encoderValidator.GetFFmpegVersion(versionOutput));
         }
 
         [Theory]
@@ -28,8 +29,7 @@ namespace Jellyfin.MediaEncoding.Tests
         [InlineData(EncoderValidatorTestsData.FFmpegGitUnknownOutput, false)]
         public void ValidateVersionInternalTest(string versionOutput, bool valid)
         {
-            var val = new EncoderValidator(new NullLogger<EncoderValidatorTests>());
-            Assert.Equal(valid, val.ValidateVersionInternal(versionOutput));
+            Assert.Equal(valid, _encoderValidator.ValidateVersionInternal(versionOutput));
         }
 
         private class GetFFmpegVersionTestData : IEnumerable<object?[]>