Add systemvolume plugin
ClosedPublic

Authored by nicolasfella on Sep 26 2017, 10:58 AM.

Details

Summary

This plugin allows controlling the system value from a remote device.

Test Plan

Apply Android patch, open up MPRIS Activity and play with the slider

Diff Detail

Repository
R224 KDE Connect
Branch
arcpatch-D7992_1
Lint
No Linters Available
Unit
No Unit Test Coverage
nicolasfella created this revision.Sep 26 2017, 10:58 AM

You can use pactl list sinks or pacmd info to get the current volume, but then you'll have to parse that information, unfortunately.

davidedmundson added inline comments.
plugins/systemvolume/systemvolumeplugin.cpp
61

You can find Qt pulseaudio code in the plasma-pa repo.

You can use pactl list sinks or pacmd info to get the current volume, but then you'll have to parse that information, unfortunately.

I got a working solution using pactl and some pretty ugly bash-magic. It works with a single audio sink but needs some adjustment to support multiple sinks.

plugins/systemvolume/systemvolumeplugin.cpp
61

Thanks, will take a look at it

Use libpulse. I copied the code from plasma-pa without modifying it. Most of it is unused, but *could* be
useful someday for more advanced features (e.g. switching profiles). I'm not sure how to deal with that. We
could strip down to the needed code, but then we could not apply improvements from plasma-pa easily.

nicolasfella retitled this revision from WIP: Add systemvolume plugin to Add systemvolume plugin.Feb 5 2018, 6:34 PM
nicolasfella edited the summary of this revision. (Show Details)Feb 5 2018, 6:42 PM

As discussed by email, we need to find a way to not duplicate all this code. Apart from that, I really like how it works :)

One small bug I found: if the audio sink changes (eg: from pavucontrol I chose a different output), I have to close the MPRIS activity on Android and open it again for it to refresh. Very minor though, we can merge it unfixed.

Merge branch 'master' into arcpatch-D7992

Fix issue raised by Albert

nicolasfella marked 2 inline comments as done.Mar 17 2018, 2:04 AM

Don't know if this will help but I use two combined plasmoids to achieve this nice effect:


The firs one controls the default sink while the second one change it's volume.

  • Use PulseAudioQt
  • Merge branch 'master' into arcpatch-D7992_1
  • Remove QT_NO_KEYWORDS
mtijink requested changes to this revision.Mar 24 2018, 4:23 PM

Looks good. Is depending on PulseAudioQt okay already?

plugins/systemvolume/systemvolumeplugin.cpp
63

Do we need to check the packet type here? We already know it's destined for this plugin.

70

Needs a check to see if the requested sink still exists.

73

Same here.

94

What happens if we already had that sink? Do we connect to this signal twice?

132

This code doesn't connect to the signals, so that's not correct, right?

This revision now requires changes to proceed.Mar 24 2018, 4:23 PM
  • Only include Plugin if PulseAudioQt is available
mtijink added inline comments.Mar 25 2018, 10:51 AM
plugins/CMakeLists.txt
23

The subdirectory is added twice.

  • Don't include folder twice
  • Merge branch 'master' into arcpatch-D7992_1
  • For real

The code looks good. I'll test once the Android side is done.

  • Namespace changed
  • Adapt to changed API in PulseAudioQt
  • Remove unneeded methode declaration
  • Adapt to proposed changes in pulseaudio-qt
  • Adapt to changes in API
apol added a subscriber: apol.Jul 26 2018, 5:34 PM

What's the status of this?

Restricted Application added a subscriber: kdeconnect. · View Herald TranscriptJul 26 2018, 5:34 PM

Waiting for pulseaudio-qt to be released

albertvaka accepted this revision.Jul 26 2018, 5:47 PM

We don't need to wait the release to merge this patch, since we have the if(KF5PulseAudioQt_FOUND). Let's merge it now and in parallel @apol can help with getting pulseaudio-qt released soon.

This revision was not accepted when it landed; it landed in state Needs Review.Jul 27 2018, 7:22 AM
This revision was automatically updated to reflect the committed changes.