jackaudio
Jack exit read if xrun recovery is not recovering the device
This Commit depends on https://github.com/jackaudio/jack2/pull/435
Currently on XRUN, Jack tries a recovery and goes back to read again. But in a case where the device is not recovering at all, Jack is looping continuously taking up 100% cpu as there is no other exit condition.
So with the 37cc0e3daaac0b50b5ebf5a54872391db175b062 patch, Jack will retry for MAX_RECOVERY_RETRY Number of times and exit if the device is unable to recover.
With 37cc0e3daaac0b50b5ebf5a54872391db175b062, We delay xrun recovery if first xrun recovery does not heal the xruns. A delay of RECOVER_DELAY_MS is introduced before start.
I reviewed the source code changes and did not see any issues.
Acked-by: Timo Wischer <[email protected]>
Look okay, though I don't see why we need 2 macros for recovery max attempts.
Can we safely remove the local MAX_RECOVERY_RETRY one?
EDIT: the local one is MAX_RECOVERY_RETRY
Hello @falkTX
Actually these 2 Macros are different in the below sense: MAX_RECOVERY_RETRY : is used for an XRUN . MAX_RETRY_COUNT : This is used for poll timing out.
So since they are used for different reasons, I had kept them separate.
okay, but then please put the 2nd macro together with the other ones in the header file. I find having a macro in the middle of the implementation code to be quite ugly. since you already have a few macros together, adding the other ones there makes sense.
Hello @falkTX ,
@okay, but then please put the 2nd macro together with the other ones in the header file. ->Currently the Macro is in JackAlsaDriver.cpp, So should I move it to JackAlsaDriver.h ? But I don’t see any other Macros being defined there.
The other macro RECOVER_DELAY_MS for poll timeout is defined at the top of alsa_driver.c
Hello @falkTX ,
I have moved the macros from linux/alsa/JackAlsaDriver.cpp to linux/alsa/JackAlsaDriver.h. But there were some macros already present in linux/alsa/alsa_driver.c so I retained the new macro in the same file.
I have merged the PR that this one depended on, leading to conflicts now. Please fix.
This looks clean and I want to merge, but as I wish to push for a new release on the 15th, will hold on to this one for after the release, just in case there are side-effects as we had before with #592