From dde02770509d6bee1e18d041eb6a1dc471dc8111 Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Mon, 2 Mar 2020 21:12:35 +0100 Subject: [PATCH 01/10] Check for duplicates when adding items to a playlist --- .../Playlists/PlaylistManager.cs | 40 ++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/Emby.Server.Implementations/Playlists/PlaylistManager.cs b/Emby.Server.Implementations/Playlists/PlaylistManager.cs index b26f4026c5..edba30eb39 100644 --- a/Emby.Server.Implementations/Playlists/PlaylistManager.cs +++ b/Emby.Server.Implementations/Playlists/PlaylistManager.cs @@ -189,41 +189,45 @@ namespace Emby.Server.Implementations.Playlists private void AddToPlaylistInternal(string playlistId, IEnumerable itemIds, User user, DtoOptions options) { - var playlist = _libraryManager.GetItemById(playlistId) as Playlist; - - if (playlist == null) - { - throw new ArgumentException("No Playlist exists with the supplied Id"); - } - - var list = new List(); + // Retrieve the existing playlist + var playlist = _libraryManager.GetItemById(playlistId) as Playlist + ?? throw new ArgumentException("No Playlist exists with Id " + playlistId); + // Retrieve all the items to be added to the playlist var items = GetPlaylistItems(itemIds, playlist.MediaType, user, options) .Where(i => i.SupportsAddingToPlaylist) .ToList(); - foreach (var item in items) + // Remove duplicates from the new items to be added + var existingIds = playlist.LinkedChildren.Select(c => c.ItemId).ToHashSet(); + var uniqueItems = items + .Where(i => !existingIds.Contains(i.Id)) + .GroupBy(i => i.Id) + .Select(group => group.First()) + .ToList(); + + // Log duplicates that have been ignored, if any + if (uniqueItems.Count != items.Count) { - list.Add(LinkedChild.Create(item)); + var numDuplicates = items.Count - uniqueItems.Count; + _logger.LogWarning("Ignored adding {DuplicateCount} duplicate items to playlist {PlaylistName}.", numDuplicates, playlist.Name); } - var newList = playlist.LinkedChildren.ToList(); - newList.AddRange(list); - playlist.LinkedChildren = newList.ToArray(); - + // Update the playlist in the repository + var linkedItems = uniqueItems.Select(i => LinkedChild.Create(i)); + playlist.LinkedChildren = playlist.LinkedChildren.Concat(linkedItems).ToArray(); playlist.UpdateToRepository(ItemUpdateType.MetadataEdit, CancellationToken.None); + // Update the playlist on disk if (playlist.IsFile) { SavePlaylistFile(playlist); } + // Refresh playlist metadata _providerManager.QueueRefresh( playlist.Id, - new MetadataRefreshOptions(new DirectoryService(_fileSystem)) - { - ForceSave = true - }, + new MetadataRefreshOptions(new DirectoryService(_fileSystem)) { ForceSave = true }, RefreshPriority.High); } From 77533fd433315f9c1b684da2f0417caab9f1ee27 Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Tue, 3 Mar 2020 14:40:07 +0100 Subject: [PATCH 02/10] Revert unnecessary style change --- Emby.Server.Implementations/Playlists/PlaylistManager.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Emby.Server.Implementations/Playlists/PlaylistManager.cs b/Emby.Server.Implementations/Playlists/PlaylistManager.cs index edba30eb39..0adb714b4f 100644 --- a/Emby.Server.Implementations/Playlists/PlaylistManager.cs +++ b/Emby.Server.Implementations/Playlists/PlaylistManager.cs @@ -227,7 +227,10 @@ namespace Emby.Server.Implementations.Playlists // Refresh playlist metadata _providerManager.QueueRefresh( playlist.Id, - new MetadataRefreshOptions(new DirectoryService(_fileSystem)) { ForceSave = true }, + new MetadataRefreshOptions(new DirectoryService(_fileSystem)) + { + ForceSave = true + }, RefreshPriority.High); } From 7f96fce15d550423685eba2d6a702f1e4c24916d Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Tue, 3 Mar 2020 14:40:49 +0100 Subject: [PATCH 03/10] Add SegiH as a contributor This PR is based on a change originally proposed by this author --- CONTRIBUTORS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index e14636a577..6cd4015793 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -35,7 +35,7 @@ - [geilername](https://github.com/geilername) - [pR0Ps](https://github.com/pR0Ps) - [artiume](https://github.com/Artiume) - + - [SegiH](https://github.com/SegiH) # Emby Contributors From 4d32b59a0b0c1ce1709781760a4d623bd08c38a2 Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Tue, 3 Mar 2020 17:47:16 +0100 Subject: [PATCH 04/10] Performance improvements Use arrays instead of lists; use Array.CopyTo to concat playlist items; only count number of duplicates once --- .../Playlists/PlaylistManager.cs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/Emby.Server.Implementations/Playlists/PlaylistManager.cs b/Emby.Server.Implementations/Playlists/PlaylistManager.cs index 0adb714b4f..78b29be43a 100644 --- a/Emby.Server.Implementations/Playlists/PlaylistManager.cs +++ b/Emby.Server.Implementations/Playlists/PlaylistManager.cs @@ -196,7 +196,7 @@ namespace Emby.Server.Implementations.Playlists // Retrieve all the items to be added to the playlist var items = GetPlaylistItems(itemIds, playlist.MediaType, user, options) .Where(i => i.SupportsAddingToPlaylist) - .ToList(); + .ToArray(); // Remove duplicates from the new items to be added var existingIds = playlist.LinkedChildren.Select(c => c.ItemId).ToHashSet(); @@ -204,18 +204,23 @@ namespace Emby.Server.Implementations.Playlists .Where(i => !existingIds.Contains(i.Id)) .GroupBy(i => i.Id) .Select(group => group.First()) - .ToList(); + .Select(i => LinkedChild.Create(i)) + .ToArray(); // Log duplicates that have been ignored, if any - if (uniqueItems.Count != items.Count) + int numDuplicates = items.Length - uniqueItems.Length; + if (numDuplicates > 0) { - var numDuplicates = items.Count - uniqueItems.Count; _logger.LogWarning("Ignored adding {DuplicateCount} duplicate items to playlist {PlaylistName}.", numDuplicates, playlist.Name); } + // Create a new array with the updated playlist items + var newLinkedChildren = new LinkedChild[playlist.LinkedChildren.Length + uniqueItems.Length]; + playlist.LinkedChildren.CopyTo(newLinkedChildren, 0); + uniqueItems.CopyTo(newLinkedChildren, playlist.LinkedChildren.Length); + // Update the playlist in the repository - var linkedItems = uniqueItems.Select(i => LinkedChild.Create(i)); - playlist.LinkedChildren = playlist.LinkedChildren.Concat(linkedItems).ToArray(); + playlist.LinkedChildren = newLinkedChildren; playlist.UpdateToRepository(ItemUpdateType.MetadataEdit, CancellationToken.None); // Update the playlist on disk From 6438771212cd7e8b46a71d5b1a78d6e6c5d42f69 Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Tue, 3 Mar 2020 17:48:11 +0100 Subject: [PATCH 05/10] Exit method early if there are no unique playlist items to add --- Emby.Server.Implementations/Playlists/PlaylistManager.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Emby.Server.Implementations/Playlists/PlaylistManager.cs b/Emby.Server.Implementations/Playlists/PlaylistManager.cs index 78b29be43a..42d14a9e5b 100644 --- a/Emby.Server.Implementations/Playlists/PlaylistManager.cs +++ b/Emby.Server.Implementations/Playlists/PlaylistManager.cs @@ -214,6 +214,12 @@ namespace Emby.Server.Implementations.Playlists _logger.LogWarning("Ignored adding {DuplicateCount} duplicate items to playlist {PlaylistName}.", numDuplicates, playlist.Name); } + // Do nothing else if there are no items to add to the playlist + if (uniqueItems.Length == 0) + { + return; + } + // Create a new array with the updated playlist items var newLinkedChildren = new LinkedChild[playlist.LinkedChildren.Length + uniqueItems.Length]; playlist.LinkedChildren.CopyTo(newLinkedChildren, 0); From 3cb98fba553024a1a47d5d4179351184ce76ccf7 Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Tue, 3 Mar 2020 18:18:00 +0100 Subject: [PATCH 06/10] Use ToList() instead of ToArray() on sequences of unknown size --- .../Playlists/PlaylistManager.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Emby.Server.Implementations/Playlists/PlaylistManager.cs b/Emby.Server.Implementations/Playlists/PlaylistManager.cs index 42d14a9e5b..5a6ecf5c17 100644 --- a/Emby.Server.Implementations/Playlists/PlaylistManager.cs +++ b/Emby.Server.Implementations/Playlists/PlaylistManager.cs @@ -196,7 +196,7 @@ namespace Emby.Server.Implementations.Playlists // Retrieve all the items to be added to the playlist var items = GetPlaylistItems(itemIds, playlist.MediaType, user, options) .Where(i => i.SupportsAddingToPlaylist) - .ToArray(); + .ToList(); // Remove duplicates from the new items to be added var existingIds = playlist.LinkedChildren.Select(c => c.ItemId).ToHashSet(); @@ -205,23 +205,23 @@ namespace Emby.Server.Implementations.Playlists .GroupBy(i => i.Id) .Select(group => group.First()) .Select(i => LinkedChild.Create(i)) - .ToArray(); + .ToList(); // Log duplicates that have been ignored, if any - int numDuplicates = items.Length - uniqueItems.Length; + int numDuplicates = items.Count - uniqueItems.Count; if (numDuplicates > 0) { _logger.LogWarning("Ignored adding {DuplicateCount} duplicate items to playlist {PlaylistName}.", numDuplicates, playlist.Name); } // Do nothing else if there are no items to add to the playlist - if (uniqueItems.Length == 0) + if (uniqueItems.Count == 0) { return; } // Create a new array with the updated playlist items - var newLinkedChildren = new LinkedChild[playlist.LinkedChildren.Length + uniqueItems.Length]; + var newLinkedChildren = new LinkedChild[playlist.LinkedChildren.Length + uniqueItems.Count]; playlist.LinkedChildren.CopyTo(newLinkedChildren, 0); uniqueItems.CopyTo(newLinkedChildren, playlist.LinkedChildren.Length); From d276e0f8f4b63d2fa8eda8208d8db15600d57a71 Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Wed, 11 Mar 2020 23:26:55 +0100 Subject: [PATCH 07/10] Use Distinct() to filter out duplicates when adding items to playlist --- Emby.Server.Implementations/Playlists/PlaylistManager.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Emby.Server.Implementations/Playlists/PlaylistManager.cs b/Emby.Server.Implementations/Playlists/PlaylistManager.cs index 0864169ce3..5363a4b33f 100644 --- a/Emby.Server.Implementations/Playlists/PlaylistManager.cs +++ b/Emby.Server.Implementations/Playlists/PlaylistManager.cs @@ -202,8 +202,7 @@ namespace Emby.Server.Implementations.Playlists var existingIds = playlist.LinkedChildren.Select(c => c.ItemId).ToHashSet(); var uniqueItems = items .Where(i => !existingIds.Contains(i.Id)) - .GroupBy(i => i.Id) - .Select(group => group.First()) + .Distinct() .Select(i => LinkedChild.Create(i)) .ToList(); From c594b1a4c39c81b11feefe5885543e3014ca44eb Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Fri, 13 Mar 2020 22:46:11 +0100 Subject: [PATCH 08/10] Fix bad merge in contributors list --- CONTRIBUTORS.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index d0e69369af..f195c125f1 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -80,8 +80,6 @@ - [pjeanjean](https://github.com/pjeanjean) - [ploughpuff](https://github.com/ploughpuff) - [pR0Ps](https://github.com/pR0Ps) - - [artiume](https://github.com/Artiume) - - [SegiH](https://github.com/SegiH) - [PrplHaz4](https://github.com/PrplHaz4) - [RazeLighter777](https://github.com/RazeLighter777) - [redSpoutnik](https://github.com/redSpoutnik) @@ -93,6 +91,7 @@ - [samuel9554](https://github.com/samuel9554) - [scheidleon](https://github.com/scheidleon) - [sebPomme](https://github.com/sebPomme) + - [SegiH](https://github.com/SegiH) - [SenorSmartyPants](https://github.com/SenorSmartyPants) - [shemanaev](https://github.com/shemanaev) - [skaro13](https://github.com/skaro13) From 79413d9f33aad2aae933e79ad22a8a01ae3b6807 Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Fri, 13 Mar 2020 23:11:59 +0100 Subject: [PATCH 09/10] Add a configuration flag to allow/disallow duplicates in playlists --- .../ConfigurationOptions.cs | 3 ++- .../Extensions/ConfigurationExtensions.cs | 25 ++++++++++++++----- .../MediaBrowser.Controller.csproj | 1 + 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/Emby.Server.Implementations/ConfigurationOptions.cs b/Emby.Server.Implementations/ConfigurationOptions.cs index d0f3d67230..31fb5ca58c 100644 --- a/Emby.Server.Implementations/ConfigurationOptions.cs +++ b/Emby.Server.Implementations/ConfigurationOptions.cs @@ -9,7 +9,8 @@ namespace Emby.Server.Implementations { { "HttpListenerHost:DefaultRedirectPath", "web/index.html" }, { FfmpegProbeSizeKey, "1G" }, - { FfmpegAnalyzeDurationKey, "200M" } + { FfmpegAnalyzeDurationKey, "200M" }, + { PlaylistsAllowDuplicatesKey, bool.TrueString } }; } } diff --git a/MediaBrowser.Controller/Extensions/ConfigurationExtensions.cs b/MediaBrowser.Controller/Extensions/ConfigurationExtensions.cs index 76c9b4b26c..48316499a4 100644 --- a/MediaBrowser.Controller/Extensions/ConfigurationExtensions.cs +++ b/MediaBrowser.Controller/Extensions/ConfigurationExtensions.cs @@ -13,24 +13,37 @@ namespace MediaBrowser.Controller.Extensions public const string FfmpegProbeSizeKey = "FFmpeg:probesize"; /// - /// The key for the FFmpeg analyse duration option. + /// The key for the FFmpeg analyze duration option. /// public const string FfmpegAnalyzeDurationKey = "FFmpeg:analyzeduration"; /// - /// Retrieves the FFmpeg probe size from the . + /// The key for a setting that indicates whether playlists should allow duplicate entries. /// - /// This configuration. + public const string PlaylistsAllowDuplicatesKey = "playlists:allowDuplicates"; + + /// + /// Gets the FFmpeg probe size from the . + /// + /// The configuration to read the setting from. /// The FFmpeg probe size option. public static string GetFFmpegProbeSize(this IConfiguration configuration) => configuration[FfmpegProbeSizeKey]; /// - /// Retrieves the FFmpeg analyse duration from the . + /// Gets the FFmpeg analyze duration from the . /// - /// This configuration. - /// The FFmpeg analyse duration option. + /// The configuration to read the setting from. + /// The FFmpeg analyze duration option. public static string GetFFmpegAnalyzeDuration(this IConfiguration configuration) => configuration[FfmpegAnalyzeDurationKey]; + + /// + /// Gets a value indicating whether playlists should allow duplicate entries from the . + /// + /// The configuration to read the setting from. + /// True if playlists should allow duplicates, otherwise false. + public static bool DoPlaylistsAllowDuplicates(this IConfiguration configuration) + => configuration.GetValue(PlaylistsAllowDuplicatesKey); } } diff --git a/MediaBrowser.Controller/MediaBrowser.Controller.csproj b/MediaBrowser.Controller/MediaBrowser.Controller.csproj index 88e9055e84..bcca9e4a18 100644 --- a/MediaBrowser.Controller/MediaBrowser.Controller.csproj +++ b/MediaBrowser.Controller/MediaBrowser.Controller.csproj @@ -9,6 +9,7 @@ + From 9a7875b6f96b8cf962e51d68694b4fb30b9995af Mon Sep 17 00:00:00 2001 From: Mark Monteiro Date: Fri, 13 Mar 2020 23:14:25 +0100 Subject: [PATCH 10/10] Do not check for duplicates if they are allowed in playlist configuration --- .../Playlists/PlaylistManager.cs | 40 ++++++++++++------- .../Playlists/IPlaylistManager.cs | 2 +- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/Emby.Server.Implementations/Playlists/PlaylistManager.cs b/Emby.Server.Implementations/Playlists/PlaylistManager.cs index 5363a4b33f..9b1510ac97 100644 --- a/Emby.Server.Implementations/Playlists/PlaylistManager.cs +++ b/Emby.Server.Implementations/Playlists/PlaylistManager.cs @@ -8,12 +8,14 @@ using System.Threading.Tasks; using MediaBrowser.Controller.Dto; using MediaBrowser.Controller.Entities; using MediaBrowser.Controller.Entities.Audio; +using MediaBrowser.Controller.Extensions; using MediaBrowser.Controller.Library; using MediaBrowser.Controller.Playlists; using MediaBrowser.Controller.Providers; using MediaBrowser.Model.Entities; using MediaBrowser.Model.IO; using MediaBrowser.Model.Playlists; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Logging; using PlaylistsNET.Content; using PlaylistsNET.Models; @@ -28,6 +30,7 @@ namespace Emby.Server.Implementations.Playlists private readonly ILogger _logger; private readonly IUserManager _userManager; private readonly IProviderManager _providerManager; + private readonly IConfiguration _appConfig; public PlaylistManager( ILibraryManager libraryManager, @@ -35,7 +38,8 @@ namespace Emby.Server.Implementations.Playlists ILibraryMonitor iLibraryMonitor, ILogger logger, IUserManager userManager, - IProviderManager providerManager) + IProviderManager providerManager, + IConfiguration appConfig) { _libraryManager = libraryManager; _fileSystem = fileSystem; @@ -43,6 +47,7 @@ namespace Emby.Server.Implementations.Playlists _logger = logger; _userManager = userManager; _providerManager = providerManager; + _appConfig = appConfig; } public IEnumerable GetPlaylists(Guid userId) @@ -177,7 +182,7 @@ namespace Emby.Server.Implementations.Playlists return Playlist.GetPlaylistItems(playlistMediaType, items, user, options); } - public void AddToPlaylist(string playlistId, IEnumerable itemIds, Guid userId) + public void AddToPlaylist(string playlistId, ICollection itemIds, Guid userId) { var user = userId.Equals(Guid.Empty) ? null : _userManager.GetUserById(userId); @@ -187,42 +192,47 @@ namespace Emby.Server.Implementations.Playlists }); } - private void AddToPlaylistInternal(string playlistId, IEnumerable itemIds, User user, DtoOptions options) + private void AddToPlaylistInternal(string playlistId, ICollection newItemIds, User user, DtoOptions options) { // Retrieve the existing playlist var playlist = _libraryManager.GetItemById(playlistId) as Playlist ?? throw new ArgumentException("No Playlist exists with Id " + playlistId); // Retrieve all the items to be added to the playlist - var items = GetPlaylistItems(itemIds, playlist.MediaType, user, options) - .Where(i => i.SupportsAddingToPlaylist) - .ToList(); + var newItems = GetPlaylistItems(newItemIds, playlist.MediaType, user, options) + .Where(i => i.SupportsAddingToPlaylist); + + // Filter out duplicate items, if necessary + if (!_appConfig.DoPlaylistsAllowDuplicates()) + { + var existingIds = playlist.LinkedChildren.Select(c => c.ItemId).ToHashSet(); + newItems = newItems + .Where(i => !existingIds.Contains(i.Id)) + .Distinct(); + } - // Remove duplicates from the new items to be added - var existingIds = playlist.LinkedChildren.Select(c => c.ItemId).ToHashSet(); - var uniqueItems = items - .Where(i => !existingIds.Contains(i.Id)) - .Distinct() + // Create a list of the new linked children to add to the playlist + var childrenToAdd = newItems .Select(i => LinkedChild.Create(i)) .ToList(); // Log duplicates that have been ignored, if any - int numDuplicates = items.Count - uniqueItems.Count; + int numDuplicates = newItemIds.Count - childrenToAdd.Count; if (numDuplicates > 0) { _logger.LogWarning("Ignored adding {DuplicateCount} duplicate items to playlist {PlaylistName}.", numDuplicates, playlist.Name); } // Do nothing else if there are no items to add to the playlist - if (uniqueItems.Count == 0) + if (childrenToAdd.Count == 0) { return; } // Create a new array with the updated playlist items - var newLinkedChildren = new LinkedChild[playlist.LinkedChildren.Length + uniqueItems.Count]; + var newLinkedChildren = new LinkedChild[playlist.LinkedChildren.Length + childrenToAdd.Count]; playlist.LinkedChildren.CopyTo(newLinkedChildren, 0); - uniqueItems.CopyTo(newLinkedChildren, playlist.LinkedChildren.Length); + childrenToAdd.CopyTo(newLinkedChildren, playlist.LinkedChildren.Length); // Update the playlist in the repository playlist.LinkedChildren = newLinkedChildren; diff --git a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs index 5001f68420..544cd2643f 100644 --- a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs +++ b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs @@ -29,7 +29,7 @@ namespace MediaBrowser.Controller.Playlists /// The item ids. /// The user identifier. /// Task. - void AddToPlaylist(string playlistId, IEnumerable itemIds, Guid userId); + void AddToPlaylist(string playlistId, ICollection itemIds, Guid userId); /// /// Removes from playlist.