subtitle fixes for software, videotoolbox, vaapi (#736)

* fix subtitles using software encoders

* videotoolbox fixes

* fix some vaapi subtitle edge cases

CHANGELOG.md

@@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Fix subtitles edge case with NVENC
 - Only select picture subtitles (text subtitles are not yet supported)
   - Supported picture subtitles are `hdmv_pgs_subtitle` and `dvd_subtitle`
+- Fix subtitles using software encoders, videotoolbox, VAAPI
 
 ## [0.5.0-beta] - 2022-04-13
 ### Fixed

ErsatzTV.Core.Tests/ErsatzTV.Core.Tests.csproj

@@ -8,7 +8,7 @@
 
     <ItemGroup>
       <PackageReference Include="Bugsnag" Version="3.0.1" />
-      <PackageReference Include="CliWrap" Version="3.4.1" />
+      <PackageReference Include="CliWrap" Version="3.4.2" />
       <PackageReference Include="FluentAssertions" Version="6.6.0" />
       <PackageReference Include="LanguageExt.Core" Version="4.0.4" />
       <PackageReference Include="Microsoft.Extensions.Caching.Memory" Version="6.0.1" />

ErsatzTV.Core.Tests/FFmpeg/TranscodingTests.cs

@@ -172,8 +172,8 @@ public class TranscodingTests
         [ValueSource(typeof(TestData), nameof(TestData.Watermarks))] Watermark watermark,
         [ValueSource(typeof(TestData), nameof(TestData.Subtitles))] Subtitle subtitle,
         [ValueSource(typeof(TestData), nameof(TestData.VideoFormats))] FFmpegProfileVideoFormat profileVideoFormat,
-        // [ValueSource(typeof(TestData), nameof(TestData.NoAcceleration))] HardwareAccelerationKind profileAcceleration)
-        [ValueSource(typeof(TestData), nameof(TestData.NvidiaAcceleration))] HardwareAccelerationKind profileAcceleration)
+        [ValueSource(typeof(TestData), nameof(TestData.NoAcceleration))] HardwareAccelerationKind profileAcceleration)
+        // [ValueSource(typeof(TestData), nameof(TestData.NvidiaAcceleration))] HardwareAccelerationKind profileAcceleration)
         // [ValueSource(typeof(TestData), nameof(TestData.VaapiAcceleration))] HardwareAccelerationKind profileAcceleration)
         // [ValueSource(typeof(TestData), nameof(TestData.QsvAcceleration))] HardwareAccelerationKind profileAcceleration)
         // [ValueSource(typeof(TestData), nameof(TestData.VideoToolboxAcceleration))] HardwareAccelerationKind profileAcceleration)
@@ -429,7 +429,7 @@ public class TranscodingTests
             result = await Cli.Wrap(process.StartInfo.FileName)
                 .WithArguments(process.StartInfo.ArgumentList)
                 .WithValidation(CommandResultValidation.None)
-                .WithStandardOutputPipe(PipeTarget.Null)
+                .WithStandardOutputPipe(PipeTarget.ToStream(Stream.Null))
                 .WithStandardErrorPipe(PipeTarget.ToStringBuilder(sb))
                 .ExecuteAsync(timeoutSignal.Token);
         }

ErsatzTV.FFmpeg/Encoder/AvailableEncoders.cs

@@ -35,10 +35,12 @@ public static class AvailableEncoders
 
             (HardwareAccelerationMode.Vaapi, VideoFormat.Hevc) => new EncoderHevcVaapi(
                 currentState,
-                maybeWatermarkInputFile),
+                maybeWatermarkInputFile,
+                maybeSubtitleInputFile),
             (HardwareAccelerationMode.Vaapi, VideoFormat.H264) => new EncoderH264Vaapi(
                 currentState,
-                maybeWatermarkInputFile),
+                maybeWatermarkInputFile,
+                maybeSubtitleInputFile),
 
             (HardwareAccelerationMode.VideoToolbox, VideoFormat.Hevc) => new EncoderHevcVideoToolbox(),
             (HardwareAccelerationMode.VideoToolbox, VideoFormat.H264) => new EncoderH264VideoToolbox(),

ErsatzTV.FFmpeg/Encoder/Vaapi/EncoderH264Vaapi.cs

@@ -6,11 +6,16 @@ public class EncoderH264Vaapi : EncoderBase
 {
     private readonly FrameState _currentState;
     private readonly Option<WatermarkInputFile> _maybeWatermarkInputFile;
+    private readonly Option<SubtitleInputFile> _maybeSubtitleInputFile;
 
-    public EncoderH264Vaapi(FrameState currentState, Option<WatermarkInputFile> maybeWatermarkInputFile)
+    public EncoderH264Vaapi(
+        FrameState currentState,
+        Option<WatermarkInputFile> maybeWatermarkInputFile,
+        Option<SubtitleInputFile> maybeSubtitleInputFile)
     {
         _currentState = currentState;
         _maybeWatermarkInputFile = maybeWatermarkInputFile;
+        _maybeSubtitleInputFile = maybeSubtitleInputFile;
     }
 
     public override FrameState NextState(FrameState currentState) => currentState with
@@ -22,15 +27,20 @@ public class EncoderH264Vaapi : EncoderBase
     public override string Name => "h264_vaapi";
     public override StreamKind Kind => StreamKind.Video;
 
-    // need to upload if we're still in software unless a watermark is used
+    // need to upload if we're still in software unless a watermark or picture subtitle is used
     public override string Filter
     {
         get
         {
-            if (_maybeWatermarkInputFile.IsNone && _currentState.FrameDataLocation == FrameDataLocation.Software)
+            if (_currentState.FrameDataLocation == FrameDataLocation.Software)
+            {
+                bool isNotImageSubtitle = _maybeSubtitleInputFile.Map(s => s.IsImageBased).IfNone(false) == false; 
+                
+                if (_maybeWatermarkInputFile.IsNone && isNotImageSubtitle)
                 {
                     return "format=nv12|vaapi,hwupload";
                 }
+            }
 
             return string.Empty;
         }

ErsatzTV.FFmpeg/Encoder/Vaapi/EncoderHevcVaapi.cs

@@ -6,11 +6,16 @@ public class EncoderHevcVaapi : EncoderBase
 {
     private readonly FrameState _currentState;
     private readonly Option<WatermarkInputFile> _maybeWatermarkInputFile;
+    private readonly Option<SubtitleInputFile> _maybeSubtitleInputFile;
 
-    public EncoderHevcVaapi(FrameState currentState, Option<WatermarkInputFile> maybeWatermarkInputFile)
+    public EncoderHevcVaapi(
+        FrameState currentState,
+        Option<WatermarkInputFile> maybeWatermarkInputFile,
+        Option<SubtitleInputFile> maybeSubtitleInputFile)
     {
         _currentState = currentState;
         _maybeWatermarkInputFile = maybeWatermarkInputFile;
+        _maybeSubtitleInputFile = maybeSubtitleInputFile;
     }
 
     public override FrameState NextState(FrameState currentState) => currentState with
@@ -22,15 +27,20 @@ public class EncoderHevcVaapi : EncoderBase
     public override string Name => "hevc_vaapi";
     public override StreamKind Kind => StreamKind.Video;
 
-    // need to upload if we're still in software unless a watermark is used
+    // need to upload if we're still in software unless a watermark or picture subtitle is used
     public override string Filter
     {
         get
         {
-            if (_maybeWatermarkInputFile.IsNone && _currentState.FrameDataLocation == FrameDataLocation.Software)
+            if (_currentState.FrameDataLocation == FrameDataLocation.Software)
+            {
+                bool isNotImageSubtitle = _maybeSubtitleInputFile.Map(s => s.IsImageBased).IfNone(false) == false; 
+                
+                if (_maybeWatermarkInputFile.IsNone && isNotImageSubtitle)
                 {
                     return "format=nv12|vaapi,hwupload";
                 }
+            }
 
             return string.Empty;
         }

ErsatzTV.FFmpeg/ErsatzTV.FFmpeg.csproj

@@ -7,7 +7,7 @@
     </PropertyGroup>
 
     <ItemGroup>
-      <PackageReference Include="CliWrap" Version="3.4.1" />
+      <PackageReference Include="CliWrap" Version="3.4.2" />
       <PackageReference Include="LanguageExt.Core" Version="4.0.4" />
       <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="6.0.1" />
     </ItemGroup>

ErsatzTV.FFmpeg/Filter/ComplexFilter.cs

@@ -1,4 +1,5 @@
 using ErsatzTV.FFmpeg.Environment;
+using ErsatzTV.FFmpeg.Format;
 
 namespace ErsatzTV.FFmpeg.Filter;
 
@@ -144,8 +145,13 @@ public class ComplexFilter : IPipelineStep
                         : videoLabel;
 
                     // vaapi uses software overlay and needs to upload
+                    // videotoolbox seems to require a hwupload for hevc
+                    // also wait to upload videotoolbox if a subtitle overlay is coming
                     string uploadFilter = string.Empty;
-                    if (_ffmpegState.HardwareAccelerationMode == HardwareAccelerationMode.Vaapi)
+                    if (_maybeSubtitleInputFile.Map(s => s.IsImageBased).IfNone(false) == false &&
+                        (_ffmpegState.HardwareAccelerationMode == HardwareAccelerationMode.Vaapi ||
+                         _ffmpegState.HardwareAccelerationMode == HardwareAccelerationMode.VideoToolbox &&
+                         _currentState.VideoFormat == VideoFormat.Hevc))
                     {
                         uploadFilter = new HardwareUploadFilter(_ffmpegState).Filter;
                     }
@@ -178,6 +184,10 @@ public class ComplexFilter : IPipelineStep
                     subtitleLabel = "[st]";
                     subtitleFilterComplex += subtitleLabel;
                 }
+                else
+                {
+                    subtitleLabel = $"[{subtitleLabel}]";
+                }
 
                 IPipelineFilterStep overlayFilter =
                     AvailableSubtitleOverlayFilters.ForAcceleration(_ffmpegState.HardwareAccelerationMode);
@@ -190,8 +200,11 @@ public class ComplexFilter : IPipelineStep
                         : videoLabel;
 
                     // vaapi uses software overlay and needs to upload
+                    // videotoolbox seems to require a hwupload for hevc
                     string uploadFilter = string.Empty;
-                    if (_ffmpegState.HardwareAccelerationMode == HardwareAccelerationMode.Vaapi)
+                    if (_ffmpegState.HardwareAccelerationMode == HardwareAccelerationMode.Vaapi
+                        || _ffmpegState.HardwareAccelerationMode == HardwareAccelerationMode.VideoToolbox &&
+                        _currentState.VideoFormat == VideoFormat.Hevc)
                     {
                         uploadFilter = new HardwareUploadFilter(_ffmpegState).Filter;
                     }