-
-
Notifications
You must be signed in to change notification settings - Fork 48
refactor(pitcher): replace crepe with torchcrepe for pitch detection #249
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
base: main
Are you sure you want to change the base?
Conversation
This commit replaces the crepe library with torchcrepe for pitch detection, improving performance and simplifying dependencies. The changes include updating the pitching logic, modifying configuration settings, and removing tensorflow-related code. Additionally, the commit includes various bug fixes and improvements to error handling in the pitch detection and MIDI note creation processes.
Extracted helper functions to improve code readability and maintainability. This includes functions for finding pitch indices, extracting pitch data, filtering confident frequencies, converting frequencies to notes, determining the most common note, and converting notes to MIDI numbers. The main `create_midi_note_from_pitched_data` function now delegates to these helper functions, making the logic clearer and easier to follow.
|
NICE!! |
Remove Tensorflow installation in dockerfile
| f"{UltrastarTxtNoteTypeTag.GOLDEN} ", | ||
| f"{UltrastarTxtNoteTypeTag.RAP} ", | ||
| f"{UltrastarTxtNoteTypeTag.RAP_GOLDEN} ", | ||
| ": ", "* ", "F ", "R ", "G " |
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.
Why the replace here?
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.
idk why sometime the result still have the UltrastarTxtNoteTypeTag
| # 'n3' pitch where 0 == C4 | ||
| # 'w' lyric | ||
| line = f"{UltrastarTxtNoteTypeTag.NORMAL} " \ | ||
| line = f": " \ |
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.
Here also. Why not using the enum?
|
I do some error's I will correct it |
|
I tested against crepe with new song from yt. I don't know why, but I get worse points. Crepe | Score: total: 9077, notes: 8260 line: 817, golden: 0 |
|
I attempted the same in this PR #87 |
|
@BWagener the main approche here is to get rid of Tensorflow. But this PR has a problem with the accuracy. |
|
It would be nice if this could get figured out - getting rid of Tensorflow was my motivation back then as well ^^ |
|
Yes |
This commit replaces the crepe library with torchcrepe for pitch detection, improving performance and simplifying dependencies. The changes include updating the pitching logic, modifying configuration settings, and removing tensorflow-related code. Additionally, the commit includes various bug fixes and improvements to error handling in the pitch detection and MIDI note creation processes.