From 2a689f268bc88ee7ab7e25121a6d43f71c1f8a5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Nie=C5=82acny?= Date: Sat, 21 Mar 2026 21:57:58 +0100 Subject: [PATCH 1/4] Embed external subtitles into MKV when transcoding Allow external subtitle files (SRT, ASS, PGS, etc.) to be muxed into MKV output containers when the device profile requests Embed delivery. Previously, the IsExternal guard in GetSubtitleProfile excluded external subtitles from Embed consideration entirely, forcing them to be served as separate sidecar files even when the output container supports embedding. Changes: - Extract CanConsiderEmbedSubtitle in StreamBuilder to allow external subs through when transcoding to MKV - Add external subtitle file as FFmpeg input (-i) for Embed delivery - Map external embedded subs from the correct FFmpeg input index - Fix external audio map index to account for the new subtitle input - Extract NeedsExternalSubtitleMuxing in EncodingHelper to deduplicate the external subtitle input check Fixes #16403 --- .../MediaEncoding/EncodingHelper.cs | 58 ++++++++++++------- MediaBrowser.Model/Dlna/StreamBuilder.cs | 15 ++++- .../Dlna/StreamBuilderTests.cs | 54 +++++++++++++++++ 3 files changed, 104 insertions(+), 23 deletions(-) diff --git a/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs b/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs index 65f6b79656..1fdb5fd4bd 100644 --- a/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs +++ b/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs @@ -1267,22 +1267,23 @@ namespace MediaBrowser.Controller.MediaEncoding .Append(_mediaEncoder.GetInputPathArgument(state)); } - // sub2video for external graphical subtitles - if (state.SubtitleStream is not null - && ShouldEncodeSubtitle(state) - && !state.SubtitleStream.IsTextSubtitleStream - && state.SubtitleStream.IsExternal) + if (NeedsExternalSubtitleMuxing(state)) { var subtitlePath = state.SubtitleStream.Path; - var subtitleExtension = Path.GetExtension(subtitlePath.AsSpan()); + var isGraphicalBurnIn = ShouldEncodeSubtitle(state) && !state.SubtitleStream.IsTextSubtitleStream; - // dvdsub/vobsub graphical subtitles use .sub+.idx pairs - if (subtitleExtension.Equals(".sub", StringComparison.OrdinalIgnoreCase)) + if (isGraphicalBurnIn) { - var idxFile = Path.ChangeExtension(subtitlePath, ".idx"); - if (File.Exists(idxFile)) + var subtitleExtension = Path.GetExtension(subtitlePath.AsSpan()); + + // dvdsub/vobsub graphical subtitles use .sub+.idx pairs + if (subtitleExtension.Equals(".sub", StringComparison.OrdinalIgnoreCase)) { - subtitlePath = idxFile; + var idxFile = Path.ChangeExtension(subtitlePath, ".idx"); + if (File.Exists(idxFile)) + { + subtitlePath = idxFile; + } } } @@ -1307,7 +1308,7 @@ namespace MediaBrowser.Controller.MediaEncoding arg.Append(' ').Append(seekSubParam); } - if (!string.IsNullOrEmpty(canvasArgs)) + if (isGraphicalBurnIn && !string.IsNullOrEmpty(canvasArgs)) { arg.Append(canvasArgs); } @@ -3072,11 +3073,8 @@ namespace MediaBrowser.Controller.MediaEncoding int audioStreamIndex = FindIndex(state.MediaSource.MediaStreams, state.AudioStream); if (state.AudioStream.IsExternal) { - bool hasExternalGraphicsSubs = state.SubtitleStream is not null - && ShouldEncodeSubtitle(state) - && state.SubtitleStream.IsExternal - && !state.SubtitleStream.IsTextSubtitleStream; - int externalAudioMapIndex = hasExternalGraphicsSubs ? 2 : 1; + bool hasExternalSubAsInput = NeedsExternalSubtitleMuxing(state); + int externalAudioMapIndex = hasExternalSubAsInput ? 2 : 1; args += string.Format( CultureInfo.InvariantCulture, @@ -3104,12 +3102,20 @@ namespace MediaBrowser.Controller.MediaEncoding } else if (subtitleMethod == SubtitleDeliveryMethod.Embed) { - int subtitleStreamIndex = FindIndex(state.MediaSource.MediaStreams, state.SubtitleStream); + if (state.SubtitleStream.IsExternal) + { + // External subtitle file is added as second FFmpeg input + args += " -map 1:0"; + } + else + { + int subtitleStreamIndex = FindIndex(state.MediaSource.MediaStreams, state.SubtitleStream); - args += string.Format( - CultureInfo.InvariantCulture, - " -map 0:{0}", - subtitleStreamIndex); + args += string.Format( + CultureInfo.InvariantCulture, + " -map 0:{0}", + subtitleStreamIndex); + } } else if (state.SubtitleStream.IsExternal && !state.SubtitleStream.IsTextSubtitleStream) { @@ -7886,6 +7892,14 @@ namespace MediaBrowser.Controller.MediaEncoding || (state.BaseRequest.AlwaysBurnInSubtitleWhenTranscoding && !IsCopyCodec(state.OutputVideoCodec)); } + private static bool NeedsExternalSubtitleMuxing(EncodingJobInfo state) + { + return state.SubtitleStream is not null + && state.SubtitleStream.IsExternal + && (state.SubtitleDeliveryMethod == SubtitleDeliveryMethod.Embed + || (ShouldEncodeSubtitle(state) && !state.SubtitleStream.IsTextSubtitleStream)); + } + public static string GetVideoSyncOption(string videoSync, Version encoderVersion) { if (string.IsNullOrEmpty(videoSync)) diff --git a/MediaBrowser.Model/Dlna/StreamBuilder.cs b/MediaBrowser.Model/Dlna/StreamBuilder.cs index 44697837ca..2ccd2a6c28 100644 --- a/MediaBrowser.Model/Dlna/StreamBuilder.cs +++ b/MediaBrowser.Model/Dlna/StreamBuilder.cs @@ -1451,7 +1451,7 @@ namespace MediaBrowser.Model.Dlna string? outputContainer, MediaStreamProtocol? transcodingSubProtocol) { - if (!subtitleStream.IsExternal && (playMethod != PlayMethod.Transcode || transcodingSubProtocol != MediaStreamProtocol.hls)) + if (CanConsiderEmbedSubtitle(subtitleStream, playMethod, transcodingSubProtocol, outputContainer)) { // Look for supported embedded subs of the same format foreach (var profile in subtitleProfiles) @@ -1540,6 +1540,19 @@ namespace MediaBrowser.Model.Dlna return false; } + private static bool CanConsiderEmbedSubtitle(MediaStream subtitleStream, PlayMethod playMethod, MediaStreamProtocol? transcodingSubProtocol, string? outputContainer) + { + if (subtitleStream.IsExternal) + { + return playMethod == PlayMethod.Transcode + && transcodingSubProtocol != MediaStreamProtocol.hls + && IsSubtitleEmbedSupported(outputContainer); + } + + return playMethod != PlayMethod.Transcode + || transcodingSubProtocol != MediaStreamProtocol.hls; + } + private static SubtitleProfile? GetExternalSubtitleProfile(MediaSourceInfo mediaSource, MediaStream subtitleStream, SubtitleProfile[] subtitleProfiles, PlayMethod playMethod, ITranscoderSupport transcoderSupport, bool allowConversion) { foreach (var profile in subtitleProfiles) diff --git a/tests/Jellyfin.Model.Tests/Dlna/StreamBuilderTests.cs b/tests/Jellyfin.Model.Tests/Dlna/StreamBuilderTests.cs index 0b103debad..16c586bcda 100644 --- a/tests/Jellyfin.Model.Tests/Dlna/StreamBuilderTests.cs +++ b/tests/Jellyfin.Model.Tests/Dlna/StreamBuilderTests.cs @@ -675,5 +675,59 @@ namespace Jellyfin.Model.Tests Assert.Equal(expectedMethod, result.Method); } + + [Theory] + // External text subs embedded into MKV when transcoding (#16403) + [InlineData("srt", true, PlayMethod.Transcode, "mkv", MediaStreamProtocol.http, SubtitleDeliveryMethod.Embed)] + [InlineData("ass", true, PlayMethod.Transcode, "mkv", MediaStreamProtocol.http, SubtitleDeliveryMethod.Embed)] + // External graphical subs embedded into MKV when transcoding + [InlineData("pgssub", true, PlayMethod.Transcode, "mkv", MediaStreamProtocol.http, SubtitleDeliveryMethod.Embed)] + [InlineData("dvdsub", true, PlayMethod.Transcode, "mkv", MediaStreamProtocol.http, SubtitleDeliveryMethod.Embed)] + // External subs remain external when transcoding to non-MKV containers + [InlineData("srt", true, PlayMethod.Transcode, "mp4", MediaStreamProtocol.hls, SubtitleDeliveryMethod.External)] + [InlineData("srt", true, PlayMethod.Transcode, "ts", MediaStreamProtocol.hls, SubtitleDeliveryMethod.External)] + // External subs remain external during DirectPlay even with MKV + [InlineData("srt", true, PlayMethod.DirectPlay, "mkv", null, SubtitleDeliveryMethod.External)] + // Internal subs still embedded into MKV when transcoding (existing behavior) + [InlineData("srt", false, PlayMethod.Transcode, "mkv", MediaStreamProtocol.http, SubtitleDeliveryMethod.Embed)] + [InlineData("pgssub", false, PlayMethod.Transcode, "mkv", MediaStreamProtocol.http, SubtitleDeliveryMethod.Embed)] + public void GetSubtitleProfile_ReturnsExpectedDeliveryMethod( + string codec, + bool isExternal, + PlayMethod playMethod, + string outputContainer, + MediaStreamProtocol? transcodingSubProtocol, + SubtitleDeliveryMethod expectedMethod) + { + var mediaSource = new MediaSourceInfo(); + var subtitleStream = new MediaStream + { + Codec = codec, + Language = "eng", + IsExternal = isExternal, + Type = MediaStreamType.Subtitle, + SupportsExternalStream = true + }; + + var subtitleProfiles = new[] + { + new SubtitleProfile { Format = codec, Method = SubtitleDeliveryMethod.Embed }, + new SubtitleProfile { Format = codec, Method = SubtitleDeliveryMethod.External } + }; + + var transcoderSupport = new Mock(); + transcoderSupport.Setup(x => x.CanExtractSubtitles(It.IsAny())).Returns(true); + + var result = StreamBuilder.GetSubtitleProfile( + mediaSource, + subtitleStream, + subtitleProfiles, + playMethod, + transcoderSupport.Object, + outputContainer, + transcodingSubProtocol); + + Assert.Equal(expectedMethod, result.Method); + } } } From a15b426e73afb46d7337b98f8279e83847e20f2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Nie=C5=82acny?= Date: Tue, 24 Mar 2026 11:02:10 +0100 Subject: [PATCH 2/4] Fix external subtitle stream mapping for multi-stream containers Compute the in-file stream index for external subtitles instead of hardcoding -map 1:0. For single-stream files (SRT/ASS/VTT) the index is always 0, preserving existing behavior. For multi-stream containers like MKS, the correct track is selected by counting sibling streams that share the same Path. Add unit tests for GetMapArgs covering internal subs, external SRT, multiple external files, and multi-stream MKS containers. --- .../MediaEncoding/EncodingHelper.cs | 15 +- .../MediaEncoding/EncodingHelperTests.cs | 216 ++++++++++++++++++ 2 files changed, 229 insertions(+), 2 deletions(-) create mode 100644 tests/Jellyfin.Controller.Tests/MediaEncoding/EncodingHelperTests.cs diff --git a/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs b/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs index 1fdb5fd4bd..40bb3913e9 100644 --- a/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs +++ b/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs @@ -3104,8 +3104,19 @@ namespace MediaBrowser.Controller.MediaEncoding { if (state.SubtitleStream.IsExternal) { - // External subtitle file is added as second FFmpeg input - args += " -map 1:0"; + // External subtitle file is added as second FFmpeg input. + // For single-stream files (SRT/ASS/VTT) the in-file index is always 0. + // For multi-stream containers (MKS) we count how many streams from + // the same file appear before the selected one. + var inFileIndex = state.MediaSource.MediaStreams + .Where(s => string.Equals(s.Path, state.SubtitleStream.Path, StringComparison.Ordinal)) + .TakeWhile(s => s.Index != state.SubtitleStream.Index) + .Count(); + + args += string.Format( + CultureInfo.InvariantCulture, + " -map 1:{0}", + inFileIndex); } else { diff --git a/tests/Jellyfin.Controller.Tests/MediaEncoding/EncodingHelperTests.cs b/tests/Jellyfin.Controller.Tests/MediaEncoding/EncodingHelperTests.cs new file mode 100644 index 0000000000..e6276f0e9e --- /dev/null +++ b/tests/Jellyfin.Controller.Tests/MediaEncoding/EncodingHelperTests.cs @@ -0,0 +1,216 @@ +using System; +using System.Collections.Generic; +using Jellyfin.Data.Enums; +using MediaBrowser.Common.Configuration; +using MediaBrowser.Controller.IO; +using MediaBrowser.Controller.Library; +using MediaBrowser.Controller.MediaEncoding; +using MediaBrowser.Controller.Streaming; +using MediaBrowser.Model.Dlna; +using MediaBrowser.Model.Dto; +using MediaBrowser.Model.Entities; +using Moq; +using Xunit; + +using IConfiguration = Microsoft.Extensions.Configuration.IConfiguration; + +namespace Jellyfin.Controller.Tests.MediaEncoding +{ + public class EncodingHelperTests + { + [Fact] + public void GetMapArgs_NoSubtitle_ExcludesAllSubs() + { + var state = BuildState(subtitle: null, deliveryMethod: null); + var args = CreateHelper().GetMapArgs(state); + + Assert.Contains("-map -0:s", args, StringComparison.Ordinal); + Assert.DoesNotContain("-map 1:", args, StringComparison.Ordinal); + } + + [Fact] + public void GetMapArgs_InternalSrt_MapsFromPrimaryInput() + { + var sub = new MediaStream { Index = 2, Type = MediaStreamType.Subtitle, Codec = "srt" }; + var state = BuildState(sub, SubtitleDeliveryMethod.Embed); + var args = CreateHelper().GetMapArgs(state); + + Assert.Contains("-map 0:2", args, StringComparison.Ordinal); + Assert.DoesNotContain("-map 1:", args, StringComparison.Ordinal); + } + + [Fact] + public void GetMapArgs_InternalSubAtHigherIndex_MapsCorrectIndex() + { + var sub0 = new MediaStream { Index = 2, Type = MediaStreamType.Subtitle, Codec = "srt" }; + var sub1 = new MediaStream { Index = 3, Type = MediaStreamType.Subtitle, Codec = "ass" }; + var state = BuildState(sub1, SubtitleDeliveryMethod.Embed, additionalStreams: [sub0, sub1]); + var args = CreateHelper().GetMapArgs(state); + + Assert.Contains("-map 0:3", args, StringComparison.Ordinal); + } + + [Fact] + public void GetMapArgs_ExternalSrt_MapsFirstStreamFromInput1() + { + var sub = new MediaStream + { + Index = 2, + Type = MediaStreamType.Subtitle, + Codec = "srt", + IsExternal = true, + SupportsExternalStream = true, + Path = "/media/movie.en.srt" + }; + var state = BuildState(sub, SubtitleDeliveryMethod.Embed); + var args = CreateHelper().GetMapArgs(state); + + Assert.Contains("-map 1:0", args, StringComparison.Ordinal); + } + + [Fact] + public void GetMapArgs_SecondExternalSrt_StillMaps1Colon0() + { + var ext1 = new MediaStream + { + Index = 2, + Type = MediaStreamType.Subtitle, + Codec = "srt", + IsExternal = true, + SupportsExternalStream = true, + Path = "/media/movie.en.srt" + }; + var ext2 = new MediaStream + { + Index = 3, + Type = MediaStreamType.Subtitle, + Codec = "srt", + IsExternal = true, + SupportsExternalStream = true, + Path = "/media/movie.fr.srt" + }; + var state = BuildState(ext2, SubtitleDeliveryMethod.Embed, additionalStreams: [ext1, ext2]); + var args = CreateHelper().GetMapArgs(state); + + // Different file from ext1, so in-file index is 0 + Assert.Contains("-map 1:0", args, StringComparison.Ordinal); + } + + [Fact] + public void GetMapArgs_MksFirstTrack_MapsInFileIndex0() + { + var mks0 = new MediaStream + { + Index = 2, + Type = MediaStreamType.Subtitle, + Codec = "subrip", + IsExternal = true, + SupportsExternalStream = true, + Path = "/media/movie.mks" + }; + var mks1 = new MediaStream + { + Index = 3, + Type = MediaStreamType.Subtitle, + Codec = "ass", + IsExternal = true, + SupportsExternalStream = true, + Path = "/media/movie.mks" + }; + var state = BuildState(mks0, SubtitleDeliveryMethod.Embed, additionalStreams: [mks0, mks1]); + var args = CreateHelper().GetMapArgs(state); + + Assert.Contains("-map 1:0", args, StringComparison.Ordinal); + } + + [Fact] + public void GetMapArgs_MksSecondTrack_MapsInFileIndex1() + { + var mks0 = new MediaStream + { + Index = 2, + Type = MediaStreamType.Subtitle, + Codec = "subrip", + IsExternal = true, + SupportsExternalStream = true, + Path = "/media/movie.mks" + }; + var mks1 = new MediaStream + { + Index = 3, + Type = MediaStreamType.Subtitle, + Codec = "ass", + IsExternal = true, + SupportsExternalStream = true, + Path = "/media/movie.mks" + }; + var mks2 = new MediaStream + { + Index = 4, + Type = MediaStreamType.Subtitle, + Codec = "subrip", + IsExternal = true, + SupportsExternalStream = true, + Path = "/media/movie.mks" + }; + var state = BuildState(mks1, SubtitleDeliveryMethod.Embed, additionalStreams: [mks0, mks1, mks2]); + var args = CreateHelper().GetMapArgs(state); + + // Second track in the same .mks file → in-file index 1 + Assert.Contains("-map 1:1", args, StringComparison.Ordinal); + } + + private static EncodingJobInfo BuildState( + MediaStream? subtitle, + SubtitleDeliveryMethod? deliveryMethod, + MediaStream[]? additionalStreams = null) + { + var video = new MediaStream { Index = 0, Type = MediaStreamType.Video, Codec = "h264" }; + var audio = new MediaStream { Index = 1, Type = MediaStreamType.Audio, Codec = "aac" }; + var streams = new List { video, audio }; + + if (additionalStreams is not null) + { + streams.AddRange(additionalStreams); + } + else if (subtitle is not null) + { + streams.Add(subtitle); + } + + return new EncodingJobInfo(TranscodingJobType.Progressive) + { + MediaSource = new MediaSourceInfo + { + Container = "mkv", + MediaStreams = streams, + }, + VideoStream = video, + AudioStream = audio, + SubtitleStream = subtitle, + SubtitleDeliveryMethod = deliveryMethod ?? SubtitleDeliveryMethod.Drop, + BaseRequest = new VideoRequestDto(), + IsVideoRequest = true, + IsInputVideo = true, + }; + } + + private static EncodingHelper CreateHelper() + { + var appPaths = Mock.Of(); + var mediaEncoder = new Mock(); + var subtitleEncoder = new Mock(); + var config = new Mock(); + var configurationManager = new Mock(); + var pathManager = new Mock(); + + return new EncodingHelper( + appPaths, + mediaEncoder.Object, + subtitleEncoder.Object, + config.Object, + configurationManager.Object, + pathManager.Object); + } + } +} From 405d987557b2638afc89edf3dff20360e39cb09a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Nie=C5=82acny?= Date: Tue, 24 Mar 2026 14:26:25 +0100 Subject: [PATCH 3/4] Normalize VobSub .sub to .idx for embedding, add EncodingHelper tests Move the .sub to .idx path normalization outside the burn-in check so it applies to subtitle embedding as well. ffmpeg requires the .idx file to read VobSub subtitles. Add unit tests for GetMapArgs and GetInputArgument covering internal subs, external SRT, multi-file SRT, multi-stream MKS containers, and VobSub .sub/.idx path normalization. --- .../MediaEncoding/EncodingHelper.cs | 16 +++---- .../MediaEncoding/EncodingHelperTests.cs | 47 ++++++++++++++++++- 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs b/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs index 40bb3913e9..6ff2873cc5 100644 --- a/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs +++ b/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs @@ -1272,18 +1272,14 @@ namespace MediaBrowser.Controller.MediaEncoding var subtitlePath = state.SubtitleStream.Path; var isGraphicalBurnIn = ShouldEncodeSubtitle(state) && !state.SubtitleStream.IsTextSubtitleStream; - if (isGraphicalBurnIn) + // dvdsub/vobsub graphical subtitles use .sub+.idx pairs + var subtitleExtension = Path.GetExtension(subtitlePath.AsSpan()); + if (subtitleExtension.Equals(".sub", StringComparison.OrdinalIgnoreCase)) { - var subtitleExtension = Path.GetExtension(subtitlePath.AsSpan()); - - // dvdsub/vobsub graphical subtitles use .sub+.idx pairs - if (subtitleExtension.Equals(".sub", StringComparison.OrdinalIgnoreCase)) + var idxFile = Path.ChangeExtension(subtitlePath, ".idx"); + if (File.Exists(idxFile)) { - var idxFile = Path.ChangeExtension(subtitlePath, ".idx"); - if (File.Exists(idxFile)) - { - subtitlePath = idxFile; - } + subtitlePath = idxFile; } } diff --git a/tests/Jellyfin.Controller.Tests/MediaEncoding/EncodingHelperTests.cs b/tests/Jellyfin.Controller.Tests/MediaEncoding/EncodingHelperTests.cs index e6276f0e9e..6563dd63ab 100644 --- a/tests/Jellyfin.Controller.Tests/MediaEncoding/EncodingHelperTests.cs +++ b/tests/Jellyfin.Controller.Tests/MediaEncoding/EncodingHelperTests.cs @@ -1,11 +1,13 @@ using System; using System.Collections.Generic; +using System.IO; using Jellyfin.Data.Enums; using MediaBrowser.Common.Configuration; using MediaBrowser.Controller.IO; using MediaBrowser.Controller.Library; using MediaBrowser.Controller.MediaEncoding; using MediaBrowser.Controller.Streaming; +using MediaBrowser.Model.Configuration; using MediaBrowser.Model.Dlna; using MediaBrowser.Model.Dto; using MediaBrowser.Model.Entities; @@ -71,6 +73,8 @@ namespace Jellyfin.Controller.Tests.MediaEncoding [Fact] public void GetMapArgs_SecondExternalSrt_StillMaps1Colon0() { + // Two separate .srt files — selecting the second one still maps 1:0 + // because Jellyfin feeds only the selected file as ffmpeg input 1. var ext1 = new MediaStream { Index = 2, @@ -92,7 +96,6 @@ namespace Jellyfin.Controller.Tests.MediaEncoding var state = BuildState(ext2, SubtitleDeliveryMethod.Embed, additionalStreams: [ext1, ext2]); var args = CreateHelper().GetMapArgs(state); - // Different file from ext1, so in-file index is 0 Assert.Contains("-map 1:0", args, StringComparison.Ordinal); } @@ -156,10 +159,50 @@ namespace Jellyfin.Controller.Tests.MediaEncoding var state = BuildState(mks1, SubtitleDeliveryMethod.Embed, additionalStreams: [mks0, mks1, mks2]); var args = CreateHelper().GetMapArgs(state); - // Second track in the same .mks file → in-file index 1 Assert.Contains("-map 1:1", args, StringComparison.Ordinal); } + [Theory] + [InlineData(SubtitleDeliveryMethod.Embed, true, "movie.idx")] + [InlineData(SubtitleDeliveryMethod.Encode, true, "movie.idx")] + [InlineData(SubtitleDeliveryMethod.Embed, false, "movie.sub")] + [InlineData(SubtitleDeliveryMethod.Encode, false, "movie.sub")] + public void GetInputArgument_VobSub_UsesCorrectPath( + SubtitleDeliveryMethod deliveryMethod, + bool createIdxFile, + string expectedFilename) + { + var tempDir = Directory.CreateTempSubdirectory("jellyfin-test-"); + try + { + var subFile = Path.Combine(tempDir.FullName, "movie.sub"); + File.WriteAllText(subFile, "dummy"); + + if (createIdxFile) + { + File.WriteAllText(Path.Combine(tempDir.FullName, "movie.idx"), "dummy"); + } + + var sub = new MediaStream + { + Index = 2, + Type = MediaStreamType.Subtitle, + Codec = "dvdsub", + IsExternal = true, + SupportsExternalStream = true, + Path = subFile + }; + var state = BuildState(sub, deliveryMethod); + var inputArgs = CreateHelper().GetInputArgument(state, new EncodingOptions(), null); + + Assert.Contains(expectedFilename, inputArgs, StringComparison.Ordinal); + } + finally + { + tempDir.Delete(true); + } + } + private static EncodingJobInfo BuildState( MediaStream? subtitle, SubtitleDeliveryMethod? deliveryMethod, From f6af1a9fb626e4a2f39f0caceebb590340d54c72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Nie=C5=82acny?= Date: Tue, 19 May 2026 13:01:25 +0200 Subject: [PATCH 4/4] Use file-scoped namespace in EncodingHelperTests --- .../MediaEncoding/EncodingHelperTests.cs | 435 +++++++++--------- 1 file changed, 217 insertions(+), 218 deletions(-) diff --git a/tests/Jellyfin.Controller.Tests/MediaEncoding/EncodingHelperTests.cs b/tests/Jellyfin.Controller.Tests/MediaEncoding/EncodingHelperTests.cs index 6563dd63ab..d7ae6a8a18 100644 --- a/tests/Jellyfin.Controller.Tests/MediaEncoding/EncodingHelperTests.cs +++ b/tests/Jellyfin.Controller.Tests/MediaEncoding/EncodingHelperTests.cs @@ -16,244 +16,243 @@ using Xunit; using IConfiguration = Microsoft.Extensions.Configuration.IConfiguration; -namespace Jellyfin.Controller.Tests.MediaEncoding +namespace Jellyfin.Controller.Tests.MediaEncoding; + +public class EncodingHelperTests { - public class EncodingHelperTests + [Fact] + public void GetMapArgs_NoSubtitle_ExcludesAllSubs() { - [Fact] - public void GetMapArgs_NoSubtitle_ExcludesAllSubs() + var state = BuildState(subtitle: null, deliveryMethod: null); + var args = CreateHelper().GetMapArgs(state); + + Assert.Contains("-map -0:s", args, StringComparison.Ordinal); + Assert.DoesNotContain("-map 1:", args, StringComparison.Ordinal); + } + + [Fact] + public void GetMapArgs_InternalSrt_MapsFromPrimaryInput() + { + var sub = new MediaStream { Index = 2, Type = MediaStreamType.Subtitle, Codec = "srt" }; + var state = BuildState(sub, SubtitleDeliveryMethod.Embed); + var args = CreateHelper().GetMapArgs(state); + + Assert.Contains("-map 0:2", args, StringComparison.Ordinal); + Assert.DoesNotContain("-map 1:", args, StringComparison.Ordinal); + } + + [Fact] + public void GetMapArgs_InternalSubAtHigherIndex_MapsCorrectIndex() + { + var sub0 = new MediaStream { Index = 2, Type = MediaStreamType.Subtitle, Codec = "srt" }; + var sub1 = new MediaStream { Index = 3, Type = MediaStreamType.Subtitle, Codec = "ass" }; + var state = BuildState(sub1, SubtitleDeliveryMethod.Embed, additionalStreams: [sub0, sub1]); + var args = CreateHelper().GetMapArgs(state); + + Assert.Contains("-map 0:3", args, StringComparison.Ordinal); + } + + [Fact] + public void GetMapArgs_ExternalSrt_MapsFirstStreamFromInput1() + { + var sub = new MediaStream { - var state = BuildState(subtitle: null, deliveryMethod: null); - var args = CreateHelper().GetMapArgs(state); + Index = 2, + Type = MediaStreamType.Subtitle, + Codec = "srt", + IsExternal = true, + SupportsExternalStream = true, + Path = "/media/movie.en.srt" + }; + var state = BuildState(sub, SubtitleDeliveryMethod.Embed); + var args = CreateHelper().GetMapArgs(state); - Assert.Contains("-map -0:s", args, StringComparison.Ordinal); - Assert.DoesNotContain("-map 1:", args, StringComparison.Ordinal); - } + Assert.Contains("-map 1:0", args, StringComparison.Ordinal); + } - [Fact] - public void GetMapArgs_InternalSrt_MapsFromPrimaryInput() + [Fact] + public void GetMapArgs_SecondExternalSrt_StillMaps1Colon0() + { + // Two separate .srt files — selecting the second one still maps 1:0 + // because Jellyfin feeds only the selected file as ffmpeg input 1. + var ext1 = new MediaStream { - var sub = new MediaStream { Index = 2, Type = MediaStreamType.Subtitle, Codec = "srt" }; - var state = BuildState(sub, SubtitleDeliveryMethod.Embed); - var args = CreateHelper().GetMapArgs(state); - - Assert.Contains("-map 0:2", args, StringComparison.Ordinal); - Assert.DoesNotContain("-map 1:", args, StringComparison.Ordinal); - } - - [Fact] - public void GetMapArgs_InternalSubAtHigherIndex_MapsCorrectIndex() + Index = 2, + Type = MediaStreamType.Subtitle, + Codec = "srt", + IsExternal = true, + SupportsExternalStream = true, + Path = "/media/movie.en.srt" + }; + var ext2 = new MediaStream { - var sub0 = new MediaStream { Index = 2, Type = MediaStreamType.Subtitle, Codec = "srt" }; - var sub1 = new MediaStream { Index = 3, Type = MediaStreamType.Subtitle, Codec = "ass" }; - var state = BuildState(sub1, SubtitleDeliveryMethod.Embed, additionalStreams: [sub0, sub1]); - var args = CreateHelper().GetMapArgs(state); + Index = 3, + Type = MediaStreamType.Subtitle, + Codec = "srt", + IsExternal = true, + SupportsExternalStream = true, + Path = "/media/movie.fr.srt" + }; + var state = BuildState(ext2, SubtitleDeliveryMethod.Embed, additionalStreams: [ext1, ext2]); + var args = CreateHelper().GetMapArgs(state); - Assert.Contains("-map 0:3", args, StringComparison.Ordinal); - } + Assert.Contains("-map 1:0", args, StringComparison.Ordinal); + } - [Fact] - public void GetMapArgs_ExternalSrt_MapsFirstStreamFromInput1() + [Fact] + public void GetMapArgs_MksFirstTrack_MapsInFileIndex0() + { + var mks0 = new MediaStream { + Index = 2, + Type = MediaStreamType.Subtitle, + Codec = "subrip", + IsExternal = true, + SupportsExternalStream = true, + Path = "/media/movie.mks" + }; + var mks1 = new MediaStream + { + Index = 3, + Type = MediaStreamType.Subtitle, + Codec = "ass", + IsExternal = true, + SupportsExternalStream = true, + Path = "/media/movie.mks" + }; + var state = BuildState(mks0, SubtitleDeliveryMethod.Embed, additionalStreams: [mks0, mks1]); + var args = CreateHelper().GetMapArgs(state); + + Assert.Contains("-map 1:0", args, StringComparison.Ordinal); + } + + [Fact] + public void GetMapArgs_MksSecondTrack_MapsInFileIndex1() + { + var mks0 = new MediaStream + { + Index = 2, + Type = MediaStreamType.Subtitle, + Codec = "subrip", + IsExternal = true, + SupportsExternalStream = true, + Path = "/media/movie.mks" + }; + var mks1 = new MediaStream + { + Index = 3, + Type = MediaStreamType.Subtitle, + Codec = "ass", + IsExternal = true, + SupportsExternalStream = true, + Path = "/media/movie.mks" + }; + var mks2 = new MediaStream + { + Index = 4, + Type = MediaStreamType.Subtitle, + Codec = "subrip", + IsExternal = true, + SupportsExternalStream = true, + Path = "/media/movie.mks" + }; + var state = BuildState(mks1, SubtitleDeliveryMethod.Embed, additionalStreams: [mks0, mks1, mks2]); + var args = CreateHelper().GetMapArgs(state); + + Assert.Contains("-map 1:1", args, StringComparison.Ordinal); + } + + [Theory] + [InlineData(SubtitleDeliveryMethod.Embed, true, "movie.idx")] + [InlineData(SubtitleDeliveryMethod.Encode, true, "movie.idx")] + [InlineData(SubtitleDeliveryMethod.Embed, false, "movie.sub")] + [InlineData(SubtitleDeliveryMethod.Encode, false, "movie.sub")] + public void GetInputArgument_VobSub_UsesCorrectPath( + SubtitleDeliveryMethod deliveryMethod, + bool createIdxFile, + string expectedFilename) + { + var tempDir = Directory.CreateTempSubdirectory("jellyfin-test-"); + try + { + var subFile = Path.Combine(tempDir.FullName, "movie.sub"); + File.WriteAllText(subFile, "dummy"); + + if (createIdxFile) + { + File.WriteAllText(Path.Combine(tempDir.FullName, "movie.idx"), "dummy"); + } + var sub = new MediaStream { Index = 2, Type = MediaStreamType.Subtitle, - Codec = "srt", + Codec = "dvdsub", IsExternal = true, SupportsExternalStream = true, - Path = "/media/movie.en.srt" + Path = subFile }; - var state = BuildState(sub, SubtitleDeliveryMethod.Embed); - var args = CreateHelper().GetMapArgs(state); + var state = BuildState(sub, deliveryMethod); + var inputArgs = CreateHelper().GetInputArgument(state, new EncodingOptions(), null); - Assert.Contains("-map 1:0", args, StringComparison.Ordinal); + Assert.Contains(expectedFilename, inputArgs, StringComparison.Ordinal); } - - [Fact] - public void GetMapArgs_SecondExternalSrt_StillMaps1Colon0() + finally { - // Two separate .srt files — selecting the second one still maps 1:0 - // because Jellyfin feeds only the selected file as ffmpeg input 1. - var ext1 = new MediaStream - { - Index = 2, - Type = MediaStreamType.Subtitle, - Codec = "srt", - IsExternal = true, - SupportsExternalStream = true, - Path = "/media/movie.en.srt" - }; - var ext2 = new MediaStream - { - Index = 3, - Type = MediaStreamType.Subtitle, - Codec = "srt", - IsExternal = true, - SupportsExternalStream = true, - Path = "/media/movie.fr.srt" - }; - var state = BuildState(ext2, SubtitleDeliveryMethod.Embed, additionalStreams: [ext1, ext2]); - var args = CreateHelper().GetMapArgs(state); - - Assert.Contains("-map 1:0", args, StringComparison.Ordinal); - } - - [Fact] - public void GetMapArgs_MksFirstTrack_MapsInFileIndex0() - { - var mks0 = new MediaStream - { - Index = 2, - Type = MediaStreamType.Subtitle, - Codec = "subrip", - IsExternal = true, - SupportsExternalStream = true, - Path = "/media/movie.mks" - }; - var mks1 = new MediaStream - { - Index = 3, - Type = MediaStreamType.Subtitle, - Codec = "ass", - IsExternal = true, - SupportsExternalStream = true, - Path = "/media/movie.mks" - }; - var state = BuildState(mks0, SubtitleDeliveryMethod.Embed, additionalStreams: [mks0, mks1]); - var args = CreateHelper().GetMapArgs(state); - - Assert.Contains("-map 1:0", args, StringComparison.Ordinal); - } - - [Fact] - public void GetMapArgs_MksSecondTrack_MapsInFileIndex1() - { - var mks0 = new MediaStream - { - Index = 2, - Type = MediaStreamType.Subtitle, - Codec = "subrip", - IsExternal = true, - SupportsExternalStream = true, - Path = "/media/movie.mks" - }; - var mks1 = new MediaStream - { - Index = 3, - Type = MediaStreamType.Subtitle, - Codec = "ass", - IsExternal = true, - SupportsExternalStream = true, - Path = "/media/movie.mks" - }; - var mks2 = new MediaStream - { - Index = 4, - Type = MediaStreamType.Subtitle, - Codec = "subrip", - IsExternal = true, - SupportsExternalStream = true, - Path = "/media/movie.mks" - }; - var state = BuildState(mks1, SubtitleDeliveryMethod.Embed, additionalStreams: [mks0, mks1, mks2]); - var args = CreateHelper().GetMapArgs(state); - - Assert.Contains("-map 1:1", args, StringComparison.Ordinal); - } - - [Theory] - [InlineData(SubtitleDeliveryMethod.Embed, true, "movie.idx")] - [InlineData(SubtitleDeliveryMethod.Encode, true, "movie.idx")] - [InlineData(SubtitleDeliveryMethod.Embed, false, "movie.sub")] - [InlineData(SubtitleDeliveryMethod.Encode, false, "movie.sub")] - public void GetInputArgument_VobSub_UsesCorrectPath( - SubtitleDeliveryMethod deliveryMethod, - bool createIdxFile, - string expectedFilename) - { - var tempDir = Directory.CreateTempSubdirectory("jellyfin-test-"); - try - { - var subFile = Path.Combine(tempDir.FullName, "movie.sub"); - File.WriteAllText(subFile, "dummy"); - - if (createIdxFile) - { - File.WriteAllText(Path.Combine(tempDir.FullName, "movie.idx"), "dummy"); - } - - var sub = new MediaStream - { - Index = 2, - Type = MediaStreamType.Subtitle, - Codec = "dvdsub", - IsExternal = true, - SupportsExternalStream = true, - Path = subFile - }; - var state = BuildState(sub, deliveryMethod); - var inputArgs = CreateHelper().GetInputArgument(state, new EncodingOptions(), null); - - Assert.Contains(expectedFilename, inputArgs, StringComparison.Ordinal); - } - finally - { - tempDir.Delete(true); - } - } - - private static EncodingJobInfo BuildState( - MediaStream? subtitle, - SubtitleDeliveryMethod? deliveryMethod, - MediaStream[]? additionalStreams = null) - { - var video = new MediaStream { Index = 0, Type = MediaStreamType.Video, Codec = "h264" }; - var audio = new MediaStream { Index = 1, Type = MediaStreamType.Audio, Codec = "aac" }; - var streams = new List { video, audio }; - - if (additionalStreams is not null) - { - streams.AddRange(additionalStreams); - } - else if (subtitle is not null) - { - streams.Add(subtitle); - } - - return new EncodingJobInfo(TranscodingJobType.Progressive) - { - MediaSource = new MediaSourceInfo - { - Container = "mkv", - MediaStreams = streams, - }, - VideoStream = video, - AudioStream = audio, - SubtitleStream = subtitle, - SubtitleDeliveryMethod = deliveryMethod ?? SubtitleDeliveryMethod.Drop, - BaseRequest = new VideoRequestDto(), - IsVideoRequest = true, - IsInputVideo = true, - }; - } - - private static EncodingHelper CreateHelper() - { - var appPaths = Mock.Of(); - var mediaEncoder = new Mock(); - var subtitleEncoder = new Mock(); - var config = new Mock(); - var configurationManager = new Mock(); - var pathManager = new Mock(); - - return new EncodingHelper( - appPaths, - mediaEncoder.Object, - subtitleEncoder.Object, - config.Object, - configurationManager.Object, - pathManager.Object); + tempDir.Delete(true); } } + + private static EncodingJobInfo BuildState( + MediaStream? subtitle, + SubtitleDeliveryMethod? deliveryMethod, + MediaStream[]? additionalStreams = null) + { + var video = new MediaStream { Index = 0, Type = MediaStreamType.Video, Codec = "h264" }; + var audio = new MediaStream { Index = 1, Type = MediaStreamType.Audio, Codec = "aac" }; + var streams = new List { video, audio }; + + if (additionalStreams is not null) + { + streams.AddRange(additionalStreams); + } + else if (subtitle is not null) + { + streams.Add(subtitle); + } + + return new EncodingJobInfo(TranscodingJobType.Progressive) + { + MediaSource = new MediaSourceInfo + { + Container = "mkv", + MediaStreams = streams, + }, + VideoStream = video, + AudioStream = audio, + SubtitleStream = subtitle, + SubtitleDeliveryMethod = deliveryMethod ?? SubtitleDeliveryMethod.Drop, + BaseRequest = new VideoRequestDto(), + IsVideoRequest = true, + IsInputVideo = true, + }; + } + + private static EncodingHelper CreateHelper() + { + var appPaths = Mock.Of(); + var mediaEncoder = new Mock(); + var subtitleEncoder = new Mock(); + var config = new Mock(); + var configurationManager = new Mock(); + var pathManager = new Mock(); + + return new EncodingHelper( + appPaths, + mediaEncoder.Object, + subtitleEncoder.Object, + config.Object, + configurationManager.Object, + pathManager.Object); + } }