Prevent ffmpeg from hanging extracting subtitles (#17297)

* Prevent ffmpeg from hanging extracting subtitles
Add `RunSubtitleExtractionProcess` to unify the external
_ffmpeg_ process handling and error management.
Add a `-nostdin` flag that prevents _ffmpeg_ from reading from
_stdin_ and blocking on an inherited stdin handle (e.g. when
Jellyfin runs as a service under NSSM), which otherwise hangs
subtitle extraction forever when _ffmpeg_ blocks on any
keyboard-interaction read until the timeout (30 minutes).
Close the redirected _stdin_ to ensure immediage EOF.
Drain the _stderr_ to a string and log it, to ensure we don't block
the _ffmpeg_ process on errors that exceed the pipe length.
Pass `-y` to _ffmpeg_ to ensure it overwrites any existing output file
without prompting for confirmation.

* Address review comments
Make sure we always drain stderr.
Make sure the timeout also honors the cancellationToken.
Make sure when we get cancelled we don't log it as a ffmpeg error.

MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs

@@ -445,98 +445,15 @@ namespace MediaBrowser.MediaEncoding.Subtitles
                 encodingParam = " -sub_charenc " + encodingParam;
             }
 
-            int exitCode;
+            var args = string.Format(CultureInfo.InvariantCulture, "-y {0} -i \"{1}\" -c:s srt \"{2}\"", encodingParam, inputPath, outputPath);
 
-            using (var process = new Process
+            await ExtractSubtitlesForFile(
-            {
+                inputPath,
-                StartInfo = new ProcessStartInfo
+                args,
-                {
+                [outputPath],
-                    CreateNoWindow = true,
+                cancellationToken).ConfigureAwait(false);
-                    UseShellExecute = false,
-                    FileName = _mediaEncoder.EncoderPath,
-                    Arguments = string.Format(CultureInfo.InvariantCulture, "-y {0} -i \"{1}\" -c:s srt \"{2}\"", encodingParam, inputPath, outputPath),
-                    WindowStyle = ProcessWindowStyle.Hidden,
-                    ErrorDialog = false
-                },
-                EnableRaisingEvents = true
-            })
-            {
-                _logger.LogInformation("{0} {1}", process.StartInfo.FileName, process.StartInfo.Arguments);
-
-                try
-                {
-                    process.Start();
-                }
-                catch (Exception ex)
-                {
-                    _logger.LogError(ex, "Error starting ffmpeg");
-
-                    throw;
-                }
-
-                try
-                {
-                    var timeoutMinutes = _serverConfigurationManager.GetEncodingOptions().SubtitleExtractionTimeoutMinutes;
-                    await process.WaitForExitAsync(TimeSpan.FromMinutes(timeoutMinutes)).ConfigureAwait(false);
-                    exitCode = process.ExitCode;
-                }
-                catch (OperationCanceledException)
-                {
-                    process.Kill(true);
-                    exitCode = -1;
-                }
-            }
-
-            var failed = false;
-
-            if (exitCode == -1)
-            {
-                failed = true;
-
-                if (File.Exists(outputPath))
-                {
-                    try
-                    {
-                        _logger.LogInformation("Deleting converted subtitle due to failure: {Path}", outputPath);
-                        _fileSystem.DeleteFile(outputPath);
-                    }
-                    catch (IOException ex)
-                    {
-                        _logger.LogError(ex, "Error deleting converted subtitle {Path}", outputPath);
-                    }
-                }
-            }
-            else if (!File.Exists(outputPath) || _fileSystem.GetFileInfo(outputPath).Length == 0)
-            {
-                failed = true;
-
-                try
-                {
-                    _logger.LogWarning("Deleting converted subtitle due to failure: {Path}", outputPath);
-                    _fileSystem.DeleteFile(outputPath);
-                }
-                catch (FileNotFoundException)
-                {
-                }
-                catch (IOException ex)
-                {
-                    _logger.LogError(ex, "Error deleting converted subtitle {Path}", outputPath);
-                }
-            }
-
-            if (failed)
-            {
-                _logger.LogError("ffmpeg subtitle conversion failed for {Path}", inputPath);
-
-                throw new FfmpegException(
-                    string.Format(CultureInfo.InvariantCulture, "ffmpeg subtitle conversion failed for {0}", inputPath));
-            }
-
-            await SetAssFont(outputPath, cancellationToken).ConfigureAwait(false);
 
             WriteCacheMeta(outputPath, inputPath);
-
-            _logger.LogInformation("ffmpeg subtitle conversion succeeded for {Path}", inputPath);
         }
 
         private string GetExtractableSubtitleFormat(MediaStream subtitleStream)
@@ -727,7 +644,7 @@ namespace MediaBrowser.MediaEncoding.Subtitles
             var outputPaths = new List<string>();
             var args = string.Format(
                 CultureInfo.InvariantCulture,
-                "-i {0}",
+                "-y -i {0}",
                 inputPath);
 
             foreach (var subtitleStream in subtitleStreams)
@@ -781,50 +698,10 @@ namespace MediaBrowser.MediaEncoding.Subtitles
         private async Task ExtractSubtitlesForFile(
             string inputPath,
             string args,
-            List<string> outputPaths,
+            IReadOnlyList<string> outputPaths,
             CancellationToken cancellationToken)
         {
-            int exitCode;
+            var (exitCode, ffmpegError) = await RunSubtitleExtractionProcess(args, cancellationToken).ConfigureAwait(false);
-
-            using (var process = new Process
-            {
-                StartInfo = new ProcessStartInfo
-                {
-                    CreateNoWindow = true,
-                    UseShellExecute = false,
-                    FileName = _mediaEncoder.EncoderPath,
-                    Arguments = args,
-                    WindowStyle = ProcessWindowStyle.Hidden,
-                    ErrorDialog = false
-                },
-                EnableRaisingEvents = true
-            })
-            {
-                _logger.LogInformation("{File} {Arguments}", process.StartInfo.FileName, process.StartInfo.Arguments);
-
-                try
-                {
-                    process.Start();
-                }
-                catch (Exception ex)
-                {
-                    _logger.LogError(ex, "Error starting ffmpeg");
-
-                    throw;
-                }
-
-                try
-                {
-                    var timeoutMinutes = _serverConfigurationManager.GetEncodingOptions().SubtitleExtractionTimeoutMinutes;
-                    await process.WaitForExitAsync(TimeSpan.FromMinutes(timeoutMinutes)).ConfigureAwait(false);
-                    exitCode = process.ExitCode;
-                }
-                catch (OperationCanceledException)
-                {
-                    process.Kill(true);
-                    exitCode = -1;
-                }
-            }
 
             var failed = false;
 
@@ -884,6 +761,13 @@ namespace MediaBrowser.MediaEncoding.Subtitles
 
             if (failed)
             {
+                cancellationToken.ThrowIfCancellationRequested();
+
+                if (!string.IsNullOrWhiteSpace(ffmpegError))
+                {
+                    _logger.LogError("ffmpeg subtitle extraction failed for {InputPath}: {FfmpegOutput}", inputPath, ffmpegError);
+                }
+
                 throw new FfmpegException(
                     string.Format(CultureInfo.InvariantCulture, "ffmpeg subtitle extraction failed for {0}", inputPath));
             }
@@ -941,16 +825,38 @@ namespace MediaBrowser.MediaEncoding.Subtitles
             ArgumentException.ThrowIfNullOrEmpty(outputPath);
 
             Directory.CreateDirectory(Path.GetDirectoryName(outputPath) ?? throw new ArgumentException($"Provided path ({outputPath}) is not valid.", nameof(outputPath)));
-
             var processArgs = string.Format(
                 CultureInfo.InvariantCulture,
-                "-i {0} -copyts -map 0:{1} -an -vn -c:s {2} \"{3}\"",
+                "-y -i {0} -copyts -map 0:{1} -an -vn -c:s {2} \"{3}\"",
                 inputPath,
                 subtitleStreamIndex,
                 outputCodec,
                 outputPath);
 
+            await ExtractSubtitlesForFile(
+                inputPath,
+                processArgs,
+                [outputPath],
+                cancellationToken).ConfigureAwait(false);
+        }
+
+        /// <summary>
+        /// Runs ffmpeg to extract or convert subtitles, capturing its exit code and stderr output.
+        /// </summary>
+        /// <remarks>
+        /// stdin is redirected and closed, and <c>-nostdin</c> is prepended to the arguments, so ffmpeg can never
+        /// block reading an inherited stdin handle (which happens when Jellyfin runs as a service, e.g. under NSSM,
+        /// and stalls subtitle extraction until the timeout). stderr is redirected and drained so a full pipe buffer
+        /// cannot deadlock ffmpeg and so its output can be surfaced on failure; stdout is left un-redirected as it is
+        /// unused for subtitle extraction.
+        /// </remarks>
+        /// <param name="arguments">The ffmpeg command line arguments.</param>
+        /// <param name="cancellationToken">The cancellation token.</param>
+        /// <returns>The ffmpeg exit code (-1 on timeout) and its captured stderr output.</returns>
+        private async Task<(int ExitCode, string StandardError)> RunSubtitleExtractionProcess(string arguments, CancellationToken cancellationToken)
+        {
             int exitCode;
+            var standardError = string.Empty;
 
             using (var process = new Process
             {
@@ -958,8 +864,10 @@ namespace MediaBrowser.MediaEncoding.Subtitles
                 {
                     CreateNoWindow = true,
                     UseShellExecute = false,
+                    RedirectStandardInput = true,
+                    RedirectStandardError = true,
                     FileName = _mediaEncoder.EncoderPath,
-                    Arguments = processArgs,
+                    Arguments = "-nostdin " + arguments,
                     WindowStyle = ProcessWindowStyle.Hidden,
                     ErrorDialog = false
                 },
@@ -975,14 +883,21 @@ namespace MediaBrowser.MediaEncoding.Subtitles
                 catch (Exception ex)
                 {
                     _logger.LogError(ex, "Error starting ffmpeg");
-
                     throw;
                 }
 
+                // Close stdin so ffmpeg observes EOF instead of blocking on an inherited handle.
+                process.StandardInput.Close();
+
+                // Begin draining stderr before waiting for exit; a full stderr pipe buffer would otherwise deadlock ffmpeg.
+                var standardErrorTask = process.StandardError.ReadToEndAsync(CancellationToken.None);
+                var timeoutMinutes = _serverConfigurationManager.GetEncodingOptions().SubtitleExtractionTimeoutMinutes;
+                using var waitSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
+                waitSource.CancelAfter(TimeSpan.FromMinutes(timeoutMinutes));
+
                 try
                 {
-                    var timeoutMinutes = _serverConfigurationManager.GetEncodingOptions().SubtitleExtractionTimeoutMinutes;
+                    await process.WaitForExitAsync(waitSource.Token).ConfigureAwait(false);
-                    await process.WaitForExitAsync(TimeSpan.FromMinutes(timeoutMinutes)).ConfigureAwait(false);
                     exitCode = process.ExitCode;
                 }
                 catch (OperationCanceledException)
@@ -990,59 +905,18 @@ namespace MediaBrowser.MediaEncoding.Subtitles
                     process.Kill(true);
                     exitCode = -1;
                 }
-            }
-
-            var failed = false;
-
-            if (exitCode == -1)
-            {
-                failed = true;
 
                 try
                 {
-                    _logger.LogWarning("Deleting extracted subtitle due to failure: {Path}", outputPath);
+                    standardError = await standardErrorTask.ConfigureAwait(false);
-                    _fileSystem.DeleteFile(outputPath);
                 }
-                catch (FileNotFoundException)
+                catch (OperationCanceledException)
                 {
-                }
+                    // Reading ffmpeg output was cancelled; nothing more to capture.
-                catch (IOException ex)
-                {
-                    _logger.LogError(ex, "Error deleting extracted subtitle {Path}", outputPath);
-                }
-            }
-            else if (!File.Exists(outputPath) || _fileSystem.GetFileInfo(outputPath).Length == 0)
-            {
-                failed = true;
-
-                try
-                {
-                    _logger.LogWarning("Deleting extracted subtitle due to failure: {Path}", outputPath);
-                    _fileSystem.DeleteFile(outputPath);
-                }
-                catch (FileNotFoundException)
-                {
-                }
-                catch (IOException ex)
-                {
-                    _logger.LogError(ex, "Error deleting extracted subtitle {Path}", outputPath);
                 }
             }
 
-            if (failed)
+            return (exitCode, standardError);
-            {
-                _logger.LogError("ffmpeg subtitle extraction failed for {InputPath} to {OutputPath}", inputPath, outputPath);
-
-                throw new FfmpegException(
-                    string.Format(CultureInfo.InvariantCulture, "ffmpeg subtitle extraction failed for {0} to {1}", inputPath, outputPath));
-            }
-
-            _logger.LogInformation("ffmpeg subtitle extraction completed for {InputPath} to {OutputPath}", inputPath, outputPath);
-
-            if (string.Equals(outputCodec, "ass", StringComparison.OrdinalIgnoreCase))
-            {
-                await SetAssFont(outputPath, cancellationToken).ConfigureAwait(false);
-            }
         }
 
         /// <summary>