Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pipewire: Long timeout stalls user application #820

Open
daschuer opened this issue May 23, 2023 · 4 comments
Open

Pipewire: Long timeout stalls user application #820

daschuer opened this issue May 23, 2023 · 4 comments
Labels
P2 Priority: High src-jack JACK Audio Connection Kit Host API /src/hostapi/jack

Comments

@daschuer
Copy link
Contributor

Describe the bug
In case the audio callback stops because of an other error condition, the WaitCondition() will never happen and the long timeout of 10 min takes place. This is so long that the user will likely kill the application before, loosing his unsaved data.

To Reproduce

  • Start Mixxx Pipwire API
  • Simulate an issue with pipwire by systemctl --user restart pipewire.service
  • The sound stops. (Automatic continue would be nice, but OK)
  • Close Mixxx.

It happens here:

result = WaitCondition( stream->hostApi );

Expected behavior

  • Mixxx should end normally

Actual behavior

  • It crashes after 10 minutes, because assertions are enabled in the Debian package. .

Desktop (please complete the following information):

  • OS: Ubuntu Focal
  • OS Version 20.4
  • PortAudio version: 19.6.0
  • If Windows or Linux, which Host API (e.g. WASAPI): ALSA

Additional context
The original bug is here: mixxxdj/mixxx#11587

Is it possible to lower the timeout to a reasonable short time?
Can we detect the error condition before start waiting?

@RossBencina
Copy link
Collaborator

RossBencina commented May 26, 2023

Is this possibly related to #795

@RossBencina RossBencina added the src-jack JACK Audio Connection Kit Host API /src/hostapi/jack label May 26, 2023
@RossBencina
Copy link
Collaborator

Some of my personal thoughts:

  • 10 mins is too long for anything in PortAudio I think
  • Stick to thinking about JACK and pipewire, not PortAudio in general. Probably for JACK the timeouts can be very short due to JACK low latency requirements
  • WaitCondition is called in multiple places. Not sure the timeout should be the same in all cases.
  • In AbortStream the timeout should be 0 or very short
  • In StopStream an appropriate timeout might be 2x the max(total input stream latency, total output stream latency) [open for discussion] (need to have enough time to (1) drain buffers, and (2) account for the time it takes for a callback to be invoked and respond)

Phil and I agree that 10mins is too long. The question is what is the best value. Ultimately it's up to interested parties to develop a patch.

@philburk
Copy link
Collaborator

It may take a while to decide on the optimal timeout.
So we could just change the 10 minutes to 3 seconds as a quick fix.
It may not be short enough for some cases. For example stopping a stream requires waiting for all the audio to drain out.
That might be more than 3 seconds. But I think we can agree 3 seconds is better than 10 minutes.

@philburk philburk added the P2 Priority: High label May 26, 2023
@RossBencina
Copy link
Collaborator

Having looked at the pa_jack.c code in more detail, i think all of the calls to WaitCondition need to be reviewed as it doesn't seem like pthread_cond_timedwait is being used correctly (i.e. a condition is not always checked).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Priority: High src-jack JACK Audio Connection Kit Host API /src/hostapi/jack
Projects
None yet
Development

No branches or pull requests

3 participants