Add torchcrepe #87
Add torchcrepe #87
Conversation
…ut outliers for y axis bounds
conftest.py
Outdated
There was a problem hiding this comment.
Interestingly enough this was the only way I could get pytest to properly discover the tests (microsoft/vscode-python#15062 (comment)).
I understand this is specific to the dev environment and not useful for everyone. So if you prefer I can remove the file from the repo and add it to my local gitignore file.
src/modules/Pitcher/pitcher.py
Outdated
| hop_length = sample_rate // steps_per_second | ||
|
|
||
| # Provide a sensible frequency range for your domain (upper limit is 2006 Hz) | ||
| # TODO: determine appropriate range for vocals |
There was a problem hiding this comment.
Difficult. The record is lower 0,189hz and 25000hz. But typically, it should be around ~87hz to ~1397hz. I think your values are good. Maybe start an issue or discussion? Or maybe adding a flag to open all frequencies?
There was a problem hiding this comment.
I started a discussion to ask for feedback regarding this. What do you think about introducing two cli arguments to leave it up to the user to specify custom bounds?
I.e.:
--crepe_frequency_limit_lower
--crepe_frequency_limit_upper
Or something like that
There was a problem hiding this comment.
I have already found out the hard way the lower bound of 50Hz is creating strange results. I ran the program on a file of me singing "test" and it produced this graph when the range was 50-2006 (albeit with pretty much 0 confidence, I had to allow plotting of confidences below 0.4 to produce it):
With a range from 0-2006 this did not happen and confidence was high were appropriate:
So for now I suggest we enable the entire range.
There was a problem hiding this comment.
Yes lets for now enable the full range and also the add the flags, so more people can test and hopefully give an feedback
…ring plotting if there is no high confidence pitch data
BWagener
force-pushed
the
feature/torchcrepe
branch
from
161f8be to
fbbf29a
Compare
|
I'll close this PR for #89 |
Mainly this PR is for replacing crepe with torchcrepe.
If you don't like the other changes in this PR let me know I can split it up.
Other changes introduced in this PR: