Review Request 120796: Don't set the volume if PulseAudio is available

[Martin Klapetek]

> On Oct. 25, 2014, 4:18 p.m., Alexander Patrakov wrote:
> > Wrong. You should not test for PulseAudio via dbus, as the git version has module-dbus-protocol removed from the default installation. See http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=dcb52b0844c21c7fe591ef1cfacafbda1df770f0
> 
> Martin Klapetek wrote:
>     The only other way is to check running processes (or any other ideas?). Will see what I can do.
> 
> Alexander Patrakov wrote:
>     The very fact that you need this check indicates a layering violation or a leaky abstraction elsewhere in the stack. See https://bugzilla.gnome.org/show_bug.cgi?id=680779 (which is effectively a WONTFIX given the amount of time it exists).
>     
>     So, I think the correct answer is "just don't check". Checking for a process won't work, especially since you try hard to avoid spawning PulseAudio at this point. I'm afraid that you really have no choice except always assuming that PulseAudio exists together with its flat volumes.
>     
>     Please either drop the "notification volume control" feature completely (even though it would technically be a regression on non-PulseAudio systems) or implement it by attenuating samples in software before sending them to PulseAudio or elsewhere.
>     
>     As for "I cannot reproduce the problem locally", I guess that you are on ubuntu or kubuntu. They disable flat volumes by default in /etc/pulse/daemon.conf for safety reasons, while the upstream default is to enable them. My opinion is that flat volumes are a security issue by themselves, but one PulseAudio developer has a link to a scientific paper (http://www.patrickbaudisch.com/publications/2004-Baudisch-CHI04-FlatVolumeControl.pdf) that he uses for defending them, and I don't really have a good weapon against that.
> 
> Christoph Feck wrote:
>     Martin, do you agree to discard this one? From what I understand, there is no easy way to check if the user runs pulseaudio.

Yes, will do.


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120796/#review69141
-----------------------------------------------------------


On Oct. 26, 2014, 5:21 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120796/
> -----------------------------------------------------------
> 
> (Updated Oct. 26, 2014, 5:21 p.m.)
> 
> 
> Review request for Gardening Team and Christoph Feck.
> 
> 
> Repository: kde-runtime
> 
> 
> Description
> -------
> 
> Simple approach to bug 324975, implementing a suggestion from comment #6. I was however never able to reproduce this locally, so this is "coding blind".
> 
> If this is good-enough, I'll make a similar patch to the kcm to hide the slider in there (which was said in the bug that these should be controllable by the "event sounds" slider).
> 
> (I'm not sure to whom should I assign this so please add people as necessary)
> 
> 
> Diffs
> -----
> 
>   knotify/notifybysound.cpp 0f5cc50 
> 
> Diff: https://git.reviewboard.kde.org/r/120796/diff/
> 
> 
> Testing
> -------
> 
> It builds but I cannot reproduce the problem locally, so cannot really say if it fixes the bug.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-gardening/attachments/20141202/6164d960/attachment.html>