Merge pull request #17342 from LTe/fix-subtitle-conversion-race

Fix race condition in concurrent subtitle conversion

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
         /// </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.

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
         }
 
         /// <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 />

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);
 

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<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);
         }
     }
 }

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<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);
         }
     }
 }

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);
         }
     }
 }

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<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();
+        }
     }
 }