-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
Feature/evaluation #146
base: main
Are you sure you want to change the base?
Feature/evaluation #146
Conversation
# Conflicts: # pytest/modules/UltraStar/test_ultrastar_writer.py # requirements.txt # src/Settings.py # src/UltraSinger.py # src/modules/Audio/separation.py # src/modules/Pitcher/pitcher.py # src/modules/Speech_Recognition/TranscribedData.py # src/modules/Speech_Recognition/Whisper.py # src/modules/Ultrastar/ultrastar_parser.py # src/modules/Ultrastar/ultrastar_txt.py # src/modules/Ultrastar/ultrastar_writer.py
pytest/modules/UltraSinger.py
Outdated
|
||
class PitcherTest(unittest.TestCase): | ||
# @pytest.mark.skip(reason="Skipping this FUNCTION level test, can be used for manual tests") | ||
def test_get_pitch_with_crepe_file(self): |
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.
This test can be removed. PitcherTest class is in test_pitcher.py
import pytest | ||
from src.modules.plot import plot | ||
|
||
|
||
class PitcherTest(unittest.TestCase): | ||
@pytest.mark.skip(reason="Skipping this FUNCTION level test, can be used for manual tests") | ||
# @pytest.mark.skip(reason="Skipping this FUNCTION level test, can be used for manual tests") |
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.
commented skip
@@ -1,6 +1,7 @@ | |||
MIT License | |||
|
|||
Copyright (c) 2023 Vadim Rangnau | |||
Copyright (c) 2020 Max Morrison (torchcrepe code adapted for crepe output filtering abd thresholding) |
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.
maybe fork crepe an make a separate package for it? Or PR the changes to the crepe project?
pytest/modules/UltraSinger.py
Outdated
from src.modules.plot import plot | ||
|
||
|
||
class PitcherTest(unittest.TestCase): |
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.
This is also in test_pitcher.py
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.
resolved
@@ -1,6 +1,7 @@ | |||
MIT License | |||
|
|||
Copyright (c) 2023 Vadim Rangnau | |||
Copyright (c) 2020 Max Morrison (torchcrepe code adapted for crepe output filtering abd thresholding) |
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 not fork crepe, move the changes there and release an package?
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 wasn't sure how to give proper credit here. I would say it's up to you how to handle this and if you want to accept the code in question.
The code here https://github.com/rakuri255/UltraSinger/pull/146/files#diff-7bda13ea2689179c7952b68174b8b8ea2cc2250f9b9699c5cf55939fb6c4ac7d is copied and adapted from here https://github.com/maxrmorrison/torchcrepe/blob/master/torchcrepe/loudness.py
@@ -881,6 +1024,19 @@ def denoise_vocal_audio(input_path: str, output_path: str) -> None: | |||
"""Denoise vocal audio""" | |||
ffmpeg_reduce_noise(input_path, output_path) | |||
|
|||
# Fixme: Merge issue |
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.
fixme
src/modules/console_colors.py
Outdated
@@ -8,6 +8,11 @@ def blue_highlighted(text: str) -> str: | |||
return f"{Bcolors.blue}{text}{Bcolors.endc}" | |||
|
|||
|
|||
def green_highlighted(text: str) -> str: | |||
"""Returns a blue highlighted text""" |
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.
Commend says blue
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.
resolved
FILE_ENCODING, | ||
) | ||
|
||
CHARACTERS_TO_REMOVE = ["\ufeff"] |
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.
Is this "BOM"? Than we should name it "BOM"
ultrastar_class = UltrastarTxtValue() | ||
count = 0 | ||
|
||
# Strips the newline character | ||
for line in txt: | ||
filtered_line = line | ||
for character_to_remove in CHARACTERS_TO_REMOVE: |
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 remove in each line? Isnt this just an encoding issue?
|
||
@dataclass_json | ||
@dataclass | ||
class TranscriptionResult: |
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.
maybe just Transcription with data and detected_language?
# Conflicts: # src/UltraSinger.py # src/modules/Speech_Recognition/TranscribedData.py # src/modules/plot.py
start_time = ( | ||
beat_to_second(int(ultrastar_class.startBeat[pos]), real_bpm) + gap | ||
) | ||
gap = int(float(ultrastar_class.gap.replace(",", ".")) / 1000) |
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.
This breaks the score calculation
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.
resolved
real_bpm = ultrastar_bpm_to_real_bpm( | ||
float(ultrastar_class.bpm.replace(",", ".")) | ||
) | ||
gap = int(float(ultrastar_class.gap.replace(",", ".")) / 1000) |
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.
This breaks the score calculation
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.
good catch.
another contributing factor to broken score calculation was the wrong usage of the constructor here:
76a4956#diff-f8287eea965b098ba5cef0e0b678bd57ea9d41c3985c3a311c940792de15c57fR987
Calling the constructor like this
new_transcribed_data.append(TranscribedData({"word": midi_segment.word, "start": midi_segment.start, "end": midi_segment.end, "is_hyphen": None, "confidence": 1}))
just initializes the confidence field to have the map as it's value and all other fields will have their default value. So all start and end values are 0 from then on.
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.
resolved
No description provided.