Skip to content
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

fix: change default f0 method from crepe to dio #100

Merged
merged 8 commits into from
Mar 25, 2023

Conversation

34j
Copy link
Collaborator

@34j 34j commented Mar 24, 2023

Closes #86

BREAKING CHANGE: all default values for f0 method changed

**kwargs,
):
with timer() as t:
wav_numpy = wav_numpy.astype(np.float32)
wav_numpy /= np.quantile(np.abs(wav_numpy), 0.999)
if method in ["dio", "harvest"]:
f0 = compute_f0_pyworld(wav_numpy, p_len, sampling_rate, hop_length, method)
elif method == "crepe":
elif method == "dio":
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a typo, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nearly missed it!!!!

@codecov-commenter
Copy link

codecov-commenter commented Mar 24, 2023

Codecov Report

Merging #100 (839dd06) into main (9b9cb6f) will not change coverage.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##             main     #100   +/-   ##
=======================================
  Coverage   16.80%   16.80%           
=======================================
  Files          28       28           
  Lines        3391     3391           
  Branches      393      393           
=======================================
  Hits          570      570           
  Misses       2810     2810           
  Partials       11       11           
Impacted Files Coverage Δ
src/so_vits_svc_fork/__main__.py 0.00% <ø> (ø)
src/so_vits_svc_fork/inference/infer_tool.py 19.16% <ø> (ø)
src/so_vits_svc_fork/inference_main.py 18.57% <ø> (ø)
src/so_vits_svc_fork/preprocess_hubert_f0.py 35.55% <ø> (ø)
src/so_vits_svc_fork/utils.py 17.91% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

BREAKING CHANGE: all default values for f0 method  changed
@34j 34j force-pushed the fix/change-default-f0-method branch from b2819b5 to 24353e6 Compare March 24, 2023 06:09
@34j 34j marked this pull request as draft March 24, 2023 06:10
@34j
Copy link
Collaborator Author

34j commented Mar 24, 2023

I think we should leave the defaults for inference as they were after all since not many files are processed.

@34j
Copy link
Collaborator Author

34j commented Mar 25, 2023

I changed my mind, all the defaults will be dio

@34j 34j marked this pull request as ready for review March 25, 2023 07:39
@34j 34j merged commit baf58d2 into main Mar 25, 2023
@34j 34j deleted the fix/change-default-f0-method branch March 25, 2023 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not use crepe by default because of its performance
3 participants