WIP: Use ffmpeg when sox binary is missing

[hagenw]

Closes #62.

This falls back to ffmpeg if the sox binary is missing on the system as pysox does not ensure the binary to be present.

In addition, it adds tests to make sure the error messages a user gets for broken or missing files are the same if the sox binary is missing or not.

I also updated the installation instructions, highlighting that sox is only needed for faster MP3 file handling:

[codecov]

Codecov Report

Merging #64 (a9fa760) into master (d5c8221) will not change coverage.
The diff coverage is 100.0%.

Impacted Files	Coverage Δ
audiofile/core/convert.py	100.0% <100.0%> (ø)
audiofile/core/info.py	100.0% <100.0%> (ø)
audiofile/core/utils.py	100.0% <100.0%> (ø)

[hagenw]

@frankenjoe in principle the code works, but I cannot make Windows find a program at a custom system path when symlinking it there. For Linux and Mac it works. If I copy the file instead of using symlink it does also not work under Linux.

[frankenjoe]

Maybe the copying solution does not work due to missing dependencies? We probably have to copy some shared libraries as well.

[hagenw]

Locally it works for me when doing it with Unix commands.
In the Github runner it does not fail when executing ffmpeg, but it cannot find it in the path: https://github.com/audeering/audiofile/runs/5756308827?check_suite_focus=true

[frankenjoe]

Did you try with .exe?

[frankenjoe]

According to https://stackoverflow.com/questions/56399696/shutil-which-not-finding-programs-without-appending-file-extension: shutil.which('ffmpeg') will look for ffmpeg.EXE in PATH, but I guess the executable is called ffmpeg.exe.

[hagenw]

I'm not a big fan of skipping the tests. But as we seem to need a fix as soon as possible and we don't have time to first investigate #65 it might be ok for an intermediate release to skip the test.

I will take a look and update the code accordingly.

[hagenw]

On the other hand this problem would disappear when we just remove sox.

I started running the benchmarks with and without sox, let's first see what the outcome will be.

[frankenjoe]

Ok, so we agree to stick with sox but skip the failing tests on Windows, correct?

[hagenw]

I solved it in #69 by using Github runners with and without sox installed. Then we don't need to mess around with the path as we do here.

[frankenjoe]

Hehe, I also thought about using different Github runners, but was afraid to suggest it after you had already spent so much effort on hiding sox from PATH :)

[frankenjoe]

Nice tests! The indirect argument to pass values to fixtures was new to me, very convenient.

[frankenjoe]

Nice tests! The indirect argument to pass values to fixtures was new to me, very convenient.

[hagenw]

Nice tests! The indirect argument to pass values to fixtures was new to me, very convenient.

Hehe, yes, I copied from @ChristianGeng who used it in #56

[frankenjoe]

Looking at the benchmark for reading mp3/mp4, audioread using ffmpeg (?) seems not to be slower than audiofile using sox. If that is true, why do we need sox at all?

Btw: the description says that sox does not support mp3, but I tested it and audiofile is using sox to read mp3.

[hagenw]

sox is more lightweight than ffmpeg and on some systems it supports MP3. I think this was the main motivation to include it.

[hagenw]

For audioread it is not easy to predict what it is using:

In addition, it can be slow at accessing metadata:

So, in general I was not very happy using it.

[hagenw]

sox is more lightweight than ffmpeg and on some systems it supports MP3. I think this was the main motivation to include it.

It might be that it is faster in reading MP3 files and accessing it's metadata than ffmpeg and mediainfo (I cannot remember if I have checked that). We should maybe test that. If it is not the case we might indeed remove sox completely.

[frankenjoe]

So, in general I was not very happy using it.

I'm not suggesting to use audioread, but getting rid of sox and directly fallback on ffmpeg. This also has the advantage that the result does not depend on whether sox is installed or not.

[hagenw]

I created #65 to track the issue of removing sox altogether.

[hagenw]

Looking at the results from #65 (comment) I would be in favor of sticking with sox. To handle the failing test, we first need to merge #68 where I switch to conda for installing sox.

I also updated the installation instructions accordingly and added it to the description of this pull request.

[hagenw]

Closing in favor of #69.