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.
This commit is contained in:
@@ -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
|
||||
/// </summary>
|
||||
/// <param name="stream">The stream.</param>
|
||||
/// <param name="fileExtension">The file extension.</param>
|
||||
/// <returns>SubtitleTrackInfo.</returns>
|
||||
SubtitleTrackInfo Parse(Stream stream, string fileExtension);
|
||||
/// <returns>The parsed subtitle.</returns>
|
||||
Subtitle Parse(Stream stream, string fileExtension);
|
||||
|
||||
/// <summary>
|
||||
/// Determines whether the file extension is supported by the parser.
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
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;
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -15,13 +15,13 @@ namespace Jellyfin.MediaEncoding.Subtitles.Tests
|
||||
using var stream = File.OpenRead("Test Data/example.ass");
|
||||
|
||||
var parsed = new SubtitleEditParser(new NullLogger<SubtitleEditParser>()).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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -15,19 +15,19 @@ namespace Jellyfin.MediaEncoding.Subtitles.Tests
|
||||
using var stream = File.OpenRead("Test Data/example.srt");
|
||||
|
||||
var parsed = new SubtitleEditParser(new NullLogger<SubtitleEditParser>()).Parse(stream, "srt");
|
||||
Assert.Equal(2, parsed.TrackEvents.Count);
|
||||
Assert.Equal(2, parsed.Paragraphs.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 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 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);
|
||||
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<SubtitleEditParser>()).Parse(stream, "srt");
|
||||
Assert.Equal(2, parsed.TrackEvents.Count);
|
||||
Assert.Equal(2, parsed.Paragraphs.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 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 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);
|
||||
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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<MediaSourceInfo, MediaStream, SubtitleEncoder.SubtitleInfo> GetReadableFile_Valid_TestData()
|
||||
{
|
||||
var data = new TheoryData<MediaSourceInfo, MediaStream, SubtitleEncoder.SubtitleInfo>();
|
||||
@@ -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<ISubtitleParser>(new SubtitleEditParser(NullLogger<SubtitleEditParser>.Instance));
|
||||
return fixture.Create<SubtitleEncoder>();
|
||||
}
|
||||
|
||||
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();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user