From f2e506e059f06de761751a3d82f887d8b2973502 Mon Sep 17 00:00:00 2001 From: Krut Patel Date: Wed, 31 Jul 2024 16:39:28 +0000 Subject: [PATCH] mpris-remote: Support for fetching album art Implementation of sending album art from phone to PC. Complementary MR for the android side: https://invent.kde.org/network/kdeconnect-android/-/merge_requests/353 Fixes: https://bugs.kde.org/show_bug.cgi?id=422136 --- plugins/mprisremote/CMakeLists.txt | 2 + plugins/mprisremote/albumart_cache.cpp | 121 ++++++++++++++++++ plugins/mprisremote/albumart_cache.h | 99 ++++++++++++++ plugins/mprisremote/mprisremoteplayer.cpp | 49 +++++++ plugins/mprisremote/mprisremoteplayer.h | 13 ++ .../mprisremoteplayermediaplayer2player.cpp | 3 + plugins/mprisremote/mprisremoteplugin.cpp | 15 +++ plugins/mprisremote/mprisremoteplugin.h | 2 + 8 files changed, 304 insertions(+) create mode 100644 plugins/mprisremote/albumart_cache.cpp create mode 100644 plugins/mprisremote/albumart_cache.h diff --git a/plugins/mprisremote/CMakeLists.txt b/plugins/mprisremote/CMakeLists.txt index b3be05e09..c106bf8b1 100644 --- a/plugins/mprisremote/CMakeLists.txt +++ b/plugins/mprisremote/CMakeLists.txt @@ -2,6 +2,7 @@ kdeconnect_add_plugin(kdeconnect_mprisremote) target_sources(kdeconnect_mprisremote PRIVATE mprisremoteplugin.cpp + albumart_cache.cpp mprisremoteplayer.cpp mprisremoteplayermediaplayer2.cpp mprisremoteplayermediaplayer2player.cpp @@ -9,4 +10,5 @@ target_sources(kdeconnect_mprisremote PRIVATE target_link_libraries(kdeconnect_mprisremote kdeconnectcore Qt::DBus + Qt::Core # for hash ) diff --git a/plugins/mprisremote/albumart_cache.cpp b/plugins/mprisremote/albumart_cache.cpp new file mode 100644 index 000000000..c72c6acc9 --- /dev/null +++ b/plugins/mprisremote/albumart_cache.cpp @@ -0,0 +1,121 @@ + +#include "albumart_cache.h" + +#include +#include +#include + +#include "filetransferjob.h" +#include "kjob.h" +#include "mprisremoteplugin.h" +#include "plugin_mprisremote_debug.h" + +// TODO: Not sure where to put such utils +constexpr std::size_t operator""_MB(unsigned long long v) +{ + return 1024u * 1024u * v; +} + +static constexpr qsizetype CACHE_SIZE = 5_MB; + +AlbumArtCache::AlbumArtCache() +{ + m_cachedFiles.setMaxCost(CACHE_SIZE); + m_cacheDir = QDir{QStandardPaths::writableLocation(QStandardPaths::CacheLocation).append(QStringLiteral("/kdeconnect/albumart"))}; + if (!m_cacheDir.exists()) { + m_cacheDir.mkpath(QStringLiteral(".")); + } else { + // clear the directory + // TODO: Better thing to do would be to re-populate the m_cachedFiles + qDebug() << "Clearing existing entries" << m_cacheDir.entryList(QDir::Files).size(); + for (auto &file : m_cacheDir.entryList(QDir::Files)) { + m_cacheDir.remove(file); + } + } +} + +AlbumArtCache::~AlbumArtCache() = default; + +AlbumArtCache *AlbumArtCache::instance() +{ + static auto *s_albumArtCache = new AlbumArtCache(); + + return s_albumArtCache; +} + +AlbumArtCache::IndexItem AlbumArtCache::getAlbumArt(const QString &remoteUrl, MprisRemotePlugin *plugin, const QString &player) +{ + if (remoteUrl.isEmpty()) { + // Can't fetch an empty remoteUrl. Do we want to add a separate status for this? + return IndexItem{IndexItem::FAILED}; + } + QReadLocker locker{&instance()->m_cacheLock}; + IndexItem *item = instance()->m_cachedFiles.object(remoteUrl); + if (item != nullptr) { + if (item->fetchStatus == IndexItem::SUCCESS) { + qCDebug(KDECONNECT_PLUGIN_MPRISREMOTE) << "album art already present" << remoteUrl << "at" << item->file->localPath; + if (!item->file->localPath.isLocalFile()) { + qCWarning(KDECONNECT_PLUGIN_MPRISREMOTE) << "No file for cached art!" << item->file->localPath; + return IndexItem{IndexItem::FAILED}; + } + } + return *item; + } else { + // TODO: First check if we are already fetching it + + // fetch the album art from plugin + plugin->requestAlbumArt(player, remoteUrl); + return IndexItem{IndexItem::FETCHING}; + } +} + +void AlbumArtCache::handleAlbumArt(const NetworkPacket &np) +{ + auto remoteUrl = np.get(QStringLiteral("albumArtUrl")); + if (np.payloadSize() > CACHE_SIZE) { + qCWarning(KDECONNECT_PLUGIN_MPRISREMOTE) << "Art is too big! Ignoring."; + return; + } + if (remoteUrl.isEmpty()) { + qCWarning(KDECONNECT_PLUGIN_MPRISREMOTE) << "No url with art"; + return; + } + auto player = np.get(QStringLiteral("player")); + // TODO: Track in-flight requests and reject if not present + { + QReadLocker locker{&instance()->m_cacheLock}; + auto *entry = m_cachedFiles.object(remoteUrl); + if (entry != nullptr) { + if (entry->fetchStatus == IndexItem::SUCCESS) { + qCDebug(KDECONNECT_PLUGIN_MPRISREMOTE) << "Cache hit" << entry->file->localPath.fileName() << "for" << remoteUrl; + Q_EMIT albumArtFetched(player, remoteUrl, entry->file); + return; + } else { + // fetch again + } + } + } + // FIXME: better local file path + auto filename = QStringLiteral("%1.jpg").arg(qHash(remoteUrl)); + auto localUrl = QUrl::fromLocalFile(m_cacheDir.filePath(filename)); + auto *job = np.createPayloadTransferJob(localUrl); + connect(job, &FileTransferJob::result, this, [this, job, remoteUrl, localUrl, player]() { + if (job->error()) { + // TODO: Handle error, retry? + qCWarning(KDECONNECT_PLUGIN_MPRISREMOTE) << "art transfer error" << remoteUrl << "to" << localUrl << job->errorString(); + return; + } + auto fileSize = static_cast(job->totalAmount(KJob::Unit::Bytes)); + qCInfo(KDECONNECT_PLUGIN_MPRISREMOTE) << "Finished art transfer! from" << remoteUrl << "to" << localUrl << "size" << fileSize; + auto *indexItem = new IndexItem{localUrl}; + auto localPath = indexItem->file; + QWriteLocker locker{&instance()->m_cacheLock}; + m_cachedFiles.insert(remoteUrl, indexItem, fileSize); + if (!QFile{localUrl.toLocalFile()}.exists()) { + qCWarning(KDECONNECT_PLUGIN_MPRISREMOTE) << "File doesn't exist" << localUrl; + return; + } + Q_EMIT albumArtFetched(player, remoteUrl, localPath); + }); + job->start(); +} diff --git a/plugins/mprisremote/albumart_cache.h b/plugins/mprisremote/albumart_cache.h new file mode 100644 index 000000000..7b250b7a8 --- /dev/null +++ b/plugins/mprisremote/albumart_cache.h @@ -0,0 +1,99 @@ +/** + * SPDX-FileCopyrightText: 2023 Krut Patel + * + * SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted-GPL + */ + +#ifndef ALBUMARTCACHE_H +#define ALBUMARTCACHE_H + +#include "networkpacket.h" +#include +#include +#include +#include +#include +#include + +class MprisRemotePlugin; + +/** + * Wrapper that automatically deletes the file from disk when destroyed. + */ +class LocalFile +{ +public: + QUrl localPath; + explicit LocalFile(QUrl localPath) + : localPath(std::move(localPath)) + { + } + + ~LocalFile() + { + // delete the file from disk + // TODO: Log warning if this failed + QFile::remove(localPath.toLocalFile()); + } +}; + +class AlbumArtCache : public QObject +{ + Q_OBJECT + + AlbumArtCache(); + + ~AlbumArtCache() override; + +public: + struct IndexItem { + enum Status { + FETCHING, + SUCCESS, + FAILED, + }; + Status fetchStatus; + QSharedPointer file; + + explicit IndexItem(QUrl localPath) + : fetchStatus(Status::SUCCESS) + , file(new LocalFile{std::move(localPath)}) + { + } + explicit IndexItem(Status fetchStatus) + : fetchStatus(fetchStatus) + , file(nullptr) + { + // Need localPath in this case + Q_ASSERT(fetchStatus != Status::SUCCESS); + } + }; + + static AlbumArtCache *instance(); + + /** + * @brief Get the Album Art object. Called from mpris media player to be sent to dbus. + * + * @return IndexItem Current status of the art file. + */ + static IndexItem getAlbumArt(const QString &remoteUrl, MprisRemotePlugin *plugin, const QString &player); + + /** + * @brief Callback for when plugin receives a packet with album art. + * Can't use slot since NetworkPacket isn't a registered type. + */ + void handleAlbumArt(const NetworkPacket &np); + + // called by handleAlbumArt when the album art is fetched and stored to disk +Q_SIGNALS: + + void albumArtFetched(const QString &player, const QString &remoteUrl, QSharedPointer localPath); + +private: + using Index = QCache; + QDir m_cacheDir; + Index m_cachedFiles; + QReadWriteLock m_cacheLock; +}; + +#endif // ALBUMARTCACHE_H diff --git a/plugins/mprisremote/mprisremoteplayer.cpp b/plugins/mprisremote/mprisremoteplayer.cpp index e61ad6262..e92859a1b 100644 --- a/plugins/mprisremote/mprisremoteplayer.cpp +++ b/plugins/mprisremote/mprisremoteplayer.cpp @@ -5,6 +5,7 @@ */ #include "mprisremoteplayer.h" +#include "albumart_cache.h" #include "mprisremoteplayermediaplayer2.h" #include "mprisremoteplayermediaplayer2player.h" #include "mprisremoteplugin.h" @@ -14,6 +15,8 @@ #include #include +#include "plugin_mprisremote_debug.h" + MprisRemotePlayer::MprisRemotePlayer(QString id, MprisRemotePlugin *plugin) : QObject(plugin) , id(id) @@ -33,6 +36,7 @@ MprisRemotePlayer::MprisRemotePlayer(QString id, MprisRemotePlugin *plugin) , m_dbusConnectionName(QStringLiteral("mpris_") + QUuid::createUuid().toString(QUuid::Id128)) , m_dbusConnection(QDBusConnection::connectToBus(QDBusConnection::SessionBus, m_dbusConnectionName)) { + connect(AlbumArtCache::instance(), &AlbumArtCache::albumArtFetched, this, &MprisRemotePlayer::albumArtFetched); // Expose this player on the newly created connection. This allows multiple mpris services in the same Qt process new MprisRemotePlayerMediaPlayer2(this, plugin); new MprisRemotePlayerMediaPlayer2Player(this, plugin); @@ -56,7 +60,17 @@ void MprisRemotePlayer::parseNetworkPacket(const NetworkPacket &np) QString newTitle = np.get(QStringLiteral("title"), m_title); QString newArtist = np.get(QStringLiteral("artist"), m_artist); QString newAlbum = np.get(QStringLiteral("album"), m_album); + QString newAlbumArtUrl = np.get(QStringLiteral("albumArtUrl"), QStringLiteral("")); int newLength = np.get(QStringLiteral("length"), m_length); + if (newAlbumArtUrl != m_albumArtUrl) { + // album art changed + m_albumArtUrl = newAlbumArtUrl; + auto url = AlbumArtCache::getAlbumArt(newAlbumArtUrl, (MprisRemotePlugin *)(parent()), identity()); + // if empty, will be populated once album art has been fetched + if (url.fetchStatus != AlbumArtCache::IndexItem::FETCHING) { + setLocalAlbumArtUrl(url.file); + } + } // Check if they changed if (newTitle != m_title || newArtist != m_artist || newAlbum != m_album || newLength != m_length) { @@ -126,6 +140,13 @@ void MprisRemotePlayer::setPosition(long position) m_lastPositionTime = QDateTime::currentMSecsSinceEpoch(); } +void MprisRemotePlayer::setLocalAlbumArtUrl(const QSharedPointer &url) +{ + m_localAlbumArtUrl = url; + qCDebug(KDECONNECT_PLUGIN_MPRISREMOTE) << "Setting local url" << (url ? url->localPath.toDisplayString() : QStringLiteral("(null)")); + Q_EMIT trackInfoChanged(); +} + int MprisRemotePlayer::volume() const { return m_volume; @@ -191,4 +212,32 @@ QDBusConnection &MprisRemotePlayer::dbus() return m_dbusConnection; } +QString MprisRemotePlayer::albumArtUrl() const +{ + return m_albumArtUrl; +} + +QUrl MprisRemotePlayer::localAlbumArtUrl() const +{ + return m_localAlbumArtUrl ? m_localAlbumArtUrl->localPath : QUrl{}; +} + +void MprisRemotePlayer::albumArtFetched(const QString &player, const QString &remoteUrl, const QSharedPointer &localPath) +{ + if (identity() != player) { + // doesn't concern us + return; + } + if (albumArtUrl() != remoteUrl) { + // doesn't concern us + return; + } + Q_ASSERT(localPath); + if (localAlbumArtUrl() == localPath->localPath) { + // already set + return; + } + setLocalAlbumArtUrl(localPath); +} + #include "moc_mprisremoteplayer.cpp" diff --git a/plugins/mprisremote/mprisremoteplayer.h b/plugins/mprisremote/mprisremoteplayer.h index fa8cce8fa..fd875c674 100644 --- a/plugins/mprisremote/mprisremoteplayer.h +++ b/plugins/mprisremote/mprisremoteplayer.h @@ -5,8 +5,10 @@ */ #pragma once +#include "albumart_cache.h" #include #include +#include class NetworkPacket; class MprisRemotePlugin; @@ -22,12 +24,15 @@ public: void parseNetworkPacket(const NetworkPacket &np); long position() const; void setPosition(long position); + void setLocalAlbumArtUrl(const QSharedPointer &url); int volume() const; long length() const; bool playing() const; QString title() const; QString artist() const; QString album() const; + QString albumArtUrl() const; + QUrl localAlbumArtUrl() const; QString identity() const; bool canSeek() const; @@ -45,6 +50,9 @@ Q_SIGNALS: void volumeChanged(); void playingChanged(); +private: + void albumArtFetched(const QString &player, const QString &remoteUrl, const QSharedPointer &localPath); + private: QString id; bool m_playing; @@ -59,6 +67,11 @@ private: QString m_title; QString m_artist; QString m_album; + QString m_albumArtUrl; + // hold a strong reference so that the file doesn't get deleted while in use + QSharedPointer m_localAlbumArtUrl; + +private: bool m_canSeek; // Use an unique connection for every player, otherwise we can't distinguish which mpris player is being controlled diff --git a/plugins/mprisremote/mprisremoteplayermediaplayer2player.cpp b/plugins/mprisremote/mprisremoteplayermediaplayer2player.cpp index 5c4971d64..0e700c5bb 100644 --- a/plugins/mprisremote/mprisremoteplayermediaplayer2player.cpp +++ b/plugins/mprisremote/mprisremoteplayermediaplayer2player.cpp @@ -58,6 +58,9 @@ QVariantMap MprisRemotePlayerMediaPlayer2Player::Metadata() const if (!m_parent->album().isEmpty()) { metadata[QStringLiteral("xesam:album")] = m_parent->album(); } + if (!m_parent->localAlbumArtUrl().isEmpty()) { + metadata[QStringLiteral("mpris:artUrl")] = m_parent->localAlbumArtUrl().toString(); + } return metadata; } diff --git a/plugins/mprisremote/mprisremoteplugin.cpp b/plugins/mprisremote/mprisremoteplugin.cpp index 5bfe8a56f..c7d8dae63 100644 --- a/plugins/mprisremote/mprisremoteplugin.cpp +++ b/plugins/mprisremote/mprisremoteplugin.cpp @@ -9,9 +9,12 @@ #include #include +#include #include +#include +#include "albumart_cache.h" #include "plugin_mprisremote_debug.h" K_PLUGIN_CLASS_WITH_JSON(MprisRemotePlugin, "kdeconnect_mprisremote.json") @@ -21,6 +24,11 @@ void MprisRemotePlugin::receivePacket(const NetworkPacket &np) if (np.type() != PACKET_TYPE_MPRIS) return; + if (np.get(QStringLiteral("transferringAlbumArt"), false)) { + AlbumArtCache::instance()->handleAlbumArt(np); + return; + } + if (np.has(QStringLiteral("player"))) { const QString player = np.get(QStringLiteral("player")); if (!m_players.contains(player)) { @@ -83,6 +91,13 @@ void MprisRemotePlugin::requestPlayerList() sendPacket(np); } +void MprisRemotePlugin::requestAlbumArt(const QString &player, const QString &album_art_url) +{ + NetworkPacket np(PACKET_TYPE_MPRIS_REQUEST, {{QStringLiteral("player"), player}, {QStringLiteral("albumArtUrl"), album_art_url}}); + qInfo(KDECONNECT_PLUGIN_MPRISREMOTE) << "Requesting album art " << np.serialize(); + sendPacket(np); +} + void MprisRemotePlugin::sendAction(const QString &action) { NetworkPacket np(PACKET_TYPE_MPRIS_REQUEST, {{QStringLiteral("player"), m_currentPlayer}, {QStringLiteral("action"), action}}); diff --git a/plugins/mprisremote/mprisremoteplugin.h b/plugins/mprisremote/mprisremoteplugin.h index 29922f951..1c4025293 100644 --- a/plugins/mprisremote/mprisremoteplugin.h +++ b/plugins/mprisremote/mprisremoteplugin.h @@ -54,6 +54,8 @@ public: Q_SCRIPTABLE void seek(int offset) const; Q_SCRIPTABLE void requestPlayerList(); Q_SCRIPTABLE void sendAction(const QString &action); + // we don't want this to be exposed via dbus, right? + void requestAlbumArt(const QString &player, const QString &album_art_url); Q_SIGNALS: Q_SCRIPTABLE void propertiesChanged();