Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Implementation proposal for audio resample #1392

Open
AlexFuster opened this issue May 3, 2021 · 6 comments
Open

Implementation proposal for audio resample #1392

AlexFuster opened this issue May 3, 2021 · 6 comments

Comments

@AlexFuster
Copy link

At Voicemod, we have made a tensorflow version of the audio resampling function from torchaudio https://pytorch.org/audio/stable/transforms.html?highlight=resample#torchaudio.transforms.Resample.

I think It would be a good alternative/complement for the current implementation of audio resampling of this repo https://www.tensorflow.org/io/api_docs/python/tfio/audio/resample for 4 reasons.

  1. Supports batches: I know that this was recently added to the current implementation)
  2. Supports gradients: Unlike the current implementation. This feature is very useful if you want to resample inside a training graph
  3. The results are as good as the ones given by the current implementation and they are close to resampy
  4. For some reason, the current implementation always adds a misalignment of size 32 or so, as one can see in the image below
    image

Tell me if you are interested

@yongtang
Copy link
Member

yongtang commented May 3, 2021

@AlexFuster The resample was implemented wit speex in the current repo. If an alternative implementation brings additional advantage (like matching resampy/etc) then we certainly welcome PRs.

@AlexFuster
Copy link
Author

So, do I replace the resample function in audio_ops.py or do I create a separate function?

@yongtang
Copy link
Member

@AlexFuster You can replace the resample function in audio_ops.py and replace C++ kernel in https://github.com/tensorflow/io/blob/master/tensorflow_io/core/kernels/audio_kernels.cc#L220-L299

@AlexFuster
Copy link
Author

AlexFuster commented May 10, 2021

But our implementation is fully in python (using Tensorflow operations). I'll just replace the resample function

@yongtang
Copy link
Member

@AlexFuster Sure. Please go ahead and submit the PR.

We can decide in the PR if the original C++ kernel can be removed or we want to keep them for now.

@AlexFuster
Copy link
Author

Sorry, I forgot to add an import to the first one

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants