decode mp3 with librosa if torchaudio is > 0.12 as a temporary workaround #4923
Conversation
…fmpeg should be checked too)
|
The documentation is not available anymore as the PR was closed or merged. |
|
Thanks ! Should we still support torchaudio>0.12 if it works ? And if it doesn't we can explain that downgrading is the right solution, or alternatively use librosa |
I'm not sure here, because from the one hand, if |
|
It seems a bit too constraining to not allow users who have a working torchaudio 0.12 setup to not use it. If the issue is about avoiding silent errors if the decoding changes, maybe we can log which back-end is used ? It can even be a warning with performance suggestions ("you're using librosa but torchaudio 0.xx is recommended"). Note that users can still have a requirements.txt or whatever in their projects if they really want full reproducibility (and it's the bare minimum imo) There are multiple possible back-ends so it's maybe not reasonable to only allow one back-end, especially since each back-end has installation constrains and there's no "best" back-end. |
check if it works and fails in a single test
src/datasets/features/audio.py
Outdated
| "`pip install librosa`. Note that decoding will be extremely slow in that case." | ||
| ) from err | ||
| # try to decode with librosa for torchaudio>=0.12.0 as a workaround | ||
| logger.warning("Decoding mp3 with `librosa` instead of `torchaudio`, decoding is slow.") |
There was a problem hiding this comment.
Maybe we should warn this only once ? You can use warnings.warn to do that
There was a problem hiding this comment.
I can't figure out how to do this. I changed logger.warning to warnings.warn here and tried setting warnings.filterwarnings("once") / warnings.simplefilter("once") but it didn't work, What am I missing?
There was a problem hiding this comment.
ah lol I used warnings from logger to check if decoding was done with librosa (as they arrays have the same shapes), and now warnings from .warn are not captured in caplog in pytest, so these tests fail. maybe leave it as is?
There was a problem hiding this comment.
I think it does the warning always once by default, you don't need to use an extra filter.
And I think with pytest.warns(UserWarning): should work in the test
There was a problem hiding this comment.
thanks! fixed it here ebf77d7
I don't know why but for some reason in librosa case warnings are shown at each decoding anyway, I checked it on Colab, see pic. Might it be because of that librosa.load itself gives warnings on decoding: UserWarning: PySoundFile failed. Trying audioread instead.
Anyway, maybe not that important for now? Users can use warnings.filterwarnings("ignore")
There was a problem hiding this comment.
Users can still mute those warnings, I don't think we can do something about it.
Co-authored-by: Quentin Lhoest <42851186+lhoestq@users.noreply.github.com>
…datasets into workaround-torchaudio-0.12
…datasets into workaround-torchaudio-0.12
|
Woohoo all green ! Feel free to merge if it's all good for you :) |
polinaeterna
changed the title
torchaudio>0.12fails with decoding mp3 files ifffmpeg<4. currently we ask users to downgrade torchaudio, but sometimes it's not possible as torchaudio version is binded to torch version. as a temporary workaround we can decode mp3 with librosa (though it 60 times slower, at least it works)another option would be to ask users to install the required version of
ffmpeg, but is non-trivial on colab: it's not in apt packages in ubuntu 18 andcondais not preinstalled (withcondait would be easily installable)see #4776 and #3663 (comment) (there is a Colab notebook to reproduce the error)