From 6e3c1874936b767754cd9267749f35b27f2ae65f Mon Sep 17 00:00:00 2001 From: Piotr Niełacny Date: Wed, 15 Jul 2026 14:55:55 +0200 Subject: Fix race condition in concurrent subtitle conversion SubtitleEncoder.ConvertSubtitles parsed subtitles with libse's static Subtitle.Parse, which iterates a statically cached list of shared SubtitleFormat instances. Format parsers keep mutable per-parse state on the instance, so concurrent subtitle requests corrupted each other's output (cues mixed across streams and languages, truncated files) or failed with NullReferenceException when format detection broke down and Subtitle.Parse returned null. Parse through the injected ISubtitleParser instead. SubtitleEditParser instantiates a fresh format parser per call, so requests no longer share state. Its Parse method now returns the libse Subtitle directly (the SubtitleTrackInfo flattening was unused since the SubtitleEdit writer rework) so the writers keep full fidelity such as ASS styling. --- MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs') diff --git a/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs b/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs index 5301f52e01..974dded9ea 100644 --- a/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs +++ b/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs @@ -73,7 +73,7 @@ namespace MediaBrowser.MediaEncoding.Subtitles _serverConfigurationManager = serverConfigurationManager; } - private MemoryStream ConvertSubtitles( + internal MemoryStream ConvertSubtitles( Stream stream, SubtitleInfo inputInfo, string outputFormat, @@ -81,7 +81,7 @@ namespace MediaBrowser.MediaEncoding.Subtitles long endTimeTicks, bool preserveOriginalTimestamps) { - var subtitle = Subtitle.Parse(stream, Path.GetExtension(inputInfo.Path)); + var subtitle = _subtitleParser.Parse(stream, inputInfo.Format); FilterEvents(subtitle, startTimeTicks, endTimeTicks, preserveOriginalTimestamps); -- cgit v1.2.3 From cab108a8395bca7afbc0f0b587b55c13d4bc0d2a Mon Sep 17 00:00:00 2001 From: Marc Brooks Date: Fri, 17 Jul 2026 16:23:57 -0500 Subject: 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. --- .../Subtitles/SubtitleEncoder.cs | 240 +++++---------------- 1 file changed, 57 insertions(+), 183 deletions(-) (limited to 'MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs') diff --git a/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs b/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs index 5301f52e01..28f7b92268 100644 --- a/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs +++ b/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs @@ -445,98 +445,15 @@ namespace MediaBrowser.MediaEncoding.Subtitles encodingParam = " -sub_charenc " + encodingParam; } - int exitCode; - - using (var process = new Process - { - StartInfo = new ProcessStartInfo - { - CreateNoWindow = true, - 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; + var args = string.Format(CultureInfo.InvariantCulture, "-y {0} -i \"{1}\" -c:s srt \"{2}\"", encodingParam, inputPath, outputPath); - 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); + await ExtractSubtitlesForFile( + inputPath, + args, + [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(); 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 outputPaths, + IReadOnlyList outputPaths, CancellationToken cancellationToken) { - int exitCode; - - 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 (exitCode, ffmpegError) = await RunSubtitleExtractionProcess(args, cancellationToken).ConfigureAwait(false); 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); + } + + /// + /// Runs ffmpeg to extract or convert subtitles, capturing its exit code and stderr output. + /// + /// + /// stdin is redirected and closed, and -nostdin 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. + /// + /// The ffmpeg command line arguments. + /// The cancellation token. + /// The ffmpeg exit code (-1 on timeout) and its captured stderr output. + 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(TimeSpan.FromMinutes(timeoutMinutes)).ConfigureAwait(false); + await process.WaitForExitAsync(waitSource.Token).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); - _fileSystem.DeleteFile(outputPath); - } - catch (FileNotFoundException) - { - } - catch (IOException ex) - { - _logger.LogError(ex, "Error deleting extracted subtitle {Path}", outputPath); + standardError = await standardErrorTask.ConfigureAwait(false); } - } - 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) + catch (OperationCanceledException) { - _logger.LogError(ex, "Error deleting extracted subtitle {Path}", outputPath); + // Reading ffmpeg output was cancelled; nothing more to capture. } } - if (failed) - { - _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); - } + return (exitCode, standardError); } /// -- cgit v1.2.3