From e2d9d592bcf8ee0d6391d093f8e5d428145e9b6b Mon Sep 17 00:00:00 2001 From: theguymadmax Date: Sat, 11 Jul 2026 17:33:06 -0400 Subject: Fix incorrect protocol used for subtitle charset detection --- MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs') diff --git a/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs b/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs index 67e323177b..5301f52e01 100644 --- a/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs +++ b/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs @@ -167,7 +167,7 @@ namespace MediaBrowser.MediaEncoding.Subtitles { if (fileInfo.Protocol == MediaProtocol.Http) { - var result = await DetectCharset(fileInfo.Path, fileInfo.Protocol, cancellationToken).ConfigureAwait(false); + var result = await DetectCharset(fileInfo.Path, cancellationToken).ConfigureAwait(false); var detected = result.Detected; if (detected is not null) @@ -1104,7 +1104,7 @@ namespace MediaBrowser.MediaEncoding.Subtitles } } - var result = await DetectCharset(path, mediaSource.Protocol, cancellationToken).ConfigureAwait(false); + var result = await DetectCharset(path, cancellationToken).ConfigureAwait(false); var charset = result.Detected?.EncodingName ?? string.Empty; // UTF16 is automatically converted to UTF8 by FFmpeg, do not specify a character encoding @@ -1120,8 +1120,9 @@ namespace MediaBrowser.MediaEncoding.Subtitles return charset; } - private async Task DetectCharset(string path, MediaProtocol protocol, CancellationToken cancellationToken) + private async Task DetectCharset(string path, CancellationToken cancellationToken) { + var protocol = _mediaSourceManager.GetPathProtocol(path); switch (protocol) { case MediaProtocol.Http: @@ -1141,7 +1142,7 @@ namespace MediaBrowser.MediaEncoding.Subtitles } default: - throw new ArgumentOutOfRangeException(nameof(protocol), protocol, "Unsupported protocol"); + throw new NotSupportedException($"Unsupported protocol: {protocol}"); } } -- cgit v1.2.3 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. --- .../Subtitles/ISubtitleParser.cs | 6 +- .../Subtitles/SubtitleEditParser.cs | 20 +---- .../Subtitles/SubtitleEncoder.cs | 4 +- .../Subtitles/AssParserTests.cs | 12 +-- .../Subtitles/SrtParserTests.cs | 52 ++++++------ .../Subtitles/SsaParserTests.cs | 24 +++--- .../Subtitles/SubtitleEncoderTests.cs | 94 ++++++++++++++++++++++ 7 files changed, 145 insertions(+), 67 deletions(-) (limited to 'MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs') diff --git a/MediaBrowser.MediaEncoding/Subtitles/ISubtitleParser.cs b/MediaBrowser.MediaEncoding/Subtitles/ISubtitleParser.cs index bd13437fb6..7566616f70 100644 --- a/MediaBrowser.MediaEncoding/Subtitles/ISubtitleParser.cs +++ b/MediaBrowser.MediaEncoding/Subtitles/ISubtitleParser.cs @@ -1,7 +1,7 @@ #pragma warning disable CS1591 using System.IO; -using MediaBrowser.Model.MediaInfo; +using Nikse.SubtitleEdit.Core.Common; namespace MediaBrowser.MediaEncoding.Subtitles { @@ -12,8 +12,8 @@ namespace MediaBrowser.MediaEncoding.Subtitles /// /// The stream. /// The file extension. - /// SubtitleTrackInfo. - SubtitleTrackInfo Parse(Stream stream, string fileExtension); + /// The parsed subtitle. + Subtitle Parse(Stream stream, string fileExtension); /// /// Determines whether the file extension is supported by the parser. diff --git a/MediaBrowser.MediaEncoding/Subtitles/SubtitleEditParser.cs b/MediaBrowser.MediaEncoding/Subtitles/SubtitleEditParser.cs index d060b247da..d75eea5904 100644 --- a/MediaBrowser.MediaEncoding/Subtitles/SubtitleEditParser.cs +++ b/MediaBrowser.MediaEncoding/Subtitles/SubtitleEditParser.cs @@ -1,10 +1,8 @@ using System; using System.Collections.Generic; -using System.Globalization; using System.IO; using System.Linq; using Jellyfin.Extensions; -using MediaBrowser.Model.MediaInfo; using Microsoft.Extensions.Logging; using Nikse.SubtitleEdit.Core.Common; using SubtitleFormat = Nikse.SubtitleEdit.Core.SubtitleFormats.SubtitleFormat; @@ -30,7 +28,7 @@ namespace MediaBrowser.MediaEncoding.Subtitles } /// - public SubtitleTrackInfo Parse(Stream stream, string fileExtension) + public Subtitle Parse(Stream stream, string fileExtension) { var subtitle = new Subtitle(); var lines = stream.ReadAllLines().ToList(); @@ -76,21 +74,7 @@ namespace MediaBrowser.MediaEncoding.Subtitles throw new ArgumentException("Unsupported format: " + fileExtension); } - var trackInfo = new SubtitleTrackInfo(); - int len = subtitle.Paragraphs.Count; - var trackEvents = new SubtitleTrackEvent[len]; - for (int i = 0; i < len; i++) - { - var p = subtitle.Paragraphs[i]; - trackEvents[i] = new SubtitleTrackEvent(p.Number.ToString(CultureInfo.InvariantCulture), p.Text) - { - StartPositionTicks = p.StartTime.TimeSpan.Ticks, - EndPositionTicks = p.EndTime.TimeSpan.Ticks - }; - } - - trackInfo.TrackEvents = trackEvents; - return trackInfo; + return subtitle; } /// 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); diff --git a/tests/Jellyfin.MediaEncoding.Tests/Subtitles/AssParserTests.cs b/tests/Jellyfin.MediaEncoding.Tests/Subtitles/AssParserTests.cs index 1f908d7e0e..b03651e5e9 100644 --- a/tests/Jellyfin.MediaEncoding.Tests/Subtitles/AssParserTests.cs +++ b/tests/Jellyfin.MediaEncoding.Tests/Subtitles/AssParserTests.cs @@ -15,13 +15,13 @@ namespace Jellyfin.MediaEncoding.Subtitles.Tests using var stream = File.OpenRead("Test Data/example.ass"); var parsed = new SubtitleEditParser(new NullLogger()).Parse(stream, "ass"); - Assert.Single(parsed.TrackEvents); - var trackEvent = parsed.TrackEvents[0]; + Assert.Single(parsed.Paragraphs); + var paragraph = parsed.Paragraphs[0]; - Assert.Equal("1", trackEvent.Id); - Assert.Equal(TimeSpan.Parse("00:00:01.18", CultureInfo.InvariantCulture).Ticks, trackEvent.StartPositionTicks); - Assert.Equal(TimeSpan.Parse("00:00:06.85", CultureInfo.InvariantCulture).Ticks, trackEvent.EndPositionTicks); - Assert.Equal("{\\pos(400,570)}Like an Angel with pity on nobody" + Environment.NewLine + "The second line in subtitle", trackEvent.Text); + Assert.Equal(1, paragraph.Number); + Assert.Equal(TimeSpan.Parse("00:00:01.18", CultureInfo.InvariantCulture).Ticks, paragraph.StartTime.TimeSpan.Ticks); + Assert.Equal(TimeSpan.Parse("00:00:06.85", CultureInfo.InvariantCulture).Ticks, paragraph.EndTime.TimeSpan.Ticks); + Assert.Equal("{\\pos(400,570)}Like an Angel with pity on nobody" + Environment.NewLine + "The second line in subtitle", paragraph.Text); } } } diff --git a/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SrtParserTests.cs b/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SrtParserTests.cs index b7152961cd..01a35e6cb0 100644 --- a/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SrtParserTests.cs +++ b/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SrtParserTests.cs @@ -15,19 +15,19 @@ namespace Jellyfin.MediaEncoding.Subtitles.Tests using var stream = File.OpenRead("Test Data/example.srt"); var parsed = new SubtitleEditParser(new NullLogger()).Parse(stream, "srt"); - Assert.Equal(2, parsed.TrackEvents.Count); - - var trackEvent1 = parsed.TrackEvents[0]; - Assert.Equal("1", trackEvent1.Id); - Assert.Equal(TimeSpan.Parse("00:02:17.440", CultureInfo.InvariantCulture).Ticks, trackEvent1.StartPositionTicks); - Assert.Equal(TimeSpan.Parse("00:02:20.375", CultureInfo.InvariantCulture).Ticks, trackEvent1.EndPositionTicks); - Assert.Equal("Senator, we're making" + Environment.NewLine + "our final approach into Coruscant.", trackEvent1.Text); - - var trackEvent2 = parsed.TrackEvents[1]; - Assert.Equal("2", trackEvent2.Id); - Assert.Equal(TimeSpan.Parse("00:02:20.476", CultureInfo.InvariantCulture).Ticks, trackEvent2.StartPositionTicks); - Assert.Equal(TimeSpan.Parse("00:02:22.501", CultureInfo.InvariantCulture).Ticks, trackEvent2.EndPositionTicks); - Assert.Equal("Very good, Lieutenant.", trackEvent2.Text); + Assert.Equal(2, parsed.Paragraphs.Count); + + var paragraph1 = parsed.Paragraphs[0]; + Assert.Equal(1, paragraph1.Number); + Assert.Equal(TimeSpan.Parse("00:02:17.440", CultureInfo.InvariantCulture).Ticks, paragraph1.StartTime.TimeSpan.Ticks); + Assert.Equal(TimeSpan.Parse("00:02:20.375", CultureInfo.InvariantCulture).Ticks, paragraph1.EndTime.TimeSpan.Ticks); + Assert.Equal("Senator, we're making" + Environment.NewLine + "our final approach into Coruscant.", paragraph1.Text); + + var paragraph2 = parsed.Paragraphs[1]; + Assert.Equal(2, paragraph2.Number); + Assert.Equal(TimeSpan.Parse("00:02:20.476", CultureInfo.InvariantCulture).Ticks, paragraph2.StartTime.TimeSpan.Ticks); + Assert.Equal(TimeSpan.Parse("00:02:22.501", CultureInfo.InvariantCulture).Ticks, paragraph2.EndTime.TimeSpan.Ticks); + Assert.Equal("Very good, Lieutenant.", paragraph2.Text); } [Fact] @@ -36,19 +36,19 @@ namespace Jellyfin.MediaEncoding.Subtitles.Tests using var stream = File.OpenRead("Test Data/example2.srt"); var parsed = new SubtitleEditParser(new NullLogger()).Parse(stream, "srt"); - Assert.Equal(2, parsed.TrackEvents.Count); - - var trackEvent1 = parsed.TrackEvents[0]; - Assert.Equal("311", trackEvent1.Id); - Assert.Equal(TimeSpan.Parse("00:16:46.465", CultureInfo.InvariantCulture).Ticks, trackEvent1.StartPositionTicks); - Assert.Equal(TimeSpan.Parse("00:16:49.009", CultureInfo.InvariantCulture).Ticks, trackEvent1.EndPositionTicks); - Assert.Equal("Una vez que la gente se entere" + Environment.NewLine + Environment.NewLine + "de que ustedes están aquí,", trackEvent1.Text); - - var trackEvent2 = parsed.TrackEvents[1]; - Assert.Equal("312", trackEvent2.Id); - Assert.Equal(TimeSpan.Parse("00:16:49.092", CultureInfo.InvariantCulture).Ticks, trackEvent2.StartPositionTicks); - Assert.Equal(TimeSpan.Parse("00:16:51.470", CultureInfo.InvariantCulture).Ticks, trackEvent2.EndPositionTicks); - Assert.Equal("este lugar se convertirá" + Environment.NewLine + Environment.NewLine + "en un maldito zoológico.", trackEvent2.Text); + Assert.Equal(2, parsed.Paragraphs.Count); + + var paragraph1 = parsed.Paragraphs[0]; + Assert.Equal(311, paragraph1.Number); + Assert.Equal(TimeSpan.Parse("00:16:46.465", CultureInfo.InvariantCulture).Ticks, paragraph1.StartTime.TimeSpan.Ticks); + Assert.Equal(TimeSpan.Parse("00:16:49.009", CultureInfo.InvariantCulture).Ticks, paragraph1.EndTime.TimeSpan.Ticks); + Assert.Equal("Una vez que la gente se entere" + Environment.NewLine + Environment.NewLine + "de que ustedes están aquí,", paragraph1.Text); + + var paragraph2 = parsed.Paragraphs[1]; + Assert.Equal(312, paragraph2.Number); + Assert.Equal(TimeSpan.Parse("00:16:49.092", CultureInfo.InvariantCulture).Ticks, paragraph2.StartTime.TimeSpan.Ticks); + Assert.Equal(TimeSpan.Parse("00:16:51.470", CultureInfo.InvariantCulture).Ticks, paragraph2.EndTime.TimeSpan.Ticks); + Assert.Equal("este lugar se convertirá" + Environment.NewLine + Environment.NewLine + "en un maldito zoológico.", paragraph2.Text); } } } diff --git a/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SsaParserTests.cs b/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SsaParserTests.cs index 5b7aa7eaa9..d814088593 100644 --- a/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SsaParserTests.cs +++ b/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SsaParserTests.cs @@ -20,19 +20,19 @@ namespace Jellyfin.MediaEncoding.Subtitles.Tests { using Stream stream = new MemoryStream(Encoding.UTF8.GetBytes(ssa)); - SubtitleTrackInfo subtitleTrackInfo = _parser.Parse(stream, "ssa"); + var subtitle = _parser.Parse(stream, "ssa"); - Assert.Equal(expectedSubtitleTrackEvents.Count, subtitleTrackInfo.TrackEvents.Count); + Assert.Equal(expectedSubtitleTrackEvents.Count, subtitle.Paragraphs.Count); for (int i = 0; i < expectedSubtitleTrackEvents.Count; ++i) { SubtitleTrackEvent expected = expectedSubtitleTrackEvents[i]; - SubtitleTrackEvent actual = subtitleTrackInfo.TrackEvents[i]; + var actual = subtitle.Paragraphs[i]; - Assert.Equal(expected.Id, actual.Id); + Assert.Equal(expected.Id, actual.Number.ToString(CultureInfo.InvariantCulture)); Assert.Equal(expected.Text, actual.Text); - Assert.Equal(expected.StartPositionTicks, actual.StartPositionTicks); - Assert.Equal(expected.EndPositionTicks, actual.EndPositionTicks); + Assert.Equal(expected.StartPositionTicks, actual.StartTime.TimeSpan.Ticks); + Assert.Equal(expected.EndPositionTicks, actual.EndTime.TimeSpan.Ticks); } } @@ -75,13 +75,13 @@ namespace Jellyfin.MediaEncoding.Subtitles.Tests using var stream = File.OpenRead("Test Data/example.ssa"); var parsed = _parser.Parse(stream, "ssa"); - Assert.Single(parsed.TrackEvents); - var trackEvent = parsed.TrackEvents[0]; + Assert.Single(parsed.Paragraphs); + var paragraph = parsed.Paragraphs[0]; - Assert.Equal("1", trackEvent.Id); - Assert.Equal(TimeSpan.Parse("00:00:01.18", CultureInfo.InvariantCulture).Ticks, trackEvent.StartPositionTicks); - Assert.Equal(TimeSpan.Parse("00:00:06.85", CultureInfo.InvariantCulture).Ticks, trackEvent.EndPositionTicks); - Assert.Equal("{\\pos(400,570)}Like an angel with pity on nobody", trackEvent.Text); + Assert.Equal(1, paragraph.Number); + Assert.Equal(TimeSpan.Parse("00:00:01.18", CultureInfo.InvariantCulture).Ticks, paragraph.StartTime.TimeSpan.Ticks); + Assert.Equal(TimeSpan.Parse("00:00:06.85", CultureInfo.InvariantCulture).Ticks, paragraph.EndTime.TimeSpan.Ticks); + Assert.Equal("{\\pos(400,570)}Like an angel with pity on nobody", paragraph.Text); } } } diff --git a/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SubtitleEncoderTests.cs b/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SubtitleEncoderTests.cs index ce1f005f40..2d0fa29c9a 100644 --- a/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SubtitleEncoderTests.cs +++ b/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SubtitleEncoderTests.cs @@ -1,3 +1,8 @@ +using System; +using System.Globalization; +using System.IO; +using System.Linq; +using System.Text; using System.Threading; using System.Threading.Tasks; using AutoFixture; @@ -6,12 +11,16 @@ using MediaBrowser.MediaEncoding.Subtitles; using MediaBrowser.Model.Dto; using MediaBrowser.Model.Entities; using MediaBrowser.Model.MediaInfo; +using Microsoft.Extensions.Logging.Abstractions; using Xunit; namespace Jellyfin.MediaEncoding.Subtitles.Tests { public class SubtitleEncoderTests { + private const int StreamCount = 8; + private const int CueCount = 500; + public static TheoryData GetReadableFile_Valid_TestData() { var data = new TheoryData(); @@ -103,5 +112,90 @@ namespace Jellyfin.MediaEncoding.Subtitles.Tests Assert.Equal(subtitleInfo.Format, result.Format); Assert.Equal(subtitleInfo.IsExternal, result.IsExternal); } + + [Fact] + public void ConvertSubtitles_SequentialCalls_AreDeterministic() + { + using var encoder = CreateEncoder(); + var sources = GenerateSources(); + + var first = ConvertAllSequential(encoder, sources); + var second = ConvertAllSequential(encoder, sources); + + for (var i = 0; i < StreamCount; i++) + { + Assert.Contains($"S{i}C{CueCount - 1}", first[i], StringComparison.Ordinal); + Assert.Equal(first[i], second[i]); + } + } + + [Fact] + public async Task ConvertSubtitles_ConcurrentCalls_MatchSequentialBaseline() + { + const int Iterations = 10; + + using var encoder = CreateEncoder(); + var sources = GenerateSources(); + var baseline = ConvertAllSequential(encoder, sources); + + for (var iteration = 0; iteration < Iterations; iteration++) + { + var results = await Task.WhenAll(Enumerable.Range(0, StreamCount) + .Select(i => Task.Run(() => Convert(encoder, sources[i], i))) + .ToArray()); + + for (var i = 0; i < StreamCount; i++) + { + Assert.True( + string.Equals(baseline[i], results[i], StringComparison.Ordinal), + $"Iteration {iteration}: stream {i} returned corrupted content ({results[i].Length} chars vs {baseline[i].Length} baseline)"); + } + } + } + + private static SubtitleEncoder CreateEncoder() + { + var fixture = new Fixture().Customize(new AutoMoqCustomization { ConfigureMembers = true }); + fixture.Inject(new SubtitleEditParser(NullLogger.Instance)); + return fixture.Create(); + } + + private static byte[][] GenerateSources() + { + return Enumerable.Range(0, StreamCount) + .Select(i => Encoding.UTF8.GetBytes(GenerateSrt(i, CueCount))) + .ToArray(); + } + + private static string Convert(SubtitleEncoder encoder, byte[] source, int streamIndex) + { + using var input = new MemoryStream(source); + var info = new SubtitleEncoder.SubtitleInfo { Path = $"track{streamIndex}.srt", Format = "srt" }; + using var output = encoder.ConvertSubtitles(input, info, "vtt", 0, 0, false); + return Encoding.UTF8.GetString(output.ToArray()); + } + + private static string[] ConvertAllSequential(SubtitleEncoder encoder, byte[][] sources) + { + return sources.Select((source, i) => Convert(encoder, source, i)).ToArray(); + } + + private static string GenerateSrt(int streamIndex, int cueCount) + { + var builder = new StringBuilder(); + for (var i = 0; i < cueCount; i++) + { + var start = TimeSpan.FromSeconds(i * 4); + var end = start + TimeSpan.FromSeconds(2); + builder.Append(i + 1).AppendLine() + .Append(start.ToString(@"hh\:mm\:ss\,fff", CultureInfo.InvariantCulture)) + .Append(" --> ") + .AppendLine(end.ToString(@"hh\:mm\:ss\,fff", CultureInfo.InvariantCulture)) + .Append('S').Append(streamIndex).Append('C').Append(i).AppendLine() + .AppendLine(); + } + + return builder.ToString(); + } } } -- 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 From ed61acc19a958ceaf8de6f6f0f481b2b5370463a Mon Sep 17 00:00:00 2001 From: Tim Eisele Date: Tue, 21 Jul 2026 14:52:59 +0200 Subject: Fix subtitle encoding for local files (#17281) * Fix subtitle encoding * Add short-circuit * Use IsTextFormat * Update MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs Co-authored-by: Bond-009 --------- Co-authored-by: Bond-009 --- .../Subtitles/SubtitleEncoder.cs | 34 +-- .../Subtitles/SubtitleEncoderTests.cs | 244 +++++++++++++++------ 2 files changed, 201 insertions(+), 77 deletions(-) (limited to 'MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs') diff --git a/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs b/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs index c568a74b01..bd516f0a9f 100644 --- a/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs +++ b/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs @@ -163,28 +163,36 @@ namespace MediaBrowser.MediaEncoding.Subtitles return (stream, fileInfo); } - private async Task GetSubtitleStream(SubtitleInfo fileInfo, CancellationToken cancellationToken) + internal async Task GetSubtitleStream(SubtitleInfo fileInfo, CancellationToken cancellationToken) { - if (fileInfo.Protocol == MediaProtocol.Http) + if (fileInfo.IsExternal && MediaStream.IsTextFormat(fileInfo.Format)) { var result = await DetectCharset(fileInfo.Path, cancellationToken).ConfigureAwait(false); var detected = result.Detected; - if (detected is not null) + var stream = fileInfo.Protocol == MediaProtocol.Http + ? await _httpClientFactory.CreateClient(NamedClient.Default) + .GetStreamAsync(new Uri(fileInfo.Path), cancellationToken) + .ConfigureAwait(false) + : AsyncFile.OpenRead(fileInfo.Path); + + // Short-circuit when the file is already UTF-8/ASCII. + if (detected is null + || string.Equals(detected.EncodingName, "utf-8", StringComparison.OrdinalIgnoreCase) + || string.Equals(detected.EncodingName, "ascii", StringComparison.OrdinalIgnoreCase) + || string.Equals(detected.EncodingName, "us-ascii", StringComparison.OrdinalIgnoreCase)) { - _logger.LogDebug("charset {CharSet} detected for {Path}", detected.EncodingName, fileInfo.Path); + return stream; + } - using var stream = await _httpClientFactory.CreateClient(NamedClient.Default) - .GetStreamAsync(new Uri(fileInfo.Path), cancellationToken) - .ConfigureAwait(false); + _logger.LogDebug("charset {CharSet} detected for {Path}", detected.EncodingName, fileInfo.Path); - await using (stream.ConfigureAwait(false)) - { - using var reader = new StreamReader(stream, detected.Encoding); - var text = await reader.ReadToEndAsync(cancellationToken).ConfigureAwait(false); + await using (stream.ConfigureAwait(false)) + { + using var reader = new StreamReader(stream, detected.Encoding); + var text = await reader.ReadToEndAsync(cancellationToken).ConfigureAwait(false); - return new MemoryStream(Encoding.UTF8.GetBytes(text)); - } + return new MemoryStream(Encoding.UTF8.GetBytes(text)); } } diff --git a/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SubtitleEncoderTests.cs b/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SubtitleEncoderTests.cs index 2d0fa29c9a..48850b2f67 100644 --- a/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SubtitleEncoderTests.cs +++ b/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SubtitleEncoderTests.cs @@ -21,81 +21,110 @@ namespace Jellyfin.MediaEncoding.Subtitles.Tests private const int StreamCount = 8; private const int CueCount = 500; - public static TheoryData GetReadableFile_Valid_TestData() + // A Greek line that requires a non-UTF-8 legacy encoding to reproduce the bug. The accented + // characters (ά, έ, ή, ί, ό, ύ, ώ) share the same code points in windows-1253 and iso-8859-7, + // so a Greek-vs-Greek charset misdetection still round-trips correctly. + private const string GreekText = "Καλημέρα κόσμε, αυτό είναι ένας υπότιτλος."; + + static SubtitleEncoderTests() { - var data = new TheoryData(); + // Mirrors Jellyfin.Server startup so legacy code pages (e.g. Greek windows-1253) are available. + Encoding.RegisterProvider(CodePagesEncodingProvider.Instance); + } - data.Add( - new MediaSourceInfo() - { - Protocol = MediaProtocol.File - }, - new MediaStream() - { - Path = "/media/sub.ass", - IsExternal = true - }, - new SubtitleEncoder.SubtitleInfo() - { - Path = "/media/sub.ass", - Protocol = MediaProtocol.File, - Format = "ass", - IsExternal = true - }); + // Enough Greek text to give the charset detector a strong, unambiguous signal. + private static string BuildGreekSrt() + { + var builder = new StringBuilder(); + for (var i = 1; i <= 8; i++) + { + builder.Append(i.ToString(CultureInfo.InvariantCulture)).Append('\n'); + builder.Append("00:00:0").Append(i.ToString(CultureInfo.InvariantCulture)) + .Append(",000 --> 00:00:0").Append((i + 1).ToString(CultureInfo.InvariantCulture)).Append(",000\n"); + builder.Append(GreekText).Append('\n'); + builder.Append("Η γρήγορη καφέ αλεπού πηδάει πάνω από το τεμπέλικο σκυλί.\n\n"); + } - data.Add( - new MediaSourceInfo() - { - Protocol = MediaProtocol.File - }, - new MediaStream() - { - Path = "/media/sub.ssa", - IsExternal = true - }, - new SubtitleEncoder.SubtitleInfo() - { - Path = "/media/sub.ssa", - Protocol = MediaProtocol.File, - Format = "ssa", - IsExternal = true - }); + return builder.ToString(); + } - data.Add( - new MediaSourceInfo() + public static TheoryData GetReadableFile_Valid_TestData() + { + var data = new TheoryData + { { - Protocol = MediaProtocol.File + new MediaSourceInfo() + { + Protocol = MediaProtocol.File + }, + new MediaStream() + { + Path = "/media/sub.ass", + IsExternal = true + }, + new SubtitleEncoder.SubtitleInfo() + { + Path = "/media/sub.ass", + Protocol = MediaProtocol.File, + Format = "ass", + IsExternal = true + } }, - new MediaStream() { - Path = "/media/sub.srt", - IsExternal = true + new MediaSourceInfo() + { + Protocol = MediaProtocol.File + }, + new MediaStream() + { + Path = "/media/sub.ssa", + IsExternal = true + }, + new SubtitleEncoder.SubtitleInfo() + { + Path = "/media/sub.ssa", + Protocol = MediaProtocol.File, + Format = "ssa", + IsExternal = true + } }, - new SubtitleEncoder.SubtitleInfo() { - Path = "/media/sub.srt", - Protocol = MediaProtocol.File, - Format = "srt", - IsExternal = true - }); - - data.Add( - new MediaSourceInfo() - { - Protocol = MediaProtocol.Http + new MediaSourceInfo() + { + Protocol = MediaProtocol.File + }, + new MediaStream() + { + Path = "/media/sub.srt", + IsExternal = true + }, + new SubtitleEncoder.SubtitleInfo() + { + Path = "/media/sub.srt", + Protocol = MediaProtocol.File, + Format = "srt", + IsExternal = true + } }, - new MediaStream() { - Path = "/media/sub.ass", - IsExternal = true - }, - new SubtitleEncoder.SubtitleInfo() - { - Path = "/media/sub.ass", - Protocol = MediaProtocol.File, - Format = "ass", - IsExternal = true - }); + new MediaSourceInfo() + { + Protocol = MediaProtocol.Http + }, + new MediaStream() + { + Path = "/media/sub.ass", + IsExternal = true + }, + new SubtitleEncoder.SubtitleInfo() + { + Path = "/media/sub.ass", + Protocol = MediaProtocol.File, + Format = "ass", + IsExternal = true + } + } + }; return data; } @@ -113,6 +142,55 @@ namespace Jellyfin.MediaEncoding.Subtitles.Tests Assert.Equal(subtitleInfo.IsExternal, result.IsExternal); } + public static TheoryData GetSubtitleStream_NonUtf8LocalFile_TestData() + { + return + [ + // Greek legacy encodings – the exact scenario reported in issue #17267. + Encoding.GetEncoding("windows-1253"), + Encoding.GetEncoding("iso-8859-7"), + // Wide encoding with a BOM. + new UnicodeEncoding(bigEndian: false, byteOrderMark: true), + ]; + } + + [Theory] + [MemberData(nameof(GetSubtitleStream_NonUtf8LocalFile_TestData))] + public async Task GetSubtitleStream_NonUtf8LocalFile_ConvertedToUtf8(Encoding sourceEncoding) + { + var cancellationToken = TestContext.Current.CancellationToken; + var srt = BuildGreekSrt(); + var path = Path.GetTempFileName(); + try + { + await File.WriteAllTextAsync(path, srt, sourceEncoding, cancellationToken); + + var fixture = new Fixture().Customize(new AutoMoqCustomization { ConfigureMembers = true }); + var subtitleEncoder = fixture.Create(); + + var fileInfo = new SubtitleEncoder.SubtitleInfo + { + Path = path, + Protocol = MediaProtocol.File, + Format = "srt", + IsExternal = true + }; + + using var stream = await subtitleEncoder.GetSubtitleStream(fileInfo, cancellationToken); + using var reader = new StreamReader(stream, new UTF8Encoding(false)); + var text = await reader.ReadToEndAsync(cancellationToken); + + // The Greek text must survive round-trip and contain no replacement characters. + Assert.Contains(GreekText, text, StringComparison.Ordinal); + Assert.DoesNotContain('�', text); + Assert.DoesNotContain('?', text); + } + finally + { + File.Delete(path); + } + } + [Fact] public void ConvertSubtitles_SequentialCalls_AreDeterministic() { @@ -129,6 +207,44 @@ namespace Jellyfin.MediaEncoding.Subtitles.Tests } } + [Fact] + public async Task GetSubtitleStream_Utf8LocalFile_PreservesContent() + { + var cancellationToken = TestContext.Current.CancellationToken; + var srt = BuildGreekSrt(); + var path = Path.GetTempFileName(); + try + { + await File.WriteAllTextAsync(path, srt, new UTF8Encoding(false), cancellationToken); + + var fixture = new Fixture().Customize(new AutoMoqCustomization { ConfigureMembers = true }); + var subtitleEncoder = fixture.Create(); + + var fileInfo = new SubtitleEncoder.SubtitleInfo + { + Path = path, + Protocol = MediaProtocol.File, + Format = "srt", + IsExternal = true + }; + + using var stream = await subtitleEncoder.GetSubtitleStream(fileInfo, cancellationToken); + + // An already-UTF-8 file must be short-circuited and served directly from disk, + // not read into memory and re-encoded (which would produce a MemoryStream). + Assert.IsNotType(stream); + + using var reader = new StreamReader(stream, new UTF8Encoding(false)); + var text = await reader.ReadToEndAsync(cancellationToken); + + Assert.Contains(GreekText, text, StringComparison.Ordinal); + } + finally + { + File.Delete(path); + } + } + [Fact] public async Task ConvertSubtitles_ConcurrentCalls_MatchSequentialBaseline() { -- cgit v1.2.3