From 2e0550651e17062c07505c8298fce78341d336a0 Mon Sep 17 00:00:00 2001 From: Albert Vaca Cintora Date: Wed, 7 Jun 2023 20:20:21 +0200 Subject: [PATCH] Remove mpris "nowPlaying" field We've had separate title & artist for a while, and all clients should be using those by now. Also fixes the position change not being emitted when the song changes, and fixes the values being written after emitting that they changed. --- app/qml/MprisSlider.qml | 12 ++---- app/qml/mpris.qml | 14 ++----- interfaces/dbusinterfaces.h | 1 - .../mpriscontrol/mpriscontrolplugin-win.cpp | 4 -- plugins/mpriscontrol/mpriscontrolplugin.cpp | 17 ++------ plugins/mprisremote/mprisremoteplayer.cpp | 40 +++++++------------ plugins/mprisremote/mprisremoteplayer.h | 2 - .../mprisremoteplayermediaplayer2player.cpp | 2 +- plugins/mprisremote/mprisremoteplugin.cpp | 6 --- plugins/mprisremote/mprisremoteplugin.h | 2 - 10 files changed, 26 insertions(+), 74 deletions(-) diff --git a/app/qml/MprisSlider.qml b/app/qml/MprisSlider.qml index 8b07d3cdf..07785d2bd 100644 --- a/app/qml/MprisSlider.qml +++ b/app/qml/MprisSlider.qml @@ -17,7 +17,10 @@ Loader { sourceComponent: plugin.canSeek ? seekBar : progressBar onLastPositionChanged: { - lastPositionTime = new Date(); + if (item != null) { + item.value = lastPosition + lastPositionTime = new Date(); + } } Component { @@ -54,11 +57,4 @@ Loader { onTriggered: item.value = lastPosition + (new Date().getTime() - lastPositionTime.getTime()) } - - Connections { - target: plugin - function onNowPlayingChanged() { - item.value = lastPosition - } - } } diff --git a/app/qml/mpris.qml b/app/qml/mpris.qml index 2447588ea..b470abae8 100644 --- a/app/qml/mpris.qml +++ b/app/qml/mpris.qml @@ -78,36 +78,28 @@ Kirigami.Page model: root.pluginInterface.playerList onCurrentTextChanged: root.pluginInterface.player = currentText } - Label { - id: nowPlaying - Layout.fillWidth: true - text: root.pluginInterface.nowPlaying - visible: root.pluginInterface.title.length == 0 - wrapMode: Text.Wrap - } Label { Layout.fillWidth: true text: root.pluginInterface.title - visible: !nowPlaying.visible wrapMode: Text.Wrap } Label { Layout.fillWidth: true text: root.pluginInterface.artist - visible: !nowPlaying.visible && !artistAlbum.visible && root.pluginInterface.artist.length > 0 + visible: !artistAlbum.visible && root.pluginInterface.artist.length > 0 wrapMode: Text.Wrap } Label { Layout.fillWidth: true text: root.pluginInterface.album - visible: !nowPlaying.visible && !artistAlbum.visible && root.pluginInterface.album.length > 0 + visible: !artistAlbum.visible && root.pluginInterface.album.length > 0 wrapMode: Text.Wrap } Label { id: artistAlbum Layout.fillWidth: true text: i18nd("kdeconnect-app", "%1 - %2", root.pluginInterface.artist, root.pluginInterface.album) - visible: !nowPlaying.visible && root.pluginInterface.album.length > 0 && root.pluginInterface.artist.length > 0 + visible: root.pluginInterface.album.length > 0 && root.pluginInterface.artist.length > 0 wrapMode: Text.Wrap } RowLayout { diff --git a/interfaces/dbusinterfaces.h b/interfaces/dbusinterfaces.h index 4e6daf140..931cdf1c4 100644 --- a/interfaces/dbusinterfaces.h +++ b/interfaces/dbusinterfaces.h @@ -152,7 +152,6 @@ class KDECONNECTINTERFACES_EXPORT MprisDbusInterface : public OrgKdeKdeconnectDe // the signals for the properties Q_PROPERTY(bool isPlaying READ isPlaying NOTIFY propertiesChangedProxy) Q_PROPERTY(int length READ length NOTIFY propertiesChangedProxy) - Q_PROPERTY(QString nowPlaying READ nowPlaying NOTIFY propertiesChangedProxy) Q_PROPERTY(QString title READ title NOTIFY propertiesChangedProxy) Q_PROPERTY(QString artist READ artist NOTIFY propertiesChangedProxy) Q_PROPERTY(QString album READ album NOTIFY propertiesChangedProxy) diff --git a/plugins/mpriscontrol/mpriscontrolplugin-win.cpp b/plugins/mpriscontrol/mpriscontrolplugin-win.cpp index c47fcd379..7e9783842 100644 --- a/plugins/mpriscontrol/mpriscontrolplugin-win.cpp +++ b/plugins/mpriscontrol/mpriscontrolplugin-win.cpp @@ -71,10 +71,6 @@ void MprisControlPlugin::sendMediaProperties(std::variantcanSeek()) { + // Always also update the position if can seek + bool canSeek = mediaPlayer2PlayerInterface->canSeek(); + np.set(QStringLiteral("canSeek"), canSeek); + if (canSeek) { long long pos = mediaPlayer2PlayerInterface->position(); np.set(QStringLiteral("pos"), pos / 1000); // Send milis instead of nanos } @@ -388,17 +386,10 @@ void MprisControlPlugin::mprisPlayerMetadataToNetworkPacket(NetworkPacket &np, c } } - QString nowPlaying = title; - - if (!artist.isEmpty()) { - nowPlaying = artist + QStringLiteral(" - ") + title; - } - np.set(QStringLiteral("title"), title); np.set(QStringLiteral("artist"), artist); np.set(QStringLiteral("album"), album); np.set(QStringLiteral("albumArtUrl"), albumArtUrl); - np.set(QStringLiteral("nowPlaying"), nowPlaying); bool hasLength = false; long long length = nowPlayingMap[QStringLiteral("mpris:length")].toLongLong(&hasLength) / 1000; // nanoseconds to milliseconds diff --git a/plugins/mprisremote/mprisremoteplayer.cpp b/plugins/mprisremote/mprisremoteplayer.cpp index 691083463..7cbdae0ff 100644 --- a/plugins/mprisremote/mprisremoteplayer.cpp +++ b/plugins/mprisremote/mprisremoteplayer.cpp @@ -22,7 +22,6 @@ MprisRemotePlayer::MprisRemotePlayer(QString id, MprisRemotePlugin *plugin) , m_canPause(true) , m_canGoPrevious(true) , m_canGoNext(true) - , m_nowPlaying() , m_volume(50) , m_length(0) , m_lastPosition(0) @@ -54,30 +53,27 @@ void MprisRemotePlayer::parseNetworkPacket(const NetworkPacket &np) bool trackInfoHasChanged = false; // Track properties - QString newNowPlaying = np.get(QStringLiteral("nowPlaying"), m_nowPlaying); QString newTitle = np.get(QStringLiteral("title"), m_title); QString newArtist = np.get(QStringLiteral("artist"), m_artist); QString newAlbum = np.get(QStringLiteral("album"), m_album); int newLength = np.get(QStringLiteral("length"), m_length); // Check if they changed - if (newNowPlaying != m_nowPlaying || newTitle != m_title || newArtist != m_artist || newAlbum != m_album || newLength != m_length) { + if (newTitle != m_title || newArtist != m_artist || newAlbum != m_album || newLength != m_length) { trackInfoHasChanged = true; + m_title = newTitle; + m_artist = newArtist; + m_album = newAlbum; + m_length = newLength; Q_EMIT trackInfoChanged(); } - // Set the new values - m_nowPlaying = newNowPlaying; - m_title = newTitle; - m_artist = newArtist; - m_album = newAlbum; - m_length = newLength; // Check volume changes int newVolume = np.get(QStringLiteral("volume"), m_volume); if (newVolume != m_volume) { + m_volume = newVolume; Q_EMIT volumeChanged(); } - m_volume = newVolume; if (np.has(QStringLiteral("pos"))) { // Check position @@ -86,8 +82,8 @@ void MprisRemotePlayer::parseNetworkPacket(const NetworkPacket &np) m_lastPosition = newLastPosition; m_lastPositionTime = QDateTime::currentMSecsSinceEpoch(); - // Only consider it seeking if the position changed more than 1 second, and the track has not changed - if (qAbs(positionDiff) >= 1000 && !trackInfoHasChanged) { + // Only consider it seeking if the position changed more than 1 second or the track has changed + if (qAbs(positionDiff) >= 1000 || trackInfoHasChanged) { Q_EMIT positionChanged(); } } @@ -95,9 +91,9 @@ void MprisRemotePlayer::parseNetworkPacket(const NetworkPacket &np) // Check if we started/stopped playing bool newPlaying = np.get(QStringLiteral("isPlaying"), m_playing); if (newPlaying != m_playing) { + m_playing = newPlaying; Q_EMIT playingChanged(); } - m_playing = newPlaying; // Control properties bool newCanSeek = np.get(QStringLiteral("canSeek"), m_canSeek); @@ -105,17 +101,14 @@ void MprisRemotePlayer::parseNetworkPacket(const NetworkPacket &np) bool newCanPause = np.get(QStringLiteral("canPause"), m_canPause); bool newCanGoPrevious = np.get(QStringLiteral("canGoPrevious"), m_canGoPrevious); bool newCanGoNext = np.get(QStringLiteral("canGoNext"), m_canGoNext); - - // Check if they changed if (newCanSeek != m_canSeek || newCanPlay != m_canPlay || newCanPause != m_canPause || newCanGoPrevious != m_canGoPrevious || newCanGoNext != m_canGoNext) { + m_canSeek = newCanSeek; + m_canPlay = newCanPlay; + m_canPause = newCanPause; + m_canGoPrevious = newCanGoPrevious; + m_canGoNext = newCanGoNext; Q_EMIT controlsChanged(); } - // Set the new values - m_canSeek = newCanSeek; - m_canPlay = newCanPlay; - m_canPause = newCanPause; - m_canGoPrevious = newCanGoPrevious; - m_canGoNext = newCanGoNext; } long MprisRemotePlayer::position() const @@ -148,11 +141,6 @@ bool MprisRemotePlayer::playing() const return m_playing; } -QString MprisRemotePlayer::nowPlaying() const -{ - return m_nowPlaying; -} - QString MprisRemotePlayer::title() const { return m_title; diff --git a/plugins/mprisremote/mprisremoteplayer.h b/plugins/mprisremote/mprisremoteplayer.h index ae506a19e..fa8cce8fa 100644 --- a/plugins/mprisremote/mprisremoteplayer.h +++ b/plugins/mprisremote/mprisremoteplayer.h @@ -25,7 +25,6 @@ public: int volume() const; long length() const; bool playing() const; - QString nowPlaying() const; QString title() const; QString artist() const; QString album() const; @@ -53,7 +52,6 @@ private: bool m_canPause; bool m_canGoPrevious; bool m_canGoNext; - QString m_nowPlaying; int m_volume; long m_length; long m_lastPosition; diff --git a/plugins/mprisremote/mprisremoteplayermediaplayer2player.cpp b/plugins/mprisremote/mprisremoteplayermediaplayer2player.cpp index 023c08ca7..bdc64a116 100644 --- a/plugins/mprisremote/mprisremoteplayermediaplayer2player.cpp +++ b/plugins/mprisremote/mprisremoteplayermediaplayer2player.cpp @@ -248,7 +248,7 @@ void MprisRemotePlayerMediaPlayer2Player::emitPropertiesChanged() // Send it over the correct DBus connection m_parent->dbus().send(message); - if (m_positionChanged) { + if (m_positionChanged || m_trackInfoChanged) { Q_EMIT Seeked(Position()); } diff --git a/plugins/mprisremote/mprisremoteplugin.cpp b/plugins/mprisremote/mprisremoteplugin.cpp index d0d0be8b2..6631afbe0 100644 --- a/plugins/mprisremote/mprisremoteplugin.cpp +++ b/plugins/mprisremote/mprisremoteplugin.cpp @@ -161,12 +161,6 @@ QStringList MprisRemotePlugin::playerList() const return m_players.keys(); } -QString MprisRemotePlugin::nowPlaying() const -{ - auto player = m_players.value(m_currentPlayer); - return player ? player->nowPlaying() : QString(); -} - QString MprisRemotePlugin::title() const { auto player = m_players.value(m_currentPlayer); diff --git a/plugins/mprisremote/mprisremoteplugin.h b/plugins/mprisremote/mprisremoteplugin.h index acd8cb961..d7723d58d 100644 --- a/plugins/mprisremote/mprisremoteplugin.h +++ b/plugins/mprisremote/mprisremoteplugin.h @@ -26,7 +26,6 @@ class Q_DECL_EXPORT MprisRemotePlugin : public KdeConnectPlugin Q_PROPERTY(int position READ position WRITE setPosition NOTIFY propertiesChanged) Q_PROPERTY(QStringList playerList READ playerList NOTIFY propertiesChanged) Q_PROPERTY(QString player READ player WRITE setPlayer) - Q_PROPERTY(QString nowPlaying READ nowPlaying NOTIFY propertiesChanged) Q_PROPERTY(QString title READ title NOTIFY propertiesChanged) Q_PROPERTY(QString artist READ artist NOTIFY propertiesChanged) Q_PROPERTY(QString album READ album NOTIFY propertiesChanged) @@ -42,7 +41,6 @@ public: bool isPlaying() const; QStringList playerList() const; QString player() const; - QString nowPlaying() const; QString title() const; QString artist() const; QString album() const;