xiph / rnnoise

Recurrent neural network for audio noise reduction
BSD 3-Clause "New" or "Revised" License
4.02k stars 890 forks source link

handle non-vla compilers #219

Closed marler8997 closed 6 months ago

marler8997 commented 10 months ago

Leverages the C std version and the standard __STDC_NO_VLA__ macro to detect whether VLA is supported, and if not falls back to using alloca.

This is needed to support the MSVC compiler. With this (and defining M_PI) I was able to compile rnnoise with MSVC.

jmvalin commented 6 months ago

Best solution in this case is to just use fixed sizes. See the upgrade1 branch.

marler8997 commented 6 months ago

Having troubles building the upgrade1 branch. Did you forget to commit src/rnnoise_data.h?

jmvalin commented 6 months ago

rnnoise_data.[ch] are supposed to be automatically downloaded when you run autogen.sh.

marler8997 commented 6 months ago

I'm building on Windows so no autogen.sh, but I was able to manually download and extract them and got further. Looks like are still some type definitions missing, after adding this to rnn_data.h I was able to build:

typedef int rnn_weight;
typedef struct {
    const rnn_weight *bias;
    const rnn_weight *input_weights;
    int nb_inputs;
    int nb_neurons;
    float activation;
} DenseLayer;
typedef struct {
    const rnn_weight *bias;
    const rnn_weight *input_weights;
    const rnn_weight *recurrent_weights;
    int nb_inputs;
    int nb_neurons;
    float activation;
} GRULayer;

This was what I inferred from how these types were being used, but they could be wrong (i.e. I could have nb_inputs and nb_neorons swapped. Do you know why I'm missing these types? Also is upgrade1 expected to be merged to master or is it a permanent branch that we should start releasing from?

jmvalin commented 6 months ago

Actually, rnn_data.[ch] are no longer useful at all. See the Makefile.am file for the new list of files in the build.

jmvalin commented 6 months ago

...and I just pushed some cleanup changes to remove (hopefully) all useless files

marler8997 commented 6 months ago

Cool thanks for the fixes and the help. Looking forward to integrating the recent changes into our project. Would love official support for windows, there's a few PRs around this (one of the CMake PRs would do).

RytoEX commented 6 months ago

Cool thanks for the fixes and the help. Looking forward to integrating the recent changes into our project. Would love official support for windows, there's a few PRs around this (one of the CMake PRs would do).

Echoing this sentiment for CMake support. We at OBS Studio have been building RNNoise on Windows with #88 for years now. If updates are coming to the master branch, that PR would either need updated or replaced. I haven't looked at #188 to see if that's comparable or if it suits our needs.

jmvalin commented 6 months ago

Well, I suggest you give the upgrade1 a try. It's still experimental but it should already have better quality than master. It uses a larger model (hence the download), but if you compile with with AVX2 support (or even just with SSE 4.1) you shouldn't really see a slowdown compared to master. As for CMake support, I don't have time to maintain that, but if someone is willing to spend the time keeping it up-to-date, I'm not against. Could work similarly to what's in Opus where the list of files are kept separately from the autotools/cmake/meson build systems.