From ad4d7e4cfdbec8d971aec07c4f07a64e9f557add Mon Sep 17 00:00:00 2001 From: ta264 Date: Sun, 25 Aug 2019 20:46:19 +0100 Subject: [PATCH] New: Use ImageSharp for resizing (#934) * New: Swap to ImageSharp for image resizing to avoid leaks Stop resizing album images also * Fixed: MediaCoverModule falls back to fullsize for png and gif too * Fixed: Look for all image extensions in DeleteBadMediaCovers.cs --- .../MediaCovers/MediaCoverModule.cs | 6 +-- .../Frontend/Mappers/MediaCoverMapper.cs | 4 +- .../MediaCoverServiceFixture.cs | 8 ++-- .../Housekeepers/DeleteBadMediaCovers.cs | 4 +- src/NzbDrone.Core/Lidarr.Core.csproj | 2 +- .../MediaCover/GdiPlusInterop.cs | 44 ------------------- src/NzbDrone.Core/MediaCover/ImageResizer.cs | 24 +++++----- .../MediaCover/MediaCoverService.cs | 30 +------------ 8 files changed, 25 insertions(+), 97 deletions(-) delete mode 100644 src/NzbDrone.Core/MediaCover/GdiPlusInterop.cs diff --git a/src/Lidarr.Api.V1/MediaCovers/MediaCoverModule.cs b/src/Lidarr.Api.V1/MediaCovers/MediaCoverModule.cs index 70d0cee8c..5914988b7 100644 --- a/src/Lidarr.Api.V1/MediaCovers/MediaCoverModule.cs +++ b/src/Lidarr.Api.V1/MediaCovers/MediaCoverModule.cs @@ -10,7 +10,7 @@ namespace Lidarr.Api.V1.MediaCovers { public class MediaCoverModule : LidarrV1Module { - private static readonly Regex RegexResizedImage = new Regex(@"-\d+\.jpg$", RegexOptions.Compiled | RegexOptions.IgnoreCase); + private static readonly Regex RegexResizedImage = new Regex(@"-\d+(?=\.(jpg|png|gif)$)", RegexOptions.Compiled | RegexOptions.IgnoreCase); private const string MEDIA_COVER_ARTIST_ROUTE = @"/Artist/(?\d+)/(?(.+)\.(jpg|png|gif))"; private const string MEDIA_COVER_ALBUM_ROUTE = @"/Album/(?\d+)/(?(.+)\.(jpg|png|gif))"; @@ -35,7 +35,7 @@ namespace Lidarr.Api.V1.MediaCovers { // Return the full sized image if someone requests a non-existing resized one. // TODO: This code can be removed later once everyone had the update for a while. - var basefilePath = RegexResizedImage.Replace(filePath, ".jpg"); + var basefilePath = RegexResizedImage.Replace(filePath, ""); if (basefilePath == filePath || !_diskProvider.FileExists(basefilePath)) { return new NotFoundResponse(); @@ -54,7 +54,7 @@ namespace Lidarr.Api.V1.MediaCovers { // Return the full sized image if someone requests a non-existing resized one. // TODO: This code can be removed later once everyone had the update for a while. - var basefilePath = RegexResizedImage.Replace(filePath, ".jpg"); + var basefilePath = RegexResizedImage.Replace(filePath, ""); if (basefilePath == filePath || !_diskProvider.FileExists(basefilePath)) { return new NotFoundResponse(); diff --git a/src/Lidarr.Http/Frontend/Mappers/MediaCoverMapper.cs b/src/Lidarr.Http/Frontend/Mappers/MediaCoverMapper.cs index 58d46916e..b794ad686 100644 --- a/src/Lidarr.Http/Frontend/Mappers/MediaCoverMapper.cs +++ b/src/Lidarr.Http/Frontend/Mappers/MediaCoverMapper.cs @@ -10,7 +10,7 @@ namespace Lidarr.Http.Frontend.Mappers { public class MediaCoverMapper : StaticResourceMapperBase { - private static readonly Regex RegexResizedImage = new Regex(@"-\d+\.jpg($|\?)", RegexOptions.Compiled | RegexOptions.IgnoreCase); + private static readonly Regex RegexResizedImage = new Regex(@"-\d+(?=\.(jpg|png|gif)($|\?))", RegexOptions.Compiled | RegexOptions.IgnoreCase); private readonly IAppFolderInfo _appFolderInfo; private readonly IDiskProvider _diskProvider; @@ -31,7 +31,7 @@ namespace Lidarr.Http.Frontend.Mappers if (!_diskProvider.FileExists(resourcePath) || _diskProvider.GetFileSize(resourcePath) == 0) { - var baseResourcePath = RegexResizedImage.Replace(resourcePath, ".jpg$1"); + var baseResourcePath = RegexResizedImage.Replace(resourcePath, ""); if (baseResourcePath != resourcePath) { return baseResourcePath; diff --git a/src/NzbDrone.Core.Test/MediaCoverTests/MediaCoverServiceFixture.cs b/src/NzbDrone.Core.Test/MediaCoverTests/MediaCoverServiceFixture.cs index 03c687ab2..f05bb6406 100644 --- a/src/NzbDrone.Core.Test/MediaCoverTests/MediaCoverServiceFixture.cs +++ b/src/NzbDrone.Core.Test/MediaCoverTests/MediaCoverServiceFixture.cs @@ -153,7 +153,7 @@ namespace NzbDrone.Core.Test.MediaCoverTests Subject.HandleAsync(new ArtistRefreshCompleteEvent(_artist)); Mocker.GetMock() - .Verify(v => v.Resize(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(3)); + .Verify(v => v.Resize(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); } [Test] @@ -174,7 +174,7 @@ namespace NzbDrone.Core.Test.MediaCoverTests Subject.HandleAsync(new ArtistRefreshCompleteEvent(_artist)); Mocker.GetMock() - .Verify(v => v.Resize(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(3)); + .Verify(v => v.Resize(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); } [Test] @@ -224,7 +224,7 @@ namespace NzbDrone.Core.Test.MediaCoverTests Subject.HandleAsync(new ArtistRefreshCompleteEvent(_artist)); Mocker.GetMock() - .Verify(v => v.Resize(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(3)); + .Verify(v => v.Resize(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); } [Test] @@ -249,7 +249,7 @@ namespace NzbDrone.Core.Test.MediaCoverTests Subject.HandleAsync(new ArtistRefreshCompleteEvent(_artist)); Mocker.GetMock() - .Verify(v => v.Resize(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(3)); + .Verify(v => v.Resize(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); } } } diff --git a/src/NzbDrone.Core/Housekeeping/Housekeepers/DeleteBadMediaCovers.cs b/src/NzbDrone.Core/Housekeeping/Housekeepers/DeleteBadMediaCovers.cs index d61df11b3..df2fd5389 100644 --- a/src/NzbDrone.Core/Housekeeping/Housekeepers/DeleteBadMediaCovers.cs +++ b/src/NzbDrone.Core/Housekeeping/Housekeepers/DeleteBadMediaCovers.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.IO; using System.Linq; using NLog; @@ -35,11 +36,12 @@ namespace NzbDrone.Core.Housekeeping.Housekeepers if (!_configService.CleanupMetadataImages) return; var artists = _artistService.GetAllArtists(); + var imageExtensions = new List { ".jpg", ".png", ".gif" }; foreach (var artist in artists) { var images = _metaFileService.GetFilesByArtist(artist.Id) - .Where(c => c.LastUpdated > new DateTime(2014, 12, 27) && c.RelativePath.EndsWith(".jpg", StringComparison.InvariantCultureIgnoreCase)); + .Where(c => c.LastUpdated > new DateTime(2014, 12, 27) && imageExtensions.Any(x => c.RelativePath.EndsWith(x, StringComparison.InvariantCultureIgnoreCase))); foreach (var image in images) { diff --git a/src/NzbDrone.Core/Lidarr.Core.csproj b/src/NzbDrone.Core/Lidarr.Core.csproj index 2b76d4fc8..f1ab32a5c 100644 --- a/src/NzbDrone.Core/Lidarr.Core.csproj +++ b/src/NzbDrone.Core/Lidarr.Core.csproj @@ -6,7 +6,6 @@ - @@ -16,6 +15,7 @@ + diff --git a/src/NzbDrone.Core/MediaCover/GdiPlusInterop.cs b/src/NzbDrone.Core/MediaCover/GdiPlusInterop.cs deleted file mode 100644 index 659a15d41..000000000 --- a/src/NzbDrone.Core/MediaCover/GdiPlusInterop.cs +++ /dev/null @@ -1,44 +0,0 @@ -using System; -using System.Drawing; -using NzbDrone.Common.EnvironmentInfo; - -namespace NzbDrone.Core.MediaCover -{ - public static class GdiPlusInterop - { - private static Exception _gdiPlusException; - - static GdiPlusInterop() - { - TestLibrary(); - } - - private static void TestLibrary() - { - if (OsInfo.IsWindows) - { - return; - } - - try - { - // We use StringFormat as test coz it gets properly cleaned up by the finalizer even if gdiplus is absent and is relatively non-invasive. - var strFormat = new StringFormat(); - - strFormat.Dispose(); - } - catch (Exception ex) - { - _gdiPlusException = ex; - } - } - - public static void CheckGdiPlus() - { - if (_gdiPlusException != null) - { - throw new DllNotFoundException("Couldn't load GDIPlus library", _gdiPlusException); - } - } - } -} diff --git a/src/NzbDrone.Core/MediaCover/ImageResizer.cs b/src/NzbDrone.Core/MediaCover/ImageResizer.cs index 9673cbec6..0b684d28e 100644 --- a/src/NzbDrone.Core/MediaCover/ImageResizer.cs +++ b/src/NzbDrone.Core/MediaCover/ImageResizer.cs @@ -1,5 +1,8 @@ -using ImageResizer; +using System; using NzbDrone.Common.Disk; +using SixLabors.ImageSharp; +using SixLabors.ImageSharp.Processing; +using SixLabors.Memory; namespace NzbDrone.Core.MediaCover { @@ -15,25 +18,20 @@ namespace NzbDrone.Core.MediaCover public ImageResizer(IDiskProvider diskProvider) { _diskProvider = diskProvider; + + // More conservative memory allocation + SixLabors.ImageSharp.Configuration.Default.MemoryAllocator = new SimpleGcMemoryAllocator(); } public void Resize(string source, string destination, int height) { try { - GdiPlusInterop.CheckGdiPlus(); - - using (var sourceStream = _diskProvider.OpenReadStream(source)) + using (var image = Image.Load(source)) { - using (var outputStream = _diskProvider.OpenWriteStream(destination)) - { - var settings = new Instructions(); - settings.Height = height; - - var job = new ImageJob(sourceStream, outputStream, settings); - - ImageBuilder.Current.Build(job); - } + var width = (int)Math.Floor((double)image.Width * (double)height / (double)image.Height); + image.Mutate(x => x.Resize(width, height)); + image.Save(destination); } } catch diff --git a/src/NzbDrone.Core/MediaCover/MediaCoverService.cs b/src/NzbDrone.Core/MediaCover/MediaCoverService.cs index 108714142..db03637c5 100644 --- a/src/NzbDrone.Core/MediaCover/MediaCoverService.cs +++ b/src/NzbDrone.Core/MediaCover/MediaCoverService.cs @@ -163,8 +163,6 @@ namespace NzbDrone.Core.MediaCover { _logger.Error(e, "Couldn't download media cover for {0}", album); } - - EnsureResizedAlbumCovers(album, cover, !alreadyExists); } } @@ -227,31 +225,6 @@ namespace NzbDrone.Core.MediaCover } } - private void EnsureResizedAlbumCovers(Album album, MediaCover cover, bool forceResize) - { - int[] heights = GetDefaultHeights(cover.CoverType); - - foreach (var height in heights) - { - var mainFileName = GetCoverPath(album.Id, MediaCoverEntity.Album, cover.CoverType, cover.Extension, null); - var resizeFileName = GetCoverPath(album.Id, MediaCoverEntity.Album, cover.CoverType, cover.Extension, height); - - if (forceResize || !_diskProvider.FileExists(resizeFileName) || _diskProvider.GetFileSize(resizeFileName) == 0) - { - _logger.Debug("Resizing {0}-{1} for {2}", cover.CoverType, height, album); - - try - { - _resizer.Resize(mainFileName, resizeFileName, height); - } - catch - { - _logger.Debug("Couldn't resize media cover {0}-{1} for album {2}, using full size image instead.", cover.CoverType, height, album); - } - } - } - } - private int[] GetDefaultHeights(MediaCoverTypes coverType) { switch (coverType) @@ -261,6 +234,7 @@ namespace NzbDrone.Core.MediaCover case MediaCoverTypes.Poster: case MediaCoverTypes.Disc: + case MediaCoverTypes.Cover: case MediaCoverTypes.Logo: case MediaCoverTypes.Headshot: return new[] { 500, 250 }; @@ -271,8 +245,6 @@ namespace NzbDrone.Core.MediaCover case MediaCoverTypes.Fanart: case MediaCoverTypes.Screenshot: return new[] { 360, 180 }; - case MediaCoverTypes.Cover: - return new[] { 250 }; } }