From 9ba7bf96ef4fd7a8042c594bede42f4f8b7ef329 Mon Sep 17 00:00:00 2001 From: Joe Rogers <1337joe@gmail.com> Date: Thu, 18 Nov 2021 15:04:30 +0100 Subject: [PATCH] Query MediaSourceManager directly in image providers Add doc comments/minor tweaks to AudioImageProvider --- MediaBrowser.Controller/Entities/BaseItem.cs | 8 +-- .../LiveTv/LiveTvChannel.cs | 5 -- .../MediaInfo/AudioImageProvider.cs | 37 ++++++++---- .../MediaInfo/EmbeddedImageProvider.cs | 16 ++++-- .../MediaInfo/VideoImageProvider.cs | 16 ++++-- .../MediaInfo/EmbeddedImageProviderTests.cs | 40 ++++++------- .../MediaInfo/VideoImageProviderTests.cs | 57 +++++++++---------- 7 files changed, 99 insertions(+), 80 deletions(-) diff --git a/MediaBrowser.Controller/Entities/BaseItem.cs b/MediaBrowser.Controller/Entities/BaseItem.cs index 853488a1d0..a76ca23055 100644 --- a/MediaBrowser.Controller/Entities/BaseItem.cs +++ b/MediaBrowser.Controller/Entities/BaseItem.cs @@ -1107,16 +1107,10 @@ namespace MediaBrowser.Controller.Entities } public virtual List GetMediaStreams() - { - return GetMediaStreams(null); - } - - public virtual List GetMediaStreams(MediaStreamType? type) { return MediaSourceManager.GetMediaStreams(new MediaStreamQuery { - ItemId = Id, - Type = type + ItemId = Id }); } diff --git a/MediaBrowser.Controller/LiveTv/LiveTvChannel.cs b/MediaBrowser.Controller/LiveTv/LiveTvChannel.cs index d01009a536..074e023e8d 100644 --- a/MediaBrowser.Controller/LiveTv/LiveTvChannel.cs +++ b/MediaBrowser.Controller/LiveTv/LiveTvChannel.cs @@ -154,11 +154,6 @@ namespace MediaBrowser.Controller.LiveTv return new List(); } - public override List GetMediaStreams(MediaStreamType? type) - { - return new List(); - } - protected override string GetInternalMetadataPath(string basePath) { return System.IO.Path.Combine(basePath, "livetv", Id.ToString("N", CultureInfo.InvariantCulture), "metadata"); diff --git a/MediaBrowser.Providers/MediaInfo/AudioImageProvider.cs b/MediaBrowser.Providers/MediaInfo/AudioImageProvider.cs index 6b662ebc44..b4b1895f51 100644 --- a/MediaBrowser.Providers/MediaInfo/AudioImageProvider.cs +++ b/MediaBrowser.Providers/MediaInfo/AudioImageProvider.cs @@ -1,7 +1,5 @@ #nullable disable -#pragma warning disable CA1002, CS1591 - using System; using System.Collections.Generic; using System.Globalization; @@ -13,7 +11,9 @@ using MediaBrowser.Common.Extensions; using MediaBrowser.Controller.Configuration; using MediaBrowser.Controller.Entities; using MediaBrowser.Controller.Entities.Audio; +using MediaBrowser.Controller.Library; using MediaBrowser.Controller.MediaEncoding; +using MediaBrowser.Controller.Persistence; using MediaBrowser.Controller.Providers; using MediaBrowser.Model.Entities; using MediaBrowser.Model.IO; @@ -21,35 +21,49 @@ using MediaBrowser.Model.IO; namespace MediaBrowser.Providers.MediaInfo { /// - /// Uses ffmpeg to create video images. + /// Uses to extract embedded images. /// public class AudioImageProvider : IDynamicImageProvider { + private readonly IMediaSourceManager _mediaSourceManager; private readonly IMediaEncoder _mediaEncoder; private readonly IServerConfigurationManager _config; private readonly IFileSystem _fileSystem; - public AudioImageProvider(IMediaEncoder mediaEncoder, IServerConfigurationManager config, IFileSystem fileSystem) + /// + /// Initializes a new instance of the class. + /// + /// The media source manager for fetching item streams. + /// The media encoder for extracting embedded images. + /// The server configuration manager for getting image paths. + /// The filesystem. + public AudioImageProvider(IMediaSourceManager mediaSourceManager, IMediaEncoder mediaEncoder, IServerConfigurationManager config, IFileSystem fileSystem) { + _mediaSourceManager = mediaSourceManager; _mediaEncoder = mediaEncoder; _config = config; _fileSystem = fileSystem; } - public string AudioImagesPath => Path.Combine(_config.ApplicationPaths.CachePath, "extracted-audio-images"); + private string AudioImagesPath => Path.Combine(_config.ApplicationPaths.CachePath, "extracted-audio-images"); + /// public string Name => "Image Extractor"; + /// public IEnumerable GetSupportedImages(BaseItem item) { - return new List { ImageType.Primary }; + return new[] { ImageType.Primary }; } + /// public Task GetImage(BaseItem item, ImageType type, CancellationToken cancellationToken) { - var audio = (Audio)item; - - var imageStreams = audio.GetMediaStreams(MediaStreamType.EmbeddedImage); + var imageStreams = _mediaSourceManager.GetMediaStreams(new MediaStreamQuery + { + ItemId = item.Id, + Type = MediaStreamType.EmbeddedImage + }); // Can't extract if we didn't find a video stream in the file if (imageStreams.Count == 0) @@ -60,7 +74,7 @@ namespace MediaBrowser.Providers.MediaInfo return GetImage((Audio)item, imageStreams, cancellationToken); } - public async Task GetImage(Audio item, List imageStreams, CancellationToken cancellationToken) + private async Task GetImage(Audio item, List imageStreams, CancellationToken cancellationToken) { var path = GetAudioImagePath(item); @@ -72,7 +86,7 @@ namespace MediaBrowser.Providers.MediaInfo imageStreams.FirstOrDefault(i => (i.Comment ?? string.Empty).IndexOf("cover", StringComparison.OrdinalIgnoreCase) != -1) ?? imageStreams.FirstOrDefault(); - var imageStreamIndex = imageStream == null ? (int?)null : imageStream.Index; + var imageStreamIndex = imageStream?.Index; var tempFile = await _mediaEncoder.ExtractAudioImage(item.Path, imageStreamIndex, cancellationToken).ConfigureAwait(false); @@ -124,6 +138,7 @@ namespace MediaBrowser.Providers.MediaInfo return Path.Join(AudioImagesPath, prefix, filename); } + /// public bool Supports(BaseItem item) { if (item.IsShortcut) diff --git a/MediaBrowser.Providers/MediaInfo/EmbeddedImageProvider.cs b/MediaBrowser.Providers/MediaInfo/EmbeddedImageProvider.cs index e27fd094c4..96d7d139a4 100644 --- a/MediaBrowser.Providers/MediaInfo/EmbeddedImageProvider.cs +++ b/MediaBrowser.Providers/MediaInfo/EmbeddedImageProvider.cs @@ -8,7 +8,9 @@ using System.Threading; using System.Threading.Tasks; using MediaBrowser.Controller.Entities; using MediaBrowser.Controller.Entities.TV; +using MediaBrowser.Controller.Library; using MediaBrowser.Controller.MediaEncoding; +using MediaBrowser.Controller.Persistence; using MediaBrowser.Controller.Providers; using MediaBrowser.Model.Drawing; using MediaBrowser.Model.Dto; @@ -45,16 +47,19 @@ namespace MediaBrowser.Providers.MediaInfo "logo", }; + private readonly IMediaSourceManager _mediaSourceManager; private readonly IMediaEncoder _mediaEncoder; private readonly ILogger _logger; /// /// Initializes a new instance of the class. /// + /// The media source manager for fetching item streams and attachments. /// The media encoder for extracting attached/embedded images. /// The logger. - public EmbeddedImageProvider(IMediaEncoder mediaEncoder, ILogger logger) + public EmbeddedImageProvider(IMediaSourceManager mediaSourceManager, IMediaEncoder mediaEncoder, ILogger logger) { + _mediaSourceManager = mediaSourceManager; _mediaEncoder = mediaEncoder; _logger = logger; } @@ -128,8 +133,7 @@ namespace MediaBrowser.Providers.MediaInfo } // Try attachments first - var attachmentStream = item.GetMediaSources(false) - .SelectMany(source => source.MediaAttachments) + var attachmentStream = _mediaSourceManager.GetMediaAttachments(item.Id) .FirstOrDefault(attachment => !string.IsNullOrEmpty(attachment.FileName) && imageFileNames.Any(name => attachment.FileName.Contains(name, StringComparison.OrdinalIgnoreCase))); @@ -139,7 +143,11 @@ namespace MediaBrowser.Providers.MediaInfo } // Fall back to EmbeddedImage streams - var imageStreams = item.GetMediaStreams(MediaStreamType.EmbeddedImage); + var imageStreams = _mediaSourceManager.GetMediaStreams(new MediaStreamQuery + { + ItemId = item.Id, + Type = MediaStreamType.EmbeddedImage + }); if (imageStreams.Count == 0) { diff --git a/MediaBrowser.Providers/MediaInfo/VideoImageProvider.cs b/MediaBrowser.Providers/MediaInfo/VideoImageProvider.cs index a90ef4739d..ad44b8b0d9 100644 --- a/MediaBrowser.Providers/MediaInfo/VideoImageProvider.cs +++ b/MediaBrowser.Providers/MediaInfo/VideoImageProvider.cs @@ -4,7 +4,9 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; using MediaBrowser.Controller.Entities; +using MediaBrowser.Controller.Library; using MediaBrowser.Controller.MediaEncoding; +using MediaBrowser.Controller.Persistence; using MediaBrowser.Controller.Providers; using MediaBrowser.Model.Drawing; using MediaBrowser.Model.Dto; @@ -19,16 +21,19 @@ namespace MediaBrowser.Providers.MediaInfo /// public class VideoImageProvider : IDynamicImageProvider, IHasOrder { + private readonly IMediaSourceManager _mediaSourceManager; private readonly IMediaEncoder _mediaEncoder; private readonly ILogger _logger; /// /// Initializes a new instance of the class. /// + /// The media source manager for fetching item streams. /// The media encoder for capturing images. /// The logger. - public VideoImageProvider(IMediaEncoder mediaEncoder, ILogger logger) + public VideoImageProvider(IMediaSourceManager mediaSourceManager, IMediaEncoder mediaEncoder, ILogger logger) { + _mediaSourceManager = mediaSourceManager; _mediaEncoder = mediaEncoder; _logger = logger; } @@ -78,12 +83,15 @@ namespace MediaBrowser.Providers.MediaInfo // If we know the duration, grab it from 10% into the video. Otherwise just 10 seconds in. // Always use 10 seconds for dvd because our duration could be out of whack - var imageOffset = item.VideoType != VideoType.Dvd && item.RunTimeTicks.HasValue && - item.RunTimeTicks.Value > 0 + var imageOffset = item.VideoType != VideoType.Dvd && item.RunTimeTicks is > 0 ? TimeSpan.FromTicks(item.RunTimeTicks.Value / 10) : TimeSpan.FromSeconds(10); - var videoStream = item.GetDefaultVideoStream() ?? item.GetMediaStreams(MediaStreamType.Video).FirstOrDefault(); + var defaultQuery = new MediaStreamQuery { ItemId = item.Id, Index = item.DefaultVideoStreamIndex }; + var videoQuery = new MediaStreamQuery { ItemId = item.Id, Type = MediaStreamType.Video }; + + var videoStream = _mediaSourceManager.GetMediaStreams(defaultQuery).FirstOrDefault() + ?? _mediaSourceManager.GetMediaStreams(videoQuery).FirstOrDefault(); if (videoStream == null) { diff --git a/tests/Jellyfin.Providers.Tests/MediaInfo/EmbeddedImageProviderTests.cs b/tests/Jellyfin.Providers.Tests/MediaInfo/EmbeddedImageProviderTests.cs index ede285c2e2..5583218101 100644 --- a/tests/Jellyfin.Providers.Tests/MediaInfo/EmbeddedImageProviderTests.cs +++ b/tests/Jellyfin.Providers.Tests/MediaInfo/EmbeddedImageProviderTests.cs @@ -6,7 +6,9 @@ using System.Threading.Tasks; using MediaBrowser.Controller.Entities; using MediaBrowser.Controller.Entities.Movies; using MediaBrowser.Controller.Entities.TV; +using MediaBrowser.Controller.Library; using MediaBrowser.Controller.MediaEncoding; +using MediaBrowser.Controller.Persistence; using MediaBrowser.Model.Drawing; using MediaBrowser.Model.Dto; using MediaBrowser.Model.Entities; @@ -29,7 +31,7 @@ namespace Jellyfin.Providers.Tests.MediaInfo public void GetSupportedImages_AnyBaseItem_ReturnsExpected(Type type, params ImageType[] expected) { BaseItem item = (BaseItem)Activator.CreateInstance(type)!; - var embeddedImageProvider = new EmbeddedImageProvider(Mock.Of(), new NullLogger()); + var embeddedImageProvider = new EmbeddedImageProvider(Mock.Of(), Mock.Of(), new NullLogger()); var actual = embeddedImageProvider.GetSupportedImages(item); Assert.Equal(expected.OrderBy(i => i.ToString()), actual.OrderBy(i => i.ToString())); } @@ -37,9 +39,10 @@ namespace Jellyfin.Providers.Tests.MediaInfo [Fact] public async void GetImage_NoStreams_ReturnsNoImage() { - var embeddedImageProvider = new EmbeddedImageProvider(null, new NullLogger()); + var input = new Movie(); - var input = GetMovie(new List(), new List()); + var mediaSourceManager = GetMediaSourceManager(input, new List(), new List()); + var embeddedImageProvider = new EmbeddedImageProvider(mediaSourceManager, null, new NullLogger()); var actual = await embeddedImageProvider.GetImage(input, ImageType.Primary, CancellationToken.None); Assert.NotNull(actual); @@ -67,12 +70,13 @@ namespace Jellyfin.Providers.Tests.MediaInfo }); } + var input = new Movie(); + var mediaEncoder = new Mock(MockBehavior.Strict); mediaEncoder.Setup(encoder => encoder.ExtractVideoImage(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .Returns((_, _, _, _, index, ext, _) => Task.FromResult(pathPrefix + index + "." + ext)); - var embeddedImageProvider = new EmbeddedImageProvider(mediaEncoder.Object, new NullLogger()); - - var input = GetMovie(attachments, new List()); + var mediaSourceManager = GetMediaSourceManager(input, attachments, new List()); + var embeddedImageProvider = new EmbeddedImageProvider(mediaSourceManager, mediaEncoder.Object, new NullLogger()); var actual = await embeddedImageProvider.GetImage(input, type, CancellationToken.None); Assert.NotNull(actual); @@ -112,6 +116,8 @@ namespace Jellyfin.Providers.Tests.MediaInfo }); } + var input = new Movie(); + var pathPrefix = "path"; var mediaEncoder = new Mock(MockBehavior.Strict); mediaEncoder.Setup(encoder => encoder.ExtractVideoImage(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) @@ -120,9 +126,8 @@ namespace Jellyfin.Providers.Tests.MediaInfo Assert.Equal(streams[index - 1], stream); return Task.FromResult(pathPrefix + index + "." + ext); }); - var embeddedImageProvider = new EmbeddedImageProvider(mediaEncoder.Object, new NullLogger()); - - var input = GetMovie(new List(), streams); + var mediaSourceManager = GetMediaSourceManager(input, new List(), streams); + var embeddedImageProvider = new EmbeddedImageProvider(mediaSourceManager, mediaEncoder.Object, new NullLogger()); var actual = await embeddedImageProvider.GetImage(input, type, CancellationToken.None); Assert.NotNull(actual); @@ -138,19 +143,14 @@ namespace Jellyfin.Providers.Tests.MediaInfo } } - private static Movie GetMovie(List mediaAttachments, List mediaStreams) + private static IMediaSourceManager GetMediaSourceManager(BaseItem item, List mediaAttachments, List mediaStreams) { - // Mocking IMediaSourceManager GetMediaAttachments and GetMediaStreams instead of mocking Movie works, but - // has concurrency problems between this and VideoImageProviderTests due to BaseItem.MediaSourceManager - // being static - var movie = new Mock(); - - movie.Setup(item => item.GetMediaSources(It.IsAny())) - .Returns(new List { new () { MediaAttachments = mediaAttachments } } ); - movie.Setup(item => item.GetMediaStreams(MediaStreamType.EmbeddedImage)) + var mediaSourceManager = new Mock(MockBehavior.Strict); + mediaSourceManager.Setup(i => i.GetMediaAttachments(item.Id)) + .Returns(mediaAttachments); + mediaSourceManager.Setup(i => i.GetMediaStreams(It.Is(q => q.ItemId == item.Id && q.Type == MediaStreamType.EmbeddedImage))) .Returns(mediaStreams); - - return movie.Object; + return mediaSourceManager.Object; } } } diff --git a/tests/Jellyfin.Providers.Tests/MediaInfo/VideoImageProviderTests.cs b/tests/Jellyfin.Providers.Tests/MediaInfo/VideoImageProviderTests.cs index 3a2daf9f2c..839925dd10 100644 --- a/tests/Jellyfin.Providers.Tests/MediaInfo/VideoImageProviderTests.cs +++ b/tests/Jellyfin.Providers.Tests/MediaInfo/VideoImageProviderTests.cs @@ -4,7 +4,9 @@ using System.Threading; using System.Threading.Tasks; using MediaBrowser.Controller.Entities; using MediaBrowser.Controller.Entities.Movies; +using MediaBrowser.Controller.Library; using MediaBrowser.Controller.MediaEncoding; +using MediaBrowser.Controller.Persistence; using MediaBrowser.Model.Drawing; using MediaBrowser.Model.Dto; using MediaBrowser.Model.Entities; @@ -26,7 +28,7 @@ namespace Jellyfin.Providers.Tests.MediaInfo new Movie { DefaultVideoStreamIndex = null }, // set a default index but don't put anything there (invalid input, but provider shouldn't break) - GetMovie(0, null, new List()) + new Movie { DefaultVideoStreamIndex = 0 } }; } @@ -34,7 +36,8 @@ namespace Jellyfin.Providers.Tests.MediaInfo [MemberData(nameof(GetImage_UnsupportedInput_ReturnsNoImage_TestData))] public async void GetImage_UnsupportedInput_ReturnsNoImage(Video input) { - var videoImageProvider = GetVideoImageProvider(null); + var mediaSourceManager = GetMediaSourceManager(input, null, new List()); + var videoImageProvider = new VideoImageProvider(mediaSourceManager, Mock.Of(), new NullLogger()); var actual = await videoImageProvider.GetImage(input, ImageType.Primary, CancellationToken.None); Assert.NotNull(actual); @@ -46,6 +49,8 @@ namespace Jellyfin.Providers.Tests.MediaInfo [InlineData(5, 0)] // default out of valid range public async void GetImage_DefaultVideoStreams_ReturnsCorrectStreamImage(int defaultIndex, int targetIndex) { + var input = new Movie { DefaultVideoStreamIndex = defaultIndex }; + string targetPath = "path.jpg"; var mediaStreams = new List(); var mediaEncoder = new Mock(MockBehavior.Strict); @@ -60,10 +65,10 @@ namespace Jellyfin.Providers.Tests.MediaInfo .Returns(Task.FromResult(path)); } - var videoImageProvider = GetVideoImageProvider(mediaEncoder.Object); - var defaultStream = defaultIndex < mediaStreams.Count ? mediaStreams[targetIndex] : null; - var input = GetMovie(defaultIndex, defaultStream, mediaStreams ); + var mediaSourceManager = GetMediaSourceManager(input, defaultStream, mediaStreams); + + var videoImageProvider = new VideoImageProvider(mediaSourceManager, mediaEncoder.Object, new NullLogger()); var actual = await videoImageProvider.GetImage(input, ImageType.Primary, CancellationToken.None); Assert.NotNull(actual); @@ -78,6 +83,13 @@ namespace Jellyfin.Providers.Tests.MediaInfo public async void GetImage_TimeSpan_SelectsCorrectTime(int? runTimeSeconds, long expectedSeconds) { MediaStream targetStream = new () { Type = MediaStreamType.Video, Index = 0 }; + var input = new Movie + { + DefaultVideoStreamIndex = 0, + RunTimeTicks = runTimeSeconds * TimeSpan.TicksPerSecond + }; + + var mediaSourceManager = GetMediaSourceManager(input, targetStream, new List { targetStream }); // use a callback to catch the actual value // provides more information on failure than verifying a specific input was called on the mock @@ -86,10 +98,8 @@ namespace Jellyfin.Providers.Tests.MediaInfo mediaEncoder.Setup(encoder => encoder.ExtractVideoImage(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), CancellationToken.None)) .Callback((_, _, _, _, _, timeSpan, _) => actualTimeSpan = timeSpan) .Returns(Task.FromResult("path")); - var videoImageProvider = GetVideoImageProvider(mediaEncoder.Object); - var input = GetMovie(0, targetStream, new List { targetStream }); - input.RunTimeTicks = runTimeSeconds * TimeSpan.TicksPerSecond; + var videoImageProvider = new VideoImageProvider(mediaSourceManager, mediaEncoder.Object, new NullLogger()); // not testing return, just verifying what gets requested for time span await videoImageProvider.GetImage(input, ImageType.Primary, CancellationToken.None); @@ -97,31 +107,20 @@ namespace Jellyfin.Providers.Tests.MediaInfo Assert.Equal(TimeSpan.FromSeconds(expectedSeconds), actualTimeSpan); } - private static VideoImageProvider GetVideoImageProvider(IMediaEncoder? mediaEncoder) + private static IMediaSourceManager GetMediaSourceManager(Video item, MediaStream? defaultStream, List mediaStreams) { - // strict to ensure this isn't accidentally used where a prepared mock is intended - mediaEncoder ??= new Mock(MockBehavior.Strict).Object; - return new VideoImageProvider(mediaEncoder, new NullLogger()); - } - - private static Movie GetMovie(int defaultVideoStreamIndex, MediaStream? defaultStream, List mediaStreams) - { - // Mocking IMediaSourceManager GetMediaStreams instead of mocking Movie works, but has concurrency problems - // between this and EmbeddedImageProviderTests due to BaseItem.MediaSourceManager being static - var movie = new Mock + var defaultStreamList = new List(); + if (defaultStream != null) { - Object = - { - DefaultVideoStreamIndex = defaultVideoStreamIndex - } - }; + defaultStreamList.Add(defaultStream); + } - movie.Setup(item => item.GetDefaultVideoStream()) - .Returns(defaultStream!); - movie.Setup(item => item.GetMediaStreams(MediaStreamType.Video)) + var mediaSourceManager = new Mock(MockBehavior.Strict); + mediaSourceManager.Setup(i => i.GetMediaStreams(It.Is(q => q.ItemId == item.Id && q.Index == item.DefaultVideoStreamIndex))) + .Returns(defaultStreamList); + mediaSourceManager.Setup(i => i.GetMediaStreams(It.Is(q => q.ItemId == item.Id && q.Type == MediaStreamType.Video))) .Returns(mediaStreams); - - return movie.Object; + return mediaSourceManager.Object; } } }