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

Fix detection of USB disconnect for ALSA devices #348

Closed
wants to merge 1 commit into from
Closed

Fix detection of USB disconnect for ALSA devices #348

wants to merge 1 commit into from

Conversation

agoode
Copy link

@agoode agoode commented May 16, 2018

Without this change, jack gets stuck in a tight poll() loop that it never recovers from. This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1416091.

Without this change, jack gets stuck in a tight poll()
loop that it never recovers from. This fixes
https://bugzilla.redhat.com/show_bug.cgi?id=1416091
@falkTX
Copy link
Member

falkTX commented May 16, 2018

This is something that has been bothering me and I wanted to fix eventually, but have some things to handle first.
So thanks for the fix!
Question.. what does then happen with jack when you remove the soundcard? Do non-audio events still keep happening? Does jack shut down?
I will test this on the weekend anyway.

Last thing, please dont add company names in the ChangeLog.
If you want to, leave your personal name as contributor, not companies.
Thanks.

@agoode
Copy link
Author

agoode commented May 16, 2018

Hi,

First, I am required to add the company name in the patch, as part of my employment:
https://opensource.google.com/docs/patching/#common-rules
Sorry for causing problems with that. If you want to ignore my patch and write a new one yourself, that is fine with me.

Here is what happens with my patch when the USB device goes away:

ALSA: playback device disconnected
JackAudioDriver::ProcessAsync: read error, stopping...
10:18:44.616 JACK connection graph change.
JackPosixProcessSync::LockedTimedWait error usec = 29020 err = Connection timed out
JackEngine::ClientDeactivate wait error ref = 3 name = Pianoteq61
JackPosixProcessSync::LockedTimedWait error usec = 5804 err = Connection timed out
JackEngine::ClientCloseAux wait error ref = 3
10:18:44.796 JACK connection change.
JackPosixProcessSync::LockedTimedWait error usec = 5000000 err = Connection timed out
Driver is not running
Cannot create new client

So the driver stops, but the server keeps running, and I can't reconnect to it. It is still better than using 100% realtime CPU though.

@falkTX
Copy link
Member

falkTX commented May 16, 2018

It is way better than 100% CPU use for sure.
Preferably it would leave the server side in such state that it is possible to stop it cleanly.
Or perhaps we could shutdown jack.

I dont really understand why you have to give company name for personal PRs you do to opensource projects.. are you doing this PR as part of Google, or for yourself?
I dont know about others, but I really do not want to put company names in the list of authors.
Companies are not people.
Feels quite abusive for an employer to require workers to put its name on all their public commits. I am not taking this.
I will patch it myself some time later this week.

Still, your contribution is very appreciated.

@falkTX falkTX closed this May 16, 2018
@agoode
Copy link
Author

agoode commented May 16, 2018

Thanks! I will try out the patch when it's ready and let you know if it works.

For background, Google has two processes for open source patching. The first one (where they retain the copyright) requires no paperwork on my part. The second one (where I get the copyright) requires filling out some forms and takes time. I almost always just use process 1 but sometimes use process 2.

@jackaudio jackaudio deleted a comment from unfa Feb 8, 2021
@jackaudio jackaudio locked as off-topic and limited conversation to collaborators Feb 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants