rbn42 / panon

An Audio Visualizer Widget in KDE Plasma
GNU General Public License v3.0
191 stars 31 forks source link

[BUG] High latency when mixing all speakers #45

Closed yuannan closed 3 years ago

yuannan commented 3 years ago

Desktop (please complete the following information):

Describe the bug image

With variable latency devices such as my USB dac there is a bug with the rendering. When my USB DAC is not used it's latency is changed to the highest it can be. I presume this is to save CPU and power.

Problem is that when using this in "mixing all speakers" mode it causes the bar to stutter really really badly. I presume it's because the code is deadlocked while it's waiting for the buffer. Not sure if there is even a fix possible or just the nature of the Linux audio.

Anyhow I do think it's time this issue is addressed in a different manner. I propose a new separate "smart" single audio device setting. As most of the time users will only be using one device it should be the better alternative to mixing all for most users. The current mixing of all inputs is causing weird bugs like this.

In "smart" audio mode there is 2 processes. One is the current code. The second "watchdog" process is for monitoring which devices are currently in use and actually being played to. This can be achieved quite easily via some commands to see what is actually being played and to what sink/card. pacmd and other packages offer features like this, you probably know much more about this than I do. Along with what devices are used it then checks the latencies and ensures it's below 1/FPS ms. In the case it's above the required latency show a warning triangle so users know something could be wrong or be user configurable to cut the stream completely to ensure a smooth FPS. Other checks such as actually having a none zero audio data should also be done.

The biggest problem I can see with this approach is that certain devices could give off white noise constantly. e.g. microphones and lead to it being the one selected instead. This is however quite easily mitigable with the introduction of a white/blacklist.

This second process is needed as the deadlock is still going to happen (unless you can fix the upstream issue) therefore it must be multithreaded so the watchdog code is not within the current rendering code. This polling rate of the devices in the watchdog process should be user configurable just like FPS currently is. It should try to be in real time but it doesn't need to be so even if it's not fully fixed it's mitigable via user configuration.

The current latency is Σ O(input device) while after this change should bring it down to O(current device). It's not O(watchdog) + O(current device) as the watchdog is another process and pushes the state to the parent process which can get the result in O(1) time. The stutter is completely removed :) This does lead to the issue that the device currently used is not up to date but at most it's out of date by O(watchdog).

The advantages of this approach is that this allows panon to be self healing. Other small visual bugs like the below will also be fixed. Such as when I change audio devices and panon just freezes on the last frame rendered. image

Issue #26 could also fixed for example.

Do you like my very long bug reports with suggestions and extra info? Or do would you prefer I just kept it short? Personally as a dev I would my reports to be like how I've made mine but I understand if you don't :)

rbn42 commented 3 years ago

The biggest problem I can see with this approach is that certain devices could give off white noise constantly. This is however quite easily mitigable with the introduction of a white/blacklist.

I think this design is too complex and people will get confused. I can just blacklist all microphones.

Do you like my very long bug reports with suggestions and extra info?

Your detailed report is very helpful, Thank you.

yuannan commented 3 years ago

I think this design is too complex and people will get confused. I can just blacklist all microphones.

Yeah okay, just leave it out for now. If not you can always add it at a later date :) I'm just trying to plan ahead just in case there are other devices that give off white noise.

rbn42 commented 3 years ago

Since I have only one device, I am not sure whether this commit works.

The logic is

  1. Executing "pacmd list-sinks"
  2. Find the first "name: " with its "state: " == RUNNING.
  3. Then set soundcard's device_id to name+".monitor".

And only doing this when the current device is not producing audio data, for every 0.5 second.

yuannan commented 3 years ago

And only doing this when the current device is not producing audio data, for every 0.5 second.

that works better actually, round robin the devices in cycle instead of a on going cpu drain.

I'll try out your latest commit but in the meantime you can try adding a virtual device for dev use.

https://unix.stackexchange.com/questions/174379/how-can-i-create-a-virtual-output-in-pulseaudio

You can add a sink with

pacmd load-module module-null-sink sink_name=MySink
pacmd update-sink-proplist MySink device.description=MySink

You can add a loopback device with the command

pacmd load-module module-loopback sink=MySink
rbn42 commented 3 years ago

I'll try out your latest commit but in the meantime you can try adding a virtual device for dev use.

Sorry a virtual device is not enough. There is a lot of related knowledge I lacks. For example, I don't know what a real microphone looks like in "pacmd list-sinks". I am not familiar with the structure of PulseAudio and I don't have a real microphone.

Actually, I am making an assumption that "pacmd list-sinks" only prints speakers, no microphones.

My only knowledge is, in python-soundcard, speakers have their ID ending with ".monitor". It seems to be not the case in "pacmd list-sinks"

rbn42 commented 3 years ago

Maybe I should use "pacmd list-sink-inputs" instead of "pacmd list-sinks". But I can't decide when I don't have a real microphone.

yuannan commented 3 years ago

Actually, I am making an assumption that "pacmd list-sinks" only prints speakers, no microphones.

afaik this is true as well.

anyhow I cannot run your latest commit. I think your other commits are bugged. could be commit 8e146fdfeb22fc843b172c54a3581a15a01e9e77 or anything since 4.4 since that works just fine. image

yuannan commented 3 years ago

Maybe I should use "pacmd list-sink-inputs" instead of "pacmd list-sinks". But I can't decide when I don't have a real microphone.

list-sinks-inputs works much better actually. Test it with watch -n .1 pacmd list-sink-inputs

This works per application and will only appear when they are actually running. Should make your task of text extraction a bit easier as well.

I think using "pacmd list-sink-inputs" you can in fact fix mixing all speakers as well. Since it lists what sinks each process uses we know if a sink is not included in the output of "list-sink-inputs" the sink must not be used.

rbn42 commented 3 years ago

Failed to load the visual effect.

I have no idea why this error occurs. Would you mind run test.sh and tell me the result? If this error occurs again, we should be able to find some information there.