From 20775116f76b12bf77672fa37c4ea5f82b69f157 Mon Sep 17 00:00:00 2001 From: PloughPuff Date: Fri, 8 Feb 2019 13:35:26 +0000 Subject: [PATCH 1/6] Reworked FFmpeg path discovery and always display to user 1) Reworked FFmpeg and FFprobe path discovery (CLI switch, Custom xml, system $PATH, UI update trigger). Removed FFMpeg folder from Emby.Server.Implementations. All path discovery now in MediaEncoder. 2) Always display FFmpeg path to user in Transcode page. 3) Allow user to remove a Custome FFmpeg path and return to using system $PATH (or --ffmpeg if available). 4) Remove unused code associated with 'prebuilt' FFmpeg. 5) Much improved logging during path discovery. --- .../ApplicationHost.cs | 72 +-- .../FFMpeg/FFMpegInfo.cs | 24 - .../FFMpeg/FFMpegInstallInfo.cs | 17 - .../FFMpeg/FFMpegLoader.cs | 132 ------ .../Encoder/EncoderValidator.cs | 2 +- .../Encoder/MediaEncoder.cs | 433 ++++++++---------- .../Configuration/EncodingOptions.cs | 3 +- 7 files changed, 212 insertions(+), 471 deletions(-) delete mode 100644 Emby.Server.Implementations/FFMpeg/FFMpegInfo.cs delete mode 100644 Emby.Server.Implementations/FFMpeg/FFMpegInstallInfo.cs delete mode 100644 Emby.Server.Implementations/FFMpeg/FFMpegLoader.cs diff --git a/Emby.Server.Implementations/ApplicationHost.cs b/Emby.Server.Implementations/ApplicationHost.cs index 94d2cd5daf..2c0d0e7469 100644 --- a/Emby.Server.Implementations/ApplicationHost.cs +++ b/Emby.Server.Implementations/ApplicationHost.cs @@ -28,7 +28,6 @@ using Emby.Server.Implementations.Data; using Emby.Server.Implementations.Devices; using Emby.Server.Implementations.Diagnostics; using Emby.Server.Implementations.Dto; -using Emby.Server.Implementations.FFMpeg; using Emby.Server.Implementations.HttpServer; using Emby.Server.Implementations.HttpServer.Security; using Emby.Server.Implementations.IO; @@ -792,7 +791,8 @@ namespace Emby.Server.Implementations ChapterManager = new ChapterManager(LibraryManager, LoggerFactory, ServerConfigurationManager, ItemRepository); serviceCollection.AddSingleton(ChapterManager); - RegisterMediaEncoder(serviceCollection); + MediaEncoder = new MediaBrowser.MediaEncoding.Encoder.MediaEncoder(LoggerFactory, JsonSerializer, StartupOptions.FFmpegPath, StartupOptions.FFprobePath, ServerConfigurationManager, FileSystemManager, () => SubtitleEncoder, () => MediaSourceManager, ProcessFactory, 5000); + serviceCollection.AddSingleton(MediaEncoder); EncodingManager = new MediaEncoder.EncodingManager(FileSystemManager, LoggerFactory, MediaEncoder, ChapterManager, LibraryManager); serviceCollection.AddSingleton(EncodingManager); @@ -908,83 +908,25 @@ namespace Emby.Server.Implementations return new ImageProcessor(LoggerFactory, ServerConfigurationManager.ApplicationPaths, FileSystemManager, ImageEncoder, () => LibraryManager, () => MediaEncoder); } - protected virtual FFMpegInstallInfo GetFfmpegInstallInfo() - { - var info = new FFMpegInstallInfo(); - - // Windows builds: http://ffmpeg.zeranoe.com/builds/ - // Linux builds: http://johnvansickle.com/ffmpeg/ - // OS X builds: http://ffmpegmac.net/ - // OS X x64: http://www.evermeet.cx/ffmpeg/ - - if (EnvironmentInfo.OperatingSystem == MediaBrowser.Model.System.OperatingSystem.Linux) - { - info.FFMpegFilename = "ffmpeg"; - info.FFProbeFilename = "ffprobe"; - info.ArchiveType = "7z"; - info.Version = "20170308"; - } - else if (EnvironmentInfo.OperatingSystem == MediaBrowser.Model.System.OperatingSystem.Windows) - { - info.FFMpegFilename = "ffmpeg.exe"; - info.FFProbeFilename = "ffprobe.exe"; - info.Version = "20170308"; - info.ArchiveType = "7z"; - } - else if (EnvironmentInfo.OperatingSystem == MediaBrowser.Model.System.OperatingSystem.OSX) - { - info.FFMpegFilename = "ffmpeg"; - info.FFProbeFilename = "ffprobe"; - info.ArchiveType = "7z"; - info.Version = "20170308"; - } - - return info; - } - - protected virtual FFMpegInfo GetFFMpegInfo() - { - return new FFMpegLoader(ApplicationPaths, FileSystemManager, GetFfmpegInstallInfo()) - .GetFFMpegInfo(StartupOptions); - } - /// /// Registers the media encoder. /// /// Task. - private void RegisterMediaEncoder(IServiceCollection serviceCollection) + private void RegisterMediaEncoder(IAssemblyInfo assemblyInfo) { - string encoderPath = null; - string probePath = null; - - var info = GetFFMpegInfo(); - - encoderPath = info.EncoderPath; - probePath = info.ProbePath; - var hasExternalEncoder = string.Equals(info.Version, "external", StringComparison.OrdinalIgnoreCase); - - var mediaEncoder = new MediaBrowser.MediaEncoding.Encoder.MediaEncoder( + MediaEncoder = new MediaBrowser.MediaEncoding.Encoder.MediaEncoder( LoggerFactory, JsonSerializer, - encoderPath, - probePath, - hasExternalEncoder, + StartupOptions.FFmpegPath, + StartupOptions.FFprobePath, ServerConfigurationManager, FileSystemManager, - LiveTvManager, - IsoManager, - LibraryManager, - ChannelManager, - SessionManager, () => SubtitleEncoder, () => MediaSourceManager, - HttpClient, - ZipClient, ProcessFactory, 5000); - MediaEncoder = mediaEncoder; - serviceCollection.AddSingleton(MediaEncoder); + RegisterSingleInstance(MediaEncoder); } /// diff --git a/Emby.Server.Implementations/FFMpeg/FFMpegInfo.cs b/Emby.Server.Implementations/FFMpeg/FFMpegInfo.cs deleted file mode 100644 index 60cd7b3d72..0000000000 --- a/Emby.Server.Implementations/FFMpeg/FFMpegInfo.cs +++ /dev/null @@ -1,24 +0,0 @@ -namespace Emby.Server.Implementations.FFMpeg -{ - /// - /// Class FFMpegInfo - /// - public class FFMpegInfo - { - /// - /// Gets or sets the path. - /// - /// The path. - public string EncoderPath { get; set; } - /// - /// Gets or sets the probe path. - /// - /// The probe path. - public string ProbePath { get; set; } - /// - /// Gets or sets the version. - /// - /// The version. - public string Version { get; set; } - } -} diff --git a/Emby.Server.Implementations/FFMpeg/FFMpegInstallInfo.cs b/Emby.Server.Implementations/FFMpeg/FFMpegInstallInfo.cs deleted file mode 100644 index fa9cb5e01b..0000000000 --- a/Emby.Server.Implementations/FFMpeg/FFMpegInstallInfo.cs +++ /dev/null @@ -1,17 +0,0 @@ -namespace Emby.Server.Implementations.FFMpeg -{ - public class FFMpegInstallInfo - { - public string Version { get; set; } - public string FFMpegFilename { get; set; } - public string FFProbeFilename { get; set; } - public string ArchiveType { get; set; } - - public FFMpegInstallInfo() - { - Version = "Path"; - FFMpegFilename = "ffmpeg"; - FFProbeFilename = "ffprobe"; - } - } -} diff --git a/Emby.Server.Implementations/FFMpeg/FFMpegLoader.cs b/Emby.Server.Implementations/FFMpeg/FFMpegLoader.cs deleted file mode 100644 index bbf51dd246..0000000000 --- a/Emby.Server.Implementations/FFMpeg/FFMpegLoader.cs +++ /dev/null @@ -1,132 +0,0 @@ -using System; -using System.Collections.Generic; -using System.IO; -using System.Linq; -using MediaBrowser.Common.Configuration; -using MediaBrowser.Model.IO; - -namespace Emby.Server.Implementations.FFMpeg -{ - public class FFMpegLoader - { - private readonly IApplicationPaths _appPaths; - private readonly IFileSystem _fileSystem; - private readonly FFMpegInstallInfo _ffmpegInstallInfo; - - public FFMpegLoader(IApplicationPaths appPaths, IFileSystem fileSystem, FFMpegInstallInfo ffmpegInstallInfo) - { - _appPaths = appPaths; - _fileSystem = fileSystem; - _ffmpegInstallInfo = ffmpegInstallInfo; - } - - public FFMpegInfo GetFFMpegInfo(IStartupOptions options) - { - var customffMpegPath = options.FFmpegPath; - var customffProbePath = options.FFprobePath; - - if (!string.IsNullOrWhiteSpace(customffMpegPath) && !string.IsNullOrWhiteSpace(customffProbePath)) - { - return new FFMpegInfo - { - ProbePath = customffProbePath, - EncoderPath = customffMpegPath, - Version = "external" - }; - } - - var downloadInfo = _ffmpegInstallInfo; - - var prebuiltFolder = _appPaths.ProgramSystemPath; - var prebuiltffmpeg = Path.Combine(prebuiltFolder, downloadInfo.FFMpegFilename); - var prebuiltffprobe = Path.Combine(prebuiltFolder, downloadInfo.FFProbeFilename); - if (File.Exists(prebuiltffmpeg) && File.Exists(prebuiltffprobe)) - { - return new FFMpegInfo - { - ProbePath = prebuiltffprobe, - EncoderPath = prebuiltffmpeg, - Version = "external" - }; - } - - var version = downloadInfo.Version; - - if (string.Equals(version, "0", StringComparison.OrdinalIgnoreCase)) - { - return new FFMpegInfo(); - } - - var rootEncoderPath = Path.Combine(_appPaths.ProgramDataPath, "ffmpeg"); - var versionedDirectoryPath = Path.Combine(rootEncoderPath, version); - - var info = new FFMpegInfo - { - ProbePath = Path.Combine(versionedDirectoryPath, downloadInfo.FFProbeFilename), - EncoderPath = Path.Combine(versionedDirectoryPath, downloadInfo.FFMpegFilename), - Version = version - }; - - Directory.CreateDirectory(versionedDirectoryPath); - - var excludeFromDeletions = new List { versionedDirectoryPath }; - - if (!File.Exists(info.ProbePath) || !File.Exists(info.EncoderPath)) - { - // ffmpeg not present. See if there's an older version we can start with - var existingVersion = GetExistingVersion(info, rootEncoderPath); - - // No older version. Need to download and block until complete - if (existingVersion == null) - { - return new FFMpegInfo(); - } - else - { - info = existingVersion; - versionedDirectoryPath = Path.GetDirectoryName(info.EncoderPath); - excludeFromDeletions.Add(versionedDirectoryPath); - } - } - - // Allow just one of these to be overridden, if desired. - if (!string.IsNullOrWhiteSpace(customffMpegPath)) - { - info.EncoderPath = customffMpegPath; - } - if (!string.IsNullOrWhiteSpace(customffProbePath)) - { - info.ProbePath = customffProbePath; - } - - return info; - } - - private FFMpegInfo GetExistingVersion(FFMpegInfo info, string rootEncoderPath) - { - var encoderFilename = Path.GetFileName(info.EncoderPath); - var probeFilename = Path.GetFileName(info.ProbePath); - - foreach (var directory in _fileSystem.GetDirectoryPaths(rootEncoderPath)) - { - var allFiles = _fileSystem.GetFilePaths(directory, true).ToList(); - - var encoder = allFiles.FirstOrDefault(i => string.Equals(Path.GetFileName(i), encoderFilename, StringComparison.OrdinalIgnoreCase)); - var probe = allFiles.FirstOrDefault(i => string.Equals(Path.GetFileName(i), probeFilename, StringComparison.OrdinalIgnoreCase)); - - if (!string.IsNullOrWhiteSpace(encoder) && - !string.IsNullOrWhiteSpace(probe)) - { - return new FFMpegInfo - { - EncoderPath = encoder, - ProbePath = probe, - Version = Path.GetFileName(Path.GetDirectoryName(probe)) - }; - } - } - - return null; - } - } -} diff --git a/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs b/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs index f725d2c019..1eeea87a06 100644 --- a/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs +++ b/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs @@ -19,7 +19,7 @@ namespace MediaBrowser.MediaEncoding.Encoder _processFactory = processFactory; } - public (IEnumerable decoders, IEnumerable encoders) Validate(string encoderPath) + public (IEnumerable decoders, IEnumerable encoders) GetAvailableCoders(string encoderPath) { _logger.LogInformation("Validating media encoder at {EncoderPath}", encoderPath); diff --git a/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs b/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs index 7f29c06b4c..36d72cad90 100644 --- a/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs +++ b/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs @@ -7,13 +7,9 @@ using System.Threading; using System.Threading.Tasks; using MediaBrowser.Common.Configuration; using MediaBrowser.Common.Extensions; -using MediaBrowser.Common.Net; -using MediaBrowser.Controller.Channels; using MediaBrowser.Controller.Configuration; using MediaBrowser.Controller.Library; -using MediaBrowser.Controller.LiveTv; using MediaBrowser.Controller.MediaEncoding; -using MediaBrowser.Controller.Session; using MediaBrowser.MediaEncoding.Probing; using MediaBrowser.Model.Configuration; using MediaBrowser.Model.Diagnostics; @@ -32,323 +28,288 @@ namespace MediaBrowser.MediaEncoding.Encoder public class MediaEncoder : IMediaEncoder, IDisposable { /// - /// The _logger - /// - private readonly ILogger _logger; - - /// - /// Gets the json serializer. + /// Gets the encoder path. /// - /// The json serializer. - private readonly IJsonSerializer _jsonSerializer; + /// The encoder path. + public string EncoderPath => FFmpegPath; /// - /// The _thumbnail resource pool + /// External: path supplied via command line + /// Custom: coming from UI or config/encoding.xml file + /// System: FFmpeg found in system $PATH + /// null: No FFmpeg found /// - private readonly SemaphoreSlim _thumbnailResourcePool = new SemaphoreSlim(1, 1); - - public string FFMpegPath { get; private set; } - - public string FFProbePath { get; private set; } + public string EncoderLocationType { get; private set; } + private readonly ILogger _logger; + private readonly IJsonSerializer _jsonSerializer; + private string FFmpegPath { get; set; } + private string FFprobePath { get; set; } protected readonly IServerConfigurationManager ConfigurationManager; protected readonly IFileSystem FileSystem; - protected readonly ILiveTvManager LiveTvManager; - protected readonly IIsoManager IsoManager; - protected readonly ILibraryManager LibraryManager; - protected readonly IChannelManager ChannelManager; - protected readonly ISessionManager SessionManager; protected readonly Func SubtitleEncoder; protected readonly Func MediaSourceManager; - private readonly IHttpClient _httpClient; - private readonly IZipClient _zipClient; private readonly IProcessFactory _processFactory; + private readonly int DefaultImageExtractionTimeoutMs; + private readonly string StartupOptionFFmpegPath; + private readonly string StartupOptionFFprobePath; + private readonly SemaphoreSlim _thumbnailResourcePool = new SemaphoreSlim(1, 1); private readonly List _runningProcesses = new List(); - private readonly bool _hasExternalEncoder; - private readonly string _originalFFMpegPath; - private readonly string _originalFFProbePath; - private readonly int DefaultImageExtractionTimeoutMs; public MediaEncoder( ILoggerFactory loggerFactory, IJsonSerializer jsonSerializer, - string ffMpegPath, - string ffProbePath, - bool hasExternalEncoder, + string startupOptionsFFmpegPath, + string startupOptionsFFprobePath, IServerConfigurationManager configurationManager, IFileSystem fileSystem, - ILiveTvManager liveTvManager, - IIsoManager isoManager, - ILibraryManager libraryManager, - IChannelManager channelManager, - ISessionManager sessionManager, Func subtitleEncoder, Func mediaSourceManager, - IHttpClient httpClient, - IZipClient zipClient, IProcessFactory processFactory, int defaultImageExtractionTimeoutMs) { _logger = loggerFactory.CreateLogger(nameof(MediaEncoder)); _jsonSerializer = jsonSerializer; + StartupOptionFFmpegPath = startupOptionsFFmpegPath; + StartupOptionFFprobePath = startupOptionsFFprobePath; ConfigurationManager = configurationManager; FileSystem = fileSystem; - LiveTvManager = liveTvManager; - IsoManager = isoManager; - LibraryManager = libraryManager; - ChannelManager = channelManager; - SessionManager = sessionManager; SubtitleEncoder = subtitleEncoder; - MediaSourceManager = mediaSourceManager; - _httpClient = httpClient; - _zipClient = zipClient; _processFactory = processFactory; DefaultImageExtractionTimeoutMs = defaultImageExtractionTimeoutMs; - FFProbePath = ffProbePath; - FFMpegPath = ffMpegPath; - _originalFFProbePath = ffProbePath; - _originalFFMpegPath = ffMpegPath; - _hasExternalEncoder = hasExternalEncoder; } - public string EncoderLocationType + /// + /// Run at startup or if the user removes a Custom path from transcode page. + /// Sets global variables FFmpegPath and EncoderLocationType. + /// If startup options --ffprobe is given then FFprobePath is set too. + /// + public void Init() { - get + // 1) If given, use the --ffmpeg CLI switch + if (ValidatePathFFmpeg("From CLI Switch", StartupOptionFFmpegPath)) { - if (_hasExternalEncoder) - { - return "External"; - } - - if (string.IsNullOrWhiteSpace(FFMpegPath)) - { - return null; - } - - if (IsSystemInstalledPath(FFMpegPath)) - { - return "System"; - } - - return "Custom"; + _logger.LogInformation("FFmpeg: Using path from command line switch --ffmpeg"); + EncoderLocationType = "External"; } - } - private bool IsSystemInstalledPath(string path) - { - if (path.IndexOf("/", StringComparison.Ordinal) == -1 && path.IndexOf("\\", StringComparison.Ordinal) == -1) + // 2) Try Custom path stroed in config/encoding xml file under tag + else if (ValidatePathFFmpeg("From Config File", ConfigurationManager.GetConfiguration("encoding").EncoderAppPathCustom)) { - return true; + _logger.LogInformation("FFmpeg: Using path from config/encoding.xml file"); + EncoderLocationType = "Custom"; } - return false; - } - - public void Init() - { - InitPaths(); - - if (!string.IsNullOrWhiteSpace(FFMpegPath)) + // 3) Search system $PATH environment variable for valid FFmpeg + else if (ValidatePathFFmpeg("From $PATH", ExistsOnSystemPath("ffmpeg"))) { - var result = new EncoderValidator(_logger, _processFactory).Validate(FFMpegPath); - - SetAvailableDecoders(result.decoders); - SetAvailableEncoders(result.encoders); + _logger.LogInformation("FFmpeg: Using system $PATH for FFmpeg"); + EncoderLocationType = "System"; + } + else + { + _logger.LogError("FFmpeg: No suitable executable found"); + FFmpegPath = null; + EncoderLocationType = null; } - } - - private void InitPaths() - { - ConfigureEncoderPaths(); - if (_hasExternalEncoder) + // If given, use the --ffprobe CLI switch + if (ValidatePathFFprobe("CLI Switch", StartupOptionFFprobePath)) { - LogPaths(); - return; + _logger.LogInformation("FFprobe: Using path from command line switch --ffprobe"); + } + else + { + // FFprobe path from command line is no good, so set to null and let ReInit() try + // and set using the FFmpeg path. + FFprobePath = null; } - // If the path was passed in, save it into config now. - var encodingOptions = GetEncodingOptions(); - var appPath = encodingOptions.EncoderAppPath; + ReInit(); + } - var valueToSave = FFMpegPath; + /// + /// Writes the currently used FFmpeg to config/encoding.xml file. + /// Sets the FFprobe path if not currently set. + /// Interrogates the FFmpeg tool to identify what encoders/decodres are available. + /// + private void ReInit() + { + // Write the FFmpeg path to the config/encoding.xml file so it appears in UI + var config = ConfigurationManager.GetConfiguration("encoding"); + config.EncoderAppPath = FFmpegPath ?? string.Empty; + ConfigurationManager.SaveConfiguration("encoding", config); - if (!string.IsNullOrWhiteSpace(valueToSave)) + // Only if mpeg path is set, try and set path to probe + if (FFmpegPath != null) { - // if using system variable, don't save this. - if (IsSystemInstalledPath(valueToSave) || _hasExternalEncoder) + // Probe would be null here if no valid --ffprobe path was given + // at startup, or we're performing ReInit following mpeg path update from UI + if (FFprobePath == null) { - valueToSave = null; + // Use the mpeg path to create a probe path + if (ValidatePathFFprobe("Copied from FFmpeg:", GetProbePathFromEncoderPath(FFmpegPath))) + { + _logger.LogInformation("FFprobe: Using FFprobe in same folders as FFmpeg"); + } + else + { + _logger.LogError("FFprobe: No suitable executable found"); + } } - } - if (!string.Equals(valueToSave, appPath, StringComparison.Ordinal)) - { - encodingOptions.EncoderAppPath = valueToSave; - ConfigurationManager.SaveConfiguration("encoding", encodingOptions); + // Interrogate to understand what coders it supports + var result = new EncoderValidator(_logger, _processFactory).GetAvailableCoders(FFmpegPath); + + SetAvailableDecoders(result.decoders); + SetAvailableEncoders(result.encoders); } + + // Stamp FFmpeg paths to the log file + LogPaths(); } + /// + /// Triggered from the Settings > Trascoding UI page when users sumits Custom FFmpeg path to use. + /// + /// + /// public void UpdateEncoderPath(string path, string pathType) { - if (_hasExternalEncoder) - { - return; - } - _logger.LogInformation("Attempting to update encoder path to {0}. pathType: {1}", path ?? string.Empty, pathType ?? string.Empty); - Tuple newPaths; - - if (string.Equals(pathType, "system", StringComparison.OrdinalIgnoreCase)) + if (!string.Equals(pathType, "custom", StringComparison.OrdinalIgnoreCase)) { - path = "ffmpeg"; - - newPaths = TestForInstalledVersions(); + throw new ArgumentException("Unexpected pathType value"); } - else if (string.Equals(pathType, "custom", StringComparison.OrdinalIgnoreCase)) + else { if (string.IsNullOrWhiteSpace(path)) { - throw new ArgumentNullException(nameof(path)); - } + // User had cleared the cutom path in UI. Clear the Custom config + // setting and peform full Init to relook any CLI switches and system $PATH + var config = ConfigurationManager.GetConfiguration("encoding"); + config.EncoderAppPathCustom = string.Empty; + ConfigurationManager.SaveConfiguration("encoding", config); - if (!File.Exists(path) && !Directory.Exists(path)) + Init(); + } + else if (!File.Exists(path) && !Directory.Exists(path)) { + // Given path is neither file or folder throw new ResourceNotFoundException(); } - newPaths = GetEncoderPaths(path); - } - else - { - throw new ArgumentException("Unexpected pathType value"); - } - - if (string.IsNullOrWhiteSpace(newPaths.Item1)) - { - throw new ResourceNotFoundException("ffmpeg not found"); - } - if (string.IsNullOrWhiteSpace(newPaths.Item2)) - { - throw new ResourceNotFoundException("ffprobe not found"); - } - - path = newPaths.Item1; + else + { + // Supplied path could be either file path or folder path. + // Resolve down to file path and validate + path = GetEncoderPath(path); - if (!ValidateVersion(path, true)) - { - throw new ResourceNotFoundException("ffmpeg version 3.0 or greater is required."); - } + if (path == null) + { + throw new ResourceNotFoundException("FFmpeg not found"); + } + else if (!ValidatePathFFmpeg("New From UI", path)) + { + throw new ResourceNotFoundException("Failed validation checks. Version 4.0 or greater is required"); + } + else + { + EncoderLocationType = "Custom"; - var config = GetEncodingOptions(); - config.EncoderAppPath = path; - ConfigurationManager.SaveConfiguration("encoding", config); + // Write the validated mpeg path to the xml as + // This ensures its not lost on new startup + var config = ConfigurationManager.GetConfiguration("encoding"); + config.EncoderAppPathCustom = FFmpegPath; + ConfigurationManager.SaveConfiguration("encoding", config); - Init(); - } + FFprobePath = null; // Clear probe path so it gets relooked in ReInit() - private bool ValidateVersion(string path, bool logOutput) - { - return new EncoderValidator(_logger, _processFactory).ValidateVersion(path, logOutput); + ReInit(); + } + } + } } - private void ConfigureEncoderPaths() + private bool ValidatePath(string type, string path) { - if (_hasExternalEncoder) + if (!string.IsNullOrEmpty(path)) { - return; - } - - var appPath = GetEncodingOptions().EncoderAppPath; + if (File.Exists(path)) + { + var valid = new EncoderValidator(_logger, _processFactory).ValidateVersion(path, true); - if (string.IsNullOrWhiteSpace(appPath)) - { - appPath = Path.Combine(ConfigurationManager.ApplicationPaths.ProgramDataPath, "ffmpeg"); + if (valid == true) + { + return true; + } + else + { + _logger.LogError("{0}: Failed validation checks. Version 4.0 or greater is required: {1}", type, path); + } + } + else + { + _logger.LogError("{0}: File not found: {1}", type, path); + } } - var newPaths = GetEncoderPaths(appPath); - if (string.IsNullOrWhiteSpace(newPaths.Item1) || string.IsNullOrWhiteSpace(newPaths.Item2) || IsSystemInstalledPath(appPath)) - { - newPaths = TestForInstalledVersions(); - } + return false; + } - if (!string.IsNullOrWhiteSpace(newPaths.Item1) && !string.IsNullOrWhiteSpace(newPaths.Item2)) + private bool ValidatePathFFmpeg(string comment, string path) + { + if (ValidatePath("FFmpeg: " + comment, path) == true) { - FFMpegPath = newPaths.Item1; - FFProbePath = newPaths.Item2; + FFmpegPath = path; + return true; } - LogPaths(); + return false; } - private Tuple GetEncoderPaths(string configuredPath) + private bool ValidatePathFFprobe(string comment, string path) { - var appPath = configuredPath; - - if (!string.IsNullOrWhiteSpace(appPath)) + if (ValidatePath("FFprobe: " + comment, path) == true) { - if (Directory.Exists(appPath)) - { - return GetPathsFromDirectory(appPath); - } - - if (File.Exists(appPath)) - { - return new Tuple(appPath, GetProbePathFromEncoderPath(appPath)); - } + FFprobePath = path; + return true; } - return new Tuple(null, null); + return false; } - private Tuple TestForInstalledVersions() + private string GetEncoderPath(string path) { - string encoderPath = null; - string probePath = null; - - if (_hasExternalEncoder && ValidateVersion(_originalFFMpegPath, true)) + if (Directory.Exists(path)) { - encoderPath = _originalFFMpegPath; - probePath = _originalFFProbePath; + return GetEncoderPathFromDirectory(path); } - if (string.IsNullOrWhiteSpace(encoderPath)) + if (File.Exists(path)) { - if (ValidateVersion("ffmpeg", true) && ValidateVersion("ffprobe", false)) - { - encoderPath = "ffmpeg"; - probePath = "ffprobe"; - } + return path; } - return new Tuple(encoderPath, probePath); + return null; } - private Tuple GetPathsFromDirectory(string path) + private string GetEncoderPathFromDirectory(string path) { - // Since we can't predict the file extension, first try directly within the folder - // If that doesn't pan out, then do a recursive search - var files = FileSystem.GetFilePaths(path); - - var excludeExtensions = new[] { ".c" }; - - var ffmpegPath = files.FirstOrDefault(i => string.Equals(Path.GetFileNameWithoutExtension(i), "ffmpeg", StringComparison.OrdinalIgnoreCase) && !excludeExtensions.Contains(Path.GetExtension(i) ?? string.Empty)); - var ffprobePath = files.FirstOrDefault(i => string.Equals(Path.GetFileNameWithoutExtension(i), "ffprobe", StringComparison.OrdinalIgnoreCase) && !excludeExtensions.Contains(Path.GetExtension(i) ?? string.Empty)); - - if (string.IsNullOrWhiteSpace(ffmpegPath) || !File.Exists(ffmpegPath)) + try { - files = FileSystem.GetFilePaths(path, true); + var files = FileSystem.GetFilePaths(path); - ffmpegPath = files.FirstOrDefault(i => string.Equals(Path.GetFileNameWithoutExtension(i), "ffmpeg", StringComparison.OrdinalIgnoreCase) && !excludeExtensions.Contains(Path.GetExtension(i) ?? string.Empty)); + var excludeExtensions = new[] { ".c" }; - if (!string.IsNullOrWhiteSpace(ffmpegPath)) - { - ffprobePath = GetProbePathFromEncoderPath(ffmpegPath); - } + return files.FirstOrDefault(i => string.Equals(Path.GetFileNameWithoutExtension(i), "ffmpeg", StringComparison.OrdinalIgnoreCase) && !excludeExtensions.Contains(Path.GetExtension(i) ?? string.Empty)); + } + catch (Exception) + { + // Trap all exceptions, like DirNotExists, and return null + return null; } - - return new Tuple(ffmpegPath, ffprobePath); } private string GetProbePathFromEncoderPath(string appPath) @@ -357,15 +318,31 @@ namespace MediaBrowser.MediaEncoding.Encoder .FirstOrDefault(i => string.Equals(Path.GetFileNameWithoutExtension(i), "ffprobe", StringComparison.OrdinalIgnoreCase)); } - private void LogPaths() + /// + /// Search the system $PATH environment variable looking for given filename. + /// + /// + /// + private string ExistsOnSystemPath(string fileName) { - _logger.LogInformation("FFMpeg: {0}", FFMpegPath ?? "not found"); - _logger.LogInformation("FFProbe: {0}", FFProbePath ?? "not found"); + var values = Environment.GetEnvironmentVariable("PATH"); + + foreach (var path in values.Split(Path.PathSeparator)) + { + var candidatePath = GetEncoderPathFromDirectory(path); + + if (ValidatePath("Found on PATH", candidatePath)) + { + return candidatePath; + } + } + return null; } - private EncodingOptions GetEncodingOptions() + private void LogPaths() { - return ConfigurationManager.GetConfiguration("encoding"); + _logger.LogInformation("FFMpeg: {0}", FFmpegPath ?? "not found"); + _logger.LogInformation("FFProbe: {0}", FFprobePath ?? "not found"); } private List _encoders = new List(); @@ -412,12 +389,6 @@ namespace MediaBrowser.MediaEncoding.Encoder return true; } - /// - /// Gets the encoder path. - /// - /// The encoder path. - public string EncoderPath => FFMpegPath; - /// /// Gets the media info. /// @@ -489,7 +460,7 @@ namespace MediaBrowser.MediaEncoding.Encoder // Must consume both or ffmpeg may hang due to deadlocks. See comments below. RedirectStandardOutput = true, - FileName = FFProbePath, + FileName = FFprobePath, Arguments = string.Format(args, probeSizeArgument, inputPath).Trim(), IsHidden = true, @@ -691,7 +662,7 @@ namespace MediaBrowser.MediaEncoding.Encoder { CreateNoWindow = true, UseShellExecute = false, - FileName = FFMpegPath, + FileName = FFmpegPath, Arguments = args, IsHidden = true, ErrorDialog = false, @@ -814,7 +785,7 @@ namespace MediaBrowser.MediaEncoding.Encoder { CreateNoWindow = true, UseShellExecute = false, - FileName = FFMpegPath, + FileName = FFmpegPath, Arguments = args, IsHidden = true, ErrorDialog = false, diff --git a/MediaBrowser.Model/Configuration/EncodingOptions.cs b/MediaBrowser.Model/Configuration/EncodingOptions.cs index 8584bd3ddb..ff697437ac 100644 --- a/MediaBrowser.Model/Configuration/EncodingOptions.cs +++ b/MediaBrowser.Model/Configuration/EncodingOptions.cs @@ -8,7 +8,8 @@ namespace MediaBrowser.Model.Configuration public bool EnableThrottling { get; set; } public int ThrottleDelaySeconds { get; set; } public string HardwareAccelerationType { get; set; } - public string EncoderAppPath { get; set; } + public string EncoderAppPathCustom { get; set; } // FFmpeg path as set by the user via the UI + public string EncoderAppPath { get; set; } // The current FFmpeg path being used by the system public string VaapiDevice { get; set; } public int H264Crf { get; set; } public string H264Preset { get; set; } From ed69e690b89e6a3e6e22bbf448af08a25c38e71b Mon Sep 17 00:00:00 2001 From: PloughPuff Date: Tue, 12 Feb 2019 22:05:42 +0000 Subject: [PATCH 2/6] Review comments Address review comments from JustAMan, Bond-009 and cvium. --- .../ApplicationHost.cs | 35 +-- .../MediaEncoding/IMediaEncoder.cs | 3 +- .../Encoder/EncoderValidator.cs | 8 + .../Encoder/MediaEncoder.cs | 247 ++++++++---------- MediaBrowser.Model/System/SystemInfo.cs | 17 +- 5 files changed, 150 insertions(+), 160 deletions(-) diff --git a/Emby.Server.Implementations/ApplicationHost.cs b/Emby.Server.Implementations/ApplicationHost.cs index 2c0d0e7469..dd29f2adec 100644 --- a/Emby.Server.Implementations/ApplicationHost.cs +++ b/Emby.Server.Implementations/ApplicationHost.cs @@ -791,7 +791,17 @@ namespace Emby.Server.Implementations ChapterManager = new ChapterManager(LibraryManager, LoggerFactory, ServerConfigurationManager, ItemRepository); serviceCollection.AddSingleton(ChapterManager); - MediaEncoder = new MediaBrowser.MediaEncoding.Encoder.MediaEncoder(LoggerFactory, JsonSerializer, StartupOptions.FFmpegPath, StartupOptions.FFprobePath, ServerConfigurationManager, FileSystemManager, () => SubtitleEncoder, () => MediaSourceManager, ProcessFactory, 5000); + MediaEncoder = new MediaBrowser.MediaEncoding.Encoder.MediaEncoder( + LoggerFactory, + JsonSerializer, + StartupOptions.FFmpegPath, + StartupOptions.FFprobePath, + ServerConfigurationManager, + FileSystemManager, + () => SubtitleEncoder, + () => MediaSourceManager, + ProcessFactory, + 5000); serviceCollection.AddSingleton(MediaEncoder); EncodingManager = new MediaEncoder.EncodingManager(FileSystemManager, LoggerFactory, MediaEncoder, ChapterManager, LibraryManager); @@ -908,27 +918,6 @@ namespace Emby.Server.Implementations return new ImageProcessor(LoggerFactory, ServerConfigurationManager.ApplicationPaths, FileSystemManager, ImageEncoder, () => LibraryManager, () => MediaEncoder); } - /// - /// Registers the media encoder. - /// - /// Task. - private void RegisterMediaEncoder(IAssemblyInfo assemblyInfo) - { - MediaEncoder = new MediaBrowser.MediaEncoding.Encoder.MediaEncoder( - LoggerFactory, - JsonSerializer, - StartupOptions.FFmpegPath, - StartupOptions.FFprobePath, - ServerConfigurationManager, - FileSystemManager, - () => SubtitleEncoder, - () => MediaSourceManager, - ProcessFactory, - 5000); - - RegisterSingleInstance(MediaEncoder); - } - /// /// Gets the user repository. /// @@ -1404,7 +1393,7 @@ namespace Emby.Server.Implementations ServerName = FriendlyName, LocalAddress = localAddress, SupportsLibraryMonitor = true, - EncoderLocationType = MediaEncoder.EncoderLocationType, + EncoderLocation = MediaEncoder.EncoderLocation, SystemArchitecture = EnvironmentInfo.SystemArchitecture, SystemUpdateLevel = SystemUpdateLevel, PackageName = StartupOptions.PackageName diff --git a/MediaBrowser.Controller/MediaEncoding/IMediaEncoder.cs b/MediaBrowser.Controller/MediaEncoding/IMediaEncoder.cs index 057e439104..8852dac051 100644 --- a/MediaBrowser.Controller/MediaEncoding/IMediaEncoder.cs +++ b/MediaBrowser.Controller/MediaEncoding/IMediaEncoder.cs @@ -6,6 +6,7 @@ using MediaBrowser.Model.Dlna; using MediaBrowser.Model.Entities; using MediaBrowser.Model.IO; using MediaBrowser.Model.MediaInfo; +using MediaBrowser.Model.System; namespace MediaBrowser.Controller.MediaEncoding { @@ -14,7 +15,7 @@ namespace MediaBrowser.Controller.MediaEncoding /// public interface IMediaEncoder : ITranscoderSupport { - string EncoderLocationType { get; } + FFmpegLocation EncoderLocation { get; } /// /// Gets the encoder path. diff --git a/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs b/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs index 1eeea87a06..3eed891cb3 100644 --- a/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs +++ b/MediaBrowser.MediaEncoding/Encoder/EncoderValidator.cs @@ -48,6 +48,10 @@ namespace MediaBrowser.MediaEncoding.Encoder if (string.IsNullOrWhiteSpace(output)) { + if (logOutput) + { + _logger.LogError("FFmpeg validation: The process returned no result"); + } return false; } @@ -55,6 +59,10 @@ namespace MediaBrowser.MediaEncoding.Encoder if (output.IndexOf("Libav developers", StringComparison.OrdinalIgnoreCase) != -1) { + if (logOutput) + { + _logger.LogError("FFmpeg validation: avconv instead of ffmpeg is not supported"); + } return false; } diff --git a/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs b/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs index 36d72cad90..9aad67ec70 100644 --- a/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs +++ b/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Globalization; using System.IO; using System.Linq; +using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; using MediaBrowser.Common.Configuration; @@ -18,6 +19,7 @@ using MediaBrowser.Model.Entities; using MediaBrowser.Model.IO; using MediaBrowser.Model.MediaInfo; using MediaBrowser.Model.Serialization; +using MediaBrowser.Model.System; using Microsoft.Extensions.Logging; namespace MediaBrowser.MediaEncoding.Encoder @@ -34,17 +36,16 @@ namespace MediaBrowser.MediaEncoding.Encoder public string EncoderPath => FFmpegPath; /// - /// External: path supplied via command line - /// Custom: coming from UI or config/encoding.xml file - /// System: FFmpeg found in system $PATH - /// null: No FFmpeg found + /// The location of the discovered FFmpeg tool. /// - public string EncoderLocationType { get; private set; } + public FFmpegLocation EncoderLocation { get; private set; } + + private FFmpegLocation ProbeLocation; private readonly ILogger _logger; private readonly IJsonSerializer _jsonSerializer; - private string FFmpegPath { get; set; } - private string FFprobePath { get; set; } + private string FFmpegPath; + private string FFprobePath; protected readonly IServerConfigurationManager ConfigurationManager; protected readonly IFileSystem FileSystem; protected readonly Func SubtitleEncoder; @@ -54,6 +55,11 @@ namespace MediaBrowser.MediaEncoding.Encoder private readonly string StartupOptionFFmpegPath; private readonly string StartupOptionFFprobePath; + /// + /// Enum to identify the two types of FF utilities of interest. + /// + private enum FFtype { Mpeg, Probe }; + private readonly SemaphoreSlim _thumbnailResourcePool = new SemaphoreSlim(1, 1); private readonly List _runningProcesses = new List(); @@ -82,48 +88,24 @@ namespace MediaBrowser.MediaEncoding.Encoder /// /// Run at startup or if the user removes a Custom path from transcode page. - /// Sets global variables FFmpegPath and EncoderLocationType. - /// If startup options --ffprobe is given then FFprobePath is set too. + /// Sets global variables FFmpegPath. + /// Precedence is: Config > CLI > $PATH /// public void Init() { - // 1) If given, use the --ffmpeg CLI switch - if (ValidatePathFFmpeg("From CLI Switch", StartupOptionFFmpegPath)) - { - _logger.LogInformation("FFmpeg: Using path from command line switch --ffmpeg"); - EncoderLocationType = "External"; - } - - // 2) Try Custom path stroed in config/encoding xml file under tag - else if (ValidatePathFFmpeg("From Config File", ConfigurationManager.GetConfiguration("encoding").EncoderAppPathCustom)) - { - _logger.LogInformation("FFmpeg: Using path from config/encoding.xml file"); - EncoderLocationType = "Custom"; - } - - // 3) Search system $PATH environment variable for valid FFmpeg - else if (ValidatePathFFmpeg("From $PATH", ExistsOnSystemPath("ffmpeg"))) + // 1) Custom path stored in config/encoding xml file under tag takes precedence + if (!ValidatePath(FFtype.Mpeg, ConfigurationManager.GetConfiguration("encoding").EncoderAppPathCustom, FFmpegLocation.Custom)) { - _logger.LogInformation("FFmpeg: Using system $PATH for FFmpeg"); - EncoderLocationType = "System"; - } - else - { - _logger.LogError("FFmpeg: No suitable executable found"); - FFmpegPath = null; - EncoderLocationType = null; - } - - // If given, use the --ffprobe CLI switch - if (ValidatePathFFprobe("CLI Switch", StartupOptionFFprobePath)) - { - _logger.LogInformation("FFprobe: Using path from command line switch --ffprobe"); - } - else - { - // FFprobe path from command line is no good, so set to null and let ReInit() try - // and set using the FFmpeg path. - FFprobePath = null; + // 2) Check if the --ffmpeg CLI switch has been given + if (!ValidatePath(FFtype.Mpeg, StartupOptionFFmpegPath, FFmpegLocation.SetByArgument)) + { + // 3) Search system $PATH environment variable for valid FFmpeg + if (!ValidatePath(FFtype.Mpeg, ExistsOnSystemPath("ffmpeg"), FFmpegLocation.System)) + { + EncoderLocation = FFmpegLocation.NotFound; + FFmpegPath = null; + } + } } ReInit(); @@ -136,27 +118,27 @@ namespace MediaBrowser.MediaEncoding.Encoder /// private void ReInit() { - // Write the FFmpeg path to the config/encoding.xml file so it appears in UI + // Write the FFmpeg path to the config/encoding.xml file as so it appears in UI var config = ConfigurationManager.GetConfiguration("encoding"); config.EncoderAppPath = FFmpegPath ?? string.Empty; ConfigurationManager.SaveConfiguration("encoding", config); + // Clear probe settings in case probe validation fails + ProbeLocation = FFmpegLocation.NotFound; + FFprobePath = null; + // Only if mpeg path is set, try and set path to probe if (FFmpegPath != null) { - // Probe would be null here if no valid --ffprobe path was given - // at startup, or we're performing ReInit following mpeg path update from UI - if (FFprobePath == null) + if (EncoderLocation == FFmpegLocation.Custom || StartupOptionFFprobePath == null) { - // Use the mpeg path to create a probe path - if (ValidatePathFFprobe("Copied from FFmpeg:", GetProbePathFromEncoderPath(FFmpegPath))) - { - _logger.LogInformation("FFprobe: Using FFprobe in same folders as FFmpeg"); - } - else - { - _logger.LogError("FFprobe: No suitable executable found"); - } + // If mpeg was read from config, or CLI switch not given, try and set probe from mpeg path + ValidatePath(FFtype.Probe, GetProbePathFromEncoderPath(FFmpegPath), EncoderLocation); + } + else + { + // Else try and set probe path from CLI switch + ValidatePath(FFtype.Probe, StartupOptionFFmpegPath, FFmpegLocation.SetByArgument); } // Interrogate to understand what coders it supports @@ -183,108 +165,95 @@ namespace MediaBrowser.MediaEncoding.Encoder { throw new ArgumentException("Unexpected pathType value"); } - else + + if (string.IsNullOrWhiteSpace(path)) { - if (string.IsNullOrWhiteSpace(path)) - { - // User had cleared the cutom path in UI. Clear the Custom config - // setting and peform full Init to relook any CLI switches and system $PATH - var config = ConfigurationManager.GetConfiguration("encoding"); - config.EncoderAppPathCustom = string.Empty; - ConfigurationManager.SaveConfiguration("encoding", config); + // User had cleared the custom path in UI. Clear the Custom config + // setting and perform full Init to reinspect any CLI switches and system $PATH + var config = ConfigurationManager.GetConfiguration("encoding"); + config.EncoderAppPathCustom = string.Empty; + ConfigurationManager.SaveConfiguration("encoding", config); - Init(); - } - else if (!File.Exists(path) && !Directory.Exists(path)) + Init(); + } + else if (!File.Exists(path) && !Directory.Exists(path)) + { + // Given path is neither file or folder + throw new ResourceNotFoundException(); + } + else + { + // Supplied path could be either file path or folder path. + // Resolve down to file path and validate + if (!ValidatePath(FFtype.Mpeg, GetEncoderPath(path), FFmpegLocation.Custom)) { - // Given path is neither file or folder - throw new ResourceNotFoundException(); + throw new ResourceNotFoundException("Failed validation checks."); } else { - // Supplied path could be either file path or folder path. - // Resolve down to file path and validate - path = GetEncoderPath(path); - - if (path == null) - { - throw new ResourceNotFoundException("FFmpeg not found"); - } - else if (!ValidatePathFFmpeg("New From UI", path)) - { - throw new ResourceNotFoundException("Failed validation checks. Version 4.0 or greater is required"); - } - else - { - EncoderLocationType = "Custom"; - - // Write the validated mpeg path to the xml as - // This ensures its not lost on new startup - var config = ConfigurationManager.GetConfiguration("encoding"); - config.EncoderAppPathCustom = FFmpegPath; - ConfigurationManager.SaveConfiguration("encoding", config); - - FFprobePath = null; // Clear probe path so it gets relooked in ReInit() + // Write the validated mpeg path to the xml as + // This ensures its not lost on new startup + var config = ConfigurationManager.GetConfiguration("encoding"); + config.EncoderAppPathCustom = FFmpegPath; + ConfigurationManager.SaveConfiguration("encoding", config); - ReInit(); - } + ReInit(); } } } - private bool ValidatePath(string type, string path) + /// + /// Validates the supplied FQPN to ensure it is a FFxxx utility. + /// If checks pass, global variable FFmpegPath (or FFprobePath) and + /// EncoderLocation (or ProbeLocation) are updated. + /// + /// Either mpeg or probe + /// FQPN to test + /// Location (External, Custom, System) of tool + /// + private bool ValidatePath(FFtype type, string path, FFmpegLocation location) { + bool rc = false; + if (!string.IsNullOrEmpty(path)) { if (File.Exists(path)) { - var valid = new EncoderValidator(_logger, _processFactory).ValidateVersion(path, true); + rc = new EncoderValidator(_logger, _processFactory).ValidateVersion(path, false); - if (valid == true) + // Only update the global variables if the checks passed + if (rc) { - return true; + if (type == FFtype.Mpeg) + { + FFmpegPath = path; + EncoderLocation = location; + } + else + { + FFprobePath = path; + ProbeLocation = location; + } } else { - _logger.LogError("{0}: Failed validation checks. Version 4.0 or greater is required: {1}", type, path); + _logger.LogError("{0}: {1}: Failed version check: {2}", type.ToString(), location.ToString(), path); } } else { - _logger.LogError("{0}: File not found: {1}", type, path); + _logger.LogError("{0}: {1}: File not found: {2}", type.ToString(), location.ToString(), path); } } - return false; - } - - private bool ValidatePathFFmpeg(string comment, string path) - { - if (ValidatePath("FFmpeg: " + comment, path) == true) - { - FFmpegPath = path; - return true; - } - - return false; - } - - private bool ValidatePathFFprobe(string comment, string path) - { - if (ValidatePath("FFprobe: " + comment, path) == true) - { - FFprobePath = path; - return true; - } - - return false; + return rc; } private string GetEncoderPath(string path) { if (Directory.Exists(path)) { - return GetEncoderPathFromDirectory(path); + return GetEncoderPathFromDirectory(path, "ffmpeg"); } if (File.Exists(path)) @@ -295,7 +264,7 @@ namespace MediaBrowser.MediaEncoding.Encoder return null; } - private string GetEncoderPathFromDirectory(string path) + private string GetEncoderPathFromDirectory(string path, string filename) { try { @@ -303,7 +272,8 @@ namespace MediaBrowser.MediaEncoding.Encoder var excludeExtensions = new[] { ".c" }; - return files.FirstOrDefault(i => string.Equals(Path.GetFileNameWithoutExtension(i), "ffmpeg", StringComparison.OrdinalIgnoreCase) && !excludeExtensions.Contains(Path.GetExtension(i) ?? string.Empty)); + return files.FirstOrDefault(i => string.Equals(Path.GetFileNameWithoutExtension(i), filename, StringComparison.OrdinalIgnoreCase) + && !excludeExtensions.Contains(Path.GetExtension(i) ?? string.Empty)); } catch (Exception) { @@ -314,8 +284,15 @@ namespace MediaBrowser.MediaEncoding.Encoder private string GetProbePathFromEncoderPath(string appPath) { - return FileSystem.GetFilePaths(Path.GetDirectoryName(appPath)) - .FirstOrDefault(i => string.Equals(Path.GetFileNameWithoutExtension(i), "ffprobe", StringComparison.OrdinalIgnoreCase)); + if (!string.IsNullOrEmpty(appPath)) + { + string pattern = @"[^\/\\]+?(\.[^\/\\\n.]+)?$"; + string substitution = @"ffprobe$1"; + + return Regex.Replace(appPath, pattern, substitution); + } + + return null; } /// @@ -323,15 +300,15 @@ namespace MediaBrowser.MediaEncoding.Encoder /// /// /// - private string ExistsOnSystemPath(string fileName) + private string ExistsOnSystemPath(string filename) { var values = Environment.GetEnvironmentVariable("PATH"); foreach (var path in values.Split(Path.PathSeparator)) { - var candidatePath = GetEncoderPathFromDirectory(path); + var candidatePath = GetEncoderPathFromDirectory(path, filename); - if (ValidatePath("Found on PATH", candidatePath)) + if (!string.IsNullOrEmpty(candidatePath)) { return candidatePath; } @@ -341,8 +318,8 @@ namespace MediaBrowser.MediaEncoding.Encoder private void LogPaths() { - _logger.LogInformation("FFMpeg: {0}", FFmpegPath ?? "not found"); - _logger.LogInformation("FFProbe: {0}", FFprobePath ?? "not found"); + _logger.LogInformation("FFmpeg: {0}: {1}", EncoderLocation.ToString(), FFmpegPath ?? string.Empty); + _logger.LogInformation("FFprobe: {0}: {1}", ProbeLocation.ToString(), FFprobePath ?? string.Empty); } private List _encoders = new List(); diff --git a/MediaBrowser.Model/System/SystemInfo.cs b/MediaBrowser.Model/System/SystemInfo.cs index 581a1069cd..6482f2c840 100644 --- a/MediaBrowser.Model/System/SystemInfo.cs +++ b/MediaBrowser.Model/System/SystemInfo.cs @@ -4,6 +4,21 @@ using MediaBrowser.Model.Updates; namespace MediaBrowser.Model.System { + /// + /// Enum describing the location of the FFmpeg tool. + /// + public enum FFmpegLocation + { + /// No path to FFmpeg found. + NotFound, + /// Path supplied via command line using switch --ffmpeg. + SetByArgument, + /// User has supplied path via Transcoding UI page. + Custom, + /// FFmpeg tool found on system $PATH. + System + }; + /// /// Class SystemInfo /// @@ -122,7 +137,7 @@ namespace MediaBrowser.Model.System /// true if this instance has update available; otherwise, false. public bool HasUpdateAvailable { get; set; } - public string EncoderLocationType { get; set; } + public FFmpegLocation EncoderLocation { get; set; } public Architecture SystemArchitecture { get; set; } From 632968dc817a398bff84ca3ef82337b4e5786416 Mon Sep 17 00:00:00 2001 From: PloughPuff Date: Wed, 13 Feb 2019 16:46:03 +0000 Subject: [PATCH 3/6] Added self to contributors --- CONTRIBUTORS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 4b397b3280..81857e57c7 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -23,6 +23,7 @@ - [fruhnow](https://github.com/fruhnow) - [Lynxy](https://github.com/Lynxy) - [fasheng](https://github.com/fasheng) + - [ploughpuff](https://github.com/ploughpuff) # Emby Contributors From 8104e739d5b83d0d6563bb685f910697e60d6c12 Mon Sep 17 00:00:00 2001 From: PloughPuff Date: Wed, 27 Feb 2019 16:33:08 +0000 Subject: [PATCH 4/6] Address review comments from Bond --- MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs | 10 ++++++++-- MediaBrowser.Model/Configuration/EncodingOptions.cs | 10 ++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs b/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs index 9aad67ec70..265c043b98 100644 --- a/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs +++ b/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs @@ -282,12 +282,18 @@ namespace MediaBrowser.MediaEncoding.Encoder } } + /// + /// With the given path string, replaces the filename with ffprobe, taking case + /// of any file extension (like .exe on windows). + /// + /// + /// private string GetProbePathFromEncoderPath(string appPath) { if (!string.IsNullOrEmpty(appPath)) { - string pattern = @"[^\/\\]+?(\.[^\/\\\n.]+)?$"; - string substitution = @"ffprobe$1"; + const string pattern = @"[^\/\\]+?(\.[^\/\\\n.]+)?$"; + const string substitution = @"ffprobe$1"; return Regex.Replace(appPath, pattern, substitution); } diff --git a/MediaBrowser.Model/Configuration/EncodingOptions.cs b/MediaBrowser.Model/Configuration/EncodingOptions.cs index ff697437ac..7fc985ba02 100644 --- a/MediaBrowser.Model/Configuration/EncodingOptions.cs +++ b/MediaBrowser.Model/Configuration/EncodingOptions.cs @@ -8,8 +8,14 @@ namespace MediaBrowser.Model.Configuration public bool EnableThrottling { get; set; } public int ThrottleDelaySeconds { get; set; } public string HardwareAccelerationType { get; set; } - public string EncoderAppPathCustom { get; set; } // FFmpeg path as set by the user via the UI - public string EncoderAppPath { get; set; } // The current FFmpeg path being used by the system + /// + /// FFmpeg path as set by the user via the UI + /// + public string EncoderAppPathCustom { get; set; } + /// + /// The current FFmpeg path being used by the system + /// + public string EncoderAppPath { get; set; } public string VaapiDevice { get; set; } public int H264Crf { get; set; } public string H264Preset { get; set; } From 656bffbbb291dc9b58443bb36c8ed7d3688f6c1b Mon Sep 17 00:00:00 2001 From: PloughPuff Date: Thu, 28 Feb 2019 22:06:56 +0000 Subject: [PATCH 5/6] Remove --ffprobe logic --- Jellyfin.Server/StartupOptions.cs | 4 +- .../Encoder/MediaEncoder.cs | 185 +++++------------- .../Configuration/EncodingOptions.cs | 6 +- 3 files changed, 58 insertions(+), 137 deletions(-) diff --git a/Jellyfin.Server/StartupOptions.cs b/Jellyfin.Server/StartupOptions.cs index 5d3f7b171b..c8cdb984db 100644 --- a/Jellyfin.Server/StartupOptions.cs +++ b/Jellyfin.Server/StartupOptions.cs @@ -20,10 +20,10 @@ namespace Jellyfin.Server [Option('l', "logdir", Required = false, HelpText = "Path to use for writing log files.")] public string LogDir { get; set; } - [Option("ffmpeg", Required = false, HelpText = "Path to external FFmpeg executable to use in place of default found in PATH. Must be specified along with --ffprobe.")] + [Option("ffmpeg", Required = false, HelpText = "Path to external FFmpeg executable to use in place of default found in PATH.")] public string FFmpegPath { get; set; } - [Option("ffprobe", Required = false, HelpText = "Path to external FFprobe executable to use in place of default found in PATH. Must be specified along with --ffmpeg.")] + [Option("ffprobe", Required = false, HelpText = "(deprecated) Option has no effect and shall be removed in next release.")] public string FFprobePath { get; set; } [Option("service", Required = false, HelpText = "Run as headless service.")] diff --git a/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs b/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs index 265c043b98..51b4f6e393 100644 --- a/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs +++ b/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs @@ -40,8 +40,6 @@ namespace MediaBrowser.MediaEncoding.Encoder /// public FFmpegLocation EncoderLocation { get; private set; } - private FFmpegLocation ProbeLocation; - private readonly ILogger _logger; private readonly IJsonSerializer _jsonSerializer; private string FFmpegPath; @@ -55,11 +53,6 @@ namespace MediaBrowser.MediaEncoding.Encoder private readonly string StartupOptionFFmpegPath; private readonly string StartupOptionFFprobePath; - /// - /// Enum to identify the two types of FF utilities of interest. - /// - private enum FFtype { Mpeg, Probe }; - private readonly SemaphoreSlim _thumbnailResourcePool = new SemaphoreSlim(1, 1); private readonly List _runningProcesses = new List(); @@ -93,14 +86,20 @@ namespace MediaBrowser.MediaEncoding.Encoder /// public void Init() { - // 1) Custom path stored in config/encoding xml file under tag takes precedence - if (!ValidatePath(FFtype.Mpeg, ConfigurationManager.GetConfiguration("encoding").EncoderAppPathCustom, FFmpegLocation.Custom)) + // ToDo - Finalise removal of the --ffprobe switch + if (!string.IsNullOrEmpty(StartupOptionFFprobePath)) + { + _logger.LogWarning("--ffprobe switch is deprecated and shall be removed in the next release"); + } + + // 1) Custom path stored in config/encoding xml file under tag takes precedence + if (!ValidatePath(ConfigurationManager.GetConfiguration("encoding").EncoderAppPath, FFmpegLocation.Custom)) { // 2) Check if the --ffmpeg CLI switch has been given - if (!ValidatePath(FFtype.Mpeg, StartupOptionFFmpegPath, FFmpegLocation.SetByArgument)) + if (!ValidatePath(StartupOptionFFmpegPath, FFmpegLocation.SetByArgument)) { // 3) Search system $PATH environment variable for valid FFmpeg - if (!ValidatePath(FFtype.Mpeg, ExistsOnSystemPath("ffmpeg"), FFmpegLocation.System)) + if (!ValidatePath(ExistsOnSystemPath("ffmpeg"), FFmpegLocation.System)) { EncoderLocation = FFmpegLocation.NotFound; FFmpegPath = null; @@ -108,110 +107,80 @@ namespace MediaBrowser.MediaEncoding.Encoder } } - ReInit(); - } - - /// - /// Writes the currently used FFmpeg to config/encoding.xml file. - /// Sets the FFprobe path if not currently set. - /// Interrogates the FFmpeg tool to identify what encoders/decodres are available. - /// - private void ReInit() - { - // Write the FFmpeg path to the config/encoding.xml file as so it appears in UI + // Write the FFmpeg path to the config/encoding.xml file as so it appears in UI var config = ConfigurationManager.GetConfiguration("encoding"); - config.EncoderAppPath = FFmpegPath ?? string.Empty; + config.EncoderAppPathDisplay = FFmpegPath ?? string.Empty; ConfigurationManager.SaveConfiguration("encoding", config); - // Clear probe settings in case probe validation fails - ProbeLocation = FFmpegLocation.NotFound; - FFprobePath = null; - // Only if mpeg path is set, try and set path to probe if (FFmpegPath != null) { - if (EncoderLocation == FFmpegLocation.Custom || StartupOptionFFprobePath == null) - { - // If mpeg was read from config, or CLI switch not given, try and set probe from mpeg path - ValidatePath(FFtype.Probe, GetProbePathFromEncoderPath(FFmpegPath), EncoderLocation); - } - else - { - // Else try and set probe path from CLI switch - ValidatePath(FFtype.Probe, StartupOptionFFmpegPath, FFmpegLocation.SetByArgument); - } + // Determine a probe path from the mpeg path + FFprobePath = Regex.Replace(FFmpegPath, @"[^\/\\]+?(\.[^\/\\\n.]+)?$", @"ffprobe$1"); - // Interrogate to understand what coders it supports + // Interrogate to understand what coders are supported var result = new EncoderValidator(_logger, _processFactory).GetAvailableCoders(FFmpegPath); SetAvailableDecoders(result.decoders); SetAvailableEncoders(result.encoders); } - // Stamp FFmpeg paths to the log file - LogPaths(); + _logger.LogInformation("FFmpeg: {0}: {1}", EncoderLocation.ToString(), FFmpegPath ?? string.Empty); } /// - /// Triggered from the Settings > Trascoding UI page when users sumits Custom FFmpeg path to use. + /// Triggered from the Settings > Transcoding UI page when users submits Custom FFmpeg path to use. + /// Only write the new path to xml if it exists. Do not perform validation checks on ffmpeg here. /// /// /// public void UpdateEncoderPath(string path, string pathType) { + string newPath; + _logger.LogInformation("Attempting to update encoder path to {0}. pathType: {1}", path ?? string.Empty, pathType ?? string.Empty); if (!string.Equals(pathType, "custom", StringComparison.OrdinalIgnoreCase)) { throw new ArgumentException("Unexpected pathType value"); } - - if (string.IsNullOrWhiteSpace(path)) + else if (string.IsNullOrWhiteSpace(path)) { - // User had cleared the custom path in UI. Clear the Custom config - // setting and perform full Init to reinspect any CLI switches and system $PATH - var config = ConfigurationManager.GetConfiguration("encoding"); - config.EncoderAppPathCustom = string.Empty; - ConfigurationManager.SaveConfiguration("encoding", config); - - Init(); + // User had cleared the custom path in UI + newPath = string.Empty; } - else if (!File.Exists(path) && !Directory.Exists(path)) + else if (File.Exists(path)) { - // Given path is neither file or folder - throw new ResourceNotFoundException(); + newPath = path; + } + else if (Directory.Exists(path)) + { + // Given path is directory, so resolve down to filename + newPath = GetEncoderPathFromDirectory(path, "ffmpeg"); } else { - // Supplied path could be either file path or folder path. - // Resolve down to file path and validate - if (!ValidatePath(FFtype.Mpeg, GetEncoderPath(path), FFmpegLocation.Custom)) - { - throw new ResourceNotFoundException("Failed validation checks."); - } - else - { - // Write the validated mpeg path to the xml as - // This ensures its not lost on new startup - var config = ConfigurationManager.GetConfiguration("encoding"); - config.EncoderAppPathCustom = FFmpegPath; - ConfigurationManager.SaveConfiguration("encoding", config); - - ReInit(); - } + throw new ResourceNotFoundException(); } + + // Write the new ffmpeg path to the xml as + // This ensures its not lost on next startup + var config = ConfigurationManager.GetConfiguration("encoding"); + config.EncoderAppPath = newPath; + ConfigurationManager.SaveConfiguration("encoding", config); + + // Trigger Init so we validate the new path and setup probe path + Init(); } /// - /// Validates the supplied FQPN to ensure it is a FFxxx utility. - /// If checks pass, global variable FFmpegPath (or FFprobePath) and - /// EncoderLocation (or ProbeLocation) are updated. + /// Validates the supplied FQPN to ensure it is a ffmpeg utility. + /// If checks pass, global variable FFmpegPath and EncoderLocation are updated. /// - /// Either mpeg or probe /// FQPN to test /// Location (External, Custom, System) of tool /// - private bool ValidatePath(FFtype type, string path, FFmpegLocation location) + private bool ValidatePath(string path, FFmpegLocation location) { bool rc = false; @@ -219,51 +188,28 @@ namespace MediaBrowser.MediaEncoding.Encoder { if (File.Exists(path)) { - rc = new EncoderValidator(_logger, _processFactory).ValidateVersion(path, false); + rc = new EncoderValidator(_logger, _processFactory).ValidateVersion(path, true); - // Only update the global variables if the checks passed - if (rc) + if (!rc) { - if (type == FFtype.Mpeg) - { - FFmpegPath = path; - EncoderLocation = location; - } - else - { - FFprobePath = path; - ProbeLocation = location; - } - } - else - { - _logger.LogError("{0}: {1}: Failed version check: {2}", type.ToString(), location.ToString(), path); + _logger.LogWarning("FFmpeg: {0}: Failed version check: {1}", location.ToString(), path); } + + // ToDo - Enable the ffmpeg validator. At the moment any version can be used. + rc = true; + + FFmpegPath = path; + EncoderLocation = location; } else { - _logger.LogError("{0}: {1}: File not found: {2}", type.ToString(), location.ToString(), path); + _logger.LogWarning("FFmpeg: {0}: File not found: {1}", location.ToString(), path); } } return rc; } - private string GetEncoderPath(string path) - { - if (Directory.Exists(path)) - { - return GetEncoderPathFromDirectory(path, "ffmpeg"); - } - - if (File.Exists(path)) - { - return path; - } - - return null; - } - private string GetEncoderPathFromDirectory(string path, string filename) { try @@ -282,25 +228,6 @@ namespace MediaBrowser.MediaEncoding.Encoder } } - /// - /// With the given path string, replaces the filename with ffprobe, taking case - /// of any file extension (like .exe on windows). - /// - /// - /// - private string GetProbePathFromEncoderPath(string appPath) - { - if (!string.IsNullOrEmpty(appPath)) - { - const string pattern = @"[^\/\\]+?(\.[^\/\\\n.]+)?$"; - const string substitution = @"ffprobe$1"; - - return Regex.Replace(appPath, pattern, substitution); - } - - return null; - } - /// /// Search the system $PATH environment variable looking for given filename. /// @@ -322,12 +249,6 @@ namespace MediaBrowser.MediaEncoding.Encoder return null; } - private void LogPaths() - { - _logger.LogInformation("FFmpeg: {0}: {1}", EncoderLocation.ToString(), FFmpegPath ?? string.Empty); - _logger.LogInformation("FFprobe: {0}: {1}", ProbeLocation.ToString(), FFprobePath ?? string.Empty); - } - private List _encoders = new List(); public void SetAvailableEncoders(IEnumerable list) { diff --git a/MediaBrowser.Model/Configuration/EncodingOptions.cs b/MediaBrowser.Model/Configuration/EncodingOptions.cs index 7fc985ba02..285ff4ba58 100644 --- a/MediaBrowser.Model/Configuration/EncodingOptions.cs +++ b/MediaBrowser.Model/Configuration/EncodingOptions.cs @@ -11,11 +11,11 @@ namespace MediaBrowser.Model.Configuration /// /// FFmpeg path as set by the user via the UI /// - public string EncoderAppPathCustom { get; set; } + public string EncoderAppPath { get; set; } /// - /// The current FFmpeg path being used by the system + /// The current FFmpeg path being used by the system and displayed on the transcode page /// - public string EncoderAppPath { get; set; } + public string EncoderAppPathDisplay { get; set; } public string VaapiDevice { get; set; } public int H264Crf { get; set; } public string H264Preset { get; set; } From 2617a49b78c99f72ba36e53a4c97c4e042116a53 Mon Sep 17 00:00:00 2001 From: PloughPuff Date: Thu, 28 Feb 2019 22:47:56 +0000 Subject: [PATCH 6/6] Renamed Init() to SetFFmpegPath() --- Emby.Server.Implementations/ApplicationHost.cs | 2 +- MediaBrowser.Controller/MediaEncoding/IMediaEncoder.cs | 2 +- MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Emby.Server.Implementations/ApplicationHost.cs b/Emby.Server.Implementations/ApplicationHost.cs index dd29f2adec..325df32931 100644 --- a/Emby.Server.Implementations/ApplicationHost.cs +++ b/Emby.Server.Implementations/ApplicationHost.cs @@ -534,7 +534,7 @@ namespace Emby.Server.Implementations ConfigurationManager.ConfigurationUpdated += OnConfigurationUpdated; - MediaEncoder.Init(); + MediaEncoder.SetFFmpegPath(); //if (string.IsNullOrWhiteSpace(MediaEncoder.EncoderPath)) //{ diff --git a/MediaBrowser.Controller/MediaEncoding/IMediaEncoder.cs b/MediaBrowser.Controller/MediaEncoding/IMediaEncoder.cs index 8852dac051..d4ac3b7c37 100644 --- a/MediaBrowser.Controller/MediaEncoding/IMediaEncoder.cs +++ b/MediaBrowser.Controller/MediaEncoding/IMediaEncoder.cs @@ -92,7 +92,7 @@ namespace MediaBrowser.Controller.MediaEncoding /// System.String. string EscapeSubtitleFilterPath(string path); - void Init(); + void SetFFmpegPath(); void UpdateEncoderPath(string path, string pathType); bool SupportsEncoder(string encoder); diff --git a/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs b/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs index 51b4f6e393..2924577888 100644 --- a/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs +++ b/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs @@ -84,7 +84,7 @@ namespace MediaBrowser.MediaEncoding.Encoder /// Sets global variables FFmpegPath. /// Precedence is: Config > CLI > $PATH /// - public void Init() + public void SetFFmpegPath() { // ToDo - Finalise removal of the --ffprobe switch if (!string.IsNullOrEmpty(StartupOptionFFprobePath)) @@ -169,8 +169,8 @@ namespace MediaBrowser.MediaEncoding.Encoder config.EncoderAppPath = newPath; ConfigurationManager.SaveConfiguration("encoding", config); - // Trigger Init so we validate the new path and setup probe path - Init(); + // Trigger SetFFmpegPath so we validate the new path and setup probe path + SetFFmpegPath(); } ///