Enable nullable for more files and add tests

Adds basic tests for FFProbeVideoInfo.CreateDummyChapters
Fixed error message CreateDummyChapters instead of reporting the total minutes it only reported the minute component

Emby.Server.Implementations/Data/SqliteItemRepository.cs

@@ -1971,18 +1971,7 @@ namespace Emby.Server.Implementations.Data
             if (reader.TryGetString(2, out var imagePath))
             {
                 chapter.ImagePath = imagePath;
-
-                if (!string.IsNullOrEmpty(chapter.ImagePath))
-                {
-                    try
-                    {
-                        chapter.ImageTag = _imageProcessor.GetImageCacheTag(item, chapter);
-                    }
-                    catch (Exception ex)
-                    {
-                        Logger.LogError(ex, "Failed to create image cache tag.");
-                    }
-                }
+                chapter.ImageTag = _imageProcessor.GetImageCacheTag(item, chapter);
             }
 
             if (reader.TryReadDateTime(3, out var imageDateModified))

MediaBrowser.Controller/Drawing/IImageProcessor.cs

@@ -66,7 +66,7 @@ namespace MediaBrowser.Controller.Drawing
         /// <returns>Guid.</returns>
         string GetImageCacheTag(BaseItem item, ItemImageInfo image);
 
-        string GetImageCacheTag(BaseItem item, ChapterInfo chapter);
+        string? GetImageCacheTag(BaseItem item, ChapterInfo chapter);
 
         string? GetImageCacheTag(User user);
 

MediaBrowser.Controller/Entities/ItemImageInfo.cs

@@ -1,5 +1,3 @@
-#nullable disable
-
 #pragma warning disable CS1591
 
 using System;
@@ -14,7 +12,7 @@ namespace MediaBrowser.Controller.Entities
         /// Gets or sets the path.
         /// </summary>
         /// <value>The path.</value>
-        public string Path { get; set; }
+        public required string Path { get; set; }
 
         /// <summary>
         /// Gets or sets the type.
@@ -36,9 +34,9 @@ namespace MediaBrowser.Controller.Entities
         /// Gets or sets the blurhash.
         /// </summary>
         /// <value>The blurhash.</value>
-        public string BlurHash { get; set; }
+        public string? BlurHash { get; set; }
 
         [JsonIgnore]
-        public bool IsLocalFile => Path is null || !Path.StartsWith("http", StringComparison.OrdinalIgnoreCase);
+        public bool IsLocalFile => !Path.StartsWith("http", StringComparison.OrdinalIgnoreCase);
     }
 }

MediaBrowser.Controller/Security/IAuthenticationManager.cs

@@ -1,6 +1,4 @@
-#nullable enable
-
-using System.Collections.Generic;
+using System.Collections.Generic;
 using System.Threading.Tasks;
 
 namespace MediaBrowser.Controller.Security

MediaBrowser.Model/Entities/ChapterInfo.cs

@@ -1,4 +1,3 @@
-#nullable disable
 #pragma warning disable CS1591
 
 using System;
@@ -20,16 +19,16 @@ namespace MediaBrowser.Model.Entities
         /// Gets or sets the name.
         /// </summary>
         /// <value>The name.</value>
-        public string Name { get; set; }
+        public string? Name { get; set; }
 
         /// <summary>
         /// Gets or sets the image path.
         /// </summary>
         /// <value>The image path.</value>
-        public string ImagePath { get; set; }
+        public string? ImagePath { get; set; }
 
         public DateTime ImageDateModified { get; set; }
 
-        public string ImageTag { get; set; }
+        public string? ImageTag { get; set; }
     }
 }

MediaBrowser.Providers/MediaInfo/FFProbeVideoInfo.cs

@@ -1,11 +1,8 @@
-#nullable disable
-
 #pragma warning disable CA1068, CS1591
 
 using System;
 using System.Collections.Generic;
 using System.Globalization;
-using System.IO;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -83,9 +80,9 @@ namespace MediaBrowser.Providers.MediaInfo
             CancellationToken cancellationToken)
             where T : Video
         {
-            BlurayDiscInfo blurayDiscInfo = null;
+            BlurayDiscInfo? blurayDiscInfo = null;
 
-            Model.MediaInfo.MediaInfo mediaInfoResult = null;
+            Model.MediaInfo.MediaInfo? mediaInfoResult = null;
 
             if (!item.IsShortcut || options.EnableRemoteContentProbe)
             {
@@ -131,7 +128,7 @@ namespace MediaBrowser.Providers.MediaInfo
                     var m2ts = _mediaEncoder.GetPrimaryPlaylistM2tsFiles(item.Path);
 
                     // Return if no playable .m2ts files are found
-                    if (blurayDiscInfo.Files.Length == 0 || m2ts.Count == 0)
+                    if (blurayDiscInfo == null || blurayDiscInfo.Files.Length == 0 || m2ts.Count == 0)
                     {
                         _logger.LogError("No playable .m2ts files found in Blu-ray structure, skipping FFprobe.");
                         return ItemUpdateType.MetadataImport;
@@ -192,16 +189,14 @@ namespace MediaBrowser.Providers.MediaInfo
         protected async Task Fetch(
             Video video,
             CancellationToken cancellationToken,
-            Model.MediaInfo.MediaInfo mediaInfo,
-            BlurayDiscInfo blurayInfo,
+            Model.MediaInfo.MediaInfo? mediaInfo,
+            BlurayDiscInfo? blurayInfo,
             MetadataRefreshOptions options)
         {
-            List<MediaStream> mediaStreams;
+            List<MediaStream> mediaStreams = new List<MediaStream>();
             IReadOnlyList<MediaAttachment> mediaAttachments;
             ChapterInfo[] chapters;
 
-            mediaStreams = new List<MediaStream>();
-
             // Add external streams before adding the streams from the file to preserve stream IDs on remote videos
             await AddExternalSubtitlesAsync(video, mediaStreams, options, cancellationToken).ConfigureAwait(false);
 
@@ -221,18 +216,6 @@ namespace MediaBrowser.Providers.MediaInfo
                 video.TotalBitrate = mediaInfo.Bitrate;
                 video.RunTimeTicks = mediaInfo.RunTimeTicks;
                 video.Size = mediaInfo.Size;
-
-                if (video.VideoType == VideoType.VideoFile)
-                {
-                    var extension = (Path.GetExtension(video.Path) ?? string.Empty).TrimStart('.');
-
-                    video.Container = extension;
-                }
-                else
-                {
-                    video.Container = null;
-                }
-
                 video.Container = mediaInfo.Container;
 
                 chapters = mediaInfo.Chapters ?? Array.Empty<ChapterInfo>();
@@ -243,8 +226,7 @@ namespace MediaBrowser.Providers.MediaInfo
             }
             else
             {
-                var currentMediaStreams = video.GetMediaStreams();
-                foreach (var mediaStream in currentMediaStreams)
+                foreach (var mediaStream in video.GetMediaStreams())
                 {
                     if (!mediaStream.IsExternal)
                     {
@@ -295,8 +277,8 @@ namespace MediaBrowser.Providers.MediaInfo
                 _itemRepo.SaveMediaAttachments(video.Id, mediaAttachments, cancellationToken);
             }
 
-            if (options.MetadataRefreshMode == MetadataRefreshMode.FullRefresh ||
-                options.MetadataRefreshMode == MetadataRefreshMode.Default)
+            if (options.MetadataRefreshMode == MetadataRefreshMode.FullRefresh
+                || options.MetadataRefreshMode == MetadataRefreshMode.Default)
             {
                 if (_config.Configuration.DummyChapterDuration > 0 && chapters.Length == 0 && mediaStreams.Any(i => i.Type == MediaStreamType.Video))
                 {
@@ -321,11 +303,11 @@ namespace MediaBrowser.Providers.MediaInfo
         {
             for (int i = 0; i < chapters.Length; i++)
             {
-                string name = chapters[i].Name;
+                string? name = chapters[i].Name;
                 // Check if the name is empty and/or if the name is a time
                 // Some ripping programs do that.
-                if (string.IsNullOrWhiteSpace(name) ||
-                    TimeSpan.TryParse(name, out _))
+                if (string.IsNullOrWhiteSpace(name)
+                    || TimeSpan.TryParse(name, out _))
                 {
                     chapters[i].Name = string.Format(
                         CultureInfo.InvariantCulture,
@@ -384,23 +366,18 @@ namespace MediaBrowser.Providers.MediaInfo
             // Use the ffprobe values if these are empty
             if (videoStream is not null)
             {
-                videoStream.BitRate = IsEmpty(videoStream.BitRate) ? currentBitRate : videoStream.BitRate;
-                videoStream.Width = IsEmpty(videoStream.Width) ? currentWidth : videoStream.Width;
-                videoStream.Height = IsEmpty(videoStream.Height) ? currentHeight : videoStream.Height;
+                videoStream.BitRate = videoStream.BitRate.GetValueOrDefault() == 0 ? currentBitRate : videoStream.BitRate;
+                videoStream.Width = videoStream.Width.GetValueOrDefault() == 0 ? currentWidth : videoStream.Width;
+                videoStream.Height = videoStream.Height.GetValueOrDefault() == 0 ? currentHeight : videoStream.Height;
             }
         }
 
-        private bool IsEmpty(int? num)
-        {
-            return !num.HasValue || num.Value == 0;
-        }
-
         /// <summary>
         /// Gets information about the longest playlist on a bdrom.
         /// </summary>
         /// <param name="path">The path.</param>
         /// <returns>VideoStream.</returns>
-        private BlurayDiscInfo GetBDInfo(string path)
+        private BlurayDiscInfo? GetBDInfo(string path)
         {
             ArgumentException.ThrowIfNullOrEmpty(path);
 
@@ -527,32 +504,29 @@ namespace MediaBrowser.Providers.MediaInfo
 
         private void FetchPeople(Video video, Model.MediaInfo.MediaInfo data, MetadataRefreshOptions options)
         {
-            var replaceData = options.ReplaceAllMetadata;
+            if (video.IsLocked
+                || video.LockedFields.Contains(MetadataField.Cast)
+                || data.People.Length == 0)
+            {
+                return;
+            }
 
-            if (!video.IsLocked && !video.LockedFields.Contains(MetadataField.Cast))
+            if (options.ReplaceAllMetadata || _libraryManager.GetPeople(video).Count == 0)
             {
-                if (replaceData || _libraryManager.GetPeople(video).Count == 0)
-                {
-                    var people = new List<PersonInfo>();
+                var people = new List<PersonInfo>();
 
-                    foreach (var person in data.People)
+                foreach (var person in data.People)
+                {
+                    PeopleHelper.AddPerson(people, new PersonInfo
                     {
-                        PeopleHelper.AddPerson(people, new PersonInfo
-                        {
-                            Name = person.Name,
-                            Type = person.Type,
-                            Role = person.Role
-                        });
-                    }
-
-                    _libraryManager.UpdatePeople(video, people);
+                        Name = person.Name,
+                        Type = person.Type,
+                        Role = person.Role
+                    });
                 }
-            }
-        }
 
-        private SubtitleOptions GetOptions()
-        {
-            return _config.GetConfiguration<SubtitleOptions>("subtitles");
+                _libraryManager.UpdatePeople(video, people);
+            }
         }
 
         /// <summary>
@@ -575,7 +549,7 @@ namespace MediaBrowser.Providers.MediaInfo
             var enableSubtitleDownloading = options.MetadataRefreshMode == MetadataRefreshMode.Default ||
                                             options.MetadataRefreshMode == MetadataRefreshMode.FullRefresh;
 
-            var subtitleOptions = GetOptions();
+            var subtitleOptions = _config.GetConfiguration<SubtitleOptions>("subtitles");
 
             var libraryOptions = _libraryManager.GetLibraryOptions(video);
 
@@ -659,9 +633,9 @@ namespace MediaBrowser.Providers.MediaInfo
         /// </summary>
         /// <param name="video">The video.</param>
         /// <returns>An array of dummy chapters.</returns>
-        private ChapterInfo[] CreateDummyChapters(Video video)
+        internal ChapterInfo[] CreateDummyChapters(Video video)
         {
-            var runtime = video.RunTimeTicks ?? 0;
+            var runtime = video.RunTimeTicks.GetValueOrDefault();
 
             // Only process files with a runtime higher than 0 and lower than 12h. The latter are likely corrupted.
             if (runtime < 0 || runtime > TimeSpan.FromHours(12).Ticks)
@@ -671,30 +645,30 @@ namespace MediaBrowser.Providers.MediaInfo
                         CultureInfo.InvariantCulture,
                         "{0} has an invalid runtime of {1} minutes",
                         video.Name,
-                        TimeSpan.FromTicks(runtime).Minutes));
+                        TimeSpan.FromTicks(runtime).TotalMinutes));
             }
 
             long dummyChapterDuration = TimeSpan.FromSeconds(_config.Configuration.DummyChapterDuration).Ticks;
-            if (runtime > dummyChapterDuration)
+            if (runtime <= dummyChapterDuration)
             {
-                int chapterCount = (int)(runtime / dummyChapterDuration);
-                var chapters = new ChapterInfo[chapterCount];
+                return Array.Empty<ChapterInfo>();
+            }
 
-                long currentChapterTicks = 0;
-                for (int i = 0; i < chapterCount; i++)
-                {
-                    chapters[i] = new ChapterInfo
-                    {
-                        StartPositionTicks = currentChapterTicks
-                    };
+            int chapterCount = (int)(runtime / dummyChapterDuration);
+            var chapters = new ChapterInfo[chapterCount];
 
-                    currentChapterTicks += dummyChapterDuration;
-                }
+            long currentChapterTicks = 0;
+            for (int i = 0; i < chapterCount; i++)
+            {
+                chapters[i] = new ChapterInfo
+                {
+                    StartPositionTicks = currentChapterTicks
+                };
 
-                return chapters;
+                currentChapterTicks += dummyChapterDuration;
             }
 
-            return Array.Empty<ChapterInfo>();
+            return chapters;
         }
     }
 }

src/Jellyfin.Drawing/ImageProcessor.cs

@@ -13,7 +13,6 @@ using MediaBrowser.Controller;
 using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Controller.Drawing;
 using MediaBrowser.Controller.Entities;
-using MediaBrowser.Controller.MediaEncoding;
 using MediaBrowser.Model.Drawing;
 using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.IO;
@@ -437,8 +436,13 @@ public sealed class ImageProcessor : IImageProcessor, IDisposable
         => (item.Path + image.DateModified.Ticks).GetMD5().ToString("N", CultureInfo.InvariantCulture);
 
     /// <inheritdoc />
-    public string GetImageCacheTag(BaseItem item, ChapterInfo chapter)
+    public string? GetImageCacheTag(BaseItem item, ChapterInfo chapter)
     {
+        if (chapter.ImagePath is null)
+        {
+            return null;
+        }
+
         return GetImageCacheTag(item, new ItemImageInfo
         {
             Path = chapter.ImagePath,

tests/Jellyfin.Providers.Tests/Jellyfin.Providers.Tests.csproj

@@ -7,6 +7,9 @@
   </ItemGroup>
 
   <ItemGroup>
+    <PackageReference Include="AutoFixture" />
+    <PackageReference Include="AutoFixture.AutoMoq" />
+    <PackageReference Include="AutoFixture.Xunit2" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" />
     <PackageReference Include="Moq" />
     <PackageReference Include="xunit" />

tests/Jellyfin.Providers.Tests/MediaInfo/FFProbeVideoInfoTests.cs

@@ -0,0 +1,61 @@
+using System;
+using AutoFixture;
+using AutoFixture.AutoMoq;
+using MediaBrowser.Controller.Configuration;
+using MediaBrowser.Controller.Entities;
+using MediaBrowser.Model.Configuration;
+using MediaBrowser.Providers.MediaInfo;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.Providers.Tests.MediaInfo;
+
+public class FFProbeVideoInfoTests
+{
+    private readonly FFProbeVideoInfo _fFProbeVideoInfo;
+
+    public FFProbeVideoInfoTests()
+    {
+        var serverConfiguration = new ServerConfiguration()
+        {
+            DummyChapterDuration = (int)TimeSpan.FromMinutes(5).TotalSeconds
+        };
+        var serverConfig = new Mock<IServerConfigurationManager>();
+        serverConfig.Setup(c => c.Configuration)
+            .Returns(serverConfiguration);
+
+        IFixture fixture = new Fixture().Customize(new AutoMoqCustomization { ConfigureMembers = true });
+        fixture.Inject(serverConfig);
+        _fFProbeVideoInfo = fixture.Create<FFProbeVideoInfo>();
+    }
+
+    [Theory]
+    [InlineData(-1L)]
+    [InlineData(long.MinValue)]
+    [InlineData(long.MaxValue)]
+    public void CreateDummyChapters_InvalidRuntime_ThrowsArgumentException(long? runtime)
+    {
+        Assert.Throws<ArgumentException>(
+            () => _fFProbeVideoInfo.CreateDummyChapters(new Video()
+                {
+                    RunTimeTicks = runtime
+                }));
+    }
+
+    [Theory]
+    [InlineData(null, 0)]
+    [InlineData(0L, 0)]
+    [InlineData(1L, 0)]
+    [InlineData(TimeSpan.TicksPerMinute * 5, 0)]
+    [InlineData((TimeSpan.TicksPerMinute * 5) + 1, 1)]
+    [InlineData(TimeSpan.TicksPerMinute * 50, 10)]
+    public void CreateDummyChapters_ValidRuntime_CorrectChaptersCount(long? runtime, int chaptersCount)
+    {
+        var chapters = _fFProbeVideoInfo.CreateDummyChapters(new Video()
+                {
+                    RunTimeTicks = runtime
+                });
+
+        Assert.Equal(chaptersCount, chapters.Length);
+    }
+}