Hey! I made some performance optimizations and fixed the automatic player detection. - #2
Open
bhavya-goel-11 wants to merge 1 commit into
Open
Hey! I made some performance optimizations and fixed the automatic player detection.#2bhavya-goel-11 wants to merge 1 commit into
bhavya-goel-11 wants to merge 1 commit into
Conversation
bhavya-goel-11
commented
May 8, 2026
bhavya-goel-11
commented
- Added reliable automatic player detection and switching using global PropertiesChanged and NameOwnerChanged signals, correctly identifying background playback states for native and browser players.
- Switched lyrics fetching from Gio.File to asynchronous Soup.Session to improve loading speed and eliminate UI lag.
- Implemented a 10MB local disk cache for lyrics with automatic pruning to prevent redundant API calls.
- Added a new 'Lyrics Lookahead' preference (default 400ms) to optionally display lyrics slightly before they are sung.
- Reduced the lyric synchronization polling interval from 500ms to 200ms for noticeably smoother UI transitions.
- Added safety checks to prevent UI updates after the extension is destroyed.
…detection - Added reliable automatic player detection and switching using global PropertiesChanged and NameOwnerChanged signals, correctly identifying background playback states for native and browser players. - Switched lyrics fetching from Gio.File to asynchronous Soup.Session to improve loading speed and eliminate UI lag. - Implemented a 10MB local disk cache for lyrics with automatic pruning to prevent redundant API calls. - Added a new 'Lyrics Lookahead' preference (default 400ms) to optionally display lyrics slightly before they are sung. - Reduced the lyric synchronization polling interval from 500ms to 200ms for noticeably smoother UI transitions. - Added safety checks to prevent UI updates after the extension is destroyed.
There was a problem hiding this comment.
Pull request overview
This PR updates the GNOME Shell extension to improve automatic MPRIS player detection/switching, reduce UI lag during lyrics fetch, and add user-tunable lyric timing.
Changes:
- Added global DBus monitoring (NameOwnerChanged + PropertiesChanged) to more reliably detect active/playing MPRIS players.
- Switched lyrics fetching to async
Soup.Sessionand added an in-memory + on-disk lyrics cache with pruning. - Added a new “Lyrics Lookahead” (
lyrics-offset) preference and applied it to lyric timing; reduced sync polling interval to 200ms and added destroy-safety guards.
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| schemas/org.gnome.shell.extensions.spotify-lyrics.gschema.xml | Adds lyrics-offset integer setting (default 400ms, range ±2000ms). |
| prefs.js | Adds a preferences UI control (spin row) for the lyrics lookahead offset. |
| extension.js | Implements async lyrics fetching + caching, updated DBus player detection, lookahead offset in lyric sync, and destroy-safety checks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const titleVariant = metadataDict['xesam:title']; | ||
| if (titleVariant) { | ||
| const unpackedTitle = titleVariant.deep_unpack(); | ||
| title = Array.isArray(unpackedTitle) ? unpackedTitle[0] : unpackedTitle; |
| const artistVariant = metadataDict['xesam:artist']; | ||
| if (artistVariant) { | ||
| const unpackedArtist = artistVariant.deep_unpack(); | ||
| artist = Array.isArray(unpackedArtist) ? unpackedArtist[0] : unpackedArtist; |
Comment on lines
+621
to
+644
| const cacheKey = this._getLyricsCacheKey(title, artist); | ||
| const cachedLyrics = this._lyricsCache.get(cacheKey); | ||
| if (cachedLyrics) { | ||
| this._touchLyricsDiskCache(cacheKey, cachedLyrics); | ||
| this._displayLyricsResult(cachedLyrics, title, artist); | ||
| return; | ||
| } | ||
|
|
||
| const diskCachedLyrics = this._readLyricsFromDiskCache(cacheKey); | ||
| if (diskCachedLyrics) { | ||
| this._lyricsCache.set(cacheKey, diskCachedLyrics); | ||
| this._touchLyricsDiskCache(cacheKey, diskCachedLyrics); | ||
| this._displayLyricsResult(diskCachedLyrics, title, artist); | ||
| return; | ||
| } | ||
|
|
||
| // Build API URL | ||
| const url = `${LYRICS_API_URL}?artist_name=${encodeURIComponent(artist)}&track_name=${encodeURIComponent(title)}`; | ||
| let url = `${LYRICS_API_URL}?artist_name=${encodeURIComponent(artist)}&track_name=${encodeURIComponent(title)}`; | ||
| if (album && album !== 'Unknown Album') { | ||
| url += `&album_name=${encodeURIComponent(album)}`; | ||
| } | ||
| if (duration && duration > 0) { | ||
| url += `&duration=${duration}`; | ||
| } |
Comment on lines
+728
to
+742
| _touchLyricsDiskCache(cacheKey, lyricsResult) { | ||
| lyricsResult.frequency = (lyricsResult.frequency || 1) + 1; | ||
| this._writeLyricsToDiskCache(cacheKey, lyricsResult); | ||
| } | ||
|
|
||
| _writeLyricsToDiskCache(cacheKey, lyricsResult, shouldPrune = true) { | ||
| try { | ||
| this._initLyricsCacheDirectory(); | ||
| GLib.file_set_contents( | ||
| this._getLyricsCachePath(cacheKey), | ||
| this._serializeLyricsResult(lyricsResult) | ||
| ); | ||
| if (shouldPrune) { | ||
| this._pruneLyricsCache(); | ||
| } |
Comment on lines
+307
to
+335
| this._nameOwnerChangedId = this._dbusProxy.connect('g-signal', (proxy, senderName, signalName, parameters) => { | ||
| if (signalName === 'NameOwnerChanged') { | ||
| const [name, oldOwner, newOwner] = parameters.deep_unpack(); | ||
| if (name.startsWith('org.mpris.MediaPlayer2.')) { | ||
| this._findActivePlayer(); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| // Watch for PropertiesChanged from any MPRIS player | ||
| const connection = Gio.bus_get_sync(Gio.BusType.SESSION, null); | ||
| this._propertiesSubscriptionId = connection.signal_subscribe( | ||
| null, | ||
| 'org.freedesktop.DBus.Properties', | ||
| 'PropertiesChanged', | ||
| MPRIS_PLAYER_PATH, | ||
| null, | ||
| Gio.DBusSignalFlags.NONE, | ||
| (conn, sender, path, iface, signal, parameters) => { | ||
| try { | ||
| const [changedIface, changedProps] = parameters.deep_unpack(); | ||
| if (changedIface === MPRIS_PLAYER_INTERFACE) { | ||
| // If playback status changes to Playing on any player, re-evaluate | ||
| if (changedProps['PlaybackStatus']) { | ||
| const status = changedProps['PlaybackStatus'].deep_unpack(); | ||
| if (status === 'Playing') { | ||
| this._findActivePlayer(); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.