don't try to keep extracting images that fail

MediaBrowser.Controller/MediaInfo/FFMpegManager.cs

@@ -1,4 +1,5 @@
-using MediaBrowser.Common.IO;
+using System.Collections.Generic;
+using MediaBrowser.Common.IO;
 using MediaBrowser.Common.MediaInfo;
 using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Library;
@@ -145,9 +146,10 @@ namespace MediaBrowser.Controller.MediaInfo
         /// <param name="cancellationToken">The cancellation token.</param>
         /// <param name="extractImages">if set to <c>true</c> [extract images].</param>
         /// <param name="saveItem">if set to <c>true</c> [save item].</param>
+        /// <param name="previouslyFailedExtractions">The previously failed extractions.</param>
         /// <returns>Task.</returns>
         /// <exception cref="System.ArgumentNullException"></exception>
-        public async Task PopulateChapterImages(Video video, CancellationToken cancellationToken, bool extractImages, bool saveItem)
+        public async Task<bool> PopulateChapterImages(Video video, CancellationToken cancellationToken, bool extractImages, bool saveItem)
         {
             if (video.Chapters == null)
             {
@@ -157,9 +159,10 @@ namespace MediaBrowser.Controller.MediaInfo
             // Can't extract images if there are no video streams
             if (video.MediaStreams == null || video.MediaStreams.All(m => m.Type != MediaStreamType.Video))
             {
-                return;
+                return true;
             }
 
+            var success = true;
             var changesMade = false;
 
             foreach (var chapter in video.Chapters)
@@ -201,6 +204,7 @@ namespace MediaBrowser.Controller.MediaInfo
                         }
                         catch
                         {
+                            success = false;
                             break;
                         }
                     }
@@ -216,6 +220,8 @@ namespace MediaBrowser.Controller.MediaInfo
             {
                 await _libraryManager.UpdateItem(video, CancellationToken.None).ConfigureAwait(false);
             }
+
+            return success;
         }
 
         /// <summary>

MediaBrowser.Controller/Providers/MediaInfo/BaseFFProbeProvider.cs

@@ -72,9 +72,7 @@ namespace MediaBrowser.Controller.Providers.MediaInfo
 
                 if (video != null)
                 {
-                    await
-                        Kernel.Instance.FFMpegManager.PopulateChapterImages(video, cancellationToken, false, false)
-                              .ConfigureAwait(false);
+                    await Kernel.Instance.FFMpegManager.PopulateChapterImages(video, cancellationToken, false, false).ConfigureAwait(false);
                 }
 
                 SetLastRefreshed(item, DateTime.UtcNow);

MediaBrowser.Controller/Providers/Music/FanArtAlbumProvider.cs

@@ -83,7 +83,7 @@ namespace MediaBrowser.Controller.Providers.Music
         {
             get
             {
-                return "16";
+                return "17";
             }
         }
 
@@ -106,8 +106,20 @@ namespace MediaBrowser.Controller.Providers.Music
                 return false;
             }
 
+            var comparisonData = Guid.Empty;
+
+            var artistMusicBrainzId = item.Parent.GetProviderId(MetadataProviders.Musicbrainz);
+            
+            if (!string.IsNullOrEmpty(artistMusicBrainzId))
+            {
+                var artistXmlPath = FanArtArtistProvider.GetArtistDataPath(ConfigurationManager.CommonApplicationPaths, artistMusicBrainzId);
+                artistXmlPath = Path.Combine(artistXmlPath, "fanart.xml");
+
+                comparisonData = GetComparisonData(new FileInfo(artistXmlPath));
+            }
+            
             // Refresh anytime the parent mbz id changes
-            if (providerInfo.Data != GetComparisonData(item.Parent.GetProviderId(MetadataProviders.Musicbrainz)))
+            if (providerInfo.Data != comparisonData)
             {
                 return true;
             }
@@ -119,9 +131,9 @@ namespace MediaBrowser.Controller.Providers.Music
         /// Gets the comparison data.
         /// </summary>
         /// <returns>Guid.</returns>
-        private Guid GetComparisonData(string id)
+        private Guid GetComparisonData(FileInfo artistXmlFileInfo)
         {
-            return string.IsNullOrEmpty(id) ? Guid.Empty : id.GetMD5();
+            return artistXmlFileInfo.Exists ? (artistXmlFileInfo.FullName + artistXmlFileInfo.LastWriteTimeUtc.Ticks).GetMD5() : Guid.Empty;
         }
 
         /// <summary>
@@ -145,59 +157,75 @@ namespace MediaBrowser.Controller.Providers.Music
                 item.ProviderData[Id] = data;
             }
 
+            var comparisonData = Guid.Empty;
+
             if (!string.IsNullOrEmpty(artistMusicBrainzId))
             {
-                var album = (MusicAlbum)item;
+                var artistXmlPath = FanArtArtistProvider.GetArtistDataPath(ConfigurationManager.CommonApplicationPaths, artistMusicBrainzId);
+                artistXmlPath = Path.Combine(artistXmlPath, "fanart.xml");
 
-                if (string.IsNullOrEmpty(album.MusicBrainzReleaseGroupId))
-                {
-                    album.MusicBrainzReleaseGroupId = await GetReleaseGroupId(item.GetProviderId(MetadataProviders.Musicbrainz), cancellationToken).ConfigureAwait(false);
-                }
+                var artistXmlFileInfo = new FileInfo(artistXmlPath);
 
-                // If still empty there's nothing more we can do
-                if (!string.IsNullOrEmpty(album.MusicBrainzReleaseGroupId))
+                comparisonData = GetComparisonData(artistXmlFileInfo);
+
+                if (artistXmlFileInfo.Exists)
                 {
-                    var artistXmlPath = FanArtArtistProvider.GetArtistDataPath(ConfigurationManager.CommonApplicationPaths, artistMusicBrainzId);
-                    artistXmlPath = Path.Combine(artistXmlPath, "fanart.xml");
+                    var album = (MusicAlbum)item;
 
-                    var artistXmlFileInfo = new FileInfo(artistXmlPath);
+                    var releaseEntryId = item.GetProviderId(MetadataProviders.Musicbrainz);
 
-                    if (artistXmlFileInfo.Exists)
+                    // Fanart uses the release group id so we'll have to get that now using the release entry id
+                    if (string.IsNullOrEmpty(album.MusicBrainzReleaseGroupId))
                     {
-                        var doc = new XmlDocument();
+                        album.MusicBrainzReleaseGroupId = await GetReleaseGroupId(releaseEntryId, cancellationToken).ConfigureAwait(false);
+                    }
 
-                        doc.Load(artistXmlPath);
+                    var doc = new XmlDocument();
 
-                        cancellationToken.ThrowIfCancellationRequested();
+                    doc.Load(artistXmlPath);
 
-                        if (ConfigurationManager.Configuration.DownloadMusicAlbumImages.Disc && !item.HasImage(ImageType.Disc))
+                    cancellationToken.ThrowIfCancellationRequested();
+
+                    if (ConfigurationManager.Configuration.DownloadMusicAlbumImages.Disc && !item.HasImage(ImageType.Disc))
+                    {
+                        // Try try with the release entry Id, if that doesn't produce anything try the release group id
+                        var node = doc.SelectSingleNode("//fanart/music/albums/album[@id=\"" + releaseEntryId + "\"]/cdart/@url");
+
+                        if (node == null && !string.IsNullOrEmpty(album.MusicBrainzReleaseGroupId))
                         {
-                            var node = doc.SelectSingleNode("//fanart/music/albums/album[@id=\"" + album.MusicBrainzReleaseGroupId + "\"]/cdart/@url");
+                            node = doc.SelectSingleNode("//fanart/music/albums/album[@id=\"" + album.MusicBrainzReleaseGroupId + "\"]/cdart/@url");
+                        }
 
-                            var path = node != null ? node.Value : null;
+                        var path = node != null ? node.Value : null;
 
-                            if (!string.IsNullOrEmpty(path))
-                            {
-                                item.SetImage(ImageType.Disc, await _providerManager.DownloadAndSaveImage(item, path, DiscFile, ConfigurationManager.Configuration.SaveLocalMeta, FanArtResourcePool, cancellationToken).ConfigureAwait(false));
-                            }
+                        if (!string.IsNullOrEmpty(path))
+                        {
+                            item.SetImage(ImageType.Disc, await _providerManager.DownloadAndSaveImage(item, path, DiscFile, ConfigurationManager.Configuration.SaveLocalMeta, FanArtResourcePool, cancellationToken).ConfigureAwait(false));
                         }
+                    }
 
-                        if (ConfigurationManager.Configuration.DownloadMusicAlbumImages.Primary && !item.HasImage(ImageType.Primary))
+                    if (ConfigurationManager.Configuration.DownloadMusicAlbumImages.Primary && !item.HasImage(ImageType.Primary))
+                    {
+                        // Try try with the release entry Id, if that doesn't produce anything try the release group id
+                        var node = doc.SelectSingleNode("//fanart/music/albums/album[@id=\"" + releaseEntryId + "\"]/albumcover/@url");
+
+                        if (node == null && !string.IsNullOrEmpty(album.MusicBrainzReleaseGroupId))
                         {
-                            var node = doc.SelectSingleNode("//fanart/music/albums/album[@id=\"" + album.MusicBrainzReleaseGroupId + "\"]/albumcover/@url");
+                            node = doc.SelectSingleNode("//fanart/music/albums/album[@id=\"" + album.MusicBrainzReleaseGroupId + "\"]/albumcover/@url");
+                        }
 
-                            var path = node != null ? node.Value : null;
+                        var path = node != null ? node.Value : null;
 
-                            if (!string.IsNullOrEmpty(path))
-                            {
-                                item.SetImage(ImageType.Primary, await _providerManager.DownloadAndSaveImage(item, path, PrimaryFile, ConfigurationManager.Configuration.SaveLocalMeta, FanArtResourcePool, cancellationToken).ConfigureAwait(false));
-                            }
+                        if (!string.IsNullOrEmpty(path))
+                        {
+                            item.SetImage(ImageType.Primary, await _providerManager.DownloadAndSaveImage(item, path, PrimaryFile, ConfigurationManager.Configuration.SaveLocalMeta, FanArtResourcePool, cancellationToken).ConfigureAwait(false));
                         }
                     }
                 }
+
             }
 
-            data.Data = GetComparisonData(artistMusicBrainzId);
+            data.Data = comparisonData;
             SetLastRefreshed(item, DateTime.UtcNow);
 
             return true;

MediaBrowser.Server.Implementations/ScheduledTasks/ChapterImagesTask.cs

@@ -4,12 +4,14 @@ using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Model.Entities;
 using MediaBrowser.Model.Logging;
+using MediaBrowser.Model.Serialization;
+using MoreLinq;
 using System;
 using System.Collections.Generic;
+using System.IO;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
-using MoreLinq;
 
 namespace MediaBrowser.Server.Implementations.ScheduledTasks
 {
@@ -18,6 +20,8 @@ namespace MediaBrowser.Server.Implementations.ScheduledTasks
     /// </summary>
     class ChapterImagesTask : IScheduledTask
     {
+        private readonly IJsonSerializer _jsonSerializer;
+
         /// <summary>
         /// The _kernel
         /// </summary>
@@ -40,18 +44,20 @@ namespace MediaBrowser.Server.Implementations.ScheduledTasks
         /// </summary>
         /// <value>The new item timer.</value>
         private Timer NewItemTimer { get; set; }
-        
+
         /// <summary>
         /// Initializes a new instance of the <see cref="ChapterImagesTask" /> class.
         /// </summary>
         /// <param name="kernel">The kernel.</param>
         /// <param name="logManager">The log manager.</param>
         /// <param name="libraryManager">The library manager.</param>
-        public ChapterImagesTask(Kernel kernel, ILogManager logManager, ILibraryManager libraryManager)
+        /// <param name="jsonSerializer">The json serializer.</param>
+        public ChapterImagesTask(Kernel kernel, ILogManager logManager, ILibraryManager libraryManager, IJsonSerializer jsonSerializer)
         {
             _kernel = kernel;
             _logger = logManager.GetLogger(GetType().Name);
             _libraryManager = libraryManager;
+            _jsonSerializer = jsonSerializer;
 
             libraryManager.ItemAdded += libraryManager_ItemAdded;
             libraryManager.ItemUpdated += libraryManager_ItemAdded;
@@ -93,8 +99,9 @@ namespace MediaBrowser.Server.Implementations.ScheduledTasks
                 NewItemTimer = null;
             }
 
+            // Limit to video files to reduce changes of ffmpeg crash dialog
             foreach (var item in newItems
-                .Where(i => i.LocationType == LocationType.FileSystem && string.IsNullOrEmpty(i.PrimaryImagePath) && i.MediaStreams.Any(m => m.Type == MediaStreamType.Video))
+                .Where(i => i.LocationType == LocationType.FileSystem && i.VideoType == VideoType.VideoFile && string.IsNullOrEmpty(i.PrimaryImagePath) && i.MediaStreams.Any(m => m.Type == MediaStreamType.Video))
                 .Take(1))
             {
                 try
@@ -135,11 +142,34 @@ namespace MediaBrowser.Server.Implementations.ScheduledTasks
 
             var numComplete = 0;
 
+            var failHistoryPath = Path.Combine(_kernel.FFMpegManager.VideoImagesDataPath, "failures.json");
+
+            List<string> previouslyFailedImages;
+            
+            try
+            {
+                previouslyFailedImages = _jsonSerializer.DeserializeFromFile<List<string>>(failHistoryPath);
+            }
+            catch (FileNotFoundException)
+            {
+                previouslyFailedImages = new List<string>();
+            }
+
             foreach (var video in videos)
             {
                 cancellationToken.ThrowIfCancellationRequested();
 
-                await _kernel.FFMpegManager.PopulateChapterImages(video, cancellationToken, true, true);
+                var key = video.Path + video.DateModified.Ticks;
+
+                var extract = !previouslyFailedImages.Contains(key, StringComparer.OrdinalIgnoreCase);
+
+                var success = await _kernel.FFMpegManager.PopulateChapterImages(video, cancellationToken, extract, true);
+
+                if (!success)
+                {
+                    previouslyFailedImages.Add(key);
+                    _jsonSerializer.SerializeToFile(previouslyFailedImages, failHistoryPath);
+                }
 
                 numComplete++;
                 double percent = numComplete;