Add image provider tests and clean up

MediaBrowser.Controller/MediaEncoding/IMediaEncoder.cs

@@ -95,7 +95,7 @@ public interface IMediaEncoder : ITranscoderSupport
         /// <param name="mediaSource">Media source information.</param>
         /// <param name="imageStream">Media stream information.</param>
         /// <param name="imageStreamIndex">Index of the stream to extract from.</param>
-        /// <param name="outputExtension">The extension of the file to write.</param>
+        /// <param name="outputExtension">The extension of the file to write, including the '.'.</param>
         /// <param name="cancellationToken">CancellationToken to use for operation.</param>
         /// <returns>Location of video image.</returns>
         Task<string> ExtractVideoImage(string inputFile, string container, MediaSourceInfo mediaSource, MediaStream imageStream, int? imageStreamIndex, string outputExtension, CancellationToken cancellationToken);

MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs

@@ -468,12 +468,12 @@ public Task<string> ExtractAudioImage(string path, int? imageStreamIndex, Cancel
                 Protocol = MediaProtocol.File
             };
 
-            return ExtractImage(path, null, null, imageStreamIndex, mediaSource, true, null, null, "jpg", cancellationToken);
+            return ExtractImage(path, null, null, imageStreamIndex, mediaSource, true, null, null, ".jpg", cancellationToken);
         }
 
         public Task<string> ExtractVideoImage(string inputFile, string container, MediaSourceInfo mediaSource, MediaStream videoStream, Video3DFormat? threedFormat, TimeSpan? offset, CancellationToken cancellationToken)
         {
-            return ExtractImage(inputFile, container, videoStream, null, mediaSource, false, threedFormat, offset, "jpg", cancellationToken);
+            return ExtractImage(inputFile, container, videoStream, null, mediaSource, false, threedFormat, offset, ".jpg", cancellationToken);
         }
 
         public Task<string> ExtractVideoImage(string inputFile, string container, MediaSourceInfo mediaSource, MediaStream imageStream, int? imageStreamIndex, string outputExtension, CancellationToken cancellationToken)
@@ -548,7 +548,7 @@ private async Task<string> ExtractImageInternal(string inputPath, string contain
                 throw new ArgumentNullException(nameof(inputPath));
             }
 
-            var tempExtractPath = Path.Combine(_configurationManager.ApplicationPaths.TempDirectory, Guid.NewGuid() + "." + outputExtension);
+            var tempExtractPath = Path.Combine(_configurationManager.ApplicationPaths.TempDirectory, Guid.NewGuid() + outputExtension);
             Directory.CreateDirectory(Path.GetDirectoryName(tempExtractPath));
 
             // apply some filters to thumbnail extracted below (below) crop any black lines that we made and get the correct ar.

MediaBrowser.Providers/MediaInfo/EmbeddedImageProvider.cs

@@ -1,9 +1,7 @@
-#nullable enable
 #pragma warning disable CS1591
 
 using System;
 using System.Collections.Generic;
-using System.Collections.Immutable;
 using System.IO;
 using System.Linq;
 using System.Threading;
@@ -17,7 +15,6 @@
 using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.MediaInfo;
 using MediaBrowser.Model.Net;
-using Microsoft.Extensions.Logging;
 
 namespace MediaBrowser.Providers.MediaInfo
 {
@@ -48,12 +45,10 @@ public class EmbeddedImageProvider : IDynamicImageProvider, IHasOrder
         };
 
         private readonly IMediaEncoder _mediaEncoder;
-        private readonly ILogger<EmbeddedImageProvider> _logger;
 
-        public EmbeddedImageProvider(IMediaEncoder mediaEncoder, ILogger<EmbeddedImageProvider> logger)
+        public EmbeddedImageProvider(IMediaEncoder mediaEncoder)
         {
             _mediaEncoder = mediaEncoder;
-            _logger = logger;
         }
 
         /// <inheritdoc />
@@ -84,7 +79,7 @@ public IEnumerable<ImageType> GetSupportedImages(BaseItem item)
                 };
             }
 
-            return ImmutableList<ImageType>.Empty;
+            return new List<ImageType>();
         }
 
         /// <inheritdoc />
@@ -98,13 +93,6 @@ public Task<DynamicImageResponse> GetImage(BaseItem item, ImageType type, Cancel
                 return Task.FromResult(new DynamicImageResponse { HasImage = false });
             }
 
-            // Can't extract if we didn't find any video streams in the file
-            if (!video.DefaultVideoStreamIndex.HasValue)
-            {
-                _logger.LogInformation("Skipping image extraction due to missing DefaultVideoStreamIndex for {Path}.", video.Path ?? string.Empty);
-                return Task.FromResult(new DynamicImageResponse { HasImage = false });
-            }
-
             return GetEmbeddedImage(video, type, cancellationToken);
         }
 
@@ -128,24 +116,29 @@ private async Task<DynamicImageResponse> GetEmbeddedImage(Video item, ImageType
             // Try attachments first
             var attachmentSources = item.GetMediaSources(false).SelectMany(source => source.MediaAttachments).ToList();
             var attachmentStream = attachmentSources
-                .Where(stream => !string.IsNullOrEmpty(stream.FileName))
-                .First(stream => imageFileNames.Any(name => stream.FileName.Contains(name, StringComparison.OrdinalIgnoreCase)));
+                .Where(attachment => !string.IsNullOrEmpty(attachment.FileName))
+                .FirstOrDefault(attachment => imageFileNames.Any(name => attachment.FileName.Contains(name, StringComparison.OrdinalIgnoreCase)));
 
             if (attachmentStream != null)
             {
-                var extension = (string.IsNullOrEmpty(attachmentStream.MimeType) ?
+                var extension = string.IsNullOrEmpty(attachmentStream.MimeType) ?
                     Path.GetExtension(attachmentStream.FileName) :
-                    MimeTypes.ToExtension(attachmentStream.MimeType)) ?? "jpg";
+                    MimeTypes.ToExtension(attachmentStream.MimeType);
+
+                if (string.IsNullOrEmpty(extension))
+                {
+                    extension = ".jpg";
+                }
 
                 string extractedAttachmentPath = await _mediaEncoder.ExtractVideoImage(item.Path, item.Container, mediaSource, null, attachmentStream.Index, extension, cancellationToken).ConfigureAwait(false);
 
                 ImageFormat format = extension switch
                 {
-                    "bmp" => ImageFormat.Bmp,
-                    "gif" => ImageFormat.Gif,
-                    "jpg" => ImageFormat.Jpg,
-                    "png" => ImageFormat.Png,
-                    "webp" => ImageFormat.Webp,
+                    ".bmp" => ImageFormat.Bmp,
+                    ".gif" => ImageFormat.Gif,
+                    ".jpg" => ImageFormat.Jpg,
+                    ".png" => ImageFormat.Png,
+                    ".webp" => ImageFormat.Webp,
                     _ => ImageFormat.Jpg
                 };
 
@@ -170,7 +163,7 @@ private async Task<DynamicImageResponse> GetEmbeddedImage(Video item, ImageType
             // Extract first stream containing an element of imageFileNames
             var imageStream = imageStreams
                 .Where(stream => !string.IsNullOrEmpty(stream.Comment))
-                .First(stream => imageFileNames.Any(name => stream.Comment.Contains(name, StringComparison.OrdinalIgnoreCase)));
+                .FirstOrDefault(stream => imageFileNames.Any(name => stream.Comment.Contains(name, StringComparison.OrdinalIgnoreCase)));
 
             // Primary type only: default to first image if none found by label
             if (imageStream == null)

MediaBrowser.Providers/MediaInfo/VideoImageProvider.cs

@@ -81,7 +81,14 @@ private async Task<DynamicImageResponse> GetVideoImage(Video item, CancellationT
                                   ? TimeSpan.FromTicks(item.RunTimeTicks.Value / 10)
                                   : TimeSpan.FromSeconds(10);
 
-            var videoStream = item.GetMediaStreams().FirstOrDefault(i => i.Type == MediaStreamType.Video);
+            var videoStream = item.GetDefaultVideoStream() ?? item.GetMediaStreams().FirstOrDefault(i => i.Type == MediaStreamType.Video);
+
+            if (videoStream == null)
+            {
+                _logger.LogInformation("Skipping image extraction: no video stream found for {Path}.", item.Path ?? string.Empty);
+                return new DynamicImageResponse { HasImage = false };
+            }
+
             string extractedImagePath = await _mediaEncoder.ExtractVideoImage(item.Path, item.Container, mediaSource, videoStream, item.Video3DFormat, imageOffset, cancellationToken).ConfigureAwait(false);
 
             return new DynamicImageResponse

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

@@ -0,0 +1,211 @@
+using System.Collections.Generic;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+using MediaBrowser.Controller.Entities;
+using MediaBrowser.Controller.Entities.Movies;
+using MediaBrowser.Controller.Entities.TV;
+using MediaBrowser.Controller.MediaEncoding;
+using MediaBrowser.Model.Drawing;
+using MediaBrowser.Model.Dto;
+using MediaBrowser.Model.Entities;
+using MediaBrowser.Providers.MediaInfo;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.Providers.Tests.MediaInfo
+{
+    public class EmbeddedImageProviderTests
+    {
+        public static TheoryData<BaseItem> GetSupportedImages_Empty_TestData =>
+            new ()
+            {
+                new AudioBook(),
+                new BoxSet(),
+                new Series(),
+                new Season(),
+            };
+
+        public static TheoryData<BaseItem, IEnumerable<ImageType>> GetSupportedImages_Populated_TestData =>
+            new TheoryData<BaseItem, IEnumerable<ImageType>>
+            {
+                { new Episode(), new List<ImageType> { ImageType.Primary } },
+                { new Movie(), new List<ImageType> { ImageType.Logo, ImageType.Backdrop, ImageType.Primary } },
+            };
+
+        private EmbeddedImageProvider GetEmbeddedImageProvider(IMediaEncoder? mediaEncoder)
+        {
+            return new EmbeddedImageProvider(mediaEncoder);
+        }
+
+        [Theory]
+        [MemberData(nameof(GetSupportedImages_Empty_TestData))]
+        public void GetSupportedImages_Empty(BaseItem item)
+        {
+            var embeddedImageProvider = GetEmbeddedImageProvider(null);
+            Assert.False(embeddedImageProvider.GetSupportedImages(item).Any());
+        }
+
+        [Theory]
+        [MemberData(nameof(GetSupportedImages_Populated_TestData))]
+        public void GetSupportedImages_Populated(BaseItem item, IEnumerable<ImageType> expected)
+        {
+            var embeddedImageProvider = GetEmbeddedImageProvider(null);
+            var actual = embeddedImageProvider.GetSupportedImages(item);
+            Assert.Equal(expected.OrderBy(i => i.ToString()), actual.OrderBy(i => i.ToString()));
+        }
+
+        [Fact]
+        public async void GetImage_Empty_NoStreams()
+        {
+            var embeddedImageProvider = GetEmbeddedImageProvider(null);
+
+            var input = new Mock<Movie>();
+            input.Setup(movie => movie.GetMediaSources(It.IsAny<bool>()))
+                .Returns(new List<MediaSourceInfo>());
+            input.Setup(movie => movie.GetMediaStreams())
+                .Returns(new List<MediaStream>());
+
+            var actual = await embeddedImageProvider.GetImage(input.Object, ImageType.Primary, CancellationToken.None);
+            Assert.NotNull(actual);
+            Assert.False(actual.HasImage);
+        }
+
+        [Fact]
+        public async void GetImage_Empty_NoLabeledAttachments()
+        {
+            var embeddedImageProvider = GetEmbeddedImageProvider(null);
+
+            var input = new Mock<Movie>();
+            // add an attachment without a filename - has a list to look through but finds nothing
+            input.Setup(movie => movie.GetMediaSources(It.IsAny<bool>()))
+                .Returns(new List<MediaSourceInfo> { new () { MediaAttachments = new List<MediaAttachment> { new () } } });
+            input.Setup(movie => movie.GetMediaStreams())
+                .Returns(new List<MediaStream>());
+
+            var actual = await embeddedImageProvider.GetImage(input.Object, ImageType.Primary, CancellationToken.None);
+            Assert.NotNull(actual);
+            Assert.False(actual.HasImage);
+        }
+
+        [Fact]
+        public async void GetImage_Empty_NoEmbeddedLabeledBackdrop()
+        {
+            var embeddedImageProvider = GetEmbeddedImageProvider(null);
+
+            var input = new Mock<Movie>();
+            input.Setup(movie => movie.GetMediaSources(It.IsAny<bool>()))
+                .Returns(new List<MediaSourceInfo>());
+            input.Setup(movie => movie.GetMediaStreams())
+                .Returns(new List<MediaStream> { new () { Type = MediaStreamType.EmbeddedImage } });
+
+            var actual = await embeddedImageProvider.GetImage(input.Object, ImageType.Backdrop, CancellationToken.None);
+            Assert.NotNull(actual);
+            Assert.False(actual.HasImage);
+        }
+
+        [Fact]
+        public async void GetImage_Attached()
+        {
+            // first tests file extension detection, second uses mimetype, third defaults to jpg
+            MediaAttachment sampleAttachment1 = new () { FileName = "clearlogo.png", Index = 1 };
+            MediaAttachment sampleAttachment2 = new () { FileName = "backdrop", MimeType = "image/bmp", Index = 2 };
+            MediaAttachment sampleAttachment3 = new () { FileName = "poster", Index = 3 };
+            string targetPath1 = "path1.png";
+            string targetPath2 = "path2.bmp";
+            string targetPath3 = "path2.jpg";
+
+            var mediaEncoder = new Mock<IMediaEncoder>(MockBehavior.Strict);
+            mediaEncoder.Setup(encoder => encoder.ExtractVideoImage(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<MediaSourceInfo>(), It.IsAny<MediaStream>(), 1, ".png", CancellationToken.None))
+                .Returns(Task.FromResult(targetPath1));
+            mediaEncoder.Setup(encoder => encoder.ExtractVideoImage(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<MediaSourceInfo>(), It.IsAny<MediaStream>(), 2, ".bmp", CancellationToken.None))
+                .Returns(Task.FromResult(targetPath2));
+            mediaEncoder.Setup(encoder => encoder.ExtractVideoImage(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<MediaSourceInfo>(), It.IsAny<MediaStream>(), 3, ".jpg", CancellationToken.None))
+                .Returns(Task.FromResult(targetPath3));
+            var embeddedImageProvider = GetEmbeddedImageProvider(mediaEncoder.Object);
+
+            var input = new Mock<Movie>();
+            input.Setup(movie => movie.GetMediaSources(It.IsAny<bool>()))
+                .Returns(new List<MediaSourceInfo> { new () { MediaAttachments = new List<MediaAttachment> { sampleAttachment1, sampleAttachment2, sampleAttachment3 } } });
+            input.Setup(movie => movie.GetMediaStreams())
+                .Returns(new List<MediaStream>());
+
+            var actualLogo = await embeddedImageProvider.GetImage(input.Object, ImageType.Logo, CancellationToken.None);
+            Assert.NotNull(actualLogo);
+            Assert.True(actualLogo.HasImage);
+            Assert.Equal(targetPath1, actualLogo.Path);
+            Assert.Equal(ImageFormat.Png, actualLogo.Format);
+
+            var actualBackdrop = await embeddedImageProvider.GetImage(input.Object, ImageType.Backdrop, CancellationToken.None);
+            Assert.NotNull(actualBackdrop);
+            Assert.True(actualBackdrop.HasImage);
+            Assert.Equal(targetPath2, actualBackdrop.Path);
+            Assert.Equal(ImageFormat.Bmp, actualBackdrop.Format);
+
+            var actualPrimary = await embeddedImageProvider.GetImage(input.Object, ImageType.Primary, CancellationToken.None);
+            Assert.NotNull(actualPrimary);
+            Assert.True(actualPrimary.HasImage);
+            Assert.Equal(targetPath3, actualPrimary.Path);
+            Assert.Equal(ImageFormat.Jpg, actualPrimary.Format);
+        }
+
+        [Fact]
+        public async void GetImage_EmbeddedDefault()
+        {
+            MediaStream sampleStream = new () { Type = MediaStreamType.EmbeddedImage, Index = 1 };
+            string targetPath = "path";
+
+            var mediaEncoder = new Mock<IMediaEncoder>(MockBehavior.Strict);
+            mediaEncoder.Setup(encoder => encoder.ExtractVideoImage(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<MediaSourceInfo>(), sampleStream, 1, "jpg", CancellationToken.None))
+                .Returns(Task.FromResult(targetPath));
+            var embeddedImageProvider = GetEmbeddedImageProvider(mediaEncoder.Object);
+
+            var input = new Mock<Movie>();
+            input.Setup(movie => movie.GetMediaSources(It.IsAny<bool>()))
+                .Returns(new List<MediaSourceInfo>());
+            input.Setup(movie => movie.GetMediaStreams())
+                .Returns(new List<MediaStream>() { sampleStream });
+
+            var actual = await embeddedImageProvider.GetImage(input.Object, ImageType.Primary, CancellationToken.None);
+            Assert.NotNull(actual);
+            Assert.True(actual.HasImage);
+            Assert.Equal(targetPath, actual.Path);
+            Assert.Equal(ImageFormat.Jpg, actual.Format);
+        }
+
+        [Fact]
+        public async void GetImage_EmbeddedSelection()
+        {
+            // primary is second stream to ensure it's not defaulting, backdrop is first
+            MediaStream sampleStream1 = new () { Type = MediaStreamType.EmbeddedImage, Index = 1, Comment = "backdrop" };
+            MediaStream sampleStream2 = new () { Type = MediaStreamType.EmbeddedImage, Index = 2, Comment = "cover" };
+            string targetPath1 = "path1.jpg";
+            string targetPath2 = "path2.jpg";
+
+            var mediaEncoder = new Mock<IMediaEncoder>(MockBehavior.Strict);
+            mediaEncoder.Setup(encoder => encoder.ExtractVideoImage(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<MediaSourceInfo>(), sampleStream1, 1, "jpg", CancellationToken.None))
+                .Returns(Task.FromResult(targetPath1));
+            mediaEncoder.Setup(encoder => encoder.ExtractVideoImage(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<MediaSourceInfo>(), sampleStream2, 2, "jpg", CancellationToken.None))
+                .Returns(Task.FromResult(targetPath2));
+            var embeddedImageProvider = GetEmbeddedImageProvider(mediaEncoder.Object);
+
+            var input = new Mock<Movie>();
+            input.Setup(movie => movie.GetMediaSources(It.IsAny<bool>()))
+                .Returns(new List<MediaSourceInfo>());
+            input.Setup(movie => movie.GetMediaStreams())
+                .Returns(new List<MediaStream> { sampleStream1, sampleStream2 });
+
+            var actualPrimary = await embeddedImageProvider.GetImage(input.Object, ImageType.Primary, CancellationToken.None);
+            Assert.NotNull(actualPrimary);
+            Assert.True(actualPrimary.HasImage);
+            Assert.Equal(targetPath2, actualPrimary.Path);
+            Assert.Equal(ImageFormat.Jpg, actualPrimary.Format);
+
+            var actualBackdrop = await embeddedImageProvider.GetImage(input.Object, ImageType.Backdrop, CancellationToken.None);
+            Assert.NotNull(actualBackdrop);
+            Assert.True(actualBackdrop.HasImage);
+            Assert.Equal(targetPath1, actualBackdrop.Path);
+            Assert.Equal(ImageFormat.Jpg, actualBackdrop.Format);
+        }
+    }
+}