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 <bond.009@outlook.com>

---------

Co-authored-by: Bond-009 <bond.009@outlook.com>

2 files changed, 201 insertions, 77 deletions

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<Stream> GetSubtitleStream(SubtitleInfo fileInfo, CancellationToken cancellationToken)
+        internal async Task<Stream> 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<MediaSourceInfo, MediaStream, SubtitleEncoder.SubtitleInfo> 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<MediaSourceInfo, MediaStream, SubtitleEncoder.SubtitleInfo>();
+            // 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<MediaSourceInfo, MediaStream, SubtitleEncoder.SubtitleInfo> GetReadableFile_Valid_TestData()
+        {
+            var data = new TheoryData<MediaSourceInfo, MediaStream, SubtitleEncoder.SubtitleInfo>
+            {
                 {
-                    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<Encoding> 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<SubtitleEncoder>();
+
+                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()
         {
@@ -130,6 +208,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<SubtitleEncoder>();
+
+                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<MemoryStream>(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()
         {
             const int Iterations = 10;