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).
This commit is contained in:
@@ -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
|
||||
/// <returns><c>true</c> if the playing item changed; <c>false</c> otherwise.</returns>
|
||||
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
|
||||
/// <returns><c>true</c> if the playing item changed; <c>false</c> otherwise.</returns>
|
||||
public bool Previous()
|
||||
{
|
||||
if (GetPlaylistInternal().Count == 0)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
if (RepeatMode.Equals(GroupRepeatMode.RepeatOne))
|
||||
{
|
||||
LastChange = DateTime.UtcNow;
|
||||
|
||||
Reference in New Issue
Block a user