From 6883cd0969e40ad380d20b5b338c681767bc73f1 Mon Sep 17 00:00:00 2001 From: Enea D'Angiò Date: Fri, 3 Jul 2026 11:14:11 +0200 Subject: Fix play queue index handling in SyncPlay Three index bugs in PlayQueueManager, two of which leave PlayingItemIndex out of bounds, making every subsequent Buffering/Ready request throw and leaving the group unusable until it empties: - RemoveFromPlaylist did not compensate for removed items preceding the playing item: removing the playing item together with earlier items could select the wrong item or crash with an out-of-bounds index. - Next/Previous on an empty playlist with RepeatOne/RepeatAll reported success or set PlayingItemIndex to 0 on an empty list, crashing downstream in Group and corrupting the index. - SetPlayingItemByIndex accepted an index equal to the playlist count (latent off-by-one, callers currently pre-validate). --- .../SyncPlay/Queue/PlayQueueManager.cs | 30 +++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) (limited to 'MediaBrowser.Controller') diff --git a/MediaBrowser.Controller/SyncPlay/Queue/PlayQueueManager.cs b/MediaBrowser.Controller/SyncPlay/Queue/PlayQueueManager.cs index c0a168192e..f019a368a6 100644 --- a/MediaBrowser.Controller/SyncPlay/Queue/PlayQueueManager.cs +++ b/MediaBrowser.Controller/SyncPlay/Queue/PlayQueueManager.cs @@ -272,7 +272,7 @@ namespace MediaBrowser.Controller.SyncPlay.Queue public void SetPlayingItemByIndex(int playlistIndex) { var playlist = GetPlaylistInternal(); - if (playlistIndex < 0 || playlistIndex > playlist.Count) + if (playlistIndex < 0 || playlistIndex >= playlist.Count) { PlayingItemIndex = NoPlayingItemIndex; } @@ -293,6 +293,20 @@ namespace MediaBrowser.Controller.SyncPlay.Queue { var playingItem = GetPlayingItem(); + // Removed items that precede the playing item shift its index as well. + var removedBeforePlayingItem = 0; + if (playingItem is not null) + { + var playlist = GetPlaylistInternal(); + for (var index = 0; index < PlayingItemIndex; index++) + { + if (playlistItemIds.Contains(playlist[index].PlaylistItemId)) + { + removedBeforePlayingItem++; + } + } + } + _sortedPlaylist.RemoveAll(item => playlistItemIds.Contains(item.PlaylistItemId)); _shuffledPlaylist.RemoveAll(item => playlistItemIds.Contains(item.PlaylistItemId)); @@ -303,12 +317,12 @@ namespace MediaBrowser.Controller.SyncPlay.Queue if (playlistItemIds.Contains(playingItem.PlaylistItemId)) { // Playing item has been removed, picking previous item. - PlayingItemIndex--; + PlayingItemIndex -= removedBeforePlayingItem + 1; if (PlayingItemIndex < 0) { // Was first element, picking next if available. // Default to no playing item otherwise. - PlayingItemIndex = _sortedPlaylist.Count > 0 ? 0 : NoPlayingItemIndex; + PlayingItemIndex = GetPlaylistInternal().Count > 0 ? 0 : NoPlayingItemIndex; } return true; @@ -444,6 +458,11 @@ namespace MediaBrowser.Controller.SyncPlay.Queue /// true if the playing item changed; false otherwise. public bool Next() { + if (GetPlaylistInternal().Count == 0) + { + return false; + } + if (RepeatMode.Equals(GroupRepeatMode.RepeatOne)) { LastChange = DateTime.UtcNow; @@ -474,6 +493,11 @@ namespace MediaBrowser.Controller.SyncPlay.Queue /// true if the playing item changed; false otherwise. public bool Previous() { + if (GetPlaylistInternal().Count == 0) + { + return false; + } + if (RepeatMode.Equals(GroupRepeatMode.RepeatOne)) { LastChange = DateTime.UtcNow; -- cgit v1.2.3 From d36c8ebce8593abd9cd32f37a6206b0cde25af8d Mon Sep 17 00:00:00 2001 From: Enea D'Angiò Date: Thu, 16 Jul 2026 21:03:29 +0200 Subject: Reduce cognitive complexity of RemoveFromPlaylist --- MediaBrowser.Controller/SyncPlay/Queue/PlayQueueManager.cs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) (limited to 'MediaBrowser.Controller') diff --git a/MediaBrowser.Controller/SyncPlay/Queue/PlayQueueManager.cs b/MediaBrowser.Controller/SyncPlay/Queue/PlayQueueManager.cs index f019a368a6..9326864d78 100644 --- a/MediaBrowser.Controller/SyncPlay/Queue/PlayQueueManager.cs +++ b/MediaBrowser.Controller/SyncPlay/Queue/PlayQueueManager.cs @@ -297,14 +297,9 @@ namespace MediaBrowser.Controller.SyncPlay.Queue var removedBeforePlayingItem = 0; if (playingItem is not null) { - var playlist = GetPlaylistInternal(); - for (var index = 0; index < PlayingItemIndex; index++) - { - if (playlistItemIds.Contains(playlist[index].PlaylistItemId)) - { - removedBeforePlayingItem++; - } - } + removedBeforePlayingItem = GetPlaylistInternal() + .Take(PlayingItemIndex) + .Count(item => playlistItemIds.Contains(item.PlaylistItemId)); } _sortedPlaylist.RemoveAll(item => playlistItemIds.Contains(item.PlaylistItemId)); -- cgit v1.2.3