-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Add Encodec features for Codec toolkit #5758
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
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Since it is mixed with the current decoding and scoring PR, I will review it again after we first merge that one. Thanks for the in-time PR! |
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 for your contribution! In general, it looks great. Please check my minor comments~
espnet2/gan_codec/encodec/encodec.py
Outdated
"fs": 24000, | ||
"n_fft": 1024, | ||
"hop_length": 256, | ||
"win_length": None, | ||
"window": "hann", |
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.
You might consider to follow the new setup for multi-scale mel loss?
espnet2/gan_codec/encodec/encodec.py
Outdated
use_loss_balancer: bool = False, | ||
balance_ema_decay: float = 0.99, | ||
): | ||
# (Jinchuan) re-apply everything except the discriminator config. |
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.
One TODO here, to add the docstring
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.
For the note consistency, please use:
# NOTE(jincuan): ...
Also for TODO
) | ||
|
||
|
||
def apply_parametrization_norm(module: nn.Module, norm: str = "none"): |
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.
For consistency, I think many of the modules are already in shared
, could you please double check in case?
Thanks for your contribution! Looks great to me. |
What?
Add several Encodec features into the codec toolkit: https://arxiv.org/abs/2210.13438
@ftshijt