From dd2d9a890a35d2e6981d3317a32a0fdea774a98e Mon Sep 17 00:00:00 2001 From: Storm Dragon Date: Tue, 16 Dec 2025 03:54:17 -0500 Subject: [PATCH] Couple bugs fixed. Shuffle shuffles from beginning. --- src/api/client.py | 158 +++++++++++++++++++++++++++---- src/main_window.py | 59 ++++++++++-- src/managers/playback_manager.py | 155 ++++++++++++++++++++++++++---- 3 files changed, 330 insertions(+), 42 deletions(-) diff --git a/src/api/client.py b/src/api/client.py index 2f767db..a11d619 100644 --- a/src/api/client.py +++ b/src/api/client.py @@ -2,6 +2,8 @@ Subsonic API client for Navidrome communication """ +import hashlib +import json from typing import Optional, List, Dict, Any from urllib.parse import urlencode import requests @@ -53,6 +55,27 @@ class SubsonicClient: self.session.headers.update({ 'User-Agent': f'{self.CLIENT_NAME}/1.0' }) + self._cache: Dict[str, Any] = {} + + def _cacheKey(self, endpoint: str, params: Optional[dict] = None) -> str: + """Generate a cache key from endpoint and parameters.""" + keyData = {"endpoint": endpoint, "params": params or {}} + keyString = json.dumps(keyData, sort_keys=True) + return hashlib.md5(keyString.encode()).hexdigest() + + def _getCached(self, endpoint: str, params: Optional[dict] = None) -> Optional[Any]: + """Retrieve a cached response if available.""" + key = self._cacheKey(endpoint, params) + return self._cache.get(key) + + def _setCache(self, endpoint: str, params: Optional[dict], value: Any) -> None: + """Store a response in the cache.""" + key = self._cacheKey(endpoint, params) + self._cache[key] = value + + def clearCache(self) -> None: + """Clear all cached responses. Call this on library refresh.""" + self._cache.clear() def _makeRequest(self, endpoint: str, params: dict = None) -> dict: """ @@ -187,6 +210,10 @@ class SubsonicClient: if musicFolderId: params['musicFolderId'] = musicFolderId + cached = self._getCached('getArtists', params) + if cached is not None: + return cached + response = self._makeRequest('getArtists', params) artists = [] @@ -195,6 +222,7 @@ class SubsonicClient: for artistData in index.get('artist', []): artists.append(Artist.fromDict(artistData)) + self._setCache('getArtists', params, artists) return artists def getArtist(self, artistId: str) -> Dict[str, Any]: @@ -207,13 +235,20 @@ class SubsonicClient: Returns: dict with 'artist' (Artist) and 'albums' (List[Album]) """ - response = self._makeRequest('getArtist', {'id': artistId}) + params = {'id': artistId} + cached = self._getCached('getArtist', params) + if cached is not None: + return cached + + response = self._makeRequest('getArtist', params) artistData = response.get('artist', {}) artist = Artist.fromDict(artistData) albums = [Album.fromDict(a) for a in artistData.get('album', [])] - return {'artist': artist, 'albums': albums} + result = {'artist': artist, 'albums': albums} + self._setCache('getArtist', params, result) + return result def getAlbum(self, albumId: str) -> Dict[str, Any]: """ @@ -225,13 +260,20 @@ class SubsonicClient: Returns: dict with 'album' (Album) and 'songs' (List[Song]) """ - response = self._makeRequest('getAlbum', {'id': albumId}) + params = {'id': albumId} + cached = self._getCached('getAlbum', params) + if cached is not None: + return cached + + response = self._makeRequest('getAlbum', params) albumData = response.get('album', {}) album = Album.fromDict(albumData) songs = [Song.fromDict(s) for s in albumData.get('song', [])] - return {'album': album, 'songs': songs} + result = {'album': album, 'songs': songs} + self._setCache('getAlbum', params, result) + return result def getSong(self, songId: str) -> Song: """ @@ -243,8 +285,15 @@ class SubsonicClient: Returns: Song object """ - response = self._makeRequest('getSong', {'id': songId}) - return Song.fromDict(response.get('song', {})) + params = {'id': songId} + cached = self._getCached('getSong', params) + if cached is not None: + return cached + + response = self._makeRequest('getSong', params) + result = Song.fromDict(response.get('song', {})) + self._setCache('getSong', params, result) + return result def getGenres(self) -> List[Genre]: """ @@ -253,9 +302,15 @@ class SubsonicClient: Returns: List of Genre objects """ + cached = self._getCached('getGenres', None) + if cached is not None: + return cached + response = self._makeRequest('getGenres') genresData = response.get('genres', {}) - return [Genre.fromDict(g) for g in genresData.get('genre', [])] + result = [Genre.fromDict(g) for g in genresData.get('genre', [])] + self._setCache('getGenres', None, result) + return result def getAlbumList(self, listType: str, size: int = 20, offset: int = 0, musicFolderId: str = None, genre: str = None, @@ -289,9 +344,15 @@ class SubsonicClient: if toYear: params['toYear'] = toYear + cached = self._getCached('getAlbumList2', params) + if cached is not None: + return cached + response = self._makeRequest('getAlbumList2', params) albumsData = response.get('albumList2', {}) - return [Album.fromDict(a) for a in albumsData.get('album', [])] + result = [Album.fromDict(a) for a in albumsData.get('album', [])] + self._setCache('getAlbumList2', params, result) + return result def getSongsByGenre(self, genre: str, count: int = 100, offset: int = 0) -> List[Song]: """ @@ -310,9 +371,16 @@ class SubsonicClient: 'count': count, 'offset': offset } + + cached = self._getCached('getSongsByGenre', params) + if cached is not None: + return cached + response = self._makeRequest('getSongsByGenre', params) songsData = response.get('songsByGenre', {}) - return [Song.fromDict(s) for s in songsData.get('song', [])] + result = [Song.fromDict(s) for s in songsData.get('song', [])] + self._setCache('getSongsByGenre', params, result) + return result def getRandomSongs(self, size: int = 10, genre: str = None, fromYear: int = None, toYear: int = None) -> List[Song]: @@ -362,14 +430,21 @@ class SubsonicClient: 'albumCount': albumCount, 'songCount': songCount } + + cached = self._getCached('search3', params) + if cached is not None: + return cached + response = self._makeRequest('search3', params) searchResult = response.get('searchResult3', {}) - return { + result = { 'artists': [Artist.fromDict(a) for a in searchResult.get('artist', [])], 'albums': [Album.fromDict(a) for a in searchResult.get('album', [])], 'songs': [Song.fromDict(s) for s in searchResult.get('song', [])] } + self._setCache('search3', params, result) + return result # ==================== Playlists ==================== @@ -380,9 +455,15 @@ class SubsonicClient: Returns: List of Playlist objects """ + cached = self._getCached('getPlaylists', None) + if cached is not None: + return cached + response = self._makeRequest('getPlaylists') playlistsData = response.get('playlists', {}) - return [Playlist.fromDict(p) for p in playlistsData.get('playlist', [])] + result = [Playlist.fromDict(p) for p in playlistsData.get('playlist', [])] + self._setCache('getPlaylists', None, result) + return result def getPlaylist(self, playlistId: str) -> Playlist: """ @@ -394,8 +475,15 @@ class SubsonicClient: Returns: Playlist object with songs populated """ - response = self._makeRequest('getPlaylist', {'id': playlistId}) - return Playlist.fromDict(response.get('playlist', {})) + params = {'id': playlistId} + cached = self._getCached('getPlaylist', params) + if cached is not None: + return cached + + response = self._makeRequest('getPlaylist', params) + result = Playlist.fromDict(response.get('playlist', {})) + self._setCache('getPlaylist', params, result) + return result def createPlaylist(self, name: str, songIds: List[str] = None) -> Playlist: """ @@ -413,6 +501,7 @@ class SubsonicClient: params['songId'] = songIds response = self._makeRequest('createPlaylist', params) + self._invalidatePlaylistCache() return Playlist.fromDict(response.get('playlist', {})) def updatePlaylist(self, playlistId: str, name: str = None, @@ -436,6 +525,7 @@ class SubsonicClient: params['songIndexToRemove'] = songIndexesToRemove self._makeRequest('updatePlaylist', params) + self._invalidatePlaylistCache(playlistId) def deletePlaylist(self, playlistId: str) -> None: """ @@ -445,6 +535,15 @@ class SubsonicClient: playlistId: ID of the playlist to delete """ self._makeRequest('deletePlaylist', {'id': playlistId}) + self._invalidatePlaylistCache(playlistId) + + def _invalidatePlaylistCache(self, playlistId: str = None) -> None: + """Invalidate playlist-related cache entries.""" + listKey = self._cacheKey('getPlaylists', None) + self._cache.pop(listKey, None) + if playlistId: + detailKey = self._cacheKey('getPlaylist', {'id': playlistId}) + self._cache.pop(detailKey, None) # ==================== User Actions ==================== @@ -463,6 +562,7 @@ class SubsonicClient: }.get(itemType, 'id') self._makeRequest('star', {paramName: itemId}) + self._invalidateStarredCache() def unstar(self, itemId: str, itemType: str = 'song') -> None: """ @@ -479,6 +579,12 @@ class SubsonicClient: }.get(itemType, 'id') self._makeRequest('unstar', {paramName: itemId}) + self._invalidateStarredCache() + + def _invalidateStarredCache(self) -> None: + """Invalidate the starred/favorites cache.""" + key = self._cacheKey('getStarred2', None) + self._cache.pop(key, None) def setRating(self, itemId: str, rating: int) -> None: """ @@ -545,14 +651,20 @@ class SubsonicClient: Returns: dict with 'artists', 'albums', 'songs' lists """ + cached = self._getCached('getStarred2', None) + if cached is not None: + return cached + response = self._makeRequest('getStarred2') starredData = response.get('starred2', {}) - return { + result = { 'artists': [Artist.fromDict(a) for a in starredData.get('artist', [])], 'albums': [Album.fromDict(a) for a in starredData.get('album', [])], 'songs': [Song.fromDict(s) for s in starredData.get('song', [])] } + self._setCache('getStarred2', None, result) + return result # ==================== Recommendations ==================== @@ -570,9 +682,16 @@ class SubsonicClient: if not artist: return [] params = {'artist': artist, 'count': count} + + cached = self._getCached('getTopSongs', params) + if cached is not None: + return cached + response = self._makeRequest('getTopSongs', params) topData = response.get('topSongs', {}) - return [Song.fromDict(s) for s in topData.get('song', [])] + result = [Song.fromDict(s) for s in topData.get('song', [])] + self._setCache('getTopSongs', params, result) + return result def getSimilarSongs(self, songId: str, count: int = 50) -> List[Song]: """ @@ -585,9 +704,16 @@ class SubsonicClient: if not songId: return [] params = {'id': songId, 'count': count} + + cached = self._getCached('getSimilarSongs2', params) + if cached is not None: + return cached + response = self._makeRequest('getSimilarSongs2', params) data = response.get('similarSongs2', {}) - return [Song.fromDict(s) for s in data.get('song', [])] + result = [Song.fromDict(s) for s in data.get('song', [])] + self._setCache('getSimilarSongs2', params, result) + return result def getNowPlaying(self) -> List[Song]: """ diff --git a/src/main_window.py b/src/main_window.py index 8d12e68..378d459 100644 --- a/src/main_window.py +++ b/src/main_window.py @@ -58,6 +58,7 @@ class MainWindow(QMainWindow): self.playback = PlaybackManager(self) self.mpris: Optional[MprisService] = None self._executor = ThreadPoolExecutor(max_workers=4) + self._shutting_down = False self.logger = logging.getLogger("navipy.main_window") self.maxBulkSongs: Optional[int] = None # Unlimited; use throttling instead self.bulkRequestBatch = 5 @@ -531,6 +532,9 @@ class MainWindow(QMainWindow): if not self.client: return + # Clear cache to force fresh data from server + self.client.clearCache() + self.refreshAction.setEnabled(False) self.searchAction.setEnabled(False) self.libraryTree.clear() @@ -1058,6 +1062,8 @@ class MainWindow(QMainWindow): def collectSongsForItem(self, item: QTreeWidgetItem) -> Tuple[List[Song], str, bool]: """Resolve a tree item into a list of songs for queueing or playlist actions.""" + if self._shutting_down: + return [], "", False data: Dict = item.data(0, Qt.UserRole) or {} itemType = data.get("type") label = item.text(0) @@ -1077,11 +1083,15 @@ class MainWindow(QMainWindow): songs: List[Song] = albumData.get("songs", []) return songs, (album.name if album else label), truncated if itemType == "artist": + if self._shutting_down: + return [], label, truncated artistData = self.client.getArtist(data.get("id")) artist: Optional[Artist] = artistData.get("artist") albums: List[Album] = artistData.get("albums", []) songs: List[Song] = [] for album in albums: + if self._shutting_down: + break albumData = self.client.getAlbum(album.id) songs.extend(albumData.get("songs", [])) request_count += 1 @@ -1095,11 +1105,17 @@ class MainWindow(QMainWindow): songs = self.client.getSongsByGenre(genreName) return songs, (genreName or label), truncated if itemType == "artists_root": + if self._shutting_down: + return [], "all artists", truncated artists: List[Artist] = self.client.getArtists() songs: List[Song] = [] for artist in artists: + if self._shutting_down: + break artistData = self.client.getArtist(artist.id) for album in artistData.get("albums", []): + if self._shutting_down: + break albumData = self.client.getAlbum(album.id) songs.extend(albumData.get("songs", [])) request_count += 1 @@ -1359,12 +1375,16 @@ class MainWindow(QMainWindow): def _fetchArtistSongs(self, artistId: str) -> Tuple[Optional[Artist], List[Song]]: """Fetch all songs for an artist across albums with throttling.""" + if self._shutting_down: + return None, [] artistData = self.client.getArtist(artistId) artist: Optional[Artist] = artistData.get("artist") albums: List[Album] = artistData.get("albums", []) songs: List[Song] = [] request_count = 0 for album in albums: + if self._shutting_down: + break albumData = self.client.getAlbum(album.id) songs.extend(albumData.get("songs", [])) request_count += 1 @@ -1381,6 +1401,8 @@ class MainWindow(QMainWindow): def _progressiveLoadAllArtists(self) -> int: """Fetch songs across all artists in chunks and enqueue progressively.""" + if self._shutting_down: + return 0 artists: List[Artist] = self.client.getArtists() batch: List[Song] = [] request_count = 0 @@ -1388,8 +1410,12 @@ class MainWindow(QMainWindow): first_batch = True for artist in artists: + if self._shutting_down: + break artistData = self.client.getArtist(artist.id) for album in artistData.get("albums", []): + if self._shutting_down: + break albumData = self.client.getAlbum(album.id) songs = albumData.get("songs", []) batch.extend(songs) @@ -1401,7 +1427,7 @@ class MainWindow(QMainWindow): first_batch = False self._throttle_requests(request_count) - if batch: + if batch and not self._shutting_down: self._on_ui(lambda b=batch.copy(), first=first_batch: self._enqueueBatchForPlayback(b, "all artists", first)) return total @@ -1767,9 +1793,6 @@ class MainWindow(QMainWindow): except Exception: # As a last resort, change accessible name to trigger a change event self.liveRegion.setAccessibleName(text) - except Exception: - # If accessibility event fails, rely on status bar message - pass # Shortcut handlers with feedback when no playback is active @@ -1912,11 +1935,35 @@ class MainWindow(QMainWindow): def closeEvent(self, event): """Handle window close""" + # Signal background tasks to stop + self._shutting_down = True + self.settings.set("playback", "volume", self.volume) self.settings.save() + + # Stop playback and release media resources + try: + self.playback.player.stop() + self.playback.player.setSource("") + except Exception: + pass + + # Shutdown MPRIS (stops GLib main loop thread) if self.mpris: - self.mpris.shutdown() + try: + self.mpris.shutdown() + except Exception: + pass + + # Shutdown thread pool - don't wait for tasks if self._executor: - self._executor.shutdown(wait=False) + self._executor.shutdown(wait=False, cancel_futures=True) + self.logger.info("Main window closed") event.accept() + + # Schedule forced exit after event loop processes the close + def forceExit(): + import os + os._exit(0) + QTimer.singleShot(100, forceExit) diff --git a/src/managers/playback_manager.py b/src/managers/playback_manager.py index fde2a56..b4e83ed 100644 --- a/src/managers/playback_manager.py +++ b/src/managers/playback_manager.py @@ -5,7 +5,7 @@ Playback manager using PySide6 multimedia for streaming audio from __future__ import annotations from typing import Callable, List, Optional -from random import randint +import random from PySide6.QtCore import QObject, QUrl, Signal from PySide6.QtMultimedia import QAudioOutput, QMediaPlayer @@ -38,6 +38,11 @@ class PlaybackManager(QObject): self.shuffleEnabled = False self.repeatMode = "none" # none, one, all + # Shuffle state: shuffleOrder is a list of queue indices in shuffled order + # shufflePosition tracks where we are in that order + self._shuffleOrder: List[int] = [] + self._shufflePosition: int = -1 + self.player.playbackStateChanged.connect(self._handlePlaybackStateChanged) self.player.positionChanged.connect(self._handlePositionChanged) self.player.mediaStatusChanged.connect(self._handleMediaStatus) @@ -54,10 +59,12 @@ class PlaybackManager(QObject): def enqueue(self, song: Song, playNow: bool = False) -> None: """Add a song to the queue, optionally starting playback immediately.""" + newIndex = len(self.queue) self.queue.append(song) + self._addToShuffleOrder(newIndex) self.queueChanged.emit(self.queue.copy()) if playNow or self.currentIndex == -1: - self.playIndex(len(self.queue) - 1) + self.playIndex(newIndex) def enqueueMany(self, songs: List[Song], playFirst: bool = False) -> None: """Add multiple songs to the queue.""" @@ -65,14 +72,25 @@ class PlaybackManager(QObject): return startIndex = len(self.queue) self.queue.extend(songs) + # Add new indices to shuffle order + for i in range(startIndex, len(self.queue)): + self._addToShuffleOrder(i) self.queueChanged.emit(self.queue.copy()) if playFirst or self.currentIndex == -1: - self.playIndex(startIndex) + # When starting fresh playback with shuffle, pick a random song + if self.shuffleEnabled and self.currentIndex == -1: + self._shufflePosition = 0 + targetIndex = self._shuffleOrder[0] if self._shuffleOrder else startIndex + self.playIndex(targetIndex) + else: + self.playIndex(startIndex) def clearQueue(self) -> None: """Clear the play queue and stop playback.""" self.queue = [] self.currentIndex = -1 + self._shuffleOrder = [] + self._shufflePosition = -1 self.player.stop() self.queueChanged.emit(self.queue.copy()) self.playbackStateChanged.emit("stopped") @@ -86,6 +104,8 @@ class PlaybackManager(QObject): return self.currentIndex = index + # Sync shuffle position to match the played index + self._syncShufflePosition(index) song = self.queue[index] streamUrl = self.streamResolver(song) self.player.setSource(QUrl(streamUrl)) @@ -102,7 +122,7 @@ class PlaybackManager(QObject): self.player.play() else: if self.currentIndex == -1 and self.queue: - self.playIndex(0) + self._startPlayback() elif self.currentIndex != -1: self.player.play() @@ -115,7 +135,7 @@ class PlaybackManager(QObject): self.player.play() return if self.currentIndex == -1 and self.queue: - self.playIndex(0) + self._startPlayback() elif self.currentIndex != -1: self.player.play() @@ -133,17 +153,27 @@ class PlaybackManager(QObject): self.playIndex(self.currentIndex) return - if self.shuffleEnabled: - target = randint(0, len(self.queue) - 1) + if self.shuffleEnabled and self._shuffleOrder: + # Move to next position in shuffle order + nextPosition = self._shufflePosition + 1 + if nextPosition >= len(self._shuffleOrder): + if self.repeatMode == "all": + # Reshuffle for the next cycle + self._regenerateShuffleOrder() + nextPosition = 0 + else: + self.player.stop() + return + self._shufflePosition = nextPosition + target = self._shuffleOrder[nextPosition] else: target = self.currentIndex + 1 - - if target >= len(self.queue): - if self.repeatMode == "all": - target = 0 - else: - self.player.stop() - return + if target >= len(self.queue): + if self.repeatMode == "all": + target = 0 + else: + self.player.stop() + return self.playIndex(target) @@ -151,12 +181,25 @@ class PlaybackManager(QObject): """Return to the previous track.""" if not self.queue: return - target = self.currentIndex - 1 - if target < 0: - if self.repeatMode == "all": - target = len(self.queue) - 1 - else: - target = 0 + + if self.shuffleEnabled and self._shuffleOrder: + # Move to previous position in shuffle order + prevPosition = self._shufflePosition - 1 + if prevPosition < 0: + if self.repeatMode == "all": + prevPosition = len(self._shuffleOrder) - 1 + else: + prevPosition = 0 + self._shufflePosition = prevPosition + target = self._shuffleOrder[prevPosition] + else: + target = self.currentIndex - 1 + if target < 0: + if self.repeatMode == "all": + target = len(self.queue) - 1 + else: + target = 0 + self.playIndex(target) def seek(self, positionMs: int) -> None: @@ -165,7 +208,15 @@ class PlaybackManager(QObject): def setShuffle(self, enabled: bool) -> None: """Enable or disable shuffle mode.""" + wasEnabled = self.shuffleEnabled self.shuffleEnabled = enabled + if enabled and not wasEnabled: + # Just turned on shuffle - regenerate order with current song first + self._regenerateShuffleOrder(startWithCurrent=True) + elif not enabled and wasEnabled: + # Turned off shuffle - clear shuffle state + self._shuffleOrder = [] + self._shufflePosition = -1 def setRepeatMode(self, mode: str) -> None: """Set repeat mode: none, one, or all.""" @@ -178,6 +229,70 @@ class PlaybackManager(QObject): return self.queue[self.currentIndex] return None + # ===== Shuffle helpers ===== + + def _regenerateShuffleOrder(self, startWithCurrent: bool = False) -> None: + """Create a new shuffled order of queue indices.""" + if not self.queue: + self._shuffleOrder = [] + self._shufflePosition = -1 + return + + indices = list(range(len(self.queue))) + random.shuffle(indices) + + if startWithCurrent and self.currentIndex >= 0: + # Put current song at the front so we continue from here + if self.currentIndex in indices: + indices.remove(self.currentIndex) + indices.insert(0, self.currentIndex) + self._shufflePosition = 0 + else: + self._shufflePosition = 0 + + self._shuffleOrder = indices + + def _addToShuffleOrder(self, index: int) -> None: + """Add a new queue index to the shuffle order at a random position.""" + if not self.shuffleEnabled: + return + if not self._shuffleOrder: + self._shuffleOrder = [index] + return + # Insert at a random position after the current position + # so we don't replay songs we've already heard + if self._shufflePosition < 0: + insertPos = random.randint(0, len(self._shuffleOrder)) + else: + # Insert somewhere after current position + insertPos = random.randint(self._shufflePosition + 1, len(self._shuffleOrder)) + self._shuffleOrder.insert(insertPos, index) + + def _syncShufflePosition(self, queueIndex: int) -> None: + """Update shuffle position to match a directly-selected queue index.""" + if not self.shuffleEnabled or not self._shuffleOrder: + return + try: + self._shufflePosition = self._shuffleOrder.index(queueIndex) + except ValueError: + # Index not in shuffle order - add it and set position + self._shuffleOrder.append(queueIndex) + self._shufflePosition = len(self._shuffleOrder) - 1 + + def _startPlayback(self) -> None: + """Start playback, respecting shuffle mode for initial song selection.""" + if not self.queue: + return + if self.shuffleEnabled: + self._regenerateShuffleOrder() + if self._shuffleOrder: + self._shufflePosition = 0 + self.playIndex(self._shuffleOrder[0]) + else: + self.playIndex(0) + else: + self.playIndex(0) + # Signal handlers def _handlePlaybackStateChanged(self, state) -> None: