diff options
4 files changed, 364 insertions, 18 deletions
diff --git a/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs b/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs index 65f6b79656..6ff2873cc5 100644 --- a/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs +++ b/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs @@ -1267,16 +1267,13 @@ 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 + var subtitleExtension = Path.GetExtension(subtitlePath.AsSpan()); if (subtitleExtension.Equals(".sub", StringComparison.OrdinalIgnoreCase)) { var idxFile = Path.ChangeExtension(subtitlePath, ".idx"); @@ -1307,7 +1304,7 @@ namespace MediaBrowser.Controller.MediaEncoding arg.Append(' ').Append(seekSubParam); } - if (!string.IsNullOrEmpty(canvasArgs)) + if (isGraphicalBurnIn && !string.IsNullOrEmpty(canvasArgs)) { arg.Append(canvasArgs); } @@ -3072,11 +3069,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 +3098,31 @@ 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. + // 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 0:{0}", - subtitleStreamIndex); + args += string.Format( + CultureInfo.InvariantCulture, + " -map 1:{0}", + inFileIndex); + } + else + { + int subtitleStreamIndex = FindIndex(state.MediaSource.MediaStreams, state.SubtitleStream); + + args += string.Format( + CultureInfo.InvariantCulture, + " -map 0:{0}", + subtitleStreamIndex); + } } else if (state.SubtitleStream.IsExternal && !state.SubtitleStream.IsTextSubtitleStream) { @@ -7886,6 +7899,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.Controller.Tests/MediaEncoding/EncodingHelperTests.cs b/tests/Jellyfin.Controller.Tests/MediaEncoding/EncodingHelperTests.cs new file mode 100644 index 0000000000..d7ae6a8a18 --- /dev/null +++ b/tests/Jellyfin.Controller.Tests/MediaEncoding/EncodingHelperTests.cs @@ -0,0 +1,258 @@ +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; +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() + { + // 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<MediaStream> { 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<IApplicationPaths>(); + var mediaEncoder = new Mock<IMediaEncoder>(); + var subtitleEncoder = new Mock<ISubtitleEncoder>(); + var config = new Mock<IConfiguration>(); + var configurationManager = new Mock<IConfigurationManager>(); + var pathManager = new Mock<IPathManager>(); + + return new EncodingHelper( + appPaths, + mediaEncoder.Object, + subtitleEncoder.Object, + config.Object, + configurationManager.Object, + pathManager.Object); + } +} 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<ITranscoderSupport>(); + transcoderSupport.Setup(x => x.CanExtractSubtitles(It.IsAny<string>())).Returns(true); + + var result = StreamBuilder.GetSubtitleProfile( + mediaSource, + subtitleStream, + subtitleProfiles, + playMethod, + transcoderSupport.Object, + outputContainer, + transcodingSubProtocol); + + Assert.Equal(expectedMethod, result.Method); + } } } |