From 62366d4da33b02b4ea12dfe03b8e251830d14529 Mon Sep 17 00:00:00 2001 From: Jason Dove Date: Fri, 12 Mar 2021 02:20:18 +0000 Subject: [PATCH] ffmpeg and ffprobe validation fixes (#63) * abort building playout if any collection contains a zero-duration item * surface errors calling ffprobe * improve ffmpeg/ffprobe path validation --- .../Commands/UpdateFFmpegSettings.cs | 5 +- .../Commands/UpdateFFmpegSettingsHandler.cs | 67 +++++++++++++++++-- .../Scheduling/PlayoutBuilderTests.cs | 18 +++++ .../Metadata/ILocalStatisticsProvider.cs | 3 +- ErsatzTV.Core/Metadata/LocalFolderScanner.cs | 10 ++- .../Metadata/LocalStatisticsProvider.cs | 24 ++++--- ErsatzTV.Core/Scheduling/PlayoutBuilder.cs | 19 ++++++ ErsatzTV/Pages/FFmpeg.razor | 15 ++++- ErsatzTV/Services/SchedulerService.cs | 1 - 9 files changed, 141 insertions(+), 21 deletions(-) diff --git a/ErsatzTV.Application/FFmpegProfiles/Commands/UpdateFFmpegSettings.cs b/ErsatzTV.Application/FFmpegProfiles/Commands/UpdateFFmpegSettings.cs index 44d65d3a3..e4b3b392d 100644 --- a/ErsatzTV.Application/FFmpegProfiles/Commands/UpdateFFmpegSettings.cs +++ b/ErsatzTV.Application/FFmpegProfiles/Commands/UpdateFFmpegSettings.cs @@ -1,6 +1,7 @@ -using MediatR; +using ErsatzTV.Core; +using LanguageExt; namespace ErsatzTV.Application.FFmpegProfiles.Commands { - public record UpdateFFmpegSettings(FFmpegSettingsViewModel Settings) : IRequest; + public record UpdateFFmpegSettings(FFmpegSettingsViewModel Settings) : MediatR.IRequest>; } diff --git a/ErsatzTV.Application/FFmpegProfiles/Commands/UpdateFFmpegSettingsHandler.cs b/ErsatzTV.Application/FFmpegProfiles/Commands/UpdateFFmpegSettingsHandler.cs index ab4dec619..e1486dbec 100644 --- a/ErsatzTV.Application/FFmpegProfiles/Commands/UpdateFFmpegSettingsHandler.cs +++ b/ErsatzTV.Application/FFmpegProfiles/Commands/UpdateFFmpegSettingsHandler.cs @@ -1,21 +1,74 @@ -using System.Threading; +using System.Diagnostics; +using System.Threading; using System.Threading.Tasks; +using ErsatzTV.Core; using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Interfaces.Metadata; using ErsatzTV.Core.Interfaces.Repositories; using LanguageExt; -using MediatR; -using Unit = MediatR.Unit; namespace ErsatzTV.Application.FFmpegProfiles.Commands { - public class UpdateFFmpegSettingsHandler : IRequestHandler + public class UpdateFFmpegSettingsHandler : MediatR.IRequestHandler> { private readonly IConfigElementRepository _configElementRepository; + private readonly ILocalFileSystem _localFileSystem; - public UpdateFFmpegSettingsHandler(IConfigElementRepository configElementRepository) => + public UpdateFFmpegSettingsHandler( + IConfigElementRepository configElementRepository, + ILocalFileSystem localFileSystem) + { _configElementRepository = configElementRepository; + _localFileSystem = localFileSystem; + } + + public Task> Handle( + UpdateFFmpegSettings request, + CancellationToken cancellationToken) => + Validate(request) + .MapT(_ => ApplyUpdate(request)) + .Bind(v => v.ToEitherAsync()); + + private async Task> Validate(UpdateFFmpegSettings request) => + (await FFmpegMustExist(request), await FFprobeMustExist(request)) + .Apply((_, _) => Unit.Default); + + private Task> FFmpegMustExist(UpdateFFmpegSettings request) => + ValidateToolPath(request.Settings.FFmpegPath, "ffmpeg"); + + private Task> FFprobeMustExist(UpdateFFmpegSettings request) => + ValidateToolPath(request.Settings.FFprobePath, "ffprobe"); + + private async Task> ValidateToolPath(string path, string name) + { + if (!_localFileSystem.FileExists(path)) + { + return BaseError.New($"{name} path does not exist"); + } + + var startInfo = new ProcessStartInfo + { + FileName = path, + Arguments = "-version", + RedirectStandardOutput = true, + RedirectStandardError = true, + UseShellExecute = false + }; + + var test = new Process + { + StartInfo = startInfo + }; + + test.Start(); + string output = await test.StandardOutput.ReadToEndAsync(); + await test.WaitForExitAsync(); + return test.ExitCode == 0 && output.Contains($"{name} version") + ? Unit.Default + : BaseError.New($"Unable to verify {name} version"); + } - public async Task Handle(UpdateFFmpegSettings request, CancellationToken cancellationToken) + private async Task ApplyUpdate(UpdateFFmpegSettings request) { Option ffmpegPath = await _configElementRepository.Get(ConfigElementKey.FFmpegPath); Option ffprobePath = await _configElementRepository.Get(ConfigElementKey.FFprobePath); @@ -64,7 +117,7 @@ namespace ErsatzTV.Application.FFmpegProfiles.Commands _configElementRepository.Add(ce); }); - return Unit.Value; + return Unit.Default; } } } diff --git a/ErsatzTV.Core.Tests/Scheduling/PlayoutBuilderTests.cs b/ErsatzTV.Core.Tests/Scheduling/PlayoutBuilderTests.cs index 34984daf2..a7e411200 100644 --- a/ErsatzTV.Core.Tests/Scheduling/PlayoutBuilderTests.cs +++ b/ErsatzTV.Core.Tests/Scheduling/PlayoutBuilderTests.cs @@ -37,6 +37,24 @@ namespace ErsatzTV.Core.Tests.Scheduling _logger = factory.CreateLogger(); } + [Test] + [Timeout(2000)] + public async Task ZeroDurationItem_Should_Abort() + { + var mediaItems = new List + { + TestMovie(1, TimeSpan.Zero, DateTime.Today) + }; + + (PlayoutBuilder builder, Playout playout) = TestDataFloodForItems(mediaItems, PlaybackOrder.Random); + DateTimeOffset start = HoursAfterMidnight(0); + DateTimeOffset finish = start + TimeSpan.FromHours(6); + + Playout result = await builder.BuildPlayoutItems(playout, start, finish); + + result.Items.Should().BeNull(); + } + [Test] public async Task InitialFlood_Should_StartAtMidnight() { diff --git a/ErsatzTV.Core/Interfaces/Metadata/ILocalStatisticsProvider.cs b/ErsatzTV.Core/Interfaces/Metadata/ILocalStatisticsProvider.cs index c1b480c64..1fce8a79c 100644 --- a/ErsatzTV.Core/Interfaces/Metadata/ILocalStatisticsProvider.cs +++ b/ErsatzTV.Core/Interfaces/Metadata/ILocalStatisticsProvider.cs @@ -1,10 +1,11 @@ using System.Threading.Tasks; using ErsatzTV.Core.Domain; +using LanguageExt; namespace ErsatzTV.Core.Interfaces.Metadata { public interface ILocalStatisticsProvider { - Task RefreshStatistics(string ffprobePath, MediaItem mediaItem); + Task> RefreshStatistics(string ffprobePath, MediaItem mediaItem); } } diff --git a/ErsatzTV.Core/Metadata/LocalFolderScanner.cs b/ErsatzTV.Core/Metadata/LocalFolderScanner.cs index dd29814b7..ad6e80d8b 100644 --- a/ErsatzTV.Core/Metadata/LocalFolderScanner.cs +++ b/ErsatzTV.Core/Metadata/LocalFolderScanner.cs @@ -80,7 +80,15 @@ namespace ErsatzTV.Core.Metadata if (version.DateUpdated < _localFileSystem.GetLastWriteTime(path)) { _logger.LogDebug("Refreshing {Attribute} for {Path}", "Statistics", path); - await _localStatisticsProvider.RefreshStatistics(ffprobePath, mediaItem); + Either refreshResult = + await _localStatisticsProvider.RefreshStatistics(ffprobePath, mediaItem); + refreshResult.IfLeft( + error => + _logger.LogWarning( + "Unable to refresh {Attribute} for media item {Path}. Error: {Error}", + "Statistics", + path, + error.Value)); } return mediaItem; diff --git a/ErsatzTV.Core/Metadata/LocalStatisticsProvider.cs b/ErsatzTV.Core/Metadata/LocalStatisticsProvider.cs index cf4ebce83..1541f6870 100644 --- a/ErsatzTV.Core/Metadata/LocalStatisticsProvider.cs +++ b/ErsatzTV.Core/Metadata/LocalStatisticsProvider.cs @@ -29,7 +29,7 @@ namespace ErsatzTV.Core.Metadata _logger = logger; } - public async Task RefreshStatistics(string ffprobePath, MediaItem mediaItem) + public async Task> RefreshStatistics(string ffprobePath, MediaItem mediaItem) { try { @@ -40,14 +40,20 @@ namespace ErsatzTV.Core.Metadata _ => throw new ArgumentOutOfRangeException(nameof(mediaItem)) }; - FFprobe ffprobe = await GetProbeOutput(ffprobePath, filePath); - MediaVersion version = ProjectToMediaVersion(ffprobe); - return await ApplyVersionUpdate(mediaItem, version, filePath); + Either maybeProbe = await GetProbeOutput(ffprobePath, filePath); + return await maybeProbe.Match( + async ffprobe => + { + MediaVersion version = ProjectToMediaVersion(ffprobe); + await ApplyVersionUpdate(mediaItem, version, filePath); + return Right(Unit.Default); + }, + error => Task.FromResult(Left(error))); } catch (Exception ex) { _logger.LogWarning(ex, "Failed to refresh statistics for media item {Id}", mediaItem.Id); - return false; + return BaseError.New(ex.Message); } } @@ -76,7 +82,7 @@ namespace ErsatzTV.Core.Metadata return await _mediaItemRepository.Update(mediaItem) && durationChange; } - private Task GetProbeOutput(string ffprobePath, string filePath) + private Task> GetProbeOutput(string ffprobePath, string filePath) { var startInfo = new ProcessStartInfo { @@ -101,11 +107,13 @@ namespace ErsatzTV.Core.Metadata }; probe.Start(); - return probe.StandardOutput.ReadToEndAsync().MapAsync( + return probe.StandardOutput.ReadToEndAsync().MapAsync>( async output => { await probe.WaitForExitAsync(); - return JsonConvert.DeserializeObject(output); + return probe.ExitCode == 0 + ? JsonConvert.DeserializeObject(output) + : BaseError.New($"FFprobe at {ffprobePath} exited with code {probe.ExitCode}"); }); } diff --git a/ErsatzTV.Core/Scheduling/PlayoutBuilder.cs b/ErsatzTV.Core/Scheduling/PlayoutBuilder.cs index 1de8524f4..71951d160 100644 --- a/ErsatzTV.Core/Scheduling/PlayoutBuilder.cs +++ b/ErsatzTV.Core/Scheduling/PlayoutBuilder.cs @@ -88,6 +88,25 @@ namespace ErsatzTV.Core.Scheduling return playout; } + Option zeroDurationCollection = collectionMediaItems.Find( + c => c.Value.Any( + mi => mi switch + { + Movie m => m.MediaVersions.HeadOrNone().Map(mv => mv.Duration).IfNone(TimeSpan.Zero) == + TimeSpan.Zero, + Episode e => e.MediaVersions.HeadOrNone().Map(mv => mv.Duration).IfNone(TimeSpan.Zero) == + TimeSpan.Zero, + _ => true + })).Map(c => c.Key); + if (zeroDurationCollection.IsSome) + { + _logger.LogError( + "Unable to rebuild playout; collection {@CollectionKey} contains items with zero duration!", + zeroDurationCollection.ValueUnsafe()); + + return playout; + } + playout.Items ??= new List(); playout.ProgramScheduleAnchors ??= new List(); diff --git a/ErsatzTV/Pages/FFmpeg.razor b/ErsatzTV/Pages/FFmpeg.razor index 5c8a80447..793fb0f53 100644 --- a/ErsatzTV/Pages/FFmpeg.razor +++ b/ErsatzTV/Pages/FFmpeg.razor @@ -2,8 +2,11 @@ @using ErsatzTV.Application.FFmpegProfiles @using ErsatzTV.Application.FFmpegProfiles.Commands @using ErsatzTV.Application.FFmpegProfiles.Queries +@using Unit = LanguageExt.Unit @inject IDialogService Dialog @inject IMediator Mediator +@inject ILogger Logger +@inject ISnackbar Snackbar @@ -110,7 +113,17 @@ await LoadFFmpegProfilesAsync(); } - private Task SaveSettings() => Mediator.Send(new UpdateFFmpegSettings(_ffmpegSettings)); + private async Task SaveSettings() + { + Either result = await Mediator.Send(new UpdateFFmpegSettings(_ffmpegSettings)); + result.Match( + Left: error => + { + Snackbar.Add(error.Value, Severity.Error); + Logger.LogError("Unexpected error saving FFmpeg settings: {Error}", error.Value); + }, + Right: _ => Snackbar.Add("Successfully saved FFmpeg settings", Severity.Success)); + } private static string ValidatePathExists(string path) => !File.Exists(path) ? "Path does not exist" : null; diff --git a/ErsatzTV/Services/SchedulerService.cs b/ErsatzTV/Services/SchedulerService.cs index 2653109d4..2a5d94c4b 100644 --- a/ErsatzTV/Services/SchedulerService.cs +++ b/ErsatzTV/Services/SchedulerService.cs @@ -56,7 +56,6 @@ namespace ErsatzTV.Services await ScanLocalMediaSources(cancellationToken); } - private async Task BuildPlayouts(CancellationToken cancellationToken) { using IServiceScope scope = _serviceScopeFactory.CreateScope();