-
Notifications
You must be signed in to change notification settings - Fork 60
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
Switch to alternative pesq implementation #27
base: master
Are you sure you want to change the base?
Conversation
Recommended by original author here ludlows/PESQ#12 (comment) |
@jonashaag Thanks for the PR. |
No idea, sorry |
Or simply switch to https://github.com/vBaiCai/python-pesq, which I have worked with in the past and didn’t cause any trouble |
When we added pesq, we considered three implementations:
Since vBaiCai does not support wideband it is not an option that I want to choose. We decided against our own implementation, because of the file system overhead. |
You're right, I just checked, the vBaiCai version has modified PESQ source. Maybe I'll create my own wrapper... I mean the wrapper source codes look really simple |
What are we going to do with this problem? |
I was hoping that https://github.com/ludlows/python-pesq fixes this problem. One solution would be to release our internal code that uses subprocess. |
it is a little complicated to change the code to a thread-safe version which supports multi-threads as well. |
As far as I know, your pesq code is thread safe, because the python GIL is not released. |
yes. but it means the code cannot running in parallel since the GIL exists. |
Yes, subprocess allows to execute the computation in parallel. An another alternative is multiprocessing. Subprosess also handles the problem with the error handling. At least partially. For the error handling it would be nice, when an exception is thrown. That simplifies the bug search |
It's a fork of the original pesq repo that does proper error handling (rather than simply exiting with
exit(1);
from C code).cc FYI @mpariente