feat(volume): Add pulseaudio backend #779

NBonaparte · 2017-09-26T00:15:31Z

~~Provides an option to use PulseAudio instead of ALSA for the volume module, which is determined at compile time. If both are available, it will be compiled with PulseAudio by default.~~
Provides a new PulseAudio module, including an adapter.

The only configuration option currently is sink, which provides the sink to be used as default (name can be found by running pacmd list-sinks). The backend subscribes to any changes in the PulseAudio sinks, so if the sinks change (e.g. headphones are unplugged) and the specified sink disappears, it will automatically revert to the default sink. Similarly, when the specified sink reappears, it will use it instead of the default. It seems like usually there is only one sink, so this option should be optional. The default configuration uses the default sink at all times.

~~Currently, there is no volume cap, so it can go over 100% to an extremely high value, as supported by PulseAudio (not sure whether to limit or leave it to the user).~~
The volume cap is set to PA_VOLUME_UI_MAX, which is the recommended maximum value for UIs.

To-Do after merge:

Add module wiki page
Rename internal/volume to internal/alsa
Deprecate internal/volume

patrick96 · 2017-09-26T19:33:07Z

I have not looked at the code yet, but I think it's great that polybar gets native pulse support. One thing right away: I think it would be better to have a dedicated pulseaudio module instead of cramming the functionality into the same module.
This way people can setup either module without having to recompile everything and also the valid syntax of the volume module would not depend on how polybar was compiled. Also it would break all existing configurations that use the built-in volume module on systems that also have pulse installed, once polybar is recompiled.

I will try to take another look at this on the weekend and give some more feedback.
Thanks!

NBonaparte · 2017-09-26T20:55:10Z

Yeah, I think I made a mess of the build configuration there as well. Since the volume module is essentially the alsa module, I think it would be best to rename it to alsa and deprecate volume (but still link the option internal/volume to internal/alsa for now).

patrick96 · 2017-09-26T20:57:53Z

Yeah, I thought the same thing. And once we push the next major release we can remove the internal/volume module.

NBonaparte · 2017-09-27T02:31:08Z

Should I rename the volume module in this PR or do it separately?

patrick96 · 2017-09-27T08:26:59Z

I think we leave it like it is in this PR and open a separate one, once this one is merged

patrick96

I have now tested this on my machine and it works great.
I think we should cap the volume at 100% because the user can increase the volume with the scroll wheel which can go pretty fast.
There should probably be more log messages as well.
For example warn, whenever the fallback sink is used.

Looking at modules/pulseaudio.cpp, it seems that it shares a lot of code with the alsa module, and I think that is why you wanted them to be the same module in the first place, especially for loading the format-* and label-* settings and in the builder. Would it be feasible to create a sort of abstract volume module that contains all the functionality used by both modules (builder, event handler, common formatting config parameters etc), which both modules call upon?

NBonaparte · 2017-10-29T06:59:07Z

I've added logging, a volume limit, and cleaned up some redundant volume retrieving.

If an abstract module were to be built, it would only have get_output(), get_format(), part of update(), and part of the constructor. Creating one would probably make things too complicated.

patrick96 · 2017-12-16T13:18:05Z

Sorry for the wait on this. This is quite a big PR and I haven't had the time to review it properly. I will try to get in some thourough testing in the next few weeks.

NBonaparte · 2018-01-15T19:52:18Z

Have you had a chance to test this yet?

patrick96 · 2018-01-16T15:48:08Z

I'm sorry, I haven't. My exams start in a week and I'm busy studying. I'll try to fit in some testing on my laptop and desktop on the weekend (nothing too thourough though)

patrick96

I have now tested this on my laptop and desktop and everything seems to work. I also tested it with wired and usb headphones. I only tried it on arch, it could be that other distros may have problems, but I think we can merge this and then handle issues in other distros, as they come in

Breaking Changes: * `0 < label-NAME-maxlen < 3` will now throw an exception and disable the containing module, if ellipsis is enabled for that label. (#1198) Changelog: Deprecations: * `internal/volume` is now called `internal/alsa` (#967) * temperature: The `%temperature%` is deprecated in favor of `%temperature-c%`(#897) * mpd: `icon-repeatone` is deprecated in favor of `icon-single` (#1295), see #1279 Features: * feat(mpd): Add support for icon-consume (#861) * feat(bspwm): Add workspace separator (#942) * feat(i3): Add workspace separator (#938), see #929 * feat(build): Make polybar build on FreeBSD (#931, polybar/xpp#8), see #239 * feat(volume): Add pulseaudio backend (#779) * feat(script): Add %pid% token for tail commands (#934) * feat(temp): Add temperature tokens without unit (#897) * feat(memory): Add memory used/free ramp (#1038), see #1037 * feat(memory): Add swap tokens (#1018) * feat(net): Add unknown-as-up option (#1077), see #457 * feat(config): Support fractional size and offset (#972), see #953 * feat(xwindow): Add label-empty (#1136) * feat(battery): Add animation-discharging (analog to animation-charging) (#1190) * feat(config): Support pixel offset for bar size and offset values (#1224) * feat(mpd): Add `%album-artist%` token (#1263) * feat(net): Add local_ip6 token (#1239), see #1234 * feat(net): Add nl80211 support (#1009), see #277 Fixes: * fix(mpd): Wrong elapsed time when after standby (#921), see #915 * fix(config): Wrong min, maxlen when using the same token multiple times (#974), see #971 * fix(battery): use power_now correctly (#958), see #928 * fix(mpd): Crash when mpd isn't running (#983), see #979 * fix(xworkspaces): Respect 'enable-scroll' (#1002) * fix(xbacklight): Respect 'enable-scroll' (#1014) * fix(build): support xcb-proto >=1.13 (polybar/xpp#11), see #973 * fix(mpd): Respect MPD_HOST env variable (#1025), see #1007 * fix(i3): Reconnect i3 IPC socket on restart/error (#1099), see #762 * fix(cursor): Occasional crash on mouseover (#1124), see #1117 * fix(net): Mark 'not connected' on querying failure (#1171), see #1163 * fix(gcc): Fix -Wstringop-truncation warning (#1216, polybar/i3ipcpp#7), see #1215 * fix(builder): Don't truncate colors with same channels (#1217), see #1183 * fix(bspwm): Consistent behavior when scrolling through multiple desktops (#986), see #981 * fix(builder): Respect label-ellipsis option (#1198), see #1194

patrick96 · 2019-07-18T12:51:52Z

@NBonaparte Why do we check for the pulseaudio binary in addition to the libpulse library?

If we only checked for the library we could change the dependency in the AUR frompulseaudio to only libpulse.

NBonaparte · 2019-07-18T14:45:27Z

You can have libpulse installed without having pulseaudio. For instance, chromium only requires the former, and I tried to run pulseaudio a long time ago before realizing I actually didn’t have the binary installed.

patrick96 · 2019-07-20T16:29:28Z

Ah, I see. But the binary is not necessary for polybar to run?

NBonaparte · 2019-07-28T16:55:12Z

Yeah I suppose. It would be up to the user to ensure that they actually have the binary installed.

NBonaparte added 6 commits September 25, 2017 16:00

wip(pulseaudio): create pulseaudio backend

116e264

fix(build): Only use one volume backend

08c8f5f

refactor(pulseaudio): Move loop to inline function

8a2b5ac

fix(pulseaudio): Use inc_volume() instead of set_volume()

ab2c960

refactor(pulseaudio): Use index instead of name to track sinks

ef0fb44

fix(pulseaudio): Rewrite error checking

078cf1a

NBonaparte changed the title ~~feat(volume): Add pulseaudio backend~~ WIP: feat(volume): Add pulseaudio backend Sep 26, 2017

NBonaparte added 2 commits September 25, 2017 19:35

fix(pulseaudio): fix get_name(), cleanup

6dcb93f

fix(build): Check for pulseaudio binary

569d409

NBonaparte force-pushed the pulseaudio branch from 8e469d6 to 569d409 Compare September 26, 2017 02:38

fix(travis): Add libpulse-dev

077e24c

fix(pulseaudio): Move pulseaudio to separate module

5704693

NBonaparte force-pushed the pulseaudio branch from bd46147 to 5704693 Compare September 26, 2017 22:30

NBonaparte requested a review from patrick96 October 1, 2017 05:01

refactor(pulseaudio): remove wait timeout, fix indents

1447257

patrick96 requested changes Oct 8, 2017

View reviewed changes

NBonaparte mentioned this pull request Oct 10, 2017

The volume module stops to reflect the sound changes when changing an output device #799

Closed

patrick96 mentioned this pull request Oct 12, 2017

two volume modules - is it possible? #796

Closed

NBonaparte added 4 commits October 14, 2017 11:27

refactor(pulseaudio): remove headphone ramp

51963bb

fix(pulseaudio): use PA_VOLUME_UI_MAX

31ccb04

fix(pulseaudio): Add logging

71c8dfb

fix(pulseaudio): prevent nonresponsiveness and remove redundant getters

1d61d67

NBonaparte modified the milestone: 3.1.0 Oct 29, 2017

NBonaparte changed the title ~~WIP: feat(volume): Add pulseaudio backend~~ feat(volume): Add pulseaudio backend Nov 27, 2017

patrick96 added this to the 3.2.0 milestone Dec 3, 2017

fix(pulseaudio): warn only if default sink is not same as specified sink

dc4ff45

NBonaparte force-pushed the pulseaudio branch from ebb2d85 to dc4ff45 Compare December 16, 2017 06:48

NBonaparte requested a review from patrick96 December 16, 2017 06:49

patrick96 approved these changes Jan 20, 2018

View reviewed changes

NBonaparte merged commit 3af3aea into polybar:master Jan 20, 2018

NBonaparte mentioned this pull request Jan 21, 2018

Volume stuck after changing output device #472

Closed

NBonaparte deleted the pulseaudio branch January 27, 2018 05:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(volume): Add pulseaudio backend #779

feat(volume): Add pulseaudio backend #779

NBonaparte commented Sep 26, 2017 •

edited

Loading

patrick96 commented Sep 26, 2017

NBonaparte commented Sep 26, 2017

patrick96 commented Sep 26, 2017

NBonaparte commented Sep 27, 2017

patrick96 commented Sep 27, 2017

patrick96 left a comment

NBonaparte commented Oct 29, 2017

patrick96 commented Dec 16, 2017

NBonaparte commented Jan 15, 2018

patrick96 commented Jan 16, 2018

patrick96 left a comment

patrick96 commented Jul 18, 2019

NBonaparte commented Jul 18, 2019

patrick96 commented Jul 20, 2019

NBonaparte commented Jul 28, 2019

feat(volume): Add pulseaudio backend #779

feat(volume): Add pulseaudio backend #779

Conversation

NBonaparte commented Sep 26, 2017 • edited Loading

patrick96 commented Sep 26, 2017

NBonaparte commented Sep 26, 2017

patrick96 commented Sep 26, 2017

NBonaparte commented Sep 27, 2017

patrick96 commented Sep 27, 2017

patrick96 left a comment

Choose a reason for hiding this comment

NBonaparte commented Oct 29, 2017

patrick96 commented Dec 16, 2017

NBonaparte commented Jan 15, 2018

patrick96 commented Jan 16, 2018

patrick96 left a comment

Choose a reason for hiding this comment

patrick96 commented Jul 18, 2019

NBonaparte commented Jul 18, 2019

patrick96 commented Jul 20, 2019

NBonaparte commented Jul 28, 2019

NBonaparte commented Sep 26, 2017 •

edited

Loading