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<ITempFilePool>();
+        _tempFilePool.GetNextTempFile(Arg.Any<TempFileCategory>()).Returns(_tempSubtitleFile);
+
+        _imageCache = Substitute.For<IImageCache>();
+        _imageCache.GetPathForImage(Arg.Any<string>(), Arg.Any<ArtworkKind>(), Arg.Any<Option<int>>())
+            .Returns("/fake/watermark.png");
+
+        _ffmpegProcessService = Substitute.For<IFFmpegProcessService>();
+        _ffmpegProcessService.GenerateSongImage(
+                Arg.Any<string>(),
+                Arg.Any<string>(),
+                Arg.Any<Option<string>>(),
+                Arg.Any<Channel>(),
+                Arg.Any<MediaVersion>(),
+                Arg.Any<string>(),
+                Arg.Any<bool>(),
+                Arg.Any<Option<string>>(),
+                Arg.Any<WatermarkLocation>(),
+                Arg.Any<int>(),
+                Arg.Any<int>(),
+                Arg.Any<int>(),
+                Arg.Any<CancellationToken>())
+            .Returns(Either<BaseError, string>.Right("/fake/song-image.png"));
+
+        _localFileSystem = Substitute.For<ILocalFileSystem>();
+        _localFileSystem.GetCustomOrDefaultFile(Arg.Any<string>(), Arg.Any<string>())
+            .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<string, MediaVersion> result = await _songVideoGenerator.GenerateSongVideo(
+                song,
+                channel,
+                "/usr/bin/ffmpeg",
+                "/usr/bin/ffprobe",
+                CancellationToken.None);
+
+            result.ShouldNotBeNull();
+            result.Item1.ShouldBe("/fake/song-image.png");
+        }
+    }
+}

ErsatzTV.Core/Domain/Metadata/SongMetadata.cs

@@ -1,4 +1,4 @@
-namespace ErsatzTV.Core.Domain;
+namespace ErsatzTV.Core.Domain;
 
 public class SongMetadata : Metadata
 {

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<string> artists = Optional(metadata.Artists).Flatten().ToList();
+            List<string> 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}");
                 }
 

ErsatzTV.Infrastructure/Data/Repositories/MediaCollectionRepository.cs

@@ -1144,7 +1144,7 @@ public class MediaCollectionRepository : IMediaCollectionRepository
 
         var allArtists = items.OfType<Song>()
             .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<Song>())
         {
             string firstArtist = song.SongMetadata
-                .SelectMany(sm => sm.AlbumArtists)
+                .SelectMany(sm => Optional(sm.AlbumArtists).Flatten())
                 .HeadOrNone()
                 .Match(aa => aa, string.Empty);