From 6cc574ea5b3af874daf732a5f99290a7d8856f0e Mon Sep 17 00:00:00 2001 From: purian23 Date: Wed, 15 Jul 2026 14:15:30 -0400 Subject: [PATCH] refactor: unify media control calls to MprisController sync --- quickshell/DMSShellIPC.qml | 4 +- quickshell/Modules/DankBar/Widgets/Media.qml | 8 +-- .../Modules/DankDash/MediaPlayerTab.qml | 13 ++--- .../DankDash/Overview/MediaOverviewCard.qml | 2 +- quickshell/Modules/Lock/LockScreenContent.qml | 2 +- quickshell/Services/MprisController.qml | 33 ++++-------- quickshell/Services/TrackArtService.qml | 52 ++++++++++--------- 7 files changed, 51 insertions(+), 63 deletions(-) diff --git a/quickshell/DMSShellIPC.qml b/quickshell/DMSShellIPC.qml index 35dcb1c7c..3ea4d27ac 100644 --- a/quickshell/DMSShellIPC.qml +++ b/quickshell/DMSShellIPC.qml @@ -531,9 +531,7 @@ Item { } function next(): void { - if (MprisController.activePlayer && MprisController.activePlayer.canGoNext) { - MprisController.activePlayer.next(); - } + MprisController.next(); } function stop(): void { diff --git a/quickshell/Modules/DankBar/Widgets/Media.qml b/quickshell/Modules/DankBar/Widgets/Media.qml index 81821f9a8..ec5e20cb0 100644 --- a/quickshell/Modules/DankBar/Widgets/Media.qml +++ b/quickshell/Modules/DankBar/Widgets/Media.qml @@ -129,7 +129,7 @@ BasePill { if (deltaY > 0) { MprisController.previousOrRewind(); } else { - activePlayer.next(); + MprisController.next(); } } else { scrollAccumulatorY += deltaY; @@ -137,7 +137,7 @@ BasePill { if (scrollAccumulatorY > 0) { MprisController.previousOrRewind(); } else { - activePlayer.next(); + MprisController.next(); } scrollAccumulatorY = 0; } @@ -266,7 +266,7 @@ BasePill { } else if (mouse.button === Qt.MiddleButton) { MprisController.previousOrRewind(); } else if (mouse.button === Qt.RightButton) { - activePlayer.next(); + MprisController.next(); } } } @@ -532,7 +532,7 @@ BasePill { cursorShape: Qt.PointingHandCursor onClicked: { if (activePlayer) { - activePlayer.next(); + MprisController.next(); } } } diff --git a/quickshell/Modules/DankDash/MediaPlayerTab.qml b/quickshell/Modules/DankDash/MediaPlayerTab.qml index c685926b3..bc4485351 100644 --- a/quickshell/Modules/DankDash/MediaPlayerTab.qml +++ b/quickshell/Modules/DankDash/MediaPlayerTab.qml @@ -662,11 +662,12 @@ Item { weight: 500 } - MouseArea { - anchors.fill: parent - hoverEnabled: true - cursorShape: Qt.PointingHandCursor - onClicked: activePlayer && activePlayer.togglePlaying() + StateLayer { + id: playPauseArea + disabled: !root.activePlayer || !root.activePlayer.canTogglePlaying + stateColor: root.onAccent + cornerRadius: parent.radius + onClicked: root.activePlayer.togglePlaying() } ElevationShadow { @@ -706,7 +707,7 @@ Item { anchors.fill: parent hoverEnabled: true cursorShape: Qt.PointingHandCursor - onClicked: activePlayer && activePlayer.next() + onClicked: MprisController.next() } } } diff --git a/quickshell/Modules/DankDash/Overview/MediaOverviewCard.qml b/quickshell/Modules/DankDash/Overview/MediaOverviewCard.qml index c2cbed7fc..102a7496a 100644 --- a/quickshell/Modules/DankDash/Overview/MediaOverviewCard.qml +++ b/quickshell/Modules/DankDash/Overview/MediaOverviewCard.qml @@ -190,7 +190,7 @@ Card { anchors.fill: parent hoverEnabled: true cursorShape: Qt.PointingHandCursor - onClicked: activePlayer?.next() + onClicked: MprisController.next() } } } diff --git a/quickshell/Modules/Lock/LockScreenContent.qml b/quickshell/Modules/Lock/LockScreenContent.qml index 092d76c62..a9a7250d1 100644 --- a/quickshell/Modules/Lock/LockScreenContent.qml +++ b/quickshell/Modules/Lock/LockScreenContent.qml @@ -1635,7 +1635,7 @@ Item { enabled: MprisController.activePlayer?.canGoNext ?? false hoverEnabled: enabled cursorShape: enabled ? Qt.PointingHandCursor : Qt.ArrowCursor - onClicked: MprisController.activePlayer?.next() + onClicked: MprisController.next() } } } diff --git a/quickshell/Services/MprisController.qml b/quickshell/Services/MprisController.qml index a139a288f..1d325b07a 100644 --- a/quickshell/Services/MprisController.qml +++ b/quickshell/Services/MprisController.qml @@ -48,22 +48,12 @@ Singleton { target: root.activePlayer function onTrackTitleChanged() { root.activePlayerStableLength = (root.activePlayer && root.activePlayer.lengthSupported && root.activePlayer.length > 1) ? root.activePlayer.length : 0; - if (root.isIdle(root.activePlayer)) - root._resolveActivePlayer(); - } - function onTrackArtistChanged() { - if (root.isIdle(root.activePlayer)) - root._resolveActivePlayer(); } function onLengthChanged() { if (root.activePlayer && root.activePlayer.lengthSupported && root.activePlayer.length > 1) { root.activePlayerStableLength = root.activePlayer.length; } } - function onPlaybackStateChanged() { - if (root.isIdle(root.activePlayer)) - root._resolveActivePlayer(); - } } onActivePlayerChanged: { @@ -73,18 +63,6 @@ Singleton { onAvailablePlayersChanged: _resolveActivePlayer() Component.onCompleted: _resolveActivePlayer() - Instantiator { - model: root.availablePlayers - delegate: Connections { - required property MprisPlayer modelData - target: modelData - function onIsPlayingChanged() { - if (modelData.isPlaying) - root._resolveActivePlayer(); - } - } - } - function isIdle(player: MprisPlayer): bool { return player && player.playbackState === MprisPlaybackState.Stopped @@ -93,14 +71,15 @@ Singleton { } function _resolveActivePlayer(): void { + // Keep the selected player stable across transient metadata changes. + if (activePlayer && availablePlayers.indexOf(activePlayer) >= 0) + return; const playing = availablePlayers.find(p => p.isPlaying); if (playing) { activePlayer = playing; _persistIdentity(playing.identity); return; } - if (activePlayer && availablePlayers.indexOf(activePlayer) >= 0 && !isIdle(activePlayer)) - return; const savedId = SessionData.lastPlayerIdentity; if (savedId) { const match = availablePlayers.find(p => p.identity === savedId); @@ -150,4 +129,10 @@ Singleton { else if (activePlayer.canGoPrevious) activePlayer.previous(); } + + function next(): void { + const player = activePlayer; + if (player?.canGoNext) + player.next(); + } } diff --git a/quickshell/Services/TrackArtService.qml b/quickshell/Services/TrackArtService.qml index cd1fe8cbd..c65795f5d 100644 --- a/quickshell/Services/TrackArtService.qml +++ b/quickshell/Services/TrackArtService.qml @@ -56,13 +56,13 @@ Singleton { return ""; } - function _commit(u) { + function _commit(u, artKey, srcUrl) { resolvedArtUrl = u; - _committedArtKey = u !== "" ? _pendingArtKey : ""; - _committedSrcUrl = u !== "" ? _lastArtUrl : ""; + _committedArtKey = u !== "" ? artKey : ""; + _committedSrcUrl = u !== "" ? srcUrl : ""; } - function loadArtwork(url) { + function loadArtwork(url, artKey, requestSerial) { if (!url || url === "") { // Keep stale art; only blank once the empty url debounce settles. _lastArtUrl = ""; @@ -85,11 +85,11 @@ Singleton { // 1. First, check if the file already exists locally Proc.runCommand(null, ["test", "-f", filePath], (output, exitCode) => { - if (_lastArtUrl !== targetUrl) + if (_lastArtUrl !== targetUrl || _requestSerial !== requestSerial) return; if (exitCode === 0) { - _commit(localFileUrl); + _commit(localFileUrl, artKey, targetUrl); loading = false; } else { const dlCmd = "mkdir -p \"$(dirname \"$1\")\" && curl -f -s -L -o \"$1\" \"$2\" && mv \"$1\" \"$3\" || { rm -f \"$1\"; exit 1; }"; @@ -102,18 +102,18 @@ Singleton { const tmpPath = filePath + ".tmp"; Proc.runCommand(null, ["sh", "-c", dlCmd, "sh", tmpPath, maxresUrl, filePath], (maxOutput, maxExitCode) => { - if (_lastArtUrl !== targetUrl) + if (_lastArtUrl !== targetUrl || _requestSerial !== requestSerial) return; if (maxExitCode === 0) { - _commit(localFileUrl); + _commit(localFileUrl, artKey, targetUrl); loading = false; } else { Proc.runCommand(null, ["sh", "-c", dlCmd, "sh", tmpPath, mqUrl, filePath], (mqOutput, mqExitCode) => { - if (_lastArtUrl !== targetUrl) + if (_lastArtUrl !== targetUrl || _requestSerial !== requestSerial) return; - _commit(mqExitCode === 0 ? localFileUrl : targetUrl); + _commit(mqExitCode === 0 ? localFileUrl : targetUrl, artKey, targetUrl); loading = false; }, 50, 15000); } @@ -122,10 +122,10 @@ Singleton { // Standard curl download for other remote URLs (e.g. SoundCloud) const tmpPath = filePath + ".tmp"; Proc.runCommand(null, ["sh", "-c", dlCmd, "sh", tmpPath, targetUrl, filePath], (dlOutput, dlExitCode) => { - if (_lastArtUrl !== targetUrl) + if (_lastArtUrl !== targetUrl || _requestSerial !== requestSerial) return; - _commit(dlExitCode === 0 ? localFileUrl : targetUrl); + _commit(dlExitCode === 0 ? localFileUrl : targetUrl, artKey, targetUrl); loading = false; }, 50, 15000); } @@ -141,7 +141,7 @@ Singleton { // Cover lands after metadata, so poll; commit a content-addressed copy so identical bytes keep an identical url const script = "f=\"$1\"; d=\"$2\"; i=0; while [ ! -f \"$f\" ] && [ \"$i\" -lt 20 ]; do sleep 0.15; i=$((i + 1)); done; [ -f \"$f\" ] || exit 1; s=$(sha1sum \"$f\" | cut -c1-40); if [ ! -f \"$d/art_$s\" ]; then mkdir -p \"$d\" && cp \"$f\" \"$d/art_$s.tmp\" && mv \"$d/art_$s.tmp\" \"$d/art_$s\" || exit 1; fi; echo \"$s\""; Proc.runCommand(null, ["sh", "-c", script, "sh", filePath, cacheDir], (output, exitCode) => { - if (_lastArtUrl !== localUrl) + if (_lastArtUrl !== localUrl || _requestSerial !== requestSerial) return; loading = false; if (exitCode !== 0) @@ -149,7 +149,7 @@ Singleton { const sha = (output || "").trim(); if (_artHashDenylist.indexOf(sha) !== -1) return; - _commit("file://" + cacheDir + "/art_" + sha); + _commit("file://" + cacheDir + "/art_" + sha, artKey, localUrl); }, 50, 5000); } @@ -158,7 +158,7 @@ Singleton { interval: 800 onTriggered: { if (root._lastArtUrl === "") - root._commit(""); + root._commit("", "", ""); } } @@ -167,6 +167,7 @@ Singleton { property string _committedArtKey: "" property string _pendingArtKey: "" property string _committedSrcUrl: "" + property int _requestSerial: 0 onActivePlayerChanged: _updateArtUrl() @@ -182,9 +183,10 @@ Singleton { const p = activePlayer; if (!p) return ""; - // +title/artist: KDEconnect reuses one trackid forever; album excluded: Chrome delivers it late, orphaning the lock + // Scope artwork ownership to the MPRIS object and track. + const playerId = p.uniqueId || p.identity || ""; const tid = p.metadata && p.metadata["mpris:trackid"] ? p.metadata["mpris:trackid"].toString() : ""; - return tid + " " + (p.trackTitle || "") + " " + (p.trackArtist || ""); + return playerId + " " + tid + " " + (p.trackTitle || "") + " " + (p.trackArtist || ""); } function artReadyFor(player) { @@ -194,17 +196,19 @@ Singleton { function _updateArtUrl() { const key = _trackKey(); - // Skip once real art is committed for this track (dedup Chrome's multi-size - // re-publish). The lock is set in _commit(), never optimistically, so a rejected - // placeholder or a short-circuited duplicate url can't wedge the real cover out. - if (key !== "" && key === _committedArtKey) - return; + if (key !== _pendingArtKey) { + _requestSerial++; + loading = false; + } _pendingArtKey = key; const url = getArtworkUrl(activePlayer); + // Ignore Chrome's same-track thumbnail size updates. + if (key !== "" && key === _committedArtKey) + return; if (key !== "" && url !== "" && url === _committedSrcUrl) { - _committedArtKey = key; + // Chrome can publish track metadata before its new artwork URL. return; } - loadArtwork(url); + loadArtwork(url, key, _requestSerial); } }