Merge pull request #16448 from LTe/feat/embedded-subtitles

Embed external subtitles into MKV when transcoding

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

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)

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

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