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

Should discontiguous audio be valid? #1133

Closed
bmcfee opened this issue May 28, 2020 · 1 comment · Fixed by #1140
Closed

Should discontiguous audio be valid? #1133

bmcfee opened this issue May 28, 2020 · 1 comment · Fixed by #1140
Labels
API change Does this change the behavior of existing API? discussion Open-ended discussion for developers and users
Milestone

Comments

@bmcfee
Copy link
Member

bmcfee commented May 28, 2020

Since merging #1125 , we no longer have a strict requirement for the input to util.frame to be contiguous. A discontiguous buffer will be copied to a proper arrangement to support framing.

Relatedly, util.valid_audio requires that the input buffer be F-contiguous to pass as valid. This requirement was originally put in to ensure that framing can operate smoothly if the audio buffer is valid.

The question now is: should we relax the requirement in valid_audio?

Why does this matter: if we want to make a multi-dimensional STFT, it will need to frame a multi-channel input: (2, N) -> (2, NFFT, F). However, soundfile returns C-contiguous buffers, and the whole thing breaks. If we relax the requirement in valid_audio, then the whole thing can proceed as expected, if not quite as efficiently as possible.

Related issue: #1130

@bmcfee bmcfee added discussion Open-ended discussion for developers and users API change Does this change the behavior of existing API? labels May 28, 2020
@bmcfee bmcfee added this to the 0.8.0 milestone May 28, 2020
@bmcfee
Copy link
Member Author

bmcfee commented May 28, 2020

As implied by the PR, I've convinced myself that this requirement is no longer necessary and should be dropped ASAP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API change Does this change the behavior of existing API? discussion Open-ended discussion for developers and users
Development

Successfully merging a pull request may close this issue.

1 participant