-
Notifications
You must be signed in to change notification settings - Fork 3
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
110 review and test voice cloning task #132
Conversation
hi @900miles, can you please review this?
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #132 +/- ##
==========================================
+ Coverage 59.33% 59.43% +0.10%
==========================================
Files 83 84 +1
Lines 2850 2845 -5
==========================================
Hits 1691 1691
+ Misses 1159 1154 -5 ☔ View full report in Codecov by Sentry. |
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.
Looks great! Just a couple comments for clarity.
source_audios (List[Audio]): A list of source audio samples. | ||
target_audios (List[Audio]): A list of target audio samples. | ||
model (SenselabModel, optional): The model to use for voice cloning. | ||
Defaults to TorchModel(path_or_uri="bshall/knn-vc", revision="master"). |
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.
We should specify in the documentation itself that as of now, only KNNVC is supported.
@900miles thank you. i think i have fixed the last comments. feel free to approve the code if you think it's ready to be merged. |
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.
Thanks, just a couple more comments
tutorials/voice_cloning.ipynb
Outdated
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.
The colab link in this file points to the speech-to-text tutorial
This function performs a pairwise voice cloning operation, where each audio in the | ||
`source_audios` list is converted and mapped onto the corresponding audio in the | ||
`target_audios` list. The resulting list contains target audio samples with their | ||
voices replaced by the voices from the corresponding source samples. |
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'm pretty sure this is backwards. The resulting list contains the content of the source audio file with the voice changed to sound like the voice in the corresponding target audio file, right?
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.
nice catch
It should be ready for another review @900miles |
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.
Looks great, thanks @fabiocat93!
PR to address the questions in #110