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

Add support for float64 in audresample.remix() #17

Merged
merged 6 commits into from
Feb 2, 2022

Conversation

hagenw
Copy link
Member

@hagenw hagenw commented Feb 2, 2022

Closes #15
Alternative implementation to #16

audresample.remix() works totally fine with other data types than float32, so we don't need to restrict it.

The reason why it was retricted to float32 before was the usage of lib.do_mono_mixdown() from the C library for mixdown, but do we really need to use it?

@codecov
Copy link

codecov bot commented Feb 2, 2022

Codecov Report

Merging #17 (4ad8f8d) into master (9e83740) will not change coverage.
The diff coverage is 100.0%.

Impacted Files Coverage Δ
audresample/core/api.py 100.0% <100.0%> (ø)

@frankenjoe
Copy link
Collaborator

frankenjoe commented Feb 2, 2022

The reason why it was retricted to float32 before was the usage of lib.do_mono_mixdown() from the C library for mixdown, but do we really need to use it?

It was introduced to guarantee identical results with devAIce. But I'm fine with replacing it with a numpy function.

Now that we allow any data type in _check_signal(), did you check what consequences this has on the resample function? Does it work with any data type or do we need to add a check there?

@hagenw
Copy link
Member Author

hagenw commented Feb 2, 2022

The reason why it was retricted to float32 before was the usage of lib.do_mono_mixdown() from the C library for mixdown, but do we really need to use it?

It was introduced to guarantee identical results with devAIce. But I'm fine with replacing it with a numpy function.

Before we had a test that compared the output of the library with the numpy implementation I'm using now, compare

def mixdown(signal):
return np.atleast_2d(np.mean(signal, axis=0))

I now reverted it and use the C library in the tests. So I would assume we still guarantee identical results with devAIce.

@frankenjoe
Copy link
Collaborator

I now reverted it and use the C library in the tests. So I would assume we still guarantee identical results with devAIce.

nice

@frankenjoe frankenjoe merged commit 3f1795a into master Feb 2, 2022
@frankenjoe frankenjoe deleted the remix-support-float64 branch February 2, 2022 15:31
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.

Remix() silently changes dtype to float32
2 participants