Browse Source

Fix local JPG primary image for video being overwritten by screen grabber (#9552)

SenorSmartyPants 2 years ago
parent
commit
11d7c00de9

+ 38 - 4
MediaBrowser.Providers/Manager/ItemImageProvider.cs

@@ -32,6 +32,7 @@ namespace MediaBrowser.Providers.Manager
         private readonly ILogger _logger;
         private readonly IProviderManager _providerManager;
         private readonly IFileSystem _fileSystem;
+        private static readonly ImageType[] AllImageTypes = Enum.GetValues<ImageType>();
 
         /// <summary>
         /// Image types that are only one per item.
@@ -90,11 +91,12 @@ namespace MediaBrowser.Providers.Manager
         /// </summary>
         /// <param name="item">The <see cref="BaseItem"/> to validate images for.</param>
         /// <param name="providers">The providers to use, must include <see cref="ILocalImageProvider"/>(s) for local scanning.</param>
-        /// <param name="directoryService">The directory service for <see cref="ILocalImageProvider"/>s to use.</param>
+        /// <param name="refreshOptions">The refresh options.</param>
         /// <returns><c>true</c> if changes were made to the item; otherwise <c>false</c>.</returns>
-        public bool ValidateImages(BaseItem item, IEnumerable<IImageProvider> providers, IDirectoryService directoryService)
+        public bool ValidateImages(BaseItem item, IEnumerable<IImageProvider> providers, ImageRefreshOptions refreshOptions)
         {
             var hasChanges = false;
+            IDirectoryService directoryService = refreshOptions?.DirectoryService;
 
             if (item is not Photo)
             {
@@ -102,7 +104,7 @@ namespace MediaBrowser.Providers.Manager
                     .SelectMany(i => i.GetImages(item, directoryService))
                     .ToList();
 
-                if (MergeImages(item, images))
+                if (MergeImages(item, images, refreshOptions))
                 {
                     hasChanges = true;
                 }
@@ -381,15 +383,36 @@ namespace MediaBrowser.Providers.Manager
             item.RemoveImages(images);
         }
 
+        /// <summary>
+        /// Merges a list of images into the provided item, validating existing images and replacing them or adding new images as necessary.
+        /// </summary>
+        /// <param name="refreshOptions">The refresh options.</param>
+        /// <param name="dontReplaceImages">List of imageTypes to remove from ReplaceImages.</param>
+        public void UpdateReplaceImages(ImageRefreshOptions refreshOptions, ICollection<ImageType> dontReplaceImages)
+        {
+            if (refreshOptions is not null)
+            {
+                if (refreshOptions.ReplaceAllImages)
+                {
+                    refreshOptions.ReplaceAllImages = false;
+                    refreshOptions.ReplaceImages = AllImageTypes.ToList();
+                }
+
+                refreshOptions.ReplaceImages = refreshOptions.ReplaceImages.Except(dontReplaceImages).ToList();
+            }
+        }
+
         /// <summary>
         /// Merges a list of images into the provided item, validating existing images and replacing them or adding new images as necessary.
         /// </summary>
         /// <param name="item">The <see cref="BaseItem"/> to modify.</param>
         /// <param name="images">The new images to place in <c>item</c>.</param>
+        /// <param name="refreshOptions">The refresh options.</param>
         /// <returns><c>true</c> if changes were made to the item; otherwise <c>false</c>.</returns>
-        public bool MergeImages(BaseItem item, IReadOnlyList<LocalImageInfo> images)
+        public bool MergeImages(BaseItem item, IReadOnlyList<LocalImageInfo> images, ImageRefreshOptions refreshOptions)
         {
             var changed = item.ValidateImages();
+            var foundImageTypes = new List<ImageType>();
 
             for (var i = 0; i < _singularImages.Length; i++)
             {
@@ -399,6 +422,11 @@ namespace MediaBrowser.Providers.Manager
                 if (image is not null)
                 {
                     var currentImage = item.GetImageInfo(type, 0);
+                    // if image file is stored with media, don't replace that later
+                    if (item.ContainingFolderPath is not null && item.ContainingFolderPath.Contains(Path.GetDirectoryName(image.FileInfo.FullName), StringComparison.OrdinalIgnoreCase))
+                    {
+                        foundImageTypes.Add(type);
+                    }
 
                     if (currentImage is null || !string.Equals(currentImage.Path, image.FileInfo.FullName, StringComparison.OrdinalIgnoreCase))
                     {
@@ -425,6 +453,12 @@ namespace MediaBrowser.Providers.Manager
             if (UpdateMultiImages(item, images, ImageType.Backdrop))
             {
                 changed = true;
+                foundImageTypes.Add(ImageType.Backdrop);
+            }
+
+            if (foundImageTypes.Count > 0)
+            {
+                UpdateReplaceImages(refreshOptions, foundImageTypes);
             }
 
             return changed;

+ 7 - 12
MediaBrowser.Providers/Manager/MetadataService.cs

@@ -26,8 +26,6 @@ namespace MediaBrowser.Providers.Manager
         where TItemType : BaseItem, IHasLookupInfo<TIdType>, new()
         where TIdType : ItemLookupInfo, new()
     {
-        private static readonly ImageType[] AllImageTypes = Enum.GetValues<ImageType>();
-
         protected MetadataService(IServerConfigurationManager serverConfigurationManager, ILogger<MetadataService<TItemType, TIdType>> logger, IProviderManager providerManager, IFileSystem fileSystem, ILibraryManager libraryManager)
         {
             ServerConfigurationManager = serverConfigurationManager;
@@ -110,7 +108,7 @@ namespace MediaBrowser.Providers.Manager
             try
             {
                 // Always validate images and check for new locally stored ones.
-                if (ImageProvider.ValidateImages(item, allImageProviders.OfType<ILocalImageProvider>(), refreshOptions.DirectoryService))
+                if (ImageProvider.ValidateImages(item, allImageProviders.OfType<ILocalImageProvider>(), refreshOptions))
                 {
                     updateType |= ItemUpdateType.ImageUpdate;
                 }
@@ -674,8 +672,7 @@ namespace MediaBrowser.Providers.Manager
             }
 
             var hasLocalMetadata = false;
-            var replaceImages = AllImageTypes.ToList();
-            var localImagesFound = false;
+            var foundImageTypes = new List<ImageType>();
 
             foreach (var provider in providers.OfType<ILocalMetadataProvider<TItemType>>())
             {
@@ -703,9 +700,8 @@ namespace MediaBrowser.Providers.Manager
                                 await ProviderManager.SaveImage(item, remoteImage.Url, remoteImage.Type, null, cancellationToken).ConfigureAwait(false);
                                 refreshResult.UpdateType |= ItemUpdateType.ImageUpdate;
 
-                                // remove imagetype that has just been downloaded
-                                replaceImages.Remove(remoteImage.Type);
-                                localImagesFound = true;
+                                // remember imagetype that has just been downloaded
+                                foundImageTypes.Add(remoteImage.Type);
                             }
                             catch (HttpRequestException ex)
                             {
@@ -713,13 +709,12 @@ namespace MediaBrowser.Providers.Manager
                             }
                         }
 
-                        if (localImagesFound)
+                        if (foundImageTypes.Count > 0)
                         {
-                            options.ReplaceAllImages = false;
-                            options.ReplaceImages = replaceImages;
+                            imageService.UpdateReplaceImages(options, foundImageTypes);
                         }
 
-                        if (imageService.MergeImages(item, localItem.Images))
+                        if (imageService.MergeImages(item, localItem.Images, options))
                         {
                             refreshResult.UpdateType |= ItemUpdateType.ImageUpdate;
                         }

+ 3 - 3
tests/Jellyfin.Providers.Tests/Manager/ItemImageProviderTests.cs

@@ -94,7 +94,7 @@ namespace Jellyfin.Providers.Tests.Manager
         public void MergeImages_EmptyItemNewImagesEmpty_NoChange()
         {
             var itemImageProvider = GetItemImageProvider(null, null);
-            var changed = itemImageProvider.MergeImages(new Video(), Array.Empty<LocalImageInfo>());
+            var changed = itemImageProvider.MergeImages(new Video(), Array.Empty<LocalImageInfo>(), new ImageRefreshOptions(Mock.Of<IDirectoryService>()));
 
             Assert.False(changed);
         }
@@ -108,7 +108,7 @@ namespace Jellyfin.Providers.Tests.Manager
             var images = GetImages(imageType, imageCount, false);
 
             var itemImageProvider = GetItemImageProvider(null, null);
-            var changed = itemImageProvider.MergeImages(item, images);
+            var changed = itemImageProvider.MergeImages(item, images, new ImageRefreshOptions(Mock.Of<IDirectoryService>()));
 
             Assert.True(changed);
             // adds for types that allow multiple, replaces singular type images
@@ -151,7 +151,7 @@ namespace Jellyfin.Providers.Tests.Manager
             var images = GetImages(imageType, imageCount, true);
 
             var itemImageProvider = GetItemImageProvider(null, fileSystem);
-            var changed = itemImageProvider.MergeImages(item, images);
+            var changed = itemImageProvider.MergeImages(item, images, new ImageRefreshOptions(Mock.Of<IDirectoryService>()));
 
             if (updateTime)
             {