-
Notifications
You must be signed in to change notification settings - Fork 16
Amend upper frequency limit of mel bands in NoveltySlice to 20kHz #87
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
Conversation
thanks for the heads up! |
happy to change, although the 5k idea of @g-roma seems to make sense too. we could also change the number of mfccs to 20 instead of 13 if we go higher def? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer 13 MFCCs as the default so think this is ready to merge.
The default is also 5k if you read @g-roma right... if we're breaking this, we could try a few options on ranges. Do we really care about 10k to 20k for instance, or sharp changes. 20 bands is also common for music material, etc |
I think it ought to have the same behaviour as our I don't think there's anything to be gained by embarking on a adventure of experiments correcting something that was a mistake to begin with. When we have a separate object for making novelty features, people can freestyle as they please. |
ok let's park optimisation indeed (you all seem to agree so why should I party poop :) |
@weefuzzy this would be ready to merge in right? When you do so, feel free to make an issue of the UT not working and putting my face on it |
fix plotter outlets
closes #86
This is a trivial change, fixing what seems to be a typo in the
NoveltySliceClient
. However, existing example code and stuff you might all be working on that uses novelty slicing with MFCCs will possibly be affected and need re-tuning. Hence a PR rather than just changing it.The expected results on the test will certainly need to change.