On Fri, 2014-07-11 at 22:17 +0200, Lukas Kramer wrote: > Hello, > > I've noticed that pavucontrol consistently eats up around 20% of CPU. > This is because pavucontrol updates the level meters on every sink > callback, even when no one's looking (window isn't focused). So I've > created a patch that adds an option to only update the level meters when > the main window is focused. It's certainly a good idea to avoid updating the volume meters when it's unnecessary. I don't like the idea of a command-line switch, though. Avoiding unnecessary UI updates should be the default behaviour. So should you then just drop the command-line switch and leave the patch otherwise as it is? Unfortunately, the answer is no. If the window isn't focused, it doesn't mean that it's not visible, so using the focus state to control the meter updating isn't a good idea in my opinion. I don't think there's any reliable way to figure out whether the window is fully or partially visible or fully covered by some other window, so I think the best solution would be to have a manual toggle in the UI for enabling/disabling volume meters. It could be just a check box above the tabs (not very pretty, but otherwise I think it would be a quite user-friendly solution). Bonus points for the following features: - Instead of just disabling the UI updates, cork the streams when volume meters aren't enabled so that unnecessary audio streaming isn't done. - Cork the streams also when the volume meter is not being shown, e.g. if the volume meter is on a different tab than the currently active tab. - Make the volume meter toggle status persistent, i.e. save the state to disk. Without persistent state, the volume meters should be disabled by default, because it would be very annoying if the user wanted to permanently disable them to save CPU, but then they would be again re-enabled when restarting pavucontrol. > There's one issue with the patch: When a new > stream widget is created and the window is visible but not focused, the > level meter is missing because it's initially hidden. Anyone can explain > why? I don't know, but I can guess: if the volume meter's stream creation fails, it makes no sense to show the volume meter. Not showing the volume meter until the first update is one way to achieve this, but other solutions could be used too (like making the meter visible already when the stream creation has finished successfully, before we have received any audio data). > What are the chances of getting my patch merged? I'm not willing to merge it in the current form. If you implement the check box, I'll be happy to merge it. Please also send properly formatted patches instead of raw diffs, because raw diffs are missing commit messages and authorship information. git send-email is the preferred way to send patches, see http://www.freedesktop.org/wiki/Software/PulseAudio/HowToUseGitSendEmail/ -- Tanu
