Fix SubtitleEncoder and add regression tests

MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs

@@ -195,7 +195,7 @@ namespace MediaBrowser.MediaEncoding.Subtitles
             return AsyncFile.OpenRead(fileInfo.Path);
         }
 
-        private async Task<SubtitleInfo> GetReadableFile(
+        internal async Task<SubtitleInfo> GetReadableFile(
             MediaSourceInfo mediaSource,
             MediaStream subtitleStream,
             CancellationToken cancellationToken)
@@ -205,9 +205,9 @@ namespace MediaBrowser.MediaEncoding.Subtitles
                 string outputFormat;
                 string outputCodec;
 
-                if (string.Equals(subtitleStream.Codec, "ass", StringComparison.OrdinalIgnoreCase) ||
+                if (string.Equals(subtitleStream.Codec, "ass", StringComparison.OrdinalIgnoreCase)
-                    string.Equals(subtitleStream.Codec, "ssa", StringComparison.OrdinalIgnoreCase) ||
+                    || string.Equals(subtitleStream.Codec, "ssa", StringComparison.OrdinalIgnoreCase)
-                    string.Equals(subtitleStream.Codec, "srt", StringComparison.OrdinalIgnoreCase))
+                    || string.Equals(subtitleStream.Codec, "srt", StringComparison.OrdinalIgnoreCase))
                 {
                     // Extract
                     outputCodec = "copy";
@@ -238,7 +238,7 @@ namespace MediaBrowser.MediaEncoding.Subtitles
             var currentFormat = (Path.GetExtension(subtitleStream.Path) ?? subtitleStream.Codec)
                 .TrimStart('.');
 
-            if (TryGetReader(currentFormat, out _))
+            if (!TryGetReader(currentFormat, out _))
             {
                 // Convert
                 var outputPath = GetSubtitleCachePath(mediaSource, subtitleStream.Index, ".srt");
@@ -248,12 +248,8 @@ namespace MediaBrowser.MediaEncoding.Subtitles
                 return new SubtitleInfo(outputPath, MediaProtocol.File, "srt", true);
             }
 
-            if (subtitleStream.IsExternal)
+            // 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);
-                return new SubtitleInfo(subtitleStream.Path, _mediaSourceManager.GetPathProtocol(subtitleStream.Path), currentFormat, true);
-            }
-
-            return new SubtitleInfo(subtitleStream.Path, mediaSource.Protocol, currentFormat, true);
         }
 
         private bool TryGetReader(string format, [NotNullWhen(true)] out ISubtitleParser? value)
@@ -756,7 +752,7 @@ namespace MediaBrowser.MediaEncoding.Subtitles
             }
         }
 
-        private struct SubtitleInfo
+        internal readonly struct SubtitleInfo
         {
             public SubtitleInfo(string path, MediaProtocol protocol, string format, bool isExternal)
             {
@@ -766,13 +762,13 @@ namespace MediaBrowser.MediaEncoding.Subtitles
                 IsExternal = isExternal;
             }
 
-            public string Path { get; set; }
+            public string Path { get; }
 
-            public MediaProtocol Protocol { get; set; }
+            public MediaProtocol Protocol { get; }
 
-            public string Format { get; set; }
+            public string Format { get; }
 
-            public bool IsExternal { get; set; }
+            public bool IsExternal { get; }
         }
     }
 }

tests/Jellyfin.MediaEncoding.Tests/Jellyfin.MediaEncoding.Tests.csproj

@@ -18,10 +18,14 @@
   </ItemGroup>
 
   <ItemGroup>
+    <PackageReference Include="AutoFixture" Version="4.17.0" />
+    <PackageReference Include="AutoFixture.AutoMoq" Version="4.17.0" />
+    <PackageReference Include="AutoFixture.Xunit2" Version="4.17.0" />
+    <PackageReference Include="coverlet.collector" Version="3.1.0" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.11.0" />
+    <PackageReference Include="Moq" Version="4.16.1" />
     <PackageReference Include="xunit" Version="2.4.1" />
     <PackageReference Include="xunit.runner.visualstudio" Version="2.4.3" />
-    <PackageReference Include="coverlet.collector" Version="3.1.0" />
   </ItemGroup>
 
   <!-- Code Analyzers -->

tests/Jellyfin.MediaEncoding.Tests/Subtitles/SubtitleEncoderTests.cs

@@ -0,0 +1,83 @@
+using System;
+using System.Globalization;
+using System.IO;
+using System.Threading;
+using System.Threading.Tasks;
+using AutoFixture;
+using AutoFixture.AutoMoq;
+using MediaBrowser.Common.Configuration;
+using MediaBrowser.Common.Extensions;
+using MediaBrowser.Common.Net;
+using MediaBrowser.Controller.Entities;
+using MediaBrowser.Controller.Library;
+using MediaBrowser.Controller.MediaEncoding;
+using MediaBrowser.MediaEncoding.Subtitles;
+using MediaBrowser.Model.Dto;
+using MediaBrowser.Model.Entities;
+using MediaBrowser.Model.IO;
+using MediaBrowser.Model.MediaInfo;
+using Microsoft.Extensions.Logging.Abstractions;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.MediaEncoding.Subtitles.Tests
+{
+    public class SubtitleEncoderTests
+    {
+        internal static TheoryData<MediaSourceInfo, MediaStream, SubtitleEncoder.SubtitleInfo> GetReadableFile_Valid_TestData()
+        {
+            var data = new TheoryData<MediaSourceInfo, MediaStream, SubtitleEncoder.SubtitleInfo>();
+
+            data.Add(
+                new MediaSourceInfo()
+                {
+                    Protocol = MediaProtocol.File
+                },
+                new MediaStream()
+                {
+                    Path = "/media/sub.ass",
+                    IsExternal = true
+                },
+                new SubtitleEncoder.SubtitleInfo("/media/sub.ass", MediaProtocol.File, "ass", true));
+
+            data.Add(
+                new MediaSourceInfo()
+                {
+                    Protocol = MediaProtocol.File
+                },
+                new MediaStream()
+                {
+                    Path = "/media/sub.ssa",
+                    IsExternal = true
+                },
+                new SubtitleEncoder.SubtitleInfo("/media/sub.ssa", MediaProtocol.File, "ssa", true));
+
+            data.Add(
+                new MediaSourceInfo()
+                {
+                    Protocol = MediaProtocol.File
+                },
+                new MediaStream()
+                {
+                    Path = "/media/sub.srt",
+                    IsExternal = true
+                },
+                new SubtitleEncoder.SubtitleInfo("/media/sub.srt", MediaProtocol.File, "srt", true));
+
+            return data;
+        }
+
+        [Theory]
+        [MemberData(nameof(GetReadableFile_Valid_TestData))]
+        internal async Task GetReadableFile_Valid_Success(MediaSourceInfo mediaSource, MediaStream subtitleStream, SubtitleEncoder.SubtitleInfo subtitleInfo)
+        {
+            var fixture = new Fixture().Customize(new AutoMoqCustomization { ConfigureMembers = true });
+            var subtitleEncoder = fixture.Create<SubtitleEncoder>();
+            var result = await subtitleEncoder.GetReadableFile(mediaSource, subtitleStream, CancellationToken.None).ConfigureAwait(false);
+            Assert.Equal(subtitleInfo.Path, result.Path);
+            Assert.Equal(subtitleInfo.Protocol, result.Protocol);
+            Assert.Equal(subtitleInfo.Format, result.Format);
+            Assert.Equal(subtitleInfo.IsExternal, result.IsExternal);
+        }
+    }
+}