-
Notifications
You must be signed in to change notification settings - Fork 732
Add support for 24-bit signed LPCM wav in sox_io backend #1389
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
d53aefc to
803cc7a
Compare
|
@mthrok Let me know what you think! |
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.
Hi @iseessel
Thanks for working on this. It looks good overall.
torchaudio/csrc/sox/utils.cpp
Outdated
| case 24: // Cast 24-bit to 32-bit. That is each data chunk will | ||
| // implicitly perform 8 left-shifts. | ||
| std::cout << "Warning: Casting 24-bit signed PCM to torch::kInt32"; | ||
| return torch::kInt32; |
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 just realized that adding a warning here will issue warning regardless of the value of normalize. Ideally, the warning should be issued only when normalize=False and the input format is 24-bit PCM WAV, but let's keep that from this PR. So, let's simplify the case for 24-bit.
case 24:
case 32:
return torch::kInt32;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.
Would you like me to add warnings to apply_effects_file and apply_effects_fileobj?
803cc7a to
6a8a05f
Compare
|
Changes have been addressed.
|
If you have time and interest, please do so. The following is the direction I think works.
If you have time, please do so. I have filed a task T86709147 as well. |
|
Thanks! |
No description provided.