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
- 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
- Repository
- R225 KDE Connect - Android application
- Branch
- mediacontroller
- Lint
No Linters Available - Unit
No Unit Test Coverage
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 | ||
---|---|---|
39 ↗ | (On Diff #33187) | No final here? |
src/org/kde/kdeconnect/Plugins/MprisReceiverPlugin/MprisReceiverPlayer.java | ||
95 ↗ | (On Diff #33187) | 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 | ||
63 ↗ | (On Diff #33187) | Best to check if the notifications plugin is enabled and has it's required permissions first. Also, check that in the plugin's requirements. |
152 ↗ | (On Diff #33187) | If controller get removed, does this get called too? If so, we don't ever remove controllers from the list. |
187 ↗ | (On Diff #33187) | Don't we have a "pretty name"? |
188 ↗ | (On Diff #33187) | On the desktop, this sends artist - title (or title if the artist is empty). |
- 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 | ||
---|---|---|
187 ↗ | (On Diff #33187) | Did not find it yet |
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 | ||
---|---|---|
43 | What happens otherwise? It defaults to false? | |
77 | 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. | |
100 | Can also be null. | |
src/org/kde/kdeconnect/Plugins/MprisReceiverPlugin/MprisReceiverPlugin.java | ||
187 ↗ | (On Diff #33187) | 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) |
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.
I didn't, that seems like it should fix the issue (re-opening KCApp did work, after all).
Regarding the missing features:
I would like to fix the current issues, merge it and add the remaining features in follow-up patches