search

PortAudio / portaudio

PortAudio is a cross-platform, open-source C language library for real-time audio input and output.
Other
1.37k stars 286 forks source link

PulseAudio: time is spent perhaps due to insufficient checking of state in PaPulseAudio_CloseStreamCb function #895

Open tatsuki-makino opened 2 months ago

tatsuki-makino commented 2 months ago

Describe the bug It uses about 50 seconds (usleep(10000) * 5000) per stream, probably due to insufficient checking of state in PaPulseAudio_CloseStreamCb function.

To Reproduce

  1. Set up PulseAudio as appropriate.
  2. Build PortAudio with PulseAudio enabled.
  3. Launch Audacity 3.4.2.
  4. Wait until the window appears.

Expected behavior Audacity works smoothly.

Actual behavior The user has to wait about 10 minutes before the window appears. When touching a device associated with PulseAudio, we get the feeling that a glitch is accumulating.

Desktop (please complete the following information):

Additional context A patch of the experiment is attached. Instead of just one state, have the inline function PA_STREAM_IS_GOOD check the condition. However, I do not know if this condition is correct :) It needs to be verified by someone who is familiar with PulseAudio. switch and PA_DEBUG are included to know the state. patch.txt

illuusio commented 2 months ago

Thank you for bringing this up. I have to test this. Could you please open PR for this as it's easier to follow than just having patch..

illuusio commented 2 months ago

Now there is fix mostly based on your patch: #897. Could you please test is doesn't cause any problems.

RossBencina commented 2 months ago

897 has been merged. @tatsuki-makino please confirm that it fixes this issue

tatsuki-makino commented 2 months ago

Thank you for accepting the fix. The code at the commit of db778cf can ostensibly use the input and output from Audacity to PulseAudio without any problems. But, I don't know which states the pa_stream_disconnect and pa_stream_unref can use for streams and which states it can switch to, so I can't say that this is entirely correct :) Well, this issue would have reached a state where it could be closed.

One thing that still bothers me is that if we leave the condition for exiting this loop after 5000 times, even though it no longer occurs, it is likely to cause some kind of resource leak, so it may be better to make it an application error as soon as it reaches 5000 times.

RossBencina commented 2 months ago

it is likely to cause some kind of resource leak

what kind of resource leak, how?

tatsuki-makino commented 2 months ago

If the input and output streams are NULL under this condition, it means that pa_stream_unref has been executed before that. If it repeats 5000 times and escapes the loop, there will be a reference count left. If that happens, we will have to implement it so that it will retry, and I feel that we will not be able to make the decision to retry because we have no way of knowing that it has happened in the first place.

illuusio commented 2 months ago

I've been making it less than 5000 as I don't know does any TCP connection will be available after 10 minutes if it's terminating. It should be like 30 secs max. Probably they should be unref after that loop to make sure that there is no leakage.

illuusio commented 1 month ago

Could you take a look at #914 which should fix rest of this bug.