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

Librosa backend #282

Merged
merged 10 commits into from
Mar 20, 2020
Merged

Conversation

alreadytaikeune
Copy link
Collaborator

@alreadytaikeune alreadytaikeune commented Feb 27, 2020

[Spleeter-282] - Using librosa as an alternative backend for stft computation

Description

Tensorflow's implementation of stft and istft on CPU is extremely slow and glutton in memory. I implemented the possibility to defer these computations to librosa which results in faster inference on CPU and vastly diminished memory usage. The use of the backend can be controlled by an additional parameter on the command line, which defaults to the following logic:

  • GPU acceleration available -> tensorflow backend for stfts
  • GPU acceleration not available -> librosa backend for stfts.

How this patch was tested

Ran the separate commands using all the different backend options and checked the consistencies of the outputs as well as the memory use.

akhlif added 6 commits February 19, 2020 10:55
Changes in the EstimatorBuilder to set attributes instead of returning tensors for the _build methods.
InputProvider classes to handle the different backend cases.
New method in Separator.
@mmoussallam
Copy link
Collaborator

  • bump the spleeter version in dockerfiles
  • bump the tensorflow lib version to 1.15
  • remove duplicate logs

@jaolan
Copy link

jaolan commented Mar 11, 2020

I tried to test your Librosa backend implementation, but it seems like the -w argument is unrecognized. How are you using the new stft_backend flag on the command line?

@alreadytaikeune
Copy link
Collaborator Author

Hi @jaolan, the option is -B or --stft-backend. You can pass "tensorflow", "librosa", or "auto" as parameter.

@jaolan
Copy link

jaolan commented Mar 17, 2020

Hi @alreadytaikeune, I got it working, but with an unexpected key for the 'waveform' entry in input dict:

ValueError: Got unexpected keys in input_dict: {'waveform'}
expected: {'audio_id', 'mix_spectrogram', 'mix_stft'}

@alreadytaikeune
Copy link
Collaborator Author

Hi @jaolan can you show me a code snippet to reproduce the error? Thank's a lot.

@jaolan
Copy link

jaolan commented Mar 18, 2020

Hi @jaolan can you show me a code snippet to reproduce the error? Thank's a lot.

This is the output I get upon trying 2 or 4 stems. Thanks for your help!
image

@alreadytaikeune
Copy link
Collaborator Author

Hi @jaolan I can't figure out which version of the separator.py file you are using because your stack trace doesn't seem to correspond to the current version (commit 3cba6985f410750f9704303668e4093dde29f7a3) of the script. Can you show me the content of your separator.py? Thanks

@jaolan
Copy link

jaolan commented Mar 18, 2020

Hi @alreadytaikeune , here is the separator.py code: https://gist.github.com/jaolan/c7c5be3dc2d3d5a4090dc65d67e3e165

@alreadytaikeune
Copy link
Collaborator Author

I don't have the rights to access this.

@alreadytaikeune alreadytaikeune merged commit 243b323 into deezer:master Mar 20, 2020
@jaolan
Copy link

jaolan commented Mar 25, 2020

I don't have the rights to access this.

Apologies, everything is working as intended after merge to master. Thanks!

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.

3 participants