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: save processed audio in wav instead of pt #382

Merged
merged 2 commits into from
Apr 22, 2024
Merged

Conversation

MENGZHEGENG
Copy link
Collaborator

@MENGZHEGENG MENGZHEGENG commented Apr 11, 2024

@MENGZHEGENG MENGZHEGENG force-pushed the dev.tim/save_wave branch 2 times, most recently from 9fc1d4e to 8929b83 Compare April 12, 2024 20:03
@MENGZHEGENG
Copy link
Collaborator Author

Hi, finally this PR is ready for review :)

Look forward to your feedback!

Copy link

codecov bot commented Apr 12, 2024

The author of this PR, MENGZHEGENG, is not an activated member of this organization on Codecov.
Please activate this user on Codecov to display this PR comment.
Coverage data is still being uploaded to Codecov.io for purposes of overall coverage calculations.
Please don't hesitate to email us at [email protected] with any questions.

Copy link
Contributor

github-actions bot commented Apr 15, 2024

CLI load time: 0:00.30
Pull Request HEAD: 9b8ecf051333b41f86047372a0cee0663986cab0
Imports that take more than 0.1 s:
import time: self [us] | cumulative | imported package

Copy link
Member

@roedoejet roedoejet left a comment

Choose a reason for hiding this comment

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

Fantastic - looks great to me @MENGZHEGENG . Just some very minor changes requested in the submodules. Everything else looks good. I tested preprocessing out on my machine and it worked great. I didn't try training yet.

Could you also update the e2e dataset loader to use .wav and not .pt? https://github.com/roedoejet/EveryVoice/blob/03dca0e270e6cf173c12f8b71bd4139a9aa8db88/everyvoice/model/e2e/dataset.py#L102

@marctessier
Copy link
Collaborator

It looks good Tim.

I di a few extra tests to compared training times using this and the latest version of EV. using 1000 entries of LJ .. Both took ~ 1h2minutes. So it looks like it does not impact the training performance.

Save_wave 11:06 -> 12:08
stock 11:24 —> 12:26

@joanise
Copy link
Member

joanise commented Apr 22, 2024

Small detail: once your PRs are ready, you should rename them to remove the [WIP].

@MENGZHEGENG
Copy link
Collaborator Author

Thanks Eric for the reminder and I've removed the [WIP].

@MENGZHEGENG MENGZHEGENG changed the title [WIP]: fix: save processed audio in wav instead of pt fix: save processed audio in wav instead of pt Apr 22, 2024
Copy link
Member

@roedoejet roedoejet left a comment

Choose a reason for hiding this comment

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

thanks so much Tim, this will be very helpful! great work

Copy link
Member

@joanise joanise left a comment

Choose a reason for hiding this comment

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

Cleaned up into two commits, looks great, thanks!

@MENGZHEGENG
Copy link
Collaborator Author

Thank you so much Eric for your guidances and instructions on cleaning the commits!

@MENGZHEGENG MENGZHEGENG merged commit 23e4c27 into main Apr 22, 2024
4 of 5 checks passed
@MENGZHEGENG MENGZHEGENG deleted the dev.tim/save_wave branch April 22, 2024 19:55
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.

None yet

4 participants