diff --git a/ErsatzTV.Core.Tests/FFmpeg/SongVideoGeneratorTests.cs b/ErsatzTV.Core.Tests/FFmpeg/SongVideoGeneratorTests.cs new file mode 100644 index 000000000..e0c10c2c3 --- /dev/null +++ b/ErsatzTV.Core.Tests/FFmpeg/SongVideoGeneratorTests.cs @@ -0,0 +1,129 @@ +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.FFmpeg; +using ErsatzTV.Core.Interfaces.FFmpeg; +using ErsatzTV.Core.Interfaces.Images; +using ErsatzTV.Core.Interfaces.Metadata; +using ErsatzTV.FFmpeg.State; +using NSubstitute; +using NUnit.Framework; +using Shouldly; + +namespace ErsatzTV.Core.Tests.FFmpeg; + +[TestFixture] +public class SongVideoGeneratorTests +{ + private ITempFilePool _tempFilePool; + private IImageCache _imageCache; + private IFFmpegProcessService _ffmpegProcessService; + private ILocalFileSystem _localFileSystem; + private SongVideoGenerator _songVideoGenerator; + private string _tempSubtitleFile; + + [SetUp] + public void SetUp() + { + _tempSubtitleFile = Path.Combine(Path.GetTempPath(), $"{Guid.NewGuid()}.ass"); + + _tempFilePool = Substitute.For(); + _tempFilePool.GetNextTempFile(Arg.Any()).Returns(_tempSubtitleFile); + + _imageCache = Substitute.For(); + _imageCache.GetPathForImage(Arg.Any(), Arg.Any(), Arg.Any>()) + .Returns("/fake/watermark.png"); + + _ffmpegProcessService = Substitute.For(); + _ffmpegProcessService.GenerateSongImage( + Arg.Any(), + Arg.Any(), + Arg.Any>(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any>(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()) + .Returns(Either.Right("/fake/song-image.png")); + + _localFileSystem = Substitute.For(); + _localFileSystem.GetCustomOrDefaultFile(Arg.Any(), Arg.Any()) + .Returns("/fake/background.png"); + + _songVideoGenerator = new SongVideoGenerator( + _tempFilePool, + _imageCache, + _ffmpegProcessService, + _localFileSystem); + } + + [TearDown] + public void TearDown() + { + if (_tempSubtitleFile is not null && File.Exists(_tempSubtitleFile)) + { + File.Delete(_tempSubtitleFile); + } + } + + private static Channel BuildChannel() + { + var resolution = new Resolution { Width = 1920, Height = 1080 }; + FFmpegProfile ffmpegProfile = FFmpegProfile.New("test", resolution); + + return new Channel(Guid.NewGuid()) + { + Number = "1", + Name = "Test Channel", + FFmpegProfile = ffmpegProfile, + SongVideoMode = ChannelSongVideoMode.Default + }; + } + + private static Song BuildUntaggedSong() + { + // an untagged song: FallbackMetadataProvider.GetSongMetadata never assigns + // Artists/AlbumArtists, so they persist (and materialize) as null (ersatztv#691) + var metadata = new SongMetadata + { + MetadataKind = MetadataKind.Fallback, + Title = "Untagged Song", + Artwork = [], + Artists = null, + AlbumArtists = null + }; + + return new Song + { + SongMetadata = [metadata], + MediaVersions = [] + }; + } + + [Test] + public async Task GenerateSongVideo_should_not_throw_when_artists_and_album_artists_are_null() + { + Song song = BuildUntaggedSong(); + Channel channel = BuildChannel(); + + // SongVideoGenerator randomly picks between two rendering styles (and dereferences + // metadata.Artists/AlbumArtists differently in each); loop enough times that both + // branches -- including the AlbumArtists.Filter(... Artists.Contains ...) branch -- + // are exercised with overwhelming probability, so the null guard is proven on both. + for (var i = 0; i < 25; i++) + { + Tuple result = await _songVideoGenerator.GenerateSongVideo( + song, + channel, + "/usr/bin/ffmpeg", + "/usr/bin/ffprobe", + CancellationToken.None); + + result.ShouldNotBeNull(); + result.Item1.ShouldBe("/fake/song-image.png"); + } + } +} diff --git a/ErsatzTV.Core/Domain/Metadata/SongMetadata.cs b/ErsatzTV.Core/Domain/Metadata/SongMetadata.cs index 435502bfe..853eb84e0 100644 --- a/ErsatzTV.Core/Domain/Metadata/SongMetadata.cs +++ b/ErsatzTV.Core/Domain/Metadata/SongMetadata.cs @@ -1,4 +1,4 @@ -namespace ErsatzTV.Core.Domain; +namespace ErsatzTV.Core.Domain; public class SongMetadata : Metadata { diff --git a/ErsatzTV.Core/FFmpeg/SongVideoGenerator.cs b/ErsatzTV.Core/FFmpeg/SongVideoGenerator.cs index e2a45d880..8d330dc41 100644 --- a/ErsatzTV.Core/FFmpeg/SongVideoGenerator.cs +++ b/ErsatzTV.Core/FFmpeg/SongVideoGenerator.cs @@ -1,4 +1,4 @@ -using System.Globalization; +using System.Globalization; using System.Text; using ErsatzTV.Core.Domain; using ErsatzTV.Core.Interfaces.FFmpeg; @@ -85,6 +85,9 @@ public class SongVideoGenerator : ISongVideoGenerator var sb = new StringBuilder(); + List artists = Optional(metadata.Artists).Flatten().ToList(); + List albumArtists = Optional(metadata.AlbumArtists).Flatten().ToList(); + if (detailsStyle) { if (!string.IsNullOrWhiteSpace(metadata.Title)) @@ -92,17 +95,17 @@ public class SongVideoGenerator : ISongVideoGenerator sb.Append(CultureInfo.InvariantCulture, $"{{\\fs{largeFontSize}}}{metadata.Title}"); } - if (metadata.Artists.Count > 0) + if (artists.Count > 0) { - var allArtists = string.Join(", ", metadata.Artists); + var allArtists = string.Join(", ", artists); sb.Append(CultureInfo.InvariantCulture, $"\\N{{\\fs{fontSize}}}{allArtists}"); } } else { - if (metadata.Artists.Count > 0) + if (artists.Count > 0) { - var allArtists = string.Join(", ", metadata.Artists); + var allArtists = string.Join(", ", artists); sb.Append(allArtists); } @@ -111,11 +114,11 @@ public class SongVideoGenerator : ISongVideoGenerator sb.Append(CultureInfo.InvariantCulture, $"\\N\"{metadata.Title}\""); } - if (metadata.AlbumArtists.Count > 0) + if (albumArtists.Count > 0) { var allAlbumArtists = string.Join( ", ", - metadata.AlbumArtists.Filter(aa => !metadata.Artists.Contains(aa))); + albumArtists.Filter(aa => !artists.Contains(aa))); sb.Append(CultureInfo.InvariantCulture, $"\\N{allAlbumArtists}"); } diff --git a/ErsatzTV.Infrastructure/Data/Repositories/MediaCollectionRepository.cs b/ErsatzTV.Infrastructure/Data/Repositories/MediaCollectionRepository.cs index 47b2b9d16..56ea46160 100644 --- a/ErsatzTV.Infrastructure/Data/Repositories/MediaCollectionRepository.cs +++ b/ErsatzTV.Infrastructure/Data/Repositories/MediaCollectionRepository.cs @@ -1144,7 +1144,7 @@ public class MediaCollectionRepository : IMediaCollectionRepository var allArtists = items.OfType() .SelectMany(s => s.SongMetadata) - .Map(sm => sm.AlbumArtists.HeadOrNone().Match(aa => aa, string.Empty)) + .Map(sm => Optional(sm.AlbumArtists).Flatten().HeadOrNone().Match(aa => aa, string.Empty)) .Distinct() .ToList(); @@ -1157,7 +1157,7 @@ public class MediaCollectionRepository : IMediaCollectionRepository foreach (Song song in items.OfType()) { string firstArtist = song.SongMetadata - .SelectMany(sm => sm.AlbumArtists) + .SelectMany(sm => Optional(sm.AlbumArtists).Flatten()) .HeadOrNone() .Match(aa => aa, string.Empty);