Differential D26097

Use URL as title and album in MPRIS plugin
ClosedPublic
Actions

Authored by ashka on Dec 19 2019, 4:14 AM.

Details

Reviewers
nicolasfella
Group Reviewers
KDE Connect
Commits
R224:8aca11e102a9: Use URL as title and album in MPRIS plugin
Summary

The current behavior in plasma-workspace's mediacontroller is to get the
title and the album info from the URL contained in the MPRIS metadata,
if it is dealing with a local file and missing title and artist. This
commit aligns the behavior to the one mediacontroller has.

Diff Detail

Repository
R224 KDE Connect
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ashka created this revision.Dec 19 2019, 4:14 AM
Restricted Application added a project: KDE Connect. · View Herald TranscriptDec 19 2019, 4:14 AM
Restricted Application added a subscriber: kdeconnect. · View Herald Transcript
ashka requested review of this revision.Dec 19 2019, 4:14 AM
nicolasfella requested changes to this revision.EditedDec 19 2019, 3:19 PM
nicolasfella added a subscriber: nicolasfella.

Thanks for the patch!

There is a minor problem with this approach.

You are checking if the URL starts with "file:///" and then do QUrl::fromLocalFile on it. The docs say that fromLocalFile expects the input to have no scheme (see https://doc.qt.io/qt-5/qurl.html#fromLocalFile). Is there a particular reason to restrict it to file:// URLs? I can imagine that doing the same for e.g. sftp:// urls could be useful as well.

plugins/mpriscontrol/mpriscontrolplugin.cpp
364

You can convert it into a QUrl directly:
QUrl fileUrl = nowPlayingMap[QStringLiteral("xesam:url")].toUrl();

369

For the title you can use fileUrl.fileName()

This revision now requires changes to proceed.Dec 19 2019, 3:19 PM
ashka added a comment.EditedDec 19 2019, 3:49 PM

Thanks for the review. I had two reasons in mind for restricting to file:///:

Not restricting would mean having to select schemes that would be allowed to be parsed as title/album.

ashka updated this revision to Diff 71841.Dec 19 2019, 3:49 PM
  • Use QUrl directly for file URL
nicolasfella accepted this revision.Dec 19 2019, 5:08 PM
This revision is now accepted and ready to land.Dec 19 2019, 5:08 PM
Closed by commit R224:8aca11e102a9: Use URL as title and album in MPRIS plugin (authored by ashka, committed by nicolasfella). · Explain WhyDec 19 2019, 5:10 PM
This revision was automatically updated to reflect the committed changes.
nicolasfella added a comment.Dec 19 2019, 5:11 PM

BTW, we are moving to gitlab so please submit your future patches at https://invent.kde.org/kde/kdeconnect-kde

Revision Contents
Changeset List

Diff 71848

View Options

plugins/mpriscontrol/mpriscontrolplugin.cpp

Loading...