search

WhyNotHugo / caffeine-ng

⚠ This project has migrated to codeberg.org
https://codeberg.org/WhyNotHugo/caffeine-ng
GNU General Public License v3.0
180 stars 21 forks source link

fix #75: always blacklisting 'peak detect' to fix caffeine-ng on PipeWire #77

Closed carlocastoldi closed 3 years ago

carlocastoldi commented 3 years ago

Hi! With this pull request i want to fix #75

On PipeWire there are a lot more detectable sources & sinks than with PulseAudio: every time some process peaks for volume playback, that same process will be seen as a recording stream aswell. For this reason calling pulseaudio.get_peak_sample() created a momentary process with name="peak detect" that was then destroyed when we analysed it. I thus added a check on recording sources's name to avoid checking on the "peak detector".

Note: with current implementation, plasmashell is seen as a recording process with PipeWire. That is because the Plasma integrates into its volume mixer a colorbar showing the application/device activity volume.

DEBUG:caffeine.triggers:Video playback detected (spotify, plasmashell, plasmashell, plasmashell, plasmashell, plasmashell, plasmashell, plasmashell, plasmashell).

In my setup I fixed this by simply blacklisting plasmashell, but it may be worth it hardcoding it / warn somewhere of this "feature" or removing audio recording detection all together (or moving it to a whitelist rather than a blacklist?)

Additionally: i increased the peak sampling time from 0.1 to 0.4 seconds because otherwise, with music playing at relatively low volume, it wouldn't detect any activity

VoodaGod commented 3 years ago

i noticed something was going wrong with caffeine (amongst other things) when i got upgraded to pipewire, but i reverted to pulseaudio. thanks for figuring it out! will have to test this when i try pipewire again.

what do you think about a "ignore recording" toggle? come to think of it, audio detection in general should have a toggle in the GUI, too

carlocastoldi commented 3 years ago

thanks for figuring it out!

It bugged me so much, and it's crazy how small of a change it was needed hahaha

what do you think about a "ignore recording" toggle?

Mmmh that could work as well, but it must be said explicitly WHY that is an option (i.e. because some desktop environment may interfere), and still i would see it as something that users would struggle on understanding why their system is getting inhibited. I think I have a propensity more for either blacklisting known peaking programs or just removing recording peaking altogether.

come to think of it, audio detection in general should have a toggle in the GUI, too

There is such toggle, i think. I just tried it, but I think it doesn't work :sweat_smile: immagine

VoodaGod commented 3 years ago

That "enable audio peak filtering" toggle is just for checking, if a source/sink actually has any volume peaks, because some programs don't close them & get registered as playing/recording audio even when they're not. A tooltip explaining what exactly it does should be added i think. Maybe a better name for the button would be "ignore silent processes" or something like that. I believe you can only disable audio detection by command line "--no-pulseaudio", but i don't think that hides the audio detection GUI at all. Would make sense to connect "enable audio detection" button to that CLI parameter & a setting. There's another parameter "--no-fullscreen" that would make sense to have as a toggle button under "Automatic activation"

WhyNotHugo commented 3 years ago

 Thanks for figuring this one out.

A blacklist of processes to ignore would also be a welcome change. I don't think they need to be configurable: hardcoding them is fine, since we intend to exclude things that don't make sense for caffeine's use-case.

It's also my bad that pulseaudio is a cli flag. It would make a lot of sense for it to be a setting like everything else. That was my bad.

/merge