From 594a2cde1924a34935b44f6db842cb75e0a1649f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Garc=C3=ADa?= <66568719+arqueon@users.noreply.github.com> Date: Sun, 2 Aug 2026 15:34:19 -0600 Subject: [PATCH] refactor(mpris): enrich metadata across equivalent players (#2928) * fix(mpris): enrich metadata across equivalent players * fix(mpris): address review feedback on source stealing and title stripping - _bestPlayingPlayer: when the active player is playing but not controllable, only hand ownership to a controllable peer that passes isSameTrack; unrelated players can no longer steal the active source. - displayTrackTitle: keep the full trackTitle for display. Suffix stripping now only applies to the isSameTrack matching key, and only for a known app-name allowlist (YouTube, SoundCloud, browsers, etc.) so legitimate titles containing ' | ' are never mangled. Co-Authored-By: Claude Fable 5 --------- Port 1.5 --- .../Modules/DankDash/MediaPlayerTab.qml | 2 +- quickshell/Services/MprisController.qml | 135 ++++++++++++++++-- quickshell/Services/TrackArtService.qml | 45 ++++-- 3 files changed, 165 insertions(+), 17 deletions(-) diff --git a/quickshell/Modules/DankDash/MediaPlayerTab.qml b/quickshell/Modules/DankDash/MediaPlayerTab.qml index 61411403f..46de8f852 100644 --- a/quickshell/Modules/DankDash/MediaPlayerTab.qml +++ b/quickshell/Modules/DankDash/MediaPlayerTab.qml @@ -493,7 +493,7 @@ Item { } StyledText { - text: activePlayer?.trackAlbum || "" + text: MprisController.stableAlbum font.pixelSize: Theme.fontSizeSmall color: Theme.surfaceTextSecondary width: parent.width diff --git a/quickshell/Services/MprisController.qml b/quickshell/Services/MprisController.qml index 3e2cf946e..b3cc0bdcb 100644 --- a/quickshell/Services/MprisController.qml +++ b/quickshell/Services/MprisController.qml @@ -47,6 +47,7 @@ Singleton { // Chromium can report blank metadata between tracks property string stableTitle: "" property string stableArtist: "" + property string stableAlbum: "" Connections { target: root.activePlayer @@ -59,6 +60,9 @@ Singleton { root._syncStableMeta(); root._checkIdle(); } + function onTrackAlbumChanged() { + root._syncStableMeta(); + } function onLengthChanged() { if (root.activePlayer && root.activePlayer.lengthSupported && root.activePlayer.length > 1) { root.activePlayerStableLength = root.activePlayer.length; @@ -72,8 +76,10 @@ Singleton { onActivePlayerChanged: { activePlayerStableLength = (activePlayer && activePlayer.lengthSupported && activePlayer.length > 1) ? activePlayer.length : 0; - stableTitle = activePlayer?.trackTitle || ""; - stableArtist = activePlayer?.trackArtist || ""; + stableTitle = ""; + stableArtist = ""; + stableAlbum = ""; + _syncStableMeta(); _checkIdle(); } @@ -82,14 +88,24 @@ Singleton { if (!p) { stableTitle = ""; stableArtist = ""; + stableAlbum = ""; return; } if (isFirefoxYoutubeHoverPreview(p)) return; - if (p.trackTitle) - stableTitle = p.trackTitle; - if (p.trackArtist) - stableArtist = p.trackArtist; + const metadataPlayer = _bestMetadataPlayer(p); + const nextTitle = displayTrackTitle(metadataPlayer); + const trackChanged = nextTitle && stableTitle && nextTitle.toLowerCase() !== stableTitle.toLowerCase(); + if (trackChanged) { + stableArtist = ""; + stableAlbum = ""; + } + if (nextTitle) + stableTitle = nextTitle; + if (metadataPlayer.trackArtist) + stableArtist = metadataPlayer.trackArtist; + if (metadataPlayer.trackAlbum) + stableAlbum = metadataPlayer.trackAlbum; } // Chromium reports stopped media w/blank metadata, resolve by checking idle status @@ -101,6 +117,7 @@ Singleton { return; root.stableTitle = ""; root.stableArtist = ""; + root.stableAlbum = ""; root._resolveActivePlayer(); } } @@ -122,9 +139,30 @@ Singleton { delegate: Connections { required property MprisPlayer modelData target: modelData + ignoreUnknownSignals: true function onIsPlayingChanged() { + root._resolveActivePlayer(); + root._syncStableMeta(); + } + function onTrackTitleChanged() { if (modelData.isPlaying) root._resolveActivePlayer(); + root._syncStableMeta(); + } + function onTrackArtistChanged() { + if (modelData.isPlaying) + root._resolveActivePlayer(); + root._syncStableMeta(); + } + function onTrackAlbumChanged() { + if (modelData.isPlaying) + root._resolveActivePlayer(); + root._syncStableMeta(); + } + function onMetadataChanged() { + if (modelData.isPlaying) + root._resolveActivePlayer(); + root._syncStableMeta(); } } } @@ -133,9 +171,90 @@ Singleton { return player && player.playbackState === MprisPlaybackState.Stopped && !player.trackTitle && !player.trackArtist; } + // App-name suffixes that browsers/integrations append as " | <App>". + // Only these are stripped for track matching; display always keeps the full title. + readonly property var _appTitleSuffixes: ["youtube", "youtube music", "soundcloud", "spotify", "chrome", "chromium", "firefox", "brave", "vivaldi", "twitch"] + + function _stripAppTitleSuffix(title: string): string { + const idx = title.lastIndexOf(" | "); + if (idx <= 0) + return title; + const suffix = title.substring(idx + 3).trim().toLowerCase(); + return _appTitleSuffixes.indexOf(suffix) !== -1 ? title.substring(0, idx).trim() : title; + } + + // Matching key: strip only known app suffixes so equivalent players line up. + function normalizedTrackTitle(player: MprisPlayer): string { + return _stripAppTitleSuffix((player?.trackTitle || "").trim()).toLowerCase(); + } + + // Display: never strip — a generic " | " cut would mangle legitimate titles. + function displayTrackTitle(player: MprisPlayer): string { + return (player?.trackTitle || "").trim(); + } + + function normalizedTrackArtist(player: MprisPlayer): string { + return (player?.trackArtist || "").trim().toLowerCase(); + } + + function isSameTrack(first: MprisPlayer, second: MprisPlayer): bool { + const firstTitle = normalizedTrackTitle(first); + const secondTitle = normalizedTrackTitle(second); + if (!firstTitle || firstTitle !== secondTitle) + return false; + const firstArtist = normalizedTrackArtist(first); + const secondArtist = normalizedTrackArtist(second); + return !firstArtist || !secondArtist || firstArtist === secondArtist; + } + + function metadataQuality(player: MprisPlayer): int { + if (!player) + return -1; + let quality = player.trackArtist ? 100 : 0; + quality += player.trackTitle ? 40 : 0; + quality += player.trackAlbum ? 20 : 0; + quality += player.trackArtUrl || player.metadata?.["mpris:artUrl"] ? 10 : 0; + quality += player.metadata?.["xesam:url"] ? 5 : 0; + return quality; + } + + function _bestMetadataPlayer(player: MprisPlayer): MprisPlayer { + const equivalents = availablePlayers.filter(candidate => { + return candidate.playbackState !== MprisPlaybackState.Stopped && isSameTrack(player, candidate); + }); + if (equivalents.length === 0) + return player; + return equivalents.reduce((best, candidate) => { + return metadataQuality(candidate) > metadataQuality(best) ? candidate : best; + }, player); + } + + function _bestPlayingPlayer(): MprisPlayer { + const playing = availablePlayers.filter(player => player.isPlaying); + if (playing.length === 0) + return null; + + const controllable = playing.filter(player => player.canControl); + if (activePlayer?.isPlaying) { + if (activePlayer.canControl || controllable.length === 0) + return activePlayer; + // Active source is playing but not controllable: only hand ownership to a + // controllable *equivalent* peer (same track). Never let an unrelated + // player steal the active source while it is still playing. + const mirror = controllable.find(player => isSameTrack(activePlayer, player)); + return mirror || activePlayer; + } + + if (activePlayer?.canControl && activePlayer.playbackState === MprisPlaybackState.Paused) { + const onlyEquivalentMirrors = playing.every(player => isSameTrack(activePlayer, player)); + if (onlyEquivalentMirrors) + return null; + } + return controllable[0] || playing[0]; + } + function _resolveActivePlayer(): void { - // A playing player always wins; otherwise keep the selection stable w/idle - const playing = availablePlayers.find(p => p.isPlaying); + const playing = _bestPlayingPlayer(); if (playing) { if (activePlayer !== playing) { activePlayer = playing; diff --git a/quickshell/Services/TrackArtService.qml b/quickshell/Services/TrackArtService.qml index f955af725..98bd3aa4b 100644 --- a/quickshell/Services/TrackArtService.qml +++ b/quickshell/Services/TrackArtService.qml @@ -26,7 +26,7 @@ Singleton { return hash.toString(16).padStart(8, '0'); } - function getArtworkUrl(player) { + function _directArtworkUrl(player) { if (!player) return ""; let artUrl = player.trackArtUrl || ""; @@ -54,6 +54,20 @@ Singleton { return ""; } + function getArtworkUrl(player) { + const directUrl = _directArtworkUrl(player); + if (directUrl !== "") + return directUrl; + + const equivalent = MprisController.availablePlayers.find(candidate => { + return candidate !== player + && candidate.playbackState !== MprisPlaybackState.Stopped + && MprisController.isSameTrack(player, candidate) + && _directArtworkUrl(candidate) !== ""; + }); + return _directArtworkUrl(equivalent); + } + function _commit(u, artKey, srcUrl) { resolvedArtUrl = u; _committedArtKey = u !== "" ? artKey : ""; @@ -171,11 +185,25 @@ Singleton { onActivePlayerChanged: _updateArtUrl() Connections { - target: root.activePlayer - ignoreUnknownSignals: true - function onTrackTitleChanged() { root._updateArtUrl(); } - function onTrackArtUrlChanged() { root._updateArtUrl(); } - function onMetadataChanged() { root._updateArtUrl(); } + target: MprisController + function onAvailablePlayersChanged() { + root._updateArtUrl(); + } + } + + Instantiator { + model: MprisController.availablePlayers + delegate: Connections { + required property MprisPlayer modelData + target: modelData + ignoreUnknownSignals: true + function onIsPlayingChanged() { root._updateArtUrl(); } + function onTrackTitleChanged() { root._updateArtUrl(); } + function onTrackArtistChanged() { root._updateArtUrl(); } + function onTrackAlbumChanged() { root._updateArtUrl(); } + function onTrackArtUrlChanged() { root._updateArtUrl(); } + function onMetadataChanged() { root._updateArtUrl(); } + } } function _trackKey() { @@ -201,8 +229,9 @@ Singleton { } _pendingArtKey = key; const url = getArtworkUrl(activePlayer); - // Ignore Chrome's same-track thumbnail size updates. - if (key !== "" && key === _committedArtKey) + // Ignore duplicate notifications, but allow a richer peer to replace + // the artwork for the same canonical track. + if (key !== "" && key === _committedArtKey && url === _committedSrcUrl) return; if (key !== "" && url !== "" && url === _committedSrcUrl) { // Chrome can publish track metadata before its new artwork URL.