From 494ed7e4d2af34db10db3af90ec5d045017066c4 Mon Sep 17 00:00:00 2001 From: Joshua Boniface Date: Mon, 1 Aug 2022 20:19:16 -0400 Subject: [PATCH] Revert "Merge pull request #8087 from cvium/generic_subtitleparser" This PR was causing breakage in installs - ref #8198 This reverts commit 7323ccfc232d31797af3ceb8bad93cae1ea0898d, reversing changes made to 77a007a24d5eef1209766d31e2f5038b11d1a8d4. --- .../ApplicationHost.cs | 4 +- .../Subtitles/AssParser.cs | 19 +++++ .../Subtitles/ISubtitleParser.cs | 12 +-- .../Subtitles/SrtParser.cs | 19 +++++ .../Subtitles/SsaParser.cs | 19 +++++ .../Subtitles/SubtitleEditParser.cs | 85 +++---------------- .../Subtitles/SubtitleEncoder.cs | 47 ++++++++-- .../Subtitles/AssParserTests.cs | 2 +- .../Subtitles/SrtParserTests.cs | 4 +- .../Subtitles/SsaParserTests.cs | 6 +- 10 files changed, 119 insertions(+), 98 deletions(-) create mode 100644 MediaBrowser.MediaEncoding/Subtitles/AssParser.cs create mode 100644 MediaBrowser.MediaEncoding/Subtitles/SrtParser.cs create mode 100644 MediaBrowser.MediaEncoding/Subtitles/SsaParser.cs diff --git a/Emby.Server.Implementations/ApplicationHost.cs b/Emby.Server.Implementations/ApplicationHost.cs index 91a16c199d..bc55dc6b48 100644 --- a/Emby.Server.Implementations/ApplicationHost.cs +++ b/Emby.Server.Implementations/ApplicationHost.cs @@ -83,7 +83,6 @@ using MediaBrowser.Controller.SyncPlay; using MediaBrowser.Controller.TV; using MediaBrowser.LocalMetadata.Savers; using MediaBrowser.MediaEncoding.BdInfo; -using MediaBrowser.MediaEncoding.Subtitles; using MediaBrowser.Model.Cryptography; using MediaBrowser.Model.Dlna; using MediaBrowser.Model.Globalization; @@ -635,8 +634,7 @@ namespace Emby.Server.Implementations serviceCollection.AddSingleton(); serviceCollection.AddSingleton(); - serviceCollection.AddSingleton(); - serviceCollection.AddSingleton(); + serviceCollection.AddSingleton(); serviceCollection.AddSingleton(); diff --git a/MediaBrowser.MediaEncoding/Subtitles/AssParser.cs b/MediaBrowser.MediaEncoding/Subtitles/AssParser.cs new file mode 100644 index 0000000000..08ee5c72e5 --- /dev/null +++ b/MediaBrowser.MediaEncoding/Subtitles/AssParser.cs @@ -0,0 +1,19 @@ +using Microsoft.Extensions.Logging; +using Nikse.SubtitleEdit.Core.SubtitleFormats; + +namespace MediaBrowser.MediaEncoding.Subtitles +{ + /// + /// Advanced SubStation Alpha subtitle parser. + /// + public class AssParser : SubtitleEditParser + { + /// + /// Initializes a new instance of the class. + /// + /// The logger. + public AssParser(ILogger logger) : base(logger) + { + } + } +} diff --git a/MediaBrowser.MediaEncoding/Subtitles/ISubtitleParser.cs b/MediaBrowser.MediaEncoding/Subtitles/ISubtitleParser.cs index bd13437fb6..c0023ebf24 100644 --- a/MediaBrowser.MediaEncoding/Subtitles/ISubtitleParser.cs +++ b/MediaBrowser.MediaEncoding/Subtitles/ISubtitleParser.cs @@ -1,6 +1,7 @@ #pragma warning disable CS1591 using System.IO; +using System.Threading; using MediaBrowser.Model.MediaInfo; namespace MediaBrowser.MediaEncoding.Subtitles @@ -11,15 +12,8 @@ namespace MediaBrowser.MediaEncoding.Subtitles /// Parses the specified stream. /// /// The stream. - /// The file extension. + /// The cancellation token. /// SubtitleTrackInfo. - SubtitleTrackInfo Parse(Stream stream, string fileExtension); - - /// - /// Determines whether the file extension is supported by the parser. - /// - /// The file extension. - /// A value indicating whether the file extension is supported. - bool SupportsFileExtension(string fileExtension); + SubtitleTrackInfo Parse(Stream stream, CancellationToken cancellationToken); } } diff --git a/MediaBrowser.MediaEncoding/Subtitles/SrtParser.cs b/MediaBrowser.MediaEncoding/Subtitles/SrtParser.cs new file mode 100644 index 0000000000..78d54ca51f --- /dev/null +++ b/MediaBrowser.MediaEncoding/Subtitles/SrtParser.cs @@ -0,0 +1,19 @@ +using Microsoft.Extensions.Logging; +using Nikse.SubtitleEdit.Core.SubtitleFormats; + +namespace MediaBrowser.MediaEncoding.Subtitles +{ + /// + /// SubRip subtitle parser. + /// + public class SrtParser : SubtitleEditParser + { + /// + /// Initializes a new instance of the class. + /// + /// The logger. + public SrtParser(ILogger logger) : base(logger) + { + } + } +} diff --git a/MediaBrowser.MediaEncoding/Subtitles/SsaParser.cs b/MediaBrowser.MediaEncoding/Subtitles/SsaParser.cs new file mode 100644 index 0000000000..17c2ae40e0 --- /dev/null +++ b/MediaBrowser.MediaEncoding/Subtitles/SsaParser.cs @@ -0,0 +1,19 @@ +using Microsoft.Extensions.Logging; +using Nikse.SubtitleEdit.Core.SubtitleFormats; + +namespace MediaBrowser.MediaEncoding.Subtitles +{ + /// + /// SubStation Alpha subtitle parser. + /// + public class SsaParser : SubtitleEditParser + { + /// + /// Initializes a new instance of the class. + /// + /// The logger. + public SsaParser(ILogger logger) : base(logger) + { + } + } +} diff --git a/MediaBrowser.MediaEncoding/Subtitles/SubtitleEditParser.cs b/MediaBrowser.MediaEncoding/Subtitles/SubtitleEditParser.cs index eb8ff96246..52c1b64677 100644 --- a/MediaBrowser.MediaEncoding/Subtitles/SubtitleEditParser.cs +++ b/MediaBrowser.MediaEncoding/Subtitles/SubtitleEditParser.cs @@ -1,14 +1,12 @@ -using System; -using System.Collections.Generic; using System.Globalization; using System.IO; using System.Linq; -using System.Reflection; +using System.Threading; using Jellyfin.Extensions; using MediaBrowser.Model.MediaInfo; using Microsoft.Extensions.Logging; using Nikse.SubtitleEdit.Core.Common; -using Nikse.SubtitleEdit.Core.SubtitleFormats; +using ILogger = Microsoft.Extensions.Logging.ILogger; using SubtitleFormat = Nikse.SubtitleEdit.Core.SubtitleFormats.SubtitleFormat; namespace MediaBrowser.MediaEncoding.Subtitles @@ -16,57 +14,31 @@ namespace MediaBrowser.MediaEncoding.Subtitles /// /// SubStation Alpha subtitle parser. /// - public class SubtitleEditParser : ISubtitleParser + /// The . + public abstract class SubtitleEditParser : ISubtitleParser + where T : SubtitleFormat, new() { - private readonly ILogger _logger; - private readonly Dictionary _subtitleFormats; + private readonly ILogger _logger; /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// /// The logger. - public SubtitleEditParser(ILogger logger) + protected SubtitleEditParser(ILogger logger) { _logger = logger; - _subtitleFormats = GetSubtitleFormats() - .Where(subtitleFormat => !string.IsNullOrEmpty(subtitleFormat.Extension)) - .GroupBy(subtitleFormat => subtitleFormat.Extension.TrimStart('.'), StringComparer.OrdinalIgnoreCase) - .ToDictionary(g => g.Key, g => g.ToArray(), StringComparer.OrdinalIgnoreCase); } /// - public SubtitleTrackInfo Parse(Stream stream, string fileExtension) + public SubtitleTrackInfo Parse(Stream stream, CancellationToken cancellationToken) { var subtitle = new Subtitle(); + var subRip = new T(); var lines = stream.ReadAllLines().ToList(); - - if (!_subtitleFormats.TryGetValue(fileExtension, out var subtitleFormats)) + subRip.LoadSubtitle(subtitle, lines, "untitled"); + if (subRip.ErrorCount > 0) { - throw new ArgumentException($"Unsupported file extension: {fileExtension}", nameof(fileExtension)); - } - - foreach (var subtitleFormat in subtitleFormats) - { - _logger.LogDebug( - "Trying to parse '{FileExtension}' subtitle using the {SubtitleFormatParser} format parser", - fileExtension, - subtitleFormat.Name); - subtitleFormat.LoadSubtitle(subtitle, lines, fileExtension); - if (subtitleFormat.ErrorCount == 0) - { - break; - } - - _logger.LogError( - "{ErrorCount} errors encountered while parsing '{FileExtension}' subtitle using the {SubtitleFormatParser} format parser", - subtitleFormat.ErrorCount, - fileExtension, - subtitleFormat.Name); - } - - if (subtitle.Paragraphs.Count == 0) - { - throw new ArgumentException("Unsupported format: " + fileExtension); + _logger.LogError("{ErrorCount} errors encountered while parsing subtitle", subRip.ErrorCount); } var trackInfo = new SubtitleTrackInfo(); @@ -85,36 +57,5 @@ namespace MediaBrowser.MediaEncoding.Subtitles trackInfo.TrackEvents = trackEvents; return trackInfo; } - - /// - public bool SupportsFileExtension(string fileExtension) - => _subtitleFormats.ContainsKey(fileExtension); - - private IEnumerable GetSubtitleFormats() - { - var subtitleFormats = new List(); - var assembly = typeof(SubtitleFormat).Assembly; - - foreach (var type in assembly.GetTypes()) - { - if (!type.IsSubclassOf(typeof(SubtitleFormat)) || type.IsAbstract) - { - continue; - } - - try - { - // It shouldn't be null, but the exception is caught if it is - var subtitleFormat = (SubtitleFormat)Activator.CreateInstance(type, true)!; - subtitleFormats.Add(subtitleFormat); - } - catch (Exception ex) - { - _logger.LogWarning(ex, "Failed to create instance of the subtitle format {SubtitleFormatType}", type.Name); - } - } - - return subtitleFormats; - } } } diff --git a/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs b/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs index 50c4d92103..7091af734a 100644 --- a/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs +++ b/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs @@ -35,7 +35,6 @@ namespace MediaBrowser.MediaEncoding.Subtitles private readonly IMediaEncoder _mediaEncoder; private readonly IHttpClientFactory _httpClientFactory; private readonly IMediaSourceManager _mediaSourceManager; - private readonly ISubtitleParser _subtitleParser; /// /// The _semaphoreLocks. @@ -49,8 +48,7 @@ namespace MediaBrowser.MediaEncoding.Subtitles IFileSystem fileSystem, IMediaEncoder mediaEncoder, IHttpClientFactory httpClientFactory, - IMediaSourceManager mediaSourceManager, - ISubtitleParser subtitleParser) + IMediaSourceManager mediaSourceManager) { _logger = logger; _appPaths = appPaths; @@ -58,7 +56,6 @@ namespace MediaBrowser.MediaEncoding.Subtitles _mediaEncoder = mediaEncoder; _httpClientFactory = httpClientFactory; _mediaSourceManager = mediaSourceManager; - _subtitleParser = subtitleParser; } private string SubtitleCachePath => Path.Combine(_appPaths.DataPath, "subtitles"); @@ -76,7 +73,8 @@ namespace MediaBrowser.MediaEncoding.Subtitles try { - var trackInfo = _subtitleParser.Parse(stream, inputFormat); + var reader = GetReader(inputFormat); + var trackInfo = reader.Parse(stream, cancellationToken); FilterEvents(trackInfo, startTimeTicks, endTimeTicks, preserveOriginalTimestamps); @@ -235,8 +233,7 @@ namespace MediaBrowser.MediaEncoding.Subtitles var currentFormat = (Path.GetExtension(subtitleStream.Path) ?? subtitleStream.Codec) .TrimStart('.'); - // Fallback to ffmpeg conversion - if (!_subtitleParser.SupportsFileExtension(currentFormat)) + if (!TryGetReader(currentFormat, out _)) { // Convert var outputPath = GetSubtitleCachePath(mediaSource, subtitleStream.Index, ".srt"); @@ -246,10 +243,44 @@ namespace MediaBrowser.MediaEncoding.Subtitles return new SubtitleInfo(outputPath, MediaProtocol.File, "srt", true); } - // It's possible that the subtitleStream and mediaSource don't share the same protocol (e.g. .STRM file with local subs) + // It's possbile that the subtitleStream and mediaSource don't share the same protocol (e.g. .STRM file with local subs) return new SubtitleInfo(subtitleStream.Path, _mediaSourceManager.GetPathProtocol(subtitleStream.Path), currentFormat, true); } + private bool TryGetReader(string format, [NotNullWhen(true)] out ISubtitleParser? value) + { + if (string.Equals(format, SubtitleFormat.SRT, StringComparison.OrdinalIgnoreCase)) + { + value = new SrtParser(_logger); + return true; + } + + if (string.Equals(format, SubtitleFormat.SSA, StringComparison.OrdinalIgnoreCase)) + { + value = new SsaParser(_logger); + return true; + } + + if (string.Equals(format, SubtitleFormat.ASS, StringComparison.OrdinalIgnoreCase)) + { + value = new AssParser(_logger); + return true; + } + + value = null; + return false; + } + + private ISubtitleParser GetReader(string format) + { + if (TryGetReader(format, out var reader)) + { + return reader; + } + + throw new ArgumentException("Unsupported format: " + format); + } + private bool TryGetWriter(string format, [NotNullWhen(true)] out ISubtitleWriter? value) { if (string.Equals(format, SubtitleFormat.ASS, StringComparison.OrdinalIgnoreCase)) diff --git a/tests/Jellyfin.MediaEncoding.Tests/Subtitles/AssParserTests.cs b/tests/Jellyfin.MediaEncoding.Tests/Subtitles/AssParserTests.cs index e14850eed7..3775555dec 100644 --- a/tests/Jellyfin.MediaEncoding.Tests/Subtitles/AssParserTests.cs +++ b/tests/Jellyfin.MediaEncoding.Tests/Subtitles/AssParserTests.cs @@ -15,7 +15,7 @@ namespace Jellyfin.MediaEncoding.Subtitles.Tests { using (var stream = File.OpenRead("Test Data/example.ass")) { - var parsed = new SubtitleEditParser(new NullLogger()).Parse(stream, "ass"); + var parsed = new AssParser(new NullLogger()).Parse(stream, CancellationToken.None); Assert.Single(parsed.TrackEvents); var trackEvent = parsed.TrackEvents[0]; diff --git a/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SrtParserTests.cs b/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SrtParserTests.cs index 0038b18736..c07c9ea7db 100644 --- a/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SrtParserTests.cs +++ b/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SrtParserTests.cs @@ -15,7 +15,7 @@ namespace Jellyfin.MediaEncoding.Subtitles.Tests { using (var stream = File.OpenRead("Test Data/example.srt")) { - var parsed = new SubtitleEditParser(new NullLogger()).Parse(stream, "srt"); + var parsed = new SrtParser(new NullLogger()).Parse(stream, CancellationToken.None); Assert.Equal(2, parsed.TrackEvents.Count); var trackEvent1 = parsed.TrackEvents[0]; @@ -37,7 +37,7 @@ namespace Jellyfin.MediaEncoding.Subtitles.Tests { using (var stream = File.OpenRead("Test Data/example2.srt")) { - var parsed = new SubtitleEditParser(new NullLogger()).Parse(stream, "srt"); + var parsed = new SrtParser(new NullLogger()).Parse(stream, CancellationToken.None); Assert.Equal(2, parsed.TrackEvents.Count); var trackEvent1 = parsed.TrackEvents[0]; diff --git a/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SsaParserTests.cs b/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SsaParserTests.cs index 3b9a71690c..56649db8f8 100644 --- a/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SsaParserTests.cs +++ b/tests/Jellyfin.MediaEncoding.Tests/Subtitles/SsaParserTests.cs @@ -13,7 +13,7 @@ namespace Jellyfin.MediaEncoding.Subtitles.Tests { public class SsaParserTests { - private readonly SubtitleEditParser _parser = new SubtitleEditParser(new NullLogger()); + private readonly SsaParser _parser = new SsaParser(new NullLogger()); [Theory] [MemberData(nameof(Parse_MultipleDialogues_TestData))] @@ -21,7 +21,7 @@ namespace Jellyfin.MediaEncoding.Subtitles.Tests { using (Stream stream = new MemoryStream(Encoding.UTF8.GetBytes(ssa))) { - SubtitleTrackInfo subtitleTrackInfo = _parser.Parse(stream, "ssa"); + SubtitleTrackInfo subtitleTrackInfo = _parser.Parse(stream, CancellationToken.None); Assert.Equal(expectedSubtitleTrackEvents.Count, subtitleTrackInfo.TrackEvents.Count); @@ -76,7 +76,7 @@ namespace Jellyfin.MediaEncoding.Subtitles.Tests { using (var stream = File.OpenRead("Test Data/example.ssa")) { - var parsed = _parser.Parse(stream, "ssa"); + var parsed = _parser.Parse(stream, CancellationToken.None); Assert.Single(parsed.TrackEvents); var trackEvent = parsed.TrackEvents[0];