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>
This commit is contained in:
Tim Eisele
@@ -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)
|
||||
{
|
||||
_logger.LogDebug("charset {CharSet} detected for {Path}", detected.EncodingName, fileInfo.Path);
|
||||
|
||||
using var stream = await _httpClientFactory.CreateClient(NamedClient.Default)
|
||||
var stream = fileInfo.Protocol == MediaProtocol.Http
|
||||
? await _httpClientFactory.CreateClient(NamedClient.Default)
|
||||
.GetStreamAsync(new Uri(fileInfo.Path), cancellationToken)
|
||||
.ConfigureAwait(false);
|
||||
.ConfigureAwait(false)
|
||||
: AsyncFile.OpenRead(fileInfo.Path);
|
||||
|
||||
await using (stream.ConfigureAwait(false))
|
||||
{
|
||||
using var reader = new StreamReader(stream, detected.Encoding);
|
||||
var text = await reader.ReadToEndAsync(cancellationToken).ConfigureAwait(false);
|
||||
// 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))
|
||||
{
|
||||
return stream;
|
||||
}
|
||||
|
||||
return new MemoryStream(Encoding.UTF8.GetBytes(text));
|
||||
}
|
||||
_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);
|
||||
|
||||
return new MemoryStream(Encoding.UTF8.GetBytes(text));
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -21,81 +21,110 @@ namespace Jellyfin.MediaEncoding.Subtitles.Tests
|
||||
private const int StreamCount = 8;
|
||||
private const int CueCount = 500;
|
||||
|
||||
// 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()
|
||||
{
|
||||
// Mirrors Jellyfin.Server startup so legacy code pages (e.g. Greek windows-1253) are available.
|
||||
Encoding.RegisterProvider(CodePagesEncodingProvider.Instance);
|
||||
}
|
||||
|
||||
// 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");
|
||||
}
|
||||
|
||||
return builder.ToString();
|
||||
}
|
||||
|
||||
public static TheoryData<MediaSourceInfo, MediaStream, SubtitleEncoder.SubtitleInfo> GetReadableFile_Valid_TestData()
|
||||
{
|
||||
var data = new TheoryData<MediaSourceInfo, MediaStream, SubtitleEncoder.SubtitleInfo>();
|
||||
|
||||
data.Add(
|
||||
new MediaSourceInfo()
|
||||
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.ass",
|
||||
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.ass",
|
||||
Protocol = MediaProtocol.File,
|
||||
Format = "ass",
|
||||
IsExternal = true
|
||||
});
|
||||
|
||||
data.Add(
|
||||
new MediaSourceInfo()
|
||||
{
|
||||
Protocol = MediaProtocol.File
|
||||
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.ssa",
|
||||
IsExternal = true
|
||||
},
|
||||
new SubtitleEncoder.SubtitleInfo()
|
||||
{
|
||||
Path = "/media/sub.ssa",
|
||||
Protocol = MediaProtocol.File,
|
||||
Format = "ssa",
|
||||
IsExternal = true
|
||||
});
|
||||
|
||||
data.Add(
|
||||
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
|
||||
});
|
||||
|
||||
data.Add(
|
||||
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
|
||||
});
|
||||
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()
|
||||
{
|
||||
@@ -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<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()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user