Exposes media sessions to the desktop for use with the MprisRemote plugin and the app.
Still contains some Logs, will remove them when its time.
Details
Details
- Reviewers
mtijink - Group Reviewers
KDE Connect - Commits
- R225:0c6b584d5779: [WIP] Add MprisReceiverPlugin (Android)
What works:
- Displaying and updating title, artist and album.
- PlayPause, next and previous
- Playing state
- multiple sessions
What doesn't work:
- volume control
- length and position
- Album art
- can* abilities are not exposed, e.g. canGoNext, not sure if possible at all
Diff Detail
Diff Detail
- Repository
- R225 KDE Connect - Android application
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
Generally looks quite good, but didn't test it yet.
What doesn't work:
- volume control
Maybe we should expose a canControlVolume on our protocol: even though MPRIS doesn't support it, we can use it ourselves for these cases.
- length and position
- Album art
- can* abilities are not exposed, e.g. canGoNext, not sure if possible at all
Maybe this is useful? https://developer.android.com/reference/android/support/v4/media/session/PlaybackStateCompat
| src/org/kde/kdeconnect/Plugins/MprisReceiverPlugin/MprisReceiverCallback.java | ||
|---|---|---|
| 40 | No final here? | |
| src/org/kde/kdeconnect/Plugins/MprisReceiverPlugin/MprisReceiverPlayer.java | ||
| 96 | I think this can be simplified to 100 * controller.getPlaybackInfo().getCurrentVolume() / controller.getPlaybackInfo().getMaxVolume(), removing all the casts. | |
| src/org/kde/kdeconnect/Plugins/MprisReceiverPlugin/MprisReceiverPlugin.java | ||
| 64 | Best to check if the notifications plugin is enabled and has it's required permissions first. Also, check that in the plugin's requirements. | |
| 153 | If controller get removed, does this get called too? If so, we don't ever remove controllers from the list. | |
| 188 | Don't we have a "pretty name"? | |
| 189 | On the desktop, this sends artist - title (or title if the artist is empty). | |
Comment Actions
- Check for permission
- Simplify logic
- Add final
- Clear players before reloading, include artist in nowPlaying
- Handle missing metadata gracefully
| src/org/kde/kdeconnect/Plugins/MprisReceiverPlugin/MprisReceiverPlugin.java | ||
|---|---|---|
| 188 | Did not find it yet | |
Comment Actions
Sorry, thought of a couple of new things.
I tested it, and there's still an issue: updates (e.g. after next/previous) aren't displayed in KCApp. I don't know on which side the issue is.
Also, it is cool that I can control my pc's music via my phone's KDE Connect, but I think we should blacklist that.
| src/org/kde/kdeconnect/Plugins/MprisReceiverPlugin/MprisReceiverPlayer.java | ||
|---|---|---|
| 44 | What happens otherwise? It defaults to false? | |
| 78 | I think we need to check controller.getMetadata(), it can be null. See https://developer.android.com/reference/android/media/session/MediaController#getMetadata() Also applies elsewhere. | |
| 101 | Can also be null. | |
| src/org/kde/kdeconnect/Plugins/MprisReceiverPlugin/MprisReceiverPlugin.java | ||
| 188 | It looks as if the id is a package name, so we can look up the app name in most cases: https://developer.android.com/reference/android/content/pm/PackageManager.html#getApplicationInfo(java.lang.String,%20int) and https://developer.android.com/reference/android/content/pm/PackageManager.html#getApplicationLabel(android.content.pm.ApplicationInfo) | |
Comment Actions
Did you apply D12528?
Also, it is cool that I can control my pc's music via my phone's KDE Connect, but I think we should blacklist that.
Comment Actions
I didn't, that seems like it should fix the issue (re-opening KCApp did work, after all).
Comment Actions
Regarding the missing features:
I would like to fix the current issues, merge it and add the remaining features in follow-up patches